[PATCH 0/4] enabling Advanced protection and security features

[PATCH 0/4] enabling Advanced protection and security features

[shiva.linuxworks]

From: Shivamurthy Shastri <sshivamurthy@micron.com>

Standard protection features in SPI NOR flashes are legacy and offer a
simple way to protect the memory array against accidental or unwanted
modification of its content.

These patches enable the support for advanced sector protection which
protects memory from accidentally corrupting code and data stored, and
it also prevents malicious attacks that could intentionally modify the
code or data stored in the memory.

Micron Flashes offer some of the advanced protection methods using
volatile lock bits, non-volatile lock bits, global freeze bits, and
password.

Shivamurthy Shastri (4):
  mtd: spi-nor: micron-st: add advanced protection and security features
  mtd: spi-nor: add advanced protection and security features support
  mtd: add advanced protection and security ioctls
  mtd: spi-nor: micron-st: add mt25qu128abb and mt25ql128abb

 drivers/mtd/mtdchar.c            | 145 ++++++++++++++++++
 drivers/mtd/spi-nor/Makefile     |   2 +-
 drivers/mtd/spi-nor/advprotsec.c | 209 ++++++++++++++++++++++++++
 drivers/mtd/spi-nor/core.c       |   2 +
 drivers/mtd/spi-nor/core.h       |  20 +++
 drivers/mtd/spi-nor/micron-st.c  | 245 +++++++++++++++++++++++++++++++
 include/linux/mtd/mtd.h          |  19 +++
 include/uapi/mtd/mtd-abi.h       |  11 ++
 8 files changed, 652 insertions(+), 1 deletion(-)
 create mode 100644 drivers/mtd/spi-nor/advprotsec.c

-- 
2.25.1

[PATCH 1/4] mtd: spi-nor: micron-st: add advanced protection and security features

[shiva.linuxworks]

From: Shivamurthy Shastri <sshivamurthy@micron.com>

Micron SPI NOR flashes are enabled with advanced sector protection
features, using volatile lock bits, non-volatile lock bits, global
freeze bits and password.

Advanced sector protection and security features offers additional
levels of protection against accidentally corrupting code and data
stored, and it also prevents malicious attacks that could intentionally
modify or corrupt the code or data stored.

Signed-off-by: Shivamurthy Shastri <sshivamurthy@micron.com>
---
 drivers/mtd/spi-nor/core.h      |  20 +++
 drivers/mtd/spi-nor/micron-st.c | 238 ++++++++++++++++++++++++++++++++
 2 files changed, 258 insertions(+)

diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 3348e1dd1445..f6890973cb4a 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -187,6 +187,24 @@ struct spi_nor_locking_ops {
 	int (*is_locked)(struct spi_nor *nor, loff_t ofs, uint64_t len);
 };
 
+struct spi_nor_sec_ops {
+	int (*secure_read)(struct spi_nor *nor, size_t len, u8 *buf);
+	int (*secure_write)(struct spi_nor *nor, size_t len, u8 *buf);
+	int (*read_nvlock_bits)(struct spi_nor *nor, u32 addr, size_t len,
+				u8 *buf);
+	int (*read_vlock_bits)(struct spi_nor *nor, u32 addr, size_t len,
+			       u8 *buf);
+	int (*read_global_freeze_bits)(struct spi_nor *nor, size_t len,
+				       u8 *buf);
+	int (*read_password)(struct spi_nor *nor, size_t len, u8 *buf);
+	int (*write_global_freeze_bits)(struct spi_nor *nor, size_t len,
+					u8 *buf);
+	int (*write_vlock_bits)(struct spi_nor *nor, u32 addr, size_t len,
+				u8 *buf);
+	int (*write_nvlock_bits)(struct spi_nor *nor, u32 addr);
+	int (*erase_nvlock_bits)(struct spi_nor *nor);
+};
+
 /**
  * struct spi_nor_otp_organization - Structure to describe the SPI NOR OTP regions
  * @len:	size of one OTP region in bytes.
@@ -285,6 +303,8 @@ struct spi_nor_flash_parameter {
 	int (*setup)(struct spi_nor *nor, const struct spi_nor_hwcaps *hwcaps);
 
 	const struct spi_nor_locking_ops *locking_ops;
+
+	const struct spi_nor_sec_ops *sec_ops;
 };
 
 /**
diff --git a/drivers/mtd/spi-nor/micron-st.c b/drivers/mtd/spi-nor/micron-st.c
index c224e59820a1..b5d82e85fb92 100644
--- a/drivers/mtd/spi-nor/micron-st.c
+++ b/drivers/mtd/spi-nor/micron-st.c
@@ -16,6 +16,23 @@
 #define SPINOR_MT_OCT_DTR	0xe7	/* Enable Octal DTR. */
 #define SPINOR_MT_EXSPI		0xff	/* Enable Extended SPI (default) */
 
+#define AUTHENTA_ID		0x8c
+#define AUTHENTA_ID_BYTE	0x05
+
+#define SPINOR_OP_SECURE_READ			0x96
+#define SPINOR_OP_SECURE_WRITE			0x9b
+
+#define SPINOR_OP_RD_VOL_LOCK_BITS		0xe8
+#define SPINOR_OP_WR_VOL_LOCK_BITS		0xe5
+#define SPINOR_OP_RD_NV_LOCK_BITS		0xe2
+#define SPINOR_OP_WR_NV_LOCK_BITS		0xe3
+#define SPINOR_OP_ER_NV_LOCK_BITS		0xe4
+
+#define SPINOR_OP_RD_GLOBAL_FREEZE_BITS		0xa7
+#define SPINOR_OP_WR_GLOBAL_FREEZE_BITS		0xa6
+
+#define SPINOR_OP_RD_PASSWORD			0x27
+
 static int spi_nor_micron_octal_dtr_enable(struct spi_nor *nor, bool enable)
 {
 	struct spi_mem_op op;
@@ -247,12 +264,233 @@ static int st_micron_set_4byte_addr_mode(struct spi_nor *nor, bool enable)
 	return spi_nor_write_disable(nor);
 }
 
