linux-kernel - Re: [PATCH 1/2] mtd: spi-nor: winbond: Add support for w25q01jv

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: <mafs0o7014lc5.fsf@kernel.org>
Date: Mon, 20 Jan 2025 14:21:46 +0000
From: Pratyush Yadav <pratyush@...nel.org>
To: Miquel Raynal <miquel.raynal@...tlin.com>
Cc: Pratyush Yadav <pratyush@...nel.org>,  Tudor Ambarus
 <tudor.ambarus@...aro.org>,  Michael Walle <mwalle@...nel.org>,  Richard
 Weinberger <richard@....at>,  Vignesh Raghavendra <vigneshr@...com>,
  Thomas Petazzoni <thomas.petazzoni@...tlin.com>,  Steam Lin
 <STLin2@...bond.com>,  linux-mtd@...ts.infradead.org,
  linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] mtd: spi-nor: winbond: Add support for w25q01jv

On Mon, Jan 20 2025, Miquel Raynal wrote:

> Hi Pratyush,
>
>> Okay, I am confused because you said earlier that:
>>
>>> The bug that has been experienced followed this sequence:
>>> - send the write enable command (non-volatile)
>>> - wait for the ready/busy bit, ie. wait for the WEL bit to be set
>>>   because it is non-volatile write
>>> - active die is ready, (but idle die is not!)
>>> - enter 4-byte address mode, only the die that is ready processes the
>>>   command.
>>
>> Which says the WEL bit being set itself is racy. What I understand from
>> that is one die is ready to take writes and the other is not. Now when
>> you try to write the SR to enable 4B mode, it would only work on the die
>> that got the WEL set. The other one ignores it and stays in 3B mode. Do
>> I understand this correctly? To fix this you need to wait after the
>> write enable, before you initiate the write SR operation.
>
> Sorry for the confusion, I got myself confused as well. I double checked
> with Winbond and I think I have the correct explanation now:
>
> The WEL bit is volatile, there is no delay when setting it (well, about
> 10ns, but no specific deviation).
>
> On most chips, WEL enables all write operations:
>   * (single/dual/quad) page programs
>   * (sector/block/chip) erases
>   * status register write
>   * erase/program of other internal registers (like security registers)
>
> On Winbond, the above applies, but with the usual "Write Enable command
> (06h)", the status register bits are non-volatile, ie. they are stored
> in non-volatile cells (which takes time to program and are subject
> to deviations across dies). Hence, they added another command, called
> "Write Enable for Volatile Status Register (50h)" which is an addition
> to the usual "Write Enable command (06h)" which causes:
> - enabling writes on the status register only (if the WEL bit is not yet
>   set)
> - using volatile writes for the status register bits (ie. they are using
>   some kind of local cache which update almost immediately).
>
> So basically, if you do the following:
> - status register write
> - check the status bit with the standard helper
> - (and quickly after) do anything else on the idle die
> In this case you could experience a race, but that is not related to the
> Write Enable command.
>
> In general I believe enabling volatile status register writes would not
> be useful as long as we have the "read the status from all dies"
> workaround.
>
> Let me know if something is still unclear.

Ah, okay. That all makes sense now. With this explanation, your patches
should be doing the right thing. I will do a round of review on the v3
soon, but they looked mostly good when I last took a brief look.

-- 
Regards,
Pratyush Yadav