linux-kernel - Re: [PATCH] iommu/arm-smmu-v3: Fix L1 stream table index calculation for AmpereOne

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <3e772c30-d7b0-409c-83f1-5fc5a961567e@os.amperecomputing.com>
Date: Wed, 2 Oct 2024 10:39:36 -0700
From: Yang Shi <yang@...amperecomputing.com>
To: James Morse <james.morse@....com>, jgg@...pe.ca, nicolinc@...dia.com,
 will@...nel.org, robin.murphy@....com
Cc: linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] iommu/arm-smmu-v3: Fix L1 stream table index calculation
 for AmpereOne



On 10/2/24 9:58 AM, James Morse wrote:
> Hello!
>
> On 01/10/2024 19:03, Yang Shi wrote:
>> The commit ce410410f1a7 ("iommu/arm-smmu-v3: Add arm_smmu_strtab_l1/2_idx()")
>> calculated the last index of L1 stream table by 1 << smmu->sid_bits. 1
>> is 32 bit value.
>> However AmpereOne has 32-bit stream id size.  This resulted in
>> ouf-of-bound shift.  The disassembly of shift is:
>>
>>      ldr     w2, [x19, 828]  //, smmu_7(D)->sid_bits
>>      mov     w20, 1
>>      lsl     w20, w20, w2
>>
>> According to ARM spec, if the registers are 32 bit, the instruction actually
>> does:
>>      dest = src << (shift % 32)
>>
>> So it actually shifted by zero bit.
>>
>> This caused v6.12-rc1 failed to boot on AmpereOne
> Same here - one of arm's reference designs prints 1 giga-tonne of:
> | arm-smmu-v3 arm-smmu-v3.5.auto: event 0x02 received:
> | arm-smmu-v3 arm-smmu-v3.5.auto:  0x0000c90000000002
> | arm-smmu-v3 arm-smmu-v3.5.auto:  0x0000000000000000
> | arm-smmu-v3 arm-smmu-v3.5.auto:  0x0000000000000000
> | arm-smmu-v3 arm-smmu-v3.5.auto:  0x0000000000000000
>
> during boot - then fails to find the nvme.
> Bisect points to ce410410f1a7, and the below diff fixes it.
>
>> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
>> index 737c5b882355..01a2faee04bc 100644
>> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
>> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
>> @@ -3625,7 +3625,7 @@ static int arm_smmu_init_strtab_2lvl(struct arm_smmu_device *smmu)
>>   	u32 l1size;
>>   	struct arm_smmu_strtab_cfg *cfg = &smmu->strtab_cfg;
>>   	unsigned int last_sid_idx =
>> -		arm_smmu_strtab_l1_idx((1 << smmu->sid_bits) - 1);
>> +		arm_smmu_strtab_l1_idx((1UL << smmu->sid_bits) - 1);
>>   
>>   	/* Calculate the L1 size, capped to the SIDSIZE. */
>>   	cfg->l2.num_l1_ents = min(last_sid_idx + 1, STRTAB_MAX_L1_ENTRIES);
>
> Tested-by: James Morse <james.morse@....com>

Thank you for testing. I will change the patch subject a little bit to 
make it more general.

>
>
> Thanks,
>
> James