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

[Nathan Chen <nathanc@nvidia.com>]

On 5/14/2026 7:24 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 4/7] hw/arm/smmuv3-accel: Implement "auto" value for
>> "ssidsize"
>>
>> From: Nathan Chen<nathanc@nvidia.com>
>>
>> Allow accelerated SMMUv3 SSID size property to be derived from host
>> IOMMU capabilities. Derive host values using IOMMU_GET_HW_INFO,
>> retrieving SSID size from IDR1. When the auto SSID size is resolved
>> to a non-zero value, PASID capability is advertised to the vIOMMU
>> and accelerated use cases such as Shared Virtual Addressing (SVA)
>> are supported.
>>
>> Reviewed-by: Eric Auger<eric.auger@redhat.com>
>> Signed-off-by: Nathan Chen<nathanc@nvidia.com>
>> ---
>>   hw/arm/smmuv3-accel.c | 18 ++++++++++++++++--
>>   hw/arm/smmuv3.c       | 20 ++++++++++----------
>>   2 files changed, 26 insertions(+), 12 deletions(-)
>>
>> diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
>> index a5973bd499..5b1af062ee 100644
>> --- a/hw/arm/smmuv3-accel.c
>> +++ b/hw/arm/smmuv3-accel.c
>> @@ -62,6 +62,12 @@ static void
>> smmuv3_accel_auto_finalise(SMMUv3State *s,
>>                                  FIELD_EX32(info->idr[3], IDR3, RIL));
>>       }
>>
>> +    if (s->ssidsize == SSID_SIZE_MODE_AUTO) {
>> +        /* Store for get_viommu_flags() to determine PASID support */
>> +        s->idr[1] = FIELD_DP32(s->idr[1], IDR1, SSIDSIZE,
>> +                               FIELD_EX32(info->idr[1], IDR1, SSIDSIZE));
> Nit: Not sure that comment makes sense. We need to update it
> anyway, right?
After ssidsize is stored in s->idr[1], smmuv3_accel_get_viommu_flags() 
will call smmuv3_pasid_supported() and enable the 
VIOMMU_FLAG_PASID_SUPPORTED flag if AUTO is specified for ssidsize and 
s->idr[1] is non-zero. Maybe we can remove the comment or come up with a 
better wording such as:

/* PASID support will be enabled if ssidsize is non-zero */

But this is already documented in qemu-options.hx. I think it would be 
cleaner to omit the comment here. What do you think?

Thanks,
Nathan

> 
>> +    }
>> +
>>       accel->auto_finalised = true;
>>   }
>>
>> @@ -822,6 +828,13 @@ static AddressSpace
>> *smmuv3_accel_find_add_as(PCIBus *bus, void *opaque,
>>       }
>>   }
>>
>> +static inline bool smmuv3_pasid_supported(SMMUv3State *s)
>> +{
>> +    return s->ssidsize > SSID_SIZE_MODE_0 ||
>> +           (s->ssidsize == SSID_SIZE_MODE_AUTO &&
>> +            FIELD_EX32(s->idr[1], IDR1, SSIDSIZE));
>> +}
>> +
>>   static uint64_t smmuv3_accel_get_viommu_flags(void *opaque)
>>   {
>>       /*
>> @@ -834,7 +847,7 @@ static uint64_t
>> smmuv3_accel_get_viommu_flags(void *opaque)
>>       SMMUState *bs = opaque;
>>       SMMUv3State *s = ARM_SMMUV3(bs);
>>
>> -    if (s->ssidsize > SSID_SIZE_MODE_0) {
>> +    if (smmuv3_pasid_supported(s)) {
>>           flags |= VIOMMU_FLAG_PASID_SUPPORTED;
>>       }
>>       return flags;
>> @@ -975,7 +988,8 @@ bool smmuv3_accel_init(SMMUv3State *s, Error
>> **errp)
>>       smmuv3_accel_as_init(s);
>>
>>       if (s->ats == ON_OFF_AUTO_AUTO ||
>> -        s->ril == ON_OFF_AUTO_AUTO) {
>> +        s->ril == ON_OFF_AUTO_AUTO ||
>> +        s->ssidsize == SSID_SIZE_MODE_AUTO) {
>>           s->s_accel->auto_mode = true;
>>       }
>>
>> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
>> index 7ec4241ecf..cd30275717 100644
>> --- a/hw/arm/smmuv3.c
>> +++ b/hw/arm/smmuv3.c
>> @@ -626,7 +626,10 @@ static int decode_ste(SMMUv3State *s,
>> SMMUTransCfg *cfg,
>>       }
>>
>>       /* Multiple context descriptors require SubstreamID support */
>> -    if (s->ssidsize == SSID_SIZE_MODE_0 && STE_S1CDMAX(ste) != 0) {
>> +    if ((s->ssidsize == SSID_SIZE_MODE_0 ||
>> +         (s->ssidsize == SSID_SIZE_MODE_AUTO &&
>> +          !FIELD_EX32(s->idr[1], IDR1, SSIDSIZE))) &&
>> +        STE_S1CDMAX(ste) != 0) {
>>           qemu_log_mask(LOG_UNIMP,
>>                   "SMMUv3: multiple S1 context descriptors require SubstreamID
>> support. "
>>                   "Configure ssidsize > 0 (requires accel=on)\n");
>> @@ -1965,10 +1968,6 @@ static void smmu_reset_exit(Object *obj,
>> ResetType type)
>>
>>   static bool smmu_validate_property(SMMUv3State *s, Error **errp)
>>   {
>> -    if (s->ssidsize == SSID_SIZE_MODE_AUTO) {
>> -        error_setg(errp, "ssidsize auto mode is not supported");
>> -        return false;
>> -    }
>>       if (s->oas != OAS_MODE_44 && s->oas != OAS_MODE_48) {
>>           error_setg(errp, "QEMU SMMUv3 model only implements 44 and 48 bit"
>>                      "OAS; other OasMode values are not supported");
>> @@ -1989,7 +1988,8 @@ static bool
>> smmu_validate_property(SMMUv3State *s, Error **errp)
>>               return false;
>>           }
>>           if (s->ssidsize > SSID_SIZE_MODE_0) {
>> -            error_setg(errp, "ssidsize can only be set if accel=on");
>> +            error_setg(errp, "ssidsize can only be greater than 0 "
>> +                       "bits if accel=on");
>>               return false;
>>           }
>>           return true;
>> @@ -2173,11 +2173,11 @@ static void smmuv3_class_init(ObjectClass
>> *klass, const void *data)
>>           "are 44 or 48 bits. Defaults to 44 bits. oas=auto is not "
>>           "supported.");
>>       object_class_property_set_description(klass, "ssidsize",
>> -        "Number of bits used to represent SubstreamIDs (SSIDs). "
>> +        "Set number of bits used to represent SubstreamIDs (SSIDs). "
>> +        "Valid values are 0-20 and auto. Defaults to 0. "
>>           "A value of N allows SSIDs in the range [0 .. 2^N - 1]. "
>> -        "Valid range is 0-20, where 0 disables SubstreamID support. "
>> -        "Defaults to 0. A value greater than 0 is required to enable "
>> -        "PASID support. ssidsize=auto is not supported.");
>> +        "A value of 0 disables SubstreamID support. A value greater "
>> +        "than 0 is required to enable PASID support.");
>>   }
>>
>>   static int smmuv3_notify_flag_changed(IOMMUMemoryRegion *iommu,