Re: [PATCH v4 1/6] iommu: Add a per domain PASID for DMA API

[Baolu Lu]

On 2022/5/31 18:12, Tian, Kevin wrote:
> > > > +++ b/include/linux/iommu.h
> > > > @@ -105,6 +105,8 @@ struct iommu_domain {
> > > >   	enum iommu_page_response_code (*iopf_handler)(struct
> > iommu_fault *fault,
> > > >   						      void *data);
> > > >   	void *fault_data;
> > > > +	ioasid_t pasid;		/* Used for DMA requests with PASID */
> > > > +	atomic_t pasid_users;
> > > These are poorly named, this is really the DMA API global PASID and
> > > shouldn't be used for other things.
> > >
> > >
> > >
> > > Perhaps I misunderstood, do you mind explaining more?
> > You still haven't really explained what this is for in this patch,
> > maybe it just needs a better commit message, or maybe something is
> > wrong.
> >
> > I keep saying the DMA API usage is not special, so why do we need to
> > create a new global pasid and refcount? Realistically this is only
> > going to be used by IDXD, why can't we just allocate a PASID and
> > return it to the driver every time a driver asks for DMA API on PASI
> > mode? Why does the core need to do anything special?
> Agree. I guess it was a mistake caused by treating ENQCMD as the
> only user although the actual semantics of the invented interfaces
> have already evolved to be quite general.
>
> This is very similar to what we have been discussing for iommufd.
> a PASID is just an additional routing info when attaching a device
> to an I/O address space (DMA API in this context) and by default
> it should be a per-device resource except when ENQCMD is
> explicitly opt in.
>
> Hence it's right time for us to develop common facility working
> for both this DMA API usage and iommufd, i.e.:
>
> for normal PASID attach to a domain, driver:
>
> 	allocates a local pasid from device local space;
> 	attaches the local pasid to a domain;
>
> for PASID attach in particular for ENQCMD, driver:
>
> 	allocates a global pasid in system-wide;
> 	attaches the global pasid to a domain;
> 	set the global pasid in PASID_MSR;
>
> In both cases the pasid is stored in the attach data instead of the
> domain.
>
> DMA API pasid is no special from above except it needs to allow
> one device attached to the same domain twice (one with RID
> and the other with RID+PASID).
>
> for iommufd those operations are initiated by userspace via
> iommufd uAPI.

My understanding is that device driver owns its PASID policy. If ENQCMD
is supported on the device, the PASIDs should be allocated through
ioasid_alloc(). Otherwise, the whole PASID pool is managed by the device
driver.

For kernel DMA w/ PASID, after the driver has a PASID for this purpose,
it can just set the default domain to the PASID on device. There's no
need for enable/disable() interfaces.

Best regards,
baolu