[v6] hw/arm/virt: Add support for user-creatable accelerated SMMUv3

[PATCH v6 05/33] hw/arm/smmuv3-accel: Introduce smmuv3 accel device

Posted by Shameer Kolothum 2 months, 2 weeks ago

Set up dedicated PCIIOMMUOps for the accel SMMUv3, since it will need
different callback handling in upcoming patches. This also adds a
CONFIG_ARM_SMMUV3_ACCEL build option so the feature can be disabled
at compile time. Because we now include CONFIG_DEVICES in the header to
check for ARM_SMMUV3_ACCEL, the meson file entry for smmuv3.c needs to
be changed to arm_ss.add.

The “accel” property isn’t user visible yet and it will be introduced in
a later patch once all the supporting pieces are ready.

Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Tested-by: Zhangfei Gao <zhangfei.gao@linaro.org>
Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
---
 hw/arm/Kconfig          |  5 ++++
 hw/arm/meson.build      |  3 ++-
 hw/arm/smmuv3-accel.c   | 59 +++++++++++++++++++++++++++++++++++++++++
 hw/arm/smmuv3-accel.h   | 27 +++++++++++++++++++
 hw/arm/smmuv3.c         |  5 ++++
 include/hw/arm/smmuv3.h |  3 +++
 6 files changed, 101 insertions(+), 1 deletion(-)
 create mode 100644 hw/arm/smmuv3-accel.c
 create mode 100644 hw/arm/smmuv3-accel.h

diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 0cdeb60f1f..702b79a02b 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -12,6 +12,7 @@ config ARM_VIRT
     select ARM_GIC
     select ACPI
     select ARM_SMMUV3
+    select ARM_SMMUV3_ACCEL
     select GPIO_KEY
     select DEVICE_TREE
     select FW_CFG_DMA
@@ -629,6 +630,10 @@ config FSL_IMX8MP_EVK
 config ARM_SMMUV3
     bool
 
+config ARM_SMMUV3_ACCEL
+    bool
+    depends on ARM_SMMUV3 && IOMMUFD
+
 config FSL_IMX6UL
     bool
     default y
diff --git a/hw/arm/meson.build b/hw/arm/meson.build
index aeaf654790..c250487e64 100644
--- a/hw/arm/meson.build
+++ b/hw/arm/meson.build
@@ -84,7 +84,8 @@ arm_common_ss.add(when: 'CONFIG_ARMSSE', if_true: files('armsse.c'))
 arm_common_ss.add(when: 'CONFIG_FSL_IMX7', if_true: files('fsl-imx7.c', 'mcimx7d-sabre.c'))
 arm_common_ss.add(when: 'CONFIG_FSL_IMX8MP', if_true: files('fsl-imx8mp.c'))
 arm_common_ss.add(when: 'CONFIG_FSL_IMX8MP_EVK', if_true: files('imx8mp-evk.c'))