+/**
+ * authenta_secure_read() - read the secure packet from authenta SPI NOR
+ *
+ * @nor: pointer to 'struct spi_nor'
+ * @len: number of bytes to read
+ * @buf: pointer to dst buffer
+ *
+ * Return: 0 in case of success, a negative error code otherwise.
+ */
+static int authenta_secure_read(struct spi_nor *nor, size_t len, u8 *buf)
+{
+	struct spi_mem_op op =
+		SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_SECURE_READ, 1),
+			   SPI_MEM_OP_NO_ADDR,
+			   SPI_MEM_OP_DUMMY(1, 1),
+			   SPI_MEM_OP_DATA_IN(len, buf, 1));
+
+	return spi_mem_exec_op(nor->spimem, &op);
+}
+
+/**
+ * authenta_secure_write() - write the secure packet to authenta SPI NOR
+ *
+ * @nor: pointer to 'struct spi_nor'
+ * @len: number of bytes to be written
+ * @buf: pointer to dst buffer
+ *
+ * Return: 0 in case of success, a negative error code otherwise.
+ */
+static int authenta_secure_write(struct spi_nor *nor, size_t len, u8 *buf)
+{
+	struct spi_mem_op op =
+		SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_SECURE_WRITE, 1),
+			   SPI_MEM_OP_NO_ADDR,
+			   SPI_MEM_OP_NO_DUMMY,
+			   SPI_MEM_OP_DATA_OUT(len, buf, 1));
+
+	return spi_mem_exec_op(nor->spimem, &op);
+}
+
+/**
+ * authenta_read_vlock_bits() - read the volatile lock bits
+ *
+ * @nor: pointer to 'struct spi_nor'
+ * @addr: address for volatile lock bits
+ * @len: number of bytes to read
+ * @buf: pointer to dst buffer
+ *
+ * Return: 0 in case of success, a negative error code otherwise.
+ */
+static int authenta_read_vlock_bits(struct spi_nor *nor, u32 addr,
+				    size_t len, u8 *buf)
+{
+	struct spi_mem_op op =
+		SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_RD_VOL_LOCK_BITS, 1),
+			   SPI_MEM_OP_ADDR(nor->addr_width, addr, 1),
+			   SPI_MEM_OP_NO_DUMMY,
+			   SPI_MEM_OP_DATA_IN(len, buf, 1));
+
+	return spi_mem_exec_op(nor->spimem, &op);
+}
+
+/**
+ * authenta_write_vlock_bits() - write data to the volatile lock bits
+ *
+ * @nor: pointer to 'struct spi_nor'
+ * @addr: address for volatile lock bits
+ * @len: number of bytes to be written
+ * @buf: pointer to dst buffer
+ *
+ * Return: 0 in case of success, a negative error code otherwise.
+ */
+static int authenta_write_vlock_bits(struct spi_nor *nor, u32 addr, size_t len,
+				     u8 *buf)
+{
+	struct spi_mem_op op =
+		SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WR_VOL_LOCK_BITS, 1),
+			   SPI_MEM_OP_ADDR(nor->addr_width, addr, 1),
+			   SPI_MEM_OP_NO_DUMMY,
+			   SPI_MEM_OP_DATA_OUT(len, buf, 1));
+
+	return spi_mem_exec_op(nor->spimem, &op);
+}
+
+/**
+ * authenta_read_nvlock_bits() - read the non-volatile lock bits
+ *
+ * @nor: pointer to 'struct spi_nor'
+ * @addr: address for non-volatile lock bits
+ * @len: number of bytes to be written
+ * @buf: pointer to dst buffer
+ *
+ * Return: 0 in case of success, a negative error code otherwise.
+ */
+static int authenta_read_nvlock_bits(struct spi_nor *nor, u32 addr,
+				     size_t len, u8 *buf)
+{
+	struct spi_mem_op op =
+		SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_RD_NV_LOCK_BITS, 1),
+			   SPI_MEM_OP_ADDR(nor->addr_width, addr, 1),
+			   SPI_MEM_OP_NO_DUMMY,
+			   SPI_MEM_OP_DATA_IN(len, buf, 1));
+
+	return spi_mem_exec_op(nor->spimem, &op);
+}
+
+/**
+ * authenta_write_nvlock_bits() - write to the non-volatile lock bits
+ *
+ * @nor: pointer to 'struct spi_nor'
+ * @addr: address for non-volatile lock bits
+ *
+ * Return: 0 in case of success, a negative error code otherwise.
+ */
+static int authenta_write_nvlock_bits(struct spi_nor *nor, u32 addr)
+{
+	struct spi_mem_op op =
+		SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WR_NV_LOCK_BITS, 1),
+			   SPI_MEM_OP_ADDR(nor->addr_width, addr, 1),
+			   SPI_MEM_OP_NO_DUMMY,
+			   SPI_MEM_OP_NO_DATA);
+
+	return spi_mem_exec_op(nor->spimem, &op);
+}
+
+/**
+ * authenta_erase_nvlock_bits() - erase the non-volatile lock bits
+ *
+ * @nor: pointer to 'struct spi_nor'
+ *
+ * Return: 0 in case of success, a negative error code otherwise.
+ */
+static int authenta_erase_nvlock_bits(struct spi_nor *nor)
+{
+	struct spi_mem_op op =
+		SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_ER_NV_LOCK_BITS, 1),
+			   SPI_MEM_OP_NO_ADDR,
+			   SPI_MEM_OP_NO_DUMMY,
+			   SPI_MEM_OP_NO_DATA);
+
+	return spi_mem_exec_op(nor->spimem, &op);
+}
+
+/**
+ * authenta_read_global_freeze_bits() - read the global freeze bits
+ *
+ * @nor: pointer to 'struct spi_nor'
+ * @len: number of bytes to read
+ * @buf: pointer to dst buffer
+ *
+ * Return: 0 in case of success, a negative error code otherwise.
+ */
+static int authenta_read_global_freeze_bits(struct spi_nor *nor, size_t len,
+					    u8 *buf)
+{
+	struct spi_mem_op op =
+		SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_RD_GLOBAL_FREEZE_BITS, 1),
+			   SPI_MEM_OP_NO_ADDR,
+			   SPI_MEM_OP_NO_DUMMY,
+			   SPI_MEM_OP_DATA_IN(len, buf, 1));
+
+	return spi_mem_exec_op(nor->spimem, &op);
+}
+
+/**
+ * authenta_write_global_freeze_bits() - write data to the global freeze bits
+ *
+ * @nor: pointer to 'struct spi_nor'
+ * @len: number of bytes to be written
+ * @buf: pointer to dst buffer
+ *
+ * Return: 0 in case of success, a negative error code otherwise.
+ */
+static int authenta_write_global_freeze_bits(struct spi_nor *nor, size_t len,
+					     u8 *buf)
+{
+	struct spi_mem_op op =
+		SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WR_GLOBAL_FREEZE_BITS, 1),
+			   SPI_MEM_OP_NO_ADDR,
+			   SPI_MEM_OP_NO_DUMMY,
+			   SPI_MEM_OP_DATA_OUT(len, buf, 1));
+
+	return spi_mem_exec_op(nor->spimem, &op);
+}
+
+/**
+ * authenta_read_password() - read the password
+ *
+ * @nor: pointer to 'struct spi_nor'
+ * @len: number of bytes to read
+ * @buf: pointer to dst buffer
+ *
+ * Return: 0 in case of success, a negative error code otherwise.
+ */
+static int authenta_read_password(struct spi_nor *nor, size_t len, u8 *buf)
+{
+	struct spi_mem_op op =
+		SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_RD_PASSWORD, 1),
+			   SPI_MEM_OP_NO_ADDR,
+			   SPI_MEM_OP_NO_DUMMY,
+			   SPI_MEM_OP_DATA_IN(len, buf, 1));
+
+	return spi_mem_exec_op(nor->spimem, &op);
+}
+
+static const struct spi_nor_sec_ops authenta_ops = {
+	.secure_read = authenta_secure_read,
+	.secure_write = authenta_secure_write,
+	.read_vlock_bits = authenta_read_vlock_bits,
+	.write_vlock_bits = authenta_write_vlock_bits,
+	.read_nvlock_bits = authenta_read_nvlock_bits,
+	.write_nvlock_bits = authenta_write_nvlock_bits,
+	.erase_nvlock_bits = authenta_erase_nvlock_bits,
+	.read_global_freeze_bits = authenta_read_global_freeze_bits,
+	.write_global_freeze_bits = authenta_write_global_freeze_bits,
+	.read_password = authenta_read_password,
+};
+
 static void micron_st_default_init(struct spi_nor *nor)
 {
 	nor->flags |= SNOR_F_HAS_LOCK;
 	nor->flags &= ~SNOR_F_HAS_16BIT_SR;
 	nor->params->quad_enable = NULL;
 	nor->params->set_4byte_addr_mode = st_micron_set_4byte_addr_mode;
+
+	if (nor->info->id[AUTHENTA_ID_BYTE] == AUTHENTA_ID)
+		nor->params->sec_ops = &authenta_ops;
 }
 
 static const struct spi_nor_fixups micron_st_fixups = {
-- 
2.25.1

Re: [PATCH 1/4] mtd: spi-nor: micron-st: add advanced protection and security features

[Michael Walle]

Hi,

Am 2021-10-27 12:33, schrieb shiva.linuxworks@gmail.com:
> From: Shivamurthy Shastri <sshivamurthy@micron.com>
> 
> Micron SPI NOR flashes are enabled with advanced sector protection
> features, using volatile lock bits, non-volatile lock bits, global
> freeze bits and password.
> 
> Advanced sector protection and security features offers additional
> levels of protection against accidentally corrupting code and data
> stored, and it also prevents malicious attacks that could intentionally
> modify or corrupt the code or data stored.
> 
> Signed-off-by: Shivamurthy Shastri <sshivamurthy@micron.com>
> ---
..

> +static const struct spi_nor_sec_ops authenta_ops = {
> +	.secure_read = authenta_secure_read,
> +	.secure_write = authenta_secure_write,
> +	.read_vlock_bits = authenta_read_vlock_bits,
> +	.write_vlock_bits = authenta_write_vlock_bits,
> +	.read_nvlock_bits = authenta_read_nvlock_bits,
> +	.write_nvlock_bits = authenta_write_nvlock_bits,
> +	.erase_nvlock_bits = authenta_erase_nvlock_bits,
> +	.read_global_freeze_bits = authenta_read_global_freeze_bits,
> +	.write_global_freeze_bits = authenta_write_global_freeze_bits,
> +	.read_password = authenta_read_password,

Could you please explain what secure read and write and all the
other ops actually are? Why is there no write password?

They all seem to be used together with advanced sector protection.
But you're just exporting all these ops to userspace. We already
have lock and unlock ioctls in place. I'd expect that this
sector protection will make use of these. Just exporting all the
commands to userspace is not the way to go.

-michael

Re: [PATCH 1/4] mtd: spi-nor: micron-st: add advanced protection and security features

[Paul Barker]

[-- Attachment #1.1.1: Type: text/plain, Size: 11380 bytes --]

On 27/10/2021 11:33, shiva.linuxworks@gmail.com wrote:
> From: Shivamurthy Shastri <sshivamurthy@micron.com>
> 
> Micron SPI NOR flashes are enabled with advanced sector protection
> features, using volatile lock bits, non-volatile lock bits, global
> freeze bits and password.
> 
> Advanced sector protection and security features offers additional
> levels of protection against accidentally corrupting code and data
> stored, and it also prevents malicious attacks that could intentionally
> modify or corrupt the code or data stored.
> 
> Signed-off-by: Shivamurthy Shastri <sshivamurthy@micron.com>
> ---
>   drivers/mtd/spi-nor/core.h      |  20 +++
>   drivers/mtd/spi-nor/micron-st.c | 238 ++++++++++++++++++++++++++++++++
>   2 files changed, 258 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> index 3348e1dd1445..f6890973cb4a 100644
> --- a/drivers/mtd/spi-nor/core.h
> +++ b/drivers/mtd/spi-nor/core.h
> @@ -187,6 +187,24 @@ struct spi_nor_locking_ops {
>   	int (*is_locked)(struct spi_nor *nor, loff_t ofs, uint64_t len);
>   };
>   
> +struct spi_nor_sec_ops {
> +	int (*secure_read)(struct spi_nor *nor, size_t len, u8 *buf);
> +	int (*secure_write)(struct spi_nor *nor, size_t len, u8 *buf);
> +	int (*read_nvlock_bits)(struct spi_nor *nor, u32 addr, size_t len,
> +				u8 *buf);
> +	int (*read_vlock_bits)(struct spi_nor *nor, u32 addr, size_t len,
> +			       u8 *buf);
> +	int (*read_global_freeze_bits)(struct spi_nor *nor, size_t len,
> +				       u8 *buf);
> +	int (*read_password)(struct spi_nor *nor, size_t len, u8 *buf);
> +	int (*write_global_freeze_bits)(struct spi_nor *nor, size_t len,
> +					u8 *buf);
> +	int (*write_vlock_bits)(struct spi_nor *nor, u32 addr, size_t len,
> +				u8 *buf);
> +	int (*write_nvlock_bits)(struct spi_nor *nor, u32 addr);
> +	int (*erase_nvlock_bits)(struct spi_nor *nor);
> +};
> +
>   /**
>    * struct spi_nor_otp_organization - Structure to describe the SPI NOR OTP regions
>    * @len:	size of one OTP region in bytes.
> @@ -285,6 +303,8 @@ struct spi_nor_flash_parameter {
>   	int (*setup)(struct spi_nor *nor, const struct spi_nor_hwcaps *hwcaps);
>   
>   	const struct spi_nor_locking_ops *locking_ops;
> +
> +	const struct spi_nor_sec_ops *sec_ops;
>   };

The changes to the common spi-nor structs used by multiple drivers 
should be broken out into a separate patch from the changes to the 
micron-st driver.

>   
>   /**
> diff --git a/drivers/mtd/spi-nor/micron-st.c b/drivers/mtd/spi-nor/micron-st.c
> index c224e59820a1..b5d82e85fb92 100644
> --- a/drivers/mtd/spi-nor/micron-st.c
> +++ b/drivers/mtd/spi-nor/micron-st.c
> @@ -16,6 +16,23 @@
>   #define SPINOR_MT_OCT_DTR	0xe7	/* Enable Octal DTR. */
>   #define SPINOR_MT_EXSPI		0xff	/* Enable Extended SPI (default) */
>   
> +#define AUTHENTA_ID		0x8c
> +#define AUTHENTA_ID_BYTE	0x05
> +
> +#define SPINOR_OP_SECURE_READ			0x96
> +#define SPINOR_OP_SECURE_WRITE			0x9b
> +
> +#define SPINOR_OP_RD_VOL_LOCK_BITS		0xe8
> +#define SPINOR_OP_WR_VOL_LOCK_BITS		0xe5
> +#define SPINOR_OP_RD_NV_LOCK_BITS		0xe2
> +#define SPINOR_OP_WR_NV_LOCK_BITS		0xe3
> +#define SPINOR_OP_ER_NV_LOCK_BITS		0xe4
> +
> +#define SPINOR_OP_RD_GLOBAL_FREEZE_BITS		0xa7
> +#define SPINOR_OP_WR_GLOBAL_FREEZE_BITS		0xa6
> +
> +#define SPINOR_OP_RD_PASSWORD			0x27
> +
>   static int spi_nor_micron_octal_dtr_enable(struct spi_nor *nor, bool enable)
>   {
>   	struct spi_mem_op op;
> @@ -247,12 +264,233 @@ static int st_micron_set_4byte_addr_mode(struct spi_nor *nor, bool enable)
>   	return spi_nor_write_disable(nor);
>   }
>   
> +/**
> + * authenta_secure_read() - read the secure packet from authenta SPI NOR
> + *
> + * @nor: pointer to 'struct spi_nor'
> + * @len: number of bytes to read
> + * @buf: pointer to dst buffer
> + *
> + * Return: 0 in case of success, a negative error code otherwise.
> + */
> +static int authenta_secure_read(struct spi_nor *nor, size_t len, u8 *buf)
> +{
> +	struct spi_mem_op op =
> +		SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_SECURE_READ, 1),
> +			   SPI_MEM_OP_NO_ADDR,
> +			   SPI_MEM_OP_DUMMY(1, 1),
> +			   SPI_MEM_OP_DATA_IN(len, buf, 1));
> +
> +	return spi_mem_exec_op(nor->spimem, &op);
> +}
> +
> +/**
> + * authenta_secure_write() - write the secure packet to authenta SPI NOR
> + *
> + * @nor: pointer to 'struct spi_nor'
> + * @len: number of bytes to be written
> + * @buf: pointer to dst buffer
> + *
> + * Return: 0 in case of success, a negative error code otherwise.
> + */
> +static int authenta_secure_write(struct spi_nor *nor, size_t len, u8 *buf)
> +{
> +	struct spi_mem_op op =
> +		SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_SECURE_WRITE, 1),
> +			   SPI_MEM_OP_NO_ADDR,
> +			   SPI_MEM_OP_NO_DUMMY,
> +			   SPI_MEM_OP_DATA_OUT(len, buf, 1));
> +
> +	return spi_mem_exec_op(nor->spimem, &op);
> +}
> +
> +/**
> + * authenta_read_vlock_bits() - read the volatile lock bits
> + *
> + * @nor: pointer to 'struct spi_nor'
> + * @addr: address for volatile lock bits
> + * @len: number of bytes to read
> + * @buf: pointer to dst buffer
> + *
> + * Return: 0 in case of success, a negative error code otherwise.
> + */
> +static int authenta_read_vlock_bits(struct spi_nor *nor, u32 addr,
> +				    size_t len, u8 *buf)
> +{
> +	struct spi_mem_op op =
> +		SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_RD_VOL_LOCK_BITS, 1),
> +			   SPI_MEM_OP_ADDR(nor->addr_width, addr, 1),
> +			   SPI_MEM_OP_NO_DUMMY,
> +			   SPI_MEM_OP_DATA_IN(len, buf, 1));
> +
> +	return spi_mem_exec_op(nor->spimem, &op);
> +}
> +
> +/**
> + * authenta_write_vlock_bits() - write data to the volatile lock bits
> + *
> + * @nor: pointer to 'struct spi_nor'
> + * @addr: address for volatile lock bits
> + * @len: number of bytes to be written
> + * @buf: pointer to dst buffer
> + *
> + * Return: 0 in case of success, a negative error code otherwise.
> + */
> +static int authenta_write_vlock_bits(struct spi_nor *nor, u32 addr, size_t len,
> +				     u8 *buf)
> +{
> +	struct spi_mem_op op =
> +		SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WR_VOL_LOCK_BITS, 1),
> +			   SPI_MEM_OP_ADDR(nor->addr_width, addr, 1),
> +			   SPI_MEM_OP_NO_DUMMY,
> +			   SPI_MEM_OP_DATA_OUT(len, buf, 1));
> +
> +	return spi_mem_exec_op(nor->spimem, &op);
> +}
> +
> +/**
> + * authenta_read_nvlock_bits() - read the non-volatile lock bits
> + *
> + * @nor: pointer to 'struct spi_nor'
> + * @addr: address for non-volatile lock bits
> + * @len: number of bytes to be written
> + * @buf: pointer to dst buffer
> + *
> + * Return: 0 in case of success, a negative error code otherwise.
> + */
> +static int authenta_read_nvlock_bits(struct spi_nor *nor, u32 addr,
> +				     size_t len, u8 *buf)
> +{
> +	struct spi_mem_op op =
> +		SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_RD_NV_LOCK_BITS, 1),
> +			   SPI_MEM_OP_ADDR(nor->addr_width, addr, 1),
> +			   SPI_MEM_OP_NO_DUMMY,
> +			   SPI_MEM_OP_DATA_IN(len, buf, 1));
> +
> +	return spi_mem_exec_op(nor->spimem, &op);
> +}
> +
> +/**
> + * authenta_write_nvlock_bits() - write to the non-volatile lock bits
> + *
> + * @nor: pointer to 'struct spi_nor'
> + * @addr: address for non-volatile lock bits
> + *
> + * Return: 0 in case of success, a negative error code otherwise.
> + */
> +static int authenta_write_nvlock_bits(struct spi_nor *nor, u32 addr)
> +{
> +	struct spi_mem_op op =
> +		SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WR_NV_LOCK_BITS, 1),
> +			   SPI_MEM_OP_ADDR(nor->addr_width, addr, 1),
> +			   SPI_MEM_OP_NO_DUMMY,
> +			   SPI_MEM_OP_NO_DATA);
> +
> +	return spi_mem_exec_op(nor->spimem, &op);
> +}
> +
> +/**
> + * authenta_erase_nvlock_bits() - erase the non-volatile lock bits
> + *
> + * @nor: pointer to 'struct spi_nor'
> + *
> + * Return: 0 in case of success, a negative error code otherwise.
> + */
> +static int authenta_erase_nvlock_bits(struct spi_nor *nor)
> +{
> +	struct spi_mem_op op =
> +		SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_ER_NV_LOCK_BITS, 1),
> +			   SPI_MEM_OP_NO_ADDR,
> +			   SPI_MEM_OP_NO_DUMMY,
> +			   SPI_MEM_OP_NO_DATA);
> +
> +	return spi_mem_exec_op(nor->spimem, &op);
> +}
> +
> +/**
> + * authenta_read_global_freeze_bits() - read the global freeze bits
> + *
> + * @nor: pointer to 'struct spi_nor'
> + * @len: number of bytes to read
> + * @buf: pointer to dst buffer
> + *
> + * Return: 0 in case of success, a negative error code otherwise.
> + */
> +static int authenta_read_global_freeze_bits(struct spi_nor *nor, size_t len,
> +					    u8 *buf)
> +{
> +	struct spi_mem_op op =
> +		SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_RD_GLOBAL_FREEZE_BITS, 1),
> +			   SPI_MEM_OP_NO_ADDR,
> +			   SPI_MEM_OP_NO_DUMMY,
> +			   SPI_MEM_OP_DATA_IN(len, buf, 1));
> +
> +	return spi_mem_exec_op(nor->spimem, &op);
> +}
> +
> +/**
> + * authenta_write_global_freeze_bits() - write data to the global freeze bits
> + *
> + * @nor: pointer to 'struct spi_nor'
> + * @len: number of bytes to be written
> + * @buf: pointer to dst buffer
> + *
> + * Return: 0 in case of success, a negative error code otherwise.
> + */
> +static int authenta_write_global_freeze_bits(struct spi_nor *nor, size_t len,
> +					     u8 *buf)
> +{
> +	struct spi_mem_op op =
> +		SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WR_GLOBAL_FREEZE_BITS, 1),
> +			   SPI_MEM_OP_NO_ADDR,
> +			   SPI_MEM_OP_NO_DUMMY,
> +			   SPI_MEM_OP_DATA_OUT(len, buf, 1));
> +
> +	return spi_mem_exec_op(nor->spimem, &op);
> +}
> +
> +/**
> + * authenta_read_password() - read the password
> + *
> + * @nor: pointer to 'struct spi_nor'
> + * @len: number of bytes to read
> + * @buf: pointer to dst buffer
> + *
> + * Return: 0 in case of success, a negative error code otherwise.
> + */
> +static int authenta_read_password(struct spi_nor *nor, size_t len, u8 *buf)
> +{
> +	struct spi_mem_op op =
> +		SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_RD_PASSWORD, 1),
> +			   SPI_MEM_OP_NO_ADDR,
> +			   SPI_MEM_OP_NO_DUMMY,
> +			   SPI_MEM_OP_DATA_IN(len, buf, 1));
> +
> +	return spi_mem_exec_op(nor->spimem, &op);
> +}
> +
> +static const struct spi_nor_sec_ops authenta_ops = {
> +	.secure_read = authenta_secure_read,
> +	.secure_write = authenta_secure_write,
> +	.read_vlock_bits = authenta_read_vlock_bits,
> +	.write_vlock_bits = authenta_write_vlock_bits,
> +	.read_nvlock_bits = authenta_read_nvlock_bits,
> +	.write_nvlock_bits = authenta_write_nvlock_bits,
> +	.erase_nvlock_bits = authenta_erase_nvlock_bits,
> +	.read_global_freeze_bits = authenta_read_global_freeze_bits,
> +	.write_global_freeze_bits = authenta_write_global_freeze_bits,
> +	.read_password = authenta_read_password,
> +};
> +
>   static void micron_st_default_init(struct spi_nor *nor)
>   {
>   	nor->flags |= SNOR_F_HAS_LOCK;
>   	nor->flags &= ~SNOR_F_HAS_16BIT_SR;
>   	nor->params->quad_enable = NULL;
>   	nor->params->set_4byte_addr_mode = st_micron_set_4byte_addr_mode;
> +
> +	if (nor->info->id[AUTHENTA_ID_BYTE] == AUTHENTA_ID)
> +		nor->params->sec_ops = &authenta_ops;
>   }
>   
>   static const struct spi_nor_fixups micron_st_fixups = {
> 

Thanks,

-- 
Paul Barker
Principal Software Engineer
SanCloud Ltd

e: paul.barker@sancloud.com
w: https://sancloud.co.uk/

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 7643 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

[PATCH 2/4] mtd: spi-nor: add advanced protection and security features support

[shiva.linuxworks]

From: Shivamurthy Shastri <sshivamurthy@micron.com>

Added functionalities to support advanced securtiy and protection
features in new SPI NOR flashes.

Signed-off-by: Shivamurthy Shastri <sshivamurthy@micron.com>
---
 drivers/mtd/spi-nor/Makefile     |   2 +-
 drivers/mtd/spi-nor/advprotsec.c | 209 +++++++++++++++++++++++++++++++
 drivers/mtd/spi-nor/core.c       |   2 +
 include/linux/mtd/mtd.h          |  19 +++
 4 files changed, 231 insertions(+), 1 deletion(-)
 create mode 100644 drivers/mtd/spi-nor/advprotsec.c

diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile
index 6b904e439372..8e96e2c65c7a 100644
--- a/drivers/mtd/spi-nor/Makefile
+++ b/drivers/mtd/spi-nor/Makefile
@@ -1,6 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
 
-spi-nor-objs			:= core.o sfdp.o swp.o otp.o sysfs.o
+spi-nor-objs			:= core.o sfdp.o swp.o otp.o advprotsec.o sysfs.o
 spi-nor-objs			+= atmel.o
 spi-nor-objs			+= catalyst.o
 spi-nor-objs			+= eon.o
diff --git a/drivers/mtd/spi-nor/advprotsec.c b/drivers/mtd/spi-nor/advprotsec.c
new file mode 100644
index 000000000000..4dc8e67b16ef
--- /dev/null
+++ b/drivers/mtd/spi-nor/advprotsec.c
@@ -0,0 +1,209 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * SPI NOR Advanced Sector Protection and Security Features
+ *
+ * Copyright (C) 2021 Micron Technology, Inc.
+ */
+
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/spi-nor.h>
+
+#include "core.h"
+
+static int spi_nor_secure_read(struct mtd_info *mtd, size_t len, u8 *buf)
+{
+	struct spi_nor *nor = mtd_to_spi_nor(mtd);
+	int ret;
+
+	ret = spi_nor_lock_and_prep(nor);
+	if (ret)
+		return ret;
+
+	ret = nor->params->sec_ops->secure_read(nor, len, buf);
+
+	spi_nor_unlock_and_unprep(nor);
+	return ret;
+}
+
+static int spi_nor_secure_write(struct mtd_info *mtd, size_t len, u8 *buf)
+{
+	struct spi_nor *nor = mtd_to_spi_nor(mtd);
+	int ret;
+
+	ret = spi_nor_lock_and_prep(nor);
+	if (ret)
+		return ret;
+
+	ret = nor->params->sec_ops->secure_write(nor, len, buf);
+
+	spi_nor_unlock_and_unprep(nor);
+	return ret;
+}
+
+static int spi_nor_read_vlock_bits(struct mtd_info *mtd, u32 addr, size_t len,
+				   u8 *buf)
+{
+	struct spi_nor *nor = mtd_to_spi_nor(mtd);
+	int ret;
+
+	ret = spi_nor_lock_and_prep(nor);
+	if (ret)
+		return ret;
+
+	ret = nor->params->sec_ops->read_vlock_bits(nor, addr, len, buf);
+
+	spi_nor_unlock_and_unprep(nor);
+	return ret;
+}
+
+static int spi_nor_write_vlock_bits(struct mtd_info *mtd, u32 addr, size_t len,
+				    u8 *buf)
+{
+	struct spi_nor *nor = mtd_to_spi_nor(mtd);
+	int ret;
+
+	ret = spi_nor_lock_and_prep(nor);
+	if (ret)
+		return ret;
+
+	ret = spi_nor_write_enable(nor);
+	if (ret)
+		return ret;
+
+	ret = nor->params->sec_ops->write_vlock_bits(nor, addr, len, buf);
+	if (ret)
+		return ret;
+
+	ret = spi_nor_write_disable(nor);
+
+	spi_nor_unlock_and_unprep(nor);
+	return ret;
+}
+
+static int spi_nor_read_nvlock_bits(struct mtd_info *mtd, u32 addr, size_t len,
+				    u8 *buf)
+{
+	struct spi_nor *nor = mtd_to_spi_nor(mtd);
+	int ret;
+
+	ret = spi_nor_lock_and_prep(nor);
+	if (ret)
+		return ret;
+
+	ret = nor->params->sec_ops->read_nvlock_bits(nor, addr, len, buf);
+
+	spi_nor_unlock_and_unprep(nor);
+	return ret;
+}
+
+static int spi_nor_write_nvlock_bits(struct mtd_info *mtd, u32 addr)
+{
+	struct spi_nor *nor = mtd_to_spi_nor(mtd);
+	int ret;
+
+	ret = spi_nor_lock_and_prep(nor);
+	if (ret)
+		return ret;
+
+	ret = spi_nor_write_enable(nor);
+	if (ret)
+		return ret;
+
+	ret = nor->params->sec_ops->write_nvlock_bits(nor, addr);
+	if (ret)
+		return ret;
+
+	ret = spi_nor_write_disable(nor);
+
+	spi_nor_unlock_and_unprep(nor);
+	return ret;
+}
+
+static int spi_nor_erase_nvlock_bits(struct mtd_info *mtd)
+{
+	struct spi_nor *nor = mtd_to_spi_nor(mtd);
+	int ret;
+
+	ret = spi_nor_lock_and_prep(nor);
+	if (ret)
+		return ret;
+
+	ret = spi_nor_write_enable(nor);
+	if (ret)
+		return ret;
+
+	ret = nor->params->sec_ops->erase_nvlock_bits(nor);
+	if (ret)
+		return ret;
+
+	ret = spi_nor_write_disable(nor);
+
+	spi_nor_unlock_and_unprep(nor);
+	return ret;
+}
+
+static int spi_nor_read_global_freeze_bits(struct mtd_info *mtd, size_t len,
+					   u8 *buf)
+{
+	struct spi_nor *nor = mtd_to_spi_nor(mtd);
+	int ret;
+
+	ret = spi_nor_lock_and_prep(nor);
+	if (ret)
+		return ret;
+
+	ret = nor->params->sec_ops->read_global_freeze_bits(nor, len, buf);
+
+	spi_nor_unlock_and_unprep(nor);
+	return ret;
+}
+
+static int spi_nor_write_global_freeze_bits(struct mtd_info *mtd, size_t len,
+					    u8 *buf)
+{
+	struct spi_nor *nor = mtd_to_spi_nor(mtd);
+	int ret;
+
+	ret = spi_nor_lock_and_prep(nor);
+	if (ret)
+		return ret;
+
+	ret = nor->params->sec_ops->write_global_freeze_bits(nor, len, buf);
+
+	spi_nor_unlock_and_unprep(nor);
+	return ret;
+}
+
+static int spi_nor_read_password(struct mtd_info *mtd, size_t len, u8 *buf)
+{
+	struct spi_nor *nor = mtd_to_spi_nor(mtd);
+	int ret;
+
+	ret = spi_nor_lock_and_prep(nor);
+	if (ret)
+		return ret;
+
+	ret = nor->params->sec_ops->read_password(nor, len, buf);
+
+	spi_nor_unlock_and_unprep(nor);
+	return ret;
+}
+
+void spi_nor_register_security_ops(struct spi_nor *nor)
+{
+	struct mtd_info *mtd = &nor->mtd;
+
+	if (!nor->params->sec_ops)
+		return;
+
+	mtd->_secure_packet_read = spi_nor_secure_read;
+	mtd->_secure_packet_write = spi_nor_secure_write;
+	mtd->_read_vlock_bits = spi_nor_read_vlock_bits;
+	mtd->_write_vlock_bits = spi_nor_write_vlock_bits;
+	mtd->_read_nvlock_bits = spi_nor_read_nvlock_bits;
+	mtd->_write_nvlock_bits = spi_nor_write_nvlock_bits;
+	mtd->_erase_nvlock_bits = spi_nor_erase_nvlock_bits;
+	mtd->_read_global_freeze_bits = spi_nor_read_global_freeze_bits;
+	mtd->_write_global_freeze_bits = spi_nor_write_global_freeze_bits;
+	mtd->_read_password = spi_nor_read_password;
+}
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index cc08bd707378..864f3c7783b3 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -3199,6 +3199,8 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
 
 	spi_nor_register_locking_ops(nor);
 
+	spi_nor_register_security_ops(nor);
+
 	/* Send all the required SPI flash commands to initialize device */
 	ret = spi_nor_init(nor);
 	if (ret)
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index 88227044fc86..bce358c9fb94 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -360,6 +360,25 @@ struct mtd_info {
 	int (*_get_device) (struct mtd_info *mtd);
 	void (*_put_device) (struct mtd_info *mtd);
 
+	/*
+	 * Security Operations
+	 */
+	int (*_secure_packet_read)(struct mtd_info *mtd, size_t len, u8 *buf);
+	int (*_secure_packet_write)(struct mtd_info *mtd, size_t len, u8 *buf);
+	int (*_read_vlock_bits)(struct mtd_info *mtd, u32 addr, size_t len,
+				u8 *buf);
+	int (*_write_vlock_bits)(struct mtd_info *mtd, u32 addr, size_t len,
+				 u8 *buf);
+	int (*_read_nvlock_bits)(struct mtd_info *mtd, u32 addr, size_t len,
+				 u8 *buf);
+	int (*_write_nvlock_bits)(struct mtd_info *mtd, u32 addr);
+	int (*_erase_nvlock_bits)(struct mtd_info *mtd);
+	int (*_read_global_freeze_bits)(struct mtd_info *mtd, size_t len,
+					u8 *buf);
+	int (*_write_global_freeze_bits)(struct mtd_info *mtd, size_t len,
+					 u8 *buf);
+	int (*_read_password)(struct mtd_info *mtd, size_t len, u8 *buf);
+
 	/*
 	 * flag indicates a panic write, low level drivers can take appropriate
 	 * action if required to ensure writes go through
-- 
2.25.1

Re: [PATCH 2/4] mtd: spi-nor: add advanced protection and security features support

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 2036 bytes --]

Hi,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on mtd/mtd/next]
[also build test WARNING on mtd/mtd/fixes v5.15-rc7 next-20211027]
[cannot apply to mtd/spi-nor/next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/shiva-linuxworks-gmail-com/enabling-Advanced-protection-and-security-features/20211027-183458
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/next
config: arc-randconfig-r043-20211027 (attached as .config)
compiler: arc-elf-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/d26eac1611c4409954b4d0c44215e1a53aa7605a
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review shiva-linuxworks-gmail-com/enabling-Advanced-protection-and-security-features/20211027-183458
        git checkout d26eac1611c4409954b4d0c44215e1a53aa7605a
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross ARCH=arc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/mtd/spi-nor/advprotsec.c:192:6: warning: no previous prototype for 'spi_nor_register_security_ops' [-Wmissing-prototypes]
     192 | void spi_nor_register_security_ops(struct spi_nor *nor)
         |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~


vim +/spi_nor_register_security_ops +192 drivers/mtd/spi-nor/advprotsec.c

   191	
 > 192	void spi_nor_register_security_ops(struct spi_nor *nor)

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 27867 bytes --]

Re: [PATCH 2/4] mtd: spi-nor: add advanced protection and security features support

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 8265 bytes --]

Hi,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on mtd/mtd/next]
[also build test ERROR on mtd/mtd/fixes v5.15-rc7 next-20211027]
[cannot apply to mtd/spi-nor/next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/shiva-linuxworks-gmail-com/enabling-Advanced-protection-and-security-features/20211027-183458
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/next
config: i386-randconfig-a002-20211027 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 5db7568a6a1fcb408eb8988abdaff2a225a8eb72)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/d26eac1611c4409954b4d0c44215e1a53aa7605a
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review shiva-linuxworks-gmail-com/enabling-Advanced-protection-and-security-features/20211027-183458
        git checkout d26eac1611c4409954b4d0c44215e1a53aa7605a
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All error/warnings (new ones prefixed by >>):

>> drivers/mtd/spi-nor/core.c:3202:2: error: implicit declaration of function 'spi_nor_register_security_ops' [-Werror,-Wimplicit-function-declaration]
           spi_nor_register_security_ops(nor);
           ^
   drivers/mtd/spi-nor/core.c:3202:2: note: did you mean 'spi_nor_register_locking_ops'?
   drivers/mtd/spi-nor/core.h:572:6: note: 'spi_nor_register_locking_ops' declared here
   void spi_nor_register_locking_ops(struct spi_nor *nor);
        ^
   1 error generated.
--
>> drivers/mtd/spi-nor/advprotsec.c:192:6: warning: no previous prototype for function 'spi_nor_register_security_ops' [-Wmissing-prototypes]
   void spi_nor_register_security_ops(struct spi_nor *nor)
        ^
   drivers/mtd/spi-nor/advprotsec.c:192:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   void spi_nor_register_security_ops(struct spi_nor *nor)
   ^
   static 
   1 warning generated.


vim +/spi_nor_register_security_ops +3202 drivers/mtd/spi-nor/core.c

  3073	
  3074	int spi_nor_scan(struct spi_nor *nor, const char *name,
  3075			 const struct spi_nor_hwcaps *hwcaps)
  3076	{
  3077		const struct flash_info *info;
  3078		struct device *dev = nor->dev;
  3079		struct mtd_info *mtd = &nor->mtd;
  3080		struct device_node *np = spi_nor_get_flash_node(nor);
  3081		int ret;
  3082		int i;
  3083	
  3084		ret = spi_nor_check(nor);
  3085		if (ret)
  3086			return ret;
  3087	
  3088		/* Reset SPI protocol for all commands. */
  3089		nor->reg_proto = SNOR_PROTO_1_1_1;
  3090		nor->read_proto = SNOR_PROTO_1_1_1;
  3091		nor->write_proto = SNOR_PROTO_1_1_1;
  3092	
  3093		/*
  3094		 * We need the bounce buffer early to read/write registers when going
  3095		 * through the spi-mem layer (buffers have to be DMA-able).
  3096		 * For spi-mem drivers, we'll reallocate a new buffer if
  3097		 * nor->page_size turns out to be greater than PAGE_SIZE (which
  3098		 * shouldn't happen before long since NOR pages are usually less
  3099		 * than 1KB) after spi_nor_scan() returns.
  3100		 */
  3101		nor->bouncebuf_size = PAGE_SIZE;
  3102		nor->bouncebuf = devm_kmalloc(dev, nor->bouncebuf_size,
  3103					      GFP_KERNEL);
  3104		if (!nor->bouncebuf)
  3105			return -ENOMEM;
  3106	
  3107		info = spi_nor_get_flash_info(nor, name);
  3108		if (IS_ERR(info))
  3109			return PTR_ERR(info);
  3110	
  3111		nor->info = info;
  3112	
  3113		spi_nor_debugfs_init(nor, info);
  3114	
  3115		mutex_init(&nor->lock);
  3116	
  3117		/*
  3118		 * Make sure the XSR_RDY flag is set before calling
  3119		 * spi_nor_wait_till_ready(). Xilinx S3AN share MFR
  3120		 * with Atmel SPI NOR.
  3121		 */
  3122		if (info->flags & SPI_NOR_XSR_RDY)
  3123			nor->flags |=  SNOR_F_READY_XSR_RDY;
  3124	
  3125		if (info->flags & SPI_NOR_HAS_LOCK)
  3126			nor->flags |= SNOR_F_HAS_LOCK;
  3127	
  3128		mtd->_write = spi_nor_write;
  3129	
  3130		/* Init flash parameters based on flash_info struct and SFDP */
  3131		ret = spi_nor_init_params(nor);
  3132		if (ret)
  3133			return ret;
  3134	
  3135		if (!mtd->name)
  3136			mtd->name = dev_name(dev);
  3137		mtd->priv = nor;
  3138		mtd->type = MTD_NORFLASH;
  3139		mtd->writesize = nor->params->writesize;
  3140		mtd->flags = MTD_CAP_NORFLASH;
  3141		mtd->size = nor->params->size;
  3142		mtd->_erase = spi_nor_erase;
  3143		mtd->_read = spi_nor_read;
  3144		mtd->_suspend = spi_nor_suspend;
  3145		mtd->_resume = spi_nor_resume;
  3146		mtd->_get_device = spi_nor_get_device;
  3147		mtd->_put_device = spi_nor_put_device;
  3148	
  3149		if (info->flags & USE_FSR)
  3150			nor->flags |= SNOR_F_USE_FSR;
  3151		if (info->flags & SPI_NOR_HAS_TB) {
  3152			nor->flags |= SNOR_F_HAS_SR_TB;
  3153			if (info->flags & SPI_NOR_TB_SR_BIT6)
  3154				nor->flags |= SNOR_F_HAS_SR_TB_BIT6;
  3155		}
  3156	
  3157		if (info->flags & NO_CHIP_ERASE)
  3158			nor->flags |= SNOR_F_NO_OP_CHIP_ERASE;
  3159		if (info->flags & USE_CLSR)
  3160			nor->flags |= SNOR_F_USE_CLSR;
  3161		if (info->flags & SPI_NOR_SWP_IS_VOLATILE)
  3162			nor->flags |= SNOR_F_SWP_IS_VOLATILE;
  3163	
  3164		if (info->flags & SPI_NOR_4BIT_BP) {
  3165			nor->flags |= SNOR_F_HAS_4BIT_BP;
  3166			if (info->flags & SPI_NOR_BP3_SR_BIT6)
  3167				nor->flags |= SNOR_F_HAS_SR_BP3_BIT6;
  3168		}
  3169	
  3170		if (info->flags & SPI_NOR_NO_ERASE)
  3171			mtd->flags |= MTD_NO_ERASE;
  3172	
  3173		mtd->dev.parent = dev;
  3174		nor->page_size = nor->params->page_size;
  3175		mtd->writebufsize = nor->page_size;
  3176	
  3177		if (of_property_read_bool(np, "broken-flash-reset"))
  3178			nor->flags |= SNOR_F_BROKEN_RESET;
  3179	
  3180		/*
  3181		 * Configure the SPI memory:
  3182		 * - select op codes for (Fast) Read, Page Program and Sector Erase.
  3183		 * - set the number of dummy cycles (mode cycles + wait states).
  3184		 * - set the SPI protocols for register and memory accesses.
  3185		 */
  3186		ret = spi_nor_setup(nor, hwcaps);
  3187		if (ret)
  3188			return ret;
  3189	
  3190		if (info->flags & SPI_NOR_4B_OPCODES)
  3191			nor->flags |= SNOR_F_4B_OPCODES;
  3192	
  3193		if (info->flags & SPI_NOR_IO_MODE_EN_VOLATILE)
  3194			nor->flags |= SNOR_F_IO_MODE_EN_VOLATILE;
  3195	
  3196		ret = spi_nor_set_addr_width(nor);
  3197		if (ret)
  3198			return ret;
  3199	
  3200		spi_nor_register_locking_ops(nor);
  3201	
> 3202		spi_nor_register_security_ops(nor);
  3203	
  3204		/* Send all the required SPI flash commands to initialize device */
  3205		ret = spi_nor_init(nor);
  3206		if (ret)
  3207			return ret;
  3208	
  3209		/* Configure OTP parameters and ops */
  3210		spi_nor_otp_init(nor);
  3211	
  3212		dev_info(dev, "%s (%lld Kbytes)\n", info->name,
  3213				(long long)mtd->size >> 10);
  3214	
  3215		dev_dbg(dev,
  3216			"mtd .name = %s, .size = 0x%llx (%lldMiB), "
  3217			".erasesize = 0x%.8x (%uKiB) .numeraseregions = %d\n",
  3218			mtd->name, (long long)mtd->size, (long long)(mtd->size >> 20),
  3219			mtd->erasesize, mtd->erasesize / 1024, mtd->numeraseregions);
  3220	
  3221		if (mtd->numeraseregions)
  3222			for (i = 0; i < mtd->numeraseregions; i++)
  3223				dev_dbg(dev,
  3224					"mtd.eraseregions[%d] = { .offset = 0x%llx, "
  3225					".erasesize = 0x%.8x (%uKiB), "
  3226					".numblocks = %d }\n",
  3227					i, (long long)mtd->eraseregions[i].offset,
  3228					mtd->eraseregions[i].erasesize,
  3229					mtd->eraseregions[i].erasesize / 1024,
  3230					mtd->eraseregions[i].numblocks);
  3231		return 0;
  3232	}
  3233	EXPORT_SYMBOL_GPL(spi_nor_scan);
  3234	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 30528 bytes --]

