Re: [RFC] iommu_ops->domain_alloc_paging() enhancement to support AMD IOMMU driver

[Jason Gunthorpe <jgg@ziepe.ca>]

On Fri, Jun 28, 2024 at 12:13:43PM +0530, Vasant Hegde wrote:
> Hi All,
> 
> We are working on adding domain_alloc_paging() support in AMD driver and came
> across below issue.
> 
> BACKGROUND:
> ============
> - AMD IOMMU HW has two different page tables : V1 (host page table) and V2
> (guest page table). Only V2 page table supports PASID and PRI features.
> 
> - With V2 page table we have an aliasing issue. Hence we added
> per-device-domain-id when domain is configured with v2 page table. See upstream
> commit 87a6f1f22c97 ("iommu/amd: Introduce per-device domain ID to fix potential
> TLB aliasing issue")

Yes, but IIRC this is a shortcut to developing a proper packing
algorithm to optimize the IOTLB. HW like this that has aliasing issues
needs some more complex SW support to get optimal usage.

ie you can share DIDs if devices have a logically equivilant GCR3
table. Optimizing this is a SW problem inside the driver and should
not leak out to API.

> With iommu_ops->domain_alloc_paging(dev) API AMD driver will chose best page
> table based on device capabilities (V2 for PASID capable device and V1 page
> table for rest of the devices).

Yes, this is correct.

> But we would like to continue enforcing V1 page table for UNMANAGED domain. As
> in terms of IOMMU caching, it performs better than V2 page table.

We are getting rid of UNMANAGED domains. And we are adding PASID
support to VFIO.

There is no reason VFIO should have a V1 domain by default and end up
with a non-working VFIO PASID API. That doesn't make any sense.

You need to actually explain when and why you need V1 page table
support in the VFIO context.

> Also while adding SVA in AMD driver Jason mentioned that we should support PASID
> with UNMANAGED domain. As I understand currently we don't have this feature in
> upstream but we would like to support it in future.

My patch series enables it on SMMUv3 and Intel VT-d already supports
it. Only AMD is missing the funtionality, which is why I keep asking
you to structure things properly so it gets it too :)

> Please let us know which one is preferred -OR- is there any other better way to
> handle this.

Neither is really going to work. VFIO will have to assume the user
will want to use the PASID API and will always request a PASID capable
domain anyhow.

We already have a path where the VFIO userspace can request a v1
domain by using the NESTING_PARENT flags during user domain
allocation.

If it is really important and logical we could also add a NO_PASID
flag to hint to the driver that userspace doesn't want to use PASID in
combination with this domain. That could also trigger v1.

But I don't see any option here that doesn't involve userspace itself
making a request and indicating it wants a degrated VFIO
functionality.

Jason