[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <8b7be083-cef8-4b68-af4a-2bb5b30a9b9c@linaro.org>
Date: Mon, 19 May 2025 11:25:53 +0100
From: James Clark <james.clark@...aro.org>
To: Vladimir Oltean <olteanv@...il.com>
Cc: Mark Brown <broonie@...nel.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
<conor+dt@...nel.org>, Frank Li <Frank.Li@....com>,
Chester Lin <chester62515@...il.com>, Matthias Brugger <mbrugger@...e.com>,
Ghennadi Procopciuc <ghennadi.procopciuc@....nxp.com>,
NXP S32 Linux Team <s32@....com>, Shawn Guo <shawnguo@...nel.org>,
Sascha Hauer <s.hauer@...gutronix.de>,
Pengutronix Kernel Team <kernel@...gutronix.de>,
Fabio Estevam <festevam@...il.com>, larisa.grigore@....com, arnd@...aro.org,
andrei.stefanescu@....com, dan.carpenter@...aro.org,
linux-spi@...r.kernel.org, imx@...ts.linux.dev,
linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, Xulin Sun <xulin.sun@...driver.com>
Subject: Re: [PATCH 03/14] spi: spi-fsl-dspi: restrict register range for
regmap access
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@....com>
>>
>> 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@...driver.com>
>> Signed-off-by: Xulin Sun <xulin.sun@...driver.com>
>> Signed-off-by: Larisa Grigore <larisa.grigore@....com>
>> Signed-off-by: James Clark <james.clark@...aro.org>
>> ---
>
> 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.
Powered by blists - more mailing lists