Re: [PATCH v5 08/12] iommufd: Enforce pasid compatible domain for PASID-capable device

[Yi Liu <yi.l.liu@intel.com>]

On 2024/12/12 13:51, Tian, Kevin wrote:
>> From: Liu, Yi L <yi.l.liu@intel.com>
>> Sent: Thursday, December 12, 2024 11:15 AM
>>
>> On 2024/12/11 16:46, Tian, Kevin wrote:
>>>> From: Liu, Yi L <yi.l.liu@intel.com>
>>>> Sent: Tuesday, December 10, 2024 11:15 AM
>>>>
>>>> On 2024/12/9 22:57, Jason Gunthorpe wrote:
>>>>>
>>>>> We want some reasonable compromise to encourage applications to use
>>>>> IOMMU_HWPT_ALLOC_PASID properly, but not build too much
>> complexity
>>>> to
>>>>> reject driver-specific behavior.
>>>>
>>>> I'm ok to do it in iommufd as long as it is only applicable to hwpt_paging.
>>>> Otherwise, attaching nested domain to pasid would be failed according to
>>>> the aforementioned enforcement.
>>>>
>>>
>>> IMHO we may want to have a general enforcement in IOMMUFD that
>>> any domain (paging or nested) must have ALLOC_PASID set to be
>>> used in pasid-oriented operations.
>>>
>>> drivers can have more restrictions, e.g. for arm/amd allocating a nested
>>> domain with that bit set will fail at the beginning.
>>
>> ARM/AMD should allow allocating nested domain with this flag. Otherwise,
>> it does not suit the ALLOC_PASID definition. It requires both the PASID
>> path and non-PASID path to use pasid-compat domain.
> 
> hmm the main point you raised at the beginning was that ARM/AMD
> doesn't support the flag on nested domain, given the CD/PASID table
> is a per-RID thing.

yes.

> 
>>
>> So maybe we should not stick with the initial purpose of ALLOC_PASID flag.
>> It actually means selecting V2 page table. But the definition of it allows
>> us to consider the nested domains to be pasid-compat as Intel allows it.
>> And, a sane userspace running on ARM/AMD will never attach nested
>> domain
>> to PASIDs. Even it does, the ARM SMMU and AMD iommu driver can fail such
>> attempts. In this way, we can enforce the ALLOC_PASID flag for any domains
>> used by PASID-capable devices in iommufd. This suits the existing
>> ALLOC_PASID definition as well.
> 
> Isn't it what I was suggesting? IOMMUFD just enforces that flag must
> be set if a domain will be attached to PASID, and drivers will do
> additional restrictions e.g. AMD/ARM allows the flag only on paging
> domain while VT-d allows it for any type.

A slight difference. :) I think we also need to enforce it for the
non-PASID path. If not, the PASID path cannot work according to the
ALLOC_PASID definition. But we are on the same page about the additional
restrictions in ARM/AMD drivers about the nested domain used on PASIDs.
This is supposed to be done in attach phase instead of domain allocation
time.

-- 
Regards,
Yi Liu