Re: [PATCH v5 07/21] intel_iommu: Introduce a new structure VTDHostIOMMUDevice

[Yi Liu]

On 2025/8/22 14:40, Zhenzhong Duan wrote:

Introduce a new structure VTDHostIOMMUDevice which replaces
HostIOMMUDevice to be stored in hash table.

It includes a reference to HostIOMMUDevice and IntelIOMMUState,
also includes BDF information which will be used in future
patches.

Signed-off-by: Zhenzhong Duan <zhenzhong.d...@intel.com>
Reviewed-by: Eric Auger <eric.au...@redhat.com>
---
  hw/i386/intel_iommu_internal.h |  7 +++++++
  include/hw/i386/intel_iommu.h  |  2 +-
  hw/i386/intel_iommu.c          | 15 +++++++++++++--
  3 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
index 360e937989..c7046eb4e2 100644
--- a/hw/i386/intel_iommu_internal.h
+++ b/hw/i386/intel_iommu_internal.h
@@ -28,6 +28,7 @@
  #ifndef HW_I386_INTEL_IOMMU_INTERNAL_H
  #define HW_I386_INTEL_IOMMU_INTERNAL_H
  #include "hw/i386/intel_iommu.h"
+#include "system/host_iommu_device.h"

/*

   * Intel IOMMU register specification
@@ -608,4 +609,10 @@ typedef struct VTDRootEntry VTDRootEntry;
  /* Bits to decide the offset for each level */
  #define VTD_LEVEL_BITS           9

+typedef struct VTDHostIOMMUDevice {

+    IntelIOMMUState *iommu_state;
+    PCIBus *bus;
+    uint8_t devfn;
+    HostIOMMUDevice *hiod;
+} VTDHostIOMMUDevice;
  #endif
diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h
index e95477e855..50f9b27a45 100644
--- a/include/hw/i386/intel_iommu.h
+++ b/include/hw/i386/intel_iommu.h
@@ -295,7 +295,7 @@ struct IntelIOMMUState {
      /* list of registered notifiers */
      QLIST_HEAD(, VTDAddressSpace) vtd_as_with_notifiers;

- GHashTable *vtd_host_iommu_dev; /* HostIOMMUDevice */

+    GHashTable *vtd_host_iommu_dev;             /* VTDHostIOMMUDevice */

/* interrupt remapping */

      bool intr_enabled;              /* Whether guest enabled IR */
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index e3b871de70..512ca4fdc5 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -281,7 +281,10 @@ static gboolean vtd_hiod_equal(gconstpointer v1, 
gconstpointer v2)

static void vtd_hiod_destroy(gpointer v)

  {
-    object_unref(v);
+    VTDHostIOMMUDevice *vtd_hiod = v;
+
+    object_unref(vtd_hiod->hiod);
+    g_free(vtd_hiod);
  }

static gboolean vtd_hash_remove_by_domain(gpointer key, gpointer value,

@@ -4371,6 +4374,7 @@ static bool vtd_dev_set_iommu_device(PCIBus *bus, void 
*opaque, int devfn,
                                       HostIOMMUDevice *hiod, Error **errp)
  {
      IntelIOMMUState *s = opaque;
+    VTDHostIOMMUDevice *vtd_hiod;
      struct vtd_as_key key = {
          .bus = bus,
          .devfn = devfn,
@@ -4387,7 +4391,14 @@ static bool vtd_dev_set_iommu_device(PCIBus *bus, void 
*opaque, int devfn,
          return false;
      }

+ vtd_hiod = g_malloc0(sizeof(VTDHostIOMMUDevice));

+    vtd_hiod->bus = bus;
+    vtd_hiod->devfn = (uint8_t)devfn;
+    vtd_hiod->iommu_state = s;
+    vtd_hiod->hiod = hiod;

how about moving it after the below if branch? :)

      if (!vtd_check_hiod(s, hiod, errp)) {
+        g_free(vtd_hiod);
          vtd_iommu_unlock(s);
          return false;
      }
@@ -4397,7 +4408,7 @@ static bool vtd_dev_set_iommu_device(PCIBus *bus, void 
*opaque, int devfn,
      new_key->devfn = devfn;

object_ref(hiod);

-    g_hash_table_insert(s->vtd_host_iommu_dev, new_key, hiod);
+    g_hash_table_insert(s->vtd_host_iommu_dev, new_key, vtd_hiod);

vtd_iommu_unlock(s);

LGTM.

Reviewed-by: Yi Liu <yi.l....@intel.com>