Re: [RFC PATCH 00/21] iommu/amd: Introduce support for HW accelerated vIOMMU w/ nested page table

[Jason Gunthorpe]

On Wed, Jun 21, 2023 at 06:54:47PM -0500, Suravee Suthikulpanit wrote:

> Since the IOMMU hardware virtualizes the guest command buffer, this allows
> IOMMU operations to be accelerated such as invalidation of guest pages
> (i.e. stage1) when the command is issued by the guest kernel without
> intervention from the hypervisor.

This is similar to what we are doing on ARM as well.
 
> This series is implemented on top of the IOMMUFD framework. It leverages
> the exisiting APIs and ioctls for providing guest iommu information
> (i.e. struct iommu_hw_info_amd), and allowing guest to provide guest page
> table information (i.e. struct iommu_hwpt_amd_v2) for setting up user
> domain.
> 
> Please see the [4],[5], and [6] for more detail on the AMD HW-vIOMMU.
> 
> NOTES
> -----
> This series is organized into two parts:
>   * Part1: Preparing IOMMU driver for HW-vIOMMU support (Patch 1-8).
> 
>   * Part2: Introducing HW-vIOMMU support (Patch 9-21).
> 
>   * Patch 12 and 21 extends the existing IOMMUFD ioctls to support
>     additional opterations, which can be categorized into:
>     - Ioctls to init/destroy AMD HW-vIOMMU instance
>     - Ioctls to attach/detach guest devices to the AMD HW-vIOMMU instance.
>     - Ioctls to attach/detach guest domains to the AMD HW-vIOMMU instance.
>     - Ioctls to trap certain AMD HW-vIOMMU MMIO register accesses.
>     - Ioctls to trap AMD HW-vIOMMU command buffer initialization.

No one else seems to need this kind of stuff, why is AMD different?

Emulation and mediation to create the vIOMMU is supposed to be in the
VMM side, not in the kernel. I don't want to see different models by
vendor.

Even stuff like setting up the gcr3 should not be it's own ioctls,
that is now how we are modeling things at all.

I think you need to take smaller steps in line with the other
drivers so we can all progress through this step by step together.

To start focus only on user space page tables and kernel mediated
invalidation and fit into the same model as everyone else. This is
approx the same patches and uAPI you see for ARM and Intel. AFAICT
AMD's HW is very similar to ARM's, so you should be aligning to the
ARM design.

Then maybe we can argue if a kernel vIOMMU emulation/mediation is
appropriate or not, but this series is just too much as is.

I also want to see the AMD driver align with the new APIs for
PASID/etc before we start shovling more stuff into it. This is going
to be part of the iommufd contract as well, I'm very unhappy to see
drivers pick and choosing what part of the contract they implement.

Regards,
Jason