From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id E53FE6947 for ; Tue, 25 Feb 2014 01:57:02 +0100 (CET) Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga101.jf.intel.com with ESMTP; 24 Feb 2014 16:58:27 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.97,538,1389772800"; d="scan'208";a="461604362" Received: from irsmsx103.ger.corp.intel.com ([163.33.3.157]) by orsmga001.jf.intel.com with ESMTP; 24 Feb 2014 16:58:03 -0800 Received: from irsmsx153.ger.corp.intel.com (163.33.192.75) by IRSMSX103.ger.corp.intel.com (163.33.3.157) with Microsoft SMTP Server (TLS) id 14.3.123.3; Tue, 25 Feb 2014 00:58:02 +0000 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.202]) by IRSMSX153.ger.corp.intel.com ([169.254.9.196]) with mapi id 14.03.0123.003; Tue, 25 Feb 2014 00:57:52 +0000 From: "Ananyev, Konstantin" To: didier.pallard Thread-Topic: [dpdk-dev] [RFC PATCH 2/2] ixgbe: release software locked semaphores on initialization Thread-Index: AQHPLZtbY9N48dhGHkyTBenuV2sl2Jq/5h9ggASVYwCAALCqkA== Date: Tue, 25 Feb 2014 00:57:48 +0000 Message-ID: <2601191342CEEE43887BDE71AB97725808E6AA49@IRSMSX105.ger.corp.intel.com> References: <1392811162-28527-1-git-send-email-didier.pallard@6wind.com> <1392811162-28527-2-git-send-email-didier.pallard@6wind.com> <2601191342CEEE43887BDE71AB97725808E689E6@IRSMSX105.ger.corp.intel.com> <201402191752.11989.thomas.monjalon@6wind.com> <2601191342CEEE43887BDE71AB97725808E68AFB@IRSMSX105.ger.corp.intel.com> <2601191342CEEE43887BDE71AB97725808E692BE@IRSMSX105.ger.corp.intel.com> <530B54D4.5050407@6wind.com> In-Reply-To: <530B54D4.5050407@6wind.com> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [163.33.239.181] Content-Type: text/plain; charset="iso-8859-1" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [RFC PATCH 2/2] ixgbe: release software locked semaphores on initialization X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 25 Feb 2014 00:57:03 -0000 Hi, About e1000 - I suppose you refer to the eth_igb_dev_init()->e1000_setup_in= it_funcs(hw, TRUE)-> e1000_init_nvm_params(hw)? If so, I suppose we can do something like that to overcome it: e1000_setup_init_funcs(hw, FALSE); e1000_reset_swfw_lock(hw); e1000_setup_init_funcs(hw, TRUE); ? First setup_init_funcs() would just setup hw func pointers and wouldn't cal= l e1000_init_nvm_params/ e1000_init_phy_params. Then we reset the lock, then call setup_funcs once again - that time it wou= ld read/write HW regs. Konstantin -----Original Message----- From: didier.pallard [mailto:didier.pallard@6wind.com] = Sent: Monday, February 24, 2014 2:19 PM To: Ananyev, Konstantin Cc: Thomas Monjalon; dev@dpdk.org Subject: Re: [dpdk-dev] [RFC PATCH 2/2] ixgbe: release software locked sema= phores on initialization Hi, The patch (or some derivative that do the same result) should probably rath= er be integrated in base driver. For IGB implementation, it is not possible to extract patch from base drive= r, since lock release should be done before calls to e1000_init_nvm_params = or e1000_init_phy_params that use the potentially stuck locks and after eno= ugh function pointers fields are filled by e1000_setup_init_funcs to have f= unctions to access the hardware. For ixgbe, it may be possible on 82598/82599 using ixgbe_xxx_swfw_semaphore= to do the job from outside the base driver, assuming that no lock will nev= er be taken by the base driver before the return of ixgbe_init_shared_code = function. But it is not be possible on X540, since this implementation does= not use the ixgbe_get_eeprom_semaphore generic function that automatically= release the SMBI lock on timeout; So the release of this lock should be do= ne using ixgbe_release_swfw_sync_semaphore that is not accessible through t= he API. didier On 02/21/2014 05:30 PM, Ananyev, Konstantin wrote: > To be more specific, I am talking about something like the patch below. > It is just a copy-paste of your fix, but implemented and called from = > ixgbe_ethdev.c Konstantin > > diff --git a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c = > b/lib/librte_pmd_ixgbe/ixgbe_et hdev.c index 7e068c2..5d8744a 100644 > --- a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c > +++ b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c > @@ -561,6 +561,42 @@ ixgbe_dcb_init(struct ixgbe_hw *hw,struct = > ixgbe_dcb_config > *dcb_config) > } > } > > +static void > +ixgbe_swfw_lock_reset(struct ixgbe_hw *hw) { > + uint16_t mask; > + > + /* Get bus info */ > + hw->mac.ops.get_bus_info(hw); > + > + /* Ensure that all locks are released before first NVM or PHY = > + access */ > + > + /* > + * Phy lock should not fail in this early stage. If this is the c= ase, > + * it is due to an improper exit of the application. > + * So force the release of the faulty lock. Release of common lock > + * is done automatically by swfw_sync function. > + */ > + mask =3D IXGBE_GSSR_PHY0_SM << hw->bus.func; > + if (hw->mac.ops.acquire_swfw_sync(hw, mask) < 0) { > + DEBUGOUT1("SWFW phy%d lock released", hw->bus.func); > + } > + hw->mac.ops.release_swfw_sync(hw, mask); > + > + /* > + * Those one are more tricky since they are common to all ports; = but > + * swfw_sync retries last long enough (1s) to be almost sure that= if > + * lock can not be taken it is due to an improper lock of the > + * semaphore. > + */ > + mask =3D IXGBE_GSSR_EEP_SM | IXGBE_GSSR_MAC_CSR_SM | IXGBE_GSSR_S= W_MNG_SM; > + if (hw->mac.ops.acquire_swfw_sync(hw, mask) < 0) { > + DEBUGOUT("SWFW common locks released"); > + } > + hw->mac.ops.release_swfw_sync(hw, mask); } > + > + > /* > * This function is based on code in ixgbe_attach() in ixgbe/ixgbe.c. > * It returns 0 on success. > @@ -618,6 +654,11 @@ eth_ixgbe_dev_init(__attribute__((unused)) struct et= h_driver *eth_drv, > return -EIO; > } > > + if (hw->mac.type =3D=3D ixgbe_mac_82598EB || > + hw->mac.type =3D=3D ixgbe_mac_82599EB || > + hw->mac.type =3D=3D ixgbe_mac_X540) > + ixgbe_swfw_lock_reset(hw); > + > /* Initialize DCB configuration*/ > memset(dcb_config, 0, sizeof(struct ixgbe_dcb_config)); > ixgbe_dcb_init(hw,dcb_config); > > > -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ananyev, = > Konstantin > Sent: Wednesday, February 19, 2014 5:52 PM > To: Thomas Monjalon > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] [RFC PATCH 2/2] ixgbe: release software locked = > semaphores on initialization > > Hi Thomas, > I am afraid I couldn't send you an url we are using. > We take it from internal Intel ND repository. > Though I think, that latest available to download from Intel ixgbe driver= for FreeBSD should have pretty close codebase. > As a rule of thumb in our internal policy: we take shared code from ND an= d treat it as read-only (the only exception: ixgbe/ixgbe_osdep.h). > Otherwise it might become quite messy pretty quickly. > To overcome some problems or shortcomings in ND code people usually use w= rappers at the upper layer - that way was implemented bypass support, loop= back support, plus some other fixes (reported number of queues per VF, etc= ). > I wonder couldn't your fix also be pushed to the upper layer (in ixgbe_et= hdev.c or something)? > Thanks > Konstantin > > -----Original Message----- > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] > Sent: Wednesday, February 19, 2014 4:52 PM > To: Ananyev, Konstantin > Cc: Didier Pallard; dev@dpdk.org > Subject: Re: [dpdk-dev] [RFC PATCH 2/2] ixgbe: release software locked = > semaphores on initialization > > 19/02/2014 13:41, Ananyev, Konstantin: >> Can the patch be reworked to keep changes under = >> librte_pmd_ixgbe/ixgbe directory untouched? Those files are derived = >> directly from the BSD driver baseline, and any changes will make = >> future merges of newer code more challenging. The changes should be = >> limited to files in the librte_pmd_ixgbe directory (and ethdev). = >> Thanks > Please, could you send an url to the BSD driver baseline ? > By the way, git is very good at rebasing such patches. > And if the fix is good, it should be applied on the baseline. > Refusing a fix without alternative is not an option. > > -- > Thomas > -------------------------------------------------------------- > Intel Shannon Limited > Registered in Ireland > Registered Office: Collinstown Industrial Park, Leixlip, County = > Kildare Registered Number: 308263 Business address: Dromore House, = > East Park, Shannon, Co. Clare > > This e-mail and any attachments may contain confidential material for the= sole use of the intended recipient(s). Any review or distribution by other= s is strictly prohibited. If you are not the intended recipient, please con= tact the sender and delete all copies. > > > -------------------------------------------------------------- > Intel Shannon Limited > Registered in Ireland > Registered Office: Collinstown Industrial Park, Leixlip, County = > Kildare Registered Number: 308263 Business address: Dromore House, = > East Park, Shannon, Co. Clare > > This e-mail and any attachments may contain confidential material for the= sole use of the intended recipient(s). Any review or distribution by other= s is strictly prohibited. If you are not the intended recipient, please con= tact the sender and delete all copies. > > -- Didier PALLARD 6WIND Software Engineer Tel: +33 1 39 30 92 46 Mob: +33 6 49 11 40 14 Fax: +33 1 39 30 92 11 didier.pallard@6wind.com www.6wind.com This e-mail message, including any attachments, is for the sole use of the = intended recipient(s) and contains information that is confidential and pro= prietary to 6WIND. All unauthorized review, use, disclosure or distribution= is prohibited. If you are not the intended recipient, please contact the s= ender by reply e-mail and destroy all copies of the original message. Ce courriel ainsi que toutes les pi=E8ces jointes, est uniquement destin=E9= =E0 son ou ses destinataires. Il contient des informations confidentielles= qui sont la propri=E9t=E9 de 6WIND. Toute r=E9v=E9lation, distribution ou = copie des informations qu'il contient est strictement interdite. Si vous av= ez re=E7u ce message par erreur, veuillez imm=E9diatement le signaler =E0 l= '=E9metteur et d=E9truire toutes les donn=E9es re=E7ues -------------------------------------------------------------- Intel Shannon Limited Registered in Ireland Registered Office: Collinstown Industrial Park, Leixlip, County Kildare Registered Number: 308263 Business address: Dromore House, East Park, Shannon, Co. Clare This e-mail and any attachments may contain confidential material for the s= ole use of the intended recipient(s). Any review or distribution by others = is strictly prohibited. If you are not the intended recipient, please conta= ct the sender and delete all copies.