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

  Nested translation is a hardware feature that is supported by many modern
IOMMU hardwares. It has two stages (stage-1, stage-2) address translation
to get access to the physical address. stage-1 translation table is owned
by userspace (e.g. by a guest OS), while stage-2 is owned by kernel. Changes
to stage-1 translation table should be followed by an IOTLB invalidation.

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.

    .-------------.  .---------------------------.
    |   vIOMMU    |  | Guest I/O page table      |
    |             |  '---------------------------'
    .----------------/
    | PASID Entry |--- PASID cache flush --+
    '-------------'                        |
    |             |                        V
    |             |           I/O page table pointer in GPA
    '-------------'
Guest
------| Shadow |---------------------------|--------
      v        v                           v
Host
    .-------------.  .------------------------.
    |   pIOMMU    |  |  FS for GIOVA->GPA     |
    |             |  '------------------------'
    .----------------/  |
    | PASID Entry |     V (Nested xlate)
    '----------------\.----------------------------------.
    |             |   | SS for GPA->HPA, unmanaged domain|
    |             |   '----------------------------------'
    '-------------'
Where:
 - FS = First stage page tables
 - SS = Second stage page tables
<Intel VT-d Nested translation>

This series adds the cache invalidation path for the userspace to invalidate
cache after modifying the stage-1 page table. This is based on the first part
of nesting [1]

Complete code can be found in [2], QEMU could can be found in [3].

At last, this is a team work together with Nicolin Chen, Lu Baolu. Thanks
them for the help. ^_^. Look forward to your feedbacks.

[1] https://lore.kernel.org/linux-iommu/20231026044216.64964-1-yi.l.liu@intel.com/ - merged
[2] https://github.com/yiliu1765/iommufd/tree/iommufd_nesting
[3] https://github.com/yiliu1765/qemu/tree/zhenzhong/wip/iommufd_nesting_rfcv1

Change log:

v6:
 - No much change, just rebase on top of 6.7-rc1 as part 1/2 is merged

v5: https://lore.kernel.org/linux-iommu/20231020092426.13907-1-yi.l.liu@intel.com/#t
 - Split the iommufd nesting series into two parts of alloc_user and
   invalidation (Jason)
 - Split IOMMUFD_OBJ_HW_PAGETABLE to IOMMUFD_OBJ_HWPT_PAGING/_NESTED, and
   do the same with the structures/alloc()/abort()/destroy(). Reworked the
   selftest accordingly too. (Jason)
 - Move hwpt/data_type into struct iommu_user_data from standalone op
   arguments. (Jason)
 - Rename hwpt_type to be data_type, the HWPT_TYPE to be HWPT_ALLOC_DATA,
   _TYPE_DEFAULT to be _ALLOC_DATA_NONE (Jason, Kevin)
 - Rename iommu_copy_user_data() to iommu_copy_struct_from_user() (Kevin)
 - Add macro to the iommu_copy_struct_from_user() to calculate min_size
   (Jason)
 - Fix two bugs spotted by ZhaoYan

v4: https://lore.kernel.org/linux-iommu/20230921075138.124099-1-yi.l.liu@intel.com/
 - Separate HWPT alloc/destroy/abort functions between user-managed HWPTs
   and kernel-managed HWPTs
 - Rework invalidate uAPI to be a multi-request array-based design
 - Add a struct iommu_user_data_array and a helper for driver to sanitize
   and copy the entry data from user space invalidation array
 - Add a patch fixing TEST_LENGTH() in selftest program
 - Drop IOMMU_RESV_IOVA_RANGES patches
 - Update kdoc and inline comments
 - Drop the code to add IOMMU_RESV_SW_MSI to kernel-managed HWPT in nested translation,
   this does not change the rule that resv regions should only be added to the
   kernel-managed HWPT. The IOMMU_RESV_SW_MSI stuff will be added in later series
   as it is needed only by SMMU so far.

v3: https://lore.kernel.org/linux-iommu/20230724110406.107212-1-yi.l.liu@intel.com/
 - Add new uAPI things in alphabetical order
 - Pass in "enum iommu_hwpt_type hwpt_type" to op->domain_alloc_user for
   sanity, replacing the previous op->domain_alloc_user_data_len solution
 - Return ERR_PTR from domain_alloc_user instead of NULL
 - Only add IOMMU_RESV_SW_MSI to kernel-managed HWPT in nested translation (Kevin)
 - Add IOMMU_RESV_IOVA_RANGES to report resv iova ranges to userspace hence
   userspace is able to exclude the ranges in the stage-1 HWPT (e.g. guest I/O
   page table). (Kevin)
 - Add selftest coverage for the new IOMMU_RESV_IOVA_RANGES ioctl
 - Minor changes per Kevin's inputs

v2: https://lore.kernel.org/linux-iommu/20230511143844.22693-1-yi.l.liu@intel.com/
 - Add union iommu_domain_user_data to include all user data structures to avoid
   passing void * in kernel APIs.
 - Add iommu op to return user data length for user domain allocation
 - Rename struct iommu_hwpt_alloc::data_type to be hwpt_type
 - Store the invalidation data length in iommu_domain_ops::cache_invalidate_user_data_len
 - Convert cache_invalidate_user op to be int instead of void
 - Remove @data_type in struct iommu_hwpt_invalidate
 - Remove out_hwpt_type_bitmap in struct iommu_hw_info hence drop patch 08 of v1

v1: https://lore.kernel.org/linux-iommu/20230309080910.607396-1-yi.l.liu@intel.com/

Thanks,
	Yi Liu

Lu Baolu (1):
  iommu: Add cache_invalidate_user op

Nicolin Chen (4):
  iommu: Add iommu_copy_struct_from_user_array helper
  iommufd/selftest: Add mock_domain_cache_invalidate_user support
  iommufd/selftest: Add IOMMU_TEST_OP_MD_CHECK_IOTLB test op
  iommufd/selftest: Add coverage for IOMMU_HWPT_INVALIDATE ioctl

Yi Liu (1):
  iommufd: Add IOMMU_HWPT_INVALIDATE

 drivers/iommu/iommufd/hw_pagetable.c          | 35 ++++++++
 drivers/iommu/iommufd/iommufd_private.h       |  9 ++
 drivers/iommu/iommufd/iommufd_test.h          | 22 +++++
 drivers/iommu/iommufd/main.c                  |  3 +
 drivers/iommu/iommufd/selftest.c              | 69 +++++++++++++++
 include/linux/iommu.h                         | 84 +++++++++++++++++++
 include/uapi/linux/iommufd.h                  | 35 ++++++++
 tools/testing/selftests/iommu/iommufd.c       | 75 +++++++++++++++++
 tools/testing/selftests/iommu/iommufd_utils.h | 63 ++++++++++++++
 9 files changed, 395 insertions(+)
  

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
 ARM: CD table (a table of PASIDs) indexed by RID

What key does the physical HW use for invalidation:
 Intel: Domain-ID (stored in hypervisor, per PASID), PASID
 AMD: Domain-ID (stored in hypervisor, per RID), PASID
 ARM: VMID (stored in hypervisor, per RID), ASID (stored in guest)

Why key does the VM use for invalidation:
 Intel: vDomain-ID (per PASID), PASID
 AMD: vDomain-ID (per RID), PASID
 ARM: ASID

