Message ID | 20240927151541.15704-1-marcin.szycik@linux.intel.com (mailing list archive) |
---|---|
State | Awaiting Upstream |
Delegated to: | Netdev Maintainers |
Headers | show |
Series | [iwl-net] ice: Fix increasing MSI-X on VF | expand |
Context | Check | Description |
---|---|---|
netdev/tree_selection | success | Clearly marked for net |
netdev/apply | fail | Patch does not apply to net-0 |
On Fri, Sep 27, 2024 at 05:15:40PM +0200, Marcin Szycik wrote: > Increasing MSI-X value on a VF leads to invalid memory operations. This > is caused by not reallocating some arrays. > > Reproducer: > modprobe ice > echo 0 > /sys/bus/pci/devices/$PF_PCI/sriov_drivers_autoprobe > echo 1 > /sys/bus/pci/devices/$PF_PCI/sriov_numvfs > echo 17 > /sys/bus/pci/devices/$VF0_PCI/sriov_vf_msix_count > > Default MSI-X is 16, so 17 and above triggers this issue. > > KASAN reports: > > BUG: KASAN: slab-out-of-bounds in ice_vsi_alloc_ring_stats+0x38d/0x4b0 [ice] > Read of size 8 at addr ffff8888b937d180 by task bash/28433 > (...) > > Call Trace: > (...) > ? ice_vsi_alloc_ring_stats+0x38d/0x4b0 [ice] > kasan_report+0xed/0x120 > ? ice_vsi_alloc_ring_stats+0x38d/0x4b0 [ice] > ice_vsi_alloc_ring_stats+0x38d/0x4b0 [ice] > ice_vsi_cfg_def+0x3360/0x4770 [ice] > ? mutex_unlock+0x83/0xd0 > ? __pfx_ice_vsi_cfg_def+0x10/0x10 [ice] > ? __pfx_ice_remove_vsi_lkup_fltr+0x10/0x10 [ice] > ice_vsi_cfg+0x7f/0x3b0 [ice] > ice_vf_reconfig_vsi+0x114/0x210 [ice] > ice_sriov_set_msix_vec_count+0x3d0/0x960 [ice] > sriov_vf_msix_count_store+0x21c/0x300 > (...) > > Allocated by task 28201: > (...) > ice_vsi_cfg_def+0x1c8e/0x4770 [ice] > ice_vsi_cfg+0x7f/0x3b0 [ice] > ice_vsi_setup+0x179/0xa30 [ice] > ice_sriov_configure+0xcaa/0x1520 [ice] > sriov_numvfs_store+0x212/0x390 > (...) > > To fix it, use ice_vsi_rebuild() instead of ice_vf_reconfig_vsi(). This > causes the required arrays to be reallocated taking the new queue count > into account (ice_vsi_realloc_stat_arrays()). Set req_txq and req_rxq > before ice_vsi_rebuild(), so that realloc uses the newly set queue > count. > > Additionally, ice_vsi_rebuild() does not remove VSI filters > (ice_fltr_remove_all()), so ice_vf_init_host_cfg() is no longer > necessary. > > Reported-by: Jacob Keller <jacob.e.keller@intel.com> > Fixes: 2a2cb4c6c181 ("ice: replace ice_vf_recreate_vsi() with ice_vf_reconfig_vsi()") > Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com> > Signed-off-by: Marcin Szycik <marcin.szycik@linux.intel.com> Reviewed-by: Simon Horman <horms@kernel.org>
> -----Original Message----- > From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of Simon > Horman > Sent: Monday, September 30, 2024 4:52 PM > To: Marcin Szycik <marcin.szycik@linux.intel.com> > Cc: Keller, Jacob E <jacob.e.keller@intel.com>; netdev@vger.kernel.org; Michal > Swiatkowski <michal.swiatkowski@linux.intel.com>; intel-wired- > lan@lists.osuosl.org; Kitszel, Przemyslaw <przemyslaw.kitszel@intel.com> > Subject: Re: [Intel-wired-lan] [PATCH iwl-net] ice: Fix increasing MSI-X on VF > > On Fri, Sep 27, 2024 at 05:15:40PM +0200, Marcin Szycik wrote: > > Increasing MSI-X value on a VF leads to invalid memory operations. > > This is caused by not reallocating some arrays. > > > > Reproducer: > > modprobe ice > > echo 0 > /sys/bus/pci/devices/$PF_PCI/sriov_drivers_autoprobe > > echo 1 > /sys/bus/pci/devices/$PF_PCI/sriov_numvfs > > echo 17 > /sys/bus/pci/devices/$VF0_PCI/sriov_vf_msix_count > > > > Default MSI-X is 16, so 17 and above triggers this issue. > > > > KASAN reports: > > > > BUG: KASAN: slab-out-of-bounds in ice_vsi_alloc_ring_stats+0x38d/0x4b0 > [ice] > > Read of size 8 at addr ffff8888b937d180 by task bash/28433 > > (...) > > > > Call Trace: > > (...) > > ? ice_vsi_alloc_ring_stats+0x38d/0x4b0 [ice] > > kasan_report+0xed/0x120 > > ? ice_vsi_alloc_ring_stats+0x38d/0x4b0 [ice] > > ice_vsi_alloc_ring_stats+0x38d/0x4b0 [ice] > > ice_vsi_cfg_def+0x3360/0x4770 [ice] > > ? mutex_unlock+0x83/0xd0 > > ? __pfx_ice_vsi_cfg_def+0x10/0x10 [ice] > > ? __pfx_ice_remove_vsi_lkup_fltr+0x10/0x10 [ice] > > ice_vsi_cfg+0x7f/0x3b0 [ice] > > ice_vf_reconfig_vsi+0x114/0x210 [ice] > > ice_sriov_set_msix_vec_count+0x3d0/0x960 [ice] > > sriov_vf_msix_count_store+0x21c/0x300 > > (...) > > > > Allocated by task 28201: > > (...) > > ice_vsi_cfg_def+0x1c8e/0x4770 [ice] > > ice_vsi_cfg+0x7f/0x3b0 [ice] > > ice_vsi_setup+0x179/0xa30 [ice] > > ice_sriov_configure+0xcaa/0x1520 [ice] > > sriov_numvfs_store+0x212/0x390 > > (...) > > > > To fix it, use ice_vsi_rebuild() instead of ice_vf_reconfig_vsi(). > > This causes the required arrays to be reallocated taking the new queue > > count into account (ice_vsi_realloc_stat_arrays()). Set req_txq and > > req_rxq before ice_vsi_rebuild(), so that realloc uses the newly set > > queue count. > > > > Additionally, ice_vsi_rebuild() does not remove VSI filters > > (ice_fltr_remove_all()), so ice_vf_init_host_cfg() is no longer > > necessary. > > > > Reported-by: Jacob Keller <jacob.e.keller@intel.com> > > Fixes: 2a2cb4c6c181 ("ice: replace ice_vf_recreate_vsi() with > > ice_vf_reconfig_vsi()") > > Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com> > > Signed-off-by: Marcin Szycik <marcin.szycik@linux.intel.com> > > Reviewed-by: Simon Horman <horms@kernel.org> Tested-by: Rafal Romanowski <rafal.romanowski@intel.com>
diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c b/drivers/net/ethernet/intel/ice/ice_sriov.c index c2d6b2a144e9..91cb393f616f 100644 --- a/drivers/net/ethernet/intel/ice/ice_sriov.c +++ b/drivers/net/ethernet/intel/ice/ice_sriov.c @@ -1121,7 +1121,10 @@ int ice_sriov_set_msix_vec_count(struct pci_dev *vf_dev, int msix_vec_count) if (vf->first_vector_idx < 0) goto unroll; - if (ice_vf_reconfig_vsi(vf) || ice_vf_init_host_cfg(vf, vsi)) { + vsi->req_txq = queues; + vsi->req_rxq = queues; + + if (ice_vsi_rebuild(vsi, ICE_VSI_FLAG_NO_INIT)) { /* Try to rebuild with previous values */ needs_rebuild = true; goto unroll; @@ -1150,8 +1153,10 @@ int ice_sriov_set_msix_vec_count(struct pci_dev *vf_dev, int msix_vec_count) } if (needs_rebuild) { - ice_vf_reconfig_vsi(vf); - ice_vf_init_host_cfg(vf, vsi); + vsi->req_txq = prev_queues; + vsi->req_rxq = prev_queues; + + ice_vsi_rebuild(vsi, ICE_VSI_FLAG_NO_INIT); } ice_ena_vf_mappings(vf); diff --git a/drivers/net/ethernet/intel/ice/ice_vf_lib.c b/drivers/net/ethernet/intel/ice/ice_vf_lib.c index 749a08ccf267..8c434689e3f7 100644 --- a/drivers/net/ethernet/intel/ice/ice_vf_lib.c +++ b/drivers/net/ethernet/intel/ice/ice_vf_lib.c @@ -256,7 +256,7 @@ static void ice_vf_pre_vsi_rebuild(struct ice_vf *vf) * * It brings the VSI down and then reconfigures it with the hardware. */ -int ice_vf_reconfig_vsi(struct ice_vf *vf) +static int ice_vf_reconfig_vsi(struct ice_vf *vf) { struct ice_vsi *vsi = ice_get_vf_vsi(vf); struct ice_pf *pf = vf->pf; diff --git a/drivers/net/ethernet/intel/ice/ice_vf_lib_private.h b/drivers/net/ethernet/intel/ice/ice_vf_lib_private.h index 91ba7fe0eaee..0c7e77c0a09f 100644 --- a/drivers/net/ethernet/intel/ice/ice_vf_lib_private.h +++ b/drivers/net/ethernet/intel/ice/ice_vf_lib_private.h @@ -23,7 +23,6 @@ #warning "Only include ice_vf_lib_private.h in CONFIG_PCI_IOV virtualization files" #endif -int ice_vf_reconfig_vsi(struct ice_vf *vf); void ice_initialize_vf_entry(struct ice_vf *vf); void ice_dis_vf_qs(struct ice_vf *vf); int ice_check_vf_init(struct ice_vf *vf);