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
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
>
