| 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
| ||
|
Message-ID: <7762352.EvYhyI6sBW@steina-w>
Date: Tue, 14 Jan 2025 17:24:32 +0100
From: Alexander Stein <alexander.stein@...tq-group.com>
To: pratyush@...nel.org, mwalle@...nel.org, miquel.raynal@...tlin.com, richard@....at, vigneshr@...com, linux-mtd@...ts.infradead.org, linux-kernel@...r.kernel.org, Tudor Ambarus <tudor.ambarus@...aro.org>
Cc: alvinzhou@...c.com.tw, leoyu@...c.com.tw, Cheng Ming Lin <chengminglin@...c.com.tw>, stable@...r.kernel.org, Cheng Ming Lin <linchengming884@...il.com>
Subject: Re: [PATCH v2 1/1] mtd: spi-nor: core: replace dummy buswidth from addr to data
Hi Tudor,
Am Dienstag, 14. Januar 2025, 14:26:47 CET schrieb Tudor Ambarus:
> On 1/14/25 12:57 PM, Alexander Stein wrote:
> > Hello everyone,
>
> Hi,
>
> >
> > Am Dienstag, 12. November 2024, 08:52:42 CET schrieb Cheng Ming Lin:
> >> From: Cheng Ming Lin <chengminglin@...c.com.tw>
> >>
> >> The default dummy cycle for Macronix SPI NOR flash in Octal Output
> >> Read Mode(1-1-8) is 20.
> >>
> >> Currently, the dummy buswidth is set according to the address bus width.
> >> In the 1-1-8 mode, this means the dummy buswidth is 1. When converting
> >> dummy cycles to bytes, this results in 20 x 1 / 8 = 2 bytes, causing the
> >> host to read data 4 cycles too early.
> >>
> >> Since the protocol data buswidth is always greater than or equal to the
> >> address buswidth. Setting the dummy buswidth to match the data buswidth
> >> increases the likelihood that the dummy cycle-to-byte conversion will be
> >> divisible, preventing the host from reading data prematurely.
> >>
> >> Fixes: 0e30f47232ab5 ("mtd: spi-nor: add support for DTR protocol")
> >> Cc: stable@...r.kernel.org
> >> Reviewd-by: Pratyush Yadav <pratyush@...nel.org>
> >> Signed-off-by: Cheng Ming Lin <chengminglin@...c.com.tw>
> >> ---
> >> drivers/mtd/spi-nor/core.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> >> index f9c189ed7353..c7aceaa8a43f 100644
> >> --- a/drivers/mtd/spi-nor/core.c
> >> +++ b/drivers/mtd/spi-nor/core.c
> >> @@ -89,7 +89,7 @@ void spi_nor_spimem_setup_op(const struct spi_nor *nor,
> >> op->addr.buswidth = spi_nor_get_protocol_addr_nbits(proto);
> >>
> >> if (op->dummy.nbytes)
> >> - op->dummy.buswidth = spi_nor_get_protocol_addr_nbits(proto);
> >> + op->dummy.buswidth = spi_nor_get_protocol_data_nbits(proto);
> >>
> >> if (op->data.nbytes)
> >> op->data.buswidth = spi_nor_get_protocol_data_nbits(proto);
> >>
> >
> > I just noticed this commit caused a regression on my i.MX8M Plus based board,
> > detected using git bisect.
> > DT: arch/arm64/boot/dts/freescale/imx8mp-tqma8mpql-mba8mpxl.dts
> > Starting with this patch read is only 1S-1S-1S, before it was
> > 1S-1S-4S.
> >
> > before:
> >> cat /sys/kernel/debug/spi-nor/spi0.0/params
> >> name mt25qu512a
> >> id 20 bb 20 10 44 00
> >> size 64.0 MiB
> >> write size 1
> >> page size 256
> >> address nbytes 4
> >> flags HAS_SR_TB | 4B_OPCODES | HAS_4BAIT | HAS_LOCK | HAS_4BIT_BP
> >> | HAS_SR_BP3_BIT6 | SOFT_RESET
> >>
> >> opcodes
> >>
> >> read 0x6c
> >>
> >> dummy cycles 8
> >>
> >> erase 0xdc
> >> program 0x12
> >> 8D extension none
> >>
> >> protocols
> >>
> >> read 1S-1S-4S
> >> write 1S-1S-1S
> >> register 1S-1S-1S
> >>
> >> erase commands
> >>
> >> 21 (4.00 KiB) [1]
> >> dc (64.0 KiB) [3]
> >> c7 (64.0 MiB)
> >>
> >> sector map
> >>
> >> region (in hex) | erase mask | overlaid
> >> ------------------+------------+----------
> >> 00000000-03ffffff | [ 3] | no
> >
> > after:
> >> cat /sys/kernel/debug/spi-nor/spi0.0/params
> >> name mt25qu512a
> >> id 20 bb 20 10 44 00
> >> size 64.0 MiB
> >> write size 1
> >> page size 256
> >> address nbytes 4
> >> flags HAS_SR_TB | 4B_OPCODES | HAS_4BAIT | HAS_LOCK | HAS_4BIT_BP
> >> | HAS_SR_BP3_BIT6 | SOFT_RESET
> >>
> >> opcodes
> >>
> >> read 0x13
> >>
> >> dummy cycles 0
> >>
> >> erase 0xdc
> >> program 0x12
> >> 8D extension none
> >>
> >> protocols
> >>
> >> read 1S-1S-1S
> >> write 1S-1S-1S
> >> register 1S-1S-1S
> >>
> >> erase commands
> >>
> >> 21 (4.00 KiB) [1]
> >> dc (64.0 KiB) [3]
> >> c7 (64.0 MiB)
> >>
> >> sector map
> >>
> >> region (in hex) | erase mask | overlaid
> >> ------------------+------------+----------
> >> 00000000-03ffffff | [ 3] | no
> >
> > AFAICT the patch seems sane, so it probably just uncovered another
> > problem already lurking somewhere deeper.
> > Given the HW similarity I expect imx8mn and imx8mm based platforms to be
> > affected as well.
> > Reverting this commit make the read to be 1S-1S-4S again.
> > Any ideas ow to tackling down this problem?
> >
>
> My guess is that 1S-1S-4S is stripped out in
> spi_nor_spimem_adjust_hwcaps(). Maybe the controller has some limitation
> in nxp_fspi_supports_op(). Would you add some prints, and check these
> chunks of code?
Thanks for the fast response. I was able to track it down.
Eventually the buswidth check in spi_check_buswidth_req fails.
For command 0x3c:
Before revert:
> mode: 0x800, buswidth: 2
After revert
> mode: 0x800, buswidth: 1
The mode is set to SPI_RX_QUAD. Thus the check for dummy buswidth fails
now that data_nbits are used now.
For command 0x6c it's similar but op->dummy.buswidth is 4 now.
It boils down that there are SPI controllers which have
> spi-tx-bus-width = <1>;
> spi-rx-bus-width = <4>;
set in their DT nodes.
So it seems this combination is not supported.
Best regards,
Alexander
--
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/
Powered by blists - more mailing lists