linux-kernel - Re: [PATCH v3 00/15] mtd: spi-nor: macronix: workaround for device id re-use

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <e079751d-e775-4dec-9959-c9c3fdcc8ecf@linaro.org>
Date: Thu, 26 Sep 2024 11:08:25 +0100
From: Tudor Ambarus <tudor.ambarus@...aro.org>
To: Esben Haabendal <esben@...nix.com>, Pratyush Yadav <pratyush@...nel.org>,
 Michael Walle <mwalle@...nel.org>, Miquel Raynal
 <miquel.raynal@...tlin.com>, Richard Weinberger <richard@....at>,
 Vignesh Raghavendra <vigneshr@...com>,
 Nicolas Ferre <nicolas.ferre@...rochip.com>,
 Alexandre Belloni <alexandre.belloni@...tlin.com>,
 Claudiu Beznea <claudiu.beznea@...on.dev>
Cc: linux-mtd@...ts.infradead.org, linux-kernel@...r.kernel.org,
 Rasmus Villemoes <rasmus.villemoes@...vas.dk>,
 linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v3 00/15] mtd: spi-nor: macronix: workaround for device id
 re-use

Hiya, Esben,

On 7/11/24 2:00 PM, Esben Haabendal wrote:
> Following up to various discussions, this series have now been
> modified so that it gets rid of the old deprecated approach
> for detecting when to do optional SFDP parsing.
> 
> Before these changes, spi-nor flashes were handled in 4 different
> ways:
> 

I'm adding a bit of extra context on each point you made. All your
points contain static init, you missed spi_nor_init_default_params()

There's a 0/ case where we have indeed just SFDP initialized flashes,
and that is the generic flash driver.

0/ SFDP only, generic flash driver

> (1) SFDP only [size==0]

1/ static + SFDP

spi_nor_init_default_params(nor);
spi_nor_parse_sfdp();

> 
> (2a) static config only [size!=0 && no_sfdp_flags & SPI_NOR_SKIP_SFDP]

2a/ is
spi_nor_init_default_params(nor);
spi_nor_no_sfdp_init_params(nor);

> 
> (2b) static config only
>        [size!=0 &&
>         !(no_sfdp_flags & (SPI_NOR_SKIP_SFDP | 
>            SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
>            SPI_NOR_OCTAL_READ | SPI_NOR_OCTAL_DTR_READ))]

2b/ is
spi_nor_init_default_params(nor);
spi_nor_init_params_deprecated(nor); //where parse SFDP is not called
	spi_nor_no_sfdp_init_params(nor);
	spi_nor_manufacturer_init_params(nor);>
> (3) SFDP with fallback to static config
>        [size!=0 &&
>         !(no_sfdp_flags & SPI_NOR_SKIP_SFDP) &&
>         (no_sfdp_flags & SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
>            SPI_NOR_OCTAL_READ | SPI_NOR_OCTAL_DTR_READ))]
> 

3/ is 2b/ with parse SFDP called and rollback mechanism
spi_nor_init_default_params(nor);
spi_nor_init_params_deprecated(nor);
	spi_nor_no_sfdp_init_params(nor);
	spi_nor_manufacturer_init_params(nor);
	spi_nor_sfdp_init_params_deprecated(nor);

All cases from above are followed by a call to spi_nor_late_init_params().

> Cases (2a) and (2b) have been handled slightly different, with
> manufacturer and flash_info ->default_init() hooks being called in
> case (2b) but not in case (2a).

default_init() was a mistake and we shall remove it and replace it with
late_init(). The challenge is to do that without affecting backwards
compatibility. But let's move this aside for the moment
> 
> With this series, that is changed to this simpler approach instead:
> 
> (1) SFDP only [size==0]
> 
> (2) static config only
>       [size!=0 && !(no_sfdp_flags & SPI_NOR_TRY_SFDP)]
> 
> (3) SFDP with fallback to static config
>       [size!=0 && (no_sfdp_flags & SPI_NOR_TRY_SFDP)]
> 
> Existing struct flash_info entries are modified, so that all those
> that was case (2a) or (2b) are now case (2), and those that were (1)
> and (3) are still (1) and (3).

We indeed want 2a/ and 2b/ to be squashed, ideally by removing the
default_init() hook.

And if we really want SFDP-only init, we shall not call
spi_nor_init_default_params() in this case.

Your patches do not apply on latest SPI NOR, I tried locally. Can you
point me to a branch of yours where I can see them? No need to resend.

Thanks,
ta