Re: [PATCH v3 2/7] hw/arm/smmuv3-accel: Implement "auto" value for "ats"

[Nathan Chen <nathanc@nvidia.com>]

On 5/14/2026 7:11 AM, Shameer Kolothum Thodi wrote:
> 
>> -----Original Message-----
>> From: Nathan Chen<nathanc@nvidia.com>
>> Sent: 12 May 2026 20:35
>> To:qemu-arm@nongnu.org;qemu-devel@nongnu.org
>> Cc: Eric Auger<eric.auger@redhat.com>; Peter Maydell
>> <peter.maydell@linaro.org>; Philippe Mathieu-Daudé<philmd@linaro.org>;
>> Zhao Liu<zhao1.liu@intel.com>; Matt Ochs<mochs@nvidia.com>; Nicolin
>> Chen<nicolinc@nvidia.com>; Shameer Kolothum Thodi
>> <skolothumtho@nvidia.com>; Nathan Chen<nathanc@nvidia.com>
>> Subject: [PATCH v3 2/7] hw/arm/smmuv3-accel: Implement "auto" value for
>> "ats"
>>
>> 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.
>>
>> Reviewed-by: Eric Auger<eric.auger@redhat.com>
>> Signed-off-by: Nathan Chen<nathanc@nvidia.com>
>> ---
>>   hw/arm/smmuv3-accel.c |  9 +++++++++
>>   hw/arm/smmuv3.c       | 11 ++++-------
>>   2 files changed, 13 insertions(+), 7 deletions(-)
>>
>> diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
>> index 82c0800f4d..9fbd13c438 100644
>> --- a/hw/arm/smmuv3-accel.c
>> +++ b/hw/arm/smmuv3-accel.c
>> @@ -52,6 +52,11 @@ static void
>> smmuv3_accel_auto_finalise(SMMUv3State *s,
>>           return;
>>       }
>>
>> +    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;
>>   }
>>
>> @@ -964,6 +969,10 @@ bool smmuv3_accel_init(SMMUv3State *s, Error
>> **errp)
>>       bs->iommu_ops = &smmuv3_accel_ops;
>>       smmuv3_accel_as_init(s);
>>
>> +    if (s->ats == ON_OFF_AUTO_AUTO) {
>> +        s->s_accel->auto_mode = true;
>> +    }
>> +
>>       if (s->s_accel->auto_mode) {
>>           s->machine_done.notify = smmuv3_machine_done;
>>           qemu_add_machine_init_done_notifier(&s->machine_done);
>> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
>> index 5c2855c377..e558f09652 100644
>> --- a/hw/arm/smmuv3.c
>> +++ b/hw/arm/smmuv3.c
>> @@ -1965,10 +1965,6 @@ static void smmu_reset_exit(Object *obj,
>> ResetType type)
>>
>>   static bool smmu_validate_property(SMMUv3State *s, Error **errp)
>>   {
>> -    if (s->ats == ON_OFF_AUTO_AUTO) {
>> -        error_setg(errp, "ats auto mode is not supported");
>> -        return false;
>> -    }
>>       if (s->ril == ON_OFF_AUTO_AUTO) {
>>           error_setg(errp, "ril auto mode is not supported");
>>           return false;
>> @@ -2170,9 +2166,10 @@ static void smmuv3_class_init(ObjectClass *klass,
>> const void *data)
>>           "Disable range invalidation support (for accel=on). ril=auto "
>>           "is not supported.");
>>       object_class_property_set_description(klass, "ats",
>> -        "Enable/disable ATS support (for accel=on). Please ensure host "
>> -        "platform has ATS support before enabling this. ats=auto is not "
>> -        "supported.");
>> +        "Enable/disable ATS support (for accel=on). "
>> +        "Valid values are on, off, and auto. Defaults to off. "
>> +        "Please ensure host platform supports ATS before setting it "
>> +        "to on.");
>>       object_class_property_set_description(klass, "oas",
>>           "Specify Output Address Size (for accel=on). Supported values "
>>           "are 44 or 48 bits. Defaults to 44 bits. oas=auto is not "
> In smmuv3.c we have;
> 
> case SMMU_CMD_ATC_INV:
>          {
>              SMMUDevice *sdev = smmu_find_sdev(bs, CMD_SID(&cmd));
> 
>              if (!sdev || !s->ats) {
>                  trace_smmuv3_unhandled_cmd(type);
>                  break;
>              }
> 
>              if (!smmuv3_accel_issue_inv_cmd(s, &cmd, sdev, errp)) {
>                  cmd_error = SMMU_CERROR_ILL;
>                  break;
>              }
>              break;
>          }
> 
> The above !s->ats will be true for AUTO case, right? I think
> we should change the above check with smmuv3_ats_enabled().
> 
> Also, since we modified the ats to OnOffAuto in the previous
> series, I am not sure the above should be a fix patch or not.
I see, I will use smmuv3_ats_enabled() here to account for the AUTO 
case. I am not sure if it should have the Fixes tag either but let's see 
if we can get any guidance on that point.

Thanks,
Nathan