| Message ID | 20231221132735.1246164-1-andrii.staikov@intel.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [iwl-net,v5] i40e: Restore VF MSI-X state during PCI reset | expand |
On 12/21/23 14:27, 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: Wojciech Drewek <wojciech.drewek@intel.com> > Reviewed-by: Przemyslaw Kitszel <przemyslaw.kitszel@intel.com> should be "Przemek", but no need to send next version for that (but pls promise it's last time ;)) > Signed-off-by: Andrii Staikov <andrii.staikov@intel.com> > --- > v1 -> v2: Fix signed-off tags > https://patchwork.ozlabs.org/project/intel-wired-lan/patch/20231204131041.3369693-1-andrii.staikov@intel.com/ > > v2 -> v3: use @vf_dev in pci_get_device() instead of NULL and remove unnecessary call > https://patchwork.ozlabs.org/project/intel-wired-lan/patch/20231206125127.218350-1-andrii.staikov@intel.com/ > > v3 -> v4: wrap the added functionality into the CONFIG_PCI_IOV define as > this is VF-related functionality > https://patchwork.ozlabs.org/project/intel-wired-lan/patch/20231212122452.3250691-1-andrii.staikov@intel.com/ > > v4 -> v5: fix RB tags there was a question in v4 from Jake to "perhaps do it like in ice", but I think that for a bug fix it's better to keep this patch as-is (review always welcomed through). > --- > drivers/net/ethernet/intel/i40e/i40e_main.c | 3 +++ > .../ethernet/intel/i40e/i40e_virtchnl_pf.c | 26 +++++++++++++++++++ > .../ethernet/intel/i40e/i40e_virtchnl_pf.h | 3 +++ > 3 files changed, 32 insertions(+) > > diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c > index 7bb1f64833eb..bbe2d115fb15 100644 > --- a/drivers/net/ethernet/intel/i40e/i40e_main.c > +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c > @@ -16513,6 +16513,9 @@ static void i40e_pci_error_reset_done(struct pci_dev *pdev) > return; > > i40e_reset_and_rebuild(pf, false, false); > +#ifdef CONFIG_PCI_IOV > + i40e_restore_all_vfs_msi_state(pdev); > +#endif /* CONFIG_PCI_IOV */ > } > > /** > diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c > index 3f99eb198245..d60f5419d6bd 100644 > --- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c > +++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c > @@ -154,6 +154,32 @@ void i40e_vc_notify_reset(struct i40e_pf *pf) > (u8 *)&pfe, sizeof(struct virtchnl_pf_event)); > } > > +#ifdef CONFIG_PCI_IOV > +void i40e_restore_all_vfs_msi_state(struct pci_dev *pdev) > +{ > + u16 vf_id; nit: scope of @vf_id could be local to the last if (but I guess it's too late for such) > + 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); > + } > + } > +} > +#endif /* CONFIG_PCI_IOV */ > + > /** > * 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..5fd607c0de0a 100644 > --- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.h > +++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.h > @@ -137,6 +137,9 @@ 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); > +#ifdef CONFIG_PCI_IOV > +void i40e_restore_all_vfs_msi_state(struct pci_dev *pdev); > +#endif /* CONFIG_PCI_IOV */ > int i40e_get_vf_stats(struct net_device *netdev, int vf_id, > struct ifla_vf_stats *vf_stats); >
> -----Original Message----- > From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of > Przemek Kitszel > Sent: Thursday, December 21, 2023 2:49 PM > To: Staikov, Andrii <andrii.staikov@intel.com>; intel-wired-lan@lists.osuosl.org > Cc: Ostrowska, Karen <karen.ostrowska@intel.com>; netdev@vger.kernel.org; > Drewek, Wojciech <wojciech.drewek@intel.com>; Mateusz Palczewski > <mateusz.palczewski@intel.com> > Subject: Re: [Intel-wired-lan] [PATCH iwl-net v5] i40e: Restore VF MSI-X state > during PCI reset > > On 12/21/23 14:27, 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: Wojciech Drewek <wojciech.drewek@intel.com> > > Reviewed-by: Przemyslaw Kitszel <przemyslaw.kitszel@intel.com> > > should be "Przemek", but no need to send next version for that (but pls > promise it's last time ;)) > > > Signed-off-by: Andrii Staikov <andrii.staikov@intel.com> > > --- > > v1 -> v2: Fix signed-off tags > > https://patchwork.ozlabs.org/project/intel-wired-lan/patch/20231204131 > > 041.3369693-1-andrii.staikov@intel.com/ > > > > v2 -> v3: use @vf_dev in pci_get_device() instead of NULL and remove > > unnecessary call > > https://patchwork.ozlabs.org/project/intel-wired-lan/patch/20231206125 > > 127.218350-1-andrii.staikov@intel.com/ > > > > v3 -> v4: wrap the added functionality into the CONFIG_PCI_IOV define > > as this is VF-related functionality > > https://patchwork.ozlabs.org/project/intel-wired-lan/patch/20231212122 > > 452.3250691-1-andrii.staikov@intel.com/ > > > > v4 -> v5: fix RB tags > > there was a question in v4 from Jake to "perhaps do it like in ice", but I think > that for a bug fix it's better to keep this patch as-is (review always welcomed > through). > > > --- > > drivers/net/ethernet/intel/i40e/i40e_main.c | 3 +++ > > .../ethernet/intel/i40e/i40e_virtchnl_pf.c | 26 +++++++++++++++++++ > > .../ethernet/intel/i40e/i40e_virtchnl_pf.h | 3 +++ > > 3 files changed, 32 insertions(+) > > > > diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c > > b/drivers/net/ethernet/intel/i40e/i40e_main.c > > index 7bb1f64833eb..bbe2d115fb15 100644 > > --- a/drivers/net/ethernet/intel/i40e/i40e_main.c > > +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c Tested-by: Rafal Romanowski <rafal.romanowski@intel.com>
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c index 7bb1f64833eb..bbe2d115fb15 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_main.c +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c @@ -16513,6 +16513,9 @@ static void i40e_pci_error_reset_done(struct pci_dev *pdev) return; i40e_reset_and_rebuild(pf, false, false); +#ifdef CONFIG_PCI_IOV + i40e_restore_all_vfs_msi_state(pdev); +#endif /* CONFIG_PCI_IOV */ } /** diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c index 3f99eb198245..d60f5419d6bd 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c +++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c @@ -154,6 +154,32 @@ void i40e_vc_notify_reset(struct i40e_pf *pf) (u8 *)&pfe, sizeof(struct virtchnl_pf_event)); } +#ifdef CONFIG_PCI_IOV +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); + } + } +} +#endif /* CONFIG_PCI_IOV */ + /** * 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..5fd607c0de0a 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.h +++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.h @@ -137,6 +137,9 @@ 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); +#ifdef CONFIG_PCI_IOV +void i40e_restore_all_vfs_msi_state(struct pci_dev *pdev); +#endif /* CONFIG_PCI_IOV */ 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: Wojciech Drewek <wojciech.drewek@intel.com> Reviewed-by: Przemyslaw Kitszel <przemyslaw.kitszel@intel.com> Signed-off-by: Andrii Staikov <andrii.staikov@intel.com> --- v1 -> v2: Fix signed-off tags https://patchwork.ozlabs.org/project/intel-wired-lan/patch/20231204131041.3369693-1-andrii.staikov@intel.com/ v2 -> v3: use @vf_dev in pci_get_device() instead of NULL and remove unnecessary call https://patchwork.ozlabs.org/project/intel-wired-lan/patch/20231206125127.218350-1-andrii.staikov@intel.com/ v3 -> v4: wrap the added functionality into the CONFIG_PCI_IOV define as this is VF-related functionality https://patchwork.ozlabs.org/project/intel-wired-lan/patch/20231212122452.3250691-1-andrii.staikov@intel.com/ v4 -> v5: fix RB tags --- drivers/net/ethernet/intel/i40e/i40e_main.c | 3 +++ .../ethernet/intel/i40e/i40e_virtchnl_pf.c | 26 +++++++++++++++++++ .../ethernet/intel/i40e/i40e_virtchnl_pf.h | 3 +++ 3 files changed, 32 insertions(+)