Re: [PATCH 1/5] iommu: Enhance domain allocation code to take additional flags

[Vasant Hegde <vasant.hegde@amd.com>]

Hi Jacob,


On 8/29/2024 3:22 AM, Jacob Pan wrote:
> Hi Vasant,
> 
> On Sat, 24 Aug 2024 20:17:45 +0530
> Vasant Hegde <vasant.hegde@amd.com> wrote:
> 
>> Yi,
>>
>>
>> On 8/23/2024 2:28 PM, Yi Liu wrote:
>>> On 2024/8/22 20:44, Jason Gunthorpe wrote:  
>>>> On Thu, Aug 22, 2024 at 07:27:39PM +0800, Yi Liu wrote:  
>>>>> On 2024/8/22 00:31, Jason Gunthorpe wrote:
>>>>>  
>>>>>>> @@ -359,11 +359,17 @@ struct iommu_vfio_ioas {
>>>>>>>     *                                   enforced on device
>>>>>>> attachment
>>>>>>>     * @IOMMU_HWPT_FAULT_ID_VALID: The fault_id field of hwpt
>>>>>>> allocation data is
>>>>>>>     *                             valid.
>>>>>>> + * @IOMMUFD_HWPT_ALLOC_PASID: When the domain is used on a
>>>>>>> device, with no
>>>>>>> + *                            PASID, the device will support
>>>>>>> later attaching
>>>>>>> + *                            a PASID as well. Some HW
>>>>>>> requires a specific
>>>>>>> + *                            domain format on the device to
>>>>>>> allow PASID to
>>>>>>> + *                            work.  
>>>>>>
>>>>>> Maybe:
>>>>>>
>>>>>>    Requests a domain that can be used with PASID. The domain can
>>>>>> be attached to any PASID on the device. Any domain attached to
>>>>>> the non-PASID part of the device must also be flaged, otherwise
>>>>>> attaching a PASID will blocked.
>>>>>>
>>>>>> Yi will need to add a check that IOMMUFD_HWPT_ALLOC_PASID was
>>>>>> specified on the RID domain while processing attach on the PASID
>>>>>> domain.  
>>>>>
>>>>> this means if userspace wants to user pasid on a device, it
>>>>> should allocate hwpt with this flag, no matter the hwpt is going
>>>>> to be attached to RID or pasid. is it?  
>>>>  
>>>> Yes>>  
>>>>> I still have a confusion. Say on AMD, is it a legitimate
>>>>> configuration that the RID is attached to a domain that uses V1
>>>>> page table while pasid is attached to domains that use V2 page
>>>>> table? From architecture p.o.v., it seems allowed.  
>>>>
>>>> I think that is exactly what they have been saying their design
>>>> cannot do.  
>>>
>>> ok. I don't think Intel has this limitation. RID/PASID is free to
>>> use s1 or s2 format page table. So such enforcement is nop on Intel
>>> platform. 
>>>>
>>>> If you want to use PASID you must install a GCR3 table full of V2
>>>> tables. Doing this denies use of the V1 format for anything except
>>>> a NESTING_PARENT.  
>>>
>>> thanks for the explanation. So if pasid is to be used, amd iommu
>>> driver just programs the GCR3 tables and the request without pasid
>>> would use the v2 page table installed at pasid#0. It does not use
>>> the DTE->host_page_table at all. I remember the
>>> DTE->host_page_table uses v1 format.  
>>
>>
>> Right. Only V2 page table supports PASID. So when user space requests
>> hwpt with PASID AMD driver will allocate V2 page table with GCR3
>> setup and updated DTE. So it can handle attaching Device ID (uses
>> PASID#0) and PASID (as GCR3 table is already setup).
>>
>> Note that when we use NESTED page table it will have both v1 (L2 /
>> stage 2 level) and v2 page table (L1 / stage 1 level). Even in this
>> setup V2 page table inside guest can support PASID.
>>
> Sorry I am still confused. So V2 can support both PASID and no PASID
> (untagged DMA). For the nested case, if the guest wants to allocate
> a UNMANAGED domain, would you use V1? If this code in [3/5] below runs
> in the guest:

In nested scenario, Guest will always use V2 page table (and host will use V1
page table).


> 
> static struct iommu_domain *amd_iommu_domain_alloc(unsigned int type)
> 
> 	/*
> 	 * Force IOMMU v1 page table when allocating
> 	 * domain for pass-through devices.
> 	 */
> 	if (type == IOMMU_DOMAIN_UNMANAGED)
> 		pgtable = AMD_IOMMU_V1;
> 
> Hardware-wise, in nested case, can V1 be used in L1/S1 for DMA w/o
> PASID?

Actually no. L1 (Guest) will use V2 page table. I am also implementing
domain_alloc_paging()... I believe that should be sufficient. If something is
still missing we should address that as part of nested support.


-Vasant