linux-kernel - Re: [RFC PATCH v2 1/5] mtd: cfi_cmdset_0002: Add support for polling status register

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]

Date:   Mon, 25 Mar 2019 18:27:41 +0530
From:   Vignesh Raghavendra <vigneshr@...com>
To:     Joakim Tjernlund <Joakim.Tjernlund@...inera.com>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "computersforpeace@...il.com" <computersforpeace@...il.com>,
        "bbrezillon@...nel.org" <bbrezillon@...nel.org>,
        "marek.vasut@...il.com" <marek.vasut@...il.com>,
        "dwmw2@...radead.org" <dwmw2@...radead.org>,
        "richard@....at" <richard@....at>
CC:     "nsekhar@...com" <nsekhar@...com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-mtd@...ts.infradead.org" <linux-mtd@...ts.infradead.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "masonccyang@...c.com.tw" <masonccyang@...c.com.tw>,
        "tudor.ambarus@...rochip.com" <tudor.ambarus@...rochip.com>,
        "sergei.shtylyov@...entembedded.com" 
        <sergei.shtylyov@...entembedded.com>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "arnd@...db.de" <arnd@...db.de>
Subject: Re: [RFC PATCH v2 1/5] mtd: cfi_cmdset_0002: Add support for polling
 status register

Hi,

On 21/03/19 11:41 PM, Joakim Tjernlund wrote:
> On Thu, 2019-03-21 at 23:15 +0530, Vignesh Raghavendra wrote:
>>
>> HyperFlash devices are compliant with CFI AMD/Fujitsu Extended Command
>> Set(0x0002) for flash operations, therefore drivers/mtd/chips/cfi_cmdset_0002.c
>> can be use as is. But these devices do not support DQ polling method of
>> determining chip ready/good status. These flashes provide Status
>> Register whose bits can be polled to know status of flash operation.
>>
>> Cypress HyperFlash datasheet here[1], talks about CFI Amd/Fujitsu
>> Extended Query version 1.5. Bit 0 of "Software Features supported" field
>> of CFI Primary Vendor-Specific Extended Query table indicates
>> presence/absence of status register and Bit 1 indicates whether or not
>> DQ polling is supported. Using these bits, its possible to determine
>> whether flash supports DQ polling or need to use Status Register.
>>
>> Add support for polling status register to know device ready/status of
>> erase/write operations when DQ polling is not supported.
> 
> Isn't this new Status scheme just a copy of Intels(cmdset_0001)?

Yes, but with one difference: At the end of program/erase operation,
device directly enters status register mode and  starts reflecting
status register content at any address.
The device remains in the read status register state until another
command is written to the device. Therefore there is notion of device is
in "status register read mode" (FL_STATUS) state

But in case of cfi_cmdset_0002, once program/erase operation is
complete, device returns to previous address space overlay from which
operation was started from (mostly read mode)

In order to enter status register overlay mode, Read Status command is
to be written to addr_unlock1(0x555) address. The overlay is in effect
for one read access, specifically the next read access that follows the
Status Register Read command
Therefore code around FL_STATUS state in cfi_cmdset_0001 is not
applicable to cfi_cmdset_0002 as is.

> If so I think the new status impl. in 0002 should borrow from 0001 as this is a
> hardened and battle tested impl.
>

In case of cfi_cmdset_0001.c, program/erase is followed by
inval_cache_and_wait_for_operation() to poll ready bit and based on
status register value, success or the error handling is done.

Most of the code corresponding to inval_cache_and_wait_for_operation()
is already in cfi_cmdset_0002.c. So, whats missing in this patch is
handling and reporting of errors as reflected in status register after
write/erase failures. I will add that in the next version.

But, I don't see much to borrow apart from error handling sequence.
Please, let me know if I missed something.

> I know other modern 0002 chips supports both old and new impl. of Status and I world
> guess that we will see more chips with new Status only.
> 

Agreed. Newer devices would mostly be CFI 1.5.

-- 
Regards
Vignesh