Re: [PATCH v5 19/19] intel_iommu: Check compatibility with host IOMMU capabilities

[Cédric Le Goater]

On 5/8/24 11:03, Zhenzhong Duan wrote:

If check fails, host device (either VFIO or VDPA device) is not
compatible with current vIOMMU config and should not be passed to
guest.

Only aw_bits is checked for now, we don't care other capabilities
before scalable modern mode is introduced.

Signed-off-by: Yi Liu <yi.l....@intel.com>
Signed-off-by: Zhenzhong Duan <zhenzhong.d...@intel.com>
---
  hw/i386/intel_iommu.c | 27 +++++++++++++++++++++++++++
  1 file changed, 27 insertions(+)

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 747c988bc4..07bfd4f99e 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -20,6 +20,7 @@
   */

#include "qemu/osdep.h"

+#include CONFIG_DEVICES /* CONFIG_HOST_IOMMU_DEVICE */
  #include "qemu/error-report.h"
  #include "qemu/main-loop.h"
  #include "qapi/error.h"
@@ -3819,6 +3820,26 @@ VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, 
PCIBus *bus,
      return vtd_dev_as;
  }

+static bool vtd_check_hdev(IntelIOMMUState *s, VTDHostIOMMUDevice *vtd_hdev,

+                           Error **errp)
+{
+#ifdef CONFIG_HOST_IOMMU_DEVICE
+    HostIOMMUDevice *hiod = vtd_hdev->dev;
+    int ret;
+
+    /* Common checks */
+    ret = host_iommu_device_get_cap(hiod, HOST_IOMMU_DEVICE_CAP_AW_BITS, errp);

To avoid CONFIG_HOST_IOMMU_DEVICE, host_iommu_device_get_cap() could be
open coded.

+    if (ret < 0) {
+        return false;
+    }
+    if (s->aw_bits > ret) {
+        error_setg(errp, "aw-bits %d > host aw-bits %d", s->aw_bits, ret);
+        return false;
+    }
+#endif
+    return true;
+}
+
  static bool vtd_dev_set_iommu_device(PCIBus *bus, void *opaque, int devfn,
                                       HostIOMMUDevice *hiod, Error **errp)
  {
@@ -3848,6 +3869,12 @@ static bool vtd_dev_set_iommu_device(PCIBus *bus, void 
*opaque, int devfn,
      vtd_hdev->iommu_state = s;
      vtd_hdev->dev = hiod;

+ if (!vtd_check_hdev(s, vtd_hdev, errp)) {

+        g_free(vtd_hdev);
+        vtd_iommu_unlock(s);
+        return false;
+    }

This check could be first done before allocating vtd_hdev.


Thanks,

C.

      new_key = g_malloc(sizeof(*new_key));
      new_key->bus = bus;
      new_key->devfn = devfn;