Re: [PATCH 2/4] mtd: spi-nor: add advanced protection and security features support

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 8271 bytes --]

Hi,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on mtd/mtd/next]
[also build test ERROR on mtd/mtd/fixes v5.15-rc7 next-20211027]
[cannot apply to mtd/spi-nor/next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/shiva-linuxworks-gmail-com/enabling-Advanced-protection-and-security-features/20211027-183458
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/next
config: i386-buildonly-randconfig-r005-20211027 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 5db7568a6a1fcb408eb8988abdaff2a225a8eb72)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/d26eac1611c4409954b4d0c44215e1a53aa7605a
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review shiva-linuxworks-gmail-com/enabling-Advanced-protection-and-security-features/20211027-183458
        git checkout d26eac1611c4409954b4d0c44215e1a53aa7605a
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/mtd/spi-nor/core.c:3202:2: error: implicit declaration of function 'spi_nor_register_security_ops' [-Werror,-Wimplicit-function-declaration]
           spi_nor_register_security_ops(nor);
           ^
   drivers/mtd/spi-nor/core.c:3202:2: note: did you mean 'spi_nor_register_locking_ops'?
   drivers/mtd/spi-nor/core.h:572:6: note: 'spi_nor_register_locking_ops' declared here
   void spi_nor_register_locking_ops(struct spi_nor *nor);
        ^
   1 error generated.
