[PATCH v4 5/7] hw/arm/virt: Add an SMMU_IO_LEN macro

Shameer Kolothum via posted 7 patches 5 months ago
Maintainers: Eric Auger <eric.auger@redhat.com>, Peter Maydell <peter.maydell@linaro.org>, "Michael S. Tsirkin" <mst@redhat.com>, Igor Mammedov <imammedo@redhat.com>, Ani Sinha <anisinha@redhat.com>, Shannon Zhao <shannon.zhaosl@gmail.com>
There is a newer version of this series
[PATCH v4 5/7] hw/arm/virt: Add an SMMU_IO_LEN macro
Posted by Shameer Kolothum via 5 months ago
From: Nicolin Chen <nicolinc@nvidia.com>

This is useful as the subsequent support for new SMMUv3 dev will also
use the same.

Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
Reviewed-by: Donald Dutile <ddutile@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Tested-by: Nathan Chen <nathanc@nvidia.com>
Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
---
 hw/arm/virt.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 71b923f786..eeace4754d 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -146,6 +146,9 @@ static void arm_virt_compat_set(MachineClass *mc)
 #define LEGACY_RAMLIMIT_GB 255
 #define LEGACY_RAMLIMIT_BYTES (LEGACY_RAMLIMIT_GB * GiB)
 
