Re: [PATCH 2/2] iommu/ioasid: Remove custom IOASID allocator

[Jacob Pan]

Hi Jason,

On Mon, 13 Feb 2023 15:39:19 -0400, Jason Gunthorpe <jgg@xxxxxxxxxx> wrote:

> On Mon, Feb 13, 2023 at 10:34:55AM -0800, Jacob Pan wrote:
> > Hi Jean-Philippe,
> > 
> > On Mon, 13 Feb 2023 16:20:29 +0000, Jean-Philippe Brucker
> > <jean-philippe@xxxxxxxxxx> wrote:
> >   
> > > On Fri, Feb 10, 2023 at 03:02:06PM -0800, Jacob Pan wrote:  
> > > > Custom allocator feature was introduced to support VT-d's virtual
> > > > command, an enlightened interface designed for VMs to allocate
> > > > PASIDs from the host.
> > > > 
> > > > As we remove/withdraw the VT-d virtual command feature, the sole
> > > > user of custom allocator, we can safely remove the custom allocator
> > > > as well. Effectively, this will return IOASID core to the original
> > > > simple global namespace allocator.
> > > > 
> > > > Signed-off-by: Jacob Pan <jacob.jun.pan@xxxxxxxxxxxxxxx>    
> > > 
> > > You can also drop the spinlock.h include. With that:
> > >   
> > good catch, thanks  
> > > Reviewed-by: Jean-Philippe Brucker <jean-philippe@xxxxxxxxxx>
> > > 
> > > On a related note, it looks like 100b8a14a370 ("iommu/vt-d: Add pasid
> > > private data helpers") removed the last user of ioasid_set_data(). I
> > > guess that could be dropped too, unless you plan to still use it?
> > >   
> > You are right, will remove.
> > I was planning on the other way around which will convert VT-d's private
> > pasid data helpers to common ioasid code, but when I look closer the
> > private pasid xa is just holding a list of pasid/mm which could be per
> > iommu not global. Another cleanup I suppose.  
> 
> Please lets just delete this entire ioasid thing, it has no purpose
> anymore at all.
> 
> I did a sketch on how it do it here:
> 
> https://github.com/jgunthorpe/linux/commits/iommu_remove_ioasid
> 
> I wasn't very careful or elegant with the last patch, can you tidy it
> up and repost this as your v2?
> 
yes, will do.
> Your DMA API PASID thing will simply need one new API to alloc/free a
> PASID from the iommu_global_pasid_ida
It should satisfy what we need right now.
Just wondering if we were to do resource management of global PASIDs, say
with the new misc cgroup controller, do we plan to expand in iommu sva code?
If yes, do we keep DMA API PASID in a separate range/set?


Thanks,

Jacob