Re: [PATCH v2 02/12] hw/i386/intel_iommu: Do not use SysBus API to map local MMIO region

[Zhao Liu <zhao1.liu@intel.com>]

On Thu, Oct 19, 2023 at 09:16:00AM +0200, Philippe Mathieu-Daudé wrote:
> Date: Thu, 19 Oct 2023 09:16:00 +0200
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
> Subject: [PATCH v2 02/12] hw/i386/intel_iommu: Do not use SysBus API to map
>  local MMIO region
> X-Mailer: git-send-email 2.41.0
> 
> There is no point in exposing an internal MMIO region via
> SysBus and directly mapping it in the very same device.
> 
> Just map it without using the SysBus API.
> 
> Transformation done using the following coccinelle script:
> 
>   @@
>   expression sbdev;
>   expression index;
>   expression addr;
>   expression subregion;
>   @@
>   -    sysbus_init_mmio(sbdev, subregion);
>        ... when != sbdev
>   -    sysbus_mmio_map(sbdev, index, addr);
>   +    memory_region_add_subregion(get_system_memory(), addr, subregion);
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  hw/i386/intel_iommu.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)

Oh, I missed you've queued this series...

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

> 
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 2c832ab68b..e4f6cedcb1 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -4134,6 +4134,8 @@ static void vtd_realize(DeviceState *dev, Error **errp)
>      qemu_mutex_init(&s->iommu_lock);
>      memory_region_init_io(&s->csrmem, OBJECT(s), &vtd_mem_ops, s,
>                            "intel_iommu", DMAR_REG_SIZE);
> +    memory_region_add_subregion(get_system_memory(),
> +                                Q35_HOST_BRIDGE_IOMMU_ADDR, &s->csrmem);
>  
>      /* Create the shared memory regions by all devices */
>      memory_region_init(&s->mr_nodmar, OBJECT(s), "vtd-nodmar",
> @@ -4148,15 +4150,12 @@ static void vtd_realize(DeviceState *dev, Error **errp)
>      memory_region_add_subregion_overlap(&s->mr_nodmar,
>                                          VTD_INTERRUPT_ADDR_FIRST,
>                                          &s->mr_ir, 1);
> -
> -    sysbus_init_mmio(SYS_BUS_DEVICE(s), &s->csrmem);
>      /* No corresponding destroy */
>      s->iotlb = g_hash_table_new_full(vtd_iotlb_hash, vtd_iotlb_equal,
>                                       g_free, g_free);
>      s->vtd_address_spaces = g_hash_table_new_full(vtd_as_hash, vtd_as_equal,
>                                        g_free, g_free);
>      vtd_init(s);
> -    sysbus_mmio_map(SYS_BUS_DEVICE(s), 0, Q35_HOST_BRIDGE_IOMMU_ADDR);
>      pci_setup_iommu(bus, vtd_host_dma_iommu, dev);
>      /* Pseudo address space under root PCI bus. */
>      x86ms->ioapic_as = vtd_host_dma_iommu(bus, s, Q35_PSEUDO_DEVFN_IOAPIC);
> -- 
> 2.41.0
> 
> 
>