From mboxrd@z Thu Jan 1 00:00:00 1970 From: "didier.pallard" Subject: Re: [RFC PATCH 2/2] ixgbe: release software locked semaphores on initialization Date: Mon, 24 Feb 2014 15:19:00 +0100 Message-ID: <530B54D4.5050407@6wind.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> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: quoted-printable Cc: "dev-VfR2kkLFssw@public.gmane.org" To: "Ananyev, Konstantin" Return-path: In-Reply-To: <2601191342CEEE43887BDE71AB97725808E692BE-kPTMFJFq+rEu0RiL9chJVbfspsVTdybXVpNB7YpNyf8@public.gmane.org> List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces-VfR2kkLFssw@public.gmane.org Sender: "dev" Hi, The patch (or some derivative that do the same result) should probably=20 rather be integrated in base driver. For IGB implementation, it is not possible to extract patch from base=20 driver, since lock release should be done before calls to e1000_init_nvm_params or e1000_init_phy_params that use=20 the potentially stuck locks and after enough function pointers fields are filled by=20 e1000_setup_init_funcs to have functions to access the hardware. For ixgbe, it may be possible on 82598/82599 using=20 ixgbe_xxx_swfw_semaphore to do the job from outside the base driver, assuming that no lock will never be taken by the base=20 driver before the return of ixgbe_init_shared_code function. But it is not be possible on X540,=20 since this implementation does not use the ixgbe_get_eeprom_semaphore generic function that automatically=20 release the SMBI lock on timeout; So the release of this lock should be done using=20 ixgbe_release_swfw_sync_semaphore that is not accessible through the 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 ix= gbe_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_dc= b_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 a= ccess */ > + > + /* > + * Phy lock should not fail in this early stage. If this is the= case, > + * it is due to an improper exit of the application. > + * So force the release of the faulty lock. Release of common l= ock > + * 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 th= at 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= _SW_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 = eth_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-VfR2kkLFssw@public.gmane.org] On Behalf Of Ananyev, Konstanti= n > Sent: Wednesday, February 19, 2014 5:52 PM > To: Thomas Monjalon > Cc: dev-VfR2kkLFssw@public.gmane.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 driv= er for FreeBSD should have pretty close codebase. > As a rule of thumb in our internal policy: we take shared code from ND = and 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= wrappers at the upper layer - that way was implemented bypass support, = loopback support, plus some other fixes (reported number of queues per V= F, etc). > I wonder couldn't your fix also be pushed to the upper layer (in ixgbe_= ethdev.c or something)? > Thanks > Konstantin > > -----Original Message----- > From: Thomas Monjalon [mailto:thomas.monjalon-pdR9zngts4EAvxtiuMwx3w@public.gmane.org] > Sent: Wednesday, February 19, 2014 4:52 PM > To: Ananyev, Konstantin > Cc: Didier Pallard; dev-VfR2kkLFssw@public.gmane.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, Sh= annon, Co. Clare > > This e-mail and any attachments may contain confidential material for t= he sole use of the intended recipient(s). Any review or distribution by o= thers is strictly prohibited. If you are not the intended recipient, plea= se contact 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 t= he sole use of the intended recipient(s). Any review or distribution by o= thers is strictly prohibited. If you are not the intended recipient, plea= se contact the sender and delete all copies. > > --=20 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-pdR9zngts4EAvxtiuMwx3w@public.gmane.org www.6wind.com This e-mail message, including any attachments, is for the sole use of th= e intended recipient(s) and contains information that is confidential and= proprietary to 6WIND. All unauthorized review, use, disclosure or distri= bution is prohibited. If you are not the intended recipient, please conta= ct the sender by reply e-mail and destroy all copies of the original mess= age. Ce courriel ainsi que toutes les pi=E8ces jointes, est uniquement destin=E9= =E0 son ou ses destinataires. Il contient des informations confidentiell= es qui sont la propri=E9t=E9 de 6WIND. Toute r=E9v=E9lation, distribution= ou copie des informations qu'il contient est strictement interdite. Si v= ous avez re=E7u ce message par erreur, veuillez imm=E9diatement le signal= er =E0 l'=E9metteur et d=E9truire toutes les donn=E9es re=E7ues