Hi Shameer,
On 6/30/26 11:43 AM, Shameer Kolothum Thodi wrote:
>
>> -----Original Message-----
>> From: Eric Auger <[email protected]>
>> Sent: 30 June 2026 09:30
>> To: [email protected]; [email protected]; qemu-
>> [email protected]; [email protected]; [email protected];
>> [email protected]; Shameer Kolothum Thodi
>> <[email protected]>; Nicolin Chen <[email protected]>; Nathan
>> Chen <[email protected]>
>> Subject: [PATCH 2/2] hw/arm/smmuv3: Sanitize
>> SMMU_S_STRTAB_BASE_CFG.LOG2SIZE
>>
>> External email: Use caution opening links or attachments
>>
>>
>> STRTAB_BASE_CFG.LOG2SIZE is programmed by the guest through the
>> emulated SMMUv3 MMIO register interface. Currently the value is
>> not checked.
>>
>> The SMMU spec says: "Except for readback of a written value, the
>> effective LOG2SIZE is <= SMMU_IDR1.SIDSIZE for the purposes of
>> input StreamID range checking and upper/lower/linear Stream table
>> index address calculation."
>>
>> So for STE lookup make sure log2size is capped at SMMU_IDR1.SIDSIZE.
>>
>> Signed-off-by: Eric Auger <[email protected]>
>> ---
>> hw/arm/smmuv3.c | 9 +++++++++
>> 1 file changed, 9 insertions(+)
>>
>> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
>> index 0a8d2fbb6a7..db5aca26ec0 100644
>> --- a/hw/arm/smmuv3.c
>> +++ b/hw/arm/smmuv3.c
>> @@ -672,6 +672,15 @@ int smmu_find_ste(SMMUv3State *s, uint32_t sid,
>> STE *ste, SMMUEventInfo *event)
>> /*
>> * Check SID range against both guest-configured and implementation
>> limits
>> */
>> + if (log2size > SMMU_IDR1_SIDSIZE) {
>> + /*
>> + * spec says: Except for readback of a written value, the effective
>> + * LOG2SIZE is <= SMMU_IDR1.SIDSIZE for the purposes of input
>> StreamID
>> + * range checking and upper/lower/linear Stream table index address
>> + * calculation.
>> + */
>> + log2size = SMMU_IDR1_SIDSIZE;
> We are now limiting the log2size here and later in the code we use
> this for strtab_size_shift calculation.
>
> However, spec says(6.3.24):
>
> "The alignment of ADDR is affected by the literal value of the respective
> SMMU_STRTAB_BASE_CFG.LOG2SIZE field and is not limited by SIDSIZE."
>
> So this will change the behaviour now. Please check.
You right, there is this statement + the existing comment in the code +
the additional note: "Note: This means that configuring a table that is
larger than required by the incoming StreamID span results
in some entries being unreachable, but the table is still aligned to the
configured size."
and then there is
"../.. the effective LOG2SIZE is <= SMMU_IDR1.SIDSIZE for the purposes of input
StreamID range checking and upper/lower/linear Stream table index address
calculation.
"
This sounds contradictory to me.
On the other hand who is going to program an LOG2SIZE greater than the SIDSIZE?
There is no further info about log2size limit and with some of its values spec
ADDR alignment computations cannot apply
ADDR[LOG2SIZE + 5:0] = 0.
ADDR[MAX(5, (LOG2SIZE - SPLIT - 1 + 3)):0] = 0.
Adding to that, the code currently does not consider SPLIT >= LOG2SIZE case and
also the ignores aligment computation of L2PTR according to span.
I will do my best to rework that ...
Thanks
Eric
>
>> + }
>> if (sid >= (1 << MIN(log2size, SMMU_IDR1_SIDSIZE))) {
> This MIN check now is redundant here, right?
Yes it is
>
> Thanks,
> Shameer
>
