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

[Michal Swiatkowski <michal.swiatkowski@linux.intel.com>]

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

[...]