| 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
| ||
|
Message-ID: <ddf792f8-ea2a-2659-2e9c-d6d94eb5168d@ti.com>
Date: Sat, 9 Nov 2019 14:34:03 +0530
From: Vignesh Raghavendra <vigneshr@...com>
To: <Tudor.Ambarus@...rochip.com>, <boris.brezillon@...labora.com>
CC: <miquel.raynal@...tlin.com>, <richard@....at>,
<linux-mtd@...ts.infradead.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v5 2/6] mtd: spi-nor: Rework the disabling of block write
protection
On 07-Nov-19 2:11 PM, Tudor.Ambarus@...rochip.com wrote:
> From: Tudor Ambarus <tudor.ambarus@...rochip.com>
>
> spi_nor_unlock() unlocks blocks of memory or the entire flash memory
> array, if requested. clear_sr_bp() unlocks the entire flash memory
> array at boot time. This calls for some unification, clear_sr_bp() is
> just an optimization for the case when the unlock request covers the
> entire flash size.
>
> Get rid of clear_sr_bp() and introduce spi_nor_unlock_all(), which is
> just a call to spi_nor_unlock() for the entire flash memory array.
> This fixes a bug that was present in spi_nor_spansion_clear_sr_bp().
> When the QE bit was zero, we used the Write Status (01h) command with
> one data byte, which might cleared the Status Register 2. We now always
> use the Write Status (01h) command with two data bytes when
> SNOR_F_HAS_16BIT_SR is set, to avoid clearing the Status Register 2.
>
> The SNOR_F_NO_READ_CR case is treated as well. When the flash doesn't
> support the CR Read command, we make an assumption about the value of
> the QE bit. In spi_nor_init(), call spi_nor_quad_enable() first, then
> spi_nor_unlock_all(), so that at the spi_nor_unlock_all() time we can
> be sure the QE bit has value one, because of the previous call to
> spi_nor_quad_enable().
>
> Get rid of the MFR handling and implement specific manufacturer
> default_init() fixup hooks.
>
> Note that this changes a bit the logic for the SNOR_MFR_ATMEL,
> SNOR_MFR_INTEL and SNOR_MFR_SST cases. Before this patch, the Atmel,
> Intel and SST chips did not set the locking ops, but unlocked the entire
> flash at boot time, while now they are setting the locking ops to
> stm_locking_ops. This should work, since the the disable of the block
Nit: s/the the/the
> protection at the boot time used the same Status Register bits to unlock
> the flash, as in the stm_locking_ops case.
>
> Suggested-by: Boris Brezillon <boris.brezillon@...labora.com>
> Signed-off-by: Tudor Ambarus <tudor.ambarus@...rochip.com>
> ---
Reviewed-by: Vignesh Raghavendra <vigneshr@...com>
Regards
Vignesh
> drivers/mtd/spi-nor/spi-nor.c | 140 +++++++++++++++---------------------------
> include/linux/mtd/spi-nor.h | 3 -
> 2 files changed, 50 insertions(+), 93 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index d696334f25f0..06aac894ee6d 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -2185,74 +2185,6 @@ static int sr2_bit7_quad_enable(struct spi_nor *nor)
> return 0;
> }
>
> -/**
> - * spi_nor_clear_sr_bp() - clear the Status Register Block Protection bits.
> - * @nor: pointer to a 'struct spi_nor'
> - *
> - * Read-modify-write function that clears the Block Protection bits from the
> - * Status Register without affecting other bits.
> - *
> - * Return: 0 on success, -errno otherwise.
> - */
> -static int spi_nor_clear_sr_bp(struct spi_nor *nor)
> -{
> - int ret;
> - u8 mask = SR_BP2 | SR_BP1 | SR_BP0;
> -
> - ret = spi_nor_read_sr(nor, nor->bouncebuf);
> - if (ret)
> - return ret;
> -
> - nor->bouncebuf[0] &= ~mask;
> -
> - return spi_nor_write_sr(nor, nor->bouncebuf, 1);
> -}
> -
> -/**
> - * spi_nor_spansion_clear_sr_bp() - clear the Status Register Block Protection
> - * bits on spansion flashes.
> - * @nor: pointer to a 'struct spi_nor'
> - *
> - * Read-modify-write function that clears the Block Protection bits from the
> - * Status Register without affecting other bits. The function is tightly
> - * coupled with the spansion_read_cr_quad_enable() function. Both assume that
> - * the Write Register with 16 bits, together with the Read Configuration
> - * Register (35h) instructions are supported.
> - *
> - * Return: 0 on success, -errno otherwise.
> - */
> -static int spi_nor_spansion_clear_sr_bp(struct spi_nor *nor)
> -{
> - int ret;
> - u8 mask = SR_BP2 | SR_BP1 | SR_BP0;
> - u8 *sr_cr = nor->bouncebuf;
> -
> - /* Check current Quad Enable bit value. */
> - ret = spi_nor_read_cr(nor, &sr_cr[1]);
> - if (ret)
> - return ret;
> -
> - /*
> - * When the configuration register Quad Enable bit is one, only the
> - * Write Status (01h) command with two data bytes may be used.
> - */
> - if (sr_cr[1] & CR_QUAD_EN_SPAN) {
> - ret = spi_nor_read_sr(nor, sr_cr);
> - if (ret)
> - return ret;
> -
> - sr_cr[0] &= ~mask;
> -
> - return spi_nor_write_sr(nor, sr_cr, 2);
> - }
> -
> - /*
> - * If the Quad Enable bit is zero, use the Write Status (01h) command
> - * with one data byte.
> - */
> - return spi_nor_clear_sr_bp(nor);
> -}
> -
> /* Used when the "_ext_id" is two bytes at most */
> #define INFO(_jedec_id, _ext_id, _sector_size, _n_sectors, _flags) \
> .id = { \
> @@ -4634,12 +4566,27 @@ static int spi_nor_setup(struct spi_nor *nor,
> return nor->params.setup(nor, hwcaps);
> }
>
> +static void atmel_set_default_init(struct spi_nor *nor)
> +{
> + nor->flags |= SNOR_F_HAS_LOCK;
> +}
> +
> +static void intel_set_default_init(struct spi_nor *nor)
> +{
> + nor->flags |= SNOR_F_HAS_LOCK;
> +}
> +
> static void macronix_set_default_init(struct spi_nor *nor)
> {
> nor->params.quad_enable = macronix_quad_enable;
> nor->params.set_4byte = macronix_set_4byte;
> }
>
> +static void sst_set_default_init(struct spi_nor *nor)
> +{
> + nor->flags |= SNOR_F_HAS_LOCK;
> +}
> +
> static void st_micron_set_default_init(struct spi_nor *nor)
> {
> nor->flags |= SNOR_F_HAS_LOCK;
> @@ -4661,6 +4608,14 @@ static void spi_nor_manufacturer_init_params(struct spi_nor *nor)
> {
> /* Init flash parameters based on MFR */
> switch (JEDEC_MFR(nor->info)) {
> + case SNOR_MFR_ATMEL:
> + atmel_set_default_init(nor);
> + break;
> +
> + case SNOR_MFR_INTEL:
> + intel_set_default_init(nor);
> + break;
> +
> case SNOR_MFR_MACRONIX:
> macronix_set_default_init(nor);
> break;
> @@ -4670,6 +4625,10 @@ static void spi_nor_manufacturer_init_params(struct spi_nor *nor)
> st_micron_set_default_init(nor);
> break;
>
> + case SNOR_MFR_SST:
> + sst_set_default_init(nor);
> + break;
> +
> case SNOR_MFR_WINBOND:
> winbond_set_default_init(nor);
> break;
> @@ -4930,21 +4889,26 @@ static int spi_nor_quad_enable(struct spi_nor *nor)
> return nor->params.quad_enable(nor);
> }
>
> -static int spi_nor_init(struct spi_nor *nor)
> +/**
> + * spi_nor_unlock_all() - Unlocks the entire flash memory array.
> + * @nor: pointer to a 'struct spi_nor'.
> + *
> + * Some SPI NOR flashes are write protected by default after a power-on reset
> + * cycle, in order to avoid inadvertent writes during power-up. Backward
> + * compatibility imposes to unlock the entire flash memory array at power-up
> + * by default.
> + */
> +static int spi_nor_unlock_all(struct spi_nor *nor)
> {
> - int err;
> + if (nor->flags & SNOR_F_HAS_LOCK)
> + return spi_nor_unlock(&nor->mtd, 0, nor->params.size);
>
> - if (nor->clear_sr_bp) {
> - if (nor->params.quad_enable == spansion_read_cr_quad_enable)
> - nor->clear_sr_bp = spi_nor_spansion_clear_sr_bp;
> + return 0;
> +}
>
> - err = nor->clear_sr_bp(nor);
> - if (err) {
> - dev_dbg(nor->dev,
> - "fail to clear block protection bits\n");
> - return err;
> - }
> - }
> +static int spi_nor_init(struct spi_nor *nor)
> +{
> + int err;
>
> err = spi_nor_quad_enable(nor);
> if (err) {
> @@ -4952,6 +4916,12 @@ static int spi_nor_init(struct spi_nor *nor)
> return err;
> }
>
> + err = spi_nor_unlock_all(nor);
> + if (err) {
> + dev_dbg(nor->dev, "Failed to unlock the entire flash memory array\n");
> + return err;
> + }
> +
> if (nor->addr_width == 4 && !(nor->flags & SNOR_F_4B_OPCODES)) {
> /*
> * If the RESET# pin isn't hooked up properly, or the system
> @@ -5134,16 +5104,6 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
> if (info->flags & SPI_NOR_HAS_LOCK)
> nor->flags |= SNOR_F_HAS_LOCK;
>
> - /*
> - * Atmel, SST, Intel/Numonyx, and others serial NOR tend to power up
> - * with the software protection bits set.
> - */
> - if (JEDEC_MFR(nor->info) == SNOR_MFR_ATMEL ||
> - JEDEC_MFR(nor->info) == SNOR_MFR_INTEL ||
> - JEDEC_MFR(nor->info) == SNOR_MFR_SST ||
> - nor->info->flags & SPI_NOR_HAS_LOCK)
> - nor->clear_sr_bp = spi_nor_clear_sr_bp;
> -
> /* Init flash parameters based on flash_info struct and SFDP */
> spi_nor_init_params(nor);
>
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index d6ec55cc6d97..11daecc5a83d 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -581,8 +581,6 @@ struct flash_info;
> * @write_proto: the SPI protocol for write operations
> * @reg_proto the SPI protocol for read_reg/write_reg/erase operations
> * @controller_ops: SPI NOR controller driver specific operations.
> - * @clear_sr_bp: [FLASH-SPECIFIC] clears the Block Protection Bits from
> - * the SPI NOR Status Register.
> * @params: [FLASH-SPECIFIC] SPI-NOR flash parameters and settings.
> * The structure includes legacy flash parameters and
> * settings that can be overwritten by the spi_nor_fixups
> @@ -611,7 +609,6 @@ struct spi_nor {
>
> const struct spi_nor_controller_ops *controller_ops;
>
> - int (*clear_sr_bp)(struct spi_nor *nor);
> struct spi_nor_flash_parameter params;
>
> void *priv;
>
Powered by blists - more mailing lists