diff mbox series

[iwl-next,v1,4/4] ice: manage VFs MSI-X using resource tracking

Message ID 20230615123830.155927-5-michal.swiatkowski@linux.intel.com (mailing list archive)
State Awaiting Upstream
Delegated to: Netdev Maintainers
Headers show
Series change MSI-X vectors per VF | expand

Checks

Context Check Description
netdev/series_format warning Target tree name not specified in the subject
netdev/tree_selection success Guessed tree name to be net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 8 this patch: 8
netdev/cc_maintainers warning 6 maintainers not CCed: kuba@kernel.org jesse.brandeburg@intel.com davem@davemloft.net anthony.l.nguyen@intel.com pabeni@redhat.com edumazet@google.com
netdev/build_clang success Errors and warnings before: 8 this patch: 8
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 8 this patch: 8
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 220 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Michal Swiatkowski June 15, 2023, 12:38 p.m. UTC 
Track MSI-X for VFs using bitmap, by setting and clearing bitmap during
allocation and freeing.

Try to linearize irqs usage for VFs, by freeing them and allocating once
again. Do it only for VFs that aren't currently running.

Signed-off-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_sriov.c | 170 ++++++++++++++++++---
 1 file changed, 151 insertions(+), 19 deletions(-)

Comments

Jacob Keller June 15, 2023, 3:57 p.m. UTC | #1 
> -----Original Message-----
> From: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> Sent: Thursday, June 15, 2023 5:39 AM
> To: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org; Keller, Jacob E <jacob.e.keller@intel.com>; Kitszel,
> Przemyslaw <przemyslaw.kitszel@intel.com>; Michal Swiatkowski
> <michal.swiatkowski@linux.intel.com>
> Subject: [PATCH iwl-next v1 4/4] ice: manage VFs MSI-X using resource tracking
> 
> Track MSI-X for VFs using bitmap, by setting and clearing bitmap during
> allocation and freeing.
> 
> Try to linearize irqs usage for VFs, by freeing them and allocating once
> again. Do it only for VFs that aren't currently running.
> 
> Signed-off-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_sriov.c | 170 ++++++++++++++++++---
>  1 file changed, 151 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c
> b/drivers/net/ethernet/intel/ice/ice_sriov.c
> index e20ef1924fae..78a41163755b 100644
> --- a/drivers/net/ethernet/intel/ice/ice_sriov.c
> +++ b/drivers/net/ethernet/intel/ice/ice_sriov.c
> @@ -246,22 +246,6 @@ static struct ice_vsi *ice_vf_vsi_setup(struct ice_vf *vf)
>  	return vsi;
>  }
> 
> -/**
> - * ice_calc_vf_first_vector_idx - Calculate MSIX vector index in the PF space
> - * @pf: pointer to PF structure
> - * @vf: pointer to VF that the first MSIX vector index is being calculated for
> - *
> - * This returns the first MSIX vector index in PF space that is used by this VF.
> - * This index is used when accessing PF relative registers such as
> - * GLINT_VECT2FUNC and GLINT_DYN_CTL.
> - * This will always be the OICR index in the AVF driver so any functionality
> - * using vf->first_vector_idx for queue configuration will have to increment by
> - * 1 to avoid meddling with the OICR index.
> - */
> -static int ice_calc_vf_first_vector_idx(struct ice_pf *pf, struct ice_vf *vf)
> -{
> -	return pf->sriov_base_vector + vf->vf_id * pf->vfs.num_msix_per;
> -}
> 
>  /**
>   * ice_ena_vf_msix_mappings - enable VF MSIX mappings in hardware
> @@ -528,6 +512,52 @@ static int ice_set_per_vf_res(struct ice_pf *pf, u16
> num_vfs)
>  	return 0;
>  }
> 
> +/**
> + * ice_sriov_get_irqs - get irqs for SR-IOV usacase
> + * @pf: pointer to PF structure
> + * @needed: number of irqs to get
> + *
> + * This returns the first MSI-X vector index in PF space that is used by this
> + * VF. This index is used when accessing PF relative registers such as
> + * GLINT_VECT2FUNC and GLINT_DYN_CTL.
> + * This will always be the OICR index in the AVF driver so any functionality
> + * using vf->first_vector_idx for queue configuration_id: id of VF which will
> + * use this irqs
> + *
> + * Only SRIOV specific vectors are tracked in sriov_irq_bm. SRIOV vectors are
> + * allocated from the end of global irq index. First bit in sriov_irq_bm means
> + * last irq index etc. It simplifies extension of SRIOV vectors.
> + * They will be always located from sriov_base_vector to the last irq
> + * index. While increasing/decreasing sriov_base_vector can be moved.
> + */
> +static int ice_sriov_get_irqs(struct ice_pf *pf, u16 needed)
> +{
> +	int res = bitmap_find_next_zero_area(pf->sriov_irq_bm,
> +					     pf->sriov_irq_size, 0, needed, 0);
> +	/* conversion from number in bitmap to global irq index */
> +	int index = pf->sriov_irq_size - res - needed;
> +
> +	if (res >= pf->sriov_irq_size || index < pf->sriov_base_vector)
> +		return -ENOENT;
> +
> +	bitmap_set(pf->sriov_irq_bm, res, needed);
> +	return index;

Shouldn't it be possible to use the xarray that was recently done for dynamic IRQ allocation for this now? It might take a little more refactor work though, hmm. It feels weird to introduce yet another data structure for a nearly identical purpose...

> +}
> +
> +/**
> + * ice_sriov_free_irqs - free irqs used by the VF
> + * @pf: pointer to PF structure
> + * @vf: pointer to VF structure
> + */
> +static void ice_sriov_free_irqs(struct ice_pf *pf, struct ice_vf *vf)
> +{
> +	/* Move back from first vector index to first index in bitmap */
> +	int bm_i = pf->sriov_irq_size - vf->first_vector_idx - vf->num_msix;
> +
> +	bitmap_clear(pf->sriov_irq_bm, bm_i, vf->num_msix);
> +	vf->first_vector_idx = 0;
> +}
> +
>  /**
>   * ice_init_vf_vsi_res - initialize/setup VF VSI resources
>   * @vf: VF to initialize/setup the VSI for
> @@ -541,7 +571,9 @@ static int ice_init_vf_vsi_res(struct ice_vf *vf)
>  	struct ice_vsi *vsi;
>  	int err;
> 
> -	vf->first_vector_idx = ice_calc_vf_first_vector_idx(pf, vf);
> +	vf->first_vector_idx = ice_sriov_get_irqs(pf, vf->num_msix);
> +	if (vf->first_vector_idx < 0)
> +		return -ENOMEM;
> 
>  	vsi = ice_vf_vsi_setup(vf);
>  	if (!vsi)
> @@ -984,6 +1016,52 @@ u32 ice_sriov_get_vf_total_msix(struct pci_dev *pdev)
>  	return pf->sriov_irq_size - ice_get_max_used_msix_vector(pf);
>  }
> 
> +static int ice_sriov_move_base_vector(struct ice_pf *pf, int move)
> +{
> +	if (pf->sriov_base_vector - move < ice_get_max_used_msix_vector(pf))
> +		return -ENOMEM;
> +
> +	pf->sriov_base_vector -= move;
> +	return 0;
> +}
> +
> +static void ice_sriov_remap_vectors(struct ice_pf *pf, u16 restricted_id)
> +{
> +	u16 vf_ids[ICE_MAX_SRIOV_VFS];
> +	struct ice_vf *tmp_vf;
> +	int to_remap = 0, bkt;
> +
> +	/* For better irqs usage try to remap irqs of VFs
> +	 * that aren't running yet
> +	 */
> +	ice_for_each_vf(pf, bkt, tmp_vf) {
> +		/* skip VF which is changing the number of MSI-X */
> +		if (restricted_id == tmp_vf->vf_id ||
> +		    test_bit(ICE_VF_STATE_ACTIVE, tmp_vf->vf_states))
> +			continue;
> +
> +		ice_dis_vf_mappings(tmp_vf);
> +		ice_sriov_free_irqs(pf, tmp_vf);
> +
> +		vf_ids[to_remap] = tmp_vf->vf_id;
> +		to_remap += 1;
> +	}
> +
> +	for (int i = 0; i < to_remap; i++) {
> +		tmp_vf = ice_get_vf_by_id(pf, vf_ids[i]);
> +		if (!tmp_vf)
> +			continue;
> +
> +		tmp_vf->first_vector_idx =
> +			ice_sriov_get_irqs(pf, tmp_vf->num_msix);
> +		/* there is no need to rebuild VSI as we are only changing the
> +		 * vector indexes not amount of MSI-X or queues
> +		 */
> +		ice_ena_vf_mappings(tmp_vf);
> +		ice_put_vf(tmp_vf);
> +	}
> +}
> +
>  /**
>   * ice_sriov_set_msix_vec_count
>   * @vf_dev: pointer to pci_dev struct of VF device
> @@ -1002,8 +1080,9 @@ int ice_sriov_set_msix_vec_count(struct pci_dev
> *vf_dev, int msix_vec_count)
>  {
>  	struct pci_dev *pdev = pci_physfn(vf_dev);
>  	struct ice_pf *pf = pci_get_drvdata(pdev);
> +	u16 prev_msix, prev_queues, queues;
> +	bool needs_rebuild = false;
>  	struct ice_vf *vf;
> -	u16 queues;
>  	int id;
> 
>  	if (!ice_get_num_vfs(pf))
> @@ -1016,6 +1095,13 @@ int ice_sriov_set_msix_vec_count(struct pci_dev
> *vf_dev, int msix_vec_count)
>  	/* add 1 MSI-X for OICR */
>  	msix_vec_count += 1;
> 
> +	if (queues > min(ice_get_avail_txq_count(pf),
> +			 ice_get_avail_rxq_count(pf)))
> +		return -EINVAL;
> +
> +	if (msix_vec_count < ICE_MIN_INTR_PER_VF)
> +		return -EINVAL;
> +
>  	/* Transition of PCI VF function number to function_id */
>  	for (id = 0; id < pci_num_vf(pdev); id++) {
>  		if (vf_dev->devfn == pci_iov_virtfn_devfn(pdev, id))
> @@ -1030,14 +1116,60 @@ int ice_sriov_set_msix_vec_count(struct pci_dev
> *vf_dev, int msix_vec_count)
>  	if (!vf)
>  		return -ENOENT;
> 
> +	prev_msix = vf->num_msix;
> +	prev_queues = vf->num_vf_qs;
> +
> +	if (ice_sriov_move_base_vector(pf, msix_vec_count - prev_msix)) {
> +		ice_put_vf(vf);
> +		return -ENOSPC;
> +	}
> +
>  	ice_dis_vf_mappings(vf);
> +	ice_sriov_free_irqs(pf, vf);
> +
> +	/* Remap all VFs beside the one is now configured */
> +	ice_sriov_remap_vectors(pf, vf->vf_id);
> +
>  	vf->num_msix = msix_vec_count;
>  	vf->num_vf_qs = queues;
> -	ice_vsi_rebuild(ice_get_vf_vsi(vf), ICE_VSI_FLAG_NO_INIT);
> +	vf->first_vector_idx = ice_sriov_get_irqs(pf, vf->num_msix);
> +	if (vf->first_vector_idx < 0)
> +		goto unroll;
> +
> +	ice_vf_vsi_release(vf);
> +	if (vf->vf_ops->create_vsi(vf)) {
> +		/* Try to rebuild with previous values */
> +		needs_rebuild = true;
> +		goto unroll;
> +	}
> +
> +	dev_info(ice_pf_to_dev(pf),
> +		 "Changing VF %d resources to %d vectors and %d queues\n",
> +		 vf->vf_id, vf->num_msix, vf->num_vf_qs);
> +
>  	ice_ena_vf_mappings(vf);
>  	ice_put_vf(vf);
> 
>  	return 0;
> +
> +unroll:
> +	dev_info(ice_pf_to_dev(pf),
> +		 "Can't set %d vectors on VF %d, falling back to %d\n",
> +		 vf->num_msix, vf->vf_id, prev_msix);
> +
> +	vf->num_msix = prev_msix;
> +	vf->num_vf_qs = prev_queues;
> +	vf->first_vector_idx = ice_sriov_get_irqs(pf, vf->num_msix);
> +	if (vf->first_vector_idx < 0)
> +		return -EINVAL;
> +
> +	if (needs_rebuild)
> +		vf->vf_ops->create_vsi(vf);
> +
> +	ice_ena_vf_mappings(vf);
> +	ice_put_vf(vf);
> +
> +	return -EINVAL;
>  }
> 
>  /**
> --
> 2.40.1
Michal Swiatkowski June 16, 2023, 8:37 a.m. UTC | #2 
On Thu, Jun 15, 2023 at 03:57:37PM +0000, Keller, Jacob E wrote:
> 
> 
> > -----Original Message-----
> > From: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> > Sent: Thursday, June 15, 2023 5:39 AM
> > To: intel-wired-lan@lists.osuosl.org
> > Cc: netdev@vger.kernel.org; Keller, Jacob E <jacob.e.keller@intel.com>; Kitszel,
> > Przemyslaw <przemyslaw.kitszel@intel.com>; Michal Swiatkowski
> > <michal.swiatkowski@linux.intel.com>
> > Subject: [PATCH iwl-next v1 4/4] ice: manage VFs MSI-X using resource tracking
> > 
> > Track MSI-X for VFs using bitmap, by setting and clearing bitmap during
> > allocation and freeing.
> > 
> > Try to linearize irqs usage for VFs, by freeing them and allocating once
> > again. Do it only for VFs that aren't currently running.
> > 
> > Signed-off-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> > Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> > ---
> >  drivers/net/ethernet/intel/ice/ice_sriov.c | 170 ++++++++++++++++++---
> >  1 file changed, 151 insertions(+), 19 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c
> > b/drivers/net/ethernet/intel/ice/ice_sriov.c
> > index e20ef1924fae..78a41163755b 100644
> > --- a/drivers/net/ethernet/intel/ice/ice_sriov.c
> > +++ b/drivers/net/ethernet/intel/ice/ice_sriov.c
> > @@ -246,22 +246,6 @@ static struct ice_vsi *ice_vf_vsi_setup(struct ice_vf *vf)
> >  	return vsi;
> >  }
> > 
> > -/**
> > - * ice_calc_vf_first_vector_idx - Calculate MSIX vector index in the PF space
> > - * @pf: pointer to PF structure
> > - * @vf: pointer to VF that the first MSIX vector index is being calculated for
> > - *
> > - * This returns the first MSIX vector index in PF space that is used by this VF.
> > - * This index is used when accessing PF relative registers such as
> > - * GLINT_VECT2FUNC and GLINT_DYN_CTL.
> > - * This will always be the OICR index in the AVF driver so any functionality
> > - * using vf->first_vector_idx for queue configuration will have to increment by
> > - * 1 to avoid meddling with the OICR index.
> > - */
> > -static int ice_calc_vf_first_vector_idx(struct ice_pf *pf, struct ice_vf *vf)
> > -{
> > -	return pf->sriov_base_vector + vf->vf_id * pf->vfs.num_msix_per;
> > -}
> > 
> >  /**
> >   * ice_ena_vf_msix_mappings - enable VF MSIX mappings in hardware
> > @@ -528,6 +512,52 @@ static int ice_set_per_vf_res(struct ice_pf *pf, u16
> > num_vfs)
> >  	return 0;
> >  }
> > 
> > +/**
> > + * ice_sriov_get_irqs - get irqs for SR-IOV usacase
> > + * @pf: pointer to PF structure
> > + * @needed: number of irqs to get
> > + *
> > + * This returns the first MSI-X vector index in PF space that is used by this
> > + * VF. This index is used when accessing PF relative registers such as
> > + * GLINT_VECT2FUNC and GLINT_DYN_CTL.
> > + * This will always be the OICR index in the AVF driver so any functionality
> > + * using vf->first_vector_idx for queue configuration_id: id of VF which will
> > + * use this irqs
> > + *
> > + * Only SRIOV specific vectors are tracked in sriov_irq_bm. SRIOV vectors are
> > + * allocated from the end of global irq index. First bit in sriov_irq_bm means
> > + * last irq index etc. It simplifies extension of SRIOV vectors.
> > + * They will be always located from sriov_base_vector to the last irq
> > + * index. While increasing/decreasing sriov_base_vector can be moved.
> > + */
> > +static int ice_sriov_get_irqs(struct ice_pf *pf, u16 needed)
> > +{
> > +	int res = bitmap_find_next_zero_area(pf->sriov_irq_bm,
> > +					     pf->sriov_irq_size, 0, needed, 0);
> > +	/* conversion from number in bitmap to global irq index */
> > +	int index = pf->sriov_irq_size - res - needed;
> > +
> > +	if (res >= pf->sriov_irq_size || index < pf->sriov_base_vector)
> > +		return -ENOENT;
> > +
> > +	bitmap_set(pf->sriov_irq_bm, res, needed);
> > +	return index;
> 
> Shouldn't it be possible to use the xarray that was recently done for dynamic IRQ allocation for this now? It might take a little more refactor work though, hmm. It feels weird to introduce yet another data structure for a nearly identical purpose...
> 

I used bitmap because it was easy to get linear irq indexes (it is need
for VF to have linear indexes). Do you know how to assume that with
xarray? I felt like solution with storing in xarray and searching for
linear space was more complicated than bitmap, but probably I don't know
useful xarray mechanism for that purpose. If you know please point me
and I will rewrite it to use xarray.

Thanks

[...]
Jacob Keller June 20, 2023, 5:37 a.m. UTC | #3 
> -----Original Message-----
> From: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> Sent: Friday, June 16, 2023 1:37 AM
> To: Keller, Jacob E <jacob.e.keller@intel.com>
> Cc: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>
> Subject: Re: [PATCH iwl-next v1 4/4] ice: manage VFs MSI-X using resource
> tracking
> 
> On Thu, Jun 15, 2023 at 03:57:37PM +0000, Keller, Jacob E wrote:
> >
> >
> > > -----Original Message-----
> > > From: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> > > Sent: Thursday, June 15, 2023 5:39 AM
> > > To: intel-wired-lan@lists.osuosl.org
> > > Cc: netdev@vger.kernel.org; Keller, Jacob E <jacob.e.keller@intel.com>;
> Kitszel,
> > > Przemyslaw <przemyslaw.kitszel@intel.com>; Michal Swiatkowski
> > > <michal.swiatkowski@linux.intel.com>
> > > Subject: [PATCH iwl-next v1 4/4] ice: manage VFs MSI-X using resource
> tracking
> > >
> > > Track MSI-X for VFs using bitmap, by setting and clearing bitmap during
> > > allocation and freeing.
> > >
> > > Try to linearize irqs usage for VFs, by freeing them and allocating once
> > > again. Do it only for VFs that aren't currently running.
> > >
> > > Signed-off-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> > > Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> > > ---
> > >  drivers/net/ethernet/intel/ice/ice_sriov.c | 170 ++++++++++++++++++---
> > >  1 file changed, 151 insertions(+), 19 deletions(-)
> > >
> > > diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c
> > > b/drivers/net/ethernet/intel/ice/ice_sriov.c
> > > index e20ef1924fae..78a41163755b 100644
> > > --- a/drivers/net/ethernet/intel/ice/ice_sriov.c
> > > +++ b/drivers/net/ethernet/intel/ice/ice_sriov.c
> > > @@ -246,22 +246,6 @@ static struct ice_vsi *ice_vf_vsi_setup(struct ice_vf
> *vf)
> > >  	return vsi;
> > >  }
> > >
> > > -/**
> > > - * ice_calc_vf_first_vector_idx - Calculate MSIX vector index in the PF space
> > > - * @pf: pointer to PF structure
> > > - * @vf: pointer to VF that the first MSIX vector index is being calculated for
> > > - *
> > > - * This returns the first MSIX vector index in PF space that is used by this VF.
> > > - * This index is used when accessing PF relative registers such as
> > > - * GLINT_VECT2FUNC and GLINT_DYN_CTL.
> > > - * This will always be the OICR index in the AVF driver so any functionality
> > > - * using vf->first_vector_idx for queue configuration will have to increment
> by
> > > - * 1 to avoid meddling with the OICR index.
> > > - */
> > > -static int ice_calc_vf_first_vector_idx(struct ice_pf *pf, struct ice_vf *vf)
> > > -{
> > > -	return pf->sriov_base_vector + vf->vf_id * pf->vfs.num_msix_per;
> > > -}
> > >
> > >  /**
> > >   * ice_ena_vf_msix_mappings - enable VF MSIX mappings in hardware
> > > @@ -528,6 +512,52 @@ static int ice_set_per_vf_res(struct ice_pf *pf, u16
> > > num_vfs)
> > >  	return 0;
> > >  }
> > >
> > > +/**
> > > + * ice_sriov_get_irqs - get irqs for SR-IOV usacase
> > > + * @pf: pointer to PF structure
> > > + * @needed: number of irqs to get
> > > + *
> > > + * This returns the first MSI-X vector index in PF space that is used by this
> > > + * VF. This index is used when accessing PF relative registers such as
> > > + * GLINT_VECT2FUNC and GLINT_DYN_CTL.
> > > + * This will always be the OICR index in the AVF driver so any functionality
> > > + * using vf->first_vector_idx for queue configuration_id: id of VF which will
> > > + * use this irqs
> > > + *
> > > + * Only SRIOV specific vectors are tracked in sriov_irq_bm. SRIOV vectors are
> > > + * allocated from the end of global irq index. First bit in sriov_irq_bm means
> > > + * last irq index etc. It simplifies extension of SRIOV vectors.
> > > + * They will be always located from sriov_base_vector to the last irq
> > > + * index. While increasing/decreasing sriov_base_vector can be moved.
> > > + */
> > > +static int ice_sriov_get_irqs(struct ice_pf *pf, u16 needed)
> > > +{
> > > +	int res = bitmap_find_next_zero_area(pf->sriov_irq_bm,
> > > +					     pf->sriov_irq_size, 0, needed, 0);
> > > +	/* conversion from number in bitmap to global irq index */
> > > +	int index = pf->sriov_irq_size - res - needed;
> > > +
> > > +	if (res >= pf->sriov_irq_size || index < pf->sriov_base_vector)
> > > +		return -ENOENT;
> > > +
> > > +	bitmap_set(pf->sriov_irq_bm, res, needed);
> > > +	return index;
> >
> > Shouldn't it be possible to use the xarray that was recently done for dynamic
> IRQ allocation for this now? It might take a little more refactor work though,
> hmm. It feels weird to introduce yet another data structure for a nearly identical
> purpose...
> >
> 
> I used bitmap because it was easy to get linear irq indexes (it is need
> for VF to have linear indexes). Do you know how to assume that with
> xarray? I felt like solution with storing in xarray and searching for
> linear space was more complicated than bitmap, but probably I don't know
> useful xarray mechanism for that purpose. If you know please point me
> and I will rewrite it to use xarray.
> 
> Thanks
> 
> [...]

My goal wasn't specifically to use xarray, but rather to use the existing IRQ tracking data structure (which happens to be xarray), rather than adding another tracking structure that is separate. I don't know if that is feasible, so maybe having a separate bitmap is necessary.
Romanowski, Rafal Nov. 23, 2023, 5:22 p.m. UTC | #4 
> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Keller, Jacob E
> Sent: Thursday, June 15, 2023 5:58 PM
> To: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>; intel-wired-
> lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>
> Subject: Re: [Intel-wired-lan] [PATCH iwl-next v1 4/4] ice: manage VFs MSI-X
> using resource tracking
> 
> 
> 
> > -----Original Message-----
> > From: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> > Sent: Thursday, June 15, 2023 5:39 AM
> > To: intel-wired-lan@lists.osuosl.org
> > Cc: netdev@vger.kernel.org; Keller, Jacob E
> > <jacob.e.keller@intel.com>; Kitszel, Przemyslaw
> > <przemyslaw.kitszel@intel.com>; Michal Swiatkowski
> > <michal.swiatkowski@linux.intel.com>
> > Subject: [PATCH iwl-next v1 4/4] ice: manage VFs MSI-X using resource
> > tracking
> >
> > Track MSI-X for VFs using bitmap, by setting and clearing bitmap
> > during allocation and freeing.
> >
> > Try to linearize irqs usage for VFs, by freeing them and allocating
> > once again. Do it only for VFs that aren't currently running.
> >
> > Signed-off-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> > Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> > ---
> >  drivers/net/ethernet/intel/ice/ice_sriov.c | 170
> > ++++++++++++++++++---
> >  1 file changed, 151 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c
> > b/drivers/net/ethernet/intel/ice/ice_sriov.c
> > index e20ef1924fae..78a41163755b 100644
> > --- a/drivers/net/ethernet/intel/ice/ice_sriov.c
> > +++ b/drivers/net/ethernet/intel/ice/ice_sriov.c


Tested-by: Rafal Romanowski <rafal.romanowski@intel.com>
diff mbox series

Patch

diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c b/drivers/net/ethernet/intel/ice/ice_sriov.c
index e20ef1924fae..78a41163755b 100644
--- a/drivers/net/ethernet/intel/ice/ice_sriov.c
+++ b/drivers/net/ethernet/intel/ice/ice_sriov.c
@@ -246,22 +246,6 @@  static struct ice_vsi *ice_vf_vsi_setup(struct ice_vf *vf)
 	return vsi;
 }
 
