[PATCH 10/35] hw/i386: QOM-ify AddressSpace

Akihiko Odaki posted 35 patches 1 month, 4 weeks ago
[PATCH 10/35] hw/i386: QOM-ify AddressSpace
Posted by Akihiko Odaki 1 month, 4 weeks ago
Make AddressSpaces QOM objects to ensure that they are destroyed when
their owners are finalized and also to get a unique path for debugging
output.

The name arguments were used to distinguish AddresSpaces in debugging
output, but they will represent property names after QOM-ification and
debugging output will show QOM paths. So change them to make them more
concise and also avoid conflicts with other properties.

Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
---
 hw/i386/amd_iommu.c   | 5 +++--
 hw/i386/intel_iommu.c | 6 ++++--
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index af239390ba04..541b9a8c89e1 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -1494,7 +1494,7 @@ static AddressSpace *amdvi_host_dma_iommu(PCIBus *bus, void *opaque, int devfn)
 
     /* set up AMD-Vi region */
     if (!iommu_as[devfn]) {
-        snprintf(name, sizeof(name), "amd_iommu_devfn_%d", devfn);
+        snprintf(name, sizeof(name), "as-%d", devfn);
 
         iommu_as[devfn] = g_new0(AMDVIAddressSpace, 1);
         iommu_as[devfn]->bus_num = (uint8_t)bus_num;
@@ -1522,7 +1522,8 @@ static AddressSpace *amdvi_host_dma_iommu(PCIBus *bus, void *opaque, int devfn)
                                  "amd_iommu", UINT64_MAX);
         memory_region_init(&amdvi_dev_as->root, OBJECT(s),
                            "amdvi_root", UINT64_MAX);
