* [dpdk-dev] [RFC PATCH 1/2] e1000: release software locked semaphores on initialization @ 2014-02-19 11:59 Didier Pallard 2014-02-19 11:59 ` [dpdk-dev] [RFC PATCH 2/2] ixgbe: " Didier Pallard 0 siblings, 1 reply; 16+ messages in thread From: Didier Pallard @ 2014-02-19 11:59 UTC (permalink / raw) To: thomas.monjalon It may happen that DPDK application gets killed while having acquired locks on the ethernet hardware, causing these locks to be never released. On next restart of the application, DPDK skip those ports because it can not acquire the lock, this may cause some ports (or even complete board if SMBI is locked) to be inaccessible from DPDK application until reboot of the hardware. This patch release locks that are supposed to be locked due to an improper exit of the application. 8254x series do not have SWFW sempahores. 82571 implementation already has some kind of lock reset at the end of e1000_init_mac_params_82571 function. Since I have no mean to test this implementation, code is not modified for this hardware. Signed-off-by: Didier Pallard <didier.pallard@6wind.com> --- lib/librte_pmd_e1000/e1000/e1000_80003es2lan.c | 29 +++++++++++++++++++++ lib/librte_pmd_e1000/e1000/e1000_82575.c | 33 ++++++++++++++++++++++++ lib/librte_pmd_e1000/e1000/e1000_i210.c | 3 +-- lib/librte_pmd_e1000/e1000/e1000_i210.h | 1 + 4 files changed, 64 insertions(+), 2 deletions(-) diff --git a/lib/librte_pmd_e1000/e1000/e1000_80003es2lan.c b/lib/librte_pmd_e1000/e1000/e1000_80003es2lan.c index 60d7c2a..952e8de 100644 --- a/lib/librte_pmd_e1000/e1000/e1000_80003es2lan.c +++ b/lib/librte_pmd_e1000/e1000/e1000_80003es2lan.c @@ -195,6 +195,7 @@ STATIC s32 e1000_init_nvm_params_80003es2lan(struct e1000_hw *hw) STATIC s32 e1000_init_mac_params_80003es2lan(struct e1000_hw *hw) { struct e1000_mac_info *mac = &hw->mac; + u16 mask; DEBUGFUNC("e1000_init_mac_params_80003es2lan"); @@ -267,6 +268,34 @@ STATIC s32 e1000_init_mac_params_80003es2lan(struct e1000_hw *hw) /* set lan id for port to determine which phy lock to use */ hw->mac.ops.set_lan_id(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. + */ + if (e1000_acquire_phy_80003es2lan(hw) < 0) { + if (e1000_get_hw_semaphore_generic(hw) < 0) { + DEBUGOUT("SMBI lock released"); + } + e1000_put_hw_semaphore_generic(hw); + DEBUGOUT1("SWFW phy%d lock released", hw->bus.func); + } + e1000_release_phy_80003es2lan(hw); + + /* + * Those one are more tricky since they are common to all ports; but + * swfw_sync retries last long enough (250ms) to be almost sure that if + * lock can not be taken it is due to an improper lock of the + * semaphore. + */ + mask = E1000_SWFW_EEP_SM | E1000_SWFW_CSR_SM; + if (hw->mac.ops.acquire_swfw_sync(hw, mask) < 0) { + DEBUGOUT("SWFW common locks released"); + } + hw->mac.ops.release_swfw_sync(hw, mask); + return E1000_SUCCESS; } diff --git a/lib/librte_pmd_e1000/e1000/e1000_82575.c b/lib/librte_pmd_e1000/e1000/e1000_82575.c index fd15b7b..99c4a1f 100644 --- a/lib/librte_pmd_e1000/e1000/e1000_82575.c +++ b/lib/librte_pmd_e1000/e1000/e1000_82575.c @@ -507,6 +507,39 @@ STATIC s32 e1000_init_mac_params_82575(struct e1000_hw *hw) /* set lan id for port to determine which phy lock to use */ hw->mac.ops.set_lan_id(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. + */ + if (e1000_acquire_phy_82575(hw) < 0) { + if (mac->type >= e1000_i210) { + if (e1000_get_hw_semaphore_i210(hw) < 0) { + DEBUGOUT("SMBI lock released"); + } + } else { + if (e1000_get_hw_semaphore_generic(hw) < 0) { + DEBUGOUT("SMBI lock released"); + } + } + e1000_put_hw_semaphore_generic(hw); + DEBUGOUT1("SWFW phy%d lock released", hw->bus.func); + } + e1000_release_phy_82575(hw); + + /* + * This one is more tricky since it is 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. + */ + if (hw->mac.ops.acquire_swfw_sync(hw, E1000_SWFW_EEP_SM) < 0) { + DEBUGOUT("SWFW common locks released"); + } + hw->mac.ops.release_swfw_sync(hw, E1000_SWFW_EEP_SM); + return E1000_SUCCESS; } diff --git a/lib/librte_pmd_e1000/e1000/e1000_i210.c b/lib/librte_pmd_e1000/e1000/e1000_i210.c index 722877a..122fe70 100644 --- a/lib/librte_pmd_e1000/e1000/e1000_i210.c +++ b/lib/librte_pmd_e1000/e1000/e1000_i210.c @@ -36,7 +36,6 @@ POSSIBILITY OF SUCH DAMAGE. STATIC s32 e1000_acquire_nvm_i210(struct e1000_hw *hw); STATIC void e1000_release_nvm_i210(struct e1000_hw *hw); -STATIC s32 e1000_get_hw_semaphore_i210(struct e1000_hw *hw); STATIC s32 e1000_write_nvm_srwr(struct e1000_hw *hw, u16 offset, u16 words, u16 *data); STATIC s32 e1000_pool_flash_update_done_i210(struct e1000_hw *hw); @@ -158,7 +157,7 @@ void e1000_release_swfw_sync_i210(struct e1000_hw *hw, u16 mask) * * Acquire the HW semaphore to access the PHY or NVM **/ -STATIC s32 e1000_get_hw_semaphore_i210(struct e1000_hw *hw) +s32 e1000_get_hw_semaphore_i210(struct e1000_hw *hw) { u32 swsm; s32 timeout = hw->nvm.word_size + 1; diff --git a/lib/librte_pmd_e1000/e1000/e1000_i210.h b/lib/librte_pmd_e1000/e1000/e1000_i210.h index 44de54b..a9e8a64 100644 --- a/lib/librte_pmd_e1000/e1000/e1000_i210.h +++ b/lib/librte_pmd_e1000/e1000/e1000_i210.h @@ -44,6 +44,7 @@ s32 e1000_read_nvm_srrd_i210(struct e1000_hw *hw, u16 offset, u16 words, u16 *data); s32 e1000_read_invm_version(struct e1000_hw *hw, struct e1000_fw_version *invm_ver); +s32 e1000_get_hw_semaphore_i210(struct e1000_hw *hw); s32 e1000_acquire_swfw_sync_i210(struct e1000_hw *hw, u16 mask); void e1000_release_swfw_sync_i210(struct e1000_hw *hw, u16 mask); s32 e1000_read_xmdio_reg(struct e1000_hw *hw, u16 addr, u8 dev_addr, -- 1.7.10.4 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [dpdk-dev] [RFC PATCH 2/2] ixgbe: release software locked semaphores on initialization 2014-02-19 11:59 [dpdk-dev] [RFC PATCH 1/2] e1000: release software locked semaphores on initialization Didier Pallard @ 2014-02-19 11:59 ` Didier Pallard 2014-02-19 12:41 ` Ananyev, Konstantin 0 siblings, 1 reply; 16+ messages in thread From: Didier Pallard @ 2014-02-19 11:59 UTC (permalink / raw) To: thomas.monjalon It may happen that DPDK application gets killed while having acquired locks on the ethernet hardware, causing these locks to be never released. On next restart of the application, DPDK skip those ports because it can not acquire the lock, this may cause some ports (or even complete board if SMBI is locked) to be inaccessible from DPDK application until reboot of the hardware. This patch release locks that are supposed to be locked due to an improper exit of the application. Signed-off-by: Didier Pallard <didier.pallard@6wind.com> --- lib/librte_pmd_ixgbe/ixgbe/ixgbe_82598.c | 30 +++++++++++++++++++++++++++ lib/librte_pmd_ixgbe/ixgbe/ixgbe_82599.c | 29 ++++++++++++++++++++++++++ lib/librte_pmd_ixgbe/ixgbe/ixgbe_x540.c | 33 ++++++++++++++++++++++++++++++ 3 files changed, 92 insertions(+) diff --git a/lib/librte_pmd_ixgbe/ixgbe/ixgbe_82598.c b/lib/librte_pmd_ixgbe/ixgbe/ixgbe_82598.c index a9d1b9d..8e2ca1c 100644 --- a/lib/librte_pmd_ixgbe/ixgbe/ixgbe_82598.c +++ b/lib/librte_pmd_ixgbe/ixgbe/ixgbe_82598.c @@ -115,6 +115,7 @@ s32 ixgbe_init_ops_82598(struct ixgbe_hw *hw) struct ixgbe_mac_info *mac = &hw->mac; struct ixgbe_phy_info *phy = &hw->phy; s32 ret_val; + u16 mask; DEBUGFUNC("ixgbe_init_ops_82598"); @@ -166,6 +167,35 @@ s32 ixgbe_init_ops_82598(struct ixgbe_hw *hw) /* Manageability interface */ mac->ops.set_fw_drv_ver = NULL; + /* Get bus info */ + 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); + return ret_val; } diff --git a/lib/librte_pmd_ixgbe/ixgbe/ixgbe_82599.c b/lib/librte_pmd_ixgbe/ixgbe/ixgbe_82599.c index db07789..ca91967 100644 --- a/lib/librte_pmd_ixgbe/ixgbe/ixgbe_82599.c +++ b/lib/librte_pmd_ixgbe/ixgbe/ixgbe_82599.c @@ -223,6 +223,7 @@ s32 ixgbe_init_ops_82599(struct ixgbe_hw *hw) struct ixgbe_phy_info *phy = &hw->phy; struct ixgbe_eeprom_info *eeprom = &hw->eeprom; s32 ret_val; + u16 mask; DEBUGFUNC("ixgbe_init_ops_82599"); @@ -291,6 +292,34 @@ s32 ixgbe_init_ops_82599(struct ixgbe_hw *hw) /* Manageability interface */ mac->ops.set_fw_drv_ver = &ixgbe_set_fw_drv_ver_generic; + /* Get bus info */ + 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); return ret_val; } diff --git a/lib/librte_pmd_ixgbe/ixgbe/ixgbe_x540.c b/lib/librte_pmd_ixgbe/ixgbe/ixgbe_x540.c index d3e1730..607c9c7 100644 --- a/lib/librte_pmd_ixgbe/ixgbe/ixgbe_x540.c +++ b/lib/librte_pmd_ixgbe/ixgbe/ixgbe_x540.c @@ -55,6 +55,7 @@ s32 ixgbe_init_ops_X540(struct ixgbe_hw *hw) struct ixgbe_phy_info *phy = &hw->phy; struct ixgbe_eeprom_info *eeprom = &hw->eeprom; s32 ret_val; + u16 mask; DEBUGFUNC("ixgbe_init_ops_X540"); @@ -141,6 +142,38 @@ s32 ixgbe_init_ops_X540(struct ixgbe_hw *hw) /* Manageability interface */ mac->ops.set_fw_drv_ver = &ixgbe_set_fw_drv_ver_generic; + /* Get bus info */ + 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. + */ + mask = IXGBE_GSSR_PHY0_SM << hw->bus.func; + if (hw->mac.ops.acquire_swfw_sync(hw, mask) < 0) { + if (ixgbe_get_swfw_sync_semaphore(hw) < 0) { + DEBUGOUT("SMBI lock released"); + } + ixgbe_release_swfw_sync_semaphore(hw); + 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); + return ret_val; } -- 1.7.10.4 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [dpdk-dev] [RFC PATCH 2/2] ixgbe: release software locked semaphores on initialization 2014-02-19 11:59 ` [dpdk-dev] [RFC PATCH 2/2] ixgbe: " Didier Pallard @ 2014-02-19 12:41 ` Ananyev, Konstantin 2014-02-19 16:52 ` Thomas Monjalon 0 siblings, 1 reply; 16+ messages in thread From: Ananyev, Konstantin @ 2014-02-19 12:41 UTC (permalink / raw) To: Didier Pallard, thomas.monjalon; +Cc: dev Hi, 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 Konstantin -----Original Message----- From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Didier Pallard Sent: Wednesday, February 19, 2014 11:59 AM To: thomas.monjalon@6wind.com Subject: [dpdk-dev] [RFC PATCH 2/2] ixgbe: release software locked semaphores on initialization It may happen that DPDK application gets killed while having acquired locks on the ethernet hardware, causing these locks to be never released. On next restart of the application, DPDK skip those ports because it can not acquire the lock, this may cause some ports (or even complete board if SMBI is locked) to be inaccessible from DPDK application until reboot of the hardware. This patch release locks that are supposed to be locked due to an improper exit of the application. Signed-off-by: Didier Pallard <didier.pallard@6wind.com> --- lib/librte_pmd_ixgbe/ixgbe/ixgbe_82598.c | 30 +++++++++++++++++++++++++++ lib/librte_pmd_ixgbe/ixgbe/ixgbe_82599.c | 29 ++++++++++++++++++++++++++ lib/librte_pmd_ixgbe/ixgbe/ixgbe_x540.c | 33 ++++++++++++++++++++++++++++++ 3 files changed, 92 insertions(+) diff --git a/lib/librte_pmd_ixgbe/ixgbe/ixgbe_82598.c b/lib/librte_pmd_ixgbe/ixgbe/ixgbe_82598.c index a9d1b9d..8e2ca1c 100644 --- a/lib/librte_pmd_ixgbe/ixgbe/ixgbe_82598.c +++ b/lib/librte_pmd_ixgbe/ixgbe/ixgbe_82598.c @@ -115,6 +115,7 @@ s32 ixgbe_init_ops_82598(struct ixgbe_hw *hw) struct ixgbe_mac_info *mac = &hw->mac; struct ixgbe_phy_info *phy = &hw->phy; s32 ret_val; + u16 mask; DEBUGFUNC("ixgbe_init_ops_82598"); @@ -166,6 +167,35 @@ s32 ixgbe_init_ops_82598(struct ixgbe_hw *hw) /* Manageability interface */ mac->ops.set_fw_drv_ver = NULL; + /* Get bus info */ + 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); + return ret_val; } diff --git a/lib/librte_pmd_ixgbe/ixgbe/ixgbe_82599.c b/lib/librte_pmd_ixgbe/ixgbe/ixgbe_82599.c index db07789..ca91967 100644 --- a/lib/librte_pmd_ixgbe/ixgbe/ixgbe_82599.c +++ b/lib/librte_pmd_ixgbe/ixgbe/ixgbe_82599.c @@ -223,6 +223,7 @@ s32 ixgbe_init_ops_82599(struct ixgbe_hw *hw) struct ixgbe_phy_info *phy = &hw->phy; struct ixgbe_eeprom_info *eeprom = &hw->eeprom; s32 ret_val; + u16 mask; DEBUGFUNC("ixgbe_init_ops_82599"); @@ -291,6 +292,34 @@ s32 ixgbe_init_ops_82599(struct ixgbe_hw *hw) /* Manageability interface */ mac->ops.set_fw_drv_ver = &ixgbe_set_fw_drv_ver_generic; + /* Get bus info */ + 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); return ret_val; } diff --git a/lib/librte_pmd_ixgbe/ixgbe/ixgbe_x540.c b/lib/librte_pmd_ixgbe/ixgbe/ixgbe_x540.c index d3e1730..607c9c7 100644 --- a/lib/librte_pmd_ixgbe/ixgbe/ixgbe_x540.c +++ b/lib/librte_pmd_ixgbe/ixgbe/ixgbe_x540.c @@ -55,6 +55,7 @@ s32 ixgbe_init_ops_X540(struct ixgbe_hw *hw) struct ixgbe_phy_info *phy = &hw->phy; struct ixgbe_eeprom_info *eeprom = &hw->eeprom; s32 ret_val; + u16 mask; DEBUGFUNC("ixgbe_init_ops_X540"); @@ -141,6 +142,38 @@ s32 ixgbe_init_ops_X540(struct ixgbe_hw *hw) /* Manageability interface */ mac->ops.set_fw_drv_ver = &ixgbe_set_fw_drv_ver_generic; + /* Get bus info */ + 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. + */ + mask = IXGBE_GSSR_PHY0_SM << hw->bus.func; + if (hw->mac.ops.acquire_swfw_sync(hw, mask) < 0) { + if (ixgbe_get_swfw_sync_semaphore(hw) < 0) { + DEBUGOUT("SMBI lock released"); + } + ixgbe_release_swfw_sync_semaphore(hw); + 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); + return ret_val; } -- 1.7.10.4 -------------------------------------------------------------- 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. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [dpdk-dev] [RFC PATCH 2/2] ixgbe: release software locked semaphores on initialization 2014-02-19 12:41 ` Ananyev, Konstantin @ 2014-02-19 16:52 ` Thomas Monjalon 2014-02-19 17:51 ` Ananyev, Konstantin 0 siblings, 1 reply; 16+ messages in thread From: Thomas Monjalon @ 2014-02-19 16:52 UTC (permalink / raw) To: Ananyev, Konstantin; +Cc: dev 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [dpdk-dev] [RFC PATCH 2/2] ixgbe: release software locked semaphores on initialization 2014-02-19 16:52 ` Thomas Monjalon @ 2014-02-19 17:51 ` Ananyev, Konstantin 2014-02-21 16:30 ` Ananyev, Konstantin 0 siblings, 1 reply; 16+ messages in thread From: Ananyev, Konstantin @ 2014-02-19 17:51 UTC (permalink / raw) To: Thomas Monjalon; +Cc: dev 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. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [dpdk-dev] [RFC PATCH 2/2] ixgbe: release software locked semaphores on initialization 2014-02-19 17:51 ` Ananyev, Konstantin @ 2014-02-21 16:30 ` Ananyev, Konstantin 2014-02-24 14:19 ` didier.pallard 0 siblings, 1 reply; 16+ messages in thread From: Ananyev, Konstantin @ 2014-02-21 16:30 UTC (permalink / raw) To: Thomas Monjalon, 'Didier Pallard' (didier.pallard@6wind.com); +Cc: dev 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. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [dpdk-dev] [RFC PATCH 2/2] ixgbe: release software locked semaphores on initialization 2014-02-21 16:30 ` Ananyev, Konstantin @ 2014-02-24 14:19 ` didier.pallard 2014-02-24 15:34 ` Ananyev, Konstantin 2014-02-25 0:57 ` Ananyev, Konstantin 0 siblings, 2 replies; 16+ messages in thread From: didier.pallard @ 2014-02-24 14:19 UTC (permalink / raw) To: Ananyev, Konstantin; +Cc: dev 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. > > -- 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 proprietary to 6WIND. All unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply e-mail and destroy all copies of the original message. Ce courriel ainsi que toutes les pièces jointes, est uniquement destiné à son ou ses destinataires. Il contient des informations confidentielles qui sont la propriété de 6WIND. Toute révélation, distribution ou copie des informations qu'il contient est strictement interdite. Si vous avez reçu ce message par erreur, veuillez immédiatement le signaler à l'émetteur et détruire toutes les données reçues ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [dpdk-dev] [RFC PATCH 2/2] ixgbe: release software locked semaphores on initialization 2014-02-24 14:19 ` didier.pallard @ 2014-02-24 15:34 ` Ananyev, Konstantin 2014-02-25 0:57 ` Ananyev, Konstantin 1 sibling, 0 replies; 16+ messages in thread From: Ananyev, Konstantin @ 2014-02-24 15:34 UTC (permalink / raw) To: didier.pallard; +Cc: dev Hi Didier, I didn't look at e1000 yet, so can't argue about it. About X540, I suppose you are talking about these lines: + if (ixgbe_get_swfw_sync_semaphore(hw) < 0) { + DEBUGOUT("SMBI lock released"); + } + ixgbe_release_swfw_sync_semaphore(hw); Can you explain, why do we need to call these 2 static functions directly for x540 one more time? Can't we just rely on ixgbe_release_swfw_sync_X540() to call them for us? 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. > > -- 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 proprietary to 6WIND. All unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply e-mail and destroy all copies of the original message. Ce courriel ainsi que toutes les pièces jointes, est uniquement destiné à son ou ses destinataires. Il contient des informations confidentielles qui sont la propriété de 6WIND. Toute révélation, distribution ou copie des informations qu'il contient est strictement interdite. Si vous avez reçu ce message par erreur, veuillez immédiatement le signaler à l'émetteur et détruire toutes les données reçues -------------------------------------------------------------- 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. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [dpdk-dev] [RFC PATCH 2/2] ixgbe: release software locked semaphores on initialization 2014-02-24 14:19 ` didier.pallard 2014-02-24 15:34 ` Ananyev, Konstantin @ 2014-02-25 0:57 ` Ananyev, Konstantin 2014-02-26 10:52 ` didier.pallard 1 sibling, 1 reply; 16+ messages in thread From: Ananyev, Konstantin @ 2014-02-25 0:57 UTC (permalink / raw) To: didier.pallard; +Cc: dev 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. > > -- 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 proprietary to 6WIND. All unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply e-mail and destroy all copies of the original message. Ce courriel ainsi que toutes les pièces jointes, est uniquement destiné à son ou ses destinataires. Il contient des informations confidentielles qui sont la propriété de 6WIND. Toute révélation, distribution ou copie des informations qu'il contient est strictement interdite. Si vous avez reçu ce message par erreur, veuillez immédiatement le signaler à l'émetteur et détruire toutes les données reçues -------------------------------------------------------------- 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. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [dpdk-dev] [RFC PATCH 2/2] ixgbe: release software locked semaphores on initialization 2014-02-25 0:57 ` Ananyev, Konstantin @ 2014-02-26 10:52 ` didier.pallard 2014-04-08 13:29 ` [dpdk-dev] [PATCH v3 1/2] " Didier Pallard 0 siblings, 1 reply; 16+ messages in thread From: didier.pallard @ 2014-02-26 10:52 UTC (permalink / raw) To: dev 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. > > ^ permalink raw reply [flat|nested] 16+ messages in thread
* [dpdk-dev] [PATCH v3 1/2] ixgbe: release software locked semaphores on initialization 2014-02-26 10:52 ` didier.pallard @ 2014-04-08 13:29 ` Didier Pallard 2014-04-08 13:29 ` [dpdk-dev] [PATCH v3 2/2] igb: " Didier Pallard 2014-04-09 15:49 ` [dpdk-dev] [PATCH v3 1/2] ixgbe: " Ananyev, Konstantin 0 siblings, 2 replies; 16+ messages in thread From: Didier Pallard @ 2014-04-08 13:29 UTC (permalink / raw) To: dev It may happen that DPDK application gets killed while having acquired locks on the ethernet hardware, causing these locks to be never released. On next restart of the application, DPDK skip those ports because it can not acquire the lock, this may cause some ports (or even complete board if SMBI is locked) to be inaccessible from DPDK application until reboot of the hardware. This patch release locks that are supposed to be locked due to an improper exit of the application. Signed-off-by: Didier Pallard <didier.pallard@6wind.com> --- v3: Move code outside from base driver lib/librte_pmd_ixgbe/ixgbe_ethdev.c | 41 ++++++++++++++++++++++++++++++++--- 1 file changed, 38 insertions(+), 3 deletions(-) diff --git a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c index fdf6c1d..637fa88 100644 --- a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c +++ b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c @@ -587,6 +587,38 @@ ixgbe_dcb_init(struct ixgbe_hw *hw,struct ixgbe_dcb_config *dcb_config) } /* + * Ensure that all locks are released before first NVM or PHY access + */ +static void +ixgbe_swfw_lock_reset(struct ixgbe_hw *hw) { + uint16_t mask; + + /* + * 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 (ixgbe_acquire_swfw_semaphore(hw, mask) < 0) { + DEBUGOUT1("SWFW phy%d lock released", hw->bus.func); + } + ixgbe_release_swfw_semaphore(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 (ixgbe_acquire_swfw_semaphore(hw, mask) < 0) { + DEBUGOUT("SWFW common locks released"); + } + ixgbe_release_swfw_semaphore(hw, mask); +} + +/* * This function is based on code in ixgbe_attach() in ixgbe/ixgbe.c. * It returns 0 on success. */ @@ -643,6 +675,12 @@ eth_ixgbe_dev_init(__attribute__((unused)) struct eth_driver *eth_drv, return -EIO; } + /* pick up the PCI bus settings for reporting later */ + ixgbe_get_bus_info(hw); + + /* Unlock any pending hardware semaphore */ + ixgbe_swfw_lock_reset(hw); + /* Initialize DCB configuration*/ memset(dcb_config, 0, sizeof(struct ixgbe_dcb_config)); ixgbe_dcb_init(hw,dcb_config); @@ -700,9 +738,6 @@ eth_ixgbe_dev_init(__attribute__((unused)) struct eth_driver *eth_drv, /* disable interrupt */ ixgbe_disable_intr(hw); - /* pick up the PCI bus settings for reporting later */ - ixgbe_get_bus_info(hw); - /* reset mappings for queue statistics hw counters*/ ixgbe_reset_qstat_mappings(hw); -- 1.7.10.4 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [dpdk-dev] [PATCH v3 2/2] igb: release software locked semaphores on initialization 2014-04-08 13:29 ` [dpdk-dev] [PATCH v3 1/2] " Didier Pallard @ 2014-04-08 13:29 ` Didier Pallard 2014-04-09 15:58 ` Ananyev, Konstantin 2014-04-09 15:49 ` [dpdk-dev] [PATCH v3 1/2] ixgbe: " Ananyev, Konstantin 1 sibling, 1 reply; 16+ messages in thread From: Didier Pallard @ 2014-04-08 13:29 UTC (permalink / raw) To: dev It may happen that DPDK application gets killed while having acquired locks on the ethernet hardware, causing these locks to be never released. On next restart of the application, DPDK skip those ports because it can not acquire the lock, this may cause some ports (or even complete board if SMBI is locked) to be inaccessible from DPDK application until reboot of the hardware. This patch release locks that are supposed to be locked due to an improper exit of the application. Signed-off-by: Didier Pallard <didier.pallard@6wind.com> --- v3: Move code outside from base driver lib/librte_pmd_e1000/igb_ethdev.c | 69 ++++++++++++++++++++++++++++++++++++- 1 file changed, 68 insertions(+), 1 deletion(-) diff --git a/lib/librte_pmd_e1000/igb_ethdev.c b/lib/librte_pmd_e1000/igb_ethdev.c index 184e7d6..673b4de 100644 --- a/lib/librte_pmd_e1000/igb_ethdev.c +++ b/lib/librte_pmd_e1000/igb_ethdev.c @@ -320,6 +320,61 @@ igb_identify_hardware(struct rte_eth_dev *dev) } static int +igb_reset_swfw_lock(struct e1000_hw *hw) +{ + int ret_val; + + /* + * Do mac ops initialization manually here, since we will need + * some function pointers set by this call. + */ + ret_val = e1000_init_mac_params(hw); + if (ret_val) + return ret_val; + + /* + * SMBI 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. + */ + if (e1000_get_hw_semaphore_generic(hw) < 0) { + DEBUGOUT("SMBI lock released"); + } + e1000_put_hw_semaphore_generic(hw); + + if (hw->mac.ops.acquire_swfw_sync != NULL) { + uint16_t mask; + + /* + * 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. + */ + mask = E1000_SWFW_PHY0_SM << hw->bus.func; + if (hw->bus.func > E1000_FUNC_1) + mask <<= 2; + 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); + + /* + * This one is more tricky since it is 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 = E1000_SWFW_EEP_SM; + if (hw->mac.ops.acquire_swfw_sync(hw, mask) < 0) { + DEBUGOUT("SWFW common locks released"); + } + hw->mac.ops.release_swfw_sync(hw, mask); + } + + return E1000_SUCCESS; +} + +static int eth_igb_dev_init(__attribute__((unused)) struct eth_driver *eth_drv, struct rte_eth_dev *eth_dev) { @@ -348,13 +403,25 @@ eth_igb_dev_init(__attribute__((unused)) struct eth_driver *eth_drv, hw->hw_addr= (void *)pci_dev->mem_resource[0].addr; igb_identify_hardware(eth_dev); - if (e1000_setup_init_funcs(hw, TRUE) != E1000_SUCCESS) { + if (e1000_setup_init_funcs(hw, FALSE) != E1000_SUCCESS) { error = -EIO; goto err_late; } e1000_get_bus_info(hw); + /* Reset any pending lock */ + if (igb_reset_swfw_lock(hw) != E1000_SUCCESS) { + error = -EIO; + goto err_late; + } + + /* Finish initialization */ + if (e1000_setup_init_funcs(hw, TRUE) != E1000_SUCCESS) { + error = -EIO; + goto err_late; + } + hw->mac.autoneg = 1; hw->phy.autoneg_wait_to_complete = 0; hw->phy.autoneg_advertised = E1000_ALL_SPEED_DUPLEX; -- 1.7.10.4 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [dpdk-dev] [PATCH v3 2/2] igb: release software locked semaphores on initialization 2014-04-08 13:29 ` [dpdk-dev] [PATCH v3 2/2] igb: " Didier Pallard @ 2014-04-09 15:58 ` Ananyev, Konstantin 2014-04-09 16:39 ` Thomas Monjalon 0 siblings, 1 reply; 16+ messages in thread From: Ananyev, Konstantin @ 2014-04-09 15:58 UTC (permalink / raw) To: Didier Pallard, dev -----Original Message----- From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Didier Pallard Sent: Tuesday, April 08, 2014 2:30 PM To: dev@dpdk.org Subject: [dpdk-dev] [PATCH v3 2/2] igb: release software locked semaphores on initialization It may happen that DPDK application gets killed while having acquired locks on the ethernet hardware, causing these locks to be never released. On next restart of the application, DPDK skip those ports because it can not acquire the lock, this may cause some ports (or even complete board if SMBI is locked) to be inaccessible from DPDK application until reboot of the hardware. This patch release locks that are supposed to be locked due to an improper exit of the application. Signed-off-by: Didier Pallard <didier.pallard@6wind.com> --- Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com> ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [dpdk-dev] [PATCH v3 2/2] igb: release software locked semaphores on initialization 2014-04-09 15:58 ` Ananyev, Konstantin @ 2014-04-09 16:39 ` Thomas Monjalon 0 siblings, 0 replies; 16+ messages in thread From: Thomas Monjalon @ 2014-04-09 16:39 UTC (permalink / raw) To: Ananyev, Konstantin, Didier Pallard; +Cc: dev > It may happen that DPDK application gets killed while having acquired locks > on the ethernet hardware, causing these locks to be never released. On next > restart of the application, DPDK skip those ports because it can not > acquire the lock, this may cause some ports (or even complete board if SMBI > is locked) to be inaccessible from DPDK application until reboot of the > hardware. > > This patch release locks that are supposed to be locked due to an improper > exit of the application. > > Signed-off-by: Didier Pallard <didier.pallard@6wind.com> > Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com> Applied for version 1.6.0r2. So the decision is to try not modify base driver. But the fix should appear in the base driver one day and we must be careful when rebasing on an updated one. In this case, the fix can be done on both side without being harmful. Thanks for fixing this random failure. -- Thomas ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [dpdk-dev] [PATCH v3 1/2] ixgbe: release software locked semaphores on initialization 2014-04-08 13:29 ` [dpdk-dev] [PATCH v3 1/2] " Didier Pallard 2014-04-08 13:29 ` [dpdk-dev] [PATCH v3 2/2] igb: " Didier Pallard @ 2014-04-09 15:49 ` Ananyev, Konstantin 2014-04-09 16:37 ` Thomas Monjalon 1 sibling, 1 reply; 16+ messages in thread From: Ananyev, Konstantin @ 2014-04-09 15:49 UTC (permalink / raw) To: Didier Pallard, dev -----Original Message----- From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Didier Pallard Sent: Tuesday, April 08, 2014 2:30 PM To: dev@dpdk.org Subject: [dpdk-dev] [PATCH v3 1/2] ixgbe: release software locked semaphores on initialization It may happen that DPDK application gets killed while having acquired locks on the ethernet hardware, causing these locks to be never released. On next restart of the application, DPDK skip those ports because it can not acquire the lock, this may cause some ports (or even complete board if SMBI is locked) to be inaccessible from DPDK application until reboot of the hardware. This patch release locks that are supposed to be locked due to an improper exit of the application. Signed-off-by: Didier Pallard <didier.pallard@6wind.com> --- Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com> ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [dpdk-dev] [PATCH v3 1/2] ixgbe: release software locked semaphores on initialization 2014-04-09 15:49 ` [dpdk-dev] [PATCH v3 1/2] ixgbe: " Ananyev, Konstantin @ 2014-04-09 16:37 ` Thomas Monjalon 0 siblings, 0 replies; 16+ messages in thread From: Thomas Monjalon @ 2014-04-09 16:37 UTC (permalink / raw) To: Ananyev, Konstantin, Didier Pallard; +Cc: dev > It may happen that DPDK application gets killed while having acquired locks > on the ethernet hardware, causing these locks to be never released. On next > restart of the application, DPDK skip those ports because it can not > acquire the lock, this may cause some ports (or even complete board if SMBI > is locked) to be inaccessible from DPDK application until reboot of the > hardware. > > This patch release locks that are supposed to be locked due to an improper > exit of the application. > > Signed-off-by: Didier Pallard <didier.pallard@6wind.com> > Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com> Applied for version 1.6.0r2. So the decision is to try not modify base driver. But the fix should appear in the base driver one day and we must be careful when rebasing on an updated one. In this case, the fix can be done on both side without being harmful. Thanks for fixing this random failure. -- Thomas ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2014-04-09 16:38 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-02-19 11:59 [dpdk-dev] [RFC PATCH 1/2] e1000: release software locked semaphores on initialization Didier Pallard 2014-02-19 11:59 ` [dpdk-dev] [RFC PATCH 2/2] ixgbe: " Didier Pallard 2014-02-19 12:41 ` Ananyev, Konstantin 2014-02-19 16:52 ` Thomas Monjalon 2014-02-19 17:51 ` Ananyev, Konstantin 2014-02-21 16:30 ` Ananyev, Konstantin 2014-02-24 14:19 ` didier.pallard 2014-02-24 15:34 ` Ananyev, Konstantin 2014-02-25 0:57 ` Ananyev, Konstantin 2014-02-26 10:52 ` didier.pallard 2014-04-08 13:29 ` [dpdk-dev] [PATCH v3 1/2] " Didier Pallard 2014-04-08 13:29 ` [dpdk-dev] [PATCH v3 2/2] igb: " Didier Pallard 2014-04-09 15:58 ` Ananyev, Konstantin 2014-04-09 16:39 ` Thomas Monjalon 2014-04-09 15:49 ` [dpdk-dev] [PATCH v3 1/2] ixgbe: " Ananyev, Konstantin 2014-04-09 16:37 ` Thomas Monjalon
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).