Re: [Intel-wired-lan] [iwl-next v3 1/8] ice: devlink PF MSI-X max and min parameter
Fri, Aug 09, 2024 at 07:13:34AM CEST, michal.swiatkow...@linux.intel.com wrote: >On Thu, Aug 08, 2024 at 05:34:35PM +0200, Jiri Pirko wrote: >> Thu, Aug 08, 2024 at 09:20:09AM CEST, michal.swiatkow...@linux.intel.com >> wrote: >> >Use generic devlink PF MSI-X parameter to allow user to change MSI-X >> >range. >> > >> >Reviewed-by: Wojciech Drewek >> >Signed-off-by: Michal Swiatkowski >> >--- >> > .../net/ethernet/intel/ice/devlink/devlink.c | 56 ++- >> > drivers/net/ethernet/intel/ice/ice.h | 8 +++ >> > drivers/net/ethernet/intel/ice/ice_irq.c | 14 - >> > 3 files changed, 76 insertions(+), 2 deletions(-) >> > >> >diff --git a/drivers/net/ethernet/intel/ice/devlink/devlink.c >> >b/drivers/net/ethernet/intel/ice/devlink/devlink.c >> >index 29a5f822cb8b..bdc22ea13e0f 100644 >> >--- a/drivers/net/ethernet/intel/ice/devlink/devlink.c >> >+++ b/drivers/net/ethernet/intel/ice/devlink/devlink.c >> >@@ -1518,6 +1518,32 @@ static int ice_devlink_local_fwd_validate(struct >> >devlink *devlink, u32 id, >> >return 0; >> > } >> > >> >+static int >> >+ice_devlink_msix_max_pf_validate(struct devlink *devlink, u32 id, >> >+union devlink_param_value val, >> >+struct netlink_ext_ack *extack) >> >+{ >> >+ if (val.vu16 > ICE_MAX_MSIX) { >> >+ NL_SET_ERR_MSG_MOD(extack, "PF max MSI-X is too high"); >> >> No reason to have "PF" in the text. Also, no reason to have "max MSI-X". >> That is the name of the param. >> > >Ok, will change both, thanks. > >> >> >> >+ return -EINVAL; >> >+ } >> >+ >> >+ return 0; >> >+} >> >+ >> >+static int >> >+ice_devlink_msix_min_pf_validate(struct devlink *devlink, u32 id, >> >+union devlink_param_value val, >> >+struct netlink_ext_ack *extack) >> >+{ >> >+ if (val.vu16 <= ICE_MIN_MSIX) { >> >+ NL_SET_ERR_MSG_MOD(extack, "PF min MSI-X is too low"); >> >> Same comment as for max goes here. >> >> >> >+ return -EINVAL; >> >+ } >> >+ >> >+ return 0; >> >+} >> >+ >> > enum ice_param_id { >> >ICE_DEVLINK_PARAM_ID_BASE = DEVLINK_PARAM_GENERIC_ID_MAX, >> >ICE_DEVLINK_PARAM_ID_TX_SCHED_LAYERS, >> >@@ -1535,6 +1561,15 @@ static const struct devlink_param >> >ice_dvl_rdma_params[] = { >> > ice_devlink_enable_iw_validate), >> > }; >> > >> >+static const struct devlink_param ice_dvl_msix_params[] = { >> >+ DEVLINK_PARAM_GENERIC(MSIX_VEC_PER_PF_MAX, >> >+ BIT(DEVLINK_PARAM_CMODE_DRIVERINIT), >> >+ NULL, NULL, ice_devlink_msix_max_pf_validate), >> >+ DEVLINK_PARAM_GENERIC(MSIX_VEC_PER_PF_MIN, >> >+ BIT(DEVLINK_PARAM_CMODE_DRIVERINIT), >> >+ NULL, NULL, ice_devlink_msix_min_pf_validate), >> >+}; >> >+ >> > static const struct devlink_param ice_dvl_sched_params[] = { >> >DEVLINK_PARAM_DRIVER(ICE_DEVLINK_PARAM_ID_TX_SCHED_LAYERS, >> > "tx_scheduling_layers", >> >@@ -1637,6 +1672,7 @@ void ice_devlink_unregister(struct ice_pf *pf) >> > int ice_devlink_register_params(struct ice_pf *pf) >> > { >> >struct devlink *devlink = priv_to_devlink(pf); >> >+ union devlink_param_value value; >> >struct ice_hw *hw = &pf->hw; >> >int status; >> > >> >@@ -1645,11 +1681,27 @@ int ice_devlink_register_params(struct ice_pf *pf) >> >if (status) >> >return status; >> > >> >+ status = devl_params_register(devlink, ice_dvl_msix_params, >> >+ ARRAY_SIZE(ice_dvl_msix_params)); >> >+ if (status) >> >+ return status; >> >+ >> >if (hw->func_caps.common_cap.tx_sched_topo_comp_mode_en) >> >status = devl_params_register(devlink, ice_dvl_sched_params, >> > ARRAY_SIZE(ice_dvl_sched_params)); >> >+ if (status) >> >+ return status; >> > >> >- return status; >> >+ value.vu16 = pf->msix.max; >> >+ devl_param_driverinit_value_set(devlink, >> >+ >> >DEVLINK_PARAM_GENERIC_ID_MSIX_VEC_PER_PF_MAX, >> >+ value); >> >+ value.vu16 = pf->msix.min; >> >+ devl_param_driverinit_value_set(devlink, >> >+ >> >DEVLINK_PARAM_GENERIC_ID_MSIX_VEC_PER_PF_MIN, >> >+ value); >> >+ >> >+ return 0; >> > } >> > >> > void ice_devlink_unregister_params(struct ice_pf *pf) >> >@@ -1659,6 +1711,8 @@ void ice_devlink_unregister_params(struct ice_pf *pf) >> > >> >devl_params_unregister(devlink, ice_dvl_rdma_params, >> > ARRAY_SIZE(ice_dvl_rdma_params)); >> >+ devl_params_unregister(devlink, ice_dvl_msix_params, >> >+ ARRAY_SIZE(ice_dvl_msix_params)); >> > >> >if (hw->func_caps.common_cap.tx_sched_topo_comp_mode_en) >> >devl_params_unregister(de
Re: [Intel-wired-lan] [iwl-next v3 1/8] ice: devlink PF MSI-X max and min parameter
Fri, Aug 09, 2024 at 07:18:38AM CEST, pmen...@molgen.mpg.de wrote: >Dear Michal, > > >Thank you for your patch. > >Am 09.08.24 um 07:13 schrieb Michal Swiatkowski: >> On Thu, Aug 08, 2024 at 05:34:35PM +0200, Jiri Pirko wrote: >> > Thu, Aug 08, 2024 at 09:20:09AM CEST, michal.swiatkow...@linux.intel.com >> > wrote: >> > > Use generic devlink PF MSI-X parameter to allow user to change MSI-X >> > > range. >> > > >> > > Reviewed-by: Wojciech Drewek >> > > Signed-off-by: Michal Swiatkowski >> > > --- >> > > .../net/ethernet/intel/ice/devlink/devlink.c | 56 ++- >> > > drivers/net/ethernet/intel/ice/ice.h | 8 +++ >> > > drivers/net/ethernet/intel/ice/ice_irq.c | 14 - >> > > 3 files changed, 76 insertions(+), 2 deletions(-) >> > > >> > > diff --git a/drivers/net/ethernet/intel/ice/devlink/devlink.c >> > > b/drivers/net/ethernet/intel/ice/devlink/devlink.c >> > > index 29a5f822cb8b..bdc22ea13e0f 100644 >> > > --- a/drivers/net/ethernet/intel/ice/devlink/devlink.c >> > > +++ b/drivers/net/ethernet/intel/ice/devlink/devlink.c >> > > @@ -1518,6 +1518,32 @@ static int ice_devlink_local_fwd_validate(struct >> > > devlink *devlink, u32 id, >> > > return 0; >> > > } >> > > >> > > +static int >> > > +ice_devlink_msix_max_pf_validate(struct devlink *devlink, u32 id, >> > > + union devlink_param_value val, >> > > + struct netlink_ext_ack *extack) >> > > +{ >> > > +if (val.vu16 > ICE_MAX_MSIX) { >> > > +NL_SET_ERR_MSG_MOD(extack, "PF max MSI-X is too high"); >> > >> > No reason to have "PF" in the text. Also, no reason to have "max MSI-X". >> > That is the name of the param. >> >> Ok, will change both, thanks. > >Maybe also print both values in the error message? Why? The user is passing the value. Does not make any sense. > >> > > +return -EINVAL; >> > > +} >> > > + >> > > +return 0; >> > > +} > >[…] > > >Kind regards, > >Paul
[Intel-wired-lan] [tnguy-next-queue:main] BUILD SUCCESS 91d516d4de48532d967a77967834e00c8c53dfe6
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue.git main branch HEAD: 91d516d4de48532d967a77967834e00c8c53dfe6 net: mvpp2: Increase size of queue_name buffer elapsed time: 1890m configs tested: 282 configs skipped: 10 The following configs have been built successfully. More configs may be tested in the coming days. tested configs: alpha allnoconfig gcc-13.2.0 alpha allnoconfig gcc-13.3.0 alphaallyesconfig gcc-13.3.0 alpha defconfig gcc-13.2.0 arc allmodconfig gcc-13.2.0 arc allnoconfig gcc-13.2.0 arc allyesconfig gcc-13.2.0 arc axs101_defconfig gcc-13.2.0 arc defconfig gcc-13.2.0 arc randconfig-001-20240808 gcc-13.2.0 arc randconfig-001-20240809 gcc-13.2.0 arc randconfig-002-20240808 gcc-13.2.0 arc randconfig-002-20240809 gcc-13.2.0 arm allmodconfig gcc-13.2.0 arm allnoconfig clang-20 arm allnoconfig gcc-13.2.0 arm allyesconfig gcc-13.2.0 arm assabet_defconfig clang-15 arm collie_defconfig clang-15 arm defconfig gcc-13.2.0 armmmp2_defconfig clang-15 armmmp2_defconfig gcc-14.1.0 arm moxart_defconfig clang-15 armneponset_defconfig gcc-13.2.0 arm netwinder_defconfig gcc-14.1.0 arm randconfig-001-20240808 gcc-13.2.0 arm randconfig-001-20240809 gcc-13.2.0 arm randconfig-002-20240808 gcc-13.2.0 arm randconfig-002-20240809 gcc-13.2.0 arm randconfig-003-20240808 gcc-13.2.0 arm randconfig-003-20240809 gcc-13.2.0 arm randconfig-004-20240808 gcc-13.2.0 arm randconfig-004-20240809 gcc-13.2.0 arm rpc_defconfig gcc-13.2.0 arm s3c6400_defconfig clang-15 arm s3c6400_defconfig gcc-13.2.0 arm spear13xx_defconfig clang-15 arm spitz_defconfig gcc-14.1.0 arm stm32_defconfig gcc-13.2.0 arm64allmodconfig gcc-13.2.0 arm64 allnoconfig gcc-13.2.0 arm64 allnoconfig gcc-14.1.0 arm64 defconfig gcc-13.2.0 arm64 randconfig-001-20240808 gcc-13.2.0 arm64 randconfig-001-20240809 gcc-13.2.0 arm64 randconfig-002-20240808 gcc-13.2.0 arm64 randconfig-002-20240809 gcc-13.2.0 arm64 randconfig-003-20240808 gcc-13.2.0 arm64 randconfig-003-20240809 gcc-13.2.0 arm64 randconfig-004-20240808 gcc-13.2.0 arm64 randconfig-004-20240809 gcc-13.2.0 csky allnoconfig gcc-13.2.0 csky allnoconfig gcc-14.1.0 cskydefconfig gcc-13.2.0 csky randconfig-001-20240808 gcc-13.2.0 csky randconfig-001-20240809 gcc-13.2.0 csky randconfig-002-20240808 gcc-13.2.0 csky randconfig-002-20240809 gcc-13.2.0 hexagon allmodconfig clang-20 hexagon allnoconfig clang-20 hexagon allyesconfig clang-20 i386 allmodconfig clang-18 i386 allnoconfig clang-18 i386 allyesconfig clang-18 i386 buildonly-randconfig-001-20240808 clang-18 i386 buildonly-randconfig-001-20240809 gcc-12 i386 buildonly-randconfig-002-20240808 clang-18 i386 buildonly-randconfig-002-20240809 gcc-12 i386 buildonly-randconfig-003-20240808 clang-18 i386 buildonly-randconfig-003-20240809 gcc-12 i386 buildonly-randconfig-004-20240808 clang-18 i386 buildonly-randconfig-004-20240809 gcc-12 i386 buildonly-randconfig-005-20240808 clang-18 i386 buildonly-randconfig-005-20240809 gcc-12 i386 buildonly-randconfig-006-20240808 clang-18 i386 buildonly-randconfig-006-20240809 gcc-12 i386defconfig clang-18 i386 randconfig-001-20240808 clang-18 i386 randconfig-001-20240809 gcc-12 i386
Re: [Intel-wired-lan] [iwl-next v3 1/8] ice: devlink PF MSI-X max and min parameter
On Fri, Aug 09, 2024 at 12:51:58PM +0200, Jiri Pirko wrote: > Fri, Aug 09, 2024 at 07:13:34AM CEST, michal.swiatkow...@linux.intel.com > wrote: > >On Thu, Aug 08, 2024 at 05:34:35PM +0200, Jiri Pirko wrote: > >> Thu, Aug 08, 2024 at 09:20:09AM CEST, michal.swiatkow...@linux.intel.com > >> wrote: > >> >Use generic devlink PF MSI-X parameter to allow user to change MSI-X > >> >range. > >> > > >> >Reviewed-by: Wojciech Drewek > >> >Signed-off-by: Michal Swiatkowski > >> >--- > >> > .../net/ethernet/intel/ice/devlink/devlink.c | 56 ++- > >> > drivers/net/ethernet/intel/ice/ice.h | 8 +++ > >> > drivers/net/ethernet/intel/ice/ice_irq.c | 14 - > >> > 3 files changed, 76 insertions(+), 2 deletions(-) > >> > > >> >diff --git a/drivers/net/ethernet/intel/ice/devlink/devlink.c > >> >b/drivers/net/ethernet/intel/ice/devlink/devlink.c > >> >index 29a5f822cb8b..bdc22ea13e0f 100644 > >> >--- a/drivers/net/ethernet/intel/ice/devlink/devlink.c > >> >+++ b/drivers/net/ethernet/intel/ice/devlink/devlink.c > >> >@@ -1518,6 +1518,32 @@ static int ice_devlink_local_fwd_validate(struct > >> >devlink *devlink, u32 id, > >> > return 0; > >> > } > >> > > >> >+static int > >> >+ice_devlink_msix_max_pf_validate(struct devlink *devlink, u32 id, > >> >+ union devlink_param_value val, > >> >+ struct netlink_ext_ack *extack) > >> >+{ > >> >+ if (val.vu16 > ICE_MAX_MSIX) { > >> >+ NL_SET_ERR_MSG_MOD(extack, "PF max MSI-X is too high"); > >> > >> No reason to have "PF" in the text. Also, no reason to have "max MSI-X". > >> That is the name of the param. > >> > > > >Ok, will change both, thanks. > > > >> > >> > >> >+ return -EINVAL; > >> >+ } > >> >+ > >> >+ return 0; > >> >+} > >> >+ > >> >+static int > >> >+ice_devlink_msix_min_pf_validate(struct devlink *devlink, u32 id, > >> >+ union devlink_param_value val, > >> >+ struct netlink_ext_ack *extack) > >> >+{ > >> >+ if (val.vu16 <= ICE_MIN_MSIX) { > >> >+ NL_SET_ERR_MSG_MOD(extack, "PF min MSI-X is too low"); > >> > >> Same comment as for max goes here. > >> > >> > >> >+ return -EINVAL; > >> >+ } > >> >+ > >> >+ return 0; > >> >+} > >> >+ > >> > enum ice_param_id { > >> > ICE_DEVLINK_PARAM_ID_BASE = DEVLINK_PARAM_GENERIC_ID_MAX, > >> > ICE_DEVLINK_PARAM_ID_TX_SCHED_LAYERS, > >> >@@ -1535,6 +1561,15 @@ static const struct devlink_param > >> >ice_dvl_rdma_params[] = { > >> >ice_devlink_enable_iw_validate), > >> > }; > >> > > >> >+static const struct devlink_param ice_dvl_msix_params[] = { > >> >+ DEVLINK_PARAM_GENERIC(MSIX_VEC_PER_PF_MAX, > >> >+ BIT(DEVLINK_PARAM_CMODE_DRIVERINIT), > >> >+ NULL, NULL, ice_devlink_msix_max_pf_validate), > >> >+ DEVLINK_PARAM_GENERIC(MSIX_VEC_PER_PF_MIN, > >> >+ BIT(DEVLINK_PARAM_CMODE_DRIVERINIT), > >> >+ NULL, NULL, ice_devlink_msix_min_pf_validate), > >> >+}; > >> >+ > >> > static const struct devlink_param ice_dvl_sched_params[] = { > >> > DEVLINK_PARAM_DRIVER(ICE_DEVLINK_PARAM_ID_TX_SCHED_LAYERS, > >> > "tx_scheduling_layers", > >> >@@ -1637,6 +1672,7 @@ void ice_devlink_unregister(struct ice_pf *pf) > >> > int ice_devlink_register_params(struct ice_pf *pf) > >> > { > >> > struct devlink *devlink = priv_to_devlink(pf); > >> >+ union devlink_param_value value; > >> > struct ice_hw *hw = &pf->hw; > >> > int status; > >> > > >> >@@ -1645,11 +1681,27 @@ int ice_devlink_register_params(struct ice_pf *pf) > >> > if (status) > >> > return status; > >> > > >> >+ status = devl_params_register(devlink, ice_dvl_msix_params, > >> >+ ARRAY_SIZE(ice_dvl_msix_params)); > >> >+ if (status) > >> >+ return status; > >> >+ > >> > if (hw->func_caps.common_cap.tx_sched_topo_comp_mode_en) > >> > status = devl_params_register(devlink, ice_dvl_sched_params, > >> >ARRAY_SIZE(ice_dvl_sched_params)); > >> >+ if (status) > >> >+ return status; > >> > > >> >- return status; > >> >+ value.vu16 = pf->msix.max; > >> >+ devl_param_driverinit_value_set(devlink, > >> >+ > >> >DEVLINK_PARAM_GENERIC_ID_MSIX_VEC_PER_PF_MAX, > >> >+ value); > >> >+ value.vu16 = pf->msix.min; > >> >+ devl_param_driverinit_value_set(devlink, > >> >+ > >> >DEVLINK_PARAM_GENERIC_ID_MSIX_VEC_PER_PF_MIN, > >> >+ value); > >> >+ > >> >+ return 0; > >> > } > >> > > >> > void ice_devlink_unregister_params(struct ice_pf *pf) > >> >@@ -1659,6 +1711,8 @@ void ice_devlink_unregister_params(struct ice_pf > >> >*pf) > >> > > >> > devl_params_unregister(devlink, ice_dvl_rdma_params, > >> > ARRAY_SIZE(ice_dvl_rdma_params)); > >> >+ devl_params_
Re: [Intel-wired-lan] [iwl-next v3 1/8] ice: devlink PF MSI-X max and min parameter
Fri, Aug 09, 2024 at 01:05:00PM CEST, michal.swiatkow...@linux.intel.com wrote: >On Fri, Aug 09, 2024 at 12:51:58PM +0200, Jiri Pirko wrote: >> Fri, Aug 09, 2024 at 07:13:34AM CEST, michal.swiatkow...@linux.intel.com >> wrote: >> >On Thu, Aug 08, 2024 at 05:34:35PM +0200, Jiri Pirko wrote: >> >> Thu, Aug 08, 2024 at 09:20:09AM CEST, michal.swiatkow...@linux.intel.com >> >> wrote: >> >> >Use generic devlink PF MSI-X parameter to allow user to change MSI-X >> >> >range. >> >> > >> >> >Reviewed-by: Wojciech Drewek >> >> >Signed-off-by: Michal Swiatkowski >> >> >--- >> >> > .../net/ethernet/intel/ice/devlink/devlink.c | 56 ++- >> >> > drivers/net/ethernet/intel/ice/ice.h | 8 +++ >> >> > drivers/net/ethernet/intel/ice/ice_irq.c | 14 - >> >> > 3 files changed, 76 insertions(+), 2 deletions(-) >> >> > >> >> >diff --git a/drivers/net/ethernet/intel/ice/devlink/devlink.c >> >> >b/drivers/net/ethernet/intel/ice/devlink/devlink.c >> >> >index 29a5f822cb8b..bdc22ea13e0f 100644 >> >> >--- a/drivers/net/ethernet/intel/ice/devlink/devlink.c >> >> >+++ b/drivers/net/ethernet/intel/ice/devlink/devlink.c >> >> >@@ -1518,6 +1518,32 @@ static int ice_devlink_local_fwd_validate(struct >> >> >devlink *devlink, u32 id, >> >> > return 0; >> >> > } >> >> > >> >> >+static int >> >> >+ice_devlink_msix_max_pf_validate(struct devlink *devlink, u32 id, >> >> >+ union devlink_param_value val, >> >> >+ struct netlink_ext_ack *extack) >> >> >+{ >> >> >+if (val.vu16 > ICE_MAX_MSIX) { >> >> >+NL_SET_ERR_MSG_MOD(extack, "PF max MSI-X is too high"); >> >> >> >> No reason to have "PF" in the text. Also, no reason to have "max MSI-X". >> >> That is the name of the param. >> >> >> > >> >Ok, will change both, thanks. >> > >> >> >> >> >> >> >+return -EINVAL; >> >> >+} >> >> >+ >> >> >+return 0; >> >> >+} >> >> >+ >> >> >+static int >> >> >+ice_devlink_msix_min_pf_validate(struct devlink *devlink, u32 id, >> >> >+ union devlink_param_value val, >> >> >+ struct netlink_ext_ack *extack) >> >> >+{ >> >> >+if (val.vu16 <= ICE_MIN_MSIX) { >> >> >+NL_SET_ERR_MSG_MOD(extack, "PF min MSI-X is too low"); >> >> >> >> Same comment as for max goes here. >> >> >> >> >> >> >+return -EINVAL; >> >> >+} >> >> >+ >> >> >+return 0; >> >> >+} >> >> >+ >> >> > enum ice_param_id { >> >> > ICE_DEVLINK_PARAM_ID_BASE = DEVLINK_PARAM_GENERIC_ID_MAX, >> >> > ICE_DEVLINK_PARAM_ID_TX_SCHED_LAYERS, >> >> >@@ -1535,6 +1561,15 @@ static const struct devlink_param >> >> >ice_dvl_rdma_params[] = { >> >> > ice_devlink_enable_iw_validate), >> >> > }; >> >> > >> >> >+static const struct devlink_param ice_dvl_msix_params[] = { >> >> >+DEVLINK_PARAM_GENERIC(MSIX_VEC_PER_PF_MAX, >> >> >+ BIT(DEVLINK_PARAM_CMODE_DRIVERINIT), >> >> >+ NULL, NULL, >> >> >ice_devlink_msix_max_pf_validate), >> >> >+DEVLINK_PARAM_GENERIC(MSIX_VEC_PER_PF_MIN, >> >> >+ BIT(DEVLINK_PARAM_CMODE_DRIVERINIT), >> >> >+ NULL, NULL, >> >> >ice_devlink_msix_min_pf_validate), >> >> >+}; >> >> >+ >> >> > static const struct devlink_param ice_dvl_sched_params[] = { >> >> > DEVLINK_PARAM_DRIVER(ICE_DEVLINK_PARAM_ID_TX_SCHED_LAYERS, >> >> > "tx_scheduling_layers", >> >> >@@ -1637,6 +1672,7 @@ void ice_devlink_unregister(struct ice_pf *pf) >> >> > int ice_devlink_register_params(struct ice_pf *pf) >> >> > { >> >> > struct devlink *devlink = priv_to_devlink(pf); >> >> >+union devlink_param_value value; >> >> > struct ice_hw *hw = &pf->hw; >> >> > int status; >> >> > >> >> >@@ -1645,11 +1681,27 @@ int ice_devlink_register_params(struct ice_pf >> >> >*pf) >> >> > if (status) >> >> > return status; >> >> > >> >> >+status = devl_params_register(devlink, ice_dvl_msix_params, >> >> >+ ARRAY_SIZE(ice_dvl_msix_params)); >> >> >+if (status) >> >> >+return status; >> >> >+ >> >> > if (hw->func_caps.common_cap.tx_sched_topo_comp_mode_en) >> >> > status = devl_params_register(devlink, >> >> > ice_dvl_sched_params, >> >> > >> >> > ARRAY_SIZE(ice_dvl_sched_params)); >> >> >+if (status) >> >> >+return status; >> >> > >> >> >-return status; >> >> >+value.vu16 = pf->msix.max; >> >> >+devl_param_driverinit_value_set(devlink, >> >> >+ >> >> >DEVLINK_PARAM_GENERIC_ID_MSIX_VEC_PER_PF_MAX, >> >> >+value); >> >> >+
Re: [Intel-wired-lan] [iwl-next v3 1/8] ice: devlink PF MSI-X max and min parameter
On Fri, Aug 09, 2024 at 01:29:29PM +0200, Jiri Pirko wrote: > Fri, Aug 09, 2024 at 01:05:00PM CEST, michal.swiatkow...@linux.intel.com > wrote: > >On Fri, Aug 09, 2024 at 12:51:58PM +0200, Jiri Pirko wrote: > >> Fri, Aug 09, 2024 at 07:13:34AM CEST, michal.swiatkow...@linux.intel.com > >> wrote: > >> >On Thu, Aug 08, 2024 at 05:34:35PM +0200, Jiri Pirko wrote: > >> >> Thu, Aug 08, 2024 at 09:20:09AM CEST, > >> >> michal.swiatkow...@linux.intel.com wrote: > >> >> >Use generic devlink PF MSI-X parameter to allow user to change MSI-X > >> >> >range. > >> >> > > >> >> >Reviewed-by: Wojciech Drewek > >> >> >Signed-off-by: Michal Swiatkowski > >> >> >--- > >> >> > .../net/ethernet/intel/ice/devlink/devlink.c | 56 ++- > >> >> > drivers/net/ethernet/intel/ice/ice.h | 8 +++ > >> >> > drivers/net/ethernet/intel/ice/ice_irq.c | 14 - > >> >> > 3 files changed, 76 insertions(+), 2 deletions(-) > >> >> > > >> >> >diff --git a/drivers/net/ethernet/intel/ice/devlink/devlink.c > >> >> >b/drivers/net/ethernet/intel/ice/devlink/devlink.c > >> >> >index 29a5f822cb8b..bdc22ea13e0f 100644 > >> >> >--- a/drivers/net/ethernet/intel/ice/devlink/devlink.c > >> >> >+++ b/drivers/net/ethernet/intel/ice/devlink/devlink.c > >> >> >@@ -1518,6 +1518,32 @@ static int > >> >> >ice_devlink_local_fwd_validate(struct devlink *devlink, u32 id, > >> >> > return 0; > >> >> > } > >> >> > > >> >> >+static int > >> >> >+ice_devlink_msix_max_pf_validate(struct devlink *devlink, u32 id, > >> >> >+ union devlink_param_value val, > >> >> >+ struct netlink_ext_ack *extack) > >> >> >+{ > >> >> >+ if (val.vu16 > ICE_MAX_MSIX) { > >> >> >+ NL_SET_ERR_MSG_MOD(extack, "PF max MSI-X is too high"); > >> >> > >> >> No reason to have "PF" in the text. Also, no reason to have "max MSI-X". > >> >> That is the name of the param. > >> >> > >> > > >> >Ok, will change both, thanks. > >> > > >> >> > >> >> > >> >> >+ return -EINVAL; > >> >> >+ } > >> >> >+ > >> >> >+ return 0; > >> >> >+} > >> >> >+ > >> >> >+static int > >> >> >+ice_devlink_msix_min_pf_validate(struct devlink *devlink, u32 id, > >> >> >+ union devlink_param_value val, > >> >> >+ struct netlink_ext_ack *extack) > >> >> >+{ > >> >> >+ if (val.vu16 <= ICE_MIN_MSIX) { > >> >> >+ NL_SET_ERR_MSG_MOD(extack, "PF min MSI-X is too low"); > >> >> > >> >> Same comment as for max goes here. > >> >> > >> >> > >> >> >+ return -EINVAL; > >> >> >+ } > >> >> >+ > >> >> >+ return 0; > >> >> >+} > >> >> >+ > >> >> > enum ice_param_id { > >> >> > ICE_DEVLINK_PARAM_ID_BASE = DEVLINK_PARAM_GENERIC_ID_MAX, > >> >> > ICE_DEVLINK_PARAM_ID_TX_SCHED_LAYERS, > >> >> >@@ -1535,6 +1561,15 @@ static const struct devlink_param > >> >> >ice_dvl_rdma_params[] = { > >> >> > ice_devlink_enable_iw_validate), > >> >> > }; > >> >> > > >> >> >+static const struct devlink_param ice_dvl_msix_params[] = { > >> >> >+ DEVLINK_PARAM_GENERIC(MSIX_VEC_PER_PF_MAX, > >> >> >+BIT(DEVLINK_PARAM_CMODE_DRIVERINIT), > >> >> >+NULL, NULL, > >> >> >ice_devlink_msix_max_pf_validate), > >> >> >+ DEVLINK_PARAM_GENERIC(MSIX_VEC_PER_PF_MIN, > >> >> >+BIT(DEVLINK_PARAM_CMODE_DRIVERINIT), > >> >> >+NULL, NULL, > >> >> >ice_devlink_msix_min_pf_validate), > >> >> >+}; > >> >> >+ > >> >> > static const struct devlink_param ice_dvl_sched_params[] = { > >> >> > DEVLINK_PARAM_DRIVER(ICE_DEVLINK_PARAM_ID_TX_SCHED_LAYERS, > >> >> >"tx_scheduling_layers", > >> >> >@@ -1637,6 +1672,7 @@ void ice_devlink_unregister(struct ice_pf *pf) > >> >> > int ice_devlink_register_params(struct ice_pf *pf) > >> >> > { > >> >> > struct devlink *devlink = priv_to_devlink(pf); > >> >> >+ union devlink_param_value value; > >> >> > struct ice_hw *hw = &pf->hw; > >> >> > int status; > >> >> > > >> >> >@@ -1645,11 +1681,27 @@ int ice_devlink_register_params(struct ice_pf > >> >> >*pf) > >> >> > if (status) > >> >> > return status; > >> >> > > >> >> >+ status = devl_params_register(devlink, ice_dvl_msix_params, > >> >> >+ARRAY_SIZE(ice_dvl_msix_params)); > >> >> >+ if (status) > >> >> >+ return status; > >> >> >+ > >> >> > if (hw->func_caps.common_cap.tx_sched_topo_comp_mode_en) > >> >> > status = devl_params_register(devlink, > >> >> > ice_dvl_sched_params, > >> >> > > >> >> > ARRAY_SIZE(ice_dvl_sched_params)); > >> >> >+ if (status) > >> >> >+ return status; > >> >> > > >> >> >- return status; > >> >> >+ value.vu16 = pf->msix.max; > >> >> >+ d
Re: [Intel-wired-lan] [PATCH iwl-next v2] ice: add E830 HW VF mailbox message limit support
From: Paul Greenwalt Date: Wed, 31 Jul 2024 21:58:29 -0400 > E830 adds hardware support to prevent the VF from overflowing the PF > mailbox with VIRTCHNL messages. E830 will use the hardware feature > (ICE_F_MBX_LIMIT) instead of the software solution ice_is_malicious_vf(). > > To prevent a VF from overflowing the PF, the PF sets the number of > messages per VF that can be in the PF's mailbox queue > (ICE_MBX_OVERFLOW_WATERMARK). When the PF process a message from a VF, > the PF decrements the per VF message count using the E830_MBX_VF_DEC_TRIG > register. > > Signed-off-by: Paul Greenwalt > --- > v1 -> v2: > - Update ice_mbx_vf_dec_trig_e830 and ice_mbx_vf_clear_cnt_e830 onstack > variables to const > --- > drivers/net/ethernet/intel/ice/ice.h | 1 + > .../net/ethernet/intel/ice/ice_hw_autogen.h | 3 ++ > drivers/net/ethernet/intel/ice/ice_lib.c | 12 +++ > drivers/net/ethernet/intel/ice/ice_main.c | 24 ++ > drivers/net/ethernet/intel/ice/ice_sriov.c| 3 +- > drivers/net/ethernet/intel/ice/ice_vf_lib.c | 26 +-- > drivers/net/ethernet/intel/ice/ice_vf_mbx.c | 32 +++ > drivers/net/ethernet/intel/ice/ice_vf_mbx.h | 3 ++ > drivers/net/ethernet/intel/ice/ice_virtchnl.c | 8 +++-- > 9 files changed, 99 insertions(+), 13 deletions(-) > > diff --git a/drivers/net/ethernet/intel/ice/ice.h > b/drivers/net/ethernet/intel/ice/ice.h > index 4c563b0d57ac..53c8edbfaede 100644 > --- a/drivers/net/ethernet/intel/ice/ice.h > +++ b/drivers/net/ethernet/intel/ice/ice.h > @@ -207,6 +207,7 @@ enum ice_feature { > ICE_F_GNSS, > ICE_F_ROCE_LAG, > ICE_F_SRIOV_LAG, > + ICE_F_MBX_LIMIT, > ICE_F_MAX > }; > > diff --git a/drivers/net/ethernet/intel/ice/ice_hw_autogen.h > b/drivers/net/ethernet/intel/ice/ice_hw_autogen.h > index 91cbae1eec89..a306ea9b207c 100644 > --- a/drivers/net/ethernet/intel/ice/ice_hw_autogen.h > +++ b/drivers/net/ethernet/intel/ice/ice_hw_autogen.h > @@ -539,5 +539,8 @@ > #define E830_PRTMAC_CL01_QNT_THR_CL0_M GENMASK(15, 0) > #define VFINT_DYN_CTLN(_i) (0x3800 + ((_i) * 4)) > #define VFINT_DYN_CTLN_CLEARPBA_MBIT(1) > +#define E830_MBX_PF_IN_FLIGHT_VF_MSGS_THRESH0x00234000 > +#define E830_MBX_VF_DEC_TRIG(_VF) (0x00233800 + ((_VF) * 4)) > +#define E830_MBX_VF_IN_FLIGHT_MSGS_AT_PF_CNT(_VF) (0x00233000 + ((_VF) * 4)) Still poor indentation, there must be tabs, no spaces. Also, parenthesis around `(_VF) * 4` are redundant. > > #endif /* _ICE_HW_AUTOGEN_H_ */ > diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c > b/drivers/net/ethernet/intel/ice/ice_lib.c > index fc5b87f51702..a6fa2ed6c4ab 100644 > --- a/drivers/net/ethernet/intel/ice/ice_lib.c > +++ b/drivers/net/ethernet/intel/ice/ice_lib.c > @@ -3938,6 +3938,18 @@ void ice_init_feature_support(struct ice_pf *pf) > if (ice_gnss_is_gps_present(&pf->hw)) > ice_set_feature_support(pf, ICE_F_GNSS); > break; > + case ICE_DEV_ID_E830CC_BACKPLANE: > + case ICE_DEV_ID_E830CC_QSFP56: > + case ICE_DEV_ID_E830CC_SFP: > + case ICE_DEV_ID_E830CC_SFP_DD: > + case ICE_DEV_ID_E830C_BACKPLANE: > + case ICE_DEV_ID_E830C_QSFP: > + case ICE_DEV_ID_E830C_SFP: > + case ICE_DEV_ID_E830_XXV_BACKPLANE: > + case ICE_DEV_ID_E830_XXV_QSFP: > + case ICE_DEV_ID_E830_XXV_SFP: Can't this be somehow compressed via case A .. B There are no holes between at least some of these. > + ice_set_feature_support(pf, ICE_F_MBX_LIMIT); > + break; > default: > break; > } [...] > diff --git a/drivers/net/ethernet/intel/ice/ice_vf_mbx.c > b/drivers/net/ethernet/intel/ice/ice_vf_mbx.c > index 40cb4ba0789c..65d9c41bed21 100644 > --- a/drivers/net/ethernet/intel/ice/ice_vf_mbx.c > +++ b/drivers/net/ethernet/intel/ice/ice_vf_mbx.c > @@ -210,6 +210,38 @@ ice_mbx_detect_malvf(struct ice_hw *hw, struct > ice_mbx_vf_info *vf_info, > return 0; > } > > +/** > + * ice_mbx_vf_dec_trig_e830 - Decrements the VF mailbox queue counter > + * @hw: pointer to the HW struct > + * @event: pointer to the control queue receive event > + * > + * This function triggers to decrement the counter > + * MBX_VF_IN_FLIGHT_MSGS_AT_PF_CNT when the driver replenishes > + * the buffers at the PF mailbox queue. > + */ > +void ice_mbx_vf_dec_trig_e830(const struct ice_hw *hw, > + const struct ice_rq_event_info *event) > +{ > + const u16 vfid = le16_to_cpu(event->desc.retval); Oops, I may've confused you. You don't need to constify scalars. Mainly pointers and structures when possible. > + > + wr32(hw, E830_MBX_VF_DEC_TRIG(vfid), 1); > +} > + > +/** > + * ice_mbx_vf_clear_cnt_e830 - Clear the VF mailbox queue count > + * @hw: pointer to the HW struct > + * @vf_id: VF ID in the PF space > + * > + * This function clears the counter MBX_VF_IN_FLIGHT_MSGS
[Intel-wired-lan] [PATCH] ice: Fix improper handling of refcount in ice_sriov_set_msix_vec_count()
This patch addresses an issue with improper reference count handling in the ice_sriov_set_msix_vec_count() function. Specifically, the function calls ice_get_vf_by_id(), which increments the reference count of the vf pointer. If the subsequent call to ice_get_vf_vsi() fails, the function currently returns an error without decrementing the reference count of the vf pointer, leading to a reference count leak. The correct behavior, as implemented in this patch, is to decrement the reference count using ice_put_vf(vf) before returning an error when vsi is NULL. This bug was identified by an experimental static analysis tool developed by our team. The tool specializes in analyzing reference count operations and identifying potential mismanagement of reference counts. In this case, the tool flagged the missing decrement operation as a potential issue, leading to this patch. Fixes: 4035c72dc1ba ("ice: reconfig host after changing MSI-X on VF") Cc: sta...@vger.kernel.org Signed-off-by: Gui-Dong Han --- drivers/net/ethernet/intel/ice/ice_sriov.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c b/drivers/net/ethernet/intel/ice/ice_sriov.c index 55ef33208456..eb5030aba9a5 100644 --- a/drivers/net/ethernet/intel/ice/ice_sriov.c +++ b/drivers/net/ethernet/intel/ice/ice_sriov.c @@ -1096,8 +1096,10 @@ int ice_sriov_set_msix_vec_count(struct pci_dev *vf_dev, int msix_vec_count) return -ENOENT; vsi = ice_get_vf_vsi(vf); - if (!vsi) + if (!vsi) { + ice_put_vf(vf); return -ENOENT; + } prev_msix = vf->num_msix; prev_queues = vf->num_vf_qs; -- 2.25.1
Re: [Intel-wired-lan] [iwl-net v2 2/2] igb: Fix missing time sync events
Hi, It appears this change breaks PTP on the 82580 controller, as ptp4l reports: > timed out while polling for tx timestamp increasing tx_timestamp_timeout or > increasing kworker priority may correct this issue, but a driver bug likely > causes it The 82580 controller has a hardware bug in which reading TSICR doesn't clear it. See this thread https://lore.kernel.org/all/cdcb8be0.1ec2c%25matthew.v...@intel.com/ where the bug was confirmed by an Intel employee. Any chance we could add back the ack for 82580 specifically? Thanks!
[Intel-wired-lan] [PATCH] ice: Fix improper handling of refcount in ice_dpll_init_rclk_pins()
This patch addresses a reference count handling issue in the ice_dpll_init_rclk_pins() function. The function calls ice_dpll_get_pins(), which increments the reference count of the relevant resources. However, if the condition WARN_ON((!vsi || !vsi->netdev)) is met, the function currently returns an error without properly releasing the resources acquired by ice_dpll_get_pins(), leading to a reference count leak. To resolve this, the patch introduces a goto unregister_pins; statement when the condition is met, ensuring that the resources are correctly released and the reference count is decremented before returning the error. This change prevents potential memory leaks and ensures proper resource management within the function. This bug was identified by an experimental static analysis tool developed by our team. The tool specializes in analyzing reference count operations and detecting potential issues where resources are not properly managed. In this case, the tool flagged the missing release operation as a potential problem, which led to the development of this patch. Fixes: d7999f5ea64b ("ice: implement dpll interface to control cgu") Cc: sta...@vger.kernel.org Signed-off-by: Gui-Dong Han --- drivers/net/ethernet/intel/ice/ice_dpll.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.c b/drivers/net/ethernet/intel/ice/ice_dpll.c index e92be6f130a3..f3f204cae093 100644 --- a/drivers/net/ethernet/intel/ice/ice_dpll.c +++ b/drivers/net/ethernet/intel/ice/ice_dpll.c @@ -1641,8 +1641,10 @@ ice_dpll_init_rclk_pins(struct ice_pf *pf, struct ice_dpll_pin *pin, if (ret) goto unregister_pins; } - if (WARN_ON((!vsi || !vsi->netdev))) - return -EINVAL; + if (WARN_ON((!vsi || !vsi->netdev))) { + ret = -EINVAL; + goto unregister_pins; + } dpll_netdev_pin_set(vsi->netdev, pf->dplls.rclk.pin); return 0; -- 2.25.1
Re: [Intel-wired-lan] [PATCH iwl-next v1] i40e: Add Energy Efficient Ethernet ability for X710 Base-T/KR/KX cards
On Thu, Aug 08, 2024 at 01:22:17PM +0200, Aleksandr Loktionov wrote: ... > diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c > b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c > index 1d0d2e5..cd7509f 100644 > --- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c > +++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c > @@ -5641,50 +5641,77 @@ static int i40e_get_module_eeprom(struct net_device > *netdev, > return 0; > } > > +static void i40e_eee_capability_to_kedata_supported(__le16 eee_capability_, > + unsigned long *supported) > +{ > + const int eee_capability = le16_to_cpu(eee_capability_); Hi Aleksandr, Maybe u16 would be an appropriate type for eee_capability. Also, using const seems excessive here. > + static const int lut[] = { > + ETHTOOL_LINK_MODE_100baseT_Full_BIT, > + ETHTOOL_LINK_MODE_1000baseT_Full_BIT, > + ETHTOOL_LINK_MODE_1baseT_Full_BIT, > + ETHTOOL_LINK_MODE_1000baseKX_Full_BIT, > + ETHTOOL_LINK_MODE_1baseKX4_Full_BIT, > + ETHTOOL_LINK_MODE_1baseKR_Full_BIT, > + ETHTOOL_LINK_MODE_4baseKR4_Full_BIT, > + }; > + > + linkmode_zero(supported); > + for (unsigned int i = ARRAY_SIZE(lut); i--; ) > + if (eee_capability & (1 << (i + 1))) Perhaps: if (eee_capability & BIT(i + 1)) > + linkmode_set_bit(lut[i], supported); > +} > + > static int i40e_get_eee(struct net_device *netdev, struct ethtool_keee > *edata) > { > struct i40e_netdev_priv *np = netdev_priv(netdev); > struct i40e_aq_get_phy_abilities_resp phy_cfg; > struct i40e_vsi *vsi = np->vsi; > struct i40e_pf *pf = vsi->back; > struct i40e_hw *hw = &pf->hw; > - int status = 0; > + int status; This change seems unrelated to the subject of this patch. If so, please remove. > > /* Get initial PHY capabilities */ > status = i40e_aq_get_phy_capabilities(hw, false, true, &phy_cfg, NULL); > if (status) > return -EAGAIN; > > /* Check whether NIC configuration is compatible with Energy Efficient >* Ethernet (EEE) mode. >*/ > if (phy_cfg.eee_capability == 0) > return -EOPNOTSUPP; > > + i40e_eee_capability_to_kedata_supported(phy_cfg.eee_capability, > edata->supported); Please line-wrap: Networking still prefers code to be 80 columns wide or less. > + linkmode_copy(edata->lp_advertised, edata->supported); > + > /* Get current configuration */ > status = i40e_aq_get_phy_capabilities(hw, false, false, &phy_cfg, NULL); > if (status) > return -EAGAIN; > > + linkmode_zero(edata->advertised); > + if (phy_cfg.eee_capability) > + linkmode_copy(edata->advertised, edata->supported); > edata->eee_enabled = !!phy_cfg.eee_capability; > edata->tx_lpi_enabled = pf->stats.tx_lpi_status; > > edata->eee_active = pf->stats.tx_lpi_status && pf->stats.rx_lpi_status; > > return 0; > } > > static int i40e_is_eee_param_supported(struct net_device *netdev, > struct ethtool_keee *edata) > { > struct i40e_netdev_priv *np = netdev_priv(netdev); > struct i40e_vsi *vsi = np->vsi; > struct i40e_pf *pf = vsi->back; > struct i40e_ethtool_not_used { > - u32 value; > + int value; It is unclear to me that this type change is really related to the subject of this patch. If not, please drop it. But if so, it seems to me that bool would be the appropriate type. > const char *name; > } param[] = { > - {edata->tx_lpi_timer, "tx-timer"}, > + {!!(edata->advertised[0] & ~edata->supported[0]), "advertise"}, > + {!!edata->tx_lpi_timer, "tx-timer"}, > {edata->tx_lpi_enabled != pf->stats.tx_lpi_status, "tx-lpi"} > }; > int i; > @@ -5710,7 +5737,7 @@ static int i40e_set_eee(struct net_device *netdev, > struct ethtool_keee *edata) > struct i40e_pf *pf = vsi->back; > struct i40e_hw *hw = &pf->hw; > __le16 eee_capability; > - int status = 0; > + int status; This change seems unrelated to the subject of this patch. If so, please remove. > > /* Deny parameters we don't support */ > if (i40e_is_eee_param_supported(netdev, edata)) > @@ -5754,7 +5781,7 @@ static int i40e_set_eee(struct net_device *netdev, > struct ethtool_keee *edata) > config.eeer |= cpu_to_le32(I40E_PRTPM_EEER_TX_LPI_EN_MASK); > } else { > config.eee_capability = 0; > - config.eeer &= cpu_to_le32(~I40E_PRTPM_EEER_TX_LPI_EN_MASK); > + config.eeer &= ~cpu_to_le32(I40E_PRTPM_EEER_TX_LPI_EN_MASK); Ditto. > } > > /* Apply modified PHY configuration */ > diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c
[Intel-wired-lan] [tnguy-next-queue:1GbE] BUILD SUCCESS b014c38b53cd384394afe470f07b3136e426db57
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue.git 1GbE branch HEAD: b014c38b53cd384394afe470f07b3136e426db57 igb: add AF_XDP zero-copy Tx support elapsed time: 1309m configs tested: 180 configs skipped: 6 The following configs have been built successfully. More configs may be tested in the coming days. tested configs: alpha allnoconfig gcc-13.2.0 alphaallyesconfig gcc-13.3.0 alpha defconfig gcc-13.2.0 arc allmodconfig gcc-13.2.0 arc allnoconfig gcc-13.2.0 arc allyesconfig gcc-13.2.0 arc defconfig gcc-13.2.0 arc randconfig-001-20240809 gcc-13.2.0 arc randconfig-002-20240809 gcc-13.2.0 arm allmodconfig gcc-13.2.0 arm allnoconfig gcc-13.2.0 arm allyesconfig gcc-13.2.0 arm assabet_defconfig clang-15 arm collie_defconfig clang-15 arm defconfig gcc-13.2.0 arm h3600_defconfig gcc-14.1.0 arm imx_v6_v7_defconfig gcc-14.1.0 armmmp2_defconfig clang-15 arm moxart_defconfig clang-15 armmvebu_v7_defconfig gcc-14.1.0 arm randconfig-001-20240809 gcc-13.2.0 arm randconfig-002-20240809 gcc-13.2.0 arm randconfig-003-20240809 gcc-13.2.0 arm randconfig-004-20240809 gcc-13.2.0 arm s3c6400_defconfig clang-15 arm spear13xx_defconfig clang-15 armspear6xx_defconfig clang-15 arm64allmodconfig gcc-13.2.0 arm64 allnoconfig gcc-13.2.0 arm64 defconfig gcc-13.2.0 arm64 randconfig-001-20240809 gcc-13.2.0 arm64 randconfig-002-20240809 gcc-13.2.0 arm64 randconfig-003-20240809 gcc-13.2.0 arm64 randconfig-004-20240809 gcc-13.2.0 csky allnoconfig gcc-13.2.0 cskydefconfig gcc-13.2.0 csky randconfig-001-20240809 gcc-13.2.0 csky randconfig-002-20240809 gcc-13.2.0 hexagon allmodconfig clang-20 hexagon allyesconfig clang-20 i386 allmodconfig clang-18 i386 allnoconfig clang-18 i386 allyesconfig clang-18 i386 buildonly-randconfig-001-20240809 gcc-12 i386 buildonly-randconfig-002-20240809 gcc-12 i386 buildonly-randconfig-003-20240809 gcc-12 i386 buildonly-randconfig-004-20240809 gcc-12 i386 buildonly-randconfig-005-20240809 gcc-12 i386 buildonly-randconfig-006-20240809 gcc-12 i386defconfig clang-18 i386 randconfig-001-20240809 gcc-12 i386 randconfig-002-20240809 gcc-12 i386 randconfig-003-20240809 gcc-12 i386 randconfig-004-20240809 gcc-12 i386 randconfig-005-20240809 gcc-12 i386 randconfig-006-20240809 gcc-12 i386 randconfig-011-20240809 gcc-12 i386 randconfig-012-20240809 gcc-12 i386 randconfig-013-20240809 gcc-12 i386 randconfig-014-20240809 gcc-12 i386 randconfig-015-20240809 gcc-12 i386 randconfig-016-20240809 gcc-12 loongarchallmodconfig gcc-14.1.0 loongarch allnoconfig gcc-13.2.0 loongarch defconfig gcc-13.2.0 loongarch randconfig-001-20240809 gcc-13.2.0 loongarch randconfig-002-20240809 gcc-13.2.0 m68k allmodconfig gcc-14.1.0 m68k allnoconfig gcc-13.2.0 m68k allyesconfig gcc-14.1.0 m68kdefconfig gcc-13.2.0 microblaze allmodconfig gcc-14.1.0 microblazeallnoconfig gcc-13.2.0 microblaze allyesconfig gcc-14.1.0 microblaze defconfig gcc-13.2.0 mips allnoconfig gcc-13.2.0 mips cu1830-neo_defconfig gcc-14.1.0 mips eyeq5_defconfig gcc-14.1.0 mips fuloong2e_defconfig gcc-14.1.0 mips gcw0_defconfig gcc-14.1.0 mips
Re: [Intel-wired-lan] [iwl-net v2 2/2] igb: Fix missing time sync events
Daiwei Li writes: > Hi, > > It appears this change breaks PTP on the 82580 controller, as ptp4l reports: > >> timed out while polling for tx timestamp increasing tx_timestamp_timeout or >> increasing kworker priority may correct this issue, but a driver bug likely >> causes it > > The 82580 controller has a hardware bug in which reading TSICR doesn't clear > it. See this thread > https://lore.kernel.org/all/cdcb8be0.1ec2c%25matthew.v...@intel.com/ where the > bug was confirmed by an Intel employee. Any chance we could add back the ack > for 82580 specifically? Thanks! Of course, I'll prepare a patch for that. Thanks for digging that one up. Cheers, -- Vinicius
[Intel-wired-lan] [tnguy-next-queue:100GbE] BUILD SUCCESS bc4b2020b5a741c0e4d50fb71e3d6655bf60ebac
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue.git 100GbE branch HEAD: bc4b2020b5a741c0e4d50fb71e3d6655bf60ebac ice: allow to activate and deactivate subfunction elapsed time: 1362m configs tested: 196 configs skipped: 6 The following configs have been built successfully. More configs may be tested in the coming days. tested configs: alpha allnoconfig gcc-13.2.0 alpha allnoconfig gcc-13.3.0 alphaallyesconfig gcc-13.3.0 alpha defconfig gcc-13.2.0 arc allmodconfig gcc-13.2.0 arc allnoconfig gcc-13.2.0 arc allyesconfig gcc-13.2.0 arc defconfig gcc-13.2.0 arc randconfig-001-20240809 gcc-13.2.0 arc randconfig-002-20240809 gcc-13.2.0 arm allmodconfig gcc-13.2.0 arm allmodconfig gcc-14.1.0 arm allnoconfig clang-20 arm allnoconfig gcc-13.2.0 arm allyesconfig gcc-13.2.0 arm allyesconfig gcc-14.1.0 arm defconfig gcc-13.2.0 arm h3600_defconfig gcc-14.1.0 arm imx_v6_v7_defconfig gcc-14.1.0 armmvebu_v7_defconfig gcc-14.1.0 arm randconfig-001-20240809 gcc-13.2.0 arm randconfig-002-20240809 gcc-13.2.0 arm randconfig-003-20240809 gcc-13.2.0 arm randconfig-004-20240809 gcc-13.2.0 arm64allmodconfig clang-20 arm64allmodconfig gcc-13.2.0 arm64 allnoconfig gcc-13.2.0 arm64 allnoconfig gcc-14.1.0 arm64 defconfig gcc-13.2.0 arm64 randconfig-001-20240809 gcc-13.2.0 arm64 randconfig-002-20240809 gcc-13.2.0 arm64 randconfig-003-20240809 gcc-13.2.0 arm64 randconfig-004-20240809 gcc-13.2.0 csky allnoconfig gcc-13.2.0 csky allnoconfig gcc-14.1.0 cskydefconfig gcc-13.2.0 csky randconfig-001-20240809 gcc-13.2.0 csky randconfig-002-20240809 gcc-13.2.0 hexagon allmodconfig clang-20 hexagon allnoconfig clang-20 hexagon allyesconfig clang-20 i386 allmodconfig clang-18 i386 allmodconfig gcc-12 i386 allnoconfig clang-18 i386 allnoconfig gcc-12 i386 allyesconfig clang-18 i386 allyesconfig gcc-12 i386 buildonly-randconfig-001-20240809 gcc-12 i386 buildonly-randconfig-002-20240809 clang-18 i386 buildonly-randconfig-002-20240809 gcc-12 i386 buildonly-randconfig-003-20240809 gcc-11 i386 buildonly-randconfig-003-20240809 gcc-12 i386 buildonly-randconfig-004-20240809 clang-18 i386 buildonly-randconfig-004-20240809 gcc-12 i386 buildonly-randconfig-005-20240809 clang-18 i386 buildonly-randconfig-005-20240809 gcc-12 i386 buildonly-randconfig-006-20240809 gcc-12 i386defconfig clang-18 i386 randconfig-001-20240809 gcc-12 i386 randconfig-002-20240809 clang-18 i386 randconfig-002-20240809 gcc-12 i386 randconfig-003-20240809 clang-18 i386 randconfig-003-20240809 gcc-12 i386 randconfig-004-20240809 gcc-12 i386 randconfig-005-20240809 clang-18 i386 randconfig-005-20240809 gcc-12 i386 randconfig-006-20240809 gcc-12 i386 randconfig-011-20240809 gcc-12 i386 randconfig-012-20240809 gcc-12 i386 randconfig-013-20240809 gcc-12 i386 randconfig-014-20240809 gcc-12 i386 randconfig-015-20240809 gcc-11 i386 randconfig-015-20240809 gcc-12 i386 randconfig-016-20240809 gcc-12 loongarchallmodconfig gcc-14.1.0 loongarch allnoconfig gcc-13.2.0 loongarch allnoconfig gcc-14.1.0 loongarch defconfig gcc-13.2.0 loongarch randconfig-001-20240809 gcc-13.2.0 loongarch randconfig-002-20240809 gcc-13.2.0 m68k
Re: [Intel-wired-lan] [PATCH iwl-next v8 06/14] iavf: add initial framework for registering PTP clock
> -Original Message- > From: Lobakin, Aleksander > Sent: Thursday, August 8, 2024 5:25 AM > To: Polchlopek, Mateusz > Cc: intel-wired-...@lists.osuosl.org; net...@vger.kernel.org; Keller, Jacob E > ; Drewek, Wojciech ; Sai > Krishna ; Simon Horman ; Zaki, > Ahmed > Subject: Re: [Intel-wired-lan] [PATCH iwl-next v8 06/14] iavf: add initial > framework > for registering PTP clock > > From: Mateusz Polchlopek > Date: Thu, 8 Aug 2024 13:04:29 +0200 > > > > > > > On 7/30/2024 3:40 PM, Alexander Lobakin wrote: > >> From: Mateusz Polchlopek > >> Date: Tue, 30 Jul 2024 05:15:01 -0400 > > [...] > > >>> +bool iavf_ptp_cap_supported(struct iavf_adapter *adapter, u32 cap) > >>> +{ > >>> + if (!PTP_ALLOWED(adapter)) > >>> + return false; > >>> + > >>> + /* Only return true if every bit in cap is set in hw_caps.caps */ > >>> + return (adapter->ptp.hw_caps.caps & cap) == cap; > >> > >> Aren't these parenthesis redundant? > >> > > > > I think they are not. They wrap bit operation and also I checked it > > with checkpatch script and it doesn't complain about reduntant > > parenthesis. > > If the object code doesn't change when compiling without them, there are > no compiler complains etc, then they are :D checkpatch doesn't always > catch things, but I don't remember whether the compiler won't complain > or change the object code / logic. Could you please check? > > Thanks, > Olek They may be technically redundant in that the parenthesis don't matter.. but sometimes they can help code legibility by making it more obvious to a human reviewer who isn't immediately going to think like a compiler and realize that & is not && for example...
[Intel-wired-lan] [PATCH iwl-net v1] igb: Fix not clearing TimeSync interrupts for 82580
It was reported that 82580 NICs have a hardware bug that makes it necessary to write into the TSICR (TimeSync Interrupt Cause) register to clear it. Add a conditional so only for 82580 we write into the TSICR register, so we don't risk losing events for other models. This (partially) reverts commit ee14cc9ea19b ("igb: Fix missing time sync events"). Fixes: ee14cc9ea19b ("igb: Fix missing time sync events") Reported-by: Daiwei Li Closes: https://lore.kernel.org/intel-wired-lan/CAN0jFd1kO0MMtOh8N2Ztxn6f7vvDKp2h507sMryobkBKe=xk=w...@mail.gmail.com/ Signed-off-by: Vinicius Costa Gomes --- @Daiwei Li, I don't have a 82580 handy, please confirm that the patch fixes the issue you are having. drivers/net/ethernet/intel/igb/igb_main.c | 27 ++- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c index 11be39f435f3..edb34f67ae03 100644 --- a/drivers/net/ethernet/intel/igb/igb_main.c +++ b/drivers/net/ethernet/intel/igb/igb_main.c @@ -6960,31 +6960,48 @@ static void igb_extts(struct igb_adapter *adapter, int tsintr_tt) static void igb_tsync_interrupt(struct igb_adapter *adapter) { struct e1000_hw *hw = &adapter->hw; - u32 tsicr = rd32(E1000_TSICR); + u32 ack = 0, tsicr = rd32(E1000_TSICR); struct ptp_clock_event event; if (tsicr & TSINTR_SYS_WRAP) { event.type = PTP_CLOCK_PPS; if (adapter->ptp_caps.pps) ptp_clock_event(adapter->ptp_clock, &event); + ack |= TSINTR_SYS_WRAP; } if (tsicr & E1000_TSICR_TXTS) { /* retrieve hardware timestamp */ schedule_work(&adapter->ptp_tx_work); + ack |= E1000_TSICR_TXTS; } - if (tsicr & TSINTR_TT0) + if (tsicr & TSINTR_TT0) { igb_perout(adapter, 0); + ack |= TSINTR_TT0; + } - if (tsicr & TSINTR_TT1) + if (tsicr & TSINTR_TT1) { igb_perout(adapter, 1); + ack |= TSINTR_TT1; + } - if (tsicr & TSINTR_AUTT0) + if (tsicr & TSINTR_AUTT0) { igb_extts(adapter, 0); + ack |= TSINTR_AUTT0; + } - if (tsicr & TSINTR_AUTT1) + if (tsicr & TSINTR_AUTT1) { igb_extts(adapter, 1); + ack |= TSINTR_AUTT1; + } + + if (hw->mac.type == e1000_82580) { + /* 82580 has a hardware bug that requires a explicit +* write to clear the TimeSync interrupt cause. +*/ + wr32(E1000_TSICR, ack); + } } static irqreturn_t igb_msix_other(int irq, void *data) -- 2.45.2
Re: [Intel-wired-lan] [PATCH net-next v1 1/2] dpll: add Embedded SYNC feature for a pin
On Thu, 8 Aug 2024 13:20:12 +0200 Arkadiusz Kubalewski wrote: > +Device may provide ability to use Embedded SYNC feature. It allows > +to embed additional SYNC signal into the base frequency of a pin - a one > +special pulse of base frequency signal every time SYNC signal pulse > +happens. The user can configure the frequency of Embedded SYNC. > +The Embedded SYNC capability is always related to a given base frequency > +and HW capabilities. The user is provided a range of embedded sync > +frequencies supported, depending on current base frequency configured for > +the pin. Interesting, noob question perhaps, is the signal somehow well known or the implementation is vendor specific so both ends have to be from the same vendor? May be worth calling that out, either way.
Re: [Intel-wired-lan] [PATCH iwl-net v1] igb: Fix not clearing TimeSync interrupts for 82580
Dear Vinicius, Thank you for the patch. Am 10.08.24 um 02:23 schrieb Vinicius Costa Gomes: It was reported that 82580 NICs have a hardware bug that makes it necessary to write into the TSICR (TimeSync Interrupt Cause) register to clear it. Were you able to verify that report by checking against some errata? Is Intel aware of the problem? Add a conditional so only for 82580 we write into the TSICR register, so we don't risk losing events for other models. This (partially) reverts commit ee14cc9ea19b ("igb: Fix missing time sync events"). Fixes: ee14cc9ea19b ("igb: Fix missing time sync events") Reported-by: Daiwei Li Closes: https://lore.kernel.org/intel-wired-lan/CAN0jFd1kO0MMtOh8N2Ztxn6f7vvDKp2h507sMryobkBKe=xk=w...@mail.gmail.com/ Signed-off-by: Vinicius Costa Gomes --- @Daiwei Li, I don't have a 82580 handy, please confirm that the patch fixes the issue you are having. Please also add a description of the test case, and maybe the PCI vendor and device code of your network device. drivers/net/ethernet/intel/igb/igb_main.c | 27 ++- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c index 11be39f435f3..edb34f67ae03 100644 --- a/drivers/net/ethernet/intel/igb/igb_main.c +++ b/drivers/net/ethernet/intel/igb/igb_main.c @@ -6960,31 +6960,48 @@ static void igb_extts(struct igb_adapter *adapter, int tsintr_tt) static void igb_tsync_interrupt(struct igb_adapter *adapter) { struct e1000_hw *hw = &adapter->hw; - u32 tsicr = rd32(E1000_TSICR); + u32 ack = 0, tsicr = rd32(E1000_TSICR); struct ptp_clock_event event; if (tsicr & TSINTR_SYS_WRAP) { event.type = PTP_CLOCK_PPS; if (adapter->ptp_caps.pps) ptp_clock_event(adapter->ptp_clock, &event); + ack |= TSINTR_SYS_WRAP; } if (tsicr & E1000_TSICR_TXTS) { /* retrieve hardware timestamp */ schedule_work(&adapter->ptp_tx_work); + ack |= E1000_TSICR_TXTS; } - if (tsicr & TSINTR_TT0) + if (tsicr & TSINTR_TT0) { igb_perout(adapter, 0); + ack |= TSINTR_TT0; + } - if (tsicr & TSINTR_TT1) + if (tsicr & TSINTR_TT1) { igb_perout(adapter, 1); + ack |= TSINTR_TT1; + } - if (tsicr & TSINTR_AUTT0) + if (tsicr & TSINTR_AUTT0) { igb_extts(adapter, 0); + ack |= TSINTR_AUTT0; + } - if (tsicr & TSINTR_AUTT1) + if (tsicr & TSINTR_AUTT1) { igb_extts(adapter, 1); + ack |= TSINTR_AUTT1; + } + + if (hw->mac.type == e1000_82580) { + /* 82580 has a hardware bug that requires a explicit a*n* +* write to clear the TimeSync interrupt cause. +*/ + wr32(E1000_TSICR, ack); + } Is there a nicer way to write this, so `ack` is only assigned in case for the 82580? } static irqreturn_t igb_msix_other(int irq, void *data) Kind regards, Paul
Re: [Intel-wired-lan] [PATCH iwl-net v1] igb: Fix not clearing TimeSync interrupts for 82580
On Fri, Aug 09, 2024 at 05:23:02PM -0700, Vinicius Costa Gomes wrote: > It was reported that 82580 NICs have a hardware bug that makes it > necessary to write into the TSICR (TimeSync Interrupt Cause) register > to clear it. Bug, or was it a feature? Or IOW, maybe i210 changed the semantics of the TSICR? And what about the 82576? Thanks, Richard
Re: [Intel-wired-lan] [iwl-next v3 1/8] ice: devlink PF MSI-X max and min parameter
Fri, Aug 09, 2024 at 01:39:42PM CEST, michal.swiatkow...@linux.intel.com wrote: >On Fri, Aug 09, 2024 at 01:29:29PM +0200, Jiri Pirko wrote: >> Fri, Aug 09, 2024 at 01:05:00PM CEST, michal.swiatkow...@linux.intel.com >> wrote: >> >On Fri, Aug 09, 2024 at 12:51:58PM +0200, Jiri Pirko wrote: >> >> Fri, Aug 09, 2024 at 07:13:34AM CEST, michal.swiatkow...@linux.intel.com >> >> wrote: >> >> >On Thu, Aug 08, 2024 at 05:34:35PM +0200, Jiri Pirko wrote: >> >> >> Thu, Aug 08, 2024 at 09:20:09AM CEST, >> >> >> michal.swiatkow...@linux.intel.com wrote: >> >> >> >Use generic devlink PF MSI-X parameter to allow user to change MSI-X >> >> >> >range. >> >> >> > >> >> >> >Reviewed-by: Wojciech Drewek >> >> >> >Signed-off-by: Michal Swiatkowski >> >> >> >--- >> >> >> > .../net/ethernet/intel/ice/devlink/devlink.c | 56 >> >> >> > ++- >> >> >> > drivers/net/ethernet/intel/ice/ice.h | 8 +++ >> >> >> > drivers/net/ethernet/intel/ice/ice_irq.c | 14 - >> >> >> > 3 files changed, 76 insertions(+), 2 deletions(-) >> >> >> > >> >> >> >diff --git a/drivers/net/ethernet/intel/ice/devlink/devlink.c >> >> >> >b/drivers/net/ethernet/intel/ice/devlink/devlink.c >> >> >> >index 29a5f822cb8b..bdc22ea13e0f 100644 >> >> >> >--- a/drivers/net/ethernet/intel/ice/devlink/devlink.c >> >> >> >+++ b/drivers/net/ethernet/intel/ice/devlink/devlink.c >> >> >> >@@ -1518,6 +1518,32 @@ static int >> >> >> >ice_devlink_local_fwd_validate(struct devlink *devlink, u32 id, >> >> >> > return 0; >> >> >> > } >> >> >> > >> >> >> >+static int >> >> >> >+ice_devlink_msix_max_pf_validate(struct devlink *devlink, u32 id, >> >> >> >+ union devlink_param_value val, >> >> >> >+ struct netlink_ext_ack *extack) >> >> >> >+{ >> >> >> >+ if (val.vu16 > ICE_MAX_MSIX) { >> >> >> >+ NL_SET_ERR_MSG_MOD(extack, "PF max MSI-X is too high"); >> >> >> >> >> >> No reason to have "PF" in the text. Also, no reason to have "max >> >> >> MSI-X". >> >> >> That is the name of the param. >> >> >> >> >> > >> >> >Ok, will change both, thanks. >> >> > >> >> >> >> >> >> >> >> >> >+ return -EINVAL; >> >> >> >+ } >> >> >> >+ >> >> >> >+ return 0; >> >> >> >+} >> >> >> >+ >> >> >> >+static int >> >> >> >+ice_devlink_msix_min_pf_validate(struct devlink *devlink, u32 id, >> >> >> >+ union devlink_param_value val, >> >> >> >+ struct netlink_ext_ack *extack) >> >> >> >+{ >> >> >> >+ if (val.vu16 <= ICE_MIN_MSIX) { >> >> >> >+ NL_SET_ERR_MSG_MOD(extack, "PF min MSI-X is too low"); >> >> >> >> >> >> Same comment as for max goes here. >> >> >> >> >> >> >> >> >> >+ return -EINVAL; >> >> >> >+ } >> >> >> >+ >> >> >> >+ return 0; >> >> >> >+} >> >> >> >+ >> >> >> > enum ice_param_id { >> >> >> > ICE_DEVLINK_PARAM_ID_BASE = DEVLINK_PARAM_GENERIC_ID_MAX, >> >> >> > ICE_DEVLINK_PARAM_ID_TX_SCHED_LAYERS, >> >> >> >@@ -1535,6 +1561,15 @@ static const struct devlink_param >> >> >> >ice_dvl_rdma_params[] = { >> >> >> >ice_devlink_enable_iw_validate), >> >> >> > }; >> >> >> > >> >> >> >+static const struct devlink_param ice_dvl_msix_params[] = { >> >> >> >+ DEVLINK_PARAM_GENERIC(MSIX_VEC_PER_PF_MAX, >> >> >> >+ BIT(DEVLINK_PARAM_CMODE_DRIVERINIT), >> >> >> >+ NULL, NULL, >> >> >> >ice_devlink_msix_max_pf_validate), >> >> >> >+ DEVLINK_PARAM_GENERIC(MSIX_VEC_PER_PF_MIN, >> >> >> >+ BIT(DEVLINK_PARAM_CMODE_DRIVERINIT), >> >> >> >+ NULL, NULL, >> >> >> >ice_devlink_msix_min_pf_validate), >> >> >> >+}; >> >> >> >+ >> >> >> > static const struct devlink_param ice_dvl_sched_params[] = { >> >> >> > DEVLINK_PARAM_DRIVER(ICE_DEVLINK_PARAM_ID_TX_SCHED_LAYERS, >> >> >> > "tx_scheduling_layers", >> >> >> >@@ -1637,6 +1672,7 @@ void ice_devlink_unregister(struct ice_pf *pf) >> >> >> > int ice_devlink_register_params(struct ice_pf *pf) >> >> >> > { >> >> >> > struct devlink *devlink = priv_to_devlink(pf); >> >> >> >+ union devlink_param_value value; >> >> >> > struct ice_hw *hw = &pf->hw; >> >> >> > int status; >> >> >> > >> >> >> >@@ -1645,11 +1681,27 @@ int ice_devlink_register_params(struct ice_pf >> >> >> >*pf) >> >> >> > if (status) >> >> >> > return status; >> >> >> > >> >> >> >+ status = devl_params_register(devlink, ice_dvl_msix_params, >> >> >> >+ ARRAY_SIZE(ice_dvl_msix_params)); >> >> >> >+ if (status) >> >> >> >+ return status; >> >> >> >+ >> >> >> > if (hw->func_caps.common_cap.tx_sched_topo_comp_mode_en) >> >> >> > status = devl_params_register(devlink, >> >> >> > ice_dvl_sched_params, >> >> >> > >> >> >> > ARRAY_SIZE(ice_dvl_sched_pa