--
>> drivers/mtd/spi-nor/advprotsec.c:192:6: error: no previous prototype for function 'spi_nor_register_security_ops' [-Werror,-Wmissing-prototypes]
   void spi_nor_register_security_ops(struct spi_nor *nor)
        ^
   drivers/mtd/spi-nor/advprotsec.c:192:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   void spi_nor_register_security_ops(struct spi_nor *nor)
   ^
   static 
   1 error generated.


vim +/spi_nor_register_security_ops +3202 drivers/mtd/spi-nor/core.c

  3073	
  3074	int spi_nor_scan(struct spi_nor *nor, const char *name,
  3075			 const struct spi_nor_hwcaps *hwcaps)
  3076	{
  3077		const struct flash_info *info;
  3078		struct device *dev = nor->dev;
  3079		struct mtd_info *mtd = &nor->mtd;
  3080		struct device_node *np = spi_nor_get_flash_node(nor);
  3081		int ret;
  3082		int i;
  3083	
  3084		ret = spi_nor_check(nor);
  3085		if (ret)
  3086			return ret;
  3087	
  3088		/* Reset SPI protocol for all commands. */
  3089		nor->reg_proto = SNOR_PROTO_1_1_1;
  3090		nor->read_proto = SNOR_PROTO_1_1_1;
  3091		nor->write_proto = SNOR_PROTO_1_1_1;
  3092	
  3093		/*
  3094		 * We need the bounce buffer early to read/write registers when going
  3095		 * through the spi-mem layer (buffers have to be DMA-able).
  3096		 * For spi-mem drivers, we'll reallocate a new buffer if
  3097		 * nor->page_size turns out to be greater than PAGE_SIZE (which
  3098		 * shouldn't happen before long since NOR pages are usually less
  3099		 * than 1KB) after spi_nor_scan() returns.
  3100		 */
  3101		nor->bouncebuf_size = PAGE_SIZE;
  3102		nor->bouncebuf = devm_kmalloc(dev, nor->bouncebuf_size,
  3103					      GFP_KERNEL);
  3104		if (!nor->bouncebuf)
  3105			return -ENOMEM;
  3106	
  3107		info = spi_nor_get_flash_info(nor, name);
  3108		if (IS_ERR(info))
  3109			return PTR_ERR(info);
  3110	
  3111		nor->info = info;
  3112	
  3113		spi_nor_debugfs_init(nor, info);
  3114	
  3115		mutex_init(&nor->lock);
  3116	
  3117		/*
  3118		 * Make sure the XSR_RDY flag is set before calling
  3119		 * spi_nor_wait_till_ready(). Xilinx S3AN share MFR
  3120		 * with Atmel SPI NOR.
  3121		 */
  3122		if (info->flags & SPI_NOR_XSR_RDY)
  3123			nor->flags |=  SNOR_F_READY_XSR_RDY;
  3124	
  3125		if (info->flags & SPI_NOR_HAS_LOCK)
  3126			nor->flags |= SNOR_F_HAS_LOCK;
  3127	
  3128		mtd->_write = spi_nor_write;
  3129	
  3130		/* Init flash parameters based on flash_info struct and SFDP */
  3131		ret = spi_nor_init_params(nor);
  3132		if (ret)
  3133			return ret;
  3134	
  3135		if (!mtd->name)
  3136			mtd->name = dev_name(dev);
  3137		mtd->priv = nor;
  3138		mtd->type = MTD_NORFLASH;
  3139		mtd->writesize = nor->params->writesize;
  3140		mtd->flags = MTD_CAP_NORFLASH;
  3141		mtd->size = nor->params->size;
  3142		mtd->_erase = spi_nor_erase;
  3143		mtd->_read = spi_nor_read;
  3144		mtd->_suspend = spi_nor_suspend;
  3145		mtd->_resume = spi_nor_resume;
  3146		mtd->_get_device = spi_nor_get_device;
  3147		mtd->_put_device = spi_nor_put_device;
  3148	
  3149		if (info->flags & USE_FSR)
  3150			nor->flags |= SNOR_F_USE_FSR;
  3151		if (info->flags & SPI_NOR_HAS_TB) {
  3152			nor->flags |= SNOR_F_HAS_SR_TB;
  3153			if (info->flags & SPI_NOR_TB_SR_BIT6)
  3154				nor->flags |= SNOR_F_HAS_SR_TB_BIT6;
  3155		}
  3156	
  3157		if (info->flags & NO_CHIP_ERASE)
  3158			nor->flags |= SNOR_F_NO_OP_CHIP_ERASE;
  3159		if (info->flags & USE_CLSR)
  3160			nor->flags |= SNOR_F_USE_CLSR;
  3161		if (info->flags & SPI_NOR_SWP_IS_VOLATILE)
  3162			nor->flags |= SNOR_F_SWP_IS_VOLATILE;
  3163	
  3164		if (info->flags & SPI_NOR_4BIT_BP) {
  3165			nor->flags |= SNOR_F_HAS_4BIT_BP;
  3166			if (info->flags & SPI_NOR_BP3_SR_BIT6)
  3167				nor->flags |= SNOR_F_HAS_SR_BP3_BIT6;
  3168		}
  3169	
  3170		if (info->flags & SPI_NOR_NO_ERASE)
  3171			mtd->flags |= MTD_NO_ERASE;
  3172	
  3173		mtd->dev.parent = dev;
  3174		nor->page_size = nor->params->page_size;
  3175		mtd->writebufsize = nor->page_size;
  3176	
  3177		if (of_property_read_bool(np, "broken-flash-reset"))
  3178			nor->flags |= SNOR_F_BROKEN_RESET;
  3179	
  3180		/*
  3181		 * Configure the SPI memory:
  3182		 * - select op codes for (Fast) Read, Page Program and Sector Erase.
  3183		 * - set the number of dummy cycles (mode cycles + wait states).
  3184		 * - set the SPI protocols for register and memory accesses.
  3185		 */
  3186		ret = spi_nor_setup(nor, hwcaps);
  3187		if (ret)
  3188			return ret;
  3189	
  3190		if (info->flags & SPI_NOR_4B_OPCODES)
  3191			nor->flags |= SNOR_F_4B_OPCODES;
  3192	
  3193		if (info->flags & SPI_NOR_IO_MODE_EN_VOLATILE)
  3194			nor->flags |= SNOR_F_IO_MODE_EN_VOLATILE;
  3195	
  3196		ret = spi_nor_set_addr_width(nor);
  3197		if (ret)
  3198			return ret;
  3199	
  3200		spi_nor_register_locking_ops(nor);
  3201	
> 3202		spi_nor_register_security_ops(nor);
  3203	
  3204		/* Send all the required SPI flash commands to initialize device */
  3205		ret = spi_nor_init(nor);
  3206		if (ret)
  3207			return ret;
  3208	
  3209		/* Configure OTP parameters and ops */
  3210		spi_nor_otp_init(nor);
  3211	
  3212		dev_info(dev, "%s (%lld Kbytes)\n", info->name,
  3213				(long long)mtd->size >> 10);
  3214	
  3215		dev_dbg(dev,
  3216			"mtd .name = %s, .size = 0x%llx (%lldMiB), "
  3217			".erasesize = 0x%.8x (%uKiB) .numeraseregions = %d\n",
  3218			mtd->name, (long long)mtd->size, (long long)(mtd->size >> 20),
  3219			mtd->erasesize, mtd->erasesize / 1024, mtd->numeraseregions);
  3220	
  3221		if (mtd->numeraseregions)
  3222			for (i = 0; i < mtd->numeraseregions; i++)
  3223				dev_dbg(dev,
  3224					"mtd.eraseregions[%d] = { .offset = 0x%llx, "
  3225					".erasesize = 0x%.8x (%uKiB), "
  3226					".numblocks = %d }\n",
  3227					i, (long long)mtd->eraseregions[i].offset,
  3228					mtd->eraseregions[i].erasesize,
  3229					mtd->eraseregions[i].erasesize / 1024,
  3230					mtd->eraseregions[i].numblocks);
  3231		return 0;
  3232	}
  3233	EXPORT_SYMBOL_GPL(spi_nor_scan);
  3234	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 40094 bytes --]