What is in a Nested domain:
 Intel: A single IO page table refereed to by a PASID entry
        Each vDomain-ID,PASID allocates a unique nesting domain
 AMD: A GCR3 table pointer
      Nesting domains are created for every unique GCR3 pointer.
      vDomain-ID can possibly refer to multiple Nesting domains :(
 ARM: A CD table pointer
      Nesting domains are created for every unique CD table top pointer.

How does the hypervisor compute it's cache tag:
 Intel: Each time a nesting domain is attached to a (RID,PASID) the
        hypervisor driver will try to find a (DID,PASID) that is
	already used by this domain, or allocate a new DID.
 AMD: When creating the Nesting Domain the vDomain-ID should be passed
      in. The hypervisor driver will allocate a unique pDomain-ID for
      each vDomain-ID (hand wave). Several Nesting Domains will share
      the same p/vDomain-ID.
 ARM: The VMID is uniquely assigned to the Nesting Parent when it
      is allocated, in some configurations it has to be shared with
      KVM's VMID so allocating the Nesting Parent will require a KVM FD.

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.
 ARM: ASID is ambiguous. We have no idea which Nesting Domain/CD table
      the ASID is contained in. ARM must forward the ATC invalidation
      from the guest.

What iommufd object should receive the IOTLB invalidation command list:
 Intel: The Nesting domain. The command list has to be broken up per
        (vDomain-ID,PASID) and that batch delivered to the single
	nesting domain. Kernel ignores vDomain-ID/PASID and just
	invalidates whatever the nesting domain is actually attached to
 AMD: Any Nesting Domain in the vDomain-ID group. The command list has
      to be broken up per (vDomain-ID). Kernel replaces
      vDomain-ID with pDomain-ID from the nesting domain and executes
      the invalidation.
 ARM: The Nesting Parent domain. Kernel forces the VMID from the
      Nesting Parent and executes the invalidation.

In all cases the VM issues an ATC invalidation with (vRID, PASID) as
the tag. The VMM must translate vRID -> dev_id -> pRID

For a pure SW flow the vRID can be mapped to the dev_id and the ATC
invalidation delivered to the device object (eg IOMMUFD_DEV_INVALIDATE)

Finally, we have the HW driven invalidation DMA queues that can be
directly assigned to the guest. AMD and SMMUv3+vCMDQ support this. In
this case the HW is directly processing invalidation commands without
a hypervisor trap.

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"
 ARM: A vRID -> pRID table
      The vCMDQ is bound to a VM_ID, so to the Nesting Parent

For AMD, as above, I suggest the vDomain-ID be passed when creating
the nesting domain.

The AMD "virtual function".. It is probably best to create a new iommufd
object for this and it can be passed in to a few places

The vRID->pRID table should be some mostly common
IOMMUFD_DEV_ASSIGN_VIRTUAL_ID. AMD will need to pass in the virtual
function ID and ARM will need to pass in the Nesting Parent ID.

For the HW path some function will create the command queue and
DMA/mmap it. Taking in the virtual function/nesting parent as the
handle to associate it with.

For a SW path:
 AMD: All invalidations can be delivered to the virtual function
      and the kernel can use the vDomainID/vRID tables to translate
      them fully
 ARM: All invalidations can be delivered to the nesting parent

In many ways the nesting parent/virtual function are very similar
things. Perhaps ARM should also create a virtual function object which
is just welded to the nesting parent for API consistency.

So.. In short.. Invalidation is a PITA. The idea is the same but
annoying little details interfere with actually having a compltely
common API here. IMHO the uAPI in this series is fine. It will support
Intel invalidation and non-ATC invalidation on AMD/ARM. It should be
setup to allow that the target domain object can be any HWPT.

ARM will be able to do IOTLB invalidation using this API.

IOMMUFD_DEV_INVALIDATE should be introduced with the same design as
HWPT invalidate. This would be used for AMD/ARM's ATC invalidation
(and just force the stream ID, userspace must direct the vRID to the
correct dev_id).

Then in yet another series we can tackle the entire "virtual function"
vRID/pRID translation stuff when the mmapable queue thing is
introduced.

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.
 
Jason
  

> From: Jason Gunthorpe <jgg@nvidia.com>
> Sent: Saturday, December 9, 2023 9:47 AM
> 
> What is in a Nested domain:
>  Intel: A single IO page table refereed to by a PASID entry
>         Each vDomain-ID,PASID allocates a unique nesting domain
>  AMD: A GCR3 table pointer
>       Nesting domains are created for every unique GCR3 pointer.
>       vDomain-ID can possibly refer to multiple Nesting domains :(
>  ARM: A CD table pointer
>       Nesting domains are created for every unique CD table top pointer.

this AMD/ARM difference is not very clear to me.

How could a vDomain-ID refer to multiple GCR3 pointers? Wouldn't it
lead to cache tag conflict when a same PASID entry in multiple GCR3 tables
points to different I/O page tables?
  

On 2023/12/9 09:47, 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
>   ARM: CD table (a table of PASIDs) indexed by RID
> 
> What key does the physical HW use for invalidation:
>   Intel: Domain-ID (stored in hypervisor, per PASID), PASID
>   AMD: Domain-ID (stored in hypervisor, per RID), PASID
>   ARM: VMID (stored in hypervisor, per RID), ASID (stored in guest)
> 
> Why key does the VM use for invalidation:
>   Intel: vDomain-ID (per PASID), PASID
>   AMD: vDomain-ID (per RID), PASID
>   ARM: ASID
> 
> What is in a Nested domain:
>   Intel: A single IO page table refereed to by a PASID entry
>          Each vDomain-ID,PASID allocates a unique nesting domain
>   AMD: A GCR3 table pointer
>        Nesting domains are created for every unique GCR3 pointer.
>        vDomain-ID can possibly refer to multiple Nesting domains :(

Per section '2.2.6.3 Guest CR3 Table' in below spec, DTE has domainID,
and it points to a guest CR3 Table (it should be a set of guest CRs3)
which is indexed by PASID. So it looks like the PASID is per-DommainID.
HW should use domainID+PASID to tag the cache, otherwise there would be
cache conflict...

https://www.amd.com/content/dam/amd/en/documents/processor-tech-docs/specifications/48882_IOMMU.pdf

>   ARM: A CD table pointer
>        Nesting domains are created for every unique CD table top pointer.
> 
> How does the hypervisor compute it's cache tag:
>   Intel: Each time a nesting domain is attached to a (RID,PASID) the
>          hypervisor driver will try to find a (DID,PASID) that is
> 	already used by this domain, or allocate a new DID.
>   AMD: When creating the Nesting Domain the vDomain-ID should be passed
>        in. The hypervisor driver will allocate a unique pDomain-ID for
>        each vDomain-ID (hand wave). Several Nesting Domains will share
>        the same p/vDomain-ID.
>   ARM: The VMID is uniquely assigned to the Nesting Parent when it
>        is allocated, in some configurations it has to be shared with
>        KVM's VMID so allocating the Nesting Parent will require a KVM FD.
> 
> 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.
>   ARM: ASID is ambiguous. We have no idea which Nesting Domain/CD table
>        the ASID is contained in. ARM must forward the ATC invalidation
>        from the guest.
> 
> What iommufd object should receive the IOTLB invalidation command list:
>   Intel: The Nesting domain. The command list has to be broken up per
>          (vDomain-ID,PASID) and that batch delivered to the single
> 	nesting domain. Kernel ignores vDomain-ID/PASID and just
> 	invalidates whatever the nesting domain is actually attached to

this is what we are doing in current series[1]. is it? Guest needs to
issue invalidation request with vDomain-ID and PASID (if it is enabled),
and affected pages for sure. But in hypervisor side, use vDomainID and
PASID info to figure out the target HWPT, then invoke a cache invalidation
on the HWPT with the invalidation range is enough. Kernel can figure out
what device/pasid this HWPT has been attached and invalidate the caches.

[1] 
https://lore.kernel.org/linux-iommu/20231117131816.24359-1-yi.l.liu@intel.com/

>   AMD: Any Nesting Domain in the vDomain-ID group. The command list has
>        to be broken up per (vDomain-ID). Kernel replaces
>        vDomain-ID with pDomain-ID from the nesting domain and executes
>        the invalidation.
>   ARM: The Nesting Parent domain. Kernel forces the VMID from the
>        Nesting Parent and executes the invalidation.
> 
> In all cases the VM issues an ATC invalidation with (vRID, PASID) as
> the tag. The VMM must translate vRID -> dev_id -> pRID
> 
> For a pure SW flow the vRID can be mapped to the dev_id and the ATC
> invalidation delivered to the device object (eg IOMMUFD_DEV_INVALIDATE)
> 
> Finally, we have the HW driven invalidation DMA queues that can be
> directly assigned to the guest. AMD and SMMUv3+vCMDQ support this. In
> this case the HW is directly processing invalidation commands without
> a hypervisor trap.
> 
> 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"
>   ARM: A vRID -> pRID table
>        The vCMDQ is bound to a VM_ID, so to the Nesting Parent
> 
> For AMD, as above, I suggest the vDomain-ID be passed when creating
> the nesting domain.
> 
> The AMD "virtual function".. It is probably best to create a new iommufd
> object for this and it can be passed in to a few places
> 
> The vRID->pRID table should be some mostly common
> IOMMUFD_DEV_ASSIGN_VIRTUAL_ID. AMD will need to pass in the virtual
> function ID and ARM will need to pass in the Nesting Parent ID.
> 
> For the HW path some function will create the command queue and
> DMA/mmap it. Taking in the virtual function/nesting parent as the
> handle to associate it with.
> 
> For a SW path:
>   AMD: All invalidations can be delivered to the virtual function
>        and the kernel can use the vDomainID/vRID tables to translate
>        them fully
>   ARM: All invalidations can be delivered to the nesting parent
> 
> In many ways the nesting parent/virtual function are very similar
> things. Perhaps ARM should also create a virtual function object which
> is just welded to the nesting parent for API consistency.
> 
> So.. In short.. Invalidation is a PITA. The idea is the same but
> annoying little details interfere with actually having a compltely
> common API here. IMHO the uAPI in this series is fine. It will support
> Intel invalidation and non-ATC invalidation on AMD/ARM. It should be
> setup to allow that the target domain object can be any HWPT.

This HWPT is still nested domain. Is it? But it can represent a guest I/O
page table (VT-d), guest CD table (ARM), guest CR3 Table (AMD, it seems to
be a set of guest CR3 table pointers). May ARM and AMD guys keep me honest
here.

The Intel guest I/O page table case may be the simplest as userspace only
needs to provide the HWPT ID and the affected ranges for invalidating. As
mentioned above, kernel will find out the attached device/pasid and
invalidating cache with the device/pasid. For ARM and AMD case, extra
information is needed. Am I getting you correct?

> 
> ARM will be able to do IOTLB invalidation using this API.
> 
> IOMMUFD_DEV_INVALIDATE should be introduced with the same design as
> HWPT invalidate. This would be used for AMD/ARM's ATC invalidation
> (and just force the stream ID, userspace must direct the vRID to the
> correct dev_id).
> 
> Then in yet another series we can tackle the entire "virtual function"
> vRID/pRID translation stuff when the mmapable queue thing is
> introduced.
> 
> 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)

I've respinned the iommufd cache invalidation part with the change to
report error_code/error_data per invalidation entry. yet still busy on
making Intel VTd part to report the error_code. Besides, I didn't see
other respin needed for Intel VT-d invalidation. If I missed thing, please
do let me know.:)

>   - 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.
>   
> Jason
  

On 2023/12/11 10:29, Tian, Kevin wrote:
>> From: Jason Gunthorpe <jgg@nvidia.com>
>> Sent: Saturday, December 9, 2023 9:47 AM
>>
>> What is in a Nested domain:
>>   Intel: A single IO page table refereed to by a PASID entry
>>          Each vDomain-ID,PASID allocates a unique nesting domain
>>   AMD: A GCR3 table pointer
>>        Nesting domains are created for every unique GCR3 pointer.
>>        vDomain-ID can possibly refer to multiple Nesting domains :(
>>   ARM: A CD table pointer
>>        Nesting domains are created for every unique CD table top pointer.
> 
> this AMD/ARM difference is not very clear to me.
> 
> How could a vDomain-ID refer to multiple GCR3 pointers? Wouldn't it
> lead to cache tag conflict when a same PASID entry in multiple GCR3 tables
> points to different I/O page tables?

Perhaps due to only one DomainID in the DTE table indexed by BDF? Actually,
the vDomainID will not be used to tag cache, the host DomainId would be
used instead. @Jason?
  

On Mon, Dec 11, 2023 at 08:36:46PM +0800, Yi Liu wrote:
> On 2023/12/11 10:29, Tian, Kevin wrote:
> > > From: Jason Gunthorpe <jgg@nvidia.com>
> > > Sent: Saturday, December 9, 2023 9:47 AM
> > > 
> > > What is in a Nested domain:
> > >   Intel: A single IO page table refereed to by a PASID entry
> > >          Each vDomain-ID,PASID allocates a unique nesting domain
> > >   AMD: A GCR3 table pointer
> > >        Nesting domains are created for every unique GCR3 pointer.
> > >        vDomain-ID can possibly refer to multiple Nesting domains :(
> > >   ARM: A CD table pointer
> > >        Nesting domains are created for every unique CD table top pointer.
> > 
> > this AMD/ARM difference is not very clear to me.
> > 
> > How could a vDomain-ID refer to multiple GCR3 pointers? Wouldn't it
> > lead to cache tag conflict when a same PASID entry in multiple GCR3 tables
> > points to different I/O page tables?
> 
> Perhaps due to only one DomainID in the DTE table indexed by BDF? Actually,
> the vDomainID will not be used to tag cache, the host DomainId would be
> used instead. @Jason?

The DomainID comes from the DTE table which is indexed by the RID, and
the DTE entry points to the GCR3 table. So the VM certainly can setup
a DTE table with multiple entires having the same vDomainID but
pointing to different GCR3's. So the VMM has to do *something* with
this.

Most likely this is not a useful thing to do. However what should the
VMM do when it sees this? Block a random DTE or push the duplication
down to real HW would be my options. I'd probably try to do the latter
just on the basis of better emulation.

Jason
  

On Mon, Dec 11, 2023 at 08:35:09PM +0800, Yi Liu wrote:

> > What iommufd object should receive the IOTLB invalidation command list:
> >   Intel: The Nesting domain. The command list has to be broken up per
> >          (vDomain-ID,PASID) and that batch delivered to the single
> > 	nesting domain. Kernel ignores vDomain-ID/PASID and just
> > 	invalidates whatever the nesting domain is actually attached to
> 
> this is what we are doing in current series[1]. is it?

I think so

> > So.. In short.. Invalidation is a PITA. The idea is the same but
> > annoying little details interfere with actually having a compltely
> > common API here. IMHO the uAPI in this series is fine. It will support
> > Intel invalidation and non-ATC invalidation on AMD/ARM. It should be
> > setup to allow that the target domain object can be any HWPT.
> 
> This HWPT is still nested domain. Is it? But it can represent a guest I/O
> page table (VT-d), guest CD table (ARM), guest CR3 Table (AMD, it seems to
> be a set of guest CR3 table pointers). May ARM and AMD guys keep me honest
> here.

I was thinking ARM would not want to use a nested domain because
really the invalidation is global to the entire nesting parent.

But, there is an issue with that - the nesting parent could be
attached to multiple iommu instances but we only want to invalidate a
single instance. 

However, specifying the nesting domain instead, and restricting the
nesting domain to be single-instance would provide the kernel enough
information without providing weird new APIs. So maybe it is best to
just make everything use the nesting domain even if it is somewhat
semantically weird on arm.

> The Intel guest I/O page table case may be the simplest as userspace only
> needs to provide the HWPT ID and the affected ranges for invalidating. As
> mentioned above, kernel will find out the attached device/pasid and
> invalidating cache with the device/pasid. For ARM and AMD case, extra
> information is needed. Am I getting you correct?

Yes
 
> > 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)
> 
> I've respinned the iommufd cache invalidation part with the change to
> report error_code/error_data per invalidation entry. yet still busy on
> making Intel VTd part to report the error_code. Besides, I didn't see
> other respin needed for Intel VT-d invalidation. If I missed thing, please
> do let me know.:)

Lets see

Jason
  

On 12/11/2023 8:05 PM, Jason Gunthorpe wrote:
> On Mon, Dec 11, 2023 at 08:36:46PM +0800, Yi Liu wrote:
>> On 2023/12/11 10:29, Tian, Kevin wrote:
>>>> From: Jason Gunthorpe <jgg@nvidia.com>
>>>> Sent: Saturday, December 9, 2023 9:47 AM
>>>>
>>>> What is in a Nested domain:
>>>>    Intel: A single IO page table refereed to by a PASID entry
>>>>           Each vDomain-ID,PASID allocates a unique nesting domain
>>>>    AMD: A GCR3 table pointer
>>>>         Nesting domains are created for every unique GCR3 pointer.
>>>>         vDomain-ID can possibly refer to multiple Nesting domains :(
>>>>    ARM: A CD table pointer
>>>>         Nesting domains are created for every unique CD table top pointer.
>>>
>>> this AMD/ARM difference is not very clear to me.
>>>
>>> How could a vDomain-ID refer to multiple GCR3 pointers? Wouldn't it
>>> lead to cache tag conflict when a same PASID entry in multiple GCR3 tables
>>> points to different I/O page tables?
>>
>> Perhaps due to only one DomainID in the DTE table indexed by BDF? Actually,
>> the vDomainID will not be used to tag cache, the host DomainId would be
>> used instead. @Jason?
> 
> The DomainID comes from the DTE table which is indexed by the RID, and
> the DTE entry points to the GCR3 table. So the VM certainly can setup
> a DTE table with multiple entires having the same vDomainID but
> pointing to different GCR3's. So the VMM has to do *something* with
> this.
> 
> Most likely this is not a useful thing to do. However what should the
> VMM do when it sees this? Block a random DTE or push the duplication
> down to real HW would be my options. I'd probably try to do the latter
> just on the basis of better emulation.
> 
> Jason

For AMD, the hardware uses host DomainID (hDomainId) and PASID to tag 
the IOMMU TLB.

The VM can setup vDomainID independently from device (RID) and 
hDomainID. The vDomainId->hDomainId mapping would be managed by the host 
IOMMU driver (since this is also needed by the HW when enabling the 
HW-vIOMMU support a.k.a virtual function).

Currently, the AMD IOMMU driver allocates a DomainId per IOMMU group.
One issue with this is when we have nested translation where we could 
end up with multiple devices (RIDs) sharing same PASID and the same 
hDomainID.

For example:

   - Host view
     Device1 (RID 1) w/ hDomainId 1
     Device2 (RID 2) w/ hDomainId 1
   - Guest view
     Pass-through Device1 (vRID 3) w/ vDomainID A + PASID 0
     Pass-through Device2 (vRID 4) w/ vDomainID B + PASID 0

We should be able to workaround this by changing the way we assign 
hDomainId to be per-device for VFIO pass-through devices although 
sharing the same v1 (stage-2) page table. This would look like.

   - Host view
     Device1 (RID 1) w/ hDomainId 1
     Device2 (RID 2) w/ hDomainId 2
   - Guest view
     Pass-through Device1 (vRID 3) w/ vDomainID A + PASID 0
     Pass-through Device2 (vRID 4) w/ vDomainID B + PASID 0

This should avoid the IOMMU TLB conflict. However, the invalidation 
would need to be done for both DomainId 1 and 2 when updating the v1 
(stage-2) page table.

Thanks,
Suravee
  

On Mon, Dec 11, 2023 at 10:34:09PM +0700, Suthikulpanit, Suravee wrote:

> Currently, the AMD IOMMU driver allocates a DomainId per IOMMU group.
> One issue with this is when we have nested translation where we could end up
> with multiple devices (RIDs) sharing same PASID and the same hDomainID.

Which means you also create multiple GCR3 tables since those are
(soon) per-device and we end up with the situation I described for a
functional legitimate reason :( It is just wasting memory by
duplicating GCR3 tables.
 
> For example:
> 
>   - Host view
>     Device1 (RID 1) w/ hDomainId 1
>     Device2 (RID 2) w/ hDomainId 1

So.. Groups are another ugly mess that we may have to do something
more robust about.

The group infrastructure assumes that all devices in the group have
the same translation. This is not how the VM communicates, each member
of the group gets to have its own DTE and there are legitimate cases
where the DTEs will be different (even if just temporarily)

How to mesh this is not yet solved (most likely we need to allow group
members to have temporarily different translation). But in the long
run the group should definately not be providing the cache tag, the
driver has to be smarter than this.

I think we talked about this before.. For the AMD driver the v1 page
table should store the domainid in the iommu_domain and that value
should be used everywhere

For modes with a GCR3 table the best you can do is to de-duplicate the
GCR3 tables and assign identical GCR3 tables to identical domain ids.
Ie all devices in a group will eventually share GCR3 tables so they
can converge on the same domain id.

>   - Guest view
>     Pass-through Device1 (vRID 3) w/ vDomainID A + PASID 0
>     Pass-through Device2 (vRID 4) w/ vDomainID B + PASID 0
> 
> We should be able to workaround this by changing the way we assign hDomainId
> to be per-device for VFIO pass-through devices although sharing the same v1
> (stage-2) page table. This would look like.

As I said, this doesn't quite work since the VM could do other
things. The kernel must be aware of the vDomainID and must select an
appropriate hDomainID with that knowledge in mind, otherwise
multi-device-groups in guests are fully broken.

>   - Guest view
>     Pass-through Device1 (vRID 3) w/ vDomainID A + PASID 0
>     Pass-through Device2 (vRID 4) w/ vDomainID B + PASID 0
> 
> This should avoid the IOMMU TLB conflict. However, the invalidation would
> need to be done for both DomainId 1 and 2 when updating the v1 (stage-2)
> page table.

Which is the key problem, if the VM thinks it has only one vDomainID
the VMM can't split that into two hDomainID's and expect the viommu
acceleration will work - so we shouldn't try to make it work in SW
either, IMHO.

Jason
  

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)

> ...
> 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.

For the AMD IOMMU INVALIDE_IOTLB_PAGES (i.e. invalidate the ATC on the 
device), the hypervisor needs to map gDeviceId->hDeviceId and issue the 
command on behalf of the VM along with the PASID and GVA (or GVA range) 
provided by the guest.

>   ARM: ASID is ambiguous. We have no idea which Nesting Domain/CD table
>        the ASID is contained in. ARM must forward the ATC invalidation
>        from the guest.
> 
> What iommufd object should receive the IOTLB invalidation command list:
>   Intel: The Nesting domain. The command list has to be broken up per
>          (vDomain-ID,PASID) and that batch delivered to the single
> 	nesting domain. Kernel ignores vDomain-ID/PASID and just
> 	invalidates whatever the nesting domain is actually attached to
>   AMD: Any Nesting Domain in the vDomain-ID group. The command list has
>        to be broken up per (vDomain-ID). Kernel replaces
>        vDomain-ID with pDomain-ID from the nesting domain and executes
>        the invalidation.
>   ARM: The Nesting Parent domain. Kernel forces the VMID from the
>        Nesting Parent and executes the invalidation.
> 
> In all cases the VM issues an ATC invalidation with (vRID, PASID) as
> the tag. The VMM must translate vRID -> dev_id -> pRID
> 
> For a pure SW flow the vRID can be mapped to the dev_id and the ATC
> invalidation delivered to the device object (eg IOMMUFD_DEV_INVALIDATE)
> 
> Finally, we have the HW driven invalidation DMA queues that can be
> directly assigned to the guest. AMD and SMMUv3+vCMDQ support this. In
> this case the HW is directly processing invalidation commands without
> a hypervisor trap.
> 
> 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?

>   ARM: A vRID -> pRID table
>        The vCMDQ is bound to a VM_ID, so to the Nesting Parent
> 
> For AMD, as above, I suggest the vDomain-ID be passed when creating
> the nesting domain
Sure, we can do this part.

> The AMD "virtual function".. It is probably best to create a new iommufd
> object for this and it can be passed in to a few places

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

> The vRID->pRID table should be some mostly common
> IOMMUFD_DEV_ASSIGN_VIRTUAL_ID. AMD will need to pass in the virtual
> function ID and ARM will need to pass in the Nesting Parent ID.

Ok.

> ...
> 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.

Suravee
  

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
  

On Mon, Dec 11, 2023 at 09:20:41AM -0400, Jason Gunthorpe wrote:
> On Mon, Dec 11, 2023 at 08:35:09PM +0800, Yi Liu wrote:
> > > So.. In short.. Invalidation is a PITA. The idea is the same but
> > > annoying little details interfere with actually having a compltely
> > > common API here. IMHO the uAPI in this series is fine. It will support
> > > Intel invalidation and non-ATC invalidation on AMD/ARM. It should be
> > > setup to allow that the target domain object can be any HWPT.
> > 
> > This HWPT is still nested domain. Is it? But it can represent a guest I/O
> > page table (VT-d), guest CD table (ARM), guest CR3 Table (AMD, it seems to
> > be a set of guest CR3 table pointers). May ARM and AMD guys keep me honest
> > here.
> 
> I was thinking ARM would not want to use a nested domain because
> really the invalidation is global to the entire nesting parent.
> 
> But, there is an issue with that - the nesting parent could be
> attached to multiple iommu instances but we only want to invalidate a
> single instance. 

I am still not sure about attaching an S2 domain to multiple
SMMUs. An S2 domain is created per SMMU, and we have such a
rejection in arm_smmu_attach_dev():
	} else if (smmu_domain->smmu != smmu)
		ret = -EINVAL;

I understand that it would be probably ideal to share the S2
iopt among the SMMUs. But in the driver the objects (domain)
holding a shared S2 iopt must be different to allocate their
own VMIDs, right?

Thanks
Nicolin
  

On Fri, Dec 08, 2023 at 09:47:26PM -0400, Jason Gunthorpe wrote:
> What is in a Nested domain:
>  ARM: A CD table pointer
>       Nesting domains are created for every unique CD table top pointer.

I think we basically implemented in a way of syncing STE, i,e,
vSTE.Config must be "S1 Translate" besides a CD table pointer,
and a nested domain is freed when vSTE.Config=BYPASS even if a
CD table pointer is present, right?

> 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"
>  ARM: A vRID -> pRID table
>       The vCMDQ is bound to a VM_ID, so to the Nesting Parent

VCMDQ also has something called "virtual interface" that holds
a VMID and a list of CMDQ queues, which might be a bit similar
to AMD's "virtual function".

> For AMD, as above, I suggest the vDomain-ID be passed when creating
> the nesting domain.
> 
> The AMD "virtual function".. It is probably best to create a new iommufd
> object for this and it can be passed in to a few places
> 
> The vRID->pRID table should be some mostly common
> IOMMUFD_DEV_ASSIGN_VIRTUAL_ID. AMD will need to pass in the virtual
> function ID and ARM will need to pass in the Nesting Parent ID.

It sounds like our previous IOMMUFD_SET/UNSET_IDEV_DATA. I'm
wondering if we need to make it exclusive to the ID assigning?
Maybe set_idev_data could be reused for other potential cases?

If we do implement an IOMMUFD_DEV_ASSIGN_VIRTUAL_ID, do we need
an IOMMUFD_DEV_RESIGN_VIRTUAL_ID? (or better word than resign).

Could the structure just look like this?
struct iommu_dev_assign_virtual_id {
       __u32 size;
       __u32 dev_id;
       __u32 id_type;
       __u32 id;
};

> In many ways the nesting parent/virtual function are very similar
> things. Perhaps ARM should also create a virtual function object which
> is just welded to the nesting parent for API consistency.

A virtual function that holds an S2 domain/iopt + a VMID? If
this is for VCMDQ, the VMCDQ extension driver has that kinda
object holding an S2 domain: I implemented as the extension
function at the end of arm_smmu_finalise_s2() previously.

> So.. In short.. Invalidation is a PITA. The idea is the same but
> annoying little details interfere with actually having a compltely
> common API here. IMHO the uAPI in this series is fine. It will support
> Intel invalidation and non-ATC invalidation on AMD/ARM. It should be
> setup to allow that the target domain object can be any HWPT.
> 
> ARM will be able to do IOTLB invalidation using this API.
> 
> IOMMUFD_DEV_INVALIDATE should be introduced with the same design as
> HWPT invalidate. This would be used for AMD/ARM's ATC invalidation
> (and just force the stream ID, userspace must direct the vRID to the
> correct dev_id).

SMMU's CD invalidations could fall into this category too.

> Then in yet another series we can tackle the entire "virtual function"
> vRID/pRID translation stuff when the mmapable queue thing is
> introduced.

VCMDQ is also a mmapable queue. I feel that there could be
more common stuff between "virtual function" and "virtual
interface", I'll need to take a look at AMD's stuff though.

I previously drafted something to test it out with iommufd.
Basically it needs the pairing of vRID/pRID in attach_dev()
and another ioctl to mmap/config user queue(s):
+struct iommu_hwpt_cache_config_tegra241_vcmdq {
+       __u32 vcmdq_id;			// queue id
+       __u32 vcmdq_log2size;		// queue size
+       __aligned_u64 vcmdq_base;	// queue guest PA
+};

> Thus next steps:
>  - 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.

I will work on these two, presumably including the new
IOMMUFD_DEV_ASSIGN_VIRTUAL_ID or so.

Thanks
Nicolin
  

On Mon, Dec 11, 2023 at 12:11:25PM -0800, Nicolin Chen wrote:
> On Mon, Dec 11, 2023 at 09:20:41AM -0400, Jason Gunthorpe wrote:
> > On Mon, Dec 11, 2023 at 08:35:09PM +0800, Yi Liu wrote:
> > > > So.. In short.. Invalidation is a PITA. The idea is the same but
> > > > annoying little details interfere with actually having a compltely
> > > > common API here. IMHO the uAPI in this series is fine. It will support
> > > > Intel invalidation and non-ATC invalidation on AMD/ARM. It should be
> > > > setup to allow that the target domain object can be any HWPT.
> > > 
> > > This HWPT is still nested domain. Is it? But it can represent a guest I/O
> > > page table (VT-d), guest CD table (ARM), guest CR3 Table (AMD, it seems to
> > > be a set of guest CR3 table pointers). May ARM and AMD guys keep me honest
> > > here.
> > 
> > I was thinking ARM would not want to use a nested domain because
> > really the invalidation is global to the entire nesting parent.
> > 
> > But, there is an issue with that - the nesting parent could be
> > attached to multiple iommu instances but we only want to invalidate a
> > single instance. 
> 
> I am still not sure about attaching an S2 domain to multiple
> SMMUs. An S2 domain is created per SMMU, and we have such a
> rejection in arm_smmu_attach_dev():
> 	} else if (smmu_domain->smmu != smmu)
> 		ret = -EINVAL;

