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

[CLEMENT MATHIEU--DRIF <clement.mathieu--drif@eviden.com>]

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
>