-arm_common_ss.add(when: 'CONFIG_ARM_SMMUV3', if_true: files('smmuv3.c'))
+arm_ss.add(when: 'CONFIG_ARM_SMMUV3', if_true: files('smmuv3.c'))
+arm_ss.add(when: 'CONFIG_ARM_SMMUV3_ACCEL', if_true: files('smmuv3-accel.c'))
 arm_common_ss.add(when: 'CONFIG_FSL_IMX6UL', if_true: files('fsl-imx6ul.c', 'mcimx6ul-evk.c'))
 arm_common_ss.add(when: 'CONFIG_NRF51_SOC', if_true: files('nrf51_soc.c'))
 arm_common_ss.add(when: 'CONFIG_XEN', if_true: files(
diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
new file mode 100644
index 0000000000..99ef0db8c4
--- /dev/null
+++ b/hw/arm/smmuv3-accel.c
@@ -0,0 +1,59 @@
+/*
+ * Copyright (c) 2025 Huawei Technologies R & D (UK) Ltd
+ * Copyright (C) 2025 NVIDIA
+ * Written by Nicolin Chen, Shameer Kolothum
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+
+#include "hw/arm/smmuv3.h"
+#include "smmuv3-accel.h"
+
+static SMMUv3AccelDevice *smmuv3_accel_get_dev(SMMUState *bs, SMMUPciBus *sbus,
+                                               PCIBus *bus, int devfn)
+{
+    SMMUDevice *sdev = sbus->pbdev[devfn];
+    SMMUv3AccelDevice *accel_dev;
+
+    if (sdev) {
+        return container_of(sdev, SMMUv3AccelDevice, sdev);
+    }
+
+    accel_dev = g_new0(SMMUv3AccelDevice, 1);
+    sdev = &accel_dev->sdev;
+
+    sbus->pbdev[devfn] = sdev;
+    smmu_init_sdev(bs, sdev, bus, devfn);
+    return accel_dev;
+}
+
+/*
+ * Find or add an address space for the given PCI device.
+ *
+ * If a device matching @bus and @devfn already exists, return its
+ * corresponding address space. Otherwise, create a new device entry
+ * and initialize address space for it.
+ */
+static AddressSpace *smmuv3_accel_find_add_as(PCIBus *bus, void *opaque,
+                                              int devfn)
+{
+    SMMUState *bs = opaque;
+    SMMUPciBus *sbus = smmu_get_sbus(bs, bus);
+    SMMUv3AccelDevice *accel_dev = smmuv3_accel_get_dev(bs, sbus, bus, devfn);
+    SMMUDevice *sdev = &accel_dev->sdev;
+
+    return &sdev->as;
+}
+
+static const PCIIOMMUOps smmuv3_accel_ops = {
+    .get_address_space = smmuv3_accel_find_add_as,
+};
+
+void smmuv3_accel_init(SMMUv3State *s)
+{
+    SMMUState *bs = ARM_SMMU(s);
+
+    bs->iommu_ops = &smmuv3_accel_ops;
+}
diff --git a/hw/arm/smmuv3-accel.h b/hw/arm/smmuv3-accel.h
new file mode 100644
index 0000000000..0dc6b00d35
--- /dev/null
+++ b/hw/arm/smmuv3-accel.h
@@ -0,0 +1,27 @@
+/*
+ * Copyright (c) 2025 Huawei Technologies R & D (UK) Ltd
+ * Copyright (C) 2025 NVIDIA
+ * Written by Nicolin Chen, Shameer Kolothum
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef HW_ARM_SMMUV3_ACCEL_H
+#define HW_ARM_SMMUV3_ACCEL_H
+
+#include "hw/arm/smmu-common.h"
+#include CONFIG_DEVICES
+
+typedef struct SMMUv3AccelDevice {
+    SMMUDevice sdev;
+} SMMUv3AccelDevice;
+
+#ifdef CONFIG_ARM_SMMUV3_ACCEL
+void smmuv3_accel_init(SMMUv3State *s);
+#else
+static inline void smmuv3_accel_init(SMMUv3State *s)
+{
+}
+#endif
+
+#endif /* HW_ARM_SMMUV3_ACCEL_H */
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index bcf8af8dc7..ef991cb7d8 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -32,6 +32,7 @@
 #include "qapi/error.h"
 
 #include "hw/arm/smmuv3.h"
+#include "smmuv3-accel.h"
 #include "smmuv3-internal.h"
 #include "smmu-internal.h"
 
@@ -1882,6 +1883,10 @@ static void smmu_realize(DeviceState *d, Error **errp)
     SysBusDevice *dev = SYS_BUS_DEVICE(d);
     Error *local_err = NULL;
 
+    if (s->accel) {
+        smmuv3_accel_init(s);
+    }
+
     c->parent_realize(d, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
diff --git a/include/hw/arm/smmuv3.h b/include/hw/arm/smmuv3.h
index d183a62766..bb7076286b 100644
--- a/include/hw/arm/smmuv3.h
+++ b/include/hw/arm/smmuv3.h
@@ -63,6 +63,9 @@ struct SMMUv3State {
     qemu_irq     irq[4];
     QemuMutex mutex;
     char *stage;
+
+    /* SMMU has HW accelerator support for nested S1 + s2 */
+    bool accel;
 };
 
 typedef enum {
-- 
2.43.0

Re: [PATCH v6 05/33] hw/arm/smmuv3-accel: Introduce smmuv3 accel device

Posted by Cédric Le Goater 2 months ago

On 11/20/25 14:21, Shameer Kolothum wrote:
> Set up dedicated PCIIOMMUOps for the accel SMMUv3, since it will need
> different callback handling in upcoming patches. This also adds a
> CONFIG_ARM_SMMUV3_ACCEL build option so the feature can be disabled
> at compile time. Because we now include CONFIG_DEVICES in the header to
> check for ARM_SMMUV3_ACCEL, the meson file entry for smmuv3.c needs to
> be changed to arm_ss.add.
> 
> The “accel” property isn’t user visible yet and it will be introduced in
> a later patch once all the supporting pieces are ready.
> 
> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
> Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
> Reviewed-by: Eric Auger <eric.auger@redhat.com>
> Tested-by: Zhangfei Gao <zhangfei.gao@linaro.org>
> Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
> ---
>   hw/arm/Kconfig          |  5 ++++
>   hw/arm/meson.build      |  3 ++-
>   hw/arm/smmuv3-accel.c   | 59 +++++++++++++++++++++++++++++++++++++++++
>   hw/arm/smmuv3-accel.h   | 27 +++++++++++++++++++
>   hw/arm/smmuv3.c         |  5 ++++
>   include/hw/arm/smmuv3.h |  3 +++
>   6 files changed, 101 insertions(+), 1 deletion(-)
>   create mode 100644 hw/arm/smmuv3-accel.c
>   create mode 100644 hw/arm/smmuv3-accel.h
> 
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index 0cdeb60f1f..702b79a02b 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -12,6 +12,7 @@ config ARM_VIRT
>       select ARM_GIC
>       select ACPI
>       select ARM_SMMUV3
> +    select ARM_SMMUV3_ACCEL
>       select GPIO_KEY
>       select DEVICE_TREE
>       select FW_CFG_DMA
> @@ -629,6 +630,10 @@ config FSL_IMX8MP_EVK
>   config ARM_SMMUV3
>       bool
>   
> +config ARM_SMMUV3_ACCEL
> +    bool
> +    depends on ARM_SMMUV3 && IOMMUFD
> +
>   config FSL_IMX6UL
>       bool
>       default y
> diff --git a/hw/arm/meson.build b/hw/arm/meson.build
> index aeaf654790..c250487e64 100644
> --- a/hw/arm/meson.build
> +++ b/hw/arm/meson.build
> @@ -84,7 +84,8 @@ arm_common_ss.add(when: 'CONFIG_ARMSSE', if_true: files('armsse.c'))
>   arm_common_ss.add(when: 'CONFIG_FSL_IMX7', if_true: files('fsl-imx7.c', 'mcimx7d-sabre.c'))
>   arm_common_ss.add(when: 'CONFIG_FSL_IMX8MP', if_true: files('fsl-imx8mp.c'))
>   arm_common_ss.add(when: 'CONFIG_FSL_IMX8MP_EVK', if_true: files('imx8mp-evk.c'))
> -arm_common_ss.add(when: 'CONFIG_ARM_SMMUV3', if_true: files('smmuv3.c'))
> +arm_ss.add(when: 'CONFIG_ARM_SMMUV3', if_true: files('smmuv3.c'))
> +arm_ss.add(when: 'CONFIG_ARM_SMMUV3_ACCEL', if_true: files('smmuv3-accel.c'))
>   arm_common_ss.add(when: 'CONFIG_FSL_IMX6UL', if_true: files('fsl-imx6ul.c', 'mcimx6ul-evk.c'))
>   arm_common_ss.add(when: 'CONFIG_NRF51_SOC', if_true: files('nrf51_soc.c'))
>   arm_common_ss.add(when: 'CONFIG_XEN', if_true: files(
> diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
> new file mode 100644
> index 0000000000..99ef0db8c4
> --- /dev/null
> +++ b/hw/arm/smmuv3-accel.c
> @@ -0,0 +1,59 @@
> +/*
> + * Copyright (c) 2025 Huawei Technologies R & D (UK) Ltd
> + * Copyright (C) 2025 NVIDIA
> + * Written by Nicolin Chen, Shameer Kolothum
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "qemu/osdep.h"
> +
> +#include "hw/arm/smmuv3.h"
> +#include "smmuv3-accel.h"
> +
> +static SMMUv3AccelDevice *smmuv3_accel_get_dev(SMMUState *bs, SMMUPciBus *sbus,
> +                                               PCIBus *bus, int devfn)
> +{
> +    SMMUDevice *sdev = sbus->pbdev[devfn];
> +    SMMUv3AccelDevice *accel_dev;
> +
> +    if (sdev) {
> +        return container_of(sdev, SMMUv3AccelDevice, sdev);
> +    }
> +
> +    accel_dev = g_new0(SMMUv3AccelDevice, 1);

oh. This is not a QOM object :/

> +    sdev = &accel_dev->sdev;
> +
> +    sbus->pbdev[devfn] = sdev;
> +    smmu_init_sdev(bs, sdev, bus, devfn);
> +    return accel_dev;
> +}
> +
> +/*
> + * Find or add an address space for the given PCI device.
> + *
> + * If a device matching @bus and @devfn already exists, return its
> + * corresponding address space. Otherwise, create a new device entry
> + * and initialize address space for it.
> + */
> +static AddressSpace *smmuv3_accel_find_add_as(PCIBus *bus, void *opaque,
> +                                              int devfn)
> +{
> +    SMMUState *bs = opaque;
> +    SMMUPciBus *sbus = smmu_get_sbus(bs, bus);
> +    SMMUv3AccelDevice *accel_dev = smmuv3_accel_get_dev(bs, sbus, bus, devfn);
> +    SMMUDevice *sdev = &accel_dev->sdev;
> +
> +    return &sdev->as;
> +}
> +
> +static const PCIIOMMUOps smmuv3_accel_ops = {
> +    .get_address_space = smmuv3_accel_find_add_as,
> +};
> +
> +void smmuv3_accel_init(SMMUv3State *s)
> +{
> +    SMMUState *bs = ARM_SMMU(s);
> +
> +    bs->iommu_ops = &smmuv3_accel_ops;

again, I think this should be a sSMMUv3Class attribute.

> +}
> diff --git a/hw/arm/smmuv3-accel.h b/hw/arm/smmuv3-accel.h
> new file mode 100644
> index 0000000000..0dc6b00d35
> --- /dev/null
> +++ b/hw/arm/smmuv3-accel.h
> @@ -0,0 +1,27 @@
> +/*
> + * Copyright (c) 2025 Huawei Technologies R & D (UK) Ltd
> + * Copyright (C) 2025 NVIDIA
> + * Written by Nicolin Chen, Shameer Kolothum
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#ifndef HW_ARM_SMMUV3_ACCEL_H
> +#define HW_ARM_SMMUV3_ACCEL_H
> +
> +#include "hw/arm/smmu-common.h"
> +#include CONFIG_DEVICES
> +
> +typedef struct SMMUv3AccelDevice {
> +    SMMUDevice sdev;
> +} SMMUv3AccelDevice;
> +
> +#ifdef CONFIG_ARM_SMMUV3_ACCEL
> +void smmuv3_accel_init(SMMUv3State *s);
> +#else
> +static inline void smmuv3_accel_init(SMMUv3State *s)
> +{
> +}
> +#endif
> +
> +#endif /* HW_ARM_SMMUV3_ACCEL_H */
> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> index bcf8af8dc7..ef991cb7d8 100644
> --- a/hw/arm/smmuv3.c
> +++ b/hw/arm/smmuv3.c
> @@ -32,6 +32,7 @@
>   #include "qapi/error.h"
>   
>   #include "hw/arm/smmuv3.h"
> +#include "smmuv3-accel.h"
>   #include "smmuv3-internal.h"
>   #include "smmu-internal.h"
>   
> @@ -1882,6 +1883,10 @@ static void smmu_realize(DeviceState *d, Error **errp)
>       SysBusDevice *dev = SYS_BUS_DEVICE(d);
>       Error *local_err = NULL;
>   
> +    if (s->accel) {
> +        smmuv3_accel_init(s);
> +    }
> +
>       c->parent_realize(d, &local_err);
>       if (local_err) {
>           error_propagate(errp, local_err);
> diff --git a/include/hw/arm/smmuv3.h b/include/hw/arm/smmuv3.h
> index d183a62766..bb7076286b 100644
> --- a/include/hw/arm/smmuv3.h
> +++ b/include/hw/arm/smmuv3.h
> @@ -63,6 +63,9 @@ struct SMMUv3State {
>       qemu_irq     irq[4];
>       QemuMutex mutex;
>       char *stage;
> +
> +    /* SMMU has HW accelerator support for nested S1 + s2 */
> +    bool accel;

Have you considered modeling with a QOM object instead ?

Thanks,

C.

>   };
>   
>   typedef enum {

RE: [PATCH v6 05/33] hw/arm/smmuv3-accel: Introduce smmuv3 accel device

Posted by Shameer Kolothum 1 month, 4 weeks ago

Hi Cédric,

> -----Original Message-----
> From: Cédric Le Goater <clg@redhat.com>
> Sent: 11 December 2025 12:55
> To: Shameer Kolothum <skolothumtho@nvidia.com>; qemu-
> arm@nongnu.org; qemu-devel@nongnu.org
> Cc: eric.auger@redhat.com; peter.maydell@linaro.org; Jason Gunthorpe
> <jgg@nvidia.com>; Nicolin Chen <nicolinc@nvidia.com>;
> ddutile@redhat.com; berrange@redhat.com; Nathan Chen
> <nathanc@nvidia.com>; Matt Ochs <mochs@nvidia.com>;
> smostafa@google.com; wangzhou1@hisilicon.com;
> jiangkunkun@huawei.com; jonathan.cameron@huawei.com;
> zhangfei.gao@linaro.org; zhenzhong.duan@intel.com; yi.l.liu@intel.com;
> Krishnakant Jaju <kjaju@nvidia.com>
> Subject: Re: [PATCH v6 05/33] hw/arm/smmuv3-accel: Introduce smmuv3
> accel device
> 
> External email: Use caution opening links or attachments
> 
> 
> On 11/20/25 14:21, Shameer Kolothum wrote:
> > Set up dedicated PCIIOMMUOps for the accel SMMUv3, since it will need
> > different callback handling in upcoming patches. This also adds a
> > CONFIG_ARM_SMMUV3_ACCEL build option so the feature can be disabled
> > at compile time. Because we now include CONFIG_DEVICES in the header to
> > check for ARM_SMMUV3_ACCEL, the meson file entry for smmuv3.c needs
> to
> > be changed to arm_ss.add.
> >
> > The “accel” property isn’t user visible yet and it will be introduced in
> > a later patch once all the supporting pieces are ready.
> >
> > Signed-off-by: Shameer Kolothum
> <shameerali.kolothum.thodi@huawei.com>
> > Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
> > Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
> > Reviewed-by: Eric Auger <eric.auger@redhat.com>
> > Tested-by: Zhangfei Gao <zhangfei.gao@linaro.org>
> > Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
> > ---
> >   hw/arm/Kconfig          |  5 ++++
> >   hw/arm/meson.build      |  3 ++-
> >   hw/arm/smmuv3-accel.c   | 59
> +++++++++++++++++++++++++++++++++++++++++
> >   hw/arm/smmuv3-accel.h   | 27 +++++++++++++++++++
> >   hw/arm/smmuv3.c         |  5 ++++
> >   include/hw/arm/smmuv3.h |  3 +++
> >   6 files changed, 101 insertions(+), 1 deletion(-)
> >   create mode 100644 hw/arm/smmuv3-accel.c
> >   create mode 100644 hw/arm/smmuv3-accel.h
> >
> > diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> > index 0cdeb60f1f..702b79a02b 100644
> > --- a/hw/arm/Kconfig
> > +++ b/hw/arm/Kconfig
> > @@ -12,6 +12,7 @@ config ARM_VIRT
> >       select ARM_GIC
> >       select ACPI
> >       select ARM_SMMUV3
> > +    select ARM_SMMUV3_ACCEL
> >       select GPIO_KEY
> >       select DEVICE_TREE
> >       select FW_CFG_DMA
> > @@ -629,6 +630,10 @@ config FSL_IMX8MP_EVK
> >   config ARM_SMMUV3
> >       bool
> >
> > +config ARM_SMMUV3_ACCEL
> > +    bool
> > +    depends on ARM_SMMUV3 && IOMMUFD
> > +
> >   config FSL_IMX6UL
> >       bool
> >       default y
> > diff --git a/hw/arm/meson.build b/hw/arm/meson.build
> > index aeaf654790..c250487e64 100644
> > --- a/hw/arm/meson.build
> > +++ b/hw/arm/meson.build
> > @@ -84,7 +84,8 @@ arm_common_ss.add(when: 'CONFIG_ARMSSE',
> if_true: files('armsse.c'))
> >   arm_common_ss.add(when: 'CONFIG_FSL_IMX7', if_true: files('fsl-imx7.c',
> 'mcimx7d-sabre.c'))
> >   arm_common_ss.add(when: 'CONFIG_FSL_IMX8MP', if_true: files('fsl-
> imx8mp.c'))
> >   arm_common_ss.add(when: 'CONFIG_FSL_IMX8MP_EVK', if_true:
> files('imx8mp-evk.c'))
> > -arm_common_ss.add(when: 'CONFIG_ARM_SMMUV3', if_true:
> files('smmuv3.c'))
> > +arm_ss.add(when: 'CONFIG_ARM_SMMUV3', if_true: files('smmuv3.c'))
> > +arm_ss.add(when: 'CONFIG_ARM_SMMUV3_ACCEL', if_true:
> files('smmuv3-accel.c'))
> >   arm_common_ss.add(when: 'CONFIG_FSL_IMX6UL', if_true: files('fsl-
> imx6ul.c', 'mcimx6ul-evk.c'))
> >   arm_common_ss.add(when: 'CONFIG_NRF51_SOC', if_true:
> files('nrf51_soc.c'))
> >   arm_common_ss.add(when: 'CONFIG_XEN', if_true: files(
> > diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
> > new file mode 100644
> > index 0000000000..99ef0db8c4
> > --- /dev/null
> > +++ b/hw/arm/smmuv3-accel.c
> > @@ -0,0 +1,59 @@
> > +/*
> > + * Copyright (c) 2025 Huawei Technologies R & D (UK) Ltd
> > + * Copyright (C) 2025 NVIDIA
> > + * Written by Nicolin Chen, Shameer Kolothum
> > + *
> > + * SPDX-License-Identifier: GPL-2.0-or-later
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +
> > +#include "hw/arm/smmuv3.h"
> > +#include "smmuv3-accel.h"
> > +
> > +static SMMUv3AccelDevice *smmuv3_accel_get_dev(SMMUState *bs,
> SMMUPciBus *sbus,
> > +                                               PCIBus *bus, int devfn)
> > +{
> > +    SMMUDevice *sdev = sbus->pbdev[devfn];
> > +    SMMUv3AccelDevice *accel_dev;
> > +
> > +    if (sdev) {
> > +        return container_of(sdev, SMMUv3AccelDevice, sdev);
> > +    }
> > +
> > +    accel_dev = g_new0(SMMUv3AccelDevice, 1);
> 
> oh. This is not a QOM object :/

Right.

> 
> > +    sdev = &accel_dev->sdev;
> > +
> > +    sbus->pbdev[devfn] = sdev;
> > +    smmu_init_sdev(bs, sdev, bus, devfn);
> > +    return accel_dev;
> > +}
> > +
> > +/*
> > + * Find or add an address space for the given PCI device.
> > + *
> > + * If a device matching @bus and @devfn already exists, return its
> > + * corresponding address space. Otherwise, create a new device entry
> > + * and initialize address space for it.
> > + */
> > +static AddressSpace *smmuv3_accel_find_add_as(PCIBus *bus, void
> *opaque,
> > +                                              int devfn)
> > +{
> > +    SMMUState *bs = opaque;
> > +    SMMUPciBus *sbus = smmu_get_sbus(bs, bus);
> > +    SMMUv3AccelDevice *accel_dev = smmuv3_accel_get_dev(bs, sbus,
> bus, devfn);
> > +    SMMUDevice *sdev = &accel_dev->sdev;
> > +
> > +    return &sdev->as;
> > +}
> > +
> > +static const PCIIOMMUOps smmuv3_accel_ops = {
> > +    .get_address_space = smmuv3_accel_find_add_as,
> > +};
> > +
> > +void smmuv3_accel_init(SMMUv3State *s)
> > +{
> > +    SMMUState *bs = ARM_SMMU(s);
> > +
> > +    bs->iommu_ops = &smmuv3_accel_ops;
> 
> again, I think this should be a sSMMUv3Class attribute.

See below.
> 
> > +}
> > diff --git a/hw/arm/smmuv3-accel.h b/hw/arm/smmuv3-accel.h
> > new file mode 100644
> > index 0000000000..0dc6b00d35
> > --- /dev/null
> > +++ b/hw/arm/smmuv3-accel.h
> > @@ -0,0 +1,27 @@
> > +/*
> > + * Copyright (c) 2025 Huawei Technologies R & D (UK) Ltd
> > + * Copyright (C) 2025 NVIDIA
> > + * Written by Nicolin Chen, Shameer Kolothum
> > + *
> > + * SPDX-License-Identifier: GPL-2.0-or-later
> > + */
> > +
> > +#ifndef HW_ARM_SMMUV3_ACCEL_H
> > +#define HW_ARM_SMMUV3_ACCEL_H
> > +
> > +#include "hw/arm/smmu-common.h"
> > +#include CONFIG_DEVICES
> > +
> > +typedef struct SMMUv3AccelDevice {
> > +    SMMUDevice sdev;
> > +} SMMUv3AccelDevice;
> > +
> > +#ifdef CONFIG_ARM_SMMUV3_ACCEL
> > +void smmuv3_accel_init(SMMUv3State *s);
> > +#else
> > +static inline void smmuv3_accel_init(SMMUv3State *s)
> > +{
> > +}
> > +#endif
> > +
> > +#endif /* HW_ARM_SMMUV3_ACCEL_H */
> > diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> > index bcf8af8dc7..ef991cb7d8 100644
> > --- a/hw/arm/smmuv3.c
> > +++ b/hw/arm/smmuv3.c
> > @@ -32,6 +32,7 @@
> >   #include "qapi/error.h"
> >
> >   #include "hw/arm/smmuv3.h"
> > +#include "smmuv3-accel.h"
> >   #include "smmuv3-internal.h"
> >   #include "smmu-internal.h"
> >
> > @@ -1882,6 +1883,10 @@ static void smmu_realize(DeviceState *d, Error
> **errp)
> >       SysBusDevice *dev = SYS_BUS_DEVICE(d);
> >       Error *local_err = NULL;
> >
> > +    if (s->accel) {
> > +        smmuv3_accel_init(s);
> > +    }
> > +
> >       c->parent_realize(d, &local_err);
> >       if (local_err) {
> >           error_propagate(errp, local_err);
> > diff --git a/include/hw/arm/smmuv3.h b/include/hw/arm/smmuv3.h
> > index d183a62766..bb7076286b 100644
> > --- a/include/hw/arm/smmuv3.h
> > +++ b/include/hw/arm/smmuv3.h
> > @@ -63,6 +63,9 @@ struct SMMUv3State {
> >       qemu_irq     irq[4];
> >       QemuMutex mutex;
> >       char *stage;
> > +
> > +    /* SMMU has HW accelerator support for nested S1 + s2 */
> > +    bool accel;
> 
> Have you considered modeling with a QOM object instead ?

A bit of history on this:

The SMMUv3 accel support was introduced first as a separate device,
-device arm-smmuv3-accel

https://lore.kernel.org/qemu-devel/20250311141045.66620-4-shameerali.kolothum.thodi@huawei.com/

However, the general consensus at that time was that we should instead
model it on the -device arm-smmuv3 itself, with an added "accel" property.

Eric had also suggested making use of something similar to the
TYPE_VFIO_IOMMU_IOMMUFD / LEGACY classes for selecting the iommu_ops.
https://lore.kernel.org/qemu-devel/1105d100-dd1e-4aca-b518-50f903967416@redhat.com/

In RFCv3, I did introduce a TYPE_ARM_SMMUV3_ACCEL object class, but we
later found that it wasn’t doing much beyond helping to retrieve the iommu_ops
based on the object type, so decided to drop it.
https://lore.kernel.org/qemu-devel/71ca9132-8deb-4f57-abb0-2bcc0fe93ae9@redhat.com/

From your feedback, I understand that you would like to revisit that approach
again. Just to make sure I get this right, is the SMMUv3AccelClass you have in
mind meant to be an abstract object, like HOST_IOMMU_DEVICE, or are
you suggesting another child device object?

Please let me know if there is an example/precedent, I can look at for a similar
object model in QEMU.

Thanks,
Shameer

Re: [PATCH v6 05/33] hw/arm/smmuv3-accel: Introduce smmuv3 accel device

Posted by Cédric Le Goater 1 month, 3 weeks ago

Hello Shameer,

On 12/12/25 06:48, Shameer Kolothum wrote:
> Hi Cédric,
> 
>> -----Original Message-----
>> From: Cédric Le Goater <clg@redhat.com>
>> Sent: 11 December 2025 12:55
>> To: Shameer Kolothum <skolothumtho@nvidia.com>; qemu-
>> arm@nongnu.org; qemu-devel@nongnu.org
>> Cc: eric.auger@redhat.com; peter.maydell@linaro.org; Jason Gunthorpe
>> <jgg@nvidia.com>; Nicolin Chen <nicolinc@nvidia.com>;
>> ddutile@redhat.com; berrange@redhat.com; Nathan Chen
>> <nathanc@nvidia.com>; Matt Ochs <mochs@nvidia.com>;
>> smostafa@google.com; wangzhou1@hisilicon.com;
>> jiangkunkun@huawei.com; jonathan.cameron@huawei.com;
>> zhangfei.gao@linaro.org; zhenzhong.duan@intel.com; yi.l.liu@intel.com;
>> Krishnakant Jaju <kjaju@nvidia.com>
>> Subject: Re: [PATCH v6 05/33] hw/arm/smmuv3-accel: Introduce smmuv3
>> accel device
>>
>> External email: Use caution opening links or attachments
>>
>>
>> On 11/20/25 14:21, Shameer Kolothum wrote:
>>> Set up dedicated PCIIOMMUOps for the accel SMMUv3, since it will need
>>> different callback handling in upcoming patches. This also adds a
>>> CONFIG_ARM_SMMUV3_ACCEL build option so the feature can be disabled
>>> at compile time. Because we now include CONFIG_DEVICES in the header to
>>> check for ARM_SMMUV3_ACCEL, the meson file entry for smmuv3.c needs
>> to
>>> be changed to arm_ss.add.
>>>
>>> The “accel” property isn’t user visible yet and it will be introduced in
>>> a later patch once all the supporting pieces are ready.
>>>
>>> Signed-off-by: Shameer Kolothum
>> <shameerali.kolothum.thodi@huawei.com>
>>> Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
>>> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
>>> Reviewed-by: Eric Auger <eric.auger@redhat.com>
>>> Tested-by: Zhangfei Gao <zhangfei.gao@linaro.org>
>>> Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
>>> ---
>>>    hw/arm/Kconfig          |  5 ++++
>>>    hw/arm/meson.build      |  3 ++-
>>>    hw/arm/smmuv3-accel.c   | 59
>> +++++++++++++++++++++++++++++++++++++++++
>>>    hw/arm/smmuv3-accel.h   | 27 +++++++++++++++++++
>>>    hw/arm/smmuv3.c         |  5 ++++
>>>    include/hw/arm/smmuv3.h |  3 +++
>>>    6 files changed, 101 insertions(+), 1 deletion(-)
>>>    create mode 100644 hw/arm/smmuv3-accel.c
>>>    create mode 100644 hw/arm/smmuv3-accel.h
>>>
>>> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
>>> index 0cdeb60f1f..702b79a02b 100644
>>> --- a/hw/arm/Kconfig
>>> +++ b/hw/arm/Kconfig
>>> @@ -12,6 +12,7 @@ config ARM_VIRT
>>>        select ARM_GIC
>>>        select ACPI
>>>        select ARM_SMMUV3
>>> +    select ARM_SMMUV3_ACCEL
>>>        select GPIO_KEY
>>>        select DEVICE_TREE
>>>        select FW_CFG_DMA
>>> @@ -629,6 +630,10 @@ config FSL_IMX8MP_EVK
>>>    config ARM_SMMUV3
>>>        bool
>>>
>>> +config ARM_SMMUV3_ACCEL
>>> +    bool
>>> +    depends on ARM_SMMUV3 && IOMMUFD
>>> +
>>>    config FSL_IMX6UL
>>>        bool
>>>        default y
>>> diff --git a/hw/arm/meson.build b/hw/arm/meson.build
>>> index aeaf654790..c250487e64 100644
>>> --- a/hw/arm/meson.build
>>> +++ b/hw/arm/meson.build
>>> @@ -84,7 +84,8 @@ arm_common_ss.add(when: 'CONFIG_ARMSSE',
>> if_true: files('armsse.c'))
>>>    arm_common_ss.add(when: 'CONFIG_FSL_IMX7', if_true: files('fsl-imx7.c',
>> 'mcimx7d-sabre.c'))
>>>    arm_common_ss.add(when: 'CONFIG_FSL_IMX8MP', if_true: files('fsl-
>> imx8mp.c'))
>>>    arm_common_ss.add(when: 'CONFIG_FSL_IMX8MP_EVK', if_true:
>> files('imx8mp-evk.c'))
>>> -arm_common_ss.add(when: 'CONFIG_ARM_SMMUV3', if_true:
>> files('smmuv3.c'))
>>> +arm_ss.add(when: 'CONFIG_ARM_SMMUV3', if_true: files('smmuv3.c'))
>>> +arm_ss.add(when: 'CONFIG_ARM_SMMUV3_ACCEL', if_true:
>> files('smmuv3-accel.c'))
>>>    arm_common_ss.add(when: 'CONFIG_FSL_IMX6UL', if_true: files('fsl-
>> imx6ul.c', 'mcimx6ul-evk.c'))
>>>    arm_common_ss.add(when: 'CONFIG_NRF51_SOC', if_true:
>> files('nrf51_soc.c'))
>>>    arm_common_ss.add(when: 'CONFIG_XEN', if_true: files(
>>> diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
>>> new file mode 100644
>>> index 0000000000..99ef0db8c4
>>> --- /dev/null
>>> +++ b/hw/arm/smmuv3-accel.c
>>> @@ -0,0 +1,59 @@
>>> +/*
>>> + * Copyright (c) 2025 Huawei Technologies R & D (UK) Ltd
>>> + * Copyright (C) 2025 NVIDIA
>>> + * Written by Nicolin Chen, Shameer Kolothum
>>> + *
>>> + * SPDX-License-Identifier: GPL-2.0-or-later
>>> + */
>>> +
>>> +#include "qemu/osdep.h"
>>> +
>>> +#include "hw/arm/smmuv3.h"
>>> +#include "smmuv3-accel.h"
>>> +
>>> +static SMMUv3AccelDevice *smmuv3_accel_get_dev(SMMUState *bs,
>> SMMUPciBus *sbus,
>>> +                                               PCIBus *bus, int devfn)
>>> +{
>>> +    SMMUDevice *sdev = sbus->pbdev[devfn];
>>> +    SMMUv3AccelDevice *accel_dev;
>>> +
>>> +    if (sdev) {
>>> +        return container_of(sdev, SMMUv3AccelDevice, sdev);
>>> +    }
>>> +
>>> +    accel_dev = g_new0(SMMUv3AccelDevice, 1);
>>
>> oh. This is not a QOM object :/
> 
> Right.
> 
>>
>>> +    sdev = &accel_dev->sdev;
>>> +
>>> +    sbus->pbdev[devfn] = sdev;
>>> +    smmu_init_sdev(bs, sdev, bus, devfn);
>>> +    return accel_dev;
>>> +}
>>> +
>>> +/*
>>> + * Find or add an address space for the given PCI device.
>>> + *
>>> + * If a device matching @bus and @devfn already exists, return its
>>> + * corresponding address space. Otherwise, create a new device entry
>>> + * and initialize address space for it.
>>> + */
>>> +static AddressSpace *smmuv3_accel_find_add_as(PCIBus *bus, void
>> *opaque,
>>> +                                              int devfn)
>>> +{
>>> +    SMMUState *bs = opaque;
>>> +    SMMUPciBus *sbus = smmu_get_sbus(bs, bus);
>>> +    SMMUv3AccelDevice *accel_dev = smmuv3_accel_get_dev(bs, sbus,
>> bus, devfn);
>>> +    SMMUDevice *sdev = &accel_dev->sdev;
>>> +
>>> +    return &sdev->as;
>>> +}
>>> +
>>> +static const PCIIOMMUOps smmuv3_accel_ops = {
>>> +    .get_address_space = smmuv3_accel_find_add_as,
>>> +};
>>> +
>>> +void smmuv3_accel_init(SMMUv3State *s)
>>> +{
>>> +    SMMUState *bs = ARM_SMMU(s);
>>> +
>>> +    bs->iommu_ops = &smmuv3_accel_ops;
>>
>> again, I think this should be a sSMMUv3Class attribute.
> 
> See below.
>>
>>> +}
>>> diff --git a/hw/arm/smmuv3-accel.h b/hw/arm/smmuv3-accel.h
>>> new file mode 100644
>>> index 0000000000..0dc6b00d35
>>> --- /dev/null
>>> +++ b/hw/arm/smmuv3-accel.h
>>> @@ -0,0 +1,27 @@
>>> +/*
>>> + * Copyright (c) 2025 Huawei Technologies R & D (UK) Ltd
>>> + * Copyright (C) 2025 NVIDIA
>>> + * Written by Nicolin Chen, Shameer Kolothum
>>> + *
>>> + * SPDX-License-Identifier: GPL-2.0-or-later
>>> + */
>>> +
>>> +#ifndef HW_ARM_SMMUV3_ACCEL_H
>>> +#define HW_ARM_SMMUV3_ACCEL_H
>>> +
>>> +#include "hw/arm/smmu-common.h"
>>> +#include CONFIG_DEVICES
>>> +
>>> +typedef struct SMMUv3AccelDevice {
>>> +    SMMUDevice sdev;
>>> +} SMMUv3AccelDevice;
>>> +
>>> +#ifdef CONFIG_ARM_SMMUV3_ACCEL
>>> +void smmuv3_accel_init(SMMUv3State *s);
>>> +#else
>>> +static inline void smmuv3_accel_init(SMMUv3State *s)
>>> +{
>>> +}
>>> +#endif
>>> +
>>> +#endif /* HW_ARM_SMMUV3_ACCEL_H */
>>> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
>>> index bcf8af8dc7..ef991cb7d8 100644
>>> --- a/hw/arm/smmuv3.c
>>> +++ b/hw/arm/smmuv3.c
>>> @@ -32,6 +32,7 @@
>>>    #include "qapi/error.h"
>>>
>>>    #include "hw/arm/smmuv3.h"
>>> +#include "smmuv3-accel.h"
>>>    #include "smmuv3-internal.h"
>>>    #include "smmu-internal.h"
>>>
>>> @@ -1882,6 +1883,10 @@ static void smmu_realize(DeviceState *d, Error
>> **errp)
>>>        SysBusDevice *dev = SYS_BUS_DEVICE(d);
>>>        Error *local_err = NULL;
>>>
>>> +    if (s->accel) {
>>> +        smmuv3_accel_init(s);
>>> +    }
>>> +
>>>        c->parent_realize(d, &local_err);
>>>        if (local_err) {
>>>            error_propagate(errp, local_err);
>>> diff --git a/include/hw/arm/smmuv3.h b/include/hw/arm/smmuv3.h
>>> index d183a62766..bb7076286b 100644
>>> --- a/include/hw/arm/smmuv3.h
>>> +++ b/include/hw/arm/smmuv3.h
>>> @@ -63,6 +63,9 @@ struct SMMUv3State {
>>>        qemu_irq     irq[4];
>>>        QemuMutex mutex;
>>>        char *stage;
>>> +
>>> +    /* SMMU has HW accelerator support for nested S1 + s2 */
>>> +    bool accel;
>>
>> Have you considered modeling with a QOM object instead ?
> 
> A bit of history on this:
> 
> The SMMUv3 accel support was introduced first as a separate device,
> -device arm-smmuv3-accel
> 
> https://lore.kernel.org/qemu-devel/20250311141045.66620-4-shameerali.kolothum.thodi@huawei.com/
> 
> However, the general consensus at that time was that we should instead
> model it on the -device arm-smmuv3 itself, with an added "accel" property.
> 
> Eric had also suggested making use of something similar to the
> TYPE_VFIO_IOMMU_IOMMUFD / LEGACY classes for selecting the iommu_ops.
> https://lore.kernel.org/qemu-devel/1105d100-dd1e-4aca-b518-50f903967416@redhat.com/
> 
> In RFCv3, I did introduce a TYPE_ARM_SMMUV3_ACCEL object class, but we
> later found that it wasn’t doing much beyond helping to retrieve the iommu_ops
> based on the object type, so decided to drop it.
> https://lore.kernel.org/qemu-devel/71ca9132-8deb-4f57-abb0-2bcc0fe93ae9@redhat.com/
> 
>  From your feedback, I understand that you would like to revisit that approach
> again. Just to make sure I get this right, is the SMMUv3AccelClass you have in
> mind meant to be an abstract object, like HOST_IOMMU_DEVICE, or are
> you suggesting another child device object?

HOST_IOMMU_DEVICE are for host IOMMU backends (VFIO IOMMU Type1
legacy and IOMMUFD). I was imagining an "arm-smmuv3" children class
for acceleration.

I understand I am jumping a bit late in the discussion. Sorry about
that and the late reply also.

I have been experimenting with your series on other systems and on
an ARM system with MLX5 VFs and an L4 GPU. I had to fake CANWBS
support in the host kernel to make progress. On a side note, I also
pulled in a patch from Nicolin to add dmabuf support and it seemed
to behave ok. This would require more in-depth P2P testing. That's
another topic we should discuss in the QEMU 11.0 cycle.


> Please let me know if there is an example/precedent, I can look at for a similar
> object model in QEMU.

Now regarding modeling, it is more or less a design choice. But while
I scanned through the code, I felt there was some problems that could
be solved in a cleaner way with a sub class. The changes are mostly
about moving code in different places and changing/removing the
"if (s->accel) parts". Don't change everything now, I will check RFCv3.

Nevertheless, they are several more issues, build breakages and also
runtime breakage that should be addressed. Let's do that first.

Furthermore, an (idealistic) design principle of QEMU that we tend to
forget is to offer acceleration when possible and else fallback to
emulation, in order to guarantee parity of features between different
hardware. I expected the "arm-smmuv3" device to decide to operate in
accelerated mode when ever possible, i.e when the IOMMUFD backend
(the SMMUV3 + the FW) offers the right level of functionality. This
would allow the machine to maintain a constant view of the sMMUv3
device and facilitate its integration with management layers and
ease migration too.

I understand this is complex, it has been done for some interrupt
controllers, and this is not the approach of the series.

That said, what about integration with libvirt ?


Thanks,

C.

Re: [PATCH v6 05/33] hw/arm/smmuv3-accel: Introduce smmuv3 accel device

Posted by Eric Auger 1 month ago

Hi Cédric, Shameer,

On 12/17/25 6:38 PM, Cédric Le Goater wrote:
> Hello Shameer,
>
> On 12/12/25 06:48, Shameer Kolothum wrote:
>> Hi Cédric,
>>
>>> -----Original Message-----
>>> From: Cédric Le Goater <clg@redhat.com>
>>> Sent: 11 December 2025 12:55
>>> To: Shameer Kolothum <skolothumtho@nvidia.com>; qemu-
>>> arm@nongnu.org; qemu-devel@nongnu.org
>>> Cc: eric.auger@redhat.com; peter.maydell@linaro.org; Jason Gunthorpe
>>> <jgg@nvidia.com>; Nicolin Chen <nicolinc@nvidia.com>;
>>> ddutile@redhat.com; berrange@redhat.com; Nathan Chen
>>> <nathanc@nvidia.com>; Matt Ochs <mochs@nvidia.com>;
>>> smostafa@google.com; wangzhou1@hisilicon.com;
>>> jiangkunkun@huawei.com; jonathan.cameron@huawei.com;
>>> zhangfei.gao@linaro.org; zhenzhong.duan@intel.com; yi.l.liu@intel.com;
>>> Krishnakant Jaju <kjaju@nvidia.com>
>>> Subject: Re: [PATCH v6 05/33] hw/arm/smmuv3-accel: Introduce smmuv3
>>> accel device
>>>
>>> External email: Use caution opening links or attachments
>>>
>>>
>>> On 11/20/25 14:21, Shameer Kolothum wrote:
>>>> Set up dedicated PCIIOMMUOps for the accel SMMUv3, since it will need
>>>> different callback handling in upcoming patches. This also adds a
>>>> CONFIG_ARM_SMMUV3_ACCEL build option so the feature can be disabled
>>>> at compile time. Because we now include CONFIG_DEVICES in the
>>>> header to
>>>> check for ARM_SMMUV3_ACCEL, the meson file entry for smmuv3.c needs
>>> to
>>>> be changed to arm_ss.add.
>>>>
>>>> The “accel” property isn’t user visible yet and it will be
>>>> introduced in
>>>> a later patch once all the supporting pieces are ready.
>>>>
>>>> Signed-off-by: Shameer Kolothum
>>> <shameerali.kolothum.thodi@huawei.com>
>>>> Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
>>>> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
>>>> Reviewed-by: Eric Auger <eric.auger@redhat.com>
>>>> Tested-by: Zhangfei Gao <zhangfei.gao@linaro.org>
>>>> Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
>>>> ---
>>>>    hw/arm/Kconfig          |  5 ++++
>>>>    hw/arm/meson.build      |  3 ++-
>>>>    hw/arm/smmuv3-accel.c   | 59
>>> +++++++++++++++++++++++++++++++++++++++++
>>>>    hw/arm/smmuv3-accel.h   | 27 +++++++++++++++++++
>>>>    hw/arm/smmuv3.c         |  5 ++++
>>>>    include/hw/arm/smmuv3.h |  3 +++
>>>>    6 files changed, 101 insertions(+), 1 deletion(-)
>>>>    create mode 100644 hw/arm/smmuv3-accel.c
>>>>    create mode 100644 hw/arm/smmuv3-accel.h
>>>>
>>>> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
>>>> index 0cdeb60f1f..702b79a02b 100644
>>>> --- a/hw/arm/Kconfig
>>>> +++ b/hw/arm/Kconfig
>>>> @@ -12,6 +12,7 @@ config ARM_VIRT
>>>>        select ARM_GIC
>>>>        select ACPI
>>>>        select ARM_SMMUV3
>>>> +    select ARM_SMMUV3_ACCEL
>>>>        select GPIO_KEY
>>>>        select DEVICE_TREE
>>>>        select FW_CFG_DMA
>>>> @@ -629,6 +630,10 @@ config FSL_IMX8MP_EVK
>>>>    config ARM_SMMUV3
>>>>        bool
>>>>
>>>> +config ARM_SMMUV3_ACCEL
>>>> +    bool
>>>> +    depends on ARM_SMMUV3 && IOMMUFD
>>>> +
>>>>    config FSL_IMX6UL
>>>>        bool
>>>>        default y
>>>> diff --git a/hw/arm/meson.build b/hw/arm/meson.build
>>>> index aeaf654790..c250487e64 100644
>>>> --- a/hw/arm/meson.build
>>>> +++ b/hw/arm/meson.build
>>>> @@ -84,7 +84,8 @@ arm_common_ss.add(when: 'CONFIG_ARMSSE',
>>> if_true: files('armsse.c'))
>>>>    arm_common_ss.add(when: 'CONFIG_FSL_IMX7', if_true:
>>>> files('fsl-imx7.c',
>>> 'mcimx7d-sabre.c'))
>>>>    arm_common_ss.add(when: 'CONFIG_FSL_IMX8MP', if_true: files('fsl-
>>> imx8mp.c'))
>>>>    arm_common_ss.add(when: 'CONFIG_FSL_IMX8MP_EVK', if_true:
>>> files('imx8mp-evk.c'))
>>>> -arm_common_ss.add(when: 'CONFIG_ARM_SMMUV3', if_true:
>>> files('smmuv3.c'))
>>>> +arm_ss.add(when: 'CONFIG_ARM_SMMUV3', if_true: files('smmuv3.c'))
>>>> +arm_ss.add(when: 'CONFIG_ARM_SMMUV3_ACCEL', if_true:
>>> files('smmuv3-accel.c'))
>>>>    arm_common_ss.add(when: 'CONFIG_FSL_IMX6UL', if_true: files('fsl-
>>> imx6ul.c', 'mcimx6ul-evk.c'))
>>>>    arm_common_ss.add(when: 'CONFIG_NRF51_SOC', if_true:
>>> files('nrf51_soc.c'))
>>>>    arm_common_ss.add(when: 'CONFIG_XEN', if_true: files(
>>>> diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
>>>> new file mode 100644
>>>> index 0000000000..99ef0db8c4
>>>> --- /dev/null
>>>> +++ b/hw/arm/smmuv3-accel.c
>>>> @@ -0,0 +1,59 @@
>>>> +/*
>>>> + * Copyright (c) 2025 Huawei Technologies R & D (UK) Ltd
>>>> + * Copyright (C) 2025 NVIDIA
>>>> + * Written by Nicolin Chen, Shameer Kolothum
>>>> + *
>>>> + * SPDX-License-Identifier: GPL-2.0-or-later
>>>> + */
>>>> +
>>>> +#include "qemu/osdep.h"
>>>> +
>>>> +#include "hw/arm/smmuv3.h"
>>>> +#include "smmuv3-accel.h"
>>>> +
>>>> +static SMMUv3AccelDevice *smmuv3_accel_get_dev(SMMUState *bs,
>>> SMMUPciBus *sbus,
>>>> +                                               PCIBus *bus, int
>>>> devfn)
>>>> +{
>>>> +    SMMUDevice *sdev = sbus->pbdev[devfn];
>>>> +    SMMUv3AccelDevice *accel_dev;
>>>> +
>>>> +    if (sdev) {
>>>> +        return container_of(sdev, SMMUv3AccelDevice, sdev);
>>>> +    }
>>>> +
>>>> +    accel_dev = g_new0(SMMUv3AccelDevice, 1);
>>>
>>> oh. This is not a QOM object :/
>>
>> Right.
>>
>>>
>>>> +    sdev = &accel_dev->sdev;
>>>> +
>>>> +    sbus->pbdev[devfn] = sdev;
>>>> +    smmu_init_sdev(bs, sdev, bus, devfn);
>>>> +    return accel_dev;
>>>> +}
>>>> +
>>>> +/*
>>>> + * Find or add an address space for the given PCI device.
>>>> + *
>>>> + * If a device matching @bus and @devfn already exists, return its
>>>> + * corresponding address space. Otherwise, create a new device entry
>>>> + * and initialize address space for it.
>>>> + */
>>>> +static AddressSpace *smmuv3_accel_find_add_as(PCIBus *bus, void
>>> *opaque,
>>>> +                                              int devfn)
>>>> +{
>>>> +    SMMUState *bs = opaque;
>>>> +    SMMUPciBus *sbus = smmu_get_sbus(bs, bus);
>>>> +    SMMUv3AccelDevice *accel_dev = smmuv3_accel_get_dev(bs, sbus,
>>> bus, devfn);
>>>> +    SMMUDevice *sdev = &accel_dev->sdev;
>>>> +
>>>> +    return &sdev->as;
>>>> +}
>>>> +
>>>> +static const PCIIOMMUOps smmuv3_accel_ops = {
>>>> +    .get_address_space = smmuv3_accel_find_add_as,
>>>> +};
>>>> +
>>>> +void smmuv3_accel_init(SMMUv3State *s)
>>>> +{
>>>> +    SMMUState *bs = ARM_SMMU(s);
>>>> +
>>>> +    bs->iommu_ops = &smmuv3_accel_ops;
>>>
>>> again, I think this should be a sSMMUv3Class attribute.
>>
>> See below.
>>>
>>>> +}
>>>> diff --git a/hw/arm/smmuv3-accel.h b/hw/arm/smmuv3-accel.h
>>>> new file mode 100644
>>>> index 0000000000..0dc6b00d35
>>>> --- /dev/null
>>>> +++ b/hw/arm/smmuv3-accel.h
>>>> @@ -0,0 +1,27 @@
>>>> +/*
>>>> + * Copyright (c) 2025 Huawei Technologies R & D (UK) Ltd
>>>> + * Copyright (C) 2025 NVIDIA
>>>> + * Written by Nicolin Chen, Shameer Kolothum
>>>> + *
>>>> + * SPDX-License-Identifier: GPL-2.0-or-later
>>>> + */
>>>> +
>>>> +#ifndef HW_ARM_SMMUV3_ACCEL_H
>>>> +#define HW_ARM_SMMUV3_ACCEL_H
>>>> +
>>>> +#include "hw/arm/smmu-common.h"
>>>> +#include CONFIG_DEVICES
>>>> +
>>>> +typedef struct SMMUv3AccelDevice {
>>>> +    SMMUDevice sdev;
>>>> +} SMMUv3AccelDevice;
>>>> +
>>>> +#ifdef CONFIG_ARM_SMMUV3_ACCEL
>>>> +void smmuv3_accel_init(SMMUv3State *s);
>>>> +#else
>>>> +static inline void smmuv3_accel_init(SMMUv3State *s)
>>>> +{
>>>> +}
>>>> +#endif
>>>> +
>>>> +#endif /* HW_ARM_SMMUV3_ACCEL_H */
>>>> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
>>>> index bcf8af8dc7..ef991cb7d8 100644
>>>> --- a/hw/arm/smmuv3.c
>>>> +++ b/hw/arm/smmuv3.c
>>>> @@ -32,6 +32,7 @@
>>>>    #include "qapi/error.h"
>>>>
>>>>    #include "hw/arm/smmuv3.h"
>>>> +#include "smmuv3-accel.h"
>>>>    #include "smmuv3-internal.h"
>>>>    #include "smmu-internal.h"
>>>>
>>>> @@ -1882,6 +1883,10 @@ static void smmu_realize(DeviceState *d, Error
>>> **errp)
>>>>        SysBusDevice *dev = SYS_BUS_DEVICE(d);
>>>>        Error *local_err = NULL;
>>>>
>>>> +    if (s->accel) {
>>>> +        smmuv3_accel_init(s);
>>>> +    }
>>>> +
>>>>        c->parent_realize(d, &local_err);
>>>>        if (local_err) {
>>>>            error_propagate(errp, local_err);
>>>> diff --git a/include/hw/arm/smmuv3.h b/include/hw/arm/smmuv3.h
>>>> index d183a62766..bb7076286b 100644
>>>> --- a/include/hw/arm/smmuv3.h
>>>> +++ b/include/hw/arm/smmuv3.h
>>>> @@ -63,6 +63,9 @@ struct SMMUv3State {
>>>>        qemu_irq     irq[4];
>>>>        QemuMutex mutex;
>>>>        char *stage;
>>>> +
>>>> +    /* SMMU has HW accelerator support for nested S1 + s2 */
>>>> +    bool accel;
>>>
>>> Have you considered modeling with a QOM object instead ?
>>
>> A bit of history on this:
>>
>> The SMMUv3 accel support was introduced first as a separate device,
>> -device arm-smmuv3-accel
>>
>> https://lore.kernel.org/qemu-devel/20250311141045.66620-4-shameerali.kolothum.thodi@huawei.com/
>>
>>
>> However, the general consensus at that time was that we should instead
>> model it on the -device arm-smmuv3 itself, with an added "accel"
>> property.
>>
>> Eric had also suggested making use of something similar to the
>> TYPE_VFIO_IOMMU_IOMMUFD / LEGACY classes for selecting the iommu_ops.
>> https://lore.kernel.org/qemu-devel/1105d100-dd1e-4aca-b518-50f903967416@redhat.com/
>>
>>
>> In RFCv3, I did introduce a TYPE_ARM_SMMUV3_ACCEL object class, but we
>> later found that it wasn’t doing much beyond helping to retrieve the
>> iommu_ops
>> based on the object type, so decided to drop it.
>> https://lore.kernel.org/qemu-devel/71ca9132-8deb-4f57-abb0-2bcc0fe93ae9@redhat.com/
>>
>>
>>  From your feedback, I understand that you would like to revisit that
>> approach
>> again. Just to make sure I get this right, is the SMMUv3AccelClass
>> you have in
>> mind meant to be an abstract object, like HOST_IOMMU_DEVICE, or are
>> you suggesting another child device object?
>
> HOST_IOMMU_DEVICE are for host IOMMU backends (VFIO IOMMU Type1
> legacy and IOMMUFD). I was imagining an "arm-smmuv3" children class
> for acceleration. 
We were trying to find similar examples of inheritance in the qemu tree.
I also inspected hw/intc, for instance the GICv3 but it is not really
similar as the choice between the kvm and tcg one is not made by an option.

As Shameer explained, we attempted to use a class and eventually we
noticed it did not bring any real benefit besides complexifying the code.

Note that on intel_iommu, the "vfio" integration is opted-in with the
caching-mode option. so using an accel option sounds a similar knob.


>
> I understand I am jumping a bit late in the discussion. Sorry about
> that and the late reply also.
>
> I have been experimenting with your series on other systems and on
> an ARM system with MLX5 VFs and an L4 GPU. I had to fake CANWBS
> support in the host kernel to make progress. On a side note, I also 
I also encountered this issue but as my HW does support CANWBS, this is
a FW issue. So this does not relate to that series.
> pulled in a patch from Nicolin to add dmabuf support and it seemed
> to behave ok. This would require more in-depth P2P testing. That's
> another topic we should discuss in the QEMU 11.0 cycle. 
I tested the functionality with P2P. I am waiting for the respin before
sending my T-b.
>
>
>> Please let me know if there is an example/precedent, I can look at
>> for a similar
>> object model in QEMU.
>
> Now regarding modeling, it is more or less a design choice. But while
> I scanned through the code, I felt there was some problems that could
> be solved in a cleaner way with a sub class. The changes are mostly
> about moving code in different places and changing/removing the
> "if (s->accel) parts". Don't change everything now, I will check RFCv3.
>
> Nevertheless, they are several more issues, build breakages and also
> runtime breakage that should be addressed. Let's do that first. 

I don't know where we stand with that respect. Shameer, were you able to
address those breakages?
>
> Furthermore, an (idealistic) design principle of QEMU that we tend to
> forget is to offer acceleration when possible and else fallback to
> emulation, in order to guarantee parity of features between different
> hardware. I expected the "arm-smmuv3" device to decide to operate in
> accelerated mode when ever possible, i.e when the IOMMUFD backend
> (the SMMUV3 + the FW) offers the right level of functionality. This
> would allow the machine to maintain a constant view of the sMMUv3
> device and facilitate its integration with management layers and
> ease migration too. 

I don't think it makes sense to fall back to a non accel mode anyway
because the protected end point device wouldn't work. SMMU has no
cachine mode as Intel. So either it works in accelerated mode along with
VFIO devices or it is not able to protected host assigned devices.

Thanks

Eric
>
> I understand this is complex, it has been done for some interrupt
> controllers, and this is not the approach of the series.
>
> That said, what about integration with libvirt ?
>
>
> Thanks,
>
> C.
>

RE: [PATCH v6 05/33] hw/arm/smmuv3-accel: Introduce smmuv3 accel device

Posted by Shameer Kolothum 1 month ago

Hi Eric,

> -----Original Message-----
> From: Eric Auger <eric.auger@redhat.com>
> Sent: 06 January 2026 10:23
> To: Cédric Le Goater <clg@redhat.com>; Shameer Kolothum
> <skolothumtho@nvidia.com>; qemu-arm@nongnu.org; qemu-
> devel@nongnu.org
> Cc: peter.maydell@linaro.org; Jason Gunthorpe <jgg@nvidia.com>; Nicolin
> Chen <nicolinc@nvidia.com>; ddutile@redhat.com; berrange@redhat.com;
> Nathan Chen <nathanc@nvidia.com>; Matt Ochs <mochs@nvidia.com>;
> smostafa@google.com; wangzhou1@hisilicon.com;
> jiangkunkun@huawei.com; jonathan.cameron@huawei.com;
> zhangfei.gao@linaro.org; zhenzhong.duan@intel.com; yi.l.liu@intel.com;
> Krishnakant Jaju <kjaju@nvidia.com>
> Subject: Re: [PATCH v6 05/33] hw/arm/smmuv3-accel: Introduce smmuv3
> accel device
> 
> External email: Use caution opening links or attachments
> 
> 
> Hi Cédric, Shameer,
> 
> On 12/17/25 6:38 PM, Cédric Le Goater wrote:
> > Hello Shameer,
> >
> > On 12/12/25 06:48, Shameer Kolothum wrote:
> >> Hi Cédric,
> >>
> >>> -----Original Message-----
> >>> From: Cédric Le Goater <clg@redhat.com>
> >>> Sent: 11 December 2025 12:55
> >>> To: Shameer Kolothum <skolothumtho@nvidia.com>; qemu-
> >>> arm@nongnu.org; qemu-devel@nongnu.org
> >>> Cc: eric.auger@redhat.com; peter.maydell@linaro.org; Jason Gunthorpe
> >>> <jgg@nvidia.com>; Nicolin Chen <nicolinc@nvidia.com>;
> >>> ddutile@redhat.com; berrange@redhat.com; Nathan Chen
> >>> <nathanc@nvidia.com>; Matt Ochs <mochs@nvidia.com>;
> >>> smostafa@google.com; wangzhou1@hisilicon.com;
> >>> jiangkunkun@huawei.com; jonathan.cameron@huawei.com;
> >>> zhangfei.gao@linaro.org; zhenzhong.duan@intel.com; yi.l.liu@intel.com;
> >>> Krishnakant Jaju <kjaju@nvidia.com>
> >>> Subject: Re: [PATCH v6 05/33] hw/arm/smmuv3-accel: Introduce
> smmuv3
> >>> accel device
> >>>
> >>> External email: Use caution opening links or attachments
> >>>
> >>>
> >>> On 11/20/25 14:21, Shameer Kolothum wrote:
> >>>> Set up dedicated PCIIOMMUOps for the accel SMMUv3, since it will need
> >>>> different callback handling in upcoming patches. This also adds a
> >>>> CONFIG_ARM_SMMUV3_ACCEL build option so the feature can be
> disabled
> >>>> at compile time. Because we now include CONFIG_DEVICES in the
> >>>> header to
> >>>> check for ARM_SMMUV3_ACCEL, the meson file entry for smmuv3.c
> needs
> >>> to
> >>>> be changed to arm_ss.add.
> >>>>
> >>>> The “accel” property isn’t user visible yet and it will be
> >>>> introduced in
> >>>> a later patch once all the supporting pieces are ready.
> >>>>
> >>>> Signed-off-by: Shameer Kolothum
> >>> <shameerali.kolothum.thodi@huawei.com>
> >>>> Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
> >>>> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
> >>>> Reviewed-by: Eric Auger <eric.auger@redhat.com>
> >>>> Tested-by: Zhangfei Gao <zhangfei.gao@linaro.org>
> >>>> Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
> >>>> ---
> >>>>    hw/arm/Kconfig          |  5 ++++
> >>>>    hw/arm/meson.build      |  3 ++-
> >>>>    hw/arm/smmuv3-accel.c   | 59
> >>> +++++++++++++++++++++++++++++++++++++++++
> >>>>    hw/arm/smmuv3-accel.h   | 27 +++++++++++++++++++
> >>>>    hw/arm/smmuv3.c         |  5 ++++
> >>>>    include/hw/arm/smmuv3.h |  3 +++
> >>>>    6 files changed, 101 insertions(+), 1 deletion(-)
> >>>>    create mode 100644 hw/arm/smmuv3-accel.c
> >>>>    create mode 100644 hw/arm/smmuv3-accel.h
> >>>>
> >>>> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> >>>> index 0cdeb60f1f..702b79a02b 100644
> >>>> --- a/hw/arm/Kconfig
> >>>> +++ b/hw/arm/Kconfig
> >>>> @@ -12,6 +12,7 @@ config ARM_VIRT
> >>>>        select ARM_GIC
> >>>>        select ACPI
> >>>>        select ARM_SMMUV3
> >>>> +    select ARM_SMMUV3_ACCEL
> >>>>        select GPIO_KEY
> >>>>        select DEVICE_TREE
> >>>>        select FW_CFG_DMA
> >>>> @@ -629,6 +630,10 @@ config FSL_IMX8MP_EVK
> >>>>    config ARM_SMMUV3
> >>>>        bool
> >>>>
> >>>> +config ARM_SMMUV3_ACCEL
> >>>> +    bool
> >>>> +    depends on ARM_SMMUV3 && IOMMUFD
> >>>> +
> >>>>    config FSL_IMX6UL
> >>>>        bool
> >>>>        default y
> >>>> diff --git a/hw/arm/meson.build b/hw/arm/meson.build
> >>>> index aeaf654790..c250487e64 100644
> >>>> --- a/hw/arm/meson.build
> >>>> +++ b/hw/arm/meson.build
> >>>> @@ -84,7 +84,8 @@ arm_common_ss.add(when: 'CONFIG_ARMSSE',
> >>> if_true: files('armsse.c'))
> >>>>    arm_common_ss.add(when: 'CONFIG_FSL_IMX7', if_true:
> >>>> files('fsl-imx7.c',
> >>> 'mcimx7d-sabre.c'))
> >>>>    arm_common_ss.add(when: 'CONFIG_FSL_IMX8MP', if_true: files('fsl-
> >>> imx8mp.c'))
> >>>>    arm_common_ss.add(when: 'CONFIG_FSL_IMX8MP_EVK', if_true:
> >>> files('imx8mp-evk.c'))
> >>>> -arm_common_ss.add(when: 'CONFIG_ARM_SMMUV3', if_true:
> >>> files('smmuv3.c'))
> >>>> +arm_ss.add(when: 'CONFIG_ARM_SMMUV3', if_true:
> files('smmuv3.c'))
> >>>> +arm_ss.add(when: 'CONFIG_ARM_SMMUV3_ACCEL', if_true:
> >>> files('smmuv3-accel.c'))
> >>>>    arm_common_ss.add(when: 'CONFIG_FSL_IMX6UL', if_true: files('fsl-
> >>> imx6ul.c', 'mcimx6ul-evk.c'))
> >>>>    arm_common_ss.add(when: 'CONFIG_NRF51_SOC', if_true:
> >>> files('nrf51_soc.c'))
> >>>>    arm_common_ss.add(when: 'CONFIG_XEN', if_true: files(
> >>>> diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
> >>>> new file mode 100644
> >>>> index 0000000000..99ef0db8c4
> >>>> --- /dev/null
> >>>> +++ b/hw/arm/smmuv3-accel.c
> >>>> @@ -0,0 +1,59 @@
> >>>> +/*
> >>>> + * Copyright (c) 2025 Huawei Technologies R & D (UK) Ltd
> >>>> + * Copyright (C) 2025 NVIDIA
> >>>> + * Written by Nicolin Chen, Shameer Kolothum
> >>>> + *
> >>>> + * SPDX-License-Identifier: GPL-2.0-or-later
> >>>> + */
> >>>> +
> >>>> +#include "qemu/osdep.h"
> >>>> +
> >>>> +#include "hw/arm/smmuv3.h"
> >>>> +#include "smmuv3-accel.h"
> >>>> +
> >>>> +static SMMUv3AccelDevice *smmuv3_accel_get_dev(SMMUState *bs,
> >>> SMMUPciBus *sbus,
> >>>> +                                               PCIBus *bus, int
> >>>> devfn)
> >>>> +{
> >>>> +    SMMUDevice *sdev = sbus->pbdev[devfn];
> >>>> +    SMMUv3AccelDevice *accel_dev;
> >>>> +
> >>>> +    if (sdev) {
> >>>> +        return container_of(sdev, SMMUv3AccelDevice, sdev);
> >>>> +    }
> >>>> +
> >>>> +    accel_dev = g_new0(SMMUv3AccelDevice, 1);
> >>>
> >>> oh. This is not a QOM object :/
> >>
> >> Right.
> >>
> >>>
> >>>> +    sdev = &accel_dev->sdev;
> >>>> +
> >>>> +    sbus->pbdev[devfn] = sdev;
> >>>> +    smmu_init_sdev(bs, sdev, bus, devfn);
> >>>> +    return accel_dev;
> >>>> +}
> >>>> +
> >>>> +/*
> >>>> + * Find or add an address space for the given PCI device.
> >>>> + *
> >>>> + * If a device matching @bus and @devfn already exists, return its
> >>>> + * corresponding address space. Otherwise, create a new device entry
> >>>> + * and initialize address space for it.
> >>>> + */
> >>>> +static AddressSpace *smmuv3_accel_find_add_as(PCIBus *bus, void
> >>> *opaque,
> >>>> +                                              int devfn)
> >>>> +{
> >>>> +    SMMUState *bs = opaque;
> >>>> +    SMMUPciBus *sbus = smmu_get_sbus(bs, bus);
> >>>> +    SMMUv3AccelDevice *accel_dev = smmuv3_accel_get_dev(bs, sbus,
> >>> bus, devfn);
> >>>> +    SMMUDevice *sdev = &accel_dev->sdev;
> >>>> +
> >>>> +    return &sdev->as;
> >>>> +}
> >>>> +
> >>>> +static const PCIIOMMUOps smmuv3_accel_ops = {
> >>>> +    .get_address_space = smmuv3_accel_find_add_as,
> >>>> +};
> >>>> +
> >>>> +void smmuv3_accel_init(SMMUv3State *s)
> >>>> +{
> >>>> +    SMMUState *bs = ARM_SMMU(s);
> >>>> +
> >>>> +    bs->iommu_ops = &smmuv3_accel_ops;
> >>>
> >>> again, I think this should be a sSMMUv3Class attribute.
> >>
> >> See below.
> >>>
> >>>> +}
> >>>> diff --git a/hw/arm/smmuv3-accel.h b/hw/arm/smmuv3-accel.h
> >>>> new file mode 100644
> >>>> index 0000000000..0dc6b00d35
> >>>> --- /dev/null
> >>>> +++ b/hw/arm/smmuv3-accel.h
> >>>> @@ -0,0 +1,27 @@
> >>>> +/*
> >>>> + * Copyright (c) 2025 Huawei Technologies R & D (UK) Ltd
> >>>> + * Copyright (C) 2025 NVIDIA
> >>>> + * Written by Nicolin Chen, Shameer Kolothum
> >>>> + *
> >>>> + * SPDX-License-Identifier: GPL-2.0-or-later
> >>>> + */
> >>>> +
> >>>> +#ifndef HW_ARM_SMMUV3_ACCEL_H
> >>>> +#define HW_ARM_SMMUV3_ACCEL_H
> >>>> +
> >>>> +#include "hw/arm/smmu-common.h"
> >>>> +#include CONFIG_DEVICES
> >>>> +
> >>>> +typedef struct SMMUv3AccelDevice {
> >>>> +    SMMUDevice sdev;
> >>>> +} SMMUv3AccelDevice;
> >>>> +
> >>>> +#ifdef CONFIG_ARM_SMMUV3_ACCEL
> >>>> +void smmuv3_accel_init(SMMUv3State *s);
> >>>> +#else
> >>>> +static inline void smmuv3_accel_init(SMMUv3State *s)
> >>>> +{
> >>>> +}
> >>>> +#endif
> >>>> +
> >>>> +#endif /* HW_ARM_SMMUV3_ACCEL_H */
> >>>> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> >>>> index bcf8af8dc7..ef991cb7d8 100644
> >>>> --- a/hw/arm/smmuv3.c
> >>>> +++ b/hw/arm/smmuv3.c
> >>>> @@ -32,6 +32,7 @@
> >>>>    #include "qapi/error.h"
> >>>>
> >>>>    #include "hw/arm/smmuv3.h"
> >>>> +#include "smmuv3-accel.h"
> >>>>    #include "smmuv3-internal.h"
> >>>>    #include "smmu-internal.h"
> >>>>
> >>>> @@ -1882,6 +1883,10 @@ static void smmu_realize(DeviceState *d,
> Error
> >>> **errp)
> >>>>        SysBusDevice *dev = SYS_BUS_DEVICE(d);
> >>>>        Error *local_err = NULL;
> >>>>
> >>>> +    if (s->accel) {
> >>>> +        smmuv3_accel_init(s);
> >>>> +    }
> >>>> +
> >>>>        c->parent_realize(d, &local_err);
> >>>>        if (local_err) {
> >>>>            error_propagate(errp, local_err);
> >>>> diff --git a/include/hw/arm/smmuv3.h b/include/hw/arm/smmuv3.h
> >>>> index d183a62766..bb7076286b 100644
> >>>> --- a/include/hw/arm/smmuv3.h
> >>>> +++ b/include/hw/arm/smmuv3.h
> >>>> @@ -63,6 +63,9 @@ struct SMMUv3State {
> >>>>        qemu_irq     irq[4];
> >>>>        QemuMutex mutex;
> >>>>        char *stage;
> >>>> +
> >>>> +    /* SMMU has HW accelerator support for nested S1 + s2 */
> >>>> +    bool accel;
> >>>
> >>> Have you considered modeling with a QOM object instead ?
> >>
> >> A bit of history on this:
> >>
> >> The SMMUv3 accel support was introduced first as a separate device,
> >> -device arm-smmuv3-accel
> >>
> >> https://lore.kernel.org/qemu-devel/20250311141045.66620-4-
> shameerali.kolothum.thodi@huawei.com/
> >>
> >>
> >> However, the general consensus at that time was that we should instead
> >> model it on the -device arm-smmuv3 itself, with an added "accel"
> >> property.
> >>
> >> Eric had also suggested making use of something similar to the
> >> TYPE_VFIO_IOMMU_IOMMUFD / LEGACY classes for selecting the
> iommu_ops.
> >> https://lore.kernel.org/qemu-devel/1105d100-dd1e-4aca-b518-
> 50f903967416@redhat.com/
> >>
> >>
> >> In RFCv3, I did introduce a TYPE_ARM_SMMUV3_ACCEL object class, but
> we
> >> later found that it wasn’t doing much beyond helping to retrieve the
> >> iommu_ops
> >> based on the object type, so decided to drop it.
> >> https://lore.kernel.org/qemu-devel/71ca9132-8deb-4f57-abb0-
> 2bcc0fe93ae9@redhat.com/
> >>
> >>
> >>  From your feedback, I understand that you would like to revisit that
> >> approach
> >> again. Just to make sure I get this right, is the SMMUv3AccelClass
> >> you have in
> >> mind meant to be an abstract object, like HOST_IOMMU_DEVICE, or are
> >> you suggesting another child device object?
> >
> > HOST_IOMMU_DEVICE are for host IOMMU backends (VFIO IOMMU Type1
> > legacy and IOMMUFD). I was imagining an "arm-smmuv3" children class
> > for acceleration.
> We were trying to find similar examples of inheritance in the qemu tree.
> I also inspected hw/intc, for instance the GICv3 but it is not really
> similar as the choice between the kvm and tcg one is not made by an option.
> 
> As Shameer explained, we attempted to use a class and eventually we
> noticed it did not bring any real benefit besides complexifying the code.
> 
> Note that on intel_iommu, the "vfio" integration is opted-in with the
> caching-mode option. so using an accel option sounds a similar knob.
> 
> 
> >
> > I understand I am jumping a bit late in the discussion. Sorry about
> > that and the late reply also.
> >
> > I have been experimenting with your series on other systems and on
> > an ARM system with MLX5 VFs and an L4 GPU. I had to fake CANWBS
> > support in the host kernel to make progress. On a side note, I also
> I also encountered this issue but as my HW does support CANWBS, this is
> a FW issue. So this does not relate to that series.
> > pulled in a patch from Nicolin to add dmabuf support and it seemed
> > to behave ok. This would require more in-depth P2P testing. That's
> > another topic we should discuss in the QEMU 11.0 cycle.
> I tested the functionality with P2P. I am waiting for the respin before
> sending my T-b.
> >
> >
> >> Please let me know if there is an example/precedent, I can look at
> >> for a similar
> >> object model in QEMU.
> >
> > Now regarding modeling, it is more or less a design choice. But while
> > I scanned through the code, I felt there was some problems that could
> > be solved in a cleaner way with a sub class. The changes are mostly
> > about moving code in different places and changing/removing the
> > "if (s->accel) parts". Don't change everything now, I will check RFCv3.
> >
> > Nevertheless, they are several more issues, build breakages and also
> > runtime breakage that should be addressed. Let's do that first.
> 
> I don't know where we stand with that respect. Shameer, were you able to
> address those breakages?

Missed this. Yes, I am working on v7 addressing the build issues. For the
HostIOMMUDevice related build and runtime issues, I have sent out a
proposal here,
https://lore.kernel.org/qemu-devel/CH3PR12MB75482AA6E44111F51B5F8E47ABBBA@CH3PR12MB7548.namprd12.prod.outlook.com/

Thanks,
Shameer

RE: [PATCH v6 05/33] hw/arm/smmuv3-accel: Introduce smmuv3 accel device

Posted by Shameer Kolothum 1 month, 3 weeks ago

Hi Cédric,

> -----Original Message-----
> From: Cédric Le Goater <clg@redhat.com>
> Sent: 17 December 2025 17:39
> To: Shameer Kolothum <skolothumtho@nvidia.com>; qemu-
> arm@nongnu.org; qemu-devel@nongnu.org
> Cc: eric.auger@redhat.com; peter.maydell@linaro.org; Jason Gunthorpe
> <jgg@nvidia.com>; Nicolin Chen <nicolinc@nvidia.com>; ddutile@redhat.com;
> berrange@redhat.com; Nathan Chen <nathanc@nvidia.com>; Matt Ochs
> <mochs@nvidia.com>; smostafa@google.com; wangzhou1@hisilicon.com;
> jiangkunkun@huawei.com; jonathan.cameron@huawei.com;
> zhangfei.gao@linaro.org; zhenzhong.duan@intel.com; yi.l.liu@intel.com;
> Krishnakant Jaju <kjaju@nvidia.com>
> Subject: Re: [PATCH v6 05/33] hw/arm/smmuv3-accel: Introduce smmuv3
> accel device
> 

[...]

> >>
> >> Have you considered modeling with a QOM object instead ?
> >
> > A bit of history on this:
> >
> > The SMMUv3 accel support was introduced first as a separate device,
> > -device arm-smmuv3-accel
> >
> > https://lore.kernel.org/qemu-devel/20250311141045.66620-4-
> shameerali.kolothum.thodi@huawei.com/
> >
> > However, the general consensus at that time was that we should instead
> > model it on the -device arm-smmuv3 itself, with an added "accel" property.
> >
> > Eric had also suggested making use of something similar to the
> > TYPE_VFIO_IOMMU_IOMMUFD / LEGACY classes for selecting the
> iommu_ops.
> > https://lore.kernel.org/qemu-devel/1105d100-dd1e-4aca-b518-
> 50f903967416@redhat.com/
> >
> > In RFCv3, I did introduce a TYPE_ARM_SMMUV3_ACCEL object class, but we
> > later found that it wasn’t doing much beyond helping to retrieve the
> iommu_ops
> > based on the object type, so decided to drop it.
> > https://lore.kernel.org/qemu-devel/71ca9132-8deb-4f57-abb0-
> 2bcc0fe93ae9@redhat.com/
> >
> >  From your feedback, I understand that you would like to revisit that
> approach
> > again. Just to make sure I get this right, is the SMMUv3AccelClass you have
> in
> > mind meant to be an abstract object, like HOST_IOMMU_DEVICE, or are
> > you suggesting another child device object?
> 
> HOST_IOMMU_DEVICE are for host IOMMU backends (VFIO IOMMU Type1
> legacy and IOMMUFD). I was imagining an "arm-smmuv3" children class
> for acceleration.

Ok. As mentioned above, the accelerated SMMUv3 is currently not a separate
child device of "arm-smmuv3". Instead, acceleration support is enabled via the
accel property on "arm-smmuv3".

> I understand I am jumping a bit late in the discussion. Sorry about
> that and the late reply also.
> 
> I have been experimenting with your series on other systems and on
> an ARM system with MLX5 VFs and an L4 GPU. I had to fake CANWBS
> support in the host kernel to make progress. On a side note, I also
> pulled in a patch from Nicolin to add dmabuf support and it seemed
> to behave ok. This would require more in-depth P2P testing. That's
> another topic we should discuss in the QEMU 11.0 cycle.

Thanks for giving this a spin. 

> 
> > Please let me know if there is an example/precedent, I can look at for a
> similar
> > object model in QEMU.
> 
> Now regarding modeling, it is more or less a design choice. But while
> I scanned through the code, I felt there was some problems that could
> be solved in a cleaner way with a sub class. The changes are mostly
> about moving code in different places and changing/removing the
> "if (s->accel) parts". Don't change everything now, I will check RFCv3.

Without having a proper child device of "arm-smmuv3", it is not clear to me
how we can avoid the "if (s->accel)" checks. Please let me know if you have
any thoughts on this.
 
> Nevertheless, they are several more issues, build breakages and also
> runtime breakage that should be addressed. Let's do that first.

The build breakage you pointed out is something I missed during my
testing ☹. The same applies to the GPU mdev core dump. Hopefully, we
can address these without requiring major rework.

> Furthermore, an (idealistic) design principle of QEMU that we tend to
> forget is to offer acceleration when possible and else fallback to
> emulation, in order to guarantee parity of features between different
> hardware. I expected the "arm-smmuv3" device to decide to operate in
> accelerated mode when ever possible,  i.e when the IOMMUFD backend
> (the SMMUV3 + the FW) offers the right level of functionality.

Accelerated SMMUv3 support only makes sense for vfio pci pass-through
devices which require the nested stage configuration. Emulated endpoint
devices are not allowed with accel SMMUv3.

 This
> would allow the machine to maintain a constant view of the sMMUv3
> device and facilitate its integration with management layers and
> ease migration too.

The current model relies on specifying an accelerated SMMUv3 with the
default features of "arm-smmuv3", along with user-specified optional
features that make sense for accelerated use cases (e.g. RIL, PASID). The
implementation performs a compatibility check against the host SMMUv3
to ensure the requested configuration can be safely supported (patch #20).

> I understand this is complex, it has been done for some interrupt
> controllers, and this is not the approach of the series.
 
> That said, what about integration with libvirt ?

Nathan has this "Implement support for iommufd and multiple HW-accel
vSMMUs" libvirt series here:

https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/F4XIUZCDNMXGW5WBJ47VLVSZMGXO5B6E/

And I believe his precursor, “Implement support for multiple device-pluggable
SMMUv3s”, has already been applied.
https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/WOJKDS3N23N63SMMDLLTZJRVXWPHK7EM/

Thanks,
Shameer