I intend to remove that eventually

> I understand that it would be probably ideal to share the S2
> iopt among the SMMUs. But in the driver the objects (domain)
> holding a shared S2 iopt must be different to allocate their
> own VMIDs, right?

No, the vmid will be moved into the struct arm_smmu_master_domain

Jason
  

On Mon, Dec 11, 2023 at 01:27:49PM -0800, Nicolin Chen wrote:
> On Fri, Dec 08, 2023 at 09:47:26PM -0400, Jason Gunthorpe wrote:
> > What is in a Nested domain:
> >  ARM: A CD table pointer
> >       Nesting domains are created for every unique CD table top pointer.
> 
> I think we basically implemented in a way of syncing STE, i,e,
> vSTE.Config must be "S1 Translate" besides a CD table pointer,
> and a nested domain is freed when vSTE.Config=BYPASS even if a
> CD table pointer is present, right?

Yes, but you can also de-duplicate the nested domains based on the CD
table pointer. It is not as critical for ARM as others, but may
still be worth doing.

> > 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"
> >  ARM: A vRID -> pRID table
> >       The vCMDQ is bound to a VM_ID, so to the Nesting Parent
> 
> VCMDQ also has something called "virtual interface" that holds
> a VMID and a list of CMDQ queues, which might be a bit similar
> to AMD's "virtual function".

