Re: [PATCH v6 26/33] hw/arm/smmuv3: Add accel property for SMMUv3 device

["Cédric Le Goater" <clg@redhat.com>]

On 11/20/25 14:22, Shameer Kolothum wrote:
> Introduce an “accel” property to enable accelerator mode.
> 
> Live migration is currently unsupported when accelerator mode is enabled,
> so a migration blocker is added.
> 
> Because this mode relies on IORT RMR for MSI support, accelerator mode is
> not supported for device tree boot.
> 
> Also, in the accelerated SMMUv3 case, the host SMMUv3 is configured in nested
> mode (S1 + S2), and the guest owns the Stage-1 page table. Therefore, we
> expose only Stage-1 to the guest to ensure it uses the correct page table
> format.
> 
> Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
> Reviewed-by: Eric Auger <eric.auger@redhat.com>
> Tested-by: Zhangfei Gao <zhangfei.gao@linaro.org>
> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
> Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
> ---
>   hw/arm/smmuv3.c          | 26 ++++++++++++++++++++++++++
>   hw/arm/virt-acpi-build.c |  4 +---
>   hw/arm/virt.c            | 15 +++++++++++----
>   include/hw/arm/smmuv3.h  |  1 +
>   4 files changed, 39 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> index dba5abc8d3..8352dd5757 100644
> --- a/hw/arm/smmuv3.c
> +++ b/hw/arm/smmuv3.c
> @@ -20,6 +20,7 @@
>   #include "qemu/bitops.h"
>   #include "hw/irq.h"
>   #include "hw/sysbus.h"
> +#include "migration/blocker.h"
>   #include "migration/vmstate.h"
>   #include "hw/qdev-properties.h"
>   #include "hw/qdev-core.h"
> @@ -1915,6 +1916,17 @@ static void smmu_reset_exit(Object *obj, ResetType type)
>       smmuv3_accel_reset(s);
>   }
>   
> +static bool smmu_validate_property(SMMUv3State *s, Error **errp)
> +{
> +#ifndef CONFIG_ARM_SMMUV3_ACCEL
> +    if (s->accel) {
> +        error_setg(errp, "accel=on support not compiled in");
> +        return false;
> +    }
> +#endif
> +    return true;
> +}
> +
>   static void smmu_realize(DeviceState *d, Error **errp)
>   {
>       SMMUState *sys = ARM_SMMU(d);
> @@ -1923,8 +1935,17 @@ static void smmu_realize(DeviceState *d, Error **errp)
>       SysBusDevice *dev = SYS_BUS_DEVICE(d);
>       Error *local_err = NULL;
>   
> +    if (!smmu_validate_property(s, errp)) {
> +        return;
> +    }

You should really consider adding a new QOM model for the "accel" mode.

>       if (s->accel) {
>           smmuv3_accel_init(s);
> +        error_setg(&s->migration_blocker, "Migration not supported with SMMUv3 "
> +                   "accelerator mode enabled");
> +        if (migrate_add_blocker(&s->migration_blocker, errp) < 0) {
> +            return;
> +        }

I would add the migration blocker in a separate patch.


>       }
>   
>       c->parent_realize(d, &local_err);
> @@ -2023,6 +2044,7 @@ static const Property smmuv3_properties[] = {
>        * Defaults to stage 1
>        */
>       DEFINE_PROP_STRING("stage", SMMUv3State, stage),
> +    DEFINE_PROP_BOOL("accel", SMMUv3State, accel, false),
>       /* GPA of MSI doorbell, for SMMUv3 accel use. */
>       DEFINE_PROP_UINT64("msi-gpa", SMMUv3State, msi_gpa, 0),
>   };
> @@ -2046,6 +2068,10 @@ static void smmuv3_class_init(ObjectClass *klass, const void *data)
>       device_class_set_props(dc, smmuv3_properties);
>       dc->hotpluggable = false;
>       dc->user_creatable = true;
> +
> +    object_class_property_set_description(klass, "accel",
> +        "Enable SMMUv3 accelerator support. Allows host SMMUv3 to be "
> +        "configured in nested mode for vfio-pci dev assignment");
>   }
>   
>   static int smmuv3_notify_flag_changed(IOMMUMemoryRegion *iommu,
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 7a7b2e62c1..fd78c39317 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -400,9 +400,7 @@ static int iort_smmuv3_devices(Object *obj, void *opaque)
>       }
>   
>       bus = PCI_BUS(object_property_get_link(obj, "primary-bus", &error_abort));
> -    if (object_property_find(obj, "accel")) {
> -        sdev.accel = object_property_get_bool(obj, "accel", &error_abort);
> -    }
> +    sdev.accel = object_property_get_bool(obj, "accel", &error_abort);
>       pbus = PLATFORM_BUS_DEVICE(vms->platform_bus_dev);
>       sbdev = SYS_BUS_DEVICE(obj);
>       sdev.base = platform_bus_get_mmio_addr(pbus, sbdev, 0);
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 8503879c3d..51b15aef37 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -3052,13 +3052,21 @@ static void virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
>               /* The new SMMUv3 device is specific to the PCI bus */
>               object_property_set_bool(OBJECT(dev), "smmu_per_bus", true, NULL);
>           }
> -        if (object_property_find(OBJECT(dev), "accel") &&
> -            object_property_get_bool(OBJECT(dev), "accel", &error_abort)) {
> +        if (object_property_get_bool(OBJECT(dev), "accel", &error_abort)) {
> +            char *stage;
> +
>               if (!kvm_enabled() || !kvm_irqchip_in_kernel()) {
>                   error_setg(errp, "SMMUv3 accel=on requires KVM with "
>                              "kernel-irqchip=on support");
>                       return;
>               }
> +            stage = object_property_get_str(OBJECT(dev), "stage", &error_fatal);
> +            /* If no stage specified, SMMUv3 will default to stage 1 */
> +            if (*stage && strcmp("1", stage)) {

It would be less "ugly" to check SMMUStage enum values instead
of using string compare :/

> +                error_setg(errp, "Only stage1 is supported for SMMUV3 with "
> +                           "accel=on");

Can't this condition be checked in the realize handler instead ?

C.


> +                return;
> +            }
>           }
>       }
>   }
> @@ -3096,8 +3104,7 @@ static void virt_machine_device_plug_cb(HotplugHandler *hotplug_dev,
>               }
>   
>               create_smmuv3_dev_dtb(vms, dev, bus);
> -            if (object_property_find(OBJECT(dev), "accel") &&
> -                object_property_get_bool(OBJECT(dev), "accel", &error_abort)) {
> +            if (object_property_get_bool(OBJECT(dev), "accel", &error_abort)) {
>                   hwaddr db_start;
>   
>                   if (vms->msi_controller == VIRT_MSI_CTRL_ITS) {
> diff --git a/include/hw/arm/smmuv3.h b/include/hw/arm/smmuv3.h
> index 5616a8a2be..9c39acd5ca 100644
> --- a/include/hw/arm/smmuv3.h
> +++ b/include/hw/arm/smmuv3.h
> @@ -68,6 +68,7 @@ struct SMMUv3State {
>       bool accel;
>       struct SMMUv3AccelState *s_accel;
>       uint64_t msi_gpa;
> +    Error *migration_blocker;
>   };
>   
>   typedef enum {