Re: [PATCH v4 7/8] hw/arm/smmuv3-accel: Change OAS property to OasMode

[Nathan Chen <nathanc@nvidia.com>]

[-- Attachment #1: Type: text/plain, Size: 2423 bytes --]


On 3/19/2026 5:20 AM, Markus Armbruster wrote:
>> From: Nathan Chen<nathanc@nvidia.com>
>>
>> Change accel SMMUv3 OAS property from uint8_t to OasMode. The
> Incompatible change; okay because property "oas" is new in this
> release.  Spelling out such things out in the commit message helps
> reviewers.  Not worth a respin by itself.
Ok, I will keep this in mind and mention this is an incompatible change 
for the next refresh.
>> 'auto' value is not implemented, as this commit is meant to
>> set the property to the correct type and avoid breaking JSON/QMP
>> when the auto mode is introduced. A future patch will implement
>> resolution of 'auto' value to match the host SMMUv3 OAS value.
>>
>> Fixes: a015ac990fd3 ("hw/arm/smmuv3-accel: Add property to specify OAS bits")
>> Tested-by: Eric Auger<eric.auger@redhat.com>
>> Signed-off-by: Nathan Chen<nathanc@nvidia.com>
>> ---
>>   hw/arm/smmuv3-accel.c          |  2 +-
>>   hw/arm/smmuv3.c                | 16 ++++++++--------
>>   include/hw/arm/smmuv3-common.h |  2 --
>>   include/hw/arm/smmuv3.h        |  2 +-
>>   4 files changed, 10 insertions(+), 12 deletions(-)
>>
>> diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
>> index bc6cbfebc2..65c2f44880 100644
>> --- a/hw/arm/smmuv3-accel.c
>> +++ b/hw/arm/smmuv3-accel.c
>> @@ -850,7 +850,7 @@ void smmuv3_accel_idr_override(SMMUv3State *s)
>>       }
>>   
>>       /* Advertise 48-bit OAS in IDR5 when requested (default is 44 bits). */
>> -    if (s->oas == SMMU_OAS_48BIT) {
>> +    if (s->oas == OAS_MODE_48) {
>>           s->idr[5] = FIELD_DP32(s->idr[5], IDR5, OAS, SMMU_IDR5_OAS_48);
>>       }
>>   
>> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
>> index 79018f8d66..c67819d6f2 100644
>> --- a/hw/arm/smmuv3.c
>> +++ b/hw/arm/smmuv3.c
>> @@ -1984,6 +1984,10 @@ static bool smmu_validate_property(SMMUv3State *s, Error **errp)
>>           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, "OAS can only be set to 44 or 48 bits");
>> +        return false;
>> +    }
> So, OasMode values other than 44 and 48 are currently useless.  Correct?

Yes, in an earlier version we had only implemented auto, 44, and 48 for 
OasMode, but we included the other OasMode values according to the 
SMMUv3 spec after receiving feedback to do so.


Thanks,

Nathan

[-- Attachment #2: Type: text/html, Size: 3647 bytes --]