Yeah, there must be some kind of logical grouping of HW objects to
build that kind of stuff.

> > The vRID->pRID table should be some mostly common
> > IOMMUFD_DEV_ASSIGN_VIRTUAL_ID. AMD will need to pass in the virtual
> > function ID and ARM will need to pass in the Nesting Parent ID.
> 
> It sounds like our previous IOMMUFD_SET/UNSET_IDEV_DATA. I'm
> wondering if we need to make it exclusive to the ID assigning?
> Maybe set_idev_data could be reused for other potential cases?

No, it should be an API only for the ID
 
> If we do implement an IOMMUFD_DEV_ASSIGN_VIRTUAL_ID, do we need
> an IOMMUFD_DEV_RESIGN_VIRTUAL_ID? (or better word than resign).

I don't think so.. The vRID is basically fixed, if it needs to be
changed then the device can be destroyed (or assign can just change it)

> Could the structure just look like this?
> struct iommu_dev_assign_virtual_id {
>        __u32 size;
>        __u32 dev_id;
>        __u32 id_type;
>        __u32 id;
> };

It needs to take in the viommu_id also, and I'd make the id 64 bits
just for good luck.

> > In many ways the nesting parent/virtual function are very similar
> > things. Perhaps ARM should also create a virtual function object which
> > is just welded to the nesting parent for API consistency.
> 
> A virtual function that holds an S2 domain/iopt + a VMID? If
> this is for VCMDQ, the VMCDQ extension driver has that kinda
> object holding an S2 domain: I implemented as the extension
> function at the end of arm_smmu_finalise_s2() previously.

Not so much hold a S2, but that the VMID would be forced to be shared
amung them somehow.

> > IOMMUFD_DEV_INVALIDATE should be introduced with the same design as
> > HWPT invalidate. This would be used for AMD/ARM's ATC invalidation
> > (and just force the stream ID, userspace must direct the vRID to the
> > correct dev_id).
> 
> SMMU's CD invalidations could fall into this category too.

