linux-kernel - Re: [PATCH v2 1/2] mtd: spi-nor: core: add flag for doing optional SFDP

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: <87zfqoqh0y.fsf@geanix.com>
Date: Thu, 11 Jul 2024 13:55:57 +0200
From: Esben Haabendal <esben@...nix.com>
To: "Michael Walle" <mwalle@...nel.org>
Cc: "Tudor Ambarus" <tudor.ambarus@...aro.org>,  "Pratyush Yadav"
 <pratyush@...nel.org>,  "Miquel Raynal" <miquel.raynal@...tlin.com>,
  "Richard Weinberger" <richard@....at>,  "Vignesh Raghavendra"
 <vigneshr@...com>,  <linux-mtd@...ts.infradead.org>,
  <linux-kernel@...r.kernel.org>,  "Rasmus Villemoes"
 <rasmus.villemoes@...vas.dk>
Subject: Re: [PATCH v2 1/2] mtd: spi-nor: core: add flag for doing optional
 SFDP

"Michael Walle" <mwalle@...nel.org> writes:

>> > I actually had the same concern. But currently there is no
>> > non-deprecated way to handle this case, right?
>> >
>> > Right now we have the following cases:
>> >  (1) pure SFDP parsing
>> >  (2) non-SFDP flashes with static configuration only
>> >  (3) legacy implementation, where the magic flags decide whether we
>> >      use SFDP
>>
>> Actually, in the code we have two variants of 2.
>>
>> (2a) non-SFDP flashes with SPI_NOR_SKIP_SFDP set
>> (2b) non-SFDP flashes without SPI_NOR_SKIP_SFDP and with none of the
>>      DUAL/QUAD/OCTAL read bits set
>
> Isn't (2b) my case (3)? The SPI_NOR_SKIP_SFDP flag was intended to
> be for flashes we know for a fact, there are no SFDP tables.
>
> I'm looking at spi_nor_init_params(). Maybe I'm missing something?

Probably not. I might just be confusing things here.

Your case (3) is conditional. Based on the magic flags checking in
spi_nor_init_params_deprecated(), it is either doing static
configuration only (this is what I tried to redefine as case (2b)) or
parsing SFDP with fallback to static configuration.

The issue I am getting at is that while the 2 different ways to end up
doing static configuration only is almost identical, there is a slight
difference.

But I will be addressing this in a v3 patch of this series.

Coming up shortly, and looking forward to discuss it.

/Esben

>> These almost handled the same way. But
>> spi_nor_manufacturer_init_params() is only called for 2b, and not for
>> 2a.
>>
>> Is this desired behavior, or something that we want to align?
>>
>> > Which case is eventually used depends on the ID of the flash -
>> > assuming there will only be IDs which either fall into (1) *or* (2).
>> > That assumption is clearly wrong :)
>> >
>> > I'd propose a new case in spi_nor_init_params()
>> >  (4) try SFDP with a fallback to the static flags from the
>> >      flash_info db.
>>
>> /Esben