linux-kernel - Re: [PATCH] mtd: spi-nor: Fix w25q01jv flags

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aSTySAFSQCxoFz/t@molberding.nvidia.com>
Date: Mon, 24 Nov 2025 16:03:20 -0800
From: Marc Olberding <molberding@...dia.com>
To: Miquel Raynal <miquel.raynal@...tlin.com>
Cc: Michael Walle <mwalle@...nel.org>,
	Tudor Ambarus <tudor.ambarus@...aro.org>,
	Pratyush Yadav <pratyush@...nel.org>,
	Richard Weinberger <richard@....at>,
	Vignesh Raghavendra <vigneshr@...com>,
	linux-mtd@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mtd: spi-nor: Fix w25q01jv flags

On Mon, Nov 24, 2025 at 10:15:28AM +0100, Miquel Raynal wrote:
> On 24/11/2025 at 09:50:27 +01, "Michael Walle" <mwalle@...nel.org> wrote:
> 
> > On Mon Nov 24, 2025 at 9:25 AM CET, Miquel Raynal wrote:
> >> Hi,
> >>
> >> On 24/11/2025 at 09:12:38 +01, "Michael Walle" <mwalle@...nel.org> wrote:
> >>
> >>> Hi,
> >>>
> >>>>> +		.no_sfdp_flags = SECT_4K,
> >>>>
> >>>> This one is the right fix and should stand alone in its own patch (first
> >>>> in the series if you add support for the block protection).
> >>>
> >>> Only if that flash really doesn't have SFDP. But since the entry
> >>> didn't have a size property the flash *must* have SFDP in the first
> >>> place.  Otherwise it won't even be probed. Please provide a dump of
> >>> the SFDP tables, see [1].
> >>
> >> SFDP data is in lore
> >
> > At least yours :) And if I decode that correctly by hand, it has the
> > 4k erase size bit set as well as the correct opcode 20h or 21h for
> > 4byte addressing.
> >
> >> , but not the params which are missing (?) Marc, can
> >> you compare with your data?
> >> https://lore.kernel.org/all/20250110-winbond-6-12-rc1-nor-volatile-bit-v3-1-735363f8cc7d@bootlin.com/
> >>
> >>> Also please provide the contents of
> >>> /sys/kernel/debug/spi-nor/spiN.N/params.
> >>>
> >>> -michael
Thanks all for the discussion, I appreciate the insight. After investigating some more,
it looks like the sfdp table is identical. I'll be honest I'm not a spi expert.

This data was taken with the original patch, as sfdp didn't populate as I had provided
the size field in the spi-nor-info table, this seems to cause us to skip any sfdp parsing
and go down the deprecated path. Looking at the info table, it looks like all but 4
spi chips in the winbond table use the deprecated path. Its not clear to me however,
why it works on Miquel's board but not mine. 

/tmp/xxd -p /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
53464450060101ff00060110800000ff84000102d00000ff03000102f000
00ffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffe520fbffffffff3f44eb086b083b42bbfeffffffffff
0000ffff40eb0c200f5210d800003602a60082ea14e2e96376337a757a75
f7a2d55c19f74dffe970f9a5ffffffffffffffffffffffffffffffffff0a
f0ff21ffdcff

md5sum ./sfdp 
a7b9dbf76e99a33db99e557b6676588a  ./sfdp

cat /sys/kernel/debug/spi-nor/spi0.0/params
name            (null)
id              ef 40 21 00 00 00
size            128 MiB
write size      1
page size       256
address nbytes  4
flags           4B_OPCODES | HAS_4BAIT | HAS_16BIT_SR | NO_READ_CR | SOFT_RESET

opcodes
 read           0x6c
  dummy cycles  8
 erase          0xdc
 program        0x34
 8D extension   none

protocols
 read           1S-1S-4S
 write          1S-1S-4S
 register       1S-1S-1S

erase commands
 21 (4.00 KiB) [1]
 dc (64.0 KiB) [3]
 c7 (128 MiB)

sector map
 region (in hex)   | erase mask | overlaid
 ------------------+------------+----------
 00000000-07ffffff |     [   3] | no


> >> My understanding (which may clearly be erroneous) is that most of these
> >> flashes support 4K blocks but somehow don't advertise it in their SFDP
> >> data, so every time we describe a chip we must remember to tick that
> >> flag.
> >
> > Which flag? SECT_4K? I don't think that will be used at all, does
> > it? It's only used in spi_nor_no_sfdp_init_params() which in turn is
> > only called in spi_nor_init_params_deprecated() (or if SKIP_SFDP is
> > set). 
> >

See my above comment about setting the size in the tables. This eventually
gets used by spi_nor_needs_sfdp. This ends up being most of the winbond chips
defined today.

All in all, I'm a little confused what the difference is between Miquel's and my
tests. Any direction would be appreciated, in the meantime, I'm going to continue
debugging on my end.

Best regards,
Marc Olberding