Yes, I forgot to look closely at the CD/GCR3 table invalidations :(
I actually can't tell how AMD invalidates any GCR3 cache, maybe
INVALIDATE_DEVTAB_ENTRY?

> > Then in yet another series we can tackle the entire "virtual function"
> > vRID/pRID translation stuff when the mmapable queue thing is
> > introduced.
> 
> VCMDQ is also a mmapable queue. I feel that there could be
> more common stuff between "virtual function" and "virtual
> interface", I'll need to take a look at AMD's stuff though.

I'm not thinking of two things right now at least..

> I previously drafted something to test it out with iommufd.
> Basically it needs the pairing of vRID/pRID in attach_dev()
> and another ioctl to mmap/config user queue(s):
> +struct iommu_hwpt_cache_config_tegra241_vcmdq {
> +       __u32 vcmdq_id;			// queue id
> +       __u32 vcmdq_log2size;		// queue size
> +       __aligned_u64 vcmdq_base;	// queue guest PA
> +};

vRID/pRID pairing should come from IOMMUFD_DEV_ASSIGN_VIRTUAL_ID. When
a HWPT is allocated it would be connected to the viommu_id and then it
would all be bundled together in the HW somehow

From there you can ask the viommu_id to setup a queue.

Jason
  

On Mon, Dec 11, 2023 at 05:57:38PM -0400, Jason Gunthorpe wrote:
> On Mon, Dec 11, 2023 at 01:27:49PM -0800, Nicolin Chen wrote:
> > On Fri, Dec 08, 2023 at 09:47:26PM -0400, Jason Gunthorpe wrote:
> > > What is in a Nested domain:
> > >  ARM: A CD table pointer
> > >       Nesting domains are created for every unique CD table top pointer.
> > 
> > I think we basically implemented in a way of syncing STE, i,e,
> > vSTE.Config must be "S1 Translate" besides a CD table pointer,
> > and a nested domain is freed when vSTE.Config=BYPASS even if a
> > CD table pointer is present, right?
> 
> Yes, but you can also de-duplicate the nested domains based on the CD
> table pointer. It is not as critical for ARM as others, but may
> still be worth doing.

Ah right! Should do that.

> > If we do implement an IOMMUFD_DEV_ASSIGN_VIRTUAL_ID, do we need
> > an IOMMUFD_DEV_RESIGN_VIRTUAL_ID? (or better word than resign).
> 
> I don't think so.. The vRID is basically fixed, if it needs to be
> changed then the device can be destroyed (or assign can just change it)

OK. Will insert the cleanup piece to the unbind()/destroy().

> > Could the structure just look like this?
> > struct iommu_dev_assign_virtual_id {
> >        __u32 size;
> >        __u32 dev_id;
> >        __u32 id_type;
> >        __u32 id;
> > };
> 
> It needs to take in the viommu_id also, and I'd make the id 64 bits
> just for good luck.

What is viommu_id required for in this context? I thought we
already know which SMMU instance to issue commands via dev_id?

> > > IOMMUFD_DEV_INVALIDATE should be introduced with the same design as
> > > HWPT invalidate. This would be used for AMD/ARM's ATC invalidation
> > > (and just force the stream ID, userspace must direct the vRID to the
> > > correct dev_id).
> > 
> > SMMU's CD invalidations could fall into this category too.

Do we need a different iommu API for this ioctl? We currently
have the cache_invalidate_user as a domain op. The new device
op will be an iommu op?

And should we rename the "cache_invalidate_user"? Would VT-d
still uses it for device cache?

> > I previously drafted something to test it out with iommufd.
> > Basically it needs the pairing of vRID/pRID in attach_dev()
> > and another ioctl to mmap/config user queue(s):
> > +struct iommu_hwpt_cache_config_tegra241_vcmdq {
> > +       __u32 vcmdq_id;			// queue id
> > +       __u32 vcmdq_log2size;		// queue size
> > +       __aligned_u64 vcmdq_base;	// queue guest PA
> > +};
> 
> vRID/pRID pairing should come from IOMMUFD_DEV_ASSIGN_VIRTUAL_ID. When
> a HWPT is allocated it would be connected to the viommu_id and then it
> would all be bundled together in the HW somehow

Since we were talking about sharing stage-2 domain, the HWPT
to the stage-2 domain will be shared among the vIOMMU/pIOMMU
instances too? I think I am not quite getting the viommu_id
part yet. From the other side of this thread, viommu object
is created per vIOMMU instance and each one actually tied to
a pIOMMU by nature?

Thanks
Nicolin
  

On Mon, Dec 11, 2023 at 11:30:00PM -0800, Nicolin Chen wrote:

> > > Could the structure just look like this?
> > > struct iommu_dev_assign_virtual_id {
> > >        __u32 size;
> > >        __u32 dev_id;
> > >        __u32 id_type;
> > >        __u32 id;
> > > };
> > 
> > It needs to take in the viommu_id also, and I'd make the id 64 bits
> > just for good luck.
> 
> What is viommu_id required for in this context? I thought we
> already know which SMMU instance to issue commands via dev_id?

The viommu_id would be the container that holds the xarray that maps
the vRID to pRID

Logically we could have multiple mappings per iommufd as we could have
multiple iommu instances working here.

> > > > IOMMUFD_DEV_INVALIDATE should be introduced with the same design as
> > > > HWPT invalidate. This would be used for AMD/ARM's ATC invalidation
> > > > (and just force the stream ID, userspace must direct the vRID to the
> > > > correct dev_id).
> > > 
> > > SMMU's CD invalidations could fall into this category too.
> 
> Do we need a different iommu API for this ioctl? We currently
> have the cache_invalidate_user as a domain op. The new device
> op will be an iommu op?

Yes

> And should we rename the "cache_invalidate_user"? Would VT-d
> still uses it for device cache?

I think vt-d will not implement it
 
> > > I previously drafted something to test it out with iommufd.
> > > Basically it needs the pairing of vRID/pRID in attach_dev()
> > > and another ioctl to mmap/config user queue(s):
> > > +struct iommu_hwpt_cache_config_tegra241_vcmdq {
> > > +       __u32 vcmdq_id;			// queue id
> > > +       __u32 vcmdq_log2size;		// queue size
> > > +       __aligned_u64 vcmdq_base;	// queue guest PA
> > > +};
> > 
> > vRID/pRID pairing should come from IOMMUFD_DEV_ASSIGN_VIRTUAL_ID. When
> > a HWPT is allocated it would be connected to the viommu_id and then it
> > would all be bundled together in the HW somehow
> 
> Since we were talking about sharing stage-2 domain, the HWPT
> to the stage-2 domain will be shared among the vIOMMU/pIOMMU
> instances too?

The HWPT for the stage 2 should be shared

> I think I am not quite getting the viommu_id
> part yet. From the other side of this thread, viommu object
> is created per vIOMMU instance and each one actually tied to
> a pIOMMU by nature?

Yes

Jason
  

On Tue, Dec 12, 2023 at 10:44:21AM -0400, Jason Gunthorpe wrote:
> On Mon, Dec 11, 2023 at 11:30:00PM -0800, Nicolin Chen wrote:
> 
> > > > Could the structure just look like this?
> > > > struct iommu_dev_assign_virtual_id {
> > > >        __u32 size;
> > > >        __u32 dev_id;
> > > >        __u32 id_type;
> > > >        __u32 id;
> > > > };
> > > 
> > > It needs to take in the viommu_id also, and I'd make the id 64 bits
> > > just for good luck.
> > 
> > What is viommu_id required for in this context? I thought we
> > already know which SMMU instance to issue commands via dev_id?
> 
> The viommu_id would be the container that holds the xarray that maps
> the vRID to pRID
> 
> Logically we could have multiple mappings per iommufd as we could have
> multiple iommu instances working here.

I see. This is the object to hold a shared stage-2 HWPT/domain then.

// iommufd_private.h

enum iommufd_object_type {
	...
+	IOMMUFD_OBJ_VIOMMU,
	...
};

+struct iommufd_viommu {
+	struct iommufd_object obj;
+	struct iommufd_hwpt_paging *hwpt;
+	struct xarray devices;
+};

struct iommufd_hwpt_paging hwpt {
	...
+	struct list_head viommu_list;
	...
};

struct iommufd_group {
	...
+	struct iommufd_viommu *viommu; // should we attach to viommu instead of hwpt?
	...
};

Question to finalize how we maps vRID-pRID in the xarray:
how should IOMMUFD_DEV_INVALIDATE work? The ioctl structure has
a dev_id and a list of commands that belongs to the device. So,
it forwards the struct device pointer to the driver along with
the commands. Then, doesn't the driver already know the pRID 
from the dev pointer without looking up a vRID-pRID table?

// uapi/linux/iommufd.h

struct iommu_hwpt_alloc {
	...
+	__u32 viommu_id;
};

+enum iommu_dev_virtual_id_type {
+	IOMMU_DEV_VIRTUAL_ID_TYPE_AMD_VIOMMU_DID, // not sure how this fits the xarray in viommu obj.
+	IOMMU_DEV_VIRTUAL_ID_TYPE_AMD_VIOMMU_RID,
+	IOMMU_DEV_VIRTUAL_ID_TYPE_ARM_SMMU_SID,
+};
+
+struct iommu_dev_assign_virtual_id {
+	__u32 size;
+	__u32 dev_id;
+	__u32 viommu_id;
+	__u32 id_type;
+	__aligned_u64 id;
+};

Then, I think that we also need an iommu_viommu_alloc structure
and ioctl to allocate an object, and that VMM should know if it
needs to allocate multiple viommu objects -- this probably needs
the hw_info ioctl to return a piommu_id so VMM gets the list of
piommus from the attached devices?

Another question, introducing the viommu obj complicates things
a lot. Do we want it to come with iommu_dev_assign_virtual_id,
or maybe put in a later series? We could stage the xarray in the
iommu_hwpt_paging struct for now, so a single-IOMMU system could
still work with that.

> > > > > IOMMUFD_DEV_INVALIDATE should be introduced with the same design as
> > > > > HWPT invalidate. This would be used for AMD/ARM's ATC invalidation
> > > > > (and just force the stream ID, userspace must direct the vRID to the
> > > > > correct dev_id).
> > > > 
> > > > SMMU's CD invalidations could fall into this category too.
> > 
> > Do we need a different iommu API for this ioctl? We currently
> > have the cache_invalidate_user as a domain op. The new device
> > op will be an iommu op?
> 
> Yes

OK. FWIW, I think either VMM or iommufd core knows which nested
HWPT the device is attached to. If we ever wanted to keep it in
the domain op list, we could still do that by passing in extra
domain pointer to the driver.

> > And should we rename the "cache_invalidate_user"? Would VT-d
> > still uses it for device cache?
> 
> I think vt-d will not implement it

Then should we "s/cache_invalidate_user/iotlb_sync_user"?

Thanks
Nicolin
  

On Tue, Dec 12, 2023 at 11:13:37AM -0800, Nicolin Chen wrote:
> On Tue, Dec 12, 2023 at 10:44:21AM -0400, Jason Gunthorpe wrote:
> > On Mon, Dec 11, 2023 at 11:30:00PM -0800, Nicolin Chen wrote:
> > 
> > > > > Could the structure just look like this?
> > > > > struct iommu_dev_assign_virtual_id {
> > > > >        __u32 size;
> > > > >        __u32 dev_id;
> > > > >        __u32 id_type;
> > > > >        __u32 id;
> > > > > };
> > > > 
> > > > It needs to take in the viommu_id also, and I'd make the id 64 bits
> > > > just for good luck.
> > > 
> > > What is viommu_id required for in this context? I thought we
> > > already know which SMMU instance to issue commands via dev_id?
> > 
> > The viommu_id would be the container that holds the xarray that maps
> > the vRID to pRID
> > 
> > Logically we could have multiple mappings per iommufd as we could have
> > multiple iommu instances working here.
> 
> I see. This is the object to hold a shared stage-2 HWPT/domain then.

It could be done like that, yes. I wasn't thinking about linking the
stage two so tightly but perhaps? If we can avoid putting the hwpt
here that might be more general.

> // iommufd_private.h
> 
> enum iommufd_object_type {
> 	...
> +	IOMMUFD_OBJ_VIOMMU,
> 	...
> };
> 
> +struct iommufd_viommu {
> +	struct iommufd_object obj;
> +	struct iommufd_hwpt_paging *hwpt;
> +	struct xarray devices;
> +};
> 
> struct iommufd_hwpt_paging hwpt {
> 	...
> +	struct list_head viommu_list;
> 	...
> };

I'd probably first try to go backwards and link the hwpt to the
viommu.

> struct iommufd_group {
> 	...
> +	struct iommufd_viommu *viommu; // should we attach to viommu instead of hwpt?
> 	...
> };

No. Attach is a statement of translation so you still attach to the HWPT.

 
> Question to finalize how we maps vRID-pRID in the xarray:
> how should IOMMUFD_DEV_INVALIDATE work? The ioctl structure has
> a dev_id and a list of commands that belongs to the device. So,
> it forwards the struct device pointer to the driver along with
> the commands. Then, doesn't the driver already know the pRID 
> from the dev pointer without looking up a vRID-pRID table?

The first version of DEV_INVALIDATE should have no xarray. The
invalidate commands are stripped of the SID and executed on the given
dev_id period. VMM splits up the invalidate command list.

The second version maybe we have the xarray, or maybe we just push the
xarray to the eventual viommu series.

> struct iommu_hwpt_alloc {
> 	...
> +	__u32 viommu_id;
> };
> 
> +enum iommu_dev_virtual_id_type {
> +	IOMMU_DEV_VIRTUAL_ID_TYPE_AMD_VIOMMU_DID, // not sure how this fits the xarray in viommu obj.
> +	IOMMU_DEV_VIRTUAL_ID_TYPE_AMD_VIOMMU_RID,

It is just DID. In both cases the ID is the index to the "STE" radix
tree, whatever the driver happens to call it.

> Then, I think that we also need an iommu_viommu_alloc structure
> and ioctl to allocate an object, and that VMM should know if it
> needs to allocate multiple viommu objects -- this probably needs
> the hw_info ioctl to return a piommu_id so VMM gets the list of
> piommus from the attached devices?

Yes and yes

> Another question, introducing the viommu obj complicates things
> a lot. Do we want it to come with iommu_dev_assign_virtual_id,
> or maybe put in a later series? We could stage the xarray in the
> iommu_hwpt_paging struct for now, so a single-IOMMU system could
> still work with that.

All this would be in its own series to enable HW accelerated viommu
support on ARM & AMD as we've been doing so far.

I imagine it after we get the basic invalidation done

> > > And should we rename the "cache_invalidate_user"? Would VT-d
> > > still uses it for device cache?
> > 
> > I think vt-d will not implement it
> 
> Then should we "s/cache_invalidate_user/iotlb_sync_user"?

I think cache_invalidate is still a fine name.. vt-d will generate ATC
invalidations under that function too.

Jason
  

On Tue, Dec 12, 2023 at 03:21:00PM -0400, Jason Gunthorpe wrote:
> On Tue, Dec 12, 2023 at 11:13:37AM -0800, Nicolin Chen wrote:
> > On Tue, Dec 12, 2023 at 10:44:21AM -0400, Jason Gunthorpe wrote:
> > > On Mon, Dec 11, 2023 at 11:30:00PM -0800, Nicolin Chen wrote:
> > > 
> > > > > > Could the structure just look like this?
> > > > > > struct iommu_dev_assign_virtual_id {
> > > > > >        __u32 size;
> > > > > >        __u32 dev_id;
> > > > > >        __u32 id_type;
> > > > > >        __u32 id;
> > > > > > };
> > > > > 
> > > > > It needs to take in the viommu_id also, and I'd make the id 64 bits
> > > > > just for good luck.
> > > > 
> > > > What is viommu_id required for in this context? I thought we
> > > > already know which SMMU instance to issue commands via dev_id?
> > > 
> > > The viommu_id would be the container that holds the xarray that maps
> > > the vRID to pRID
> > > 
> > > Logically we could have multiple mappings per iommufd as we could have
> > > multiple iommu instances working here.
> > 
> > I see. This is the object to hold a shared stage-2 HWPT/domain then.
> 
> It could be done like that, yes. I wasn't thinking about linking the
> stage two so tightly but perhaps? If we can avoid putting the hwpt
> here that might be more general.
> 
> > // iommufd_private.h
> > 
> > enum iommufd_object_type {
> > 	...
> > +	IOMMUFD_OBJ_VIOMMU,
> > 	...
> > };
> > 
> > +struct iommufd_viommu {
> > +	struct iommufd_object obj;
> > +	struct iommufd_hwpt_paging *hwpt;
> > +	struct xarray devices;
> > +};
> > 
> > struct iommufd_hwpt_paging hwpt {
> > 	...
> > +	struct list_head viommu_list;
> > 	...
> > };
> 
> I'd probably first try to go backwards and link the hwpt to the
> viommu.

I think a VM should have only one hwpt_paging object while one
or more viommu objects, so we could do only viommu->hwpt_paging
and hwpt_paging->viommu_list. How to go backwards?

> > struct iommufd_group {
> > 	...
> > +	struct iommufd_viommu *viommu; // should we attach to viommu instead of hwpt?
> > 	...
> > };
> 
> No. Attach is a statement of translation so you still attach to the HWPT.

OK. It's probably not necessary since we know which piommu the
device is behind. And we only need to link viommu and piommu,
right?

> > Question to finalize how we maps vRID-pRID in the xarray:
> > how should IOMMUFD_DEV_INVALIDATE work? The ioctl structure has
> > a dev_id and a list of commands that belongs to the device. So,
> > it forwards the struct device pointer to the driver along with
> > the commands. Then, doesn't the driver already know the pRID 
> > from the dev pointer without looking up a vRID-pRID table?
> 
> The first version of DEV_INVALIDATE should have no xarray. The
> invalidate commands are stripped of the SID and executed on the given
> dev_id period. VMM splits up the invalidate command list.

Yes. This makes sense to me. VMM knows which device to issue
an IOMMUFD_DEV_INVALIDATE from the vSID/vRID in the commands.

> The second version maybe we have the xarray, or maybe we just push the
> xarray to the eventual viommu series.

I think that I still don't get the purpose of the xarray here.
It was needed previously because a cache invalidate per hwpt
doesn't know which device. Now IOMMUFD_DEV_INVALIDATE knows.

Maybe it's related to that narrative "logically we could have
multiple mappings per iommufd" that you mentioned above. Mind
elaborating a bit?

In my mind, viommu is allocated by VMM per piommu, by detecting
the piommu_id via hw_info. In that case, viommu can only have
one unique device list. If IOMMUFD_DEV_INVALIDATE passes in the
dev_id, we don't really need a mapping of vRID-pRID in a multi-
viommu case either? In another word, VMM already has a mapping
from vRID to dev_id, so it could call the DEV_INVALIDATE ioctl
in the first place?

Thanks
Nicolin
  

On Tue, Dec 12, 2023 at 12:05:41PM -0800, Nicolin Chen wrote:
> > > // iommufd_private.h
> > > 
> > > enum iommufd_object_type {
> > > 	...
> > > +	IOMMUFD_OBJ_VIOMMU,
> > > 	...
> > > };
> > > 
> > > +struct iommufd_viommu {
> > > +	struct iommufd_object obj;
> > > +	struct iommufd_hwpt_paging *hwpt;
> > > +	struct xarray devices;
> > > +};
> > > 
> > > struct iommufd_hwpt_paging hwpt {
> > > 	...
> > > +	struct list_head viommu_list;
> > > 	...
> > > };
> > 
> > I'd probably first try to go backwards and link the hwpt to the
> > viommu.
> 
> I think a VM should have only one hwpt_paging object while one
> or more viommu objects, so we could do only viommu->hwpt_paging
> and hwpt_paging->viommu_list. How to go backwards?

That is probably how things would work but I don't know if it makes
sense to enforce it in the kernel logic..

Point the S2 to a list of viommu objects it is linked to

> > > struct iommufd_group {
> > > 	...
> > > +	struct iommufd_viommu *viommu; // should we attach to viommu instead of hwpt?
> > > 	...
> > > };
> > 
> > No. Attach is a statement of translation so you still attach to the HWPT.
> 
> OK. It's probably not necessary since we know which piommu the
> device is behind. And we only need to link viommu and piommu,
> right?

Yes

> > The second version maybe we have the xarray, or maybe we just push the
> > xarray to the eventual viommu series.
> 
> I think that I still don't get the purpose of the xarray here.
> It was needed previously because a cache invalidate per hwpt
> doesn't know which device. Now IOMMUFD_DEV_INVALIDATE knows.
> 
> Maybe it's related to that narrative "logically we could have
> multiple mappings per iommufd" that you mentioned above. Mind
> elaborating a bit?
> 
> In my mind, viommu is allocated by VMM per piommu, by detecting
> the piommu_id via hw_info. In that case, viommu can only have
> one unique device list. If IOMMUFD_DEV_INVALIDATE passes in the
> dev_id, we don't really need a mapping of vRID-pRID in a multi-
> viommu case either? In another word, VMM already has a mapping
> from vRID to dev_id, so it could call the DEV_INVALIDATE ioctl
> in the first place?

The xarray exists to optimize the invalidation flow.

For SW you can imagine issuing an invalidation against the viommu
itself and *all* commands, be they ASID or ATC invalidations can be
processed in one shot. The xarray allows converting the vSID to pSID
to process ATC invalidations, and the viommu object forces a single
VMID to handle the ATC invalidations. If we want to do this, I don't
know.

For HW, the xarray holds the vSID to pSID mapping that must be
programmed into the HW operating the dedicated invalidation queue.

Jason
  

On Wed, Dec 13, 2023 at 08:40:55AM -0400, Jason Gunthorpe wrote:
> On Tue, Dec 12, 2023 at 12:05:41PM -0800, Nicolin Chen wrote:
> > > > // iommufd_private.h
> > > > 
> > > > enum iommufd_object_type {
> > > > 	...
> > > > +	IOMMUFD_OBJ_VIOMMU,
> > > > 	...
> > > > };
> > > > 
> > > > +struct iommufd_viommu {
> > > > +	struct iommufd_object obj;
> > > > +	struct iommufd_hwpt_paging *hwpt;
> > > > +	struct xarray devices;
> > > > +};
> > > > 
> > > > struct iommufd_hwpt_paging hwpt {
> > > > 	...
> > > > +	struct list_head viommu_list;
> > > > 	...
> > > > };
> > > 
> > > I'd probably first try to go backwards and link the hwpt to the
> > > viommu.
> > 
> > I think a VM should have only one hwpt_paging object while one
> > or more viommu objects, so we could do only viommu->hwpt_paging
> > and hwpt_paging->viommu_list. How to go backwards?
> 
> That is probably how things would work but I don't know if it makes
> sense to enforce it in the kernel logic..
> 
> Point the S2 to a list of viommu objects it is linked to

Hmm, isn't that hwpt_paging->viommu_list already?

> > > The second version maybe we have the xarray, or maybe we just push the
> > > xarray to the eventual viommu series.
> > 
> > I think that I still don't get the purpose of the xarray here.
> > It was needed previously because a cache invalidate per hwpt
> > doesn't know which device. Now IOMMUFD_DEV_INVALIDATE knows.
> > 
> > Maybe it's related to that narrative "logically we could have
> > multiple mappings per iommufd" that you mentioned above. Mind
> > elaborating a bit?
> > 
> > In my mind, viommu is allocated by VMM per piommu, by detecting
> > the piommu_id via hw_info. In that case, viommu can only have
> > one unique device list. If IOMMUFD_DEV_INVALIDATE passes in the
> > dev_id, we don't really need a mapping of vRID-pRID in a multi-
> > viommu case either? In another word, VMM already has a mapping
> > from vRID to dev_id, so it could call the DEV_INVALIDATE ioctl
> > in the first place?
> 
> The xarray exists to optimize the invalidation flow.
> 
> For SW you can imagine issuing an invalidation against the viommu
> itself and *all* commands, be they ASID or ATC invalidations can be
> processed in one shot. The xarray allows converting the vSID to pSID
> to process ATC invalidations, and the viommu object forces a single
> VMID to handle the ATC invalidations. If we want to do this, I don't
> know.

I drafted some patches with IOMMUFD_DEV_INVALIDATE yesterday,
and realized the same problem that you pointed out here: how
VMM should handle a group of commands interlaced with ASID and
ATC commands. If VMM dispatches commands into two groups, the
executions of the commands will be in a different order than
what the guest kernel issued in. This might be bitter if there
is an error occurring in the middle of command executions, in
which case some later invalidations are done successfully but
the CONS index would have to stop at a command prior.

And even if there are only ATC invalidations in a guest queue,
there's no guarantee that all commands are for the same dev_id,
i.e. ATC invalidations themselves would be dispatched into more
groups and separate IOMMUFD_DEV_INVALIDATE calls.

With the xarray, perhaps we could provide a viommu_id in data
structure of the current iommu_hwpt_invalidate, i.e. reshaping
the existing invalidate uAPI per viommu, so it can be reused by
ATC invalidations too instead of adding IOMMUFD_DEV_INVALIDATE?
Then we wouldn't have the out-of-order execution problem above.

> For HW, the xarray holds the vSID to pSID mapping that must be
> programmed into the HW operating the dedicated invalidation queue.

Ah, right! VCMDQ at least needs that.

Thanks
Nicolin
  

Hey Yi

I have been working with https://github.com/yiliu1765/qemu/tree/zhenzhong/wip/iommufd_nesting_rfcv1
and have some questions regarding one of the commits in that series.
I however cannot find it in lore.kernel.org. Can you please direct me to
where the rfc was posted? If it has not been posted yet, do you have an
alternate place for discussion?

Best

On Fri, Nov 17, 2023 at 05:07:11AM -0800, Yi Liu wrote:
> Nested translation is a hardware feature that is supported by many modern
> IOMMU hardwares. It has two stages (stage-1, stage-2) address translation
> to get access to the physical address. stage-1 translation table is owned
> by userspace (e.g. by a guest OS), while stage-2 is owned by kernel. Changes
> to stage-1 translation table should be followed by an IOTLB invalidation.
> 
> 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.
> 
>     .-------------.  .---------------------------.
>     |   vIOMMU    |  | Guest I/O page table      |
>     |             |  '---------------------------'
>     .----------------/
>     | PASID Entry |--- PASID cache flush --+
>     '-------------'                        |
>     |             |                        V
>     |             |           I/O page table pointer in GPA
>     '-------------'
> Guest
> ------| Shadow |---------------------------|--------
>       v        v                           v
> Host
>     .-------------.  .------------------------.
>     |   pIOMMU    |  |  FS for GIOVA->GPA     |
>     |             |  '------------------------'
>     .----------------/  |
>     | PASID Entry |     V (Nested xlate)
>     '----------------\.----------------------------------.
>     |             |   | SS for GPA->HPA, unmanaged domain|
>     |             |   '----------------------------------'
>     '-------------'
> Where:
>  - FS = First stage page tables
>  - SS = Second stage page tables
> <Intel VT-d Nested translation>
> 
> This series adds the cache invalidation path for the userspace to invalidate
> cache after modifying the stage-1 page table. This is based on the first part
> of nesting [1]
> 
> Complete code can be found in [2], QEMU could can be found in [3].
> 
> At last, this is a team work together with Nicolin Chen, Lu Baolu. Thanks
> them for the help. ^_^. Look forward to your feedbacks.
> 
> [1] https://lore.kernel.org/linux-iommu/20231026044216.64964-1-yi.l.liu@intel.com/ - merged
> [2] https://github.com/yiliu1765/iommufd/tree/iommufd_nesting
> [3] https://github.com/yiliu1765/qemu/tree/zhenzhong/wip/iommufd_nesting_rfcv1
> 
> Change log:
> 
> v6:
>  - No much change, just rebase on top of 6.7-rc1 as part 1/2 is merged
> 
> v5: https://lore.kernel.org/linux-iommu/20231020092426.13907-1-yi.l.liu@intel.com/#t
>  - Split the iommufd nesting series into two parts of alloc_user and
>    invalidation (Jason)
>  - Split IOMMUFD_OBJ_HW_PAGETABLE to IOMMUFD_OBJ_HWPT_PAGING/_NESTED, and
>    do the same with the structures/alloc()/abort()/destroy(). Reworked the
>    selftest accordingly too. (Jason)
>  - Move hwpt/data_type into struct iommu_user_data from standalone op
>    arguments. (Jason)
>  - Rename hwpt_type to be data_type, the HWPT_TYPE to be HWPT_ALLOC_DATA,
>    _TYPE_DEFAULT to be _ALLOC_DATA_NONE (Jason, Kevin)
>  - Rename iommu_copy_user_data() to iommu_copy_struct_from_user() (Kevin)
>  - Add macro to the iommu_copy_struct_from_user() to calculate min_size
>    (Jason)
>  - Fix two bugs spotted by ZhaoYan
> 
> v4: https://lore.kernel.org/linux-iommu/20230921075138.124099-1-yi.l.liu@intel.com/
>  - Separate HWPT alloc/destroy/abort functions between user-managed HWPTs
>    and kernel-managed HWPTs
>  - Rework invalidate uAPI to be a multi-request array-based design
>  - Add a struct iommu_user_data_array and a helper for driver to sanitize
>    and copy the entry data from user space invalidation array
>  - Add a patch fixing TEST_LENGTH() in selftest program
>  - Drop IOMMU_RESV_IOVA_RANGES patches
>  - Update kdoc and inline comments
>  - Drop the code to add IOMMU_RESV_SW_MSI to kernel-managed HWPT in nested translation,
>    this does not change the rule that resv regions should only be added to the
>    kernel-managed HWPT. The IOMMU_RESV_SW_MSI stuff will be added in later series
>    as it is needed only by SMMU so far.
> 
> v3: https://lore.kernel.org/linux-iommu/20230724110406.107212-1-yi.l.liu@intel.com/
>  - Add new uAPI things in alphabetical order
>  - Pass in "enum iommu_hwpt_type hwpt_type" to op->domain_alloc_user for
>    sanity, replacing the previous op->domain_alloc_user_data_len solution
>  - Return ERR_PTR from domain_alloc_user instead of NULL
>  - Only add IOMMU_RESV_SW_MSI to kernel-managed HWPT in nested translation (Kevin)
>  - Add IOMMU_RESV_IOVA_RANGES to report resv iova ranges to userspace hence
>    userspace is able to exclude the ranges in the stage-1 HWPT (e.g. guest I/O
>    page table). (Kevin)
>  - Add selftest coverage for the new IOMMU_RESV_IOVA_RANGES ioctl
>  - Minor changes per Kevin's inputs
> 
> v2: https://lore.kernel.org/linux-iommu/20230511143844.22693-1-yi.l.liu@intel.com/
>  - Add union iommu_domain_user_data to include all user data structures to avoid
>    passing void * in kernel APIs.
>  - Add iommu op to return user data length for user domain allocation
>  - Rename struct iommu_hwpt_alloc::data_type to be hwpt_type
>  - Store the invalidation data length in iommu_domain_ops::cache_invalidate_user_data_len
>  - Convert cache_invalidate_user op to be int instead of void
>  - Remove @data_type in struct iommu_hwpt_invalidate
>  - Remove out_hwpt_type_bitmap in struct iommu_hw_info hence drop patch 08 of v1
> 
> v1: https://lore.kernel.org/linux-iommu/20230309080910.607396-1-yi.l.liu@intel.com/
> 
> Thanks,
> 	Yi Liu
> 
> Lu Baolu (1):
>   iommu: Add cache_invalidate_user op
> 
> Nicolin Chen (4):
>   iommu: Add iommu_copy_struct_from_user_array helper
>   iommufd/selftest: Add mock_domain_cache_invalidate_user support
>   iommufd/selftest: Add IOMMU_TEST_OP_MD_CHECK_IOTLB test op
>   iommufd/selftest: Add coverage for IOMMU_HWPT_INVALIDATE ioctl
> 
> Yi Liu (1):
>   iommufd: Add IOMMU_HWPT_INVALIDATE
> 
>  drivers/iommu/iommufd/hw_pagetable.c          | 35 ++++++++
>  drivers/iommu/iommufd/iommufd_private.h       |  9 ++
>  drivers/iommu/iommufd/iommufd_test.h          | 22 +++++
>  drivers/iommu/iommufd/main.c                  |  3 +
>  drivers/iommu/iommufd/selftest.c              | 69 +++++++++++++++
>  include/linux/iommu.h                         | 84 +++++++++++++++++++
>  include/uapi/linux/iommufd.h                  | 35 ++++++++
>  tools/testing/selftests/iommu/iommufd.c       | 75 +++++++++++++++++
>  tools/testing/selftests/iommu/iommufd_utils.h | 63 ++++++++++++++
>  9 files changed, 395 insertions(+)
> 
> -- 
> 2.34.1
>
  

On 2023/12/17 19:21, Joel Granados wrote:
> Hey Yi
> 
> I have been working with https://github.com/yiliu1765/qemu/tree/zhenzhong/wip/iommufd_nesting_rfcv1

good to know about it.

> and have some questions regarding one of the commits in that series.
> I however cannot find it in lore.kernel.org. Can you please direct me to
> where the rfc was posted? If it has not been posted yet, do you have an
> alternate place for discussion?

the qemu series has not been posted yet as kernel side is still changing.
It still needs some time to be ready for public review. Zhenzhong Duan
is going to post it when it's ready. If you have questions to discuss,
you can post your questions to Zhenzhong and me first. I guess it may be
fine to cc Alex Williamson, Eric Auger, Nicolin Chen, Cédric Le Goater,
Kevin Tian, Jason Gunthorpe and qemu mail list as this is discussion 
something that is going to be posted in public.

> 
> Best
> 
> On Fri, Nov 17, 2023 at 05:07:11AM -0800, Yi Liu wrote:
>> Nested translation is a hardware feature that is supported by many modern
>> IOMMU hardwares. It has two stages (stage-1, stage-2) address translation
>> to get access to the physical address. stage-1 translation table is owned
>> by userspace (e.g. by a guest OS), while stage-2 is owned by kernel. Changes
>> to stage-1 translation table should be followed by an IOTLB invalidation.
>>
>> 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.
>>
>>      .-------------.  .---------------------------.
>>      |   vIOMMU    |  | Guest I/O page table      |
>>      |             |  '---------------------------'
>>      .----------------/
>>      | PASID Entry |--- PASID cache flush --+
>>      '-------------'                        |
>>      |             |                        V
>>      |             |           I/O page table pointer in GPA
>>      '-------------'
>> Guest
>> ------| Shadow |---------------------------|--------
>>        v        v                           v
>> Host
>>      .-------------.  .------------------------.
>>      |   pIOMMU    |  |  FS for GIOVA->GPA     |
>>      |             |  '------------------------'
>>      .----------------/  |
>>      | PASID Entry |     V (Nested xlate)
>>      '----------------\.----------------------------------.
>>      |             |   | SS for GPA->HPA, unmanaged domain|
>>      |             |   '----------------------------------'
>>      '-------------'
>> Where:
>>   - FS = First stage page tables
>>   - SS = Second stage page tables
>> <Intel VT-d Nested translation>
>>
>> This series adds the cache invalidation path for the userspace to invalidate
>> cache after modifying the stage-1 page table. This is based on the first part
>> of nesting [1]
>>
>> Complete code can be found in [2], QEMU could can be found in [3].
>>
>> At last, this is a team work together with Nicolin Chen, Lu Baolu. Thanks
>> them for the help. ^_^. Look forward to your feedbacks.
>>
>> [1] https://lore.kernel.org/linux-iommu/20231026044216.64964-1-yi.l.liu@intel.com/ - merged
>> [2] https://github.com/yiliu1765/iommufd/tree/iommufd_nesting
>> [3] https://github.com/yiliu1765/qemu/tree/zhenzhong/wip/iommufd_nesting_rfcv1
>>
>> Change log:
>>
>> v6:
>>   - No much change, just rebase on top of 6.7-rc1 as part 1/2 is merged
>>
>> v5: https://lore.kernel.org/linux-iommu/20231020092426.13907-1-yi.l.liu@intel.com/#t
>>   - Split the iommufd nesting series into two parts of alloc_user and
>>     invalidation (Jason)
>>   - Split IOMMUFD_OBJ_HW_PAGETABLE to IOMMUFD_OBJ_HWPT_PAGING/_NESTED, and
>>     do the same with the structures/alloc()/abort()/destroy(). Reworked the
>>     selftest accordingly too. (Jason)
>>   - Move hwpt/data_type into struct iommu_user_data from standalone op
>>     arguments. (Jason)
>>   - Rename hwpt_type to be data_type, the HWPT_TYPE to be HWPT_ALLOC_DATA,
>>     _TYPE_DEFAULT to be _ALLOC_DATA_NONE (Jason, Kevin)
>>   - Rename iommu_copy_user_data() to iommu_copy_struct_from_user() (Kevin)
>>   - Add macro to the iommu_copy_struct_from_user() to calculate min_size
>>     (Jason)
>>   - Fix two bugs spotted by ZhaoYan
>>
>> v4: https://lore.kernel.org/linux-iommu/20230921075138.124099-1-yi.l.liu@intel.com/
>>   - Separate HWPT alloc/destroy/abort functions between user-managed HWPTs
>>     and kernel-managed HWPTs
>>   - Rework invalidate uAPI to be a multi-request array-based design
>>   - Add a struct iommu_user_data_array and a helper for driver to sanitize
>>     and copy the entry data from user space invalidation array
>>   - Add a patch fixing TEST_LENGTH() in selftest program
>>   - Drop IOMMU_RESV_IOVA_RANGES patches
>>   - Update kdoc and inline comments
>>   - Drop the code to add IOMMU_RESV_SW_MSI to kernel-managed HWPT in nested translation,
>>     this does not change the rule that resv regions should only be added to the
>>     kernel-managed HWPT. The IOMMU_RESV_SW_MSI stuff will be added in later series
>>     as it is needed only by SMMU so far.
>>
>> v3: https://lore.kernel.org/linux-iommu/20230724110406.107212-1-yi.l.liu@intel.com/
>>   - Add new uAPI things in alphabetical order
>>   - Pass in "enum iommu_hwpt_type hwpt_type" to op->domain_alloc_user for
>>     sanity, replacing the previous op->domain_alloc_user_data_len solution
>>   - Return ERR_PTR from domain_alloc_user instead of NULL
>>   - Only add IOMMU_RESV_SW_MSI to kernel-managed HWPT in nested translation (Kevin)
>>   - Add IOMMU_RESV_IOVA_RANGES to report resv iova ranges to userspace hence
>>     userspace is able to exclude the ranges in the stage-1 HWPT (e.g. guest I/O
>>     page table). (Kevin)
>>   - Add selftest coverage for the new IOMMU_RESV_IOVA_RANGES ioctl
>>   - Minor changes per Kevin's inputs
>>
>> v2: https://lore.kernel.org/linux-iommu/20230511143844.22693-1-yi.l.liu@intel.com/
>>   - Add union iommu_domain_user_data to include all user data structures to avoid
>>     passing void * in kernel APIs.
>>   - Add iommu op to return user data length for user domain allocation
>>   - Rename struct iommu_hwpt_alloc::data_type to be hwpt_type
>>   - Store the invalidation data length in iommu_domain_ops::cache_invalidate_user_data_len
>>   - Convert cache_invalidate_user op to be int instead of void
>>   - Remove @data_type in struct iommu_hwpt_invalidate
>>   - Remove out_hwpt_type_bitmap in struct iommu_hw_info hence drop patch 08 of v1
>>
>> v1: https://lore.kernel.org/linux-iommu/20230309080910.607396-1-yi.l.liu@intel.com/
>>
>> Thanks,
>> 	Yi Liu
>>
>> Lu Baolu (1):
>>    iommu: Add cache_invalidate_user op
>>
>> Nicolin Chen (4):
>>    iommu: Add iommu_copy_struct_from_user_array helper
>>    iommufd/selftest: Add mock_domain_cache_invalidate_user support
>>    iommufd/selftest: Add IOMMU_TEST_OP_MD_CHECK_IOTLB test op
>>    iommufd/selftest: Add coverage for IOMMU_HWPT_INVALIDATE ioctl
>>
>> Yi Liu (1):
>>    iommufd: Add IOMMU_HWPT_INVALIDATE
>>
>>   drivers/iommu/iommufd/hw_pagetable.c          | 35 ++++++++
>>   drivers/iommu/iommufd/iommufd_private.h       |  9 ++
>>   drivers/iommu/iommufd/iommufd_test.h          | 22 +++++
>>   drivers/iommu/iommufd/main.c                  |  3 +
>>   drivers/iommu/iommufd/selftest.c              | 69 +++++++++++++++
>>   include/linux/iommu.h                         | 84 +++++++++++++++++++
>>   include/uapi/linux/iommufd.h                  | 35 ++++++++
>>   tools/testing/selftests/iommu/iommufd.c       | 75 +++++++++++++++++
>>   tools/testing/selftests/iommu/iommufd_utils.h | 63 ++++++++++++++
>>   9 files changed, 395 insertions(+)
>>
>> -- 
>> 2.34.1
>>
>
  

On Tue, Dec 19, 2023 at 05:26:21PM +0800, Yi Liu wrote:
> On 2023/12/17 19:21, Joel Granados wrote:
> > Hey Yi
> > 
> > I have been working with https://protect2.fireeye.com/v1/url?k=b58750ce-ea1c9eaa-b586db81-000babda0201-365207d33731a099&q=1&e=ee73b69d-5c35-49ef-9e62-2355fb797f21&u=https%3A%2F%2Fgithub.com%2Fyiliu1765%2Fqemu%2Ftree%2Fzhenzhong%2Fwip%2Fiommufd_nesting_rfcv1
> 
> good to know about it.
> 
> > and have some questions regarding one of the commits in that series.
> > I however cannot find it in lore.kernel.org. Can you please direct me to
> > where the rfc was posted? If it has not been posted yet, do you have an
> > alternate place for discussion?
> 
> the qemu series has not been posted yet as kernel side is still changing.
> It still needs some time to be ready for public review. Zhenzhong Duan
> is going to post it when it's ready. If you have questions to discuss,
> you can post your questions to Zhenzhong and me first. I guess it may be
> fine to cc Alex Williamson, Eric Auger, Nicolin Chen, Cédric Le Goater,
> Kevin Tian, Jason Gunthorpe and qemu mail list as this is discussion
> something that is going to be posted in public.
Thx for getting back to me. I'll direct my questions to these
recipients.

Best

> 
> > 
> > Best
> > 
> > On Fri, Nov 17, 2023 at 05:07:11AM -0800, Yi Liu wrote:
> > > Nested translation is a hardware feature that is supported by many modern
> > > IOMMU hardwares. It has two stages (stage-1, stage-2) address translation
> > > to get access to the physical address. stage-1 translation table is owned
> > > by userspace (e.g. by a guest OS), while stage-2 is owned by kernel. Changes
> > > to stage-1 translation table should be followed by an IOTLB invalidation.
> > > 
> > > 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.
> > > 
> > >      .-------------.  .---------------------------.
> > >      |   vIOMMU    |  | Guest I/O page table      |
> > >      |             |  '---------------------------'
> > >      .----------------/
> > >      | PASID Entry |--- PASID cache flush --+
> > >      '-------------'                        |
> > >      |             |                        V
> > >      |             |           I/O page table pointer in GPA
> > >      '-------------'
> > > Guest
> > > ------| Shadow |---------------------------|--------
> > >        v        v                           v
> > > Host
> > >      .-------------.  .------------------------.
> > >      |   pIOMMU    |  |  FS for GIOVA->GPA     |
> > >      |             |  '------------------------'
> > >      .----------------/  |
> > >      | PASID Entry |     V (Nested xlate)
> > >      '----------------\.----------------------------------.
> > >      |             |   | SS for GPA->HPA, unmanaged domain|
> > >      |             |   '----------------------------------'
> > >      '-------------'
> > > Where:
> > >   - FS = First stage page tables
> > >   - SS = Second stage page tables
> > > <Intel VT-d Nested translation>
> > > 
> > > This series adds the cache invalidation path for the userspace to invalidate
> > > cache after modifying the stage-1 page table. This is based on the first part
> > > of nesting [1]
> > > 
> > > Complete code can be found in [2], QEMU could can be found in [3].
> > > 
> > > At last, this is a team work together with Nicolin Chen, Lu Baolu. Thanks
> > > them for the help. ^_^. Look forward to your feedbacks.
> > > 
> > > [1] https://lore.kernel.org/linux-iommu/20231026044216.64964-1-yi.l.liu@intel.com/ - merged
> > > [2] https://protect2.fireeye.com/v1/url?k=38b56f01-672ea165-38b4e44e-000babda0201-469ae350f21411ca&q=1&e=ee73b69d-5c35-49ef-9e62-2355fb797f21&u=https%3A%2F%2Fgithub.com%2Fyiliu1765%2Fiommufd%2Ftree%2Fiommufd_nesting
> > > [3] https://protect2.fireeye.com/v1/url?k=d6e01ed1-897bd0b5-d6e1959e-000babda0201-bcf2b26a8dc8b34d&q=1&e=ee73b69d-5c35-49ef-9e62-2355fb797f21&u=https%3A%2F%2Fgithub.com%2Fyiliu1765%2Fqemu%2Ftree%2Fzhenzhong%2Fwip%2Fiommufd_nesting_rfcv1
> > > 
> > > Change log:
> > > 
> > > v6:
> > >   - No much change, just rebase on top of 6.7-rc1 as part 1/2 is merged
> > > 
> > > v5: https://lore.kernel.org/linux-iommu/20231020092426.13907-1-yi.l.liu@intel.com/#t
> > >   - Split the iommufd nesting series into two parts of alloc_user and
> > >     invalidation (Jason)
> > >   - Split IOMMUFD_OBJ_HW_PAGETABLE to IOMMUFD_OBJ_HWPT_PAGING/_NESTED, and
> > >     do the same with the structures/alloc()/abort()/destroy(). Reworked the
> > >     selftest accordingly too. (Jason)
> > >   - Move hwpt/data_type into struct iommu_user_data from standalone op
> > >     arguments. (Jason)
> > >   - Rename hwpt_type to be data_type, the HWPT_TYPE to be HWPT_ALLOC_DATA,
> > >     _TYPE_DEFAULT to be _ALLOC_DATA_NONE (Jason, Kevin)
> > >   - Rename iommu_copy_user_data() to iommu_copy_struct_from_user() (Kevin)
> > >   - Add macro to the iommu_copy_struct_from_user() to calculate min_size
> > >     (Jason)
> > >   - Fix two bugs spotted by ZhaoYan
> > > 
> > > v4: https://lore.kernel.org/linux-iommu/20230921075138.124099-1-yi.l.liu@intel.com/
> > >   - Separate HWPT alloc/destroy/abort functions between user-managed HWPTs
> > >     and kernel-managed HWPTs
> > >   - Rework invalidate uAPI to be a multi-request array-based design
> > >   - Add a struct iommu_user_data_array and a helper for driver to sanitize
> > >     and copy the entry data from user space invalidation array
> > >   - Add a patch fixing TEST_LENGTH() in selftest program
> > >   - Drop IOMMU_RESV_IOVA_RANGES patches
> > >   - Update kdoc and inline comments
> > >   - Drop the code to add IOMMU_RESV_SW_MSI to kernel-managed HWPT in nested translation,
> > >     this does not change the rule that resv regions should only be added to the
> > >     kernel-managed HWPT. The IOMMU_RESV_SW_MSI stuff will be added in later series
> > >     as it is needed only by SMMU so far.
> > > 
> > > v3: https://lore.kernel.org/linux-iommu/20230724110406.107212-1-yi.l.liu@intel.com/
> > >   - Add new uAPI things in alphabetical order
> > >   - Pass in "enum iommu_hwpt_type hwpt_type" to op->domain_alloc_user for
> > >     sanity, replacing the previous op->domain_alloc_user_data_len solution
> > >   - Return ERR_PTR from domain_alloc_user instead of NULL
> > >   - Only add IOMMU_RESV_SW_MSI to kernel-managed HWPT in nested translation (Kevin)
> > >   - Add IOMMU_RESV_IOVA_RANGES to report resv iova ranges to userspace hence
> > >     userspace is able to exclude the ranges in the stage-1 HWPT (e.g. guest I/O
> > >     page table). (Kevin)
> > >   - Add selftest coverage for the new IOMMU_RESV_IOVA_RANGES ioctl
> > >   - Minor changes per Kevin's inputs
> > > 
> > > v2: https://lore.kernel.org/linux-iommu/20230511143844.22693-1-yi.l.liu@intel.com/
> > >   - Add union iommu_domain_user_data to include all user data structures to avoid
> > >     passing void * in kernel APIs.
> > >   - Add iommu op to return user data length for user domain allocation
> > >   - Rename struct iommu_hwpt_alloc::data_type to be hwpt_type
> > >   - Store the invalidation data length in iommu_domain_ops::cache_invalidate_user_data_len
> > >   - Convert cache_invalidate_user op to be int instead of void
> > >   - Remove @data_type in struct iommu_hwpt_invalidate
> > >   - Remove out_hwpt_type_bitmap in struct iommu_hw_info hence drop patch 08 of v1
> > > 
> > > v1: https://lore.kernel.org/linux-iommu/20230309080910.607396-1-yi.l.liu@intel.com/
> > > 
> > > Thanks,
> > > 	Yi Liu
> > > 
> > > Lu Baolu (1):
> > >    iommu: Add cache_invalidate_user op
> > > 
> > > Nicolin Chen (4):
> > >    iommu: Add iommu_copy_struct_from_user_array helper
> > >    iommufd/selftest: Add mock_domain_cache_invalidate_user support
> > >    iommufd/selftest: Add IOMMU_TEST_OP_MD_CHECK_IOTLB test op
> > >    iommufd/selftest: Add coverage for IOMMU_HWPT_INVALIDATE ioctl
> > > 
> > > Yi Liu (1):
> > >    iommufd: Add IOMMU_HWPT_INVALIDATE
> > > 
> > >   drivers/iommu/iommufd/hw_pagetable.c          | 35 ++++++++
> > >   drivers/iommu/iommufd/iommufd_private.h       |  9 ++
> > >   drivers/iommu/iommufd/iommufd_test.h          | 22 +++++
> > >   drivers/iommu/iommufd/main.c                  |  3 +
> > >   drivers/iommu/iommufd/selftest.c              | 69 +++++++++++++++
> > >   include/linux/iommu.h                         | 84 +++++++++++++++++++
> > >   include/uapi/linux/iommufd.h                  | 35 ++++++++
> > >   tools/testing/selftests/iommu/iommufd.c       | 75 +++++++++++++++++
> > >   tools/testing/selftests/iommu/iommufd_utils.h | 63 ++++++++++++++
> > >   9 files changed, 395 insertions(+)
> > > 
> > > -- 
> > > 2.34.1
> > > 
> > 
> 
> -- 
> Regards,
> Yi Liu