Re: [PATCH 2/4] mtd: spi-nor: add advanced protection and security features support

[Paul Barker]

[-- Attachment #1.1.1: Type: text/plain, Size: 9258 bytes --]

On 27/10/2021 11:33, shiva.linuxworks@gmail.com wrote:
> From: Shivamurthy Shastri <sshivamurthy@micron.com>
> 
> Added functionalities to support advanced securtiy and protection
> features in new SPI NOR flashes.
> 
> Signed-off-by: Shivamurthy Shastri <sshivamurthy@micron.com>
> ---
>   drivers/mtd/spi-nor/Makefile     |   2 +-
>   drivers/mtd/spi-nor/advprotsec.c | 209 +++++++++++++++++++++++++++++++
>   drivers/mtd/spi-nor/core.c       |   2 +
>   include/linux/mtd/mtd.h          |  19 +++
>   4 files changed, 231 insertions(+), 1 deletion(-)
>   create mode 100644 drivers/mtd/spi-nor/advprotsec.c

The changes to drivers/mtd/spi-nor/core.h in patch 1 of this series can 
be merged into this patch, with the series re-ordered so this patch is 
first.

> 
> diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile
> index 6b904e439372..8e96e2c65c7a 100644
> --- a/drivers/mtd/spi-nor/Makefile
> +++ b/drivers/mtd/spi-nor/Makefile
> @@ -1,6 +1,6 @@
>   # SPDX-License-Identifier: GPL-2.0
>   
> -spi-nor-objs			:= core.o sfdp.o swp.o otp.o sysfs.o
> +spi-nor-objs			:= core.o sfdp.o swp.o otp.o advprotsec.o sysfs.o
>   spi-nor-objs			+= atmel.o
>   spi-nor-objs			+= catalyst.o
>   spi-nor-objs			+= eon.o
> diff --git a/drivers/mtd/spi-nor/advprotsec.c b/drivers/mtd/spi-nor/advprotsec.c
> new file mode 100644
> index 000000000000..4dc8e67b16ef
> --- /dev/null
> +++ b/drivers/mtd/spi-nor/advprotsec.c
> @@ -0,0 +1,209 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * SPI NOR Advanced Sector Protection and Security Features
> + *
> + * Copyright (C) 2021 Micron Technology, Inc.
> + */
> +
> +#include <linux/mtd/mtd.h>
> +#include <linux/mtd/spi-nor.h>
> +
> +#include "core.h"
> +
> +static int spi_nor_secure_read(struct mtd_info *mtd, size_t len, u8 *buf)
> +{
> +	struct spi_nor *nor = mtd_to_spi_nor(mtd);
> +	int ret;
> +
> +	ret = spi_nor_lock_and_prep(nor);
> +	if (ret)
> +		return ret;
> +
> +	ret = nor->params->sec_ops->secure_read(nor, len, buf);
> +
> +	spi_nor_unlock_and_unprep(nor);
> +	return ret;
> +}
> +
> +static int spi_nor_secure_write(struct mtd_info *mtd, size_t len, u8 *buf)
> +{
> +	struct spi_nor *nor = mtd_to_spi_nor(mtd);
> +	int ret;
> +
> +	ret = spi_nor_lock_and_prep(nor);
> +	if (ret)
> +		return ret;
> +
> +	ret = nor->params->sec_ops->secure_write(nor, len, buf);
> +
> +	spi_nor_unlock_and_unprep(nor);
> +	return ret;
> +}
> +
> +static int spi_nor_read_vlock_bits(struct mtd_info *mtd, u32 addr, size_t len,
> +				   u8 *buf)
> +{
> +	struct spi_nor *nor = mtd_to_spi_nor(mtd);
> +	int ret;
> +
> +	ret = spi_nor_lock_and_prep(nor);
> +	if (ret)
> +		return ret;
> +
> +	ret = nor->params->sec_ops->read_vlock_bits(nor, addr, len, buf);
> +
> +	spi_nor_unlock_and_unprep(nor);
> +	return ret;
> +}
> +
> +static int spi_nor_write_vlock_bits(struct mtd_info *mtd, u32 addr, size_t len,
> +				    u8 *buf)
> +{
> +	struct spi_nor *nor = mtd_to_spi_nor(mtd);
> +	int ret;
> +
> +	ret = spi_nor_lock_and_prep(nor);
> +	if (ret)
> +		return ret;
> +
> +	ret = spi_nor_write_enable(nor);
> +	if (ret)
> +		return ret;
> +
> +	ret = nor->params->sec_ops->write_vlock_bits(nor, addr, len, buf);
> +	if (ret)
> +		return ret;
> +
> +	ret = spi_nor_write_disable(nor);
> +
> +	spi_nor_unlock_and_unprep(nor);
> +	return ret;
> +}
> +
> +static int spi_nor_read_nvlock_bits(struct mtd_info *mtd, u32 addr, size_t len,
> +				    u8 *buf)
> +{
> +	struct spi_nor *nor = mtd_to_spi_nor(mtd);
> +	int ret;
> +
> +	ret = spi_nor_lock_and_prep(nor);
> +	if (ret)
> +		return ret;
> +
> +	ret = nor->params->sec_ops->read_nvlock_bits(nor, addr, len, buf);
> +
> +	spi_nor_unlock_and_unprep(nor);
> +	return ret;
> +}
> +
> +static int spi_nor_write_nvlock_bits(struct mtd_info *mtd, u32 addr)
> +{
> +	struct spi_nor *nor = mtd_to_spi_nor(mtd);
> +	int ret;
> +
> +	ret = spi_nor_lock_and_prep(nor);
> +	if (ret)
> +		return ret;
> +
> +	ret = spi_nor_write_enable(nor);
> +	if (ret)
> +		return ret;
> +
> +	ret = nor->params->sec_ops->write_nvlock_bits(nor, addr);
> +	if (ret)
> +		return ret;
> +
> +	ret = spi_nor_write_disable(nor);
> +
> +	spi_nor_unlock_and_unprep(nor);
> +	return ret;
> +}
> +
> +static int spi_nor_erase_nvlock_bits(struct mtd_info *mtd)
> +{
> +	struct spi_nor *nor = mtd_to_spi_nor(mtd);
> +	int ret;
> +
> +	ret = spi_nor_lock_and_prep(nor);
> +	if (ret)
> +		return ret;
> +
> +	ret = spi_nor_write_enable(nor);
> +	if (ret)
> +		return ret;
> +
> +	ret = nor->params->sec_ops->erase_nvlock_bits(nor);
> +	if (ret)
> +		return ret;
> +
> +	ret = spi_nor_write_disable(nor);
> +
> +	spi_nor_unlock_and_unprep(nor);
> +	return ret;
> +}
> +
> +static int spi_nor_read_global_freeze_bits(struct mtd_info *mtd, size_t len,
> +					   u8 *buf)
> +{
> +	struct spi_nor *nor = mtd_to_spi_nor(mtd);
> +	int ret;
> +
> +	ret = spi_nor_lock_and_prep(nor);
> +	if (ret)
> +		return ret;
> +
> +	ret = nor->params->sec_ops->read_global_freeze_bits(nor, len, buf);
> +
> +	spi_nor_unlock_and_unprep(nor);
> +	return ret;
> +}
> +
> +static int spi_nor_write_global_freeze_bits(struct mtd_info *mtd, size_t len,
> +					    u8 *buf)
> +{
> +	struct spi_nor *nor = mtd_to_spi_nor(mtd);
> +	int ret;
> +
> +	ret = spi_nor_lock_and_prep(nor);
> +	if (ret)
> +		return ret;
> +
> +	ret = nor->params->sec_ops->write_global_freeze_bits(nor, len, buf);
> +
> +	spi_nor_unlock_and_unprep(nor);
> +	return ret;
> +}
> +
> +static int spi_nor_read_password(struct mtd_info *mtd, size_t len, u8 *buf)
> +{
> +	struct spi_nor *nor = mtd_to_spi_nor(mtd);
> +	int ret;
> +
> +	ret = spi_nor_lock_and_prep(nor);
> +	if (ret)
> +		return ret;
> +
> +	ret = nor->params->sec_ops->read_password(nor, len, buf);
> +
> +	spi_nor_unlock_and_unprep(nor);
> +	return ret;
> +}
> +
> +void spi_nor_register_security_ops(struct spi_nor *nor)
> +{
> +	struct mtd_info *mtd = &nor->mtd;
> +
> +	if (!nor->params->sec_ops)
> +		return;
> +
> +	mtd->_secure_packet_read = spi_nor_secure_read;
> +	mtd->_secure_packet_write = spi_nor_secure_write;
> +	mtd->_read_vlock_bits = spi_nor_read_vlock_bits;
> +	mtd->_write_vlock_bits = spi_nor_write_vlock_bits;
> +	mtd->_read_nvlock_bits = spi_nor_read_nvlock_bits;
> +	mtd->_write_nvlock_bits = spi_nor_write_nvlock_bits;
> +	mtd->_erase_nvlock_bits = spi_nor_erase_nvlock_bits;
> +	mtd->_read_global_freeze_bits = spi_nor_read_global_freeze_bits;
> +	mtd->_write_global_freeze_bits = spi_nor_write_global_freeze_bits;
> +	mtd->_read_password = spi_nor_read_password;

This approach requires all or none of the sec_ops functions to be 
implemented. It doesn't consider other drivers which may be able to 
implement a subset of the sec_ops functions.

I also think it would be better not to use extra function pointers here 
and just let other code call the functions defined above directly. The 
caller of these functions will need to check that the pointers aren't 
NULL before calling them anyway, so I think we may as well call the 
functions directly and have each of them check that the corresponding 
sec_ops field is non-NULL before calling it.

> +}
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index cc08bd707378..864f3c7783b3 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -3199,6 +3199,8 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
>   
>   	spi_nor_register_locking_ops(nor);
>   
> +	spi_nor_register_security_ops(nor);
> +
>   	/* Send all the required SPI flash commands to initialize device */
>   	ret = spi_nor_init(nor);
>   	if (ret)
> diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
> index 88227044fc86..bce358c9fb94 100644
> --- a/include/linux/mtd/mtd.h
> +++ b/include/linux/mtd/mtd.h
> @@ -360,6 +360,25 @@ struct mtd_info {
>   	int (*_get_device) (struct mtd_info *mtd);
>   	void (*_put_device) (struct mtd_info *mtd);
>   
> +	/*
> +	 * Security Operations
> +	 */
> +	int (*_secure_packet_read)(struct mtd_info *mtd, size_t len, u8 *buf);
> +	int (*_secure_packet_write)(struct mtd_info *mtd, size_t len, u8 *buf);
> +	int (*_read_vlock_bits)(struct mtd_info *mtd, u32 addr, size_t len,
> +				u8 *buf);
> +	int (*_write_vlock_bits)(struct mtd_info *mtd, u32 addr, size_t len,
> +				 u8 *buf);
> +	int (*_read_nvlock_bits)(struct mtd_info *mtd, u32 addr, size_t len,
> +				 u8 *buf);
> +	int (*_write_nvlock_bits)(struct mtd_info *mtd, u32 addr);
> +	int (*_erase_nvlock_bits)(struct mtd_info *mtd);
> +	int (*_read_global_freeze_bits)(struct mtd_info *mtd, size_t len,
> +					u8 *buf);
> +	int (*_write_global_freeze_bits)(struct mtd_info *mtd, size_t len,
> +					 u8 *buf);
> +	int (*_read_password)(struct mtd_info *mtd, size_t len, u8 *buf);
> +
>   	/*
>   	 * flag indicates a panic write, low level drivers can take appropriate
>   	 * action if required to ensure writes go through
> 

Thanks,

-- 
Paul Barker
Principal Software Engineer
SanCloud Ltd

e: paul.barker@sancloud.com
w: https://sancloud.co.uk/

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 7643 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

[PATCH 3/4] mtd: add advanced protection and security ioctls

[shiva.linuxworks]

From: Shivamurthy Shastri <sshivamurthy@micron.com>

Added new ioctls for advanced protection and security features.
These features are currently supported by new Micron SPI NOR flashes.

Signed-off-by: Shivamurthy Shastri <sshivamurthy@micron.com>
---
 drivers/mtd/mtdchar.c      | 145 +++++++++++++++++++++++++++++++++++++
 include/uapi/mtd/mtd-abi.h |  11 +++
 2 files changed, 156 insertions(+)

diff --git a/drivers/mtd/mtdchar.c b/drivers/mtd/mtdchar.c
index 155e991d9d75..97b97b80276d 100644
--- a/drivers/mtd/mtdchar.c
+++ b/drivers/mtd/mtdchar.c
@@ -654,6 +654,16 @@ static int mtdchar_ioctl(struct file *file, u_int cmd, u_long arg)
 	case MTDFILEMODE:
 	case BLKPG:
 	case BLKRRPART:
+	case SECURE_PACKET_READ:
+	case SECURE_PACKET_WRITE:
+	case RD_VLOCK_BITS:
+	case WR_VLOCK_BITS:
+	case RD_NVLOCK_BITS:
+	case WR_NVLOCK_BITS:
+	case ER_NVLOCK_BITS:
+	case RD_GLOBAL_FREEZE_BITS:
+	case WR_GLOBAL_FREEZE_BITS:
+	case RD_PASSWORD:
 		break;
 
 	/* "dangerous" commands */
@@ -1017,6 +1027,141 @@ static int mtdchar_ioctl(struct file *file, u_int cmd, u_long arg)
 		ret = 0;
 		break;
 	}
+	case SECURE_PACKET_READ:
+	{
+		struct mtd_oob_buf buf;
+		u8 *oobbuf;
+
+		if (copy_from_user(&buf, argp, sizeof(buf)))
+			ret = -EFAULT;
+
+		oobbuf = kmalloc(buf.length, GFP_KERNEL);
+		ret = master->_secure_packet_read(master, buf.length, oobbuf);
+		if (copy_to_user(buf.ptr, oobbuf, buf.length))
+			ret = -EFAULT;
+		break;
+	}
+
+	case SECURE_PACKET_WRITE:
+	{
+		struct mtd_oob_buf buf;
+		u8 *oobbuf;
+
+		if (copy_from_user(&buf, argp, sizeof(buf)))
+			ret = -EFAULT;
+
+		oobbuf = memdup_user(buf.ptr, buf.length);
+		ret = master->_secure_packet_write(master, buf.length, oobbuf);
+		break;
+	}
+
+	case RD_VLOCK_BITS:
+	{
+		struct mtd_oob_buf buf;
+		u8 *oobbuf;
+
+		if (copy_from_user(&buf, argp, sizeof(buf)))
+			ret = -EFAULT;
+
+		oobbuf = kmalloc(buf.length, GFP_KERNEL);
+		ret = master->_read_vlock_bits(master, buf.start, buf.length,
+					       oobbuf);
+		if (copy_to_user(buf.ptr, oobbuf, buf.length))
+			ret = -EFAULT;
+		break;
+	}
+
+	case WR_VLOCK_BITS:
+	{
+		struct mtd_oob_buf buf;
+		u8 *oobbuf;
+
+		if (copy_from_user(&buf, argp, sizeof(buf)))
+			ret = -EFAULT;
+
+		oobbuf = memdup_user(buf.ptr, buf.length);
+		ret = master->_write_vlock_bits(master, buf. start, buf.length,
+						oobbuf);
+		break;
+	}
+
+	case RD_NVLOCK_BITS:
+	{
+		struct mtd_oob_buf buf;
+		u8 *oobbuf;
+
+		if (copy_from_user(&buf, argp, sizeof(buf)))
+			ret = -EFAULT;
+
+		oobbuf = kmalloc(buf.length, GFP_KERNEL);
+		ret = master->_read_nvlock_bits(master, buf.start, buf.length,
+						oobbuf);
+		if (copy_to_user(buf.ptr, oobbuf, buf.length))
+			ret = -EFAULT;
+		break;
+	}
+
+	case WR_NVLOCK_BITS:
+	{
+		struct mtd_oob_buf buf;
+
+		if (copy_from_user(&buf, argp, sizeof(buf)))
+			ret = -EFAULT;
+
+		ret = master->_write_nvlock_bits(master, buf.start);
+		break;
+	}
+
+	case ER_NVLOCK_BITS:
+	{
+		ret = master->_erase_nvlock_bits(master);
+		break;
+	}
+
+	case RD_GLOBAL_FREEZE_BITS:
+	{
+		struct mtd_oob_buf buf;
+		u8 *oobbuf;
+
+		if (copy_from_user(&buf, argp, sizeof(buf)))
+			ret = -EFAULT;
+
+		oobbuf = kmalloc(buf.length, GFP_KERNEL);
+		ret = master->_read_global_freeze_bits(master, buf.length,
+						       oobbuf);
+		if (copy_to_user(buf.ptr, oobbuf, buf.length))
+			ret = -EFAULT;
+		break;
+	}
+
+	case WR_GLOBAL_FREEZE_BITS:
+	{
+		struct mtd_oob_buf buf;
+		u8 *oobbuf;
+
+		if (copy_from_user(&buf, argp, sizeof(buf)))
+			ret = -EFAULT;
+
+		oobbuf = memdup_user(buf.ptr, buf.length);
+		ret = master->_write_global_freeze_bits(master, buf.length,
+							oobbuf);
+		break;
+	}
+
+	case RD_PASSWORD:
+	{
+		struct mtd_oob_buf buf;
+		u8 *oobbuf;
+
+		if (copy_from_user(&buf, argp, sizeof(buf)))
+			ret = -EFAULT;
+
+		oobbuf = kmalloc(buf.length, GFP_KERNEL);
+		ret = master->_read_password(master, buf.length, oobbuf);
+		if (copy_to_user(buf.ptr, oobbuf, buf.length))
+			ret = -EFAULT;
+		break;
+	}
 	}
 
 	return ret;
