Re: [PATCH v6 27/33] hw/arm/smmuv3-accel: Add a property to specify RIL support

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

On 11/20/25 14:22, Shameer Kolothum wrote:
> Currently QEMU SMMUv3 has RIL support by default. But if accelerated mode
> is enabled, RIL has to be compatible with host SMMUv3 support.
> 
> Add a property so that the user can specify this.
> 
> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
> Tested-by: Zhangfei Gao <zhangfei.gao@linaro.org>
> Reviewed-by: Eric Auger <eric.auger@redhat.com>
> Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
> ---
>   hw/arm/smmuv3-accel.c   | 14 ++++++++++++--
>   hw/arm/smmuv3-accel.h   |  4 ++++
>   hw/arm/smmuv3.c         | 12 ++++++++++++
>   include/hw/arm/smmuv3.h |  1 +
>   4 files changed, 29 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
> index aae7840c40..b6429c8b42 100644
> --- a/hw/arm/smmuv3-accel.c
> +++ b/hw/arm/smmuv3-accel.c
> @@ -62,8 +62,8 @@ smmuv3_accel_check_hw_compatible(SMMUv3State *s,
>           return false;
>       }
>   
> -    /* QEMU SMMUv3 supports Range Invalidation by default */
> -    if (FIELD_EX32(info->idr[3], IDR3, RIL) !=
> +    /* User can disable QEMU SMMUv3 Range Invalidation support */
> +    if (FIELD_EX32(info->idr[3], IDR3, RIL) >
>                   FIELD_EX32(s->idr[3], IDR3, RIL)) {
>           error_setg(errp, "Host SMMUv3 doesn't support Range Invalidation");
>           return false;
> @@ -639,6 +639,16 @@ static const PCIIOMMUOps smmuv3_accel_ops = {
>       .get_msi_direct_gpa = smmuv3_accel_get_msi_gpa,
>   };
>   
> +void smmuv3_accel_idr_override(SMMUv3State *s)
> +{
> +    if (!s->accel) {
> +        return;
> +    }

Those :

    if (s->accel)

in the code reveal a modeling issue.

> +
> +    /* By default QEMU SMMUv3 has RIL. Update IDR3 if user has disabled it */
> +    s->idr[3] = FIELD_DP32(s->idr[3], IDR3, RIL, s->ril);
> +}
> +
>   /* Based on SMUUv3 GPBA.ABORT configuration, attach a corresponding HWPT */
>   bool smmuv3_accel_attach_gbpa_hwpt(SMMUv3State *s, Error **errp)
>   {
> diff --git a/hw/arm/smmuv3-accel.h b/hw/arm/smmuv3-accel.h
> index 7186817264..2f2904d86b 100644
> --- a/hw/arm/smmuv3-accel.h
> +++ b/hw/arm/smmuv3-accel.h
> @@ -47,6 +47,7 @@ bool smmuv3_accel_install_ste_range(SMMUv3State *s, SMMUSIDRange *range,
>   bool smmuv3_accel_attach_gbpa_hwpt(SMMUv3State *s, Error **errp);
>   bool smmuv3_accel_issue_inv_cmd(SMMUv3State *s, void *cmd, SMMUDevice *sdev,
>                                   Error **errp);
> +void smmuv3_accel_idr_override(SMMUv3State *s);
>   void smmuv3_accel_reset(SMMUv3State *s);
>   #else
>   static inline void smmuv3_accel_init(SMMUv3State *s)
> @@ -74,6 +75,9 @@ smmuv3_accel_issue_inv_cmd(SMMUv3State *s, void *cmd, SMMUDevice *sdev,
>   {
>       return true;
>   }
> +static inline void smmuv3_accel_idr_override(SMMUv3State *s)
> +{
> +}
>   static inline void smmuv3_accel_reset(SMMUv3State *s)
>   {
>   }
> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> index 8352dd5757..296afbe503 100644
> --- a/hw/arm/smmuv3.c
> +++ b/hw/arm/smmuv3.c
> @@ -305,6 +305,7 @@ static void smmuv3_init_id_regs(SMMUv3State *s)
>       s->idr[5] = FIELD_DP32(s->idr[5], IDR5, GRAN16K, 1);
>       s->idr[5] = FIELD_DP32(s->idr[5], IDR5, GRAN64K, 1);
>       s->aidr = 0x1;
> +    smmuv3_accel_idr_override(s);
>   }
>   
>   static void smmuv3_reset(SMMUv3State *s)
> @@ -1924,6 +1925,13 @@ static bool smmu_validate_property(SMMUv3State *s, Error **errp)
>           return false;
>       }
>   #endif
> +    if (!s->accel) {
> +        if (!s->ril) {
> +            error_setg(errp, "ril can only be disabled if accel=on");
> +            return false;
> +        }
> +        return true;
> +    }
>       return true;
>   }
>   
> @@ -2047,6 +2055,8 @@ static const Property smmuv3_properties[] = {
>       DEFINE_PROP_BOOL("accel", SMMUv3State, accel, false),
>       /* GPA of MSI doorbell, for SMMUv3 accel use. */
>       DEFINE_PROP_UINT64("msi-gpa", SMMUv3State, msi_gpa, 0),
> +    /* RIL can be turned off for accel cases */
> +    DEFINE_PROP_BOOL("ril", SMMUv3State, ril, true),

yep. Adding a QOM model would clarify a lot of things.

C.


>   };
>   
>   static void smmuv3_instance_init(Object *obj)
> @@ -2072,6 +2082,8 @@ static void smmuv3_class_init(ObjectClass *klass, const void *data)
>       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");
> +    object_class_property_set_description(klass, "ril",
> +        "Disable range invalidation support (for accel=on)");
>   }
>   
>   static int smmuv3_notify_flag_changed(IOMMUMemoryRegion *iommu,
> diff --git a/include/hw/arm/smmuv3.h b/include/hw/arm/smmuv3.h
> index 9c39acd5ca..533a2182e8 100644
> --- a/include/hw/arm/smmuv3.h
> +++ b/include/hw/arm/smmuv3.h
> @@ -69,6 +69,7 @@ struct SMMUv3State {
>       struct SMMUv3AccelState *s_accel;
>       uint64_t msi_gpa;
>       Error *migration_blocker;
> +    bool ril;
>   };
>   
>   typedef enum {