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

[Baolu Lu <baolu.lu@linux.intel.com>]

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