diff --git a/include/uapi/mtd/mtd-abi.h b/include/uapi/mtd/mtd-abi.h
index b869990c2db2..dbd7bf60d484 100644
--- a/include/uapi/mtd/mtd-abi.h
+++ b/include/uapi/mtd/mtd-abi.h
@@ -208,6 +208,17 @@ struct otp_info {
 /* Erase a given range of user data (must be in mode %MTD_FILE_MODE_OTP_USER) */
 #define OTPERASE		_IOW('M', 25, struct otp_info)
 
+#define SECURE_PACKET_READ	_IOWR('M', 26, struct mtd_oob_buf)
+#define SECURE_PACKET_WRITE	_IOWR('M', 27, struct mtd_oob_buf)
+#define RD_VLOCK_BITS		_IOWR('M', 28, struct mtd_oob_buf)
+#define WR_VLOCK_BITS		_IOWR('M', 29, struct mtd_oob_buf)
+#define RD_NVLOCK_BITS		_IOWR('M', 30, struct mtd_oob_buf)
+#define WR_NVLOCK_BITS		_IOWR('M', 31, struct mtd_oob_buf)
+#define ER_NVLOCK_BITS		_IO('M', 32)
+#define RD_GLOBAL_FREEZE_BITS	_IOWR('M', 33, struct mtd_oob_buf)
+#define WR_GLOBAL_FREEZE_BITS	_IOWR('M', 34, struct mtd_oob_buf)
+#define RD_PASSWORD		_IOWR('M', 35, struct mtd_oob_buf)
+
 /*
  * Obsolete legacy interface. Keep it in order not to break userspace
  * interfaces
-- 
2.25.1

Re: [PATCH 3/4] mtd: add advanced protection and security ioctls

[Paul Barker]

On 27/10/2021 11:33, shiva.linuxworks@gmail.com wrote:
> From: Shivamurthy Shastri <sshivamurthy@micron.com>
> 
> Added new ioctls for advanced protection and security features.
> These features are currently supported by new Micron SPI NOR flashes.
> 
> Signed-off-by: Shivamurthy Shastri <sshivamurthy@micron.com>
> ---
>   drivers/mtd/mtdchar.c      | 145 +++++++++++++++++++++++++++++++++++++
>   include/uapi/mtd/mtd-abi.h |  11 +++
>   2 files changed, 156 insertions(+)
> 
> diff --git a/drivers/mtd/mtdchar.c b/drivers/mtd/mtdchar.c
> index 155e991d9d75..97b97b80276d 100644
> --- a/drivers/mtd/mtdchar.c
> +++ b/drivers/mtd/mtdchar.c
> @@ -654,6 +654,16 @@ static int mtdchar_ioctl(struct file *file, u_int cmd, u_long arg)
>   	case MTDFILEMODE:
>   	case BLKPG:
>   	case BLKRRPART:
> +	case SECURE_PACKET_READ:
> +	case SECURE_PACKET_WRITE:
> +	case RD_VLOCK_BITS:
> +	case WR_VLOCK_BITS:
> +	case RD_NVLOCK_BITS:
> +	case WR_NVLOCK_BITS:
> +	case ER_NVLOCK_BITS:
> +	case RD_GLOBAL_FREEZE_BITS:
> +	case WR_GLOBAL_FREEZE_BITS:
> +	case RD_PASSWORD:
>   		break;

It looks like you've listed all of the ioctls as "safe" commands so the 
write permission bit is not checked. My understanding is that all ioctls 
which may modify the data in flash need moving to the "dangerous" 
commands section below this so that the write permission bit is checked.

>   
>   	/* "dangerous" commands */
> @@ -1017,6 +1027,141 @@ static int mtdchar_ioctl(struct file *file, u_int cmd, u_long arg)
>   		ret = 0;
>   		break;
>   	}
> +	case SECURE_PACKET_READ:
> +	{
> +		struct mtd_oob_buf buf;
> +		u8 *oobbuf;
> +
> +		if (copy_from_user(&buf, argp, sizeof(buf)))
> +			ret = -EFAULT;
> +
> +		oobbuf = kmalloc(buf.length, GFP_KERNEL);
> +		ret = master->_secure_packet_read(master, buf.length, oobbuf);
> +		if (copy_to_user(buf.ptr, oobbuf, buf.length))
> +			ret = -EFAULT;
> +		break;
> +	}
> +
> +	case SECURE_PACKET_WRITE:
> +	{
> +		struct mtd_oob_buf buf;
> +		u8 *oobbuf;
> +
> +		if (copy_from_user(&buf, argp, sizeof(buf)))
> +			ret = -EFAULT;
> +
> +		oobbuf = memdup_user(buf.ptr, buf.length);
> +		ret = master->_secure_packet_write(master, buf.length, oobbuf);
> +		break;
> +	}
> +
> +	case RD_VLOCK_BITS:
> +	{
> +		struct mtd_oob_buf buf;
> +		u8 *oobbuf;
> +
> +		if (copy_from_user(&buf, argp, sizeof(buf)))
> +			ret = -EFAULT;
> +
> +		oobbuf = kmalloc(buf.length, GFP_KERNEL);
> +		ret = master->_read_vlock_bits(master, buf.start, buf.length,
> +					       oobbuf);
> +		if (copy_to_user(buf.ptr, oobbuf, buf.length))
> +			ret = -EFAULT;
> +		break;
> +	}
> +
> +	case WR_VLOCK_BITS:
> +	{
> +		struct mtd_oob_buf buf;
> +		u8 *oobbuf;
> +
> +		if (copy_from_user(&buf, argp, sizeof(buf)))
> +			ret = -EFAULT;
> +
> +		oobbuf = memdup_user(buf.ptr, buf.length);
> +		ret = master->_write_vlock_bits(master, buf. start, buf.length,
> +						oobbuf);
> +		break;
> +	}
> +
> +	case RD_NVLOCK_BITS:
> +	{
> +		struct mtd_oob_buf buf;
> +		u8 *oobbuf;
> +
> +		if (copy_from_user(&buf, argp, sizeof(buf)))
> +			ret = -EFAULT;
> +
> +		oobbuf = kmalloc(buf.length, GFP_KERNEL);
> +		ret = master->_read_nvlock_bits(master, buf.start, buf.length,
> +						oobbuf);
> +		if (copy_to_user(buf.ptr, oobbuf, buf.length))
> +			ret = -EFAULT;
> +		break;
> +	}
> +
> +	case WR_NVLOCK_BITS:
> +	{
> +		struct mtd_oob_buf buf;
> +
> +		if (copy_from_user(&buf, argp, sizeof(buf)))
> +			ret = -EFAULT;
> +
> +		ret = master->_write_nvlock_bits(master, buf.start);
> +		break;
> +	}
> +
> +	case ER_NVLOCK_BITS:
> +	{
> +		ret = master->_erase_nvlock_bits(master);
> +		break;
> +	}
> +
> +	case RD_GLOBAL_FREEZE_BITS:
> +	{
> +		struct mtd_oob_buf buf;
> +		u8 *oobbuf;
> +
> +		if (copy_from_user(&buf, argp, sizeof(buf)))
> +			ret = -EFAULT;
> +
> +		oobbuf = kmalloc(buf.length, GFP_KERNEL);
> +		ret = master->_read_global_freeze_bits(master, buf.length,
> +						       oobbuf);
> +		if (copy_to_user(buf.ptr, oobbuf, buf.length))
> +			ret = -EFAULT;
> +		break;
> +	}
> +
> +	case WR_GLOBAL_FREEZE_BITS:
> +	{
> +		struct mtd_oob_buf buf;
> +		u8 *oobbuf;
> +
> +		if (copy_from_user(&buf, argp, sizeof(buf)))
> +			ret = -EFAULT;
> +
> +		oobbuf = memdup_user(buf.ptr, buf.length);
> +		ret = master->_write_global_freeze_bits(master, buf.length,
> +							oobbuf);
> +		break;
> +	}
> +
> +	case RD_PASSWORD:
> +	{
> +		struct mtd_oob_buf buf;
> +		u8 *oobbuf;
> +
> +		if (copy_from_user(&buf, argp, sizeof(buf)))
> +			ret = -EFAULT;
> +
> +		oobbuf = kmalloc(buf.length, GFP_KERNEL);
> +		ret = master->_read_password(master, buf.length, oobbuf);
> +		if (copy_to_user(buf.ptr, oobbuf, buf.length))
> +			ret = -EFAULT;
> +		break;
> +	}
>   	}
>   
>   	return ret;
> diff --git a/include/uapi/mtd/mtd-abi.h b/include/uapi/mtd/mtd-abi.h
> index b869990c2db2..dbd7bf60d484 100644
> --- a/include/uapi/mtd/mtd-abi.h
> +++ b/include/uapi/mtd/mtd-abi.h
> @@ -208,6 +208,17 @@ struct otp_info {
>   /* Erase a given range of user data (must be in mode %MTD_FILE_MODE_OTP_USER) */
>   #define OTPERASE		_IOW('M', 25, struct otp_info)
>   
> +#define SECURE_PACKET_READ	_IOWR('M', 26, struct mtd_oob_buf)
> +#define SECURE_PACKET_WRITE	_IOWR('M', 27, struct mtd_oob_buf)
> +#define RD_VLOCK_BITS		_IOWR('M', 28, struct mtd_oob_buf)
> +#define WR_VLOCK_BITS		_IOWR('M', 29, struct mtd_oob_buf)
> +#define RD_NVLOCK_BITS		_IOWR('M', 30, struct mtd_oob_buf)
> +#define WR_NVLOCK_BITS		_IOWR('M', 31, struct mtd_oob_buf)
> +#define ER_NVLOCK_BITS		_IO('M', 32)
> +#define RD_GLOBAL_FREEZE_BITS	_IOWR('M', 33, struct mtd_oob_buf)
> +#define WR_GLOBAL_FREEZE_BITS	_IOWR('M', 34, struct mtd_oob_buf)
> +#define RD_PASSWORD		_IOWR('M', 35, struct mtd_oob_buf)
> +

All other ioctls defined in this header are preceeded by a comment which 
briefly explains what they do. I think this is needed for these new 
ioctls as well.

>   /*
>    * Obsolete legacy interface. Keep it in order not to break userspace
>    * interfaces
> 

Thanks,

-- 
Paul Barker
Principal Software Engineer
SanCloud Ltd

e: paul.barker@sancloud.com
w: https://sancloud.co.uk/

Re: [PATCH 3/4] mtd: add advanced protection and security ioctls

[Paul Barker]

[-- Attachment #1.1.1: Type: text/plain, Size: 8705 bytes --]

Apologies for the double-reply, spotted another thing I'd like to 
comment on...

On 06/12/2021 10:42, Paul Barker wrote:
> On 27/10/2021 11:33, shiva.linuxworks@gmail.com wrote:
>> From: Shivamurthy Shastri <sshivamurthy@micron.com>
>>
>> Added new ioctls for advanced protection and security features.
>> These features are currently supported by new Micron SPI NOR flashes.
>>
>> Signed-off-by: Shivamurthy Shastri <sshivamurthy@micron.com>
>> ---
>>   drivers/mtd/mtdchar.c      | 145 +++++++++++++++++++++++++++++++++++++
>>   include/uapi/mtd/mtd-abi.h |  11 +++
>>   2 files changed, 156 insertions(+)
>>
>> diff --git a/drivers/mtd/mtdchar.c b/drivers/mtd/mtdchar.c
>> index 155e991d9d75..97b97b80276d 100644
>> --- a/drivers/mtd/mtdchar.c
>> +++ b/drivers/mtd/mtdchar.c
>> @@ -654,6 +654,16 @@ static int mtdchar_ioctl(struct file *file, u_int 
>> cmd, u_long arg)
>>       case MTDFILEMODE:
>>       case BLKPG:
>>       case BLKRRPART:
>> +    case SECURE_PACKET_READ:
>> +    case SECURE_PACKET_WRITE:
>> +    case RD_VLOCK_BITS:
>> +    case WR_VLOCK_BITS:
>> +    case RD_NVLOCK_BITS:
>> +    case WR_NVLOCK_BITS:
>> +    case ER_NVLOCK_BITS:
>> +    case RD_GLOBAL_FREEZE_BITS:
>> +    case WR_GLOBAL_FREEZE_BITS:
>> +    case RD_PASSWORD:
>>           break;
> 
> It looks like you've listed all of the ioctls as "safe" commands so the 
> write permission bit is not checked. My understanding is that all ioctls 
> which may modify the data in flash need moving to the "dangerous" 
> commands section below this so that the write permission bit is checked.
> 
>>       /* "dangerous" commands */
>> @@ -1017,6 +1027,141 @@ static int mtdchar_ioctl(struct file *file, 
>> u_int cmd, u_long arg)
>>           ret = 0;
>>           break;
>>       }
>> +    case SECURE_PACKET_READ:
>> +    {
>> +        struct mtd_oob_buf buf;
>> +        u8 *oobbuf;
>> +
>> +        if (copy_from_user(&buf, argp, sizeof(buf)))
>> +            ret = -EFAULT;
>> +
>> +        oobbuf = kmalloc(buf.length, GFP_KERNEL);
>> +        ret = master->_secure_packet_read(master, buf.length, oobbuf);

After reviewing patch 2 in this series it's clear that these function 
pointers are NULL if a driver does not implement the sec_ops functions. 
So unless the implementation there is changed, each of these calls needs 
to be wrapped in a test to see if the corresponding pointer is valid and 
to return -EOPNOTSUPP if it is NULL.

>> +        if (copy_to_user(buf.ptr, oobbuf, buf.length))
>> +            ret = -EFAULT;
>> +        break;
>> +    }
>> +
>> +    case SECURE_PACKET_WRITE:
>> +    {
>> +        struct mtd_oob_buf buf;
>> +        u8 *oobbuf;
>> +
>> +        if (copy_from_user(&buf, argp, sizeof(buf)))
>> +            ret = -EFAULT;
>> +
>> +        oobbuf = memdup_user(buf.ptr, buf.length);
>> +        ret = master->_secure_packet_write(master, buf.length, oobbuf);
>> +        break;
>> +    }
>> +
>> +    case RD_VLOCK_BITS:
>> +    {
>> +        struct mtd_oob_buf buf;
>> +        u8 *oobbuf;
>> +
>> +        if (copy_from_user(&buf, argp, sizeof(buf)))
>> +            ret = -EFAULT;
>> +
>> +        oobbuf = kmalloc(buf.length, GFP_KERNEL);
>> +        ret = master->_read_vlock_bits(master, buf.start, buf.length,
>> +                           oobbuf);
>> +        if (copy_to_user(buf.ptr, oobbuf, buf.length))
>> +            ret = -EFAULT;
>> +        break;
>> +    }
>> +
>> +    case WR_VLOCK_BITS:
>> +    {
>> +        struct mtd_oob_buf buf;
>> +        u8 *oobbuf;
>> +
>> +        if (copy_from_user(&buf, argp, sizeof(buf)))
>> +            ret = -EFAULT;
>> +
>> +        oobbuf = memdup_user(buf.ptr, buf.length);
>> +        ret = master->_write_vlock_bits(master, buf. start, buf.length,
>> +                        oobbuf);
>> +        break;
>> +    }
>> +
>> +    case RD_NVLOCK_BITS:
>> +    {
>> +        struct mtd_oob_buf buf;
>> +        u8 *oobbuf;
>> +
>> +        if (copy_from_user(&buf, argp, sizeof(buf)))
>> +            ret = -EFAULT;
>> +
>> +        oobbuf = kmalloc(buf.length, GFP_KERNEL);
>> +        ret = master->_read_nvlock_bits(master, buf.start, buf.length,
>> +                        oobbuf);
>> +        if (copy_to_user(buf.ptr, oobbuf, buf.length))
>> +            ret = -EFAULT;
>> +        break;
>> +    }
>> +
>> +    case WR_NVLOCK_BITS:
>> +    {
>> +        struct mtd_oob_buf buf;
>> +
>> +        if (copy_from_user(&buf, argp, sizeof(buf)))
>> +            ret = -EFAULT;
>> +
>> +        ret = master->_write_nvlock_bits(master, buf.start);
>> +        break;
>> +    }
>> +
>> +    case ER_NVLOCK_BITS:
>> +    {
>> +        ret = master->_erase_nvlock_bits(master);
>> +        break;
>> +    }
>> +
>> +    case RD_GLOBAL_FREEZE_BITS:
>> +    {
>> +        struct mtd_oob_buf buf;
>> +        u8 *oobbuf;
>> +
>> +        if (copy_from_user(&buf, argp, sizeof(buf)))
>> +            ret = -EFAULT;
>> +
>> +        oobbuf = kmalloc(buf.length, GFP_KERNEL);
>> +        ret = master->_read_global_freeze_bits(master, buf.length,
>> +                               oobbuf);
>> +        if (copy_to_user(buf.ptr, oobbuf, buf.length))
>> +            ret = -EFAULT;
>> +        break;
>> +    }
>> +
>> +    case WR_GLOBAL_FREEZE_BITS:
>> +    {
>> +        struct mtd_oob_buf buf;
>> +        u8 *oobbuf;
>> +
>> +        if (copy_from_user(&buf, argp, sizeof(buf)))
>> +            ret = -EFAULT;
>> +
>> +        oobbuf = memdup_user(buf.ptr, buf.length);
>> +        ret = master->_write_global_freeze_bits(master, buf.length,
>> +                            oobbuf);
>> +        break;
>> +    }
>> +
>> +    case RD_PASSWORD:
>> +    {
>> +        struct mtd_oob_buf buf;
>> +        u8 *oobbuf;
>> +
>> +        if (copy_from_user(&buf, argp, sizeof(buf)))
>> +            ret = -EFAULT;
>> +
>> +        oobbuf = kmalloc(buf.length, GFP_KERNEL);
>> +        ret = master->_read_password(master, buf.length, oobbuf);
>> +        if (copy_to_user(buf.ptr, oobbuf, buf.length))
>> +            ret = -EFAULT;
>> +        break;
>> +    }
>>       }
>>       return ret;
>> diff --git a/include/uapi/mtd/mtd-abi.h b/include/uapi/mtd/mtd-abi.h
>> index b869990c2db2..dbd7bf60d484 100644
>> --- a/include/uapi/mtd/mtd-abi.h
>> +++ b/include/uapi/mtd/mtd-abi.h
>> @@ -208,6 +208,17 @@ struct otp_info {
>>   /* Erase a given range of user data (must be in mode 
>> %MTD_FILE_MODE_OTP_USER) */
>>   #define OTPERASE        _IOW('M', 25, struct otp_info)
>> +#define SECURE_PACKET_READ    _IOWR('M', 26, struct mtd_oob_buf)
>> +#define SECURE_PACKET_WRITE    _IOWR('M', 27, struct mtd_oob_buf)
>> +#define RD_VLOCK_BITS        _IOWR('M', 28, struct mtd_oob_buf)
>> +#define WR_VLOCK_BITS        _IOWR('M', 29, struct mtd_oob_buf)
>> +#define RD_NVLOCK_BITS        _IOWR('M', 30, struct mtd_oob_buf)
>> +#define WR_NVLOCK_BITS        _IOWR('M', 31, struct mtd_oob_buf)
>> +#define ER_NVLOCK_BITS        _IO('M', 32)
>> +#define RD_GLOBAL_FREEZE_BITS    _IOWR('M', 33, struct mtd_oob_buf)
>> +#define WR_GLOBAL_FREEZE_BITS    _IOWR('M', 34, struct mtd_oob_buf)
>> +#define RD_PASSWORD        _IOWR('M', 35, struct mtd_oob_buf)
>> +
> 
> All other ioctls defined in this header are preceeded by a comment which 
> briefly explains what they do. I think this is needed for these new 
> ioctls as well.
> 
>>   /*
>>    * Obsolete legacy interface. Keep it in order not to break userspace
>>    * interfaces
>>
> 
> Thanks,
> 

-- 
Paul Barker
Principal Software Engineer
SanCloud Ltd

e: paul.barker@sancloud.com
w: https://sancloud.co.uk/

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 7643 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

[PATCH 4/4] mtd: spi-nor: micron-st: add mt25qu128abb and mt25ql128abb

[shiva.linuxworks]

From: Shivamurthy Shastri <sshivamurthy@micron.com>

Added new Micron SPI NOR flashes to structure flash_info, which supports
advanced protection and security features.

Signed-off-by: Shivamurthy Shastri <sshivamurthy@micron.com>
---
 drivers/mtd/spi-nor/micron-st.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/mtd/spi-nor/micron-st.c b/drivers/mtd/spi-nor/micron-st.c
index b5d82e85fb92..2bebd76b091a 100644
--- a/drivers/mtd/spi-nor/micron-st.c
+++ b/drivers/mtd/spi-nor/micron-st.c
@@ -158,10 +158,17 @@ static const struct flash_info st_parts[] = {
 			      SECT_4K | SPI_NOR_QUAD_READ) },
 	{ "n25q064a",    INFO(0x20bb17, 0, 64 * 1024,  128,
 			      SECT_4K | SPI_NOR_QUAD_READ) },
+	{ "mt25qu128abb", INFO6(0x20bb18, 0x12008c, 64 * 1024,  256,
+				SECT_4K | USE_FSR | SPI_NOR_QUAD_READ |
+				SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB |
+				SPI_NOR_4BIT_BP | SPI_NOR_BP3_SR_BIT6) },
 	{ "n25q128a11",  INFO(0x20bb18, 0, 64 * 1024,  256,
 			      SECT_4K | USE_FSR | SPI_NOR_QUAD_READ |
 			      SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB |
 			      SPI_NOR_4BIT_BP | SPI_NOR_BP3_SR_BIT6) },
+	{ "mt25ql128abb", INFO6(0x20ba18, 0x12008c, 64 * 1024,  256,
+				SECT_4K | USE_FSR | SPI_NOR_HAS_LOCK |
+				SPI_NOR_QUAD_READ) },
 	{ "n25q128a13",  INFO(0x20ba18, 0, 64 * 1024,  256,
 			      SECT_4K | USE_FSR | SPI_NOR_QUAD_READ) },
 	{ "mt25ql256a",  INFO6(0x20ba19, 0x104400, 64 * 1024,  512,
-- 
2.25.1

Re: [PATCH 4/4] mtd: spi-nor: micron-st: add mt25qu128abb and mt25ql128abb

[Paul Barker]

[-- Attachment #1.1.1: Type: text/plain, Size: 1832 bytes --]

On 27/10/2021 11:33, shiva.linuxworks@gmail.com wrote:
> From: Shivamurthy Shastri <sshivamurthy@micron.com>
> 
> Added new Micron SPI NOR flashes to structure flash_info, which supports
> advanced protection and security features.
> 
> Signed-off-by: Shivamurthy Shastri <sshivamurthy@micron.com>
> ---
>   drivers/mtd/spi-nor/micron-st.c | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/micron-st.c b/drivers/mtd/spi-nor/micron-st.c
> index b5d82e85fb92..2bebd76b091a 100644
> --- a/drivers/mtd/spi-nor/micron-st.c
> +++ b/drivers/mtd/spi-nor/micron-st.c
> @@ -158,10 +158,17 @@ static const struct flash_info st_parts[] = {
>   			      SECT_4K | SPI_NOR_QUAD_READ) },
>   	{ "n25q064a",    INFO(0x20bb17, 0, 64 * 1024,  128,
>   			      SECT_4K | SPI_NOR_QUAD_READ) },
> +	{ "mt25qu128abb", INFO6(0x20bb18, 0x12008c, 64 * 1024,  256,
> +				SECT_4K | USE_FSR | SPI_NOR_QUAD_READ |
> +				SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB |
> +				SPI_NOR_4BIT_BP | SPI_NOR_BP3_SR_BIT6) },
>   	{ "n25q128a11",  INFO(0x20bb18, 0, 64 * 1024,  256,
>   			      SECT_4K | USE_FSR | SPI_NOR_QUAD_READ |
>   			      SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB |
>   			      SPI_NOR_4BIT_BP | SPI_NOR_BP3_SR_BIT6) },
> +	{ "mt25ql128abb", INFO6(0x20ba18, 0x12008c, 64 * 1024,  256,
> +				SECT_4K | USE_FSR | SPI_NOR_HAS_LOCK |
> +				SPI_NOR_QUAD_READ) },
>   	{ "n25q128a13",  INFO(0x20ba18, 0, 64 * 1024,  256,
>   			      SECT_4K | USE_FSR | SPI_NOR_QUAD_READ) },
>   	{ "mt25ql256a",  INFO6(0x20ba19, 0x104400, 64 * 1024,  512,
> 

I suspect this patch can be submitted as a standalone. It doesn't seem 
to depend on the others in this series.

Thanks,

-- 
Paul Barker
Principal Software Engineer
SanCloud Ltd

e: paul.barker@sancloud.com
w: https://sancloud.co.uk/

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 7643 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

Re: [PATCH 0/4] enabling Advanced protection and security features

[Richard Weinberger]

----- Ursprüngliche Mail -----
> Von: "shiva linuxworks" <shiva.linuxworks@gmail.com>
> An: "Tudor Ambarus" <tudor.ambarus@microchip.com>, "Michael Walle" <michael@walle.cc>, "Pratyush Yadav"
> <p.yadav@ti.com>, "Miquel Raynal" <miquel.raynal@bootlin.com>, "richard" <richard@nod.at>, "Vignesh Raghavendra"
> <vigneshr@ti.com>
> CC: "linux-mtd" <linux-mtd@lists.infradead.org>, "linux-kernel" <linux-kernel@vger.kernel.org>, "Shivamurthy Shastri"
> <sshivamurthy@micron.com>
> Gesendet: Mittwoch, 27. Oktober 2021 12:33:48
> Betreff: [PATCH 0/4] enabling Advanced protection and security features

> From: Shivamurthy Shastri <sshivamurthy@micron.com>
> 
> Standard protection features in SPI NOR flashes are legacy and offer a
> simple way to protect the memory array against accidental or unwanted
> modification of its content.
> 
> These patches enable the support for advanced sector protection which
> protects memory from accidentally corrupting code and data stored, and
> it also prevents malicious attacks that could intentionally modify the
> code or data stored in the memory.
> 
> Micron Flashes offer some of the advanced protection methods using
> volatile lock bits, non-volatile lock bits, global freeze bits, and
> password.

Can you please point us to the technical documentation of these features?
I'm especially interested in the password feature.

Thanks,
//richard

RE: [EXT] Re: [PATCH 0/4] enabling Advanced protection and security features

[Shivamurthy Shastri (sshivamurthy)]

Micron Confidential

Hi Richard,

Sorry for late reply.

> > Standard protection features in SPI NOR flashes are legacy and offer a
> > simple way to protect the memory array against accidental or unwanted
> > modification of its content.
> >
> > These patches enable the support for advanced sector protection which
> > protects memory from accidentally corrupting code and data stored, and
> > it also prevents malicious attacks that could intentionally modify the
> > code or data stored in the memory.
> >
> > Micron Flashes offer some of the advanced protection methods using
> > volatile lock bits, non-volatile lock bits, global freeze bits, and
> > password.
> 
> Can you please point us to the technical documentation of these features?
> I'm especially interested in the password feature.

Document link:
https://media-www.micron.com/-/media/client/global/documents/products/technical-note/nor-flash/tn2541_mt25q_protection_and_security.pdf?rev=132de35e149b42a4beaac789eacc01d7

Thanks,
Shiva

Micron Confidential