On 3/18/2026 6:38 AM, Eric Auger wrote:

On 3/18/26 10:31 AM, Shameer Kolothum Thodi wrote:
-----Original Message-----
From: Nathan Chen<[email protected]>
Sent: 17 March 2026 18:38
To:[email protected];[email protected]
Cc: Eric Auger<[email protected]>; Peter Maydell
<[email protected]>; Shannon Zhao<[email protected]>;
Michael S . Tsirkin<[email protected]>; Igor Mammedov
<[email protected]>; Ani Sinha<[email protected]>; Paolo Bonzini
<[email protected]>; Daniel P . BerrangĂ©<[email protected]>; Eric
Blake<[email protected]>; Markus Armbruster<[email protected]>;
Shameer Kolothum Thodi<[email protected]>; Matt Ochs
<[email protected]>; Nicolin Chen<[email protected]>; Nathan Chen
<[email protected]>
Subject: [PATCH v3 2/8] hw/arm/smmuv3-accel: Change ATS property to
OnOffAuto

From: Nathan Chen<[email protected]>

Change accel SMMUv3 ATS property from bool to OnOffAuto. The 'auto'
value is not implemented, as this commit is meant to set the property
to the correct type and avoid breaking JSON/QMP when the auto mode is
introduced. A future patch will implement resolution of the 'auto'
value to match the host SMMUv3 ATS support.

Fixes: f7f5013a55a3 ("hw/arm/smmuv3-accel: Add support for ATS")
Signed-off-by: Nathan Chen<[email protected]>
---
  hw/arm/smmuv3-accel.c    |  6 ++++--
  hw/arm/smmuv3.c          | 14 ++++++++++++--
  hw/arm/virt-acpi-build.c |  2 +-
  include/hw/arm/smmuv3.h  |  4 +++-
  4 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
index 2bb142c47f..621ac531a5 100644
--- a/hw/arm/smmuv3-accel.c
+++ b/hw/arm/smmuv3-accel.c
@@ -826,8 +826,10 @@ void smmuv3_accel_idr_override(SMMUv3State *s)
      /* 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);

-    /* QEMU SMMUv3 has no ATS. Advertise ATS if opt-in by property */
-    s->idr[0] = FIELD_DP32(s->idr[0], IDR0, ATS, s->ats);
+    /* Only override ATS if user explicitly set ON */
+    if (s->ats == ON_OFF_AUTO_ON) {
+        s->idr[0] = FIELD_DP32(s->idr[0], IDR0, ATS, 1);
+    }
Nit: I think we can keep the original comment.

      /* Advertise 48-bit OAS in IDR5 when requested (default is 44 bits). */
      if (s->oas == SMMU_OAS_48BIT) {
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 068108e49b..3dead0bcd3 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -317,6 +317,11 @@ static void smmuv3_init_id_regs(SMMUv3State *s)
      smmuv3_accel_idr_override(s);
  }

+bool smmuv3_ats_enabled(SMMUv3State *s)
+{
+    return FIELD_EX32(s->idr[0], IDR0, ATS);
+}
+
  static void smmuv3_reset(SMMUv3State *s)
  {
      s->cmdq.base = deposit64(s->cmdq.base, 0, 5, SMMU_CMDQS);
@@ -1971,7 +1976,7 @@ static bool
smmu_validate_property(SMMUv3State *s, Error **errp)
              error_setg(errp, "ril can only be disabled if accel=on");
              return false;
          }
-        if (s->ats) {
+        if (s->ats == ON_OFF_AUTO_ON) {
              error_setg(errp, "ats can only be enabled if accel=on");
              return false;
          }
@@ -1993,6 +1998,11 @@ static bool
smmu_validate_property(SMMUv3State *s, Error **errp)
          return false;
      }

+    if (s->ats == ON_OFF_AUTO_AUTO) {
+        error_setg(errp, "ats cannot be set to auto");
+        return false;
+    }
  Nit: Is "ats auto mode not supported" better?

Also, should we do this before the if (!s->accel)  check above?
Otherwise, I think it will carry on for non-accel smmuv3 cases like,

-device arm-smmuv3,primary-bus=pcie.1,accel=off,ats=auto

I think this applies to other properties in this series as well.

      if (s->oas != SMMU_OAS_44BIT && s->oas != SMMU_OAS_48BIT) {
          error_setg(errp, "OAS can only be set to 44 or 48 bits");
          return false;
@@ -2128,7 +2138,7 @@ static const Property smmuv3_properties[] = {
      DEFINE_PROP_UINT64("msi-gpa", SMMUv3State, msi_gpa, 0),
      /* RIL can be turned off for accel cases */
      DEFINE_PROP_BOOL("ril", SMMUv3State, ril, true),
-    DEFINE_PROP_BOOL("ats", SMMUv3State, ats, false),
+    DEFINE_PROP_ON_OFF_AUTO("ats", SMMUv3State, ats,
ON_OFF_AUTO_OFF),
I think we should update the description in 
object_class_property_set_description
to mention auto not supported.
I agree with Shameer wrt all above comments

Yes, I will make these changes for the next refresh.

Thanks,
Nathan

Reply via email to