-        address_space_init(&amdvi_dev_as->as, NULL, &amdvi_dev_as->root, name);
+        address_space_init(&amdvi_dev_as->as, OBJECT(s), &amdvi_dev_as->root,
+                           name);
         memory_region_add_subregion_overlap(&amdvi_dev_as->root, 0,
                                             MEMORY_REGION(&amdvi_dev_as->iommu),
                                             0);
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 1f40d904326e..5e6d7d510e03 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -4221,6 +4221,7 @@ VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus,
     vtd_iommu_unlock(s);
 
     if (!vtd_dev_as) {
+        g_autofree char *as_name = NULL;
         struct vtd_as_key *new_key;
         /* Slow path */
 
@@ -4263,8 +4264,9 @@ VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus,
         vtd_dev_as->iova_tree = iova_tree_new();
 
         memory_region_init(&vtd_dev_as->root, OBJECT(s), name, UINT64_MAX);
-        address_space_init(&vtd_dev_as->as, NULL, &vtd_dev_as->root,
-                           "vtd-root");
+        as_name = g_strconcat(name, "-as", NULL);
+        address_space_init(&vtd_dev_as->as, OBJECT(s), &vtd_dev_as->root,
+                           as_name);
 
         /*
          * Build the DMAR-disabled container with aliases to the

-- 
2.51.0
Re: [PATCH 10/35] hw/i386: QOM-ify AddressSpace
Posted by CLEMENT MATHIEU--DRIF 1 month, 3 weeks ago
Hi Akihiko,

Why do we change the naming scheme in amd-vi?  
Did you have any issue with the old one?

If we decide not to stick to the old one, maybe splitting the slot and function would be convenient.

Thanks

On Wed, 2025-09-17 at 21:56 +0900, Akihiko Odaki wrote:
> 
> 
> Make AddressSpaces QOM objects to ensure that they are destroyed when  
> their owners are finalized and also to get a unique path for debugging  
> output.
> 
> The name arguments were used to distinguish AddresSpaces in debugging  
> output, but they will represent property names after QOM-ification and  
> debugging output will show QOM paths. So change them to make them more  
> concise and also avoid conflicts with other properties.
> 
> Signed-off-by: Akihiko Odaki <[odaki@rsg.ci.i.u-tokyo.ac.jp](mailto:odaki@rsg.ci.i.u-tokyo.ac.jp)>  
> ---  
>  hw/i386/amd_iommu.c   | 5 +++--  
>  hw/i386/intel_iommu.c | 6 ++++--  
>  2 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c  
> index af239390ba04..541b9a8c89e1 100644  
> --- a/hw/i386/amd_iommu.c  
> +++ b/hw/i386/amd_iommu.c  
> @@ -1494,7 +1494,7 @@ static AddressSpace *amdvi_host_dma_iommu(PCIBus *bus, void *opaque, int devfn)
> 
>      /* set up AMD-Vi region */  
>      if (!iommu_as[devfn]) {  
> -        snprintf(name, sizeof(name), "amd_iommu_devfn_%d", devfn);  
> +        snprintf(name, sizeof(name), "as-%d", devfn);
> 
>          iommu_as[devfn] = g_new0(AMDVIAddressSpace, 1);  
>          iommu_as[devfn]->bus_num = (uint8_t)bus_num;  
> @@ -1522,7 +1522,8 @@ static AddressSpace *amdvi_host_dma_iommu(PCIBus *bus, void *opaque, int devfn)  
>                                   "amd_iommu", UINT64_MAX);  
>          memory_region_init(&amdvi_dev_as->root, OBJECT(s),  
>                             "amdvi_root", UINT64_MAX);  
> -        address_space_init(&amdvi_dev_as->as, NULL, &amdvi_dev_as->root, name);  
> +        address_space_init(&amdvi_dev_as->as, OBJECT(s), &amdvi_dev_as->root,  
> +                           name);  
>          memory_region_add_subregion_overlap(&amdvi_dev_as->root, 0,  
>                                              MEMORY_REGION(&amdvi_dev_as->iommu),  
>                                              0);  
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c  
> index 1f40d904326e..5e6d7d510e03 100644  
> --- a/hw/i386/intel_iommu.c  
> +++ b/hw/i386/intel_iommu.c  
> @@ -4221,6 +4221,7 @@ VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus,  
>      vtd_iommu_unlock(s);
> 
>      if (!vtd_dev_as) {  
> +        g_autofree char *as_name = NULL;  
>          struct vtd_as_key *new_key;  
>          /* Slow path */
> 
> @@ -4263,8 +4264,9 @@ VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus,  
>          vtd_dev_as->iova_tree = iova_tree_new();
> 
>          memory_region_init(&vtd_dev_as->root, OBJECT(s), name, UINT64_MAX);  
> -        address_space_init(&vtd_dev_as->as, NULL, &vtd_dev_as->root,  
> -                           "vtd-root");  
> +        as_name = g_strconcat(name, "-as", NULL);  
> +        address_space_init(&vtd_dev_as->as, OBJECT(s), &vtd_dev_as->root,  
> +                           as_name);
> 
>          /*  
>           * Build the DMAR-disabled container with aliases to the
> 
> --  
> 2.51.0
> 
Re: [PATCH 10/35] hw/i386: QOM-ify AddressSpace
Posted by Akihiko Odaki 1 month, 3 weeks ago
On 2025/09/18 14:53, CLEMENT MATHIEU--DRIF wrote:
> Hi Akihiko,
> 
> Why do we change the naming scheme in amd-vi?
> Did you have any issue with the old one?

QOM-ifying AddressSpaces moves the responsibility to create distinct 
identifiers for debug outputs from the callers of address_space_init() 
to AddressSpaces. QOM-ified AddressSpaces can create unique, unambigous 
identifiers by inspecting the QOM hierarchy and cover all devices, not 
just amd-iommu.

> 
> If we decide not to stick to the old one, maybe splitting the slot and function would be convenient.

Strictly speaking such a change is an out-of-scope of this patch, but it 
won't hurt to have it. I'll make the change with the next version.

Regards,
Akihiko Odaki