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<[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
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 > > > >
signature.asc
Description: PGP signature