-/**
- * ice_calc_vf_first_vector_idx - Calculate MSIX vector index in the PF space
- * @pf: pointer to PF structure
- * @vf: pointer to VF that the first MSIX vector index is being calculated for
- *
- * This returns the first MSIX vector index in PF space that is used by this VF.
- * This index is used when accessing PF relative registers such as
- * GLINT_VECT2FUNC and GLINT_DYN_CTL.
- * This will always be the OICR index in the AVF driver so any functionality
- * using vf->first_vector_idx for queue configuration will have to increment by
- * 1 to avoid meddling with the OICR index.
- */
-static int ice_calc_vf_first_vector_idx(struct ice_pf *pf, struct ice_vf *vf)
-{
-	return pf->sriov_base_vector + vf->vf_id * pf->vfs.num_msix_per;
-}
 
 /**
  * ice_ena_vf_msix_mappings - enable VF MSIX mappings in hardware
@@ -528,6 +512,52 @@  static int ice_set_per_vf_res(struct ice_pf *pf, u16 num_vfs)
 	return 0;
 }
 
+/**
+ * ice_sriov_get_irqs - get irqs for SR-IOV usacase
+ * @pf: pointer to PF structure
+ * @needed: number of irqs to get
+ *
+ * This returns the first MSI-X vector index in PF space that is used by this
+ * VF. This index is used when accessing PF relative registers such as
+ * GLINT_VECT2FUNC and GLINT_DYN_CTL.
+ * This will always be the OICR index in the AVF driver so any functionality
+ * using vf->first_vector_idx for queue configuration_id: id of VF which will
+ * use this irqs
+ *
+ * Only SRIOV specific vectors are tracked in sriov_irq_bm. SRIOV vectors are
+ * allocated from the end of global irq index. First bit in sriov_irq_bm means
+ * last irq index etc. It simplifies extension of SRIOV vectors.
+ * They will be always located from sriov_base_vector to the last irq
+ * index. While increasing/decreasing sriov_base_vector can be moved.
+ */
+static int ice_sriov_get_irqs(struct ice_pf *pf, u16 needed)
+{
+	int res = bitmap_find_next_zero_area(pf->sriov_irq_bm,
+					     pf->sriov_irq_size, 0, needed, 0);
+	/* conversion from number in bitmap to global irq index */
+	int index = pf->sriov_irq_size - res - needed;
+
+	if (res >= pf->sriov_irq_size || index < pf->sriov_base_vector)
+		return -ENOENT;
+
+	bitmap_set(pf->sriov_irq_bm, res, needed);
+	return index;
+}
+
+/**
+ * ice_sriov_free_irqs - free irqs used by the VF
+ * @pf: pointer to PF structure
+ * @vf: pointer to VF structure
+ */
+static void ice_sriov_free_irqs(struct ice_pf *pf, struct ice_vf *vf)
+{
+	/* Move back from first vector index to first index in bitmap */
+	int bm_i = pf->sriov_irq_size - vf->first_vector_idx - vf->num_msix;
+
+	bitmap_clear(pf->sriov_irq_bm, bm_i, vf->num_msix);
+	vf->first_vector_idx = 0;
+}
+
 /**
  * ice_init_vf_vsi_res - initialize/setup VF VSI resources
  * @vf: VF to initialize/setup the VSI for
@@ -541,7 +571,9 @@  static int ice_init_vf_vsi_res(struct ice_vf *vf)
 	struct ice_vsi *vsi;
 	int err;
 
-	vf->first_vector_idx = ice_calc_vf_first_vector_idx(pf, vf);
+	vf->first_vector_idx = ice_sriov_get_irqs(pf, vf->num_msix);
+	if (vf->first_vector_idx < 0)
+		return -ENOMEM;
 
 	vsi = ice_vf_vsi_setup(vf);
 	if (!vsi)
@@ -984,6 +1016,52 @@  u32 ice_sriov_get_vf_total_msix(struct pci_dev *pdev)
 	return pf->sriov_irq_size - ice_get_max_used_msix_vector(pf);
 }
 
+static int ice_sriov_move_base_vector(struct ice_pf *pf, int move)
+{
+	if (pf->sriov_base_vector - move < ice_get_max_used_msix_vector(pf))
+		return -ENOMEM;
+
+	pf->sriov_base_vector -= move;
+	return 0;
+}
+
+static void ice_sriov_remap_vectors(struct ice_pf *pf, u16 restricted_id)
+{
+	u16 vf_ids[ICE_MAX_SRIOV_VFS];
+	struct ice_vf *tmp_vf;
+	int to_remap = 0, bkt;
+
+	/* For better irqs usage try to remap irqs of VFs
+	 * that aren't running yet
+	 */
+	ice_for_each_vf(pf, bkt, tmp_vf) {
+		/* skip VF which is changing the number of MSI-X */
+		if (restricted_id == tmp_vf->vf_id ||
+		    test_bit(ICE_VF_STATE_ACTIVE, tmp_vf->vf_states))
+			continue;
+
+		ice_dis_vf_mappings(tmp_vf);
+		ice_sriov_free_irqs(pf, tmp_vf);
+
+		vf_ids[to_remap] = tmp_vf->vf_id;
+		to_remap += 1;
+	}
+
+	for (int i = 0; i < to_remap; i++) {
+		tmp_vf = ice_get_vf_by_id(pf, vf_ids[i]);
+		if (!tmp_vf)
+			continue;
+
+		tmp_vf->first_vector_idx =
+			ice_sriov_get_irqs(pf, tmp_vf->num_msix);
+		/* there is no need to rebuild VSI as we are only changing the
+		 * vector indexes not amount of MSI-X or queues
+		 */
+		ice_ena_vf_mappings(tmp_vf);
+		ice_put_vf(tmp_vf);
+	}
+}
+
 /**
  * ice_sriov_set_msix_vec_count
  * @vf_dev: pointer to pci_dev struct of VF device
@@ -1002,8 +1080,9 @@  int ice_sriov_set_msix_vec_count(struct pci_dev *vf_dev, int msix_vec_count)
 {
 	struct pci_dev *pdev = pci_physfn(vf_dev);
 	struct ice_pf *pf = pci_get_drvdata(pdev);
+	u16 prev_msix, prev_queues, queues;
+	bool needs_rebuild = false;
 	struct ice_vf *vf;
-	u16 queues;
 	int id;
 
 	if (!ice_get_num_vfs(pf))
@@ -1016,6 +1095,13 @@  int ice_sriov_set_msix_vec_count(struct pci_dev *vf_dev, int msix_vec_count)
 	/* add 1 MSI-X for OICR */
 	msix_vec_count += 1;
 
