Re: [PATCH v2 3/8] iommu/sva: Support reservation of global SVA PASIDs

[Jacob Pan <jacob.jun.pan@linux.intel.com>]

Hi Kevin,

On Tue, 28 Mar 2023 07:35:43 +0000, "Tian, Kevin" <kevin.tian@intel.com>
wrote:

> > From: Jacob Pan <jacob.jun.pan@linux.intel.com>
> > Sent: Tuesday, March 28, 2023 7:22 AM
> > 
> > +/**
> > + * @brief
> > + *	Reserve a PASID from the SVA global number space.
> > + *
> > + * @param min starting range, inclusive
> > + * @param max ending range, inclusive
> > + * @return The reserved PASID on success or IOMMU_PASID_INVALID on
> > failure.
> > + */
> > +ioasid_t iommu_sva_reserve_pasid(ioasid_t min, ioasid_t max)
> > +{
> > +	int ret;
> > +
> > +	if (!pasid_valid(min) || !pasid_valid(max) ||
> > +	    min == 0 || max < min)
> > +		return IOMMU_PASID_INVALID;
> > +
> > +	ret = ida_alloc_range(&iommu_global_pasid_ida, min, max,
> > GFP_KERNEL);
> > +	if (ret < 0)
> > +		return IOMMU_PASID_INVALID;
> > +
> > +	return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(iommu_sva_reserve_pasid);
> > +  
> 
> Look at this function. There is no single word about sva except
> in the function name.
> 
> sva is just one user of global pasids.
> 
> when a driver supports sva it has to always use global pasids even
> for non-sva usages like dma pasid.
> 
> but this doesn't mean that we should build the API around sva.
> 
> it's really about global pasids.
> 
> let's just call it clearly as iommu_alloc_global_pasid(min, max).
> 
> Then we can define a wrapper iommu_reserve_global_pasid(pasid)
> as iommu_alloc_global_pasid(pasid, pasid).
> 
> for PASID#0 driver calls iommu_reserve_global_pasid(0).
> 
> for dma pasid driver calls iommu_alloc_global_pasid() to get a random
> one instead of reserving pasid#1.
> 
> this would be future proof when the same driver starts to allocate
> more pasids for other usages e..g siov.
I don't have strong preference here. Jason and others?

For the DMA vs. SVA use cases, these APIs are used to carve out PASIDs from
the SVA space. Let it be the entire global space or a subset, we don't care.
We just don't want conflicts with SVA. e.g. if the SVA space shrank in the
future, this still works.

Thanks,

Jacob