diff mbox series

[rfcv2,12/20] intel_iommu: Introduce a new structure VTDHostIOMMUDevice

Message ID 20250219082228.3303163-13-zhenzhong.duan@intel.com (mailing list archive)
State New
Headers show
Series intel_iommu: Enable stage-1 translation for passthrough device | expand

Commit Message

Duan, Zhenzhong Feb. 19, 2025, 8:22 a.m. UTC
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.duan@intel.com>
---
 hw/i386/intel_iommu_internal.h |  7 +++++++
 include/hw/i386/intel_iommu.h  |  2 +-
 hw/i386/intel_iommu.c          | 14 ++++++++++++--
 3 files changed, 20 insertions(+), 3 deletions(-)

Comments

Eric Auger Feb. 21, 2025, 1:03 p.m. UTC | #1
On 2/19/25 9:22 AM, 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.duan@intel.com>
> ---
>  hw/i386/intel_iommu_internal.h |  7 +++++++
>  include/hw/i386/intel_iommu.h  |  2 +-
>  hw/i386/intel_iommu.c          | 14 ++++++++++++--
>  3 files changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
> index 2cda744786..18bc22fc72 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;
Just to make sure the parent

HostIOMMUDevice has aliased_bus and aliased_devfn. Can you explain why do you need both aliased and non aliased info?

Thanks

Eric

> +    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 9de60e607d..fafa199f52 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,
> @@ -4388,6 +4391,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,
> @@ -4404,6 +4408,12 @@ 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;
> +
>      if (!vtd_check_hiod(s, hiod, errp)) {
>          vtd_iommu_unlock(s);
>          return false;
> @@ -4414,7 +4424,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);
>
Duan, Zhenzhong Feb. 28, 2025, 8:58 a.m. UTC | #2
>-----Original Message-----
>From: Eric Auger <eric.auger@redhat.com>
>Subject: Re: [PATCH rfcv2 12/20] intel_iommu: Introduce a new structure
>VTDHostIOMMUDevice
>
>
>
>
>On 2/19/25 9:22 AM, 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.duan@intel.com>
>> ---
>>  hw/i386/intel_iommu_internal.h |  7 +++++++
>>  include/hw/i386/intel_iommu.h  |  2 +-
>>  hw/i386/intel_iommu.c          | 14 ++++++++++++--
>>  3 files changed, 20 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
>> index 2cda744786..18bc22fc72 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;
>Just to make sure the parent
>
>HostIOMMUDevice has aliased_bus and aliased_devfn. Can you explain why do
>you need both aliased and non aliased info?

Virtual vtd only need non aliased bdf, it uses non aliased bdf to index
HostIOMMUDevice to do attachment/detachment.

I remember virtio-iommu need aliased bdf to rebuild reserved regions
for aliased IOMMUDevice.

Thanks
Zhenzhong
diff mbox series

Patch

diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
index 2cda744786..18bc22fc72 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 9de60e607d..fafa199f52 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,
@@ -4388,6 +4391,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,
@@ -4404,6 +4408,12 @@  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;
+
     if (!vtd_check_hiod(s, hiod, errp)) {
         vtd_iommu_unlock(s);
         return false;
@@ -4414,7 +4424,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);