From: Nathan Chen <nathanc@nvidia.com>
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 <nathanc@nvidia.com>
---
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);
+ }
/* 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;
+ }
+
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),
DEFINE_PROP_UINT8("oas", SMMUv3State, oas, 44),
DEFINE_PROP_UINT8("ssidsize", SMMUv3State, ssidsize, 0),
};
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 719d2f994e..591cfc993c 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -402,7 +402,7 @@ static int iort_smmuv3_devices(Object *obj, void *opaque)
bus = PCI_BUS(object_property_get_link(obj, "primary-bus", &error_abort));
sdev.accel = object_property_get_bool(obj, "accel", &error_abort);
- sdev.ats = object_property_get_bool(obj, "ats", &error_abort);
+ sdev.ats = smmuv3_ats_enabled(ARM_SMMUV3(obj));
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/include/hw/arm/smmuv3.h b/include/hw/arm/smmuv3.h
index 26b2fc42fd..ce51a5b9b4 100644
--- a/include/hw/arm/smmuv3.h
+++ b/include/hw/arm/smmuv3.h
@@ -70,7 +70,7 @@ struct SMMUv3State {
uint64_t msi_gpa;
Error *migration_blocker;
bool ril;
- bool ats;
+ OnOffAuto ats;
uint8_t oas;
uint8_t ssidsize;
};
@@ -91,6 +91,8 @@ struct SMMUv3Class {
ResettablePhases parent_phases;
};
+bool smmuv3_ats_enabled(struct SMMUv3State *s);
+
#define TYPE_ARM_SMMUV3 "arm-smmuv3"
OBJECT_DECLARE_TYPE(SMMUv3State, SMMUv3Class, ARM_SMMUV3)
--
2.43.0
> -----Original Message-----
> From: Nathan Chen <nathanc@nvidia.com>
> Sent: 17 March 2026 18:38
> To: qemu-devel@nongnu.org; qemu-arm@nongnu.org
> Cc: Eric Auger <eric.auger@redhat.com>; Peter Maydell
> <peter.maydell@linaro.org>; Shannon Zhao <shannon.zhaosl@gmail.com>;
> Michael S . Tsirkin <mst@redhat.com>; Igor Mammedov
> <imammedo@redhat.com>; Ani Sinha <anisinha@redhat.com>; Paolo Bonzini
> <pbonzini@redhat.com>; Daniel P . Berrangé <berrange@redhat.com>; Eric
> Blake <eblake@redhat.com>; Markus Armbruster <armbru@redhat.com>;
> Shameer Kolothum Thodi <skolothumtho@nvidia.com>; Matt Ochs
> <mochs@nvidia.com>; Nicolin Chen <nicolinc@nvidia.com>; Nathan Chen
> <nathanc@nvidia.com>
> Subject: [PATCH v3 2/8] hw/arm/smmuv3-accel: Change ATS property to
> OnOffAuto
>
> From: Nathan Chen <nathanc@nvidia.com>
>
> 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 <nathanc@nvidia.com>
> ---
> 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.
Thanks,
Shameer
> DEFINE_PROP_UINT8("oas", SMMUv3State, oas, 44),
> DEFINE_PROP_UINT8("ssidsize", SMMUv3State, ssidsize, 0),
> };
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 719d2f994e..591cfc993c 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -402,7 +402,7 @@ static int iort_smmuv3_devices(Object *obj, void
> *opaque)
>
> bus = PCI_BUS(object_property_get_link(obj, "primary-bus",
> &error_abort));
> sdev.accel = object_property_get_bool(obj, "accel", &error_abort);
> - sdev.ats = object_property_get_bool(obj, "ats", &error_abort);
> + sdev.ats = smmuv3_ats_enabled(ARM_SMMUV3(obj));
> 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/include/hw/arm/smmuv3.h b/include/hw/arm/smmuv3.h
> index 26b2fc42fd..ce51a5b9b4 100644
> --- a/include/hw/arm/smmuv3.h
> +++ b/include/hw/arm/smmuv3.h
> @@ -70,7 +70,7 @@ struct SMMUv3State {
> uint64_t msi_gpa;
> Error *migration_blocker;
> bool ril;
> - bool ats;
> + OnOffAuto ats;
> uint8_t oas;
> uint8_t ssidsize;
> };
> @@ -91,6 +91,8 @@ struct SMMUv3Class {
> ResettablePhases parent_phases;
> };
>
> +bool smmuv3_ats_enabled(struct SMMUv3State *s);
> +
> #define TYPE_ARM_SMMUV3 "arm-smmuv3"
> OBJECT_DECLARE_TYPE(SMMUv3State, SMMUv3Class, ARM_SMMUV3)
>
> --
> 2.43.0
On 3/18/26 10:31 AM, Shameer Kolothum Thodi wrote:
>
>> -----Original Message-----
>> From: Nathan Chen <nathanc@nvidia.com>
>> Sent: 17 March 2026 18:38
>> To: qemu-devel@nongnu.org; qemu-arm@nongnu.org
>> Cc: Eric Auger <eric.auger@redhat.com>; Peter Maydell
>> <peter.maydell@linaro.org>; Shannon Zhao <shannon.zhaosl@gmail.com>;
>> Michael S . Tsirkin <mst@redhat.com>; Igor Mammedov
>> <imammedo@redhat.com>; Ani Sinha <anisinha@redhat.com>; Paolo Bonzini
>> <pbonzini@redhat.com>; Daniel P . Berrangé <berrange@redhat.com>; Eric
>> Blake <eblake@redhat.com>; Markus Armbruster <armbru@redhat.com>;
>> Shameer Kolothum Thodi <skolothumtho@nvidia.com>; Matt Ochs
>> <mochs@nvidia.com>; Nicolin Chen <nicolinc@nvidia.com>; Nathan Chen
>> <nathanc@nvidia.com>
>> Subject: [PATCH v3 2/8] hw/arm/smmuv3-accel: Change ATS property to
>> OnOffAuto
>>
>> From: Nathan Chen <nathanc@nvidia.com>
>>
>> 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 <nathanc@nvidia.com>
>> ---
>> 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
Eric
>
> Thanks,
> Shameer
>
>> DEFINE_PROP_UINT8("oas", SMMUv3State, oas, 44),
>> DEFINE_PROP_UINT8("ssidsize", SMMUv3State, ssidsize, 0),
>> };
>> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
>> index 719d2f994e..591cfc993c 100644
>> --- a/hw/arm/virt-acpi-build.c
>> +++ b/hw/arm/virt-acpi-build.c
>> @@ -402,7 +402,7 @@ static int iort_smmuv3_devices(Object *obj, void
>> *opaque)
>>
>> bus = PCI_BUS(object_property_get_link(obj, "primary-bus",
>> &error_abort));
>> sdev.accel = object_property_get_bool(obj, "accel", &error_abort);
>> - sdev.ats = object_property_get_bool(obj, "ats", &error_abort);
>> + sdev.ats = smmuv3_ats_enabled(ARM_SMMUV3(obj));
>> 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/include/hw/arm/smmuv3.h b/include/hw/arm/smmuv3.h
>> index 26b2fc42fd..ce51a5b9b4 100644
>> --- a/include/hw/arm/smmuv3.h
>> +++ b/include/hw/arm/smmuv3.h
>> @@ -70,7 +70,7 @@ struct SMMUv3State {
>> uint64_t msi_gpa;
>> Error *migration_blocker;
>> bool ril;
>> - bool ats;
>> + OnOffAuto ats;
>> uint8_t oas;
>> uint8_t ssidsize;
>> };
>> @@ -91,6 +91,8 @@ struct SMMUv3Class {
>> ResettablePhases parent_phases;
>> };
>>
>> +bool smmuv3_ats_enabled(struct SMMUv3State *s);
>> +
>> #define TYPE_ARM_SMMUV3 "arm-smmuv3"
>> OBJECT_DECLARE_TYPE(SMMUv3State, SMMUv3Class, ARM_SMMUV3)
>>
>> --
>> 2.43.0
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<nathanc@nvidia.com>
>>> Sent: 17 March 2026 18:38
>>> To:qemu-devel@nongnu.org;qemu-arm@nongnu.org
>>> Cc: Eric Auger<eric.auger@redhat.com>; Peter Maydell
>>> <peter.maydell@linaro.org>; Shannon Zhao<shannon.zhaosl@gmail.com>;
>>> Michael S . Tsirkin<mst@redhat.com>; Igor Mammedov
>>> <imammedo@redhat.com>; Ani Sinha<anisinha@redhat.com>; Paolo Bonzini
>>> <pbonzini@redhat.com>; Daniel P . Berrangé<berrange@redhat.com>; Eric
>>> Blake<eblake@redhat.com>; Markus Armbruster<armbru@redhat.com>;
>>> Shameer Kolothum Thodi<skolothumtho@nvidia.com>; Matt Ochs
>>> <mochs@nvidia.com>; Nicolin Chen<nicolinc@nvidia.com>; Nathan Chen
>>> <nathanc@nvidia.com>
>>> Subject: [PATCH v3 2/8] hw/arm/smmuv3-accel: Change ATS property to
>>> OnOffAuto
>>>
>>> From: Nathan Chen<nathanc@nvidia.com>
>>>
>>> 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<nathanc@nvidia.com>
>>> ---
>>> 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
© 2016 - 2026 Red Hat, Inc.