Re: [PATCH v6 0/6] iommufd: Add nesting infrastructure (part 2/2)

[Jason Gunthorpe]

On Tue, Dec 12, 2023 at 12:35:26AM +0700, Suthikulpanit, Suravee wrote:
> 
> 
> On 12/9/2023 8:47 AM, Jason Gunthorpe wrote:
> > On Fri, Nov 17, 2023 at 05:07:11AM -0800, Yi Liu wrote:
> > 
> > > Take Intel VT-d as an example, the stage-1 translation table is I/O page
> > > table. As the below diagram shows, guest I/O page table pointer in GPA
> > > (guest physical address) is passed to host and be used to perform the stage-1
> > > address translation. Along with it, modifications to present mappings in the
> > > guest I/O page table should be followed with an IOTLB invalidation.
> > 
> > I've been looking at what the three HW's need for invalidation, it is
> > a bit messy.. Here is my thinking. Please let me know if I got it right
> > 
> > What is the starting point of the guest memory walks:
> >   Intel: Single Scalable Mode PASID table entry indexed by a RID & PASID
> >   AMD: GCR3 table (a table of PASIDs) indexed by RID
> 
> GCR3 table is indexed by PASID.
> Device Table (DTE) is indexted by DeviceID (RID)

Yes, this is what I was trying to say


> > Will ATC be forwarded or synthesized:
> >   Intel: The (vDomain-ID,PASID) is a unique nesting domain so
> >          the hypervisor knows exactly which RIDs this nesting domain is
> > 	linked to and can generate an ATC invalidation. Plan is to
> > 	supress/discard the ATC invalidations from the VM and generate
> > 	them in the hypervisor.
> >   AMD: (vDomain-ID,PASID) is ambiguous, it can refer to multiple GCR3
> >        tables. We know which maximal set of RIDs it represents, but not
> >        the actual set. I expect AMD will forward the ATC invalidation
> >        to avoid over invalidation.
> 
> Not sure I understand your description here.
> 
> For the AMD IOMMU INVALIDE_IOMMU_PAGES (i.e. invalidate the IOMMU TLB), the
> hypervisor needs to map gDomainId->hDomainId and issue the command on behalf
> of the VM along with the PASID and GVA (or GVA range) provided by the guest.

Yes, that is the "forwarding" approach. Contrast this to the Intel
approach where the ATC is synthesized by the kernel emulating the
INVALIDE_IOMMU_PAGES

> > To make this work the iommu needs to be programmed with:
> >   AMD: A vDomain-ID -> pDomain-ID table
> >        A vRID -> pRID table
> >        This is all bound to some "virtual function"
> 
> By "virtual function", I assume you are referring to the AMD vIOMMU instance
> in the guest?

Yes, but it is not in the guest, it has to be some concrete iommufd
object.

> Something like IOMMUFD_OBJ_VIOMMU? Then operation would include something
> like:
>   * Init
>   * Destroy
>   * ...

Yes, something like that. It needs to be able to work for ARM vCMDQ
stuff too. I don't know what the name should be. Maybe viommu is OK
for now.

- Alloc viommu (against a single iommu instance?)
- Assign a virtual ID to an iommufd device within the same instance
- Setup a submission and completion queue in userspace memory
- mmap the doorbell page (both need this?)
- Route back completion interrupts via eventfd

When you get here you and Nicolin should work out something along
those lines that works for both

But I'd like to keep things in steps, so if we can get info, nesting
parent, nesting domain and SW IOTLB and ATC invalidation as the first
(two?) steps that would be great

> > Thus next steps:
> >   - Respin this and lets focus on Intel only (this will be tough for
> >     the holidays, but if it is available I will try)
> >   - Get an ARM patch that just does IOTLB invalidation and add it to my
> >     part 3
> >   - Start working on IOMMUFD_DEV_INVALIDATE along with an ARM
> >     implementation of it
> >   - Reorganize the AMD RFC broadly along these lines and lets see it
> >     freshened up in the next months as well. I would like to see the
> >     AMD support structured to implement the SW paths in first steps and
> >     later add in the "virtual function" acceleration stuff. The latter
> >     is going to be complex.
> 
> Working on refining the part 1 to add HW info reporting and nested
> translation (minus the invalidation stuff). Should be sending out soon.

Nice!

Jason