| Message ID | 20231206125127.218350-1-andrii.staikov@intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [iwl-net,v3] i40e: Restore VF MSI-X state during PCI reset | expand |
On 12/6/23 13:51, Andrii Staikov wrote: > During a PCI FLR the MSI-X Enable flag in the VF PCI MSI-X capability > register will be cleared. This can lead to issues when a VF is > assigned to a VM because in these cases the VF driver receives no > indication of the PF PCI error/reset and additionally it is incapable > of restoring the cleared flag in the hypervisor configuration space > without fully reinitializing the driver interrupt functionality. > > Since the VF driver is unable to easily resolve this condition on its own, > restore the VF MSI-X flag during the PF PCI reset handling. > > Fixes: 19b7960b2da1 ("i40e: implement split PCI error reset handler") > Co-developed-by: Karen Ostrowska <karen.ostrowska@intel.com> > Signed-off-by: Karen Ostrowska <karen.ostrowska@intel.com> > Co-developed-by: Mateusz Palczewski <mateusz.palczewski@intel.com> > Signed-off-by: Mateusz Palczewski <mateusz.palczewski@intel.com> > Reviewed-by: Drewek Wojciech <wojciech.drewek@intel.com> It's good that you pass tags from our e1000 ML here, however please copy them verbatim from the response body (instead of copying that from whatever MS outlook displays as sender). (Wojtek's RB was fine in previous version of the patch) > Reviewed-by: Kitszel Przemyslaw <przemyslaw.kitszel@intel.com> You should never add such tags when not explicitly provided by reviewer. > Signed-off-by: Andrii Staikov <andrii.staikov@intel.com> > --- > v1 -> v2: Fix signed-off tags > v2 -> v3: use @vf_dev in pci_get_device() instead of NULL and > remove unnecessary call Thank you, code looks fine now. > --- > drivers/net/ethernet/intel/i40e/i40e_main.c | 2 ++ > .../ethernet/intel/i40e/i40e_virtchnl_pf.c | 24 +++++++++++++++++++ > .../ethernet/intel/i40e/i40e_virtchnl_pf.h | 1 + > 3 files changed, 27 insertions(+) Assuming you will fix RB tags: Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
Hi Andrii, kernel test robot noticed the following build errors: [auto build test ERROR on tnguy-net-queue/dev-queue] url: https://github.com/intel-lab-lkp/linux/commits/Andrii-Staikov/i40e-Restore-VF-MSI-X-state-during-PCI-reset/20231206-205526 base: https://git.kernel.org/pub/scm/linux/kernel/git/tnguy/net-queue.git dev-queue patch link: https://lore.kernel.org/r/20231206125127.218350-1-andrii.staikov%40intel.com patch subject: [Intel-wired-lan] [PATCH iwl-net v3] i40e: Restore VF MSI-X state during PCI reset config: powerpc-ppc64_defconfig (https://download.01.org/0day-ci/archive/20231208/202312080413.jtUlijX8-lkp@intel.com/config) compiler: powerpc64-linux-gcc (GCC) 13.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231208/202312080413.jtUlijX8-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202312080413.jtUlijX8-lkp@intel.com/ All errors (new ones prefixed by >>): drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c: In function 'i40e_restore_all_vfs_msi_state': >> drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c:175:58: error: 'struct pci_dev' has no member named 'physfn'; did you mean 'is_physfn'? 175 | if (vf_dev->is_virtfn && vf_dev->physfn == pdev) | ^~~~~~ | is_physfn vim +175 drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c 156 157 void i40e_restore_all_vfs_msi_state(struct pci_dev *pdev) 158 { 159 u16 vf_id; 160 u16 pos; 161 162 /* Continue only if this is a PF */ 163 if (!pdev->is_physfn) 164 return; 165 166 if (!pci_num_vf(pdev)) 167 return; 168 169 pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_SRIOV); 170 if (pos) { 171 struct pci_dev *vf_dev = NULL; 172 173 pci_read_config_word(pdev, pos + PCI_SRIOV_VF_DID, &vf_id); 174 while ((vf_dev = pci_get_device(pdev->vendor, vf_id, vf_dev))) { > 175 if (vf_dev->is_virtfn && vf_dev->physfn == pdev) 176 pci_restore_msi_state(vf_dev); 177 } 178 } 179 } 180
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c index 7bb1f64833eb..7272d0227a55 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_main.c +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c @@ -16513,6 +16513,8 @@ static void i40e_pci_error_reset_done(struct pci_dev *pdev) return; i40e_reset_and_rebuild(pf, false, false); + + i40e_restore_all_vfs_msi_state(pdev); } /** diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c index 3f99eb198245..7c8ba2675ed5 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c +++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c @@ -154,6 +154,30 @@ void i40e_vc_notify_reset(struct i40e_pf *pf) (u8 *)&pfe, sizeof(struct virtchnl_pf_event)); } +void i40e_restore_all_vfs_msi_state(struct pci_dev *pdev) +{ + u16 vf_id; + u16 pos; + + /* Continue only if this is a PF */ + if (!pdev->is_physfn) + return; + + if (!pci_num_vf(pdev)) + return; + + pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_SRIOV); + if (pos) { + struct pci_dev *vf_dev = NULL; + + pci_read_config_word(pdev, pos + PCI_SRIOV_VF_DID, &vf_id); + while ((vf_dev = pci_get_device(pdev->vendor, vf_id, vf_dev))) { + if (vf_dev->is_virtfn && vf_dev->physfn == pdev) + pci_restore_msi_state(vf_dev); + } + } +} + /** * i40e_vc_notify_vf_reset * @vf: pointer to the VF structure diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.h b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.h index 2ee0f8a23248..1ff879784563 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.h +++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.h @@ -137,6 +137,7 @@ int i40e_ndo_set_vf_spoofchk(struct net_device *netdev, int vf_id, bool enable); void i40e_vc_notify_link_state(struct i40e_pf *pf); void i40e_vc_notify_reset(struct i40e_pf *pf); +void i40e_restore_all_vfs_msi_state(struct pci_dev *pdev); int i40e_get_vf_stats(struct net_device *netdev, int vf_id, struct ifla_vf_stats *vf_stats);
During a PCI FLR the MSI-X Enable flag in the VF PCI MSI-X capability register will be cleared. This can lead to issues when a VF is assigned to a VM because in these cases the VF driver receives no indication of the PF PCI error/reset and additionally it is incapable of restoring the cleared flag in the hypervisor configuration space without fully reinitializing the driver interrupt functionality. Since the VF driver is unable to easily resolve this condition on its own, restore the VF MSI-X flag during the PF PCI reset handling. Fixes: 19b7960b2da1 ("i40e: implement split PCI error reset handler") Co-developed-by: Karen Ostrowska <karen.ostrowska@intel.com> Signed-off-by: Karen Ostrowska <karen.ostrowska@intel.com> Co-developed-by: Mateusz Palczewski <mateusz.palczewski@intel.com> Signed-off-by: Mateusz Palczewski <mateusz.palczewski@intel.com> Reviewed-by: Drewek Wojciech <wojciech.drewek@intel.com> Reviewed-by: Kitszel Przemyslaw <przemyslaw.kitszel@intel.com> Signed-off-by: Andrii Staikov <andrii.staikov@intel.com> --- v1 -> v2: Fix signed-off tags v2 -> v3: use @vf_dev in pci_get_device() instead of NULL and remove unnecessary call --- drivers/net/ethernet/intel/i40e/i40e_main.c | 2 ++ .../ethernet/intel/i40e/i40e_virtchnl_pf.c | 24 +++++++++++++++++++ .../ethernet/intel/i40e/i40e_virtchnl_pf.h | 1 + 3 files changed, 27 insertions(+)