+/* MMIO region size for SMMUv3 */
+#define SMMU_IO_LEN 0x20000
+
 /* Addresses and sizes of our components.
  * 0..128MB is space for a flash device so we can run bootrom code such as UEFI.
  * 128MB..256MB is used for miscellaneous device I/O.
@@ -177,7 +180,7 @@ static const MemMapEntry base_memmap[] = {
     [VIRT_FW_CFG] =             { 0x09020000, 0x00000018 },
     [VIRT_GPIO] =               { 0x09030000, 0x00001000 },
     [VIRT_UART1] =              { 0x09040000, 0x00001000 },
-    [VIRT_SMMU] =               { 0x09050000, 0x00020000 },
+    [VIRT_SMMU] =               { 0x09050000, SMMU_IO_LEN },
     [VIRT_PCDIMM_ACPI] =        { 0x09070000, MEMORY_HOTPLUG_IO_LEN },
     [VIRT_ACPI_GED] =           { 0x09080000, ACPI_GED_EVT_SEL_LEN },
     [VIRT_NVDIMM_ACPI] =        { 0x09090000, NVDIMM_ACPI_IO_LEN},
@@ -1447,7 +1450,6 @@ static void create_smmu(const VirtMachineState *vms,
     int irq =  vms->irqmap[VIRT_SMMU];
     int i;
     hwaddr base = vms->memmap[VIRT_SMMU].base;
-    hwaddr size = vms->memmap[VIRT_SMMU].size;
     DeviceState *dev;
 
     if (vms->iommu != VIRT_IOMMU_SMMUV3 || !vms->iommu_phandle) {
@@ -1467,7 +1469,7 @@ static void create_smmu(const VirtMachineState *vms,
         sysbus_connect_irq(SYS_BUS_DEVICE(dev), i,
                            qdev_get_gpio_in(vms->gic, irq + i));
     }
-    create_smmuv3_dt_bindings(vms, base, size, irq);
+    create_smmuv3_dt_bindings(vms, base, SMMU_IO_LEN, irq);
 }
 
 static void create_virtio_iommu_dt_bindings(VirtMachineState *vms)
-- 
2.47.0
Re: [PATCH v4 5/7] hw/arm/virt: Add an SMMU_IO_LEN macro
Posted by Jonathan Cameron via 5 months ago
On Fri, 13 Jun 2025 15:44:47 +0100
Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> wrote:

> From: Nicolin Chen <nicolinc@nvidia.com>
> 
> This is useful as the subsequent support for new SMMUv3 dev will also
> use the same.
> 
> Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
> Reviewed-by: Donald Dutile <ddutile@redhat.com>
> Reviewed-by: Eric Auger <eric.auger@redhat.com>
> Tested-by: Nathan Chen <nathanc@nvidia.com>
> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
> ---
>  hw/arm/virt.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 71b923f786..eeace4754d 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -146,6 +146,9 @@ static void arm_virt_compat_set(MachineClass *mc)
>  #define LEGACY_RAMLIMIT_GB 255
>  #define LEGACY_RAMLIMIT_BYTES (LEGACY_RAMLIMIT_GB * GiB)
>  
> +/* MMIO region size for SMMUv3 */
> +#define SMMU_IO_LEN 0x20000
> +
>  /* Addresses and sizes of our components.
>   * 0..128MB is space for a flash device so we can run bootrom code such as UEFI.
>   * 128MB..256MB is used for miscellaneous device I/O.
> @@ -177,7 +180,7 @@ static const MemMapEntry base_memmap[] = {
>      [VIRT_FW_CFG] =             { 0x09020000, 0x00000018 },
>      [VIRT_GPIO] =               { 0x09030000, 0x00001000 },
>      [VIRT_UART1] =              { 0x09040000, 0x00001000 },
> -    [VIRT_SMMU] =               { 0x09050000, 0x00020000 },
> +    [VIRT_SMMU] =               { 0x09050000, SMMU_IO_LEN },

This bit is fine.

>      [VIRT_PCDIMM_ACPI] =        { 0x09070000, MEMORY_HOTPLUG_IO_LEN },
>      [VIRT_ACPI_GED] =           { 0x09080000, ACPI_GED_EVT_SEL_LEN },
>      [VIRT_NVDIMM_ACPI] =        { 0x09090000, NVDIMM_ACPI_IO_LEN},
> @@ -1447,7 +1450,6 @@ static void create_smmu(const VirtMachineState *vms,
>      int irq =  vms->irqmap[VIRT_SMMU];
>      int i;
>      hwaddr base = vms->memmap[VIRT_SMMU].base;
> -    hwaddr size = vms->memmap[VIRT_SMMU].size;
>      DeviceState *dev;
>  
>      if (vms->iommu != VIRT_IOMMU_SMMUV3 || !vms->iommu_phandle) {
> @@ -1467,7 +1469,7 @@ static void create_smmu(const VirtMachineState *vms,
>          sysbus_connect_irq(SYS_BUS_DEVICE(dev), i,
>                             qdev_get_gpio_in(vms->gic, irq + i));
>      }
> -    create_smmuv3_dt_bindings(vms, base, size, irq);
> +    create_smmuv3_dt_bindings(vms, base, SMMU_IO_LEN, irq);

This seems a little odd.  I guess it may make sense later, but for all other
cases we only use the macros to define the base_memmap.

Perhaps delay this change until it is more obvious why?


>  }
>  
>  static void create_virtio_iommu_dt_bindings(VirtMachineState *vms)
RE: [PATCH v4 5/7] hw/arm/virt: Add an SMMU_IO_LEN macro
Posted by Shameerali Kolothum Thodi via 5 months ago

> -----Original Message-----
> From: Jonathan Cameron <jonathan.cameron@huawei.com>
> Sent: Monday, June 16, 2025 12:02 PM
> To: Shameerali Kolothum Thodi
> <shameerali.kolothum.thodi@huawei.com>; Linuxarm
> <linuxarm@huawei.com>
> Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org;
> eric.auger@redhat.com; peter.maydell@linaro.org; jgg@nvidia.com;
> nicolinc@nvidia.com; ddutile@redhat.com; berrange@redhat.com;
> imammedo@redhat.com; nathanc@nvidia.com; mochs@nvidia.com;
> smostafa@google.com; Wangzhou (B) <wangzhou1@hisilicon.com>;
> jiangkunkun <jiangkunkun@huawei.com>; Jonathan Cameron
> <jonathan.cameron@huawei.com>; zhangfei.gao@linaro.org
> Subject: Re: [PATCH v4 5/7] hw/arm/virt: Add an SMMU_IO_LEN macro
> 
> On Fri, 13 Jun 2025 15:44:47 +0100
> Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> wrote:
> 
> > From: Nicolin Chen <nicolinc@nvidia.com>
> >
> > This is useful as the subsequent support for new SMMUv3 dev will also
> > use the same.
> >
> > Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
> > Reviewed-by: Donald Dutile <ddutile@redhat.com>
> > Reviewed-by: Eric Auger <eric.auger@redhat.com>
> > Tested-by: Nathan Chen <nathanc@nvidia.com>
> > Signed-off-by: Shameer Kolothum
> <shameerali.kolothum.thodi@huawei.com>
> > ---
> >  hw/arm/virt.c | 8 +++++---
> >  1 file changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> > index 71b923f786..eeace4754d 100644
> > --- a/hw/arm/virt.c
> > +++ b/hw/arm/virt.c
> > @@ -146,6 +146,9 @@ static void arm_virt_compat_set(MachineClass
> *mc)
> >  #define LEGACY_RAMLIMIT_GB 255
> >  #define LEGACY_RAMLIMIT_BYTES (LEGACY_RAMLIMIT_GB * GiB)
> >
> > +/* MMIO region size for SMMUv3 */
> > +#define SMMU_IO_LEN 0x20000
> > +
> >  /* Addresses and sizes of our components.
> >   * 0..128MB is space for a flash device so we can run bootrom code such
> as UEFI.
> >   * 128MB..256MB is used for miscellaneous device I/O.
> > @@ -177,7 +180,7 @@ static const MemMapEntry base_memmap[] = {
> >      [VIRT_FW_CFG] =             { 0x09020000, 0x00000018 },
> >      [VIRT_GPIO] =               { 0x09030000, 0x00001000 },
> >      [VIRT_UART1] =              { 0x09040000, 0x00001000 },
> > -    [VIRT_SMMU] =               { 0x09050000, 0x00020000 },
> > +    [VIRT_SMMU] =               { 0x09050000, SMMU_IO_LEN },
> 
> This bit is fine.
> 
> >      [VIRT_PCDIMM_ACPI] =        { 0x09070000, MEMORY_HOTPLUG_IO_LEN
> },
> >      [VIRT_ACPI_GED] =           { 0x09080000, ACPI_GED_EVT_SEL_LEN },
> >      [VIRT_NVDIMM_ACPI] =        { 0x09090000, NVDIMM_ACPI_IO_LEN},
> > @@ -1447,7 +1450,6 @@ static void create_smmu(const
> VirtMachineState *vms,
> >      int irq =  vms->irqmap[VIRT_SMMU];
> >      int i;
> >      hwaddr base = vms->memmap[VIRT_SMMU].base;
> > -    hwaddr size = vms->memmap[VIRT_SMMU].size;
> >      DeviceState *dev;
> >
> >      if (vms->iommu != VIRT_IOMMU_SMMUV3 || !vms->iommu_phandle) {
> > @@ -1467,7 +1469,7 @@ static void create_smmu(const
> VirtMachineState *vms,
> >          sysbus_connect_irq(SYS_BUS_DEVICE(dev), i,
> >                             qdev_get_gpio_in(vms->gic, irq + i));
> >      }
> > -    create_smmuv3_dt_bindings(vms, base, size, irq);
> > +    create_smmuv3_dt_bindings(vms, base, SMMU_IO_LEN, irq);
> 
> This seems a little odd.  I guess it may make sense later, but for all other
> cases we only use the macros to define the base_memmap.
> 
> Perhaps delay this change until it is more obvious why?

Right, looking at this patch now, I think it is not necessary at the moment
to introduce this macro now. I will take another look and discard this
if it doesn't make sense.

Thanks,
Shameer