Hi Nathan,
On 5/14/26 10:14 PM, Nathan Chen wrote:
>
>
> On 5/14/2026 7:24 AM, Shameer Kolothum Thodi wrote:
>>
>>> -----Original Message-----
>>> From: Nathan Chen<[email protected]>
>>> Sent: 12 May 2026 20:35
>>> To:[email protected];[email protected]
>>> Cc: Eric Auger<[email protected]>; Peter Maydell
>>> <[email protected]>; Philippe Mathieu-Daudé<[email protected]>;
>>> Zhao Liu<[email protected]>; Matt Ochs<[email protected]>; Nicolin
>>> Chen<[email protected]>; Shameer Kolothum Thodi
>>> <[email protected]>; Nathan Chen<[email protected]>
>>> Subject: [PATCH v3 4/7] hw/arm/smmuv3-accel: Implement "auto" value for
>>> "ssidsize"
>>>
>>> From: Nathan Chen<[email protected]>
>>>
>>> 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<[email protected]>
>>> Signed-off-by: Nathan Chen<[email protected]>
>>> ---
>>> 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?
Yes I think you can remove it
Thanks
Eric
>
> 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,
>