From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wg0-f49.google.com (mail-wg0-f49.google.com [74.125.82.49]) by dpdk.org (Postfix) with ESMTP id 1DBA769F9 for ; Wed, 26 Feb 2014 11:51:33 +0100 (CET) Received: by mail-wg0-f49.google.com with SMTP id x12so1472753wgg.8 for ; Wed, 26 Feb 2014 02:53:00 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:message-id:date:from:organization:user-agent :mime-version:to:subject:references:in-reply-to:content-type; bh=LMaDxmXSyhVQDYNW4OT5bJJ+licJXmf86AsLabm66+Y=; b=HSWxfYZnTxrES6IW7EO4u5EUKr8yBOmkODH+1FYxLb5ub0GVgI5fZSI6Jc9qxculkC 4vczBKNgHNtwegBTWmGk4R6MWdvAWBK+HbPIstVlr4qc66BPd9fmOr6q9nHWZqM9DsBA 0GkdxUs25O3RQntvuFNcn+EeeJ8wCRx3ZuTsa7Jnq+HslRJqYnjLx1iEhgmPipvbsDJX ObKUN3CumgY6rWV24/8Od/82wc9wSWrlkjgrquIW+VE4o7dKoVI5mVRjMZEQdJI38RAB c97Uhbqs7WkxQAp6zFtdBzo94JELRjyUFxdUg0RD8Tzh5xIP2ZAVdXJ68P9qxnr/2ItS 4KKQ== X-Gm-Message-State: ALoCoQmjwrt67KQBI5ZvNYikbUerbNseUf9mmqmUCOAlwht8nh75HbVDdAgqox6snqHVX+EKihMX X-Received: by 10.194.75.225 with SMTP id f1mr565903wjw.87.1393411980284; Wed, 26 Feb 2014 02:53:00 -0800 (PST) Received: from [10.16.0.123] (guy78-3-82-239-227-177.fbx.proxad.net. [82.239.227.177]) by mx.google.com with ESMTPSA id ff9sm9005098wib.11.2014.02.26.02.52.57 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Wed, 26 Feb 2014 02:52:58 -0800 (PST) Message-ID: <530DC788.8020706@6wind.com> Date: Wed, 26 Feb 2014 11:52:56 +0100 From: "didier.pallard" Organization: 6WIND User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:18.0) Gecko/20121128 Thunderbird/18.0 MIME-Version: 1.0 To: "dev@dpdk.org" 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> <2601191342CEEE43887BDE71AB97725808E6AA49@IRSMSX105.ger.corp.intel.com> In-Reply-To: <2601191342CEEE43887BDE71AB97725808E6AA49@IRSMSX105.ger.corp.intel.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-Content-Filtered-By: Mailman/MimeDel 2.1.15 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: Wed, 26 Feb 2014 10:51:34 -0000 Hi, Well, about ixgbe, you're right, i didn't notice that the ixgbe_release_swfw_sync_X540 function is releasing the SMBI lock, even if it is not the owner. I'm not sure it is really safe to do that, but it may allow to release the locks from outside the base driver in all cases using only ixgbe_acquire/release_swfw_semaphore API functions. for igb, in my tests, i found that i had to put lock release between e1000_init_mac_params and e1000_init_nvm_params because all necessary setup was not done in hw structure before e1000_init_mac_params call (so e1000_setup_init_funcs(hw, FALSE) was not enough to have all settings done to be able to reset the locks). But perhaps it may be overcome by a call to another exported function of the API. I think that what i was needing is a call to mac->ops.set_lan_id(hw) that is also done in e1000_get_bus_info_pcie_generic function. So something like that should work: e1000_setup_init_funcs(hw, FALSE); e1000_get_bus_info(hw); e1000_reset_swfw_lock(hw); e1000_setup_init_funcs(hw, TRUE); I will try this patch. thanks On 02/25/2014 01:57 AM, Ananyev, Konstantin wrote: > Hi, > About e1000 - I suppose you refer to the eth_igb_dev_init()->e1000_setup_init_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 call e1000_init_nvm_params/ e1000_init_phy_params. > Then we reset the lock, then call setup_funcs once again - that time it would 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 semaphores on initialization > > Hi, > > The patch (or some derivative that do the same result) should probably rather be integrated in base driver. > > For IGB implementation, it is not possible to extract patch from base driver, 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 enough function pointers fields are filled by e1000_setup_init_funcs to have functions 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 never 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 done using 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 >> 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 case, >> + * 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 = 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 = 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 == ixgbe_mac_82598EB || >> + hw->mac.type == ixgbe_mac_82599EB || >> + hw->mac.type == 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 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 VF, 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@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 others is strictly prohibited. If you are not the intended recipient, please 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 the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please 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 the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. > >