Re: [PATCH 3/4] iommu/sva: Support reservation of global PASIDs

[Jacob Pan]

Hi Jason,

On Mon, 6 Mar 2023 13:43:39 -0400, Jason Gunthorpe <jgg@xxxxxxxxxx> wrote:

> On Mon, Mar 06, 2023 at 09:44:08AM -0800, Jacob Pan wrote:
> > Hi Jason,
> > 
> > On Mon, 6 Mar 2023 09:01:32 -0400, Jason Gunthorpe <jgg@xxxxxxxxxx>
> > wrote: 
> > > On Fri, Mar 03, 2023 at 01:47:53PM -0800, Jacob Pan wrote:  
> > > > Hi Kevin,
> > > > 
> > > > On Thu, 2 Mar 2023 09:43:03 +0000, "Tian, Kevin"
> > > > <kevin.tian@xxxxxxxxx> wrote:
> > > >     
> > > > > > From: Jacob Pan <jacob.jun.pan@xxxxxxxxxxxxxxx>
> > > > > > Sent: Thursday, March 2, 2023 9:00 AM
> > > > > > 
> > > > > > Global PASID allocation is under IOMMU SVA code since it is the
> > > > > > primary use case.  However, some architecture such as VT-d,
> > > > > > global PASIDs are necessary for its internal use of DMA API
> > > > > > with PASID.      
> > > > > 
> > > > > No, global PASID is not a VT-d restriction. It's from ENQCMD/S
> > > > > hence a device requirement.    
> > > > I meant VT-d based platforms, it is kind of intertwined in that
> > > > ENQCMDS does not restrict RIDPASID!=DMA PASID, vt-d does. Without
> > > > this restriction, there wouldn't be a need for this patch. Let me
> > > > reword.    
> > > 
> > > No, Kevin is right, there is nothing about VT-d that needs global
> > > PASID values.
> > > 
> > > The driver should be managing RID2PASID itself to avoid conflicting
> > > with any in-use PASID, either by changing RID2PASID on demand or by
> > > setting it to a value that is not part of the PASID number space, eg
> > > we can make 0 entirely invalid, or the driver can reduce max_pasid of
> > > the devices it controls and use PASID_MAX.
> > >   
> > I see, thank you both. how about
> > "This patch provide an API for device drivers to request global PASIDs
> > as needed. The device drivers will then gain the flexibility of choosing
> > PASIDs not conflicting with anyone in-use."  
> 
> Stil no, this functionality should be clearly and unambiguously tied
> to ENQCMD:
> 
> Devices that rely on Intel ENQCMD have a single CPU register to store
> the current thread's PASID in. This necessarily makes the PASID a
> system-global value shared by all ENQCMD using devices.
> 
> This matches the current allocator being used for the SVA PASID so for
> now allow ENQCMD drivers to access this PASID allocator for other
> uses.
> 
ENQCMDS does not have the restriction of using a single CPU MSR to store
PASIDs, PASID is supplied to the instruction operand. Here we are adding
API for ENQCMDS. Should we explain this as well? i.e. due the unforgiving
nature of ENQCMD that requires global PASIDs, ENQCMDS has no choice but to
allocate from the same numberspace to avoid conflict.


Thanks,

Jacob