+	if (queues > min(ice_get_avail_txq_count(pf),
+			 ice_get_avail_rxq_count(pf)))
+		return -EINVAL;
+
+	if (msix_vec_count < ICE_MIN_INTR_PER_VF)
+		return -EINVAL;
+
 	/* Transition of PCI VF function number to function_id */
 	for (id = 0; id < pci_num_vf(pdev); id++) {
 		if (vf_dev->devfn == pci_iov_virtfn_devfn(pdev, id))
@@ -1030,14 +1116,60 @@  int ice_sriov_set_msix_vec_count(struct pci_dev *vf_dev, int msix_vec_count)
 	if (!vf)
 		return -ENOENT;
 
+	prev_msix = vf->num_msix;
+	prev_queues = vf->num_vf_qs;
+
+	if (ice_sriov_move_base_vector(pf, msix_vec_count - prev_msix)) {
+		ice_put_vf(vf);
+		return -ENOSPC;
+	}
+
 	ice_dis_vf_mappings(vf);
+	ice_sriov_free_irqs(pf, vf);
+
+	/* Remap all VFs beside the one is now configured */
+	ice_sriov_remap_vectors(pf, vf->vf_id);
+
 	vf->num_msix = msix_vec_count;
 	vf->num_vf_qs = queues;
-	ice_vsi_rebuild(ice_get_vf_vsi(vf), ICE_VSI_FLAG_NO_INIT);
+	vf->first_vector_idx = ice_sriov_get_irqs(pf, vf->num_msix);
+	if (vf->first_vector_idx < 0)
+		goto unroll;
+
+	ice_vf_vsi_release(vf);
+	if (vf->vf_ops->create_vsi(vf)) {
+		/* Try to rebuild with previous values */
+		needs_rebuild = true;
+		goto unroll;
+	}
+
+	dev_info(ice_pf_to_dev(pf),
+		 "Changing VF %d resources to %d vectors and %d queues\n",
+		 vf->vf_id, vf->num_msix, vf->num_vf_qs);
+
 	ice_ena_vf_mappings(vf);
 	ice_put_vf(vf);
 
 	return 0;
+
+unroll:
+	dev_info(ice_pf_to_dev(pf),
+		 "Can't set %d vectors on VF %d, falling back to %d\n",
+		 vf->num_msix, vf->vf_id, prev_msix);
+
+	vf->num_msix = prev_msix;
+	vf->num_vf_qs = prev_queues;
+	vf->first_vector_idx = ice_sriov_get_irqs(pf, vf->num_msix);
+	if (vf->first_vector_idx < 0)
+		return -EINVAL;
+
+	if (needs_rebuild)
+		vf->vf_ops->create_vsi(vf);
+
+	ice_ena_vf_mappings(vf);
+	ice_put_vf(vf);
+
+	return -EINVAL;
 }
 
 /**