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<[email protected]> writes:
>>>
>>>> From: Nathan Chen<[email protected]>
>>>>
>>>> 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<[email protected]>
>>>> ---
>>>> 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
>
