On Fri, Jun 14, 2024 at 11:52:50AM +0200, Eric Auger wrote:
> This series is based on Zhenzhong HostIOMMUDevice:
>
> [PATCH v7 00/17] Add a host IOMMU device abstraction to check with vIOMMU
> https://lore.kernel.org/all/20240605083043.317831-1-zhenzhong.d...@intel.com/
>
> It allows to convey host IOVA reserved regions to the virtio-iommu and
> uses the HostIOMMUDevice infrastructure. This replaces the usage of
> IOMMU MR ops which fail to satisfy this need for hotplugged devices.
>
> See below for additional background.
Reviewed-by: Michael S. Tsirkin <m...@redhat.com>
Should likely be merged together with the dependency.
I can either merge both this one and the dependency, or
Alex can do that because of the vfio changes.
> In [1] we attempted to fix a case where a VFIO-PCI device protected
> with a virtio-iommu was assigned to an x86 guest. On x86 the physical
> IOMMU may have an address width (gaw) of 39 or 48 bits whereas the
> virtio-iommu used to expose a 64b address space by default.
> Hence the guest was trying to use the full 64b space and we hit
> DMA MAP failures. To work around this issue we managed to pass
> usable IOVA regions (excluding the out of range space) from VFIO
> to the virtio-iommu device. This was made feasible by introducing
> a new IOMMU Memory Region callback dubbed iommu_set_iova_regions().
> This latter gets called when the IOMMU MR is enabled which
> causes the vfio_listener_region_add() to be called.
>
> For coldplugged devices the technique works because we make sure all
> the IOMMU MR are enabled once on the machine init done: 94df5b2180
> ("virtio-iommu: Fix 64kB host page size VFIO device assignment")
> for granule freeze. But I would be keen to get rid of this trick.
>
> However with VFIO-PCI hotplug, this technique fails due to the
> race between the call to the callback in the add memory listener
> and the virtio-iommu probe request. Indeed the probe request gets
> called before the attach to the domain. So in that case the usable
> regions are communicated after the probe request and fail to be
> conveyed to the guest.
>
> Using an IOMMU MR Ops is unpractical because this relies on the IOMMU
> MR to have been enabled and the corresponding vfio_listener_region_add()
> to be executed. Instead this series proposes to replace the usage of this
> API by the recently introduced PCIIOMMUOps: ba7d12eb8c ("hw/pci: modify
> pci_setup_iommu() to set PCIIOMMUOps"). That way, the callback can be
> called earlier, once the usable IOVA regions have been collected by
> VFIO, without the need for the IOMMU MR to be enabled.
>
> This series also removes the spurious message:
> qemu-system-aarch64: warning: virtio-iommu-memory-region-7-0: Notified about
> new host reserved regions after probe
>
> In the short term this may also be used for passing the page size
> mask, which would allow to get rid of the hacky transient IOMMU
> MR enablement mentionned above.
>
> [1] [PATCH v4 00/12] VIRTIO-IOMMU/VFIO: Don't assume 64b IOVA space
> https://lore.kernel.org/all/20231019134651.842175-1-eric.au...@redhat.com/
>
> Extra Notes:
> With that series, the reserved memory regions are communicated on time
> so that the virtio-iommu probe request grabs them. However this is not
> sufficient. In some cases (my case), I still see some DMA MAP failures
> and the guest keeps on using IOVA ranges outside the geometry of the
> physical IOMMU. This is due to the fact the VFIO-PCI device is in the
> same iommu group as the pcie root port. Normally the kernel
> iova_reserve_iommu_regions (dma-iommu.c) is supposed to call reserve_iova()
> for each reserved IOVA, which carves them out of the allocator. When
> iommu_dma_init_domain() gets called for the hotplugged vfio-pci device
> the iova domain is already allocated and set and we don't call
> iova_reserve_iommu_regions() again for the vfio-pci device. So its
> corresponding reserved regions are not properly taken into account.
>
> This is not trivial to fix because theoretically the 1st attached
> devices could already have allocated IOVAs within the reserved regions
> of the second device. Also we are somehow hijacking the reserved
> memory regions to model the geometry of the physical IOMMU so not sure
> any attempt to fix that upstream will be accepted. At the moment one
> solution is to make sure assigned devices end up in singleton group.
> Another solution is to work on a different approach where the gaw
> can be passed as an option to the virtio-iommu device, similarly at
> what is done with intel iommu.
>
> This series can be found at:
> https://github.com/eauger/qemu/tree/iommufd_nesting_preq_v7_resv_regions_v4
>
> History:
> v3 -> v4:
> - add one patch to add aliased pci bus and devfn in the HostIOMMUDevice
> - Use those for resv regions computation
> - Remove VirtioHostIOMMUDevice and simply use the base object
>
> v2 -> v3:
> - moved the series from RFC to patch
> - collected Zhenzhong's R-bs and took into account most of his comments
> (see replies on v2)
>
>
> Eric Auger (8):
> HostIOMMUDevice: Store the VFIO/VDPA agent
> virtio-iommu: Implement set|unset]_iommu_device() callbacks
> HostIOMMUDevice: Introduce get_iova_ranges callback
> HostIOMMUDevice: Store the aliased bus and devfn
> virtio-iommu: Compute host reserved regions
> virtio-iommu: Remove the implementation of iommu_set_iova_range
> hw/vfio: Remove memory_region_iommu_set_iova_ranges() call
> memory: Remove IOMMU MR iommu_set_iova_range API
>
> include/exec/memory.h | 32 ----
> include/hw/virtio/virtio-iommu.h | 2 +
> include/sysemu/host_iommu_device.h | 11 ++
> hw/pci/pci.c | 8 +-
> hw/vfio/common.c | 10 -
> hw/vfio/container.c | 17 ++
> hw/vfio/iommufd.c | 18 ++
> hw/virtio/virtio-iommu.c | 296 +++++++++++++++++++----------
> system/memory.c | 13 --
> 9 files changed, 249 insertions(+), 158 deletions(-)
>
> --
> 2.41.0
