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

From: Yang Shi
Date: Tue Oct 01 2024 - 15:09:24 EST




On 10/1/24 11:27 AM, Nicolin Chen wrote:
On Tue, Oct 01, 2024 at 11:03:46AM -0700, Yang Shi wrote:
Using 64 bit immediate when doing shift can solve the problem. The
disssembly after the fix looks like:
[...]

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);
Could a 32-bit build be a corner case where UL is no longer a
"64 bit" stated in the commit message?

It shouldn't. Because smmu v3 depends on ARM64.

config ARM_SMMU_V3
        tristate "ARM Ltd. System MMU Version 3 (SMMUv3) Support"
        depends on ARM64


Also, there are other places doing "1 << smmu->sid_bits", e.g.
arm_smmu_init_strtab_linear().

The disassembly shows it uses "sbfiz   x21, x20, 6, 32" instead of lsl. 1UL should be used if we want to take extra caution and don't prefer rely on compiler.


Then, can ssid_bits/s1cdmax be a concern similarly?

IIUC, ssid_bits is determined by IDR1_SSIDSIZE. It is GENMASK(10, 6). So it shouldn't be 32. IDR1_SIDSIZE is GENMASK(5, 0).


Thanks
Nicolin