Re: [PATCH V4 05/18] iommu/ioasid: Redefine IOASID set and allocation APIs

[Jason Gunthorpe]

On Wed, May 05, 2021 at 01:04:46PM -0700, Jacob Pan wrote:
> Hi Jason,
> 
> On Wed, 5 May 2021 15:00:23 -0300, Jason Gunthorpe <jgg@xxxxxxxxxx> wrote:
> 
> > On Wed, May 05, 2021 at 10:22:59AM -0700, Jacob Pan wrote:
> > 
> > > Global and pluggable are for slightly separate reasons.
> > > - We need global PASID on VT-d in that we need to support shared
> > > workqueues (SWQ). E.g. One SWQ can be wrapped into two mdevs then
> > > assigned to two VMs. Each VM uses its private guest PASID to submit
> > > work but each guest PASID must be translated to a global (system-wide)
> > > host PASID to avoid conflict. Also, since PASID table storage is per
> > > PF, if two mdevs of the same PF are assigned to different VMs, the
> > > PASIDs must be unique.  
> > 
> > From a protocol perspective each RID has a unique PASID table, and
> > RIDs can have overlapping PASIDs.
> > 
> True, per RID or per PF as I was referring to.
> 
> > Since your SWQ is connected to a single RID the requirement that
> > PASIDs are unique to the RID ensures they are sufficiently unique.
> > 
> True, but one process can submit work to multiple mdevs from different
> RIDs/PFs. One process uses one PASID and PASID translation table is per VM.
> The same PASID is used for all the PASID tables of each RID.

If the model is "assign this PASID to this RID" then yes, there is a
big problem keeping everything straight that can only be solved with a
global table.

But if the model is "give me a PASID for this RID" then it isn't such
a problem.

Basically trying to enforce a uniform PASID for an IOASID across all
RIDs attached to it is not such a nice choice.

> > That is fine, but all this stuff should be inside the Intel vIOMMU
> > driver not made into a global resource of the entire iommu subsystem.
> > 
> Intel vIOMMU has to use a generic uAPI to allocate PASID so the generic
> code need to have this option. I guess you are saying we should also have a
> per RID allocation option in addition to global?

There always has to be a RID involvement for the PASID, for security,
this issue really boils down to where the PASID lives.

If you need the PASID attached to the IOASID then it has to be global
because the IOASID can be attached to any RID and must keep the same
PASID.

If the PASID is learned when the IOASID is attached to a RID then the
PASID is more flexible and isn't attached to the IOASID.

Honestly I'm a little leary to bake into a UAPI a specific HW choice
that Intel made here.

I would advise making the "attach a global PASID to this IOASID"
operation explicit and opt into for case that actually need it.

Which implies the API to the iommu driver should be more like:

  'assign an IOASID to this RID and return the PASID'
  'reserve a PASID from every RID'
  'assign an IOASID to this RID and use this specific PASID'

In all cases the scope of those operations are completely local to a
certain IOMMU driver - 'reserver a PASID from every RID' is really
every RID that driver can operate on.

So it is hard to see why the allocator should be a global resource and
not something that is part of the iommu driver exclusively.

Jason