Re: [PATCH 03/14] spi: spi-fsl-dspi: restrict register range for regmap access

[James Clark]

On 09/05/2025 3:06 pm, Vladimir Oltean wrote:
> On Fri, May 09, 2025 at 12:05:50PM +0100, James Clark wrote:
> > From: Larisa Grigore <larisa.grigore@xxxxxxx>
> >
> > DSPI registers are NOT continuous, some registers are reserved and
> > accessing them from userspace will trigger external abort, add regmap
> > register access table to avoid below abort:
> >
> > Internal error: synchronous external abort: 96000210 1 PREEMPT SMP
> > Modules linked in: fuse dummy tun hse sch_fq_codel openvswitch nsh
> > nf_conncount nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4
> > CPU: 2 PID: 18231 Comm: read_all Not tainted 5.2.33-yocto-standard #1
> > Hardware name: Freescale S32G275 (DT)
> > pstate: 20000085 (nzCv daIf -PAN -UAO)
> > pc : regmap_mmio_read32le+0x24/0x48
> > lr : regmap_mmio_read+0x48/0x70
> > sp : ffffff801123bb70
> > x29: ffffff801123bb70 x28: ffffffc873b5c000
> > x27: ffffff8010b408f0 x26: 0000000000000001
> > x25: 000000000000013c x24: ffffff801123be40
> > x23: 00000000000003ff x22: ffffff801123bcfc
> > x21: ffffff801123bcfc x20: ffffffc873a9e500
> > x19: 0000000000000024 x18: 0000000000000020
> > x17: 0000000000000000 x16: 0000000000000000
> > x15: ffffffc876189160 x14: 0000000000000003
> > x13: ffffffc873bf73ff x12: ffffffc873bf707e
> > x11: 0000000000000000 x10: 0000000000000000
> > x9 : 0000000000000000 x8 : ffffffc83fca4e00
> > x7 : 000000000000000f x6 : ffffffc873bf7083
> > x5 : 00000000fffffff9 x4 : 0000000000000002
> > x3 : ffffff801061f058 x2 : ffffff801061ee18
> > x1 : 0000000000000024 x0 : ffffff8011490024
>
> I think you can leave the register dump out, it doesn't seem of much use.

Will reduce the output and add a reproducer and fixes: tag.

> > Call trace:
> > regmap_mmio_read32le+0x24/0x48
> > regmap_mmio_read+0x48/0x70
> > _regmap_bus_reg_read+0x38/0x48
> > _regmap_read+0x68/0x1b0
> > regmap_read+0x50/0x78
> > regmap_read_debugfs+0x120/0x338
> > regmap_map_read_file+0x44/0x58
> > full_proxy_read+0x68/0x98
> > __vfs_read+0x48/0x90
> > vfs_read+0xb0/0x130
> > ksys_read+0x7c/0x108
> > __arm64_sys_read+0x24/0x30
> > el0_svc_common.constprop.0+0x74/0x168
> > el0_svc_handler+0x70/0x90
> > el0_svc+0x8/0xc
> >
> > Co-developed-by: Xulin Sun <xulin.sun@xxxxxxxxxxxxx>
> > Signed-off-by: Xulin Sun <xulin.sun@xxxxxxxxxxxxx>
> > Signed-off-by: Larisa Grigore <larisa.grigore@xxxxxxx>
> > Signed-off-by: James Clark <james.clark@xxxxxxxxxx>
> > ---
>
> Do you have a reproducer for any of the supported SoCs? On LS1028A, "cat
> /sys/kernel/debug/regmap/2120000.spi/registers" runs fine and does not
> crash.

On S32G3:

 # cat /sys/kernel/debug/regmap/401d8000.spi/registers

 Internal error: synchronous external abort: 0000000096000210 [#1]  SMP
 regmap_mmio_read32le+0x14/0x38 (P)
 _regmap_bus_reg_read+0x104/0x140
 _regmap_read+0x19c/0x2d0
 regmap_read+0x60/0x90
 regmap_read_debugfs+0x15c/0x330
 ...

Not sure why it wouldn't be the same on LS1028A because the register 
layout is basically the same. Maybe it just ignores the read instead? 
Either way I think the fix is still worthwhile to backport because we 
won't be able to test every device.