Re: [RFC PATCH 16/16] hw/arm/smmuv3: Add tegra241-cmdqv property for SMMUv3 device

[Nicolin Chen]

On Wed, Dec 10, 2025 at 01:37:37PM +, Shameer Kolothum wrote:
> Introduce a “tegra241-cmdqv” property to enable Tegra241 CMDQV
> support. This is only enabled for accelerated SMMUv3 devices.
> 
> Signed-off-by: Shameer Kolothum 
> ---
>  hw/arm/smmuv3.c | 13 +
>  1 file changed, 13 insertions(+)
> 
> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> index ec8687d39a..58c35c2af3 100644
> --- a/hw/arm/smmuv3.c
> +++ b/hw/arm/smmuv3.c
> @@ -1953,6 +1953,12 @@ static bool smmu_validate_property(SMMUv3State *s, 
> Error **errp)
>  error_setg(errp, "accel=on support not compiled in");
>  return false;
>  }
> +#endif
> +#ifndef CONFIG_TEGRA241_CMDQ
> +if (s->tegra241_cmdqv) {
> +error_setg(errp, "tegra241_cmdqv=on support not compiled in");
> +return false;
> +}
>  #endif

Instead of adding this..

> @@ -2109,6 +2119,7 @@ static const Property smmuv3_properties[] = {
>  DEFINE_PROP_BOOL("ats", SMMUv3State, ats, false),
>  DEFINE_PROP_UINT8("oas", SMMUv3State, oas, 44),
>  DEFINE_PROP_BOOL("pasid", SMMUv3State, pasid, false),
> +DEFINE_PROP_BOOL("tegra241-cmdqv", SMMUv3State, tegra241_cmdqv, false),

Could this be

+#if CONFIG_TEGRA241_CMDQ
+DEFINE_PROP_BOOL("tegra241-cmdqv", SMMUv3State, tegra241_cmdqv, false),
+endif

?

> @@ -2144,6 +2155,8 @@ static void smmuv3_class_init(ObjectClass *klass, const 
> void *data)
>  "are 44 or 48 bits. Defaults to 44 bits");
>  object_class_property_set_description(klass, "pasid",
>  "Enable/disable PASID support (for accel=on)");
> +object_class_property_set_description(klass, "tegra241-cmdqv",
> +"Enable/disable Tegra241 CMDQ-Virtualisation support (for 
> accel=on)");

And here too.

The current approach looks okay to me though. So either way:

Reviewed-by: Nicolin Chen 

[RFC PATCH 16/16] hw/arm/smmuv3: Add tegra241-cmdqv property for SMMUv3 device

[Shameer Kolothum]

Introduce a “tegra241-cmdqv” property to enable Tegra241 CMDQV
support. This is only enabled for accelerated SMMUv3 devices.

Signed-off-by: Shameer Kolothum 
---
 hw/arm/smmuv3.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index ec8687d39a..58c35c2af3 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -1953,6 +1953,12 @@ static bool smmu_validate_property(SMMUv3State *s, Error 
**errp)
 error_setg(errp, "accel=on support not compiled in");
 return false;
 }
+#endif
+#ifndef CONFIG_TEGRA241_CMDQ
+if (s->tegra241_cmdqv) {
+error_setg(errp, "tegra241_cmdqv=on support not compiled in");
+return false;
+}
 #endif
 if (!s->accel) {
 if (!s->ril) {
@@ -1971,6 +1977,10 @@ static bool smmu_validate_property(SMMUv3State *s, Error 
**errp)
 error_setg(errp, "pasid can only be enabled if accel=on");
 return false;
 }
+if (s->tegra241_cmdqv) {
+error_setg(errp, "tegra241_cmdqv can only be enabled if accel=on");
+return false;
+}
 return true;
 }
 
@@ -2109,6 +2119,7 @@ static const Property smmuv3_properties[] = {
 DEFINE_PROP_BOOL("ats", SMMUv3State, ats, false),
 DEFINE_PROP_UINT8("oas", SMMUv3State, oas, 44),
 DEFINE_PROP_BOOL("pasid", SMMUv3State, pasid, false),
+DEFINE_PROP_BOOL("tegra241-cmdqv", SMMUv3State, tegra241_cmdqv, false),
 };
 
 static void smmuv3_instance_init(Object *obj)
@@ -2144,6 +2155,8 @@ static void smmuv3_class_init(ObjectClass *klass, const 
void *data)
 "are 44 or 48 bits. Defaults to 44 bits");
 object_class_property_set_description(klass, "pasid",
 "Enable/disable PASID support (for accel=on)");
+object_class_property_set_description(klass, "tegra241-cmdqv",
+"Enable/disable Tegra241 CMDQ-Virtualisation support (for accel=on)");
 }
 
 static int smmuv3_notify_flag_changed(IOMMUMemoryRegion *iommu,
-- 
2.43.0