From: Yang Shi <yang@os.amperecomputing.com>
To: jgg@ziepe.ca, nicolinc@nvidia.com, james.morse@arm.com,
will@kernel.org, robin.murphy@arm.com
Cc: yang@os.amperecomputing.com,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
Subject: [v2 PATCH] iommu/arm-smmu-v3: Fix L1 stream table index calculation for 32-bit sid size
Date: Wed, 2 Oct 2024 10:55:14 -0700 [thread overview]
Message-ID: <20241002175514.1165299-1-yang@os.amperecomputing.com> (raw)
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 some platforms, for example, AmpereOne, have 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 and other platform [1].
UBSAN also reported:
UBSAN: shift-out-of-bounds in drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c:3628:29
shift exponent 32 is too large for 32-bit type 'int'
CPU: 70 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.12.0-rc1 #4
Hardware name: ZOLLNER SUNMOONLAKE/SunMoon Lake, BIOS 00.00. 2024-08-28 18:42:45 08/28/2024
Call trace:
dump_backtrace+0xdc/0x140
show_stack+0x20/0x40
dump_stack_lvl+0x60/0x80
dump_stack+0x18/0x28
ubsan_epilogue+0x10/0x48
__ubsan_handle_shift_out_of_bounds+0xd8/0x1a0
arm_smmu_init_structures+0x374/0x3c8
arm_smmu_device_probe+0x208/0x600
platform_probe+0x70/0xe8
really_probe+0xc8/0x3a0
__driver_probe_device+0x84/0x160
driver_probe_device+0x44/0x130
__driver_attach+0xcc/0x208
bus_for_each_dev+0x84/0x100
driver_attach+0x2c/0x40
bus_add_driver+0x158/0x290
driver_register+0x70/0x138
__platform_driver_register+0x2c/0x40
arm_smmu_driver_init+0x28/0x40
do_one_initcall+0x60/0x318
do_initcalls+0x198/0x1e0
kernel_init_freeable+0x18c/0x1e8
kernel_init+0x28/0x160
ret_from_fork+0x10/0x20
Using 64 bit immediate when doing shift can solve the problem. The
disassembly after the fix looks like:
ldr w20, [x19, 828] //, smmu_7(D)->sid_bits
mov x0, 1
lsl x0, x0, x20
There are a couple of problematic places, extracted the shift into a helper.
[1] https://lore.kernel.org/lkml/d4b53bbb-333a-45b9-9eb0-23ddd0820a14@arm.com/
Fixes: ce410410f1a7 ("iommu/arm-smmu-v3: Add arm_smmu_strtab_l1/2_idx()")
Tested-by: James Morse <james.morse@arm.com>
Signed-off-by: Yang Shi <yang@os.amperecomputing.com>
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 8 +++++---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 5 +++++
2 files changed, 10 insertions(+), 3 deletions(-)
v2: * Extracted the shift into a helper per Jason Gunthorpe.
* Covered more places per Nicolin Chen and Jason Gunthorpe.
* Used 1ULL instead of 1UL to guarantee 64 bit per Robin Murphy.
* Made the subject more general since this is not AmpereOne specific
problem per the report from James Morse.
* Collected t-b tag from James Morse.
* Added Fixes tag in commit log.
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..4eafd9f04808 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -3624,8 +3624,9 @@ 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 max_sid = arm_smmu_strtab_max_sid(smmu);
unsigned int last_sid_idx =
- arm_smmu_strtab_l1_idx((1 << smmu->sid_bits) - 1);
+ arm_smmu_strtab_l1_idx(max_sid - 1);
/* Calculate the L1 size, capped to the SIDSIZE. */
cfg->l2.num_l1_ents = min(last_sid_idx + 1, STRTAB_MAX_L1_ENTRIES);
@@ -3657,8 +3658,9 @@ static int arm_smmu_init_strtab_linear(struct arm_smmu_device *smmu)
{
u32 size;
struct arm_smmu_strtab_cfg *cfg = &smmu->strtab_cfg;
+ unsigned int max_sid = arm_smmu_strtab_max_sid(smmu);
- size = (1 << smmu->sid_bits) * sizeof(struct arm_smmu_ste);
+ size = max_sid * sizeof(struct arm_smmu_ste);
cfg->linear.table = dmam_alloc_coherent(smmu->dev, size,
&cfg->linear.ste_dma,
GFP_KERNEL);
@@ -3668,7 +3670,7 @@ static int arm_smmu_init_strtab_linear(struct arm_smmu_device *smmu)
size);
return -ENOMEM;
}
- cfg->linear.num_ents = 1 << smmu->sid_bits;
+ cfg->linear.num_ents = max_sid;
arm_smmu_init_initial_stes(cfg->linear.table, cfg->linear.num_ents);
return 0;
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
index 1e9952ca989f..f7e8465c629a 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -853,6 +853,11 @@ struct arm_smmu_master_domain {
ioasid_t ssid;
};
+static inline unsigned int arm_smmu_strtab_max_sid(struct arm_smmu_device *smmu)
+{
+ return (1ULL << smmu->sid_bits);
+}
+
static inline struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom)
{
return container_of(dom, struct arm_smmu_domain, domain);
--
2.41.0
next reply other threads:[~2024-10-02 17:57 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-10-02 17:55 Yang Shi [this message]
2024-10-02 18:14 ` [v2 PATCH] iommu/arm-smmu-v3: Fix L1 stream table index calculation for 32-bit sid size Jason Gunthorpe
2024-10-02 18:17 ` Nicolin Chen
2024-10-02 19:04 ` Yang Shi
2024-10-02 19:22 ` Nicolin Chen
2024-10-02 19:40 ` Jason Gunthorpe
2024-10-02 20:00 ` Yang Shi
2024-10-02 20:05 ` Yang Shi
2024-10-03 11:16 ` Jason Gunthorpe
2024-10-03 15:31 ` Yang Shi
2024-10-04 12:43 ` Jason Gunthorpe
2024-10-04 16:05 ` Yang Shi
2024-10-04 16:14 ` Jason Gunthorpe
2024-10-04 16:29 ` Yang Shi
2024-10-04 16:37 ` Yang Shi
2024-10-04 16:41 ` Jason Gunthorpe
2024-10-04 16:46 ` Yang Shi
2024-10-02 19:50 ` Yang Shi
2024-10-02 18:21 ` Robin Murphy
2024-10-02 18:36 ` Yang Shi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20241002175514.1165299-1-yang@os.amperecomputing.com \
--to=yang@os.amperecomputing.com \
--cc=james.morse@arm.com \
--cc=jgg@ziepe.ca \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=nicolinc@nvidia.com \
--cc=robin.murphy@arm.com \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox