From: Nathan Chen <nathanc@nvidia.com>
Allow accelerated SMMUv3 Address Translation Services support property
to be derived from host IOMMU capabilities. Derive host values using
IOMMU_GET_HW_INFO, retrieving ATS capability from IDR0.
Set the default value of ATS to auto. The default for ATS support used
to be set to off, but we change it to match what the host IOMMU
properties report.
Add a "ats-enabled" read-only property for smmuv3 to address an
expected bool for the "ats" property in iort_smmuv3_devices().
Signed-off-by: Nathan Chen <nathanc@nvidia.com>
---
hw/arm/smmuv3-accel.c | 25 +++++++++++++++++++++++--
hw/arm/smmuv3.c | 12 ++++++++++--
hw/arm/virt-acpi-build.c | 2 +-
include/hw/arm/smmuv3.h | 2 +-
4 files changed, 35 insertions(+), 6 deletions(-)
diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
index 617629bacd..8fec335557 100644
--- a/hw/arm/smmuv3-accel.c
+++ b/hw/arm/smmuv3-accel.c
@@ -52,6 +52,12 @@ static void smmuv3_accel_auto_finalise(SMMUv3State *s, PCIDevice *pdev,
return;
}
+ /* Update ATS if auto from info */
+ if (s->ats == ON_OFF_AUTO_AUTO) {
+ s->idr[0] = FIELD_DP32(s->idr[0], IDR0, ATS,
+ FIELD_EX32(info->idr[0], IDR0, ATS));
+ }
+
accel->auto_finalised = true;
}
@@ -124,6 +130,13 @@ smmuv3_accel_check_hw_compatible(SMMUv3State *s,
smmuv3_oas_bits(FIELD_EX32(s->idr[5], IDR5, OAS)));
return false;
}
+ /* Check ATS value opted is compatible with Host SMMUv3 */
+ if (FIELD_EX32(info->idr[0], IDR0, ATS) <
+ FIELD_EX32(s->idr[0], IDR0, ATS)) {
+ error_setg(errp, "Host SMMUv3 doesn't support Address Translation"
+ " Services");
+ return false;
+ }
/* QEMU SMMUv3 supports GRAN4K/GRAN16K/GRAN64K translation granules */
if (FIELD_EX32(info->idr[5], IDR5, GRAN4K) !=
@@ -844,8 +857,12 @@ 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 or OFF */
+ if (s->ats == ON_OFF_AUTO_ON) {
+ s->idr[0] = FIELD_DP32(s->idr[0], IDR0, ATS, 1);
+ } else if (s->ats == ON_OFF_AUTO_OFF) {
+ s->idr[0] = FIELD_DP32(s->idr[0], IDR0, ATS, 0);
+ }
/* Advertise 48-bit OAS in IDR5 when requested (default is 44 bits). */
if (s->oas == SMMU_OAS_48BIT) {
@@ -923,4 +940,8 @@ void smmuv3_accel_init(SMMUv3State *s)
s->s_accel = g_new0(SMMUv3AccelState, 1);
bs->iommu_ops = &smmuv3_accel_ops;
smmuv3_accel_as_init(s);
+
+ if (s->ats == ON_OFF_AUTO_AUTO) {
+ s->s_accel->auto_mode = true;
+ }
}
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 068108e49b..197ba7c77b 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -317,6 +317,12 @@ static void smmuv3_init_id_regs(SMMUv3State *s)
smmuv3_accel_idr_override(s);
}
+static bool get_ats_enabled(Object *obj, Error **errp)
+{
+ SMMUv3State *s = ARM_SMMUV3(obj);
+ 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 +1977,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;
}
@@ -2128,7 +2134,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_AUTO),
DEFINE_PROP_UINT8("oas", SMMUv3State, oas, 44),
DEFINE_PROP_UINT8("ssidsize", SMMUv3State, ssidsize, 0),
};
@@ -2153,6 +2159,8 @@ static void smmuv3_class_init(ObjectClass *klass, const void *data)
dc->hotpluggable = false;
dc->user_creatable = true;
+ object_class_property_add_bool(klass, "ats-enabled", get_ats_enabled, NULL);
+
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");
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 719d2f994e..6c77fc5f6a 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 = object_property_get_bool(obj, "ats-enabled", &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/include/hw/arm/smmuv3.h b/include/hw/arm/smmuv3.h
index 26b2fc42fd..2ca49ded36 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;
};
--
2.43.0Hi Nathan,
On 3/9/26 8:21 PM, Nathan Chen wrote:
> From: Nathan Chen <nathanc@nvidia.com>
>
> Allow accelerated SMMUv3 Address Translation Services support property
> to be derived from host IOMMU capabilities. Derive host values using
> IOMMU_GET_HW_INFO, retrieving ATS capability from IDR0.
>
> Set the default value of ATS to auto. The default for ATS support used
> to be set to off, but we change it to match what the host IOMMU
> properties report.
>
> Add a "ats-enabled" read-only property for smmuv3 to address an
> expected bool for the "ats" property in iort_smmuv3_devices().
>
> Signed-off-by: Nathan Chen <nathanc@nvidia.com>
> ---
> hw/arm/smmuv3-accel.c | 25 +++++++++++++++++++++++--
> hw/arm/smmuv3.c | 12 ++++++++++--
> hw/arm/virt-acpi-build.c | 2 +-
> include/hw/arm/smmuv3.h | 2 +-
> 4 files changed, 35 insertions(+), 6 deletions(-)
>
> diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
> index 617629bacd..8fec335557 100644
> --- a/hw/arm/smmuv3-accel.c
> +++ b/hw/arm/smmuv3-accel.c
> @@ -52,6 +52,12 @@ static void smmuv3_accel_auto_finalise(SMMUv3State *s, PCIDevice *pdev,
> return;
> }
>
> + /* Update ATS if auto from info */
> + if (s->ats == ON_OFF_AUTO_AUTO) {
> + s->idr[0] = FIELD_DP32(s->idr[0], IDR0, ATS,
> + FIELD_EX32(info->idr[0], IDR0, ATS));
> + }
> +
> accel->auto_finalised = true;
> }
>
> @@ -124,6 +130,13 @@ smmuv3_accel_check_hw_compatible(SMMUv3State *s,
> smmuv3_oas_bits(FIELD_EX32(s->idr[5], IDR5, OAS)));
> return false;
> }
> + /* Check ATS value opted is compatible with Host SMMUv3 */
> + if (FIELD_EX32(info->idr[0], IDR0, ATS) <
> + FIELD_EX32(s->idr[0], IDR0, ATS)) {
> + error_setg(errp, "Host SMMUv3 doesn't support Address Translation"
> + " Services");
> + return false;
> + }
Shouldn't this chunk be a separate fix for previous accel SMMU series?
>
> /* QEMU SMMUv3 supports GRAN4K/GRAN16K/GRAN64K translation granules */
> if (FIELD_EX32(info->idr[5], IDR5, GRAN4K) !=
> @@ -844,8 +857,12 @@ 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 or OFF */
> + if (s->ats == ON_OFF_AUTO_ON) {
> + s->idr[0] = FIELD_DP32(s->idr[0], IDR0, ATS, 1);
> + } else if (s->ats == ON_OFF_AUTO_OFF) {
> + s->idr[0] = FIELD_DP32(s->idr[0], IDR0, ATS, 0);
> + }
>
> /* Advertise 48-bit OAS in IDR5 when requested (default is 44 bits). */
> if (s->oas == SMMU_OAS_48BIT) {
> @@ -923,4 +940,8 @@ void smmuv3_accel_init(SMMUv3State *s)
> s->s_accel = g_new0(SMMUv3AccelState, 1);
> bs->iommu_ops = &smmuv3_accel_ops;
> smmuv3_accel_as_init(s);
> +
> + if (s->ats == ON_OFF_AUTO_AUTO) {
> + s->s_accel->auto_mode = true;
> + }
> }
> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> index 068108e49b..197ba7c77b 100644
> --- a/hw/arm/smmuv3.c
> +++ b/hw/arm/smmuv3.c
> @@ -317,6 +317,12 @@ static void smmuv3_init_id_regs(SMMUv3State *s)
> smmuv3_accel_idr_override(s);
> }
>
> +static bool get_ats_enabled(Object *obj, Error **errp)
> +{
> + SMMUv3State *s = ARM_SMMUV3(obj);
> + 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 +1977,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;
> }
> @@ -2128,7 +2134,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_AUTO),
> DEFINE_PROP_UINT8("oas", SMMUv3State, oas, 44),
> DEFINE_PROP_UINT8("ssidsize", SMMUv3State, ssidsize, 0),
> };
> @@ -2153,6 +2159,8 @@ static void smmuv3_class_init(ObjectClass *klass, const void *data)
> dc->hotpluggable = false;
> dc->user_creatable = true;
>
> + object_class_property_add_bool(klass, "ats-enabled", get_ats_enabled, NULL);
> +
> 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");
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 719d2f994e..6c77fc5f6a 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 = object_property_get_bool(obj, "ats-enabled", &error_abort);
are we obliged to use a prop here.
we can dynamic cast to the TYPE_ARM_SMMUV3 and use an exported helper, no?
> 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..2ca49ded36 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;
> };
Thanks
Eric
On 3/11/2026 11:10 AM, Eric Auger wrote:
> On 3/9/26 8:21 PM, Nathan Chen wrote:
>> From: Nathan Chen<nathanc@nvidia.com>
>>
>> Allow accelerated SMMUv3 Address Translation Services support property
>> to be derived from host IOMMU capabilities. Derive host values using
>> IOMMU_GET_HW_INFO, retrieving ATS capability from IDR0.
>>
>> Set the default value of ATS to auto. The default for ATS support used
>> to be set to off, but we change it to match what the host IOMMU
>> properties report.
>>
>> Add a "ats-enabled" read-only property for smmuv3 to address an
>> expected bool for the "ats" property in iort_smmuv3_devices().
>>
>> Signed-off-by: Nathan Chen<nathanc@nvidia.com>
>> ---
>> hw/arm/smmuv3-accel.c | 25 +++++++++++++++++++++++--
>> hw/arm/smmuv3.c | 12 ++++++++++--
>> hw/arm/virt-acpi-build.c | 2 +-
>> include/hw/arm/smmuv3.h | 2 +-
>> 4 files changed, 35 insertions(+), 6 deletions(-)
>>
>> diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
>> index 617629bacd..8fec335557 100644
>> --- a/hw/arm/smmuv3-accel.c
>> +++ b/hw/arm/smmuv3-accel.c
>> @@ -52,6 +52,12 @@ static void smmuv3_accel_auto_finalise(SMMUv3State *s, PCIDevice *pdev,
>> return;
>> }
>>
>> + /* Update ATS if auto from info */
>> + if (s->ats == ON_OFF_AUTO_AUTO) {
>> + s->idr[0] = FIELD_DP32(s->idr[0], IDR0, ATS,
>> + FIELD_EX32(info->idr[0], IDR0, ATS));
>> + }
>> +
>> accel->auto_finalised = true;
>> }
>>
>> @@ -124,6 +130,13 @@ smmuv3_accel_check_hw_compatible(SMMUv3State *s,
>> smmuv3_oas_bits(FIELD_EX32(s->idr[5], IDR5, OAS)));
>> return false;
>> }
>> + /* Check ATS value opted is compatible with Host SMMUv3 */
>> + if (FIELD_EX32(info->idr[0], IDR0, ATS) <
>> + FIELD_EX32(s->idr[0], IDR0, ATS)) {
>> + error_setg(errp, "Host SMMUv3 doesn't support Address Translation"
>> + " Services");
>> + return false;
>> + }
> Shouldn't this chunk be a separate fix for previous accel SMMU series?
>
Yes, I'll separate this out to be a separate commit in the next refresh.
>>
>> /* QEMU SMMUv3 supports GRAN4K/GRAN16K/GRAN64K translation granules */
>> if (FIELD_EX32(info->idr[5], IDR5, GRAN4K) !=
>> @@ -844,8 +857,12 @@ 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 or OFF */
>> + if (s->ats == ON_OFF_AUTO_ON) {
>> + s->idr[0] = FIELD_DP32(s->idr[0], IDR0, ATS, 1);
>> + } else if (s->ats == ON_OFF_AUTO_OFF) {
>> + s->idr[0] = FIELD_DP32(s->idr[0], IDR0, ATS, 0);
>> + }
>>
>> /* Advertise 48-bit OAS in IDR5 when requested (default is 44 bits). */
>> if (s->oas == SMMU_OAS_48BIT) {
>> @@ -923,4 +940,8 @@ void smmuv3_accel_init(SMMUv3State *s)
>> s->s_accel = g_new0(SMMUv3AccelState, 1);
>> bs->iommu_ops = &smmuv3_accel_ops;
>> smmuv3_accel_as_init(s);
>> +
>> + if (s->ats == ON_OFF_AUTO_AUTO) {
>> + s->s_accel->auto_mode = true;
>> + }
>> }
>> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
>> index 068108e49b..197ba7c77b 100644
>> --- a/hw/arm/smmuv3.c
>> +++ b/hw/arm/smmuv3.c
>> @@ -317,6 +317,12 @@ static void smmuv3_init_id_regs(SMMUv3State *s)
>> smmuv3_accel_idr_override(s);
>> }
>>
>> +static bool get_ats_enabled(Object *obj, Error **errp)
>> +{
>> + SMMUv3State *s = ARM_SMMUV3(obj);
>> + 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 +1977,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;
>> }
>> @@ -2128,7 +2134,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_AUTO),
>> DEFINE_PROP_UINT8("oas", SMMUv3State, oas, 44),
>> DEFINE_PROP_UINT8("ssidsize", SMMUv3State, ssidsize, 0),
>> };
>> @@ -2153,6 +2159,8 @@ static void smmuv3_class_init(ObjectClass *klass, const void *data)
>> dc->hotpluggable = false;
>> dc->user_creatable = true;
>>
>> + object_class_property_add_bool(klass, "ats-enabled", get_ats_enabled, NULL);
>> +
>> 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");
>> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
>> index 719d2f994e..6c77fc5f6a 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 = object_property_get_bool(obj, "ats-enabled", &error_abort);
> are we obliged to use a prop here.
> we can dynamic cast to the TYPE_ARM_SMMUV3 and use an exported helper, no?
I'll look into replacing this ats-enabled prop with a helper that
accepts the dynamic casted TYPE_ARM_SMMUV3 object and determines whether
ATS support is being advertised.
Thanks,
Nathan
On 3/9/26 8:21 PM, Nathan Chen wrote:
> From: Nathan Chen <nathanc@nvidia.com>
>
> Allow accelerated SMMUv3 Address Translation Services support property
> to be derived from host IOMMU capabilities. Derive host values using
> IOMMU_GET_HW_INFO, retrieving ATS capability from IDR0.
>
> Set the default value of ATS to auto. The default for ATS support used
> to be set to off, but we change it to match what the host IOMMU
> properties report.
>
> Add a "ats-enabled" read-only property for smmuv3 to address an
> expected bool for the "ats" property in iort_smmuv3_devices().
>
> Signed-off-by: Nathan Chen <nathanc@nvidia.com>
> ---
> hw/arm/smmuv3-accel.c | 25 +++++++++++++++++++++++--
> hw/arm/smmuv3.c | 12 ++++++++++--
> hw/arm/virt-acpi-build.c | 2 +-
> include/hw/arm/smmuv3.h | 2 +-
> 4 files changed, 35 insertions(+), 6 deletions(-)
>
> diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
> index 617629bacd..8fec335557 100644
> --- a/hw/arm/smmuv3-accel.c
> +++ b/hw/arm/smmuv3-accel.c
> @@ -52,6 +52,12 @@ static void smmuv3_accel_auto_finalise(SMMUv3State *s, PCIDevice *pdev,
> return;
> }
>
> + /* Update ATS if auto from info */
> + if (s->ats == ON_OFF_AUTO_AUTO) {
> + s->idr[0] = FIELD_DP32(s->idr[0], IDR0, ATS,
> + FIELD_EX32(info->idr[0], IDR0, ATS));
> + }
> +
> accel->auto_finalised = true;
> }
>
> @@ -124,6 +130,13 @@ smmuv3_accel_check_hw_compatible(SMMUv3State *s,
> smmuv3_oas_bits(FIELD_EX32(s->idr[5], IDR5, OAS)));
> return false;
> }
> + /* Check ATS value opted is compatible with Host SMMUv3 */
> + if (FIELD_EX32(info->idr[0], IDR0, ATS) <
> + FIELD_EX32(s->idr[0], IDR0, ATS)) {
> + error_setg(errp, "Host SMMUv3 doesn't support Address Translation"
> + " Services");
> + return false;
> + }
>
> /* QEMU SMMUv3 supports GRAN4K/GRAN16K/GRAN64K translation granules */
> if (FIELD_EX32(info->idr[5], IDR5, GRAN4K) !=
> @@ -844,8 +857,12 @@ 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 or OFF */
> + if (s->ats == ON_OFF_AUTO_ON) {
> + s->idr[0] = FIELD_DP32(s->idr[0], IDR0, ATS, 1);
> + } else if (s->ats == ON_OFF_AUTO_OFF) {
> + s->idr[0] = FIELD_DP32(s->idr[0], IDR0, ATS, 0);
> + }
>
> /* Advertise 48-bit OAS in IDR5 when requested (default is 44 bits). */
> if (s->oas == SMMU_OAS_48BIT) {
> @@ -923,4 +940,8 @@ void smmuv3_accel_init(SMMUv3State *s)
> s->s_accel = g_new0(SMMUv3AccelState, 1);
> bs->iommu_ops = &smmuv3_accel_ops;
> smmuv3_accel_as_init(s);
> +
> + if (s->ats == ON_OFF_AUTO_AUTO) {
> + s->s_accel->auto_mode = true;
> + }
> }
> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> index 068108e49b..197ba7c77b 100644
> --- a/hw/arm/smmuv3.c
> +++ b/hw/arm/smmuv3.c
> @@ -317,6 +317,12 @@ static void smmuv3_init_id_regs(SMMUv3State *s)
> smmuv3_accel_idr_override(s);
> }
>
> +static bool get_ats_enabled(Object *obj, Error **errp)
> +{
> + SMMUv3State *s = ARM_SMMUV3(obj);
> + 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 +1977,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;
> }
> @@ -2128,7 +2134,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_AUTO),
In general you cannot change the default value like that. This requires
compat handling
in virt machine: See
https://lore.kernel.org/all/20240307134445.92296-4-eric.auger@redhat.com/
for instance
Eric
> DEFINE_PROP_UINT8("oas", SMMUv3State, oas, 44),
> DEFINE_PROP_UINT8("ssidsize", SMMUv3State, ssidsize, 0),
> };
> @@ -2153,6 +2159,8 @@ static void smmuv3_class_init(ObjectClass *klass, const void *data)
> dc->hotpluggable = false;
> dc->user_creatable = true;
>
> + object_class_property_add_bool(klass, "ats-enabled", get_ats_enabled, NULL);
> +
> 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");
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 719d2f994e..6c77fc5f6a 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 = object_property_get_bool(obj, "ats-enabled", &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/include/hw/arm/smmuv3.h b/include/hw/arm/smmuv3.h
> index 26b2fc42fd..2ca49ded36 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;
> };
On 3/11/2026 10:46 AM, Eric Auger wrote:
>
> On 3/9/26 8:21 PM, Nathan Chen wrote:
>> From: Nathan Chen<nathanc@nvidia.com>
>>
>> Allow accelerated SMMUv3 Address Translation Services support property
>> to be derived from host IOMMU capabilities. Derive host values using
>> IOMMU_GET_HW_INFO, retrieving ATS capability from IDR0.
>>
>> Set the default value of ATS to auto. The default for ATS support used
>> to be set to off, but we change it to match what the host IOMMU
>> properties report.
>>
>> Add a "ats-enabled" read-only property for smmuv3 to address an
>> expected bool for the "ats" property in iort_smmuv3_devices().
>>
>> Signed-off-by: Nathan Chen<nathanc@nvidia.com>
>> ---
>> hw/arm/smmuv3-accel.c | 25 +++++++++++++++++++++++--
>> hw/arm/smmuv3.c | 12 ++++++++++--
>> hw/arm/virt-acpi-build.c | 2 +-
>> include/hw/arm/smmuv3.h | 2 +-
>> 4 files changed, 35 insertions(+), 6 deletions(-)
>>
>> diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
>> index 617629bacd..8fec335557 100644
>> --- a/hw/arm/smmuv3-accel.c
>> +++ b/hw/arm/smmuv3-accel.c
>> @@ -52,6 +52,12 @@ static void smmuv3_accel_auto_finalise(SMMUv3State *s, PCIDevice *pdev,
>> return;
>> }
>>
>> + /* Update ATS if auto from info */
>> + if (s->ats == ON_OFF_AUTO_AUTO) {
>> + s->idr[0] = FIELD_DP32(s->idr[0], IDR0, ATS,
>> + FIELD_EX32(info->idr[0], IDR0, ATS));
>> + }
>> +
>> accel->auto_finalised = true;
>> }
>>
>> @@ -124,6 +130,13 @@ smmuv3_accel_check_hw_compatible(SMMUv3State *s,
>> smmuv3_oas_bits(FIELD_EX32(s->idr[5], IDR5, OAS)));
>> return false;
>> }
>> + /* Check ATS value opted is compatible with Host SMMUv3 */
>> + if (FIELD_EX32(info->idr[0], IDR0, ATS) <
>> + FIELD_EX32(s->idr[0], IDR0, ATS)) {
>> + error_setg(errp, "Host SMMUv3 doesn't support Address Translation"
>> + " Services");
>> + return false;
>> + }
>>
>> /* QEMU SMMUv3 supports GRAN4K/GRAN16K/GRAN64K translation granules */
>> if (FIELD_EX32(info->idr[5], IDR5, GRAN4K) !=
>> @@ -844,8 +857,12 @@ 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 or OFF */
>> + if (s->ats == ON_OFF_AUTO_ON) {
>> + s->idr[0] = FIELD_DP32(s->idr[0], IDR0, ATS, 1);
>> + } else if (s->ats == ON_OFF_AUTO_OFF) {
>> + s->idr[0] = FIELD_DP32(s->idr[0], IDR0, ATS, 0);
>> + }
>>
>> /* Advertise 48-bit OAS in IDR5 when requested (default is 44 bits). */
>> if (s->oas == SMMU_OAS_48BIT) {
>> @@ -923,4 +940,8 @@ void smmuv3_accel_init(SMMUv3State *s)
>> s->s_accel = g_new0(SMMUv3AccelState, 1);
>> bs->iommu_ops = &smmuv3_accel_ops;
>> smmuv3_accel_as_init(s);
>> +
>> + if (s->ats == ON_OFF_AUTO_AUTO) {
>> + s->s_accel->auto_mode = true;
>> + }
>> }
>> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
>> index 068108e49b..197ba7c77b 100644
>> --- a/hw/arm/smmuv3.c
>> +++ b/hw/arm/smmuv3.c
>> @@ -317,6 +317,12 @@ static void smmuv3_init_id_regs(SMMUv3State *s)
>> smmuv3_accel_idr_override(s);
>> }
>>
>> +static bool get_ats_enabled(Object *obj, Error **errp)
>> +{
>> + SMMUv3State *s = ARM_SMMUV3(obj);
>> + 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 +1977,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;
>> }
>> @@ -2128,7 +2134,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_AUTO),
> In general you cannot change the default value like that. This requires
> compat handling
> in virt machine: See
> https://lore.kernel.org/all/20240307134445.92296-4-eric.auger@redhat.com/
> for instance
Ok, I will handle the default value change similarly in the next
revision to account for virt machine compat handling.
Thanks,
Nathan
Nathan Chen <nathanc@nvidia.com> writes:
> From: Nathan Chen <nathanc@nvidia.com>
>
> Allow accelerated SMMUv3 Address Translation Services support property
> to be derived from host IOMMU capabilities. Derive host values using
> IOMMU_GET_HW_INFO, retrieving ATS capability from IDR0.
>
> Set the default value of ATS to auto. The default for ATS support used
> to be set to off, but we change it to match what the host IOMMU
> properties report.
>
> Add a "ats-enabled" read-only property for smmuv3 to address an
> expected bool for the "ats" property in iort_smmuv3_devices().
>
> Signed-off-by: Nathan Chen <nathanc@nvidia.com>
> ---
> hw/arm/smmuv3-accel.c | 25 +++++++++++++++++++++++--
> hw/arm/smmuv3.c | 12 ++++++++++--
> hw/arm/virt-acpi-build.c | 2 +-
> include/hw/arm/smmuv3.h | 2 +-
> 4 files changed, 35 insertions(+), 6 deletions(-)
>
> diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
> index 617629bacd..8fec335557 100644
> --- a/hw/arm/smmuv3-accel.c
> +++ b/hw/arm/smmuv3-accel.c
> @@ -52,6 +52,12 @@ static void smmuv3_accel_auto_finalise(SMMUv3State *s, PCIDevice *pdev,
> return;
> }
>
> + /* Update ATS if auto from info */
> + if (s->ats == ON_OFF_AUTO_AUTO) {
> + s->idr[0] = FIELD_DP32(s->idr[0], IDR0, ATS,
> + FIELD_EX32(info->idr[0], IDR0, ATS));
> + }
> +
> accel->auto_finalised = true;
> }
>
> @@ -124,6 +130,13 @@ smmuv3_accel_check_hw_compatible(SMMUv3State *s,
> smmuv3_oas_bits(FIELD_EX32(s->idr[5], IDR5, OAS)));
> return false;
> }
> + /* Check ATS value opted is compatible with Host SMMUv3 */
> + if (FIELD_EX32(info->idr[0], IDR0, ATS) <
> + FIELD_EX32(s->idr[0], IDR0, ATS)) {
> + error_setg(errp, "Host SMMUv3 doesn't support Address Translation"
> + " Services");
> + return false;
> + }
>
> /* QEMU SMMUv3 supports GRAN4K/GRAN16K/GRAN64K translation granules */
> if (FIELD_EX32(info->idr[5], IDR5, GRAN4K) !=
> @@ -844,8 +857,12 @@ 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 or OFF */
> + if (s->ats == ON_OFF_AUTO_ON) {
> + s->idr[0] = FIELD_DP32(s->idr[0], IDR0, ATS, 1);
> + } else if (s->ats == ON_OFF_AUTO_OFF) {
> + s->idr[0] = FIELD_DP32(s->idr[0], IDR0, ATS, 0);
> + }
>
> /* Advertise 48-bit OAS in IDR5 when requested (default is 44 bits). */
> if (s->oas == SMMU_OAS_48BIT) {
> @@ -923,4 +940,8 @@ void smmuv3_accel_init(SMMUv3State *s)
> s->s_accel = g_new0(SMMUv3AccelState, 1);
> bs->iommu_ops = &smmuv3_accel_ops;
> smmuv3_accel_as_init(s);
> +
> + if (s->ats == ON_OFF_AUTO_AUTO) {
> + s->s_accel->auto_mode = true;
> + }
> }
> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> index 068108e49b..197ba7c77b 100644
> --- a/hw/arm/smmuv3.c
> +++ b/hw/arm/smmuv3.c
> @@ -317,6 +317,12 @@ static void smmuv3_init_id_regs(SMMUv3State *s)
> smmuv3_accel_idr_override(s);
> }
>
> +static bool get_ats_enabled(Object *obj, Error **errp)
> +{
> + SMMUv3State *s = ARM_SMMUV3(obj);
> + 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 +1977,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;
> }
> @@ -2128,7 +2134,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_AUTO),
Is property "ats" accessible via QMP or JSON command line? If yes, this
is an incompatible change: JSON values false and true no longer work.
> DEFINE_PROP_UINT8("oas", SMMUv3State, oas, 44),
> DEFINE_PROP_UINT8("ssidsize", SMMUv3State, ssidsize, 0),
> };
> @@ -2153,6 +2159,8 @@ static void smmuv3_class_init(ObjectClass *klass, const void *data)
> dc->hotpluggable = false;
> dc->user_creatable = true;
>
> + object_class_property_add_bool(klass, "ats-enabled", get_ats_enabled, NULL);
> +
> 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");
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 719d2f994e..6c77fc5f6a 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 = object_property_get_bool(obj, "ats-enabled", &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/include/hw/arm/smmuv3.h b/include/hw/arm/smmuv3.h
> index 26b2fc42fd..2ca49ded36 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;
> };
On 3/10/26 8:05 AM, Markus Armbruster wrote:
> Nathan Chen <nathanc@nvidia.com> writes:
>
>> From: Nathan Chen <nathanc@nvidia.com>
>>
>> Allow accelerated SMMUv3 Address Translation Services support property
>> to be derived from host IOMMU capabilities. Derive host values using
>> IOMMU_GET_HW_INFO, retrieving ATS capability from IDR0.
>>
>> Set the default value of ATS to auto. The default for ATS support used
>> to be set to off, but we change it to match what the host IOMMU
>> properties report.
>>
>> Add a "ats-enabled" read-only property for smmuv3 to address an
>> expected bool for the "ats" property in iort_smmuv3_devices().
>>
>> Signed-off-by: Nathan Chen <nathanc@nvidia.com>
>> ---
>> hw/arm/smmuv3-accel.c | 25 +++++++++++++++++++++++--
>> hw/arm/smmuv3.c | 12 ++++++++++--
>> hw/arm/virt-acpi-build.c | 2 +-
>> include/hw/arm/smmuv3.h | 2 +-
>> 4 files changed, 35 insertions(+), 6 deletions(-)
>>
>> diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
>> index 617629bacd..8fec335557 100644
>> --- a/hw/arm/smmuv3-accel.c
>> +++ b/hw/arm/smmuv3-accel.c
>> @@ -52,6 +52,12 @@ static void smmuv3_accel_auto_finalise(SMMUv3State *s, PCIDevice *pdev,
>> return;
>> }
>>
>> + /* Update ATS if auto from info */
>> + if (s->ats == ON_OFF_AUTO_AUTO) {
>> + s->idr[0] = FIELD_DP32(s->idr[0], IDR0, ATS,
>> + FIELD_EX32(info->idr[0], IDR0, ATS));
>> + }
>> +
>> accel->auto_finalised = true;
>> }
>>
>> @@ -124,6 +130,13 @@ smmuv3_accel_check_hw_compatible(SMMUv3State *s,
>> smmuv3_oas_bits(FIELD_EX32(s->idr[5], IDR5, OAS)));
>> return false;
>> }
>> + /* Check ATS value opted is compatible with Host SMMUv3 */
>> + if (FIELD_EX32(info->idr[0], IDR0, ATS) <
>> + FIELD_EX32(s->idr[0], IDR0, ATS)) {
>> + error_setg(errp, "Host SMMUv3 doesn't support Address Translation"
>> + " Services");
>> + return false;
>> + }
>>
>> /* QEMU SMMUv3 supports GRAN4K/GRAN16K/GRAN64K translation granules */
>> if (FIELD_EX32(info->idr[5], IDR5, GRAN4K) !=
>> @@ -844,8 +857,12 @@ 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 or OFF */
>> + if (s->ats == ON_OFF_AUTO_ON) {
>> + s->idr[0] = FIELD_DP32(s->idr[0], IDR0, ATS, 1);
>> + } else if (s->ats == ON_OFF_AUTO_OFF) {
>> + s->idr[0] = FIELD_DP32(s->idr[0], IDR0, ATS, 0);
>> + }
>>
>> /* Advertise 48-bit OAS in IDR5 when requested (default is 44 bits). */
>> if (s->oas == SMMU_OAS_48BIT) {
>> @@ -923,4 +940,8 @@ void smmuv3_accel_init(SMMUv3State *s)
>> s->s_accel = g_new0(SMMUv3AccelState, 1);
>> bs->iommu_ops = &smmuv3_accel_ops;
>> smmuv3_accel_as_init(s);
>> +
>> + if (s->ats == ON_OFF_AUTO_AUTO) {
>> + s->s_accel->auto_mode = true;
>> + }
>> }
>> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
>> index 068108e49b..197ba7c77b 100644
>> --- a/hw/arm/smmuv3.c
>> +++ b/hw/arm/smmuv3.c
>> @@ -317,6 +317,12 @@ static void smmuv3_init_id_regs(SMMUv3State *s)
>> smmuv3_accel_idr_override(s);
>> }
>>
>> +static bool get_ats_enabled(Object *obj, Error **errp)
>> +{
>> + SMMUv3State *s = ARM_SMMUV3(obj);
>> + 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 +1977,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;
>> }
>> @@ -2128,7 +2134,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_AUTO),
> Is property "ats" accessible via QMP or JSON command line? If yes, this
> is an incompatible change: JSON values false and true no longer work.
So what do you recommend to extend the property values. I guess we had
some existing scenarios happening in the past. What is the best way to
proceed?
Eric
>
>> DEFINE_PROP_UINT8("oas", SMMUv3State, oas, 44),
>> DEFINE_PROP_UINT8("ssidsize", SMMUv3State, ssidsize, 0),
>> };
>> @@ -2153,6 +2159,8 @@ static void smmuv3_class_init(ObjectClass *klass, const void *data)
>> dc->hotpluggable = false;
>> dc->user_creatable = true;
>>
>> + object_class_property_add_bool(klass, "ats-enabled", get_ats_enabled, NULL);
>> +
>> 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");
>> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
>> index 719d2f994e..6c77fc5f6a 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 = object_property_get_bool(obj, "ats-enabled", &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/include/hw/arm/smmuv3.h b/include/hw/arm/smmuv3.h
>> index 26b2fc42fd..2ca49ded36 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;
>> };
On 3/11/2026 8:31 AM, Eric Auger wrote:
> On 3/10/26 8:05 AM, Markus Armbruster wrote:
>> Nathan Chen<nathanc@nvidia.com> writes:
>>
>>> From: Nathan Chen<nathanc@nvidia.com>
>>>
>>> Allow accelerated SMMUv3 Address Translation Services support property
>>> to be derived from host IOMMU capabilities. Derive host values using
>>> IOMMU_GET_HW_INFO, retrieving ATS capability from IDR0.
>>>
>>> Set the default value of ATS to auto. The default for ATS support used
>>> to be set to off, but we change it to match what the host IOMMU
>>> properties report.
>>>
>>> Add a "ats-enabled" read-only property for smmuv3 to address an
>>> expected bool for the "ats" property in iort_smmuv3_devices().
>>>
>>> Signed-off-by: Nathan Chen<nathanc@nvidia.com>
>>> ---
>>> hw/arm/smmuv3-accel.c | 25 +++++++++++++++++++++++--
>>> hw/arm/smmuv3.c | 12 ++++++++++--
>>> hw/arm/virt-acpi-build.c | 2 +-
>>> include/hw/arm/smmuv3.h | 2 +-
>>> 4 files changed, 35 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
>>> index 617629bacd..8fec335557 100644
>>> --- a/hw/arm/smmuv3-accel.c
>>> +++ b/hw/arm/smmuv3-accel.c
>>> @@ -52,6 +52,12 @@ static void smmuv3_accel_auto_finalise(SMMUv3State *s, PCIDevice *pdev,
>>> return;
>>> }
>>>
>>> + /* Update ATS if auto from info */
>>> + if (s->ats == ON_OFF_AUTO_AUTO) {
>>> + s->idr[0] = FIELD_DP32(s->idr[0], IDR0, ATS,
>>> + FIELD_EX32(info->idr[0], IDR0, ATS));
>>> + }
>>> +
>>> accel->auto_finalised = true;
>>> }
>>>
>>> @@ -124,6 +130,13 @@ smmuv3_accel_check_hw_compatible(SMMUv3State *s,
>>> smmuv3_oas_bits(FIELD_EX32(s->idr[5], IDR5, OAS)));
>>> return false;
>>> }
>>> + /* Check ATS value opted is compatible with Host SMMUv3 */
>>> + if (FIELD_EX32(info->idr[0], IDR0, ATS) <
>>> + FIELD_EX32(s->idr[0], IDR0, ATS)) {
>>> + error_setg(errp, "Host SMMUv3 doesn't support Address Translation"
>>> + " Services");
>>> + return false;
>>> + }
>>>
>>> /* QEMU SMMUv3 supports GRAN4K/GRAN16K/GRAN64K translation granules */
>>> if (FIELD_EX32(info->idr[5], IDR5, GRAN4K) !=
>>> @@ -844,8 +857,12 @@ 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 or OFF */
>>> + if (s->ats == ON_OFF_AUTO_ON) {
>>> + s->idr[0] = FIELD_DP32(s->idr[0], IDR0, ATS, 1);
>>> + } else if (s->ats == ON_OFF_AUTO_OFF) {
>>> + s->idr[0] = FIELD_DP32(s->idr[0], IDR0, ATS, 0);
>>> + }
>>>
>>> /* Advertise 48-bit OAS in IDR5 when requested (default is 44 bits). */
>>> if (s->oas == SMMU_OAS_48BIT) {
>>> @@ -923,4 +940,8 @@ void smmuv3_accel_init(SMMUv3State *s)
>>> s->s_accel = g_new0(SMMUv3AccelState, 1);
>>> bs->iommu_ops = &smmuv3_accel_ops;
>>> smmuv3_accel_as_init(s);
>>> +
>>> + if (s->ats == ON_OFF_AUTO_AUTO) {
>>> + s->s_accel->auto_mode = true;
>>> + }
>>> }
>>> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
>>> index 068108e49b..197ba7c77b 100644
>>> --- a/hw/arm/smmuv3.c
>>> +++ b/hw/arm/smmuv3.c
>>> @@ -317,6 +317,12 @@ static void smmuv3_init_id_regs(SMMUv3State *s)
>>> smmuv3_accel_idr_override(s);
>>> }
>>>
>>> +static bool get_ats_enabled(Object *obj, Error **errp)
>>> +{
>>> + SMMUv3State *s = ARM_SMMUV3(obj);
>>> + 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 +1977,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;
>>> }
>>> @@ -2128,7 +2134,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_AUTO),
>> Is property "ats" accessible via QMP or JSON command line? If yes, this
>> is an incompatible change: JSON values false and true no longer work.
> So what do you recommend to extend the property values. I guess we had
> some existing scenarios happening in the past. What is the best way to
> proceed?
Is it a requirement to ensure boolean values are still supported when
switching to OnOffAuto? I found that the x86-iommu intremap property had
a change from DEFINE_PROP_BOOL to DEFINE_PROP_ON_OFF_AUTO [0].
Should I add a custom property setter that accepts both boolean and
on/off/auto for backward compatibility, or is following the intremap
precedent acceptable? I'm happy to implement either approach.
[0]
https://github.com/qemu/qemu/commit/a924b3d8df55a395891fd5ed341d0deb135d9aa6
Thanks,
Nathan
On 3/11/26 6:08 PM, Nathan Chen wrote:
>
>
> On 3/11/2026 8:31 AM, Eric Auger wrote:
>> On 3/10/26 8:05 AM, Markus Armbruster wrote:
>>> Nathan Chen<nathanc@nvidia.com> writes:
>>>
>>>> From: Nathan Chen<nathanc@nvidia.com>
>>>>
>>>> Allow accelerated SMMUv3 Address Translation Services support property
>>>> to be derived from host IOMMU capabilities. Derive host values using
>>>> IOMMU_GET_HW_INFO, retrieving ATS capability from IDR0.
>>>>
>>>> Set the default value of ATS to auto. The default for ATS support used
>>>> to be set to off, but we change it to match what the host IOMMU
>>>> properties report.
>>>>
>>>> Add a "ats-enabled" read-only property for smmuv3 to address an
>>>> expected bool for the "ats" property in iort_smmuv3_devices().
>>>>
>>>> Signed-off-by: Nathan Chen<nathanc@nvidia.com>
>>>> ---
>>>> hw/arm/smmuv3-accel.c | 25 +++++++++++++++++++++++--
>>>> hw/arm/smmuv3.c | 12 ++++++++++--
>>>> hw/arm/virt-acpi-build.c | 2 +-
>>>> include/hw/arm/smmuv3.h | 2 +-
>>>> 4 files changed, 35 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
>>>> index 617629bacd..8fec335557 100644
>>>> --- a/hw/arm/smmuv3-accel.c
>>>> +++ b/hw/arm/smmuv3-accel.c
>>>> @@ -52,6 +52,12 @@ static void
>>>> smmuv3_accel_auto_finalise(SMMUv3State *s, PCIDevice *pdev,
>>>> return;
>>>> }
>>>> + /* Update ATS if auto from info */
>>>> + if (s->ats == ON_OFF_AUTO_AUTO) {
>>>> + s->idr[0] = FIELD_DP32(s->idr[0], IDR0, ATS,
>>>> + FIELD_EX32(info->idr[0], IDR0, ATS));
>>>> + }
>>>> +
>>>> accel->auto_finalised = true;
>>>> }
>>>> @@ -124,6 +130,13 @@ smmuv3_accel_check_hw_compatible(SMMUv3State
>>>> *s,
>>>> smmuv3_oas_bits(FIELD_EX32(s->idr[5], IDR5,
>>>> OAS)));
>>>> return false;
>>>> }
>>>> + /* Check ATS value opted is compatible with Host SMMUv3 */
>>>> + if (FIELD_EX32(info->idr[0], IDR0, ATS) <
>>>> + FIELD_EX32(s->idr[0], IDR0, ATS)) {
>>>> + error_setg(errp, "Host SMMUv3 doesn't support Address
>>>> Translation"
>>>> + " Services");
>>>> + return false;
>>>> + }
>>>> /* QEMU SMMUv3 supports GRAN4K/GRAN16K/GRAN64K translation
>>>> granules */
>>>> if (FIELD_EX32(info->idr[5], IDR5, GRAN4K) !=
>>>> @@ -844,8 +857,12 @@ 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 or OFF */
>>>> + if (s->ats == ON_OFF_AUTO_ON) {
>>>> + s->idr[0] = FIELD_DP32(s->idr[0], IDR0, ATS, 1);
>>>> + } else if (s->ats == ON_OFF_AUTO_OFF) {
>>>> + s->idr[0] = FIELD_DP32(s->idr[0], IDR0, ATS, 0);
>>>> + }
>>>> /* Advertise 48-bit OAS in IDR5 when requested (default is
>>>> 44 bits). */
>>>> if (s->oas == SMMU_OAS_48BIT) {
>>>> @@ -923,4 +940,8 @@ void smmuv3_accel_init(SMMUv3State *s)
>>>> s->s_accel = g_new0(SMMUv3AccelState, 1);
>>>> bs->iommu_ops = &smmuv3_accel_ops;
>>>> smmuv3_accel_as_init(s);
>>>> +
>>>> + if (s->ats == ON_OFF_AUTO_AUTO) {
>>>> + s->s_accel->auto_mode = true;
>>>> + }
>>>> }
>>>> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
>>>> index 068108e49b..197ba7c77b 100644
>>>> --- a/hw/arm/smmuv3.c
>>>> +++ b/hw/arm/smmuv3.c
>>>> @@ -317,6 +317,12 @@ static void smmuv3_init_id_regs(SMMUv3State *s)
>>>> smmuv3_accel_idr_override(s);
>>>> }
>>>> +static bool get_ats_enabled(Object *obj, Error **errp)
>>>> +{
>>>> + SMMUv3State *s = ARM_SMMUV3(obj);
>>>> + 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 +1977,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;
>>>> }
>>>> @@ -2128,7 +2134,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_AUTO),
>>> Is property "ats" accessible via QMP or JSON command line? If yes,
>>> this
>>> is an incompatible change: JSON values false and true no longer work.
>> So what do you recommend to extend the property values. I guess we had
>> some existing scenarios happening in the past. What is the best way to
>> proceed?
>
> Is it a requirement to ensure boolean values are still supported when
> switching to OnOffAuto? I found that the x86-iommu intremap property
> had a change from DEFINE_PROP_BOOL to DEFINE_PROP_ON_OFF_AUTO [0].
>
> Should I add a custom property setter that accepts both boolean and
> on/off/auto for backward compatibility, or is following the intremap
> precedent acceptable? I'm happy to implement either approach.
>
> [0]
> https://github.com/qemu/qemu/commit/a924b3d8df55a395891fd5ed341d0deb135d9aa6
For the notice, this happened in 2020:
a924b3d8df55 (HEAD) x86-iommu: switch intr_supported to OnOffAuto type
and the previous bool properties was introduced in 1121e0afdcf (in 2016)
By the way I don't say this is something that is good ;-)
Eric
>
> Thanks,
> Nathan
>
On 3/11/26 6:08 PM, Nathan Chen wrote:
>
>
> On 3/11/2026 8:31 AM, Eric Auger wrote:
>> On 3/10/26 8:05 AM, Markus Armbruster wrote:
>>> Nathan Chen<nathanc@nvidia.com> writes:
>>>
>>>> From: Nathan Chen<nathanc@nvidia.com>
>>>>
>>>> Allow accelerated SMMUv3 Address Translation Services support property
>>>> to be derived from host IOMMU capabilities. Derive host values using
>>>> IOMMU_GET_HW_INFO, retrieving ATS capability from IDR0.
>>>>
>>>> Set the default value of ATS to auto. The default for ATS support used
>>>> to be set to off, but we change it to match what the host IOMMU
>>>> properties report.
>>>>
>>>> Add a "ats-enabled" read-only property for smmuv3 to address an
>>>> expected bool for the "ats" property in iort_smmuv3_devices().
>>>>
>>>> Signed-off-by: Nathan Chen<nathanc@nvidia.com>
>>>> ---
>>>> hw/arm/smmuv3-accel.c | 25 +++++++++++++++++++++++--
>>>> hw/arm/smmuv3.c | 12 ++++++++++--
>>>> hw/arm/virt-acpi-build.c | 2 +-
>>>> include/hw/arm/smmuv3.h | 2 +-
>>>> 4 files changed, 35 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
>>>> index 617629bacd..8fec335557 100644
>>>> --- a/hw/arm/smmuv3-accel.c
>>>> +++ b/hw/arm/smmuv3-accel.c
>>>> @@ -52,6 +52,12 @@ static void
>>>> smmuv3_accel_auto_finalise(SMMUv3State *s, PCIDevice *pdev,
>>>> return;
>>>> }
>>>> + /* Update ATS if auto from info */
>>>> + if (s->ats == ON_OFF_AUTO_AUTO) {
>>>> + s->idr[0] = FIELD_DP32(s->idr[0], IDR0, ATS,
>>>> + FIELD_EX32(info->idr[0], IDR0, ATS));
>>>> + }
>>>> +
>>>> accel->auto_finalised = true;
>>>> }
>>>> @@ -124,6 +130,13 @@ smmuv3_accel_check_hw_compatible(SMMUv3State
>>>> *s,
>>>> smmuv3_oas_bits(FIELD_EX32(s->idr[5], IDR5,
>>>> OAS)));
>>>> return false;
>>>> }
>>>> + /* Check ATS value opted is compatible with Host SMMUv3 */
>>>> + if (FIELD_EX32(info->idr[0], IDR0, ATS) <
>>>> + FIELD_EX32(s->idr[0], IDR0, ATS)) {
>>>> + error_setg(errp, "Host SMMUv3 doesn't support Address
>>>> Translation"
>>>> + " Services");
>>>> + return false;
>>>> + }
>>>> /* QEMU SMMUv3 supports GRAN4K/GRAN16K/GRAN64K translation
>>>> granules */
>>>> if (FIELD_EX32(info->idr[5], IDR5, GRAN4K) !=
>>>> @@ -844,8 +857,12 @@ 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 or OFF */
>>>> + if (s->ats == ON_OFF_AUTO_ON) {
>>>> + s->idr[0] = FIELD_DP32(s->idr[0], IDR0, ATS, 1);
>>>> + } else if (s->ats == ON_OFF_AUTO_OFF) {
>>>> + s->idr[0] = FIELD_DP32(s->idr[0], IDR0, ATS, 0);
>>>> + }
>>>> /* Advertise 48-bit OAS in IDR5 when requested (default is
>>>> 44 bits). */
>>>> if (s->oas == SMMU_OAS_48BIT) {
>>>> @@ -923,4 +940,8 @@ void smmuv3_accel_init(SMMUv3State *s)
>>>> s->s_accel = g_new0(SMMUv3AccelState, 1);
>>>> bs->iommu_ops = &smmuv3_accel_ops;
>>>> smmuv3_accel_as_init(s);
>>>> +
>>>> + if (s->ats == ON_OFF_AUTO_AUTO) {
>>>> + s->s_accel->auto_mode = true;
>>>> + }
>>>> }
>>>> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
>>>> index 068108e49b..197ba7c77b 100644
>>>> --- a/hw/arm/smmuv3.c
>>>> +++ b/hw/arm/smmuv3.c
>>>> @@ -317,6 +317,12 @@ static void smmuv3_init_id_regs(SMMUv3State *s)
>>>> smmuv3_accel_idr_override(s);
>>>> }
>>>> +static bool get_ats_enabled(Object *obj, Error **errp)
>>>> +{
>>>> + SMMUv3State *s = ARM_SMMUV3(obj);
>>>> + 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 +1977,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;
>>>> }
>>>> @@ -2128,7 +2134,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_AUTO),
>>> Is property "ats" accessible via QMP or JSON command line? If yes,
>>> this
>>> is an incompatible change: JSON values false and true no longer work.
>> So what do you recommend to extend the property values. I guess we had
>> some existing scenarios happening in the past. What is the best way to
>> proceed?
>
> Is it a requirement to ensure boolean values are still supported when
> switching to OnOffAuto? I found that the x86-iommu intremap property
> had a change from DEFINE_PROP_BOOL to DEFINE_PROP_ON_OFF_AUTO [0].
>
> Should I add a custom property setter that accepts both boolean and
> on/off/auto for backward compatibility, or is following the intremap
> precedent acceptable? I'm happy to implement either approach.
>
> [0]
> https://github.com/qemu/qemu/commit/a924b3d8df55a395891fd5ed341d0deb135d9aa6
Maybe we also need to emphasize that libvirt integration is still under
work (and waiting for that conversion to happen at qemu level). So do we
really care about keeping the compat wrt QMP and JSON.
Thanks
Eric
>
> Thanks,
> Nathan
>
Eric Auger <eric.auger@redhat.com> writes:
> On 3/11/26 6:08 PM, Nathan Chen wrote:
>>
>>
>> On 3/11/2026 8:31 AM, Eric Auger wrote:
>>> On 3/10/26 8:05 AM, Markus Armbruster wrote:
>>>> Nathan Chen<nathanc@nvidia.com> writes:
>>>>
>>>>> From: Nathan Chen<nathanc@nvidia.com>
>>>>>
>>>>> Allow accelerated SMMUv3 Address Translation Services support property
>>>>> to be derived from host IOMMU capabilities. Derive host values using
>>>>> IOMMU_GET_HW_INFO, retrieving ATS capability from IDR0.
>>>>>
>>>>> Set the default value of ATS to auto. The default for ATS support used
>>>>> to be set to off, but we change it to match what the host IOMMU
>>>>> properties report.
>>>>>
>>>>> Add a "ats-enabled" read-only property for smmuv3 to address an
>>>>> expected bool for the "ats" property in iort_smmuv3_devices().
>>>>>
>>>>> Signed-off-by: Nathan Chen<nathanc@nvidia.com>
[...]
>>>>> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
>>>>> index 068108e49b..197ba7c77b 100644
>>>>> --- a/hw/arm/smmuv3.c
>>>>> +++ b/hw/arm/smmuv3.c
[...]
>>>>> @@ -2128,7 +2134,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_AUTO),
>>>>
>>>> Is property "ats" accessible via QMP or JSON command line? If yes,
>>>> this
>>>> is an incompatible change: JSON values false and true no longer work.
>>> So what do you recommend to extend the property values. I guess we had
>>> some existing scenarios happening in the past. What is the best way to
>>> proceed?
>>
>> Is it a requirement to ensure boolean values are still supported when
>> switching to OnOffAuto? I found that the x86-iommu intremap property
>> had a change from DEFINE_PROP_BOOL to DEFINE_PROP_ON_OFF_AUTO [0].
We promise to follow a certain process when changing external
interfaces: docs/about/deprecated.rst.
This is why I inquired about external access. No external access, no
compatibility worries.
I specifically asked about JSON, because going from bool to OnOffAuto is
mostly compatible in key=value syntax: values "on" and "off" keep
working.
Mostly compatible, because qapi_bool_parse() also recognizes "yes",
"true", "y", "no", "false", and "n" for convenience, and these stop
working. We have a knack for inventing convenience features that later
bite us.
JSON breaks entirely, of course: all values stop working.
Even with external access, compatibility worries start only after we
shipped the interface in a release. Did we?
If the interface was released, we should keep our promise to follow the
process. From time to time, that's so onerous and we're so certain that
nobody actually cares, we bend the rules. This should not be done
lightly.
>> Should I add a custom property setter that accepts both boolean and
>> on/off/auto for backward compatibility, or is following the intremap
>> precedent acceptable? I'm happy to implement either approach.
>>
>> [0]
>> https://github.com/qemu/qemu/commit/a924b3d8df55a395891fd5ed341d0deb135d9aa6
>
> Maybe we also need to emphasize that libvirt integration is still under
> work (and waiting for that conversion to happen at qemu level). So do we
> really care about keeping the compat wrt QMP and JSON.
I can explain our promise and the process to you, but I can't make the
decision to bend the rules for you.
On 3/12/26 9:39 AM, Markus Armbruster wrote:
> Eric Auger <eric.auger@redhat.com> writes:
>
>> On 3/11/26 6:08 PM, Nathan Chen wrote:
>>>
>>> On 3/11/2026 8:31 AM, Eric Auger wrote:
>>>> On 3/10/26 8:05 AM, Markus Armbruster wrote:
>>>>> Nathan Chen<nathanc@nvidia.com> writes:
>>>>>
>>>>>> From: Nathan Chen<nathanc@nvidia.com>
>>>>>>
>>>>>> Allow accelerated SMMUv3 Address Translation Services support property
>>>>>> to be derived from host IOMMU capabilities. Derive host values using
>>>>>> IOMMU_GET_HW_INFO, retrieving ATS capability from IDR0.
>>>>>>
>>>>>> Set the default value of ATS to auto. The default for ATS support used
>>>>>> to be set to off, but we change it to match what the host IOMMU
>>>>>> properties report.
>>>>>>
>>>>>> Add a "ats-enabled" read-only property for smmuv3 to address an
>>>>>> expected bool for the "ats" property in iort_smmuv3_devices().
>>>>>>
>>>>>> Signed-off-by: Nathan Chen<nathanc@nvidia.com>
> [...]
>
>>>>>> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
>>>>>> index 068108e49b..197ba7c77b 100644
>>>>>> --- a/hw/arm/smmuv3.c
>>>>>> +++ b/hw/arm/smmuv3.c
> [...]
>
>>>>>> @@ -2128,7 +2134,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_AUTO),
>>>>> Is property "ats" accessible via QMP or JSON command line? If yes,
>>>>> this
>>>>> is an incompatible change: JSON values false and true no longer work.
>>>> So what do you recommend to extend the property values. I guess we had
>>>> some existing scenarios happening in the past. What is the best way to
>>>> proceed?
>>> Is it a requirement to ensure boolean values are still supported when
>>> switching to OnOffAuto? I found that the x86-iommu intremap property
>>> had a change from DEFINE_PROP_BOOL to DEFINE_PROP_ON_OFF_AUTO [0].
> We promise to follow a certain process when changing external
> interfaces: docs/about/deprecated.rst.
>
> This is why I inquired about external access. No external access, no
> compatibility worries.
>
> I specifically asked about JSON, because going from bool to OnOffAuto is
> mostly compatible in key=value syntax: values "on" and "off" keep
> working.
>
> Mostly compatible, because qapi_bool_parse() also recognizes "yes",
> "true", "y", "no", "false", and "n" for convenience, and these stop
> working. We have a knack for inventing convenience features that later
> bite us.
>
> JSON breaks entirely, of course: all values stop working.
>
> Even with external access, compatibility worries start only after we
> shipped the interface in a release. Did we?
those options were introduced in qemu 11.0. Fixing them in -rc is still
an option but it is a very tight schedule
Thanks
Eric
>
> If the interface was released, we should keep our promise to follow the
> process. From time to time, that's so onerous and we're so certain that
> nobody actually cares, we bend the rules. This should not be done
> lightly.
>
>>> Should I add a custom property setter that accepts both boolean and
>>> on/off/auto for backward compatibility, or is following the intremap
>>> precedent acceptable? I'm happy to implement either approach.
>>>
>>> [0]
>>> https://github.com/qemu/qemu/commit/a924b3d8df55a395891fd5ed341d0deb135d9aa6
>> Maybe we also need to emphasize that libvirt integration is still under
>> work (and waiting for that conversion to happen at qemu level). So do we
>> really care about keeping the compat wrt QMP and JSON.
> I can explain our promise and the process to you, but I can't make the
> decision to bend the rules for you.
>
Eric Auger <eric.auger@redhat.com> writes: > On 3/12/26 9:39 AM, Markus Armbruster wrote: [...] >> We promise to follow a certain process when changing external >> interfaces: docs/about/deprecated.rst. >> >> This is why I inquired about external access. No external access, no >> compatibility worries. >> >> I specifically asked about JSON, because going from bool to OnOffAuto is >> mostly compatible in key=value syntax: values "on" and "off" keep >> working. >> >> Mostly compatible, because qapi_bool_parse() also recognizes "yes", >> "true", "y", "no", "false", and "n" for convenience, and these stop >> working. We have a knack for inventing convenience features that later >> bite us. >> >> JSON breaks entirely, of course: all values stop working. >> >> Even with external access, compatibility worries start only after we >> shipped the interface in a release. Did we? > those options were introduced in qemu 11.0. Fixing them in -rc is still > an option but it is a very tight schedule If it turns out to be too tight, rever them or declare them experimental? Then you can revise them without process hassle in the next release. [...]
Nathan, On 3/12/26 10:20 AM, Markus Armbruster wrote: > Eric Auger <eric.auger@redhat.com> writes: > >> On 3/12/26 9:39 AM, Markus Armbruster wrote: > [...] > >>> We promise to follow a certain process when changing external >>> interfaces: docs/about/deprecated.rst. >>> >>> This is why I inquired about external access. No external access, no >>> compatibility worries. >>> >>> I specifically asked about JSON, because going from bool to OnOffAuto is >>> mostly compatible in key=value syntax: values "on" and "off" keep >>> working. >>> >>> Mostly compatible, because qapi_bool_parse() also recognizes "yes", >>> "true", "y", "no", "false", and "n" for convenience, and these stop >>> working. We have a knack for inventing convenience features that later >>> bite us. >>> >>> JSON breaks entirely, of course: all values stop working. >>> >>> Even with external access, compatibility worries start only after we >>> shipped the interface in a release. Did we? >> those options were introduced in qemu 11.0. Fixing them in -rc is still >> an option but it is a very tight schedule > If it turns out to be too tight, rever them or declare them > experimental? Then you can revise them without process hassle in the > next release. Maybe Nathan, in a first step, you could just respin and convert the options to the auto form, keeping the current default value and not implementing the auto mode yet. + the ATS check fix? Shameer, Nathan, would that be sensible within the qemu 11.0 -rc timeframe. Eric > > [...] >
On 3/12/2026 2:25 AM, Eric Auger wrote: > On 3/12/26 10:20 AM, Markus Armbruster wrote: >> Eric Auger<eric.auger@redhat.com> writes: >> >>> On 3/12/26 9:39 AM, Markus Armbruster wrote: >> [...] >> >>>> We promise to follow a certain process when changing external >>>> interfaces: docs/about/deprecated.rst. >>>> >>>> This is why I inquired about external access. No external access, no >>>> compatibility worries. >>>> >>>> I specifically asked about JSON, because going from bool to OnOffAuto is >>>> mostly compatible in key=value syntax: values "on" and "off" keep >>>> working. >>>> >>>> Mostly compatible, because qapi_bool_parse() also recognizes "yes", >>>> "true", "y", "no", "false", and "n" for convenience, and these stop >>>> working. We have a knack for inventing convenience features that later >>>> bite us. >>>> >>>> JSON breaks entirely, of course: all values stop working. >>>> >>>> Even with external access, compatibility worries start only after we >>>> shipped the interface in a release. Did we? >>> those options were introduced in qemu 11.0. Fixing them in -rc is still >>> an option but it is a very tight schedule >> If it turns out to be too tight, rever them or declare them >> experimental? Then you can revise them without process hassle in the >> next release. > Maybe Nathan, in a first step, you could just respin and convert the > options to the auto form, keeping the current default value and not > implementing the auto mode yet. > + the ATS check fix? Shameer, Nathan, would that be sensible within the > qemu 11.0 -rc timeframe. Yes, we will respin with just the auto form conversion and include the ATS check fix. Thanks, Nathan
On 3/12/26 9:39 AM, Markus Armbruster wrote:
> Eric Auger <eric.auger@redhat.com> writes:
>
>> On 3/11/26 6:08 PM, Nathan Chen wrote:
>>>
>>> On 3/11/2026 8:31 AM, Eric Auger wrote:
>>>> On 3/10/26 8:05 AM, Markus Armbruster wrote:
>>>>> Nathan Chen<nathanc@nvidia.com> writes:
>>>>>
>>>>>> From: Nathan Chen<nathanc@nvidia.com>
>>>>>>
>>>>>> Allow accelerated SMMUv3 Address Translation Services support property
>>>>>> to be derived from host IOMMU capabilities. Derive host values using
>>>>>> IOMMU_GET_HW_INFO, retrieving ATS capability from IDR0.
>>>>>>
>>>>>> Set the default value of ATS to auto. The default for ATS support used
>>>>>> to be set to off, but we change it to match what the host IOMMU
>>>>>> properties report.
>>>>>>
>>>>>> Add a "ats-enabled" read-only property for smmuv3 to address an
>>>>>> expected bool for the "ats" property in iort_smmuv3_devices().
>>>>>>
>>>>>> Signed-off-by: Nathan Chen<nathanc@nvidia.com>
> [...]
>
>>>>>> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
>>>>>> index 068108e49b..197ba7c77b 100644
>>>>>> --- a/hw/arm/smmuv3.c
>>>>>> +++ b/hw/arm/smmuv3.c
> [...]
>
>>>>>> @@ -2128,7 +2134,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_AUTO),
>>>>> Is property "ats" accessible via QMP or JSON command line? If yes,
>>>>> this
>>>>> is an incompatible change: JSON values false and true no longer work.
>>>> So what do you recommend to extend the property values. I guess we had
>>>> some existing scenarios happening in the past. What is the best way to
>>>> proceed?
>>> Is it a requirement to ensure boolean values are still supported when
>>> switching to OnOffAuto? I found that the x86-iommu intremap property
>>> had a change from DEFINE_PROP_BOOL to DEFINE_PROP_ON_OFF_AUTO [0].
> We promise to follow a certain process when changing external
> interfaces: docs/about/deprecated.rst.
>
> This is why I inquired about external access. No external access, no
> compatibility worries.
>
> I specifically asked about JSON, because going from bool to OnOffAuto is
> mostly compatible in key=value syntax: values "on" and "off" keep
> working.
>
> Mostly compatible, because qapi_bool_parse() also recognizes "yes",
> "true", "y", "no", "false", and "n" for convenience, and these stop
> working. We have a knack for inventing convenience features that later
> bite us.
>
> JSON breaks entirely, of course: all values stop working.
>
> Even with external access, compatibility worries start only after we
> shipped the interface in a release. Did we?
those options were introduced in qemu 11.0. Fixing them in -rc is still
an option but it is a very tight schedule
Thanks
Eric
>
> If the interface was released, we should keep our promise to follow the
> process. From time to time, that's so onerous and we're so certain that
> nobody actually cares, we bend the rules. This should not be done
> lightly.
>
>>> Should I add a custom property setter that accepts both boolean and
>>> on/off/auto for backward compatibility, or is following the intremap
>>> precedent acceptable? I'm happy to implement either approach.
>>>
>>> [0]
>>> https://github.com/qemu/qemu/commit/a924b3d8df55a395891fd5ed341d0deb135d9aa6
>> Maybe we also need to emphasize that libvirt integration is still under
>> work (and waiting for that conversion to happen at qemu level). So do we
>> really care about keeping the compat wrt QMP and JSON.
> I can explain our promise and the process to you, but I can't make the
> decision to bend the rules for you.
>
On Wed, Mar 11, 2026 at 06:16:09PM +0100, Eric Auger wrote:
>
>
> On 3/11/26 6:08 PM, Nathan Chen wrote:
> >
> >
> > On 3/11/2026 8:31 AM, Eric Auger wrote:
> >> On 3/10/26 8:05 AM, Markus Armbruster wrote:
> >>> Nathan Chen<nathanc@nvidia.com> writes:
> >>>
> >>>> From: Nathan Chen<nathanc@nvidia.com>
> >>>>
> >>>> Allow accelerated SMMUv3 Address Translation Services support property
> >>>> to be derived from host IOMMU capabilities. Derive host values using
> >>>> IOMMU_GET_HW_INFO, retrieving ATS capability from IDR0.
> >>>>
> >>>> Set the default value of ATS to auto. The default for ATS support used
> >>>> to be set to off, but we change it to match what the host IOMMU
> >>>> properties report.
> >>>>
> >>>> Add a "ats-enabled" read-only property for smmuv3 to address an
> >>>> expected bool for the "ats" property in iort_smmuv3_devices().
> >>>>
> >>>> Signed-off-by: Nathan Chen<nathanc@nvidia.com>
> >>>> ---
> >>>> hw/arm/smmuv3-accel.c | 25 +++++++++++++++++++++++--
> >>>> hw/arm/smmuv3.c | 12 ++++++++++--
> >>>> hw/arm/virt-acpi-build.c | 2 +-
> >>>> include/hw/arm/smmuv3.h | 2 +-
> >>>> 4 files changed, 35 insertions(+), 6 deletions(-)
> >>>>
> >>>> diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
> >>>> index 617629bacd..8fec335557 100644
> >>>> --- a/hw/arm/smmuv3-accel.c
> >>>> +++ b/hw/arm/smmuv3-accel.c
> >>>> @@ -52,6 +52,12 @@ static void
> >>>> smmuv3_accel_auto_finalise(SMMUv3State *s, PCIDevice *pdev,
> >>>> return;
> >>>> }
> >>>> + /* Update ATS if auto from info */
> >>>> + if (s->ats == ON_OFF_AUTO_AUTO) {
> >>>> + s->idr[0] = FIELD_DP32(s->idr[0], IDR0, ATS,
> >>>> + FIELD_EX32(info->idr[0], IDR0, ATS));
> >>>> + }
> >>>> +
> >>>> accel->auto_finalised = true;
> >>>> }
> >>>> @@ -124,6 +130,13 @@ smmuv3_accel_check_hw_compatible(SMMUv3State
> >>>> *s,
> >>>> smmuv3_oas_bits(FIELD_EX32(s->idr[5], IDR5,
> >>>> OAS)));
> >>>> return false;
> >>>> }
> >>>> + /* Check ATS value opted is compatible with Host SMMUv3 */
> >>>> + if (FIELD_EX32(info->idr[0], IDR0, ATS) <
> >>>> + FIELD_EX32(s->idr[0], IDR0, ATS)) {
> >>>> + error_setg(errp, "Host SMMUv3 doesn't support Address
> >>>> Translation"
> >>>> + " Services");
> >>>> + return false;
> >>>> + }
> >>>> /* QEMU SMMUv3 supports GRAN4K/GRAN16K/GRAN64K translation
> >>>> granules */
> >>>> if (FIELD_EX32(info->idr[5], IDR5, GRAN4K) !=
> >>>> @@ -844,8 +857,12 @@ 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 or OFF */
> >>>> + if (s->ats == ON_OFF_AUTO_ON) {
> >>>> + s->idr[0] = FIELD_DP32(s->idr[0], IDR0, ATS, 1);
> >>>> + } else if (s->ats == ON_OFF_AUTO_OFF) {
> >>>> + s->idr[0] = FIELD_DP32(s->idr[0], IDR0, ATS, 0);
> >>>> + }
> >>>> /* Advertise 48-bit OAS in IDR5 when requested (default is
> >>>> 44 bits). */
> >>>> if (s->oas == SMMU_OAS_48BIT) {
> >>>> @@ -923,4 +940,8 @@ void smmuv3_accel_init(SMMUv3State *s)
> >>>> s->s_accel = g_new0(SMMUv3AccelState, 1);
> >>>> bs->iommu_ops = &smmuv3_accel_ops;
> >>>> smmuv3_accel_as_init(s);
> >>>> +
> >>>> + if (s->ats == ON_OFF_AUTO_AUTO) {
> >>>> + s->s_accel->auto_mode = true;
> >>>> + }
> >>>> }
> >>>> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> >>>> index 068108e49b..197ba7c77b 100644
> >>>> --- a/hw/arm/smmuv3.c
> >>>> +++ b/hw/arm/smmuv3.c
> >>>> @@ -317,6 +317,12 @@ static void smmuv3_init_id_regs(SMMUv3State *s)
> >>>> smmuv3_accel_idr_override(s);
> >>>> }
> >>>> +static bool get_ats_enabled(Object *obj, Error **errp)
> >>>> +{
> >>>> + SMMUv3State *s = ARM_SMMUV3(obj);
> >>>> + 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 +1977,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;
> >>>> }
> >>>> @@ -2128,7 +2134,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_AUTO),
> >>> Is property "ats" accessible via QMP or JSON command line? If yes,
> >>> this
> >>> is an incompatible change: JSON values false and true no longer work.
> >> So what do you recommend to extend the property values. I guess we had
> >> some existing scenarios happening in the past. What is the best way to
> >> proceed?
> >
> > Is it a requirement to ensure boolean values are still supported when
> > switching to OnOffAuto? I found that the x86-iommu intremap property
> > had a change from DEFINE_PROP_BOOL to DEFINE_PROP_ON_OFF_AUTO [0].
> >
> > Should I add a custom property setter that accepts both boolean and
> > on/off/auto for backward compatibility, or is following the intremap
> > precedent acceptable? I'm happy to implement either approach.
> >
> > [0]
> > https://github.com/qemu/qemu/commit/a924b3d8df55a395891fd5ed341d0deb135d9aa6
>
> Maybe we also need to emphasize that libvirt integration is still under
> work (and waiting for that conversion to happen at qemu level). So do we
> really care about keeping the compat wrt QMP and JSON.
> Thanks
I think it should be safe and to change the type of the property, it's
not part of any QEMU release and not even implemented in libvirt.
Pavel
>
> Eric
> >
> > Thanks,
> > Nathan
> >
>
>
On 3/10/2026 12:05 AM, Markus Armbruster wrote:
> Nathan Chen<nathanc@nvidia.com> writes:
>
>> From: Nathan Chen<nathanc@nvidia.com>
>>
>> Allow accelerated SMMUv3 Address Translation Services support property
>> to be derived from host IOMMU capabilities. Derive host values using
>> IOMMU_GET_HW_INFO, retrieving ATS capability from IDR0.
>>
>> Set the default value of ATS to auto. The default for ATS support used
>> to be set to off, but we change it to match what the host IOMMU
>> properties report.
>>
>> Add a "ats-enabled" read-only property for smmuv3 to address an
>> expected bool for the "ats" property in iort_smmuv3_devices().
>>
>> Signed-off-by: Nathan Chen<nathanc@nvidia.com>
>> ---
>> hw/arm/smmuv3-accel.c | 25 +++++++++++++++++++++++--
>> hw/arm/smmuv3.c | 12 ++++++++++--
>> hw/arm/virt-acpi-build.c | 2 +-
>> include/hw/arm/smmuv3.h | 2 +-
>> 4 files changed, 35 insertions(+), 6 deletions(-)
>>
>> diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
>> index 617629bacd..8fec335557 100644
>> --- a/hw/arm/smmuv3-accel.c
>> +++ b/hw/arm/smmuv3-accel.c
>> @@ -52,6 +52,12 @@ static void smmuv3_accel_auto_finalise(SMMUv3State *s, PCIDevice *pdev,
>> return;
>> }
>>
>> + /* Update ATS if auto from info */
>> + if (s->ats == ON_OFF_AUTO_AUTO) {
>> + s->idr[0] = FIELD_DP32(s->idr[0], IDR0, ATS,
>> + FIELD_EX32(info->idr[0], IDR0, ATS));
>> + }
>> +
>> accel->auto_finalised = true;
>> }
>>
>> @@ -124,6 +130,13 @@ smmuv3_accel_check_hw_compatible(SMMUv3State *s,
>> smmuv3_oas_bits(FIELD_EX32(s->idr[5], IDR5, OAS)));
>> return false;
>> }
>> + /* Check ATS value opted is compatible with Host SMMUv3 */
>> + if (FIELD_EX32(info->idr[0], IDR0, ATS) <
>> + FIELD_EX32(s->idr[0], IDR0, ATS)) {
>> + error_setg(errp, "Host SMMUv3 doesn't support Address Translation"
>> + " Services");
>> + return false;
>> + }
>>
>> /* QEMU SMMUv3 supports GRAN4K/GRAN16K/GRAN64K translation granules */
>> if (FIELD_EX32(info->idr[5], IDR5, GRAN4K) !=
>> @@ -844,8 +857,12 @@ 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 or OFF */
>> + if (s->ats == ON_OFF_AUTO_ON) {
>> + s->idr[0] = FIELD_DP32(s->idr[0], IDR0, ATS, 1);
>> + } else if (s->ats == ON_OFF_AUTO_OFF) {
>> + s->idr[0] = FIELD_DP32(s->idr[0], IDR0, ATS, 0);
>> + }
>>
>> /* Advertise 48-bit OAS in IDR5 when requested (default is 44 bits). */
>> if (s->oas == SMMU_OAS_48BIT) {
>> @@ -923,4 +940,8 @@ void smmuv3_accel_init(SMMUv3State *s)
>> s->s_accel = g_new0(SMMUv3AccelState, 1);
>> bs->iommu_ops = &smmuv3_accel_ops;
>> smmuv3_accel_as_init(s);
>> +
>> + if (s->ats == ON_OFF_AUTO_AUTO) {
>> + s->s_accel->auto_mode = true;
>> + }
>> }
>> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
>> index 068108e49b..197ba7c77b 100644
>> --- a/hw/arm/smmuv3.c
>> +++ b/hw/arm/smmuv3.c
>> @@ -317,6 +317,12 @@ static void smmuv3_init_id_regs(SMMUv3State *s)
>> smmuv3_accel_idr_override(s);
>> }
>>
>> +static bool get_ats_enabled(Object *obj, Error **errp)
>> +{
>> + SMMUv3State *s = ARM_SMMUV3(obj);
>> + 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 +1977,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;
>> }
>> @@ -2128,7 +2134,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_AUTO),
> Is property "ats" accessible via QMP or JSON command line? If yes, this
> is an incompatible change: JSON values false and true no longer work.
Yes it is accessible. I will implement both the ATS and RIL properties
differently, calling object_class_property_add() from
smmuv3_class_init() with an ats property setter function that accepts
both bool and on/off/auto strings, rather than using
DEFINE_PROP_ON_OFF_AUTO() here.
Thanks,
Nathan
© 2016 - 2026 Red Hat, Inc.