| Message ID | 20230912115626.105828-1-mateusz.polchlopek@intel.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [Intel-wired-lan,iwl-next,v2] ice: store VF's pci_dev ptr in ice_vf | expand |
On 9/12/2023 1:56 PM, Mateusz Polchlopek wrote: > From: Przemek Kitszel <przemyslaw.kitszel@intel.com> > > Extend struct ice_vf by vfdev. > Calculation of vfdev falls more nicely into ice_create_vf_entries(). > > Caching of vfdev enables simplification of ice_restore_all_vfs_msi_state(). > > Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com> > Reviewed-by: Jacob Keller <jacob.e.keller@intel.com> > Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com> > Co-developed-by: Mateusz Polchlopek <mateusz.polchlopek@intel.com> > Signed-off-by: Mateusz Polchlopek <mateusz.polchlopek@intel.com> I forgot to attach changelog, sorry: v2: balance vfdev refcount v2: https://lore.kernel.org/netdev/20230912115626.105828-1-mateusz.polchlopek@intel.com/ v1: https://lore.kernel.org/netdev/20230816085454.235440-1-przemyslaw.kitszel@intel.com/ > --- > drivers/net/ethernet/intel/ice/ice_main.c | 2 +- > drivers/net/ethernet/intel/ice/ice_sriov.c | 50 +++++++++++---------- > drivers/net/ethernet/intel/ice/ice_sriov.h | 4 +- > drivers/net/ethernet/intel/ice/ice_vf_lib.c | 2 + > drivers/net/ethernet/intel/ice/ice_vf_lib.h | 2 +- > 5 files changed, 32 insertions(+), 28 deletions(-) > > diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c > index a5997008bb98..38adffbe0edf 100644 > --- a/drivers/net/ethernet/intel/ice/ice_main.c > +++ b/drivers/net/ethernet/intel/ice/ice_main.c > @@ -5561,7 +5561,7 @@ static void ice_pci_err_resume(struct pci_dev *pdev) > return; > } > > - ice_restore_all_vfs_msi_state(pdev); > + ice_restore_all_vfs_msi_state(pf); > > ice_do_reset(pf, ICE_RESET_PFR); > ice_service_task_restart(pf); > diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c b/drivers/net/ethernet/intel/ice/ice_sriov.c > index 31314e7540f8..4ae59c59e22b 100644 > --- a/drivers/net/ethernet/intel/ice/ice_sriov.c > +++ b/drivers/net/ethernet/intel/ice/ice_sriov.c > @@ -789,14 +789,19 @@ static const struct ice_vf_ops ice_sriov_vf_ops = { > */ > static int ice_create_vf_entries(struct ice_pf *pf, u16 num_vfs) > { > + struct pci_dev *pdev = pf->pdev; > struct ice_vfs *vfs = &pf->vfs; > + struct pci_dev *vfdev = NULL; > struct ice_vf *vf; > - u16 vf_id; > - int err; > + u16 vf_pdev_id; > + int err, pos; > > lockdep_assert_held(&vfs->table_lock); > > - for (vf_id = 0; vf_id < num_vfs; vf_id++) { > + pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_SRIOV); > + pci_read_config_word(pdev, pos + PCI_SRIOV_VF_DID, &vf_pdev_id); > + > + for (u16 vf_id = 0; vf_id < num_vfs; vf_id++) { > vf = kzalloc(sizeof(*vf), GFP_KERNEL); > if (!vf) { > err = -ENOMEM; > @@ -812,11 +817,23 @@ static int ice_create_vf_entries(struct ice_pf *pf, u16 num_vfs) > > ice_initialize_vf_entry(vf); > > + do { > + vfdev = pci_get_device(pdev->vendor, vf_pdev_id, vfdev); > + } while (vfdev && vfdev->physfn != pdev); > + vf->vfdev = vfdev; > vf->vf_sw_id = pf->first_sw; > > + pci_dev_get(vfdev); > + > hash_add_rcu(vfs->table, &vf->entry, vf_id); > } > > + /* Decrement of refcount done by pci_get_device() inside the loop does > + * not touch the last iteration's vfdev, so it has to be done manually > + * to balance pci_dev_get() added within the loop. > + */ > + pci_dev_put(vfdev); > + > return 0; > > err_free_entries: > @@ -1709,31 +1726,16 @@ void ice_print_vfs_mdd_events(struct ice_pf *pf) > > /** > * ice_restore_all_vfs_msi_state - restore VF MSI state after PF FLR > - * @pdev: pointer to a pci_dev structure > + * @pf: pointer to the PF structure > * > * Called when recovering from a PF FLR to restore interrupt capability to > * the VFs. > */ > -void ice_restore_all_vfs_msi_state(struct pci_dev *pdev) > +void ice_restore_all_vfs_msi_state(struct ice_pf *pf) > { > - u16 vf_id; > - int pos; > - > - if (!pci_num_vf(pdev)) > - return; > + struct ice_vf *vf; > + u32 bkt; > > - pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_SRIOV); > - if (pos) { > - struct pci_dev *vfdev; > - > - pci_read_config_word(pdev, pos + PCI_SRIOV_VF_DID, > - &vf_id); > - vfdev = pci_get_device(pdev->vendor, vf_id, NULL); > - while (vfdev) { > - if (vfdev->is_virtfn && vfdev->physfn == pdev) > - pci_restore_msi_state(vfdev); > - vfdev = pci_get_device(pdev->vendor, vf_id, > - vfdev); > - } > - } > + ice_for_each_vf(pf, bkt, vf) > + pci_restore_msi_state(vf->vfdev); > } > diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.h b/drivers/net/ethernet/intel/ice/ice_sriov.h > index 346cb2666f3a..06829443d540 100644 > --- a/drivers/net/ethernet/intel/ice/ice_sriov.h > +++ b/drivers/net/ethernet/intel/ice/ice_sriov.h > @@ -33,7 +33,7 @@ int > ice_get_vf_cfg(struct net_device *netdev, int vf_id, struct ifla_vf_info *ivi); > > void ice_free_vfs(struct ice_pf *pf); > -void ice_restore_all_vfs_msi_state(struct pci_dev *pdev); > +void ice_restore_all_vfs_msi_state(struct ice_pf *pf); > > int > ice_set_vf_port_vlan(struct net_device *netdev, int vf_id, u16 vlan_id, u8 qos, > @@ -67,7 +67,7 @@ static inline > void ice_vf_lan_overflow_event(struct ice_pf *pf, struct ice_rq_event_info *event) { } > static inline void ice_print_vfs_mdd_events(struct ice_pf *pf) { } > static inline void ice_print_vf_rx_mdd_event(struct ice_vf *vf) { } > -static inline void ice_restore_all_vfs_msi_state(struct pci_dev *pdev) { } > +static inline void ice_restore_all_vfs_msi_state(struct ice_pf *pf) { } > > static inline int > ice_sriov_configure(struct pci_dev __always_unused *pdev, > diff --git a/drivers/net/ethernet/intel/ice/ice_vf_lib.c b/drivers/net/ethernet/intel/ice/ice_vf_lib.c > index 24e4f4d897b6..aca1f2ea5034 100644 > --- a/drivers/net/ethernet/intel/ice/ice_vf_lib.c > +++ b/drivers/net/ethernet/intel/ice/ice_vf_lib.c > @@ -56,6 +56,8 @@ static void ice_release_vf(struct kref *ref) > { > struct ice_vf *vf = container_of(ref, struct ice_vf, refcnt); > > + pci_dev_put(vf->vfdev); > + > vf->vf_ops->free(vf); > } > > diff --git a/drivers/net/ethernet/intel/ice/ice_vf_lib.h b/drivers/net/ethernet/intel/ice/ice_vf_lib.h > index 31a082e8a827..628396aa4a04 100644 > --- a/drivers/net/ethernet/intel/ice/ice_vf_lib.h > +++ b/drivers/net/ethernet/intel/ice/ice_vf_lib.h > @@ -82,7 +82,7 @@ struct ice_vf { > struct rcu_head rcu; > struct kref refcnt; > struct ice_pf *pf; > - > + struct pci_dev *vfdev; > /* Used during virtchnl message handling and NDO ops against the VF > * that will trigger a VFR > */
> -----Original Message----- > From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of > Mateusz Polchlopek > Sent: Tuesday, September 12, 2023 2:34 PM > To: intel-wired-lan@lists.osuosl.org > Cc: netdev@vger.kernel.org; Brandeburg, Jesse > <jesse.brandeburg@intel.com>; leon@kernel.org; Kitszel, Przemyslaw > <przemyslaw.kitszel@intel.com> > Subject: Re: [Intel-wired-lan] [PATCH] [PATCH iwl-next v2] ice: store VF's > pci_dev ptr in ice_vf > > On 9/12/2023 1:56 PM, Mateusz Polchlopek wrote: > > From: Przemek Kitszel <przemyslaw.kitszel@intel.com> > > > > Extend struct ice_vf by vfdev. > > Calculation of vfdev falls more nicely into ice_create_vf_entries(). > > > > Caching of vfdev enables simplification of ice_restore_all_vfs_msi_state(). > > > > Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com> > > Reviewed-by: Jacob Keller <jacob.e.keller@intel.com> > > Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com> > > Co-developed-by: Mateusz Polchlopek <mateusz.polchlopek@intel.com> > > Signed-off-by: Mateusz Polchlopek <mateusz.polchlopek@intel.com> > > I forgot to attach changelog, sorry: > > v2: balance vfdev refcount > > v2: > https://lore.kernel.org/netdev/20230912115626.105828-1- > mateusz.polchlopek@intel.com/ > v1: > https://lore.kernel.org/netdev/20230816085454.235440-1- > przemyslaw.kitszel@intel.com/ > > > --- > > drivers/net/ethernet/intel/ice/ice_main.c | 2 +- > > drivers/net/ethernet/intel/ice/ice_sriov.c | 50 +++++++++++---------- > > drivers/net/ethernet/intel/ice/ice_sriov.h | 4 +- > > drivers/net/ethernet/intel/ice/ice_vf_lib.c | 2 + > > drivers/net/ethernet/intel/ice/ice_vf_lib.h | 2 +- > > 5 files changed, 32 insertions(+), 28 deletions(-) > > > > diff --git a/drivers/net/ethernet/intel/ice/ice_main.c > > b/drivers/net/ethernet/intel/ice/ice_main.c > > index a5997008bb98..38adffbe0edf 100644 > > --- a/drivers/net/ethernet/intel/ice/ice_main.c > > +++ b/drivers/net/ethernet/intel/ice/ice_main.c > > @@ -5561,7 +5561,7 @@ static void ice_pci_err_resume(struct pci_dev Tested-by: Rafal Romanowski <rafal.romanowski@intel.com>
On Tue, Sep 12, 2023 at 07:56:26AM -0400, Mateusz Polchlopek wrote: > From: Przemek Kitszel <przemyslaw.kitszel@intel.com> > > Extend struct ice_vf by vfdev. > Calculation of vfdev falls more nicely into ice_create_vf_entries(). > > Caching of vfdev enables simplification of ice_restore_all_vfs_msi_state(). > > Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com> > Reviewed-by: Jacob Keller <jacob.e.keller@intel.com> > Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com> > Co-developed-by: Mateusz Polchlopek <mateusz.polchlopek@intel.com> > Signed-off-by: Mateusz Polchlopek <mateusz.polchlopek@intel.com> Reviewed-by: Simon Horman <horms@kernel.org>
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c index a5997008bb98..38adffbe0edf 100644 --- a/drivers/net/ethernet/intel/ice/ice_main.c +++ b/drivers/net/ethernet/intel/ice/ice_main.c @@ -5561,7 +5561,7 @@ static void ice_pci_err_resume(struct pci_dev *pdev) return; } - ice_restore_all_vfs_msi_state(pdev); + ice_restore_all_vfs_msi_state(pf); ice_do_reset(pf, ICE_RESET_PFR); ice_service_task_restart(pf); diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c b/drivers/net/ethernet/intel/ice/ice_sriov.c index 31314e7540f8..4ae59c59e22b 100644 --- a/drivers/net/ethernet/intel/ice/ice_sriov.c +++ b/drivers/net/ethernet/intel/ice/ice_sriov.c @@ -789,14 +789,19 @@ static const struct ice_vf_ops ice_sriov_vf_ops = { */ static int ice_create_vf_entries(struct ice_pf *pf, u16 num_vfs) { + struct pci_dev *pdev = pf->pdev; struct ice_vfs *vfs = &pf->vfs; + struct pci_dev *vfdev = NULL; struct ice_vf *vf; - u16 vf_id; - int err; + u16 vf_pdev_id; + int err, pos; lockdep_assert_held(&vfs->table_lock); - for (vf_id = 0; vf_id < num_vfs; vf_id++) { + pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_SRIOV); + pci_read_config_word(pdev, pos + PCI_SRIOV_VF_DID, &vf_pdev_id); + + for (u16 vf_id = 0; vf_id < num_vfs; vf_id++) { vf = kzalloc(sizeof(*vf), GFP_KERNEL); if (!vf) { err = -ENOMEM; @@ -812,11 +817,23 @@ static int ice_create_vf_entries(struct ice_pf *pf, u16 num_vfs) ice_initialize_vf_entry(vf); + do { + vfdev = pci_get_device(pdev->vendor, vf_pdev_id, vfdev); + } while (vfdev && vfdev->physfn != pdev); + vf->vfdev = vfdev; vf->vf_sw_id = pf->first_sw; + pci_dev_get(vfdev); + hash_add_rcu(vfs->table, &vf->entry, vf_id); } + /* Decrement of refcount done by pci_get_device() inside the loop does + * not touch the last iteration's vfdev, so it has to be done manually + * to balance pci_dev_get() added within the loop. + */ + pci_dev_put(vfdev); + return 0; err_free_entries: @@ -1709,31 +1726,16 @@ void ice_print_vfs_mdd_events(struct ice_pf *pf) /** * ice_restore_all_vfs_msi_state - restore VF MSI state after PF FLR - * @pdev: pointer to a pci_dev structure + * @pf: pointer to the PF structure * * Called when recovering from a PF FLR to restore interrupt capability to * the VFs. */ -void ice_restore_all_vfs_msi_state(struct pci_dev *pdev) +void ice_restore_all_vfs_msi_state(struct ice_pf *pf) { - u16 vf_id; - int pos; - - if (!pci_num_vf(pdev)) - return; + struct ice_vf *vf; + u32 bkt; - pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_SRIOV); - if (pos) { - struct pci_dev *vfdev; - - pci_read_config_word(pdev, pos + PCI_SRIOV_VF_DID, - &vf_id); - vfdev = pci_get_device(pdev->vendor, vf_id, NULL); - while (vfdev) { - if (vfdev->is_virtfn && vfdev->physfn == pdev) - pci_restore_msi_state(vfdev); - vfdev = pci_get_device(pdev->vendor, vf_id, - vfdev); - } - } + ice_for_each_vf(pf, bkt, vf) + pci_restore_msi_state(vf->vfdev); } diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.h b/drivers/net/ethernet/intel/ice/ice_sriov.h index 346cb2666f3a..06829443d540 100644 --- a/drivers/net/ethernet/intel/ice/ice_sriov.h +++ b/drivers/net/ethernet/intel/ice/ice_sriov.h @@ -33,7 +33,7 @@ int ice_get_vf_cfg(struct net_device *netdev, int vf_id, struct ifla_vf_info *ivi); void ice_free_vfs(struct ice_pf *pf); -void ice_restore_all_vfs_msi_state(struct pci_dev *pdev); +void ice_restore_all_vfs_msi_state(struct ice_pf *pf); int ice_set_vf_port_vlan(struct net_device *netdev, int vf_id, u16 vlan_id, u8 qos, @@ -67,7 +67,7 @@ static inline void ice_vf_lan_overflow_event(struct ice_pf *pf, struct ice_rq_event_info *event) { } static inline void ice_print_vfs_mdd_events(struct ice_pf *pf) { } static inline void ice_print_vf_rx_mdd_event(struct ice_vf *vf) { } -static inline void ice_restore_all_vfs_msi_state(struct pci_dev *pdev) { } +static inline void ice_restore_all_vfs_msi_state(struct ice_pf *pf) { } static inline int ice_sriov_configure(struct pci_dev __always_unused *pdev, diff --git a/drivers/net/ethernet/intel/ice/ice_vf_lib.c b/drivers/net/ethernet/intel/ice/ice_vf_lib.c index 24e4f4d897b6..aca1f2ea5034 100644 --- a/drivers/net/ethernet/intel/ice/ice_vf_lib.c +++ b/drivers/net/ethernet/intel/ice/ice_vf_lib.c @@ -56,6 +56,8 @@ static void ice_release_vf(struct kref *ref) { struct ice_vf *vf = container_of(ref, struct ice_vf, refcnt); + pci_dev_put(vf->vfdev); + vf->vf_ops->free(vf); } diff --git a/drivers/net/ethernet/intel/ice/ice_vf_lib.h b/drivers/net/ethernet/intel/ice/ice_vf_lib.h index 31a082e8a827..628396aa4a04 100644 --- a/drivers/net/ethernet/intel/ice/ice_vf_lib.h +++ b/drivers/net/ethernet/intel/ice/ice_vf_lib.h @@ -82,7 +82,7 @@ struct ice_vf { struct rcu_head rcu; struct kref refcnt; struct ice_pf *pf; - + struct pci_dev *vfdev; /* Used during virtchnl message handling and NDO ops against the VF * that will trigger a VFR */