[PATCH 0/4] mmc: manage boot area protection

[PATCH 0/4] mmc: manage boot area protection

[Heinrich Schuchardt]

This patch series adds sub-command 'mmc wp' to set the power on boot area
protection on eMMC devices and enhances command 'mmc info' to display the
boot area protection status.

Heinrich Schuchardt (4):
  mmc: EXT_CSD registers for write protection
  mmc: export mmc_send_ext_csd()
  cmd: mmc: display write protect state of boot partition
  cmd: mmc: provide boot area protection command

 cmd/mmc.c         | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 drivers/mmc/mmc.c |  7 ++++++-
 include/mmc.h     | 23 +++++++++++++++++++++++
 3 files changed, 75 insertions(+), 1 deletion(-)

--
2.25.1

[PATCH 1/4] mmc: EXT_CSD registers for write protection

[Heinrich Schuchardt]

Add the EXT_CSD register definition related to write protection.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 include/mmc.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/mmc.h b/include/mmc.h
index e83c22423b..4ecd6c744c 100644
--- a/include/mmc.h
+++ b/include/mmc.h
@@ -223,6 +223,9 @@ static inline bool mmc_is_tuning_cmd(uint cmdidx)
 #define EXT_CSD_WR_REL_PARAM		166	/* R */
 #define EXT_CSD_WR_REL_SET		167	/* R/W */
 #define EXT_CSD_RPMB_MULT		168	/* RO */
+#define EXT_CSD_USER_WP			171	/* R/W & R/W/C_P & R/W/E_P */
+#define EXT_CSD_BOOT_WP			173	/* R/W & R/W/C_P */
+#define EXT_CSD_BOOT_WP_STATUS		174	/* R */
 #define EXT_CSD_ERASE_GROUP_DEF		175	/* R/W */
 #define EXT_CSD_BOOT_BUS_WIDTH		177
 #define EXT_CSD_PART_CONF		179	/* R/W */
--
2.25.1

[PATCH 2/4] mmc: export mmc_send_ext_csd()

[Heinrich Schuchardt]

Export function mmc_send_ext_csd() for reading the extended CSD register.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 drivers/mmc/mmc.c |  2 +-
 include/mmc.h     | 11 +++++++++++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index 3e36566693..bfcdaa665d 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -718,7 +718,7 @@ static int mmc_complete_op_cond(struct mmc *mmc)
 }


-static int mmc_send_ext_csd(struct mmc *mmc, u8 *ext_csd)
+int mmc_send_ext_csd(struct mmc *mmc, u8 *ext_csd)
 {
 	struct mmc_cmd cmd;
 	struct mmc_data data;
diff --git a/include/mmc.h b/include/mmc.h
index 4ecd6c744c..85fc3497a1 100644
--- a/include/mmc.h
+++ b/include/mmc.h
@@ -896,6 +896,17 @@ int mmc_get_env_dev(void);
  */
 struct blk_desc *mmc_get_blk_desc(struct mmc *mmc);

+/**
+ * mmc_send_ext_csd() - read the extended CSD register
+ *
+ * @mmc:	MMC device
+ * @ext_csd	a cache aligned buffer of length MMC_MAX_BLOCK_LEN allocated by
+ *		the caller, e.g. using
+ *		ALLOC_CACHE_ALIGN_BUFFER(u8, ext_csd, MMC_MAX_BLOCK_LEN)
+ * Return:	0 for success
+ */
+int mmc_send_ext_csd(struct mmc *mmc, u8 *ext_csd);
+
 static inline enum dma_data_direction mmc_get_dma_dir(struct mmc_data *data)
 {
 	return data->flags & MMC_DATA_WRITE ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
--
2.25.1

[PATCH 3/4] cmd: mmc: display write protect state of boot partition

[Heinrich Schuchardt]

Boot partitions of eMMC devices can be power on or permanently write
protected. Let the 'mmc info' command display the protection state.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 cmd/mmc.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/cmd/mmc.c b/cmd/mmc.c
index 6f3cb85cc0..d62c85e439 100644
--- a/cmd/mmc.c
+++ b/cmd/mmc.c
@@ -54,6 +54,8 @@ static void print_mmcinfo(struct mmc *mmc)
 	if (!IS_SD(mmc) && mmc->version >= MMC_VERSION_4_41) {
 		bool has_enh = (mmc->part_support & ENHNCD_SUPPORT) != 0;
 		bool usr_enh = has_enh && (mmc->part_attr & EXT_CSD_ENH_USR);
+		u8 wp, ext_csd[MMC_MAX_BLOCK_LEN];
+		int ret;

 #if CONFIG_IS_ENABLED(MMC_HW_PARTITIONING)
 		puts("HC WP Group Size: ");
@@ -90,6 +92,28 @@ static void print_mmcinfo(struct mmc *mmc)
 					putc('\n');
 			}
 		}
+		ret = mmc_send_ext_csd(mmc, ext_csd);
+		if (ret)
+			return;
+		wp = ext_csd[EXT_CSD_BOOT_WP_STATUS];
+		for (i = 0; i < 2; ++i) {
+			printf("Boot area %d is ", i);
+			switch (wp & 3) {
+			case 0:
+				printf("not write protected\n");
+				break;
+			case 1:
+				printf("power on protected\n");
+				break;
+			case 2:
+				printf("permanently protected\n");
+				break;
+			default:
+				printf("in reserved protection state\n");
+				break;
+			}
+			wp >>= 2;
+		}
 	}
 }
 static struct mmc *init_mmc_device(int dev, bool force_init)
--
2.25.1

[PATCH 4/4] cmd: mmc: provide boot area protection command

[Heinrich Schuchardt]

Provide command 'mmc wp' to power on write protect boot areas on eMMC
devices.

The B_PWR_WP_EN bit in the extended CSD register BOOT_WP is set. The boot
area are write protected until the next power cycle occurs.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 cmd/mmc.c         | 22 ++++++++++++++++++++++
 drivers/mmc/mmc.c |  5 +++++
 include/mmc.h     |  9 +++++++++
 3 files changed, 36 insertions(+)

diff --git a/cmd/mmc.c b/cmd/mmc.c
index d62c85e439..80a32d9fa9 100644
--- a/cmd/mmc.c
+++ b/cmd/mmc.c
@@ -896,9 +896,30 @@ static int do_mmc_bkops_enable(cmd_tbl_t *cmdtp, int flag,
 }
 #endif

+static int do_mmc_boot_wp(cmd_tbl_t *cmdtp, int flag,
+			  int argc, char * const argv[])
+{
+	int err;
+	struct mmc *mmc;
+
+	mmc = init_mmc_device(curr_device, false);
+	if (!mmc)
+		return CMD_RET_FAILURE;
+	if (IS_SD(mmc)) {
+		printf("It is not an eMMC device\n");
+		return CMD_RET_FAILURE;
+	}
+	err = mmc_boot_wp(mmc);
+	if (err)
+		return CMD_RET_FAILURE;
+	printf("boot areas protected\n");
+	return CMD_RET_SUCCESS;
+}
+
 static cmd_tbl_t cmd_mmc[] = {
 	U_BOOT_CMD_MKENT(info, 1, 0, do_mmcinfo, "", ""),
 	U_BOOT_CMD_MKENT(read, 4, 1, do_mmc_read, "", ""),
+	U_BOOT_CMD_MKENT(wp, 1, 0, do_mmc_boot_wp, "", ""),
 #if CONFIG_IS_ENABLED(MMC_WRITE)
 	U_BOOT_CMD_MKENT(write, 4, 0, do_mmc_write, "", ""),
 	U_BOOT_CMD_MKENT(erase, 3, 0, do_mmc_erase, "", ""),
@@ -968,6 +989,7 @@ U_BOOT_CMD(
 	"mmc part - lists available partition on current mmc device\n"
 	"mmc dev [dev] [part] - show or set current mmc device [partition]\n"
 	"mmc list - lists available devices\n"
+	"mmc wp - power on write protect booot partitions\n"
 #if CONFIG_IS_ENABLED(MMC_HW_PARTITIONING)
 	"mmc hwpartition [args...] - does hardware partitioning\n"
 	"  arguments (sizes in 512-byte blocks):\n"
diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index bfcdaa665d..fc3123c3c3 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -810,6 +810,11 @@ int mmc_switch(struct mmc *mmc, u8 set, u8 index, u8 value)
 	return __mmc_switch(mmc, set, index, value, true);
 }

+int mmc_boot_wp(struct mmc *mmc)
+{
+	return mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_BOOT_WP, 1);
+}
+
 #if !CONFIG_IS_ENABLED(MMC_TINY)
 static int mmc_set_card_speed(struct mmc *mmc, enum bus_mode mode,
 			      bool hsdowngrade)
diff --git a/include/mmc.h b/include/mmc.h
index 85fc3497a1..e92e920560 100644
--- a/include/mmc.h
+++ b/include/mmc.h
@@ -907,6 +907,15 @@ struct blk_desc *mmc_get_blk_desc(struct mmc *mmc);
  */
 int mmc_send_ext_csd(struct mmc *mmc, u8 *ext_csd);

+/**
+ * mmc_boot_wp() - power on write protect boot partitions
+ *
+ * The boot partitions are write protected until the next power cycle.
+ *
+ * Return:	0 for success
+ */
+int mmc_boot_wp(struct mmc *mmc);
+
 static inline enum dma_data_direction mmc_get_dma_dir(struct mmc_data *data)
 {
 	return data->flags & MMC_DATA_WRITE ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
--
2.25.1

[PATCH 3/4] cmd: mmc: display write protect state of boot partition

[Jaehoon Chung]

Hi,

On 3/30/20 2:24 PM, Heinrich Schuchardt wrote:
> Boot partitions of eMMC devices can be power on or permanently write
> protected. Let the 'mmc info' command display the protection state.
> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  cmd/mmc.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/cmd/mmc.c b/cmd/mmc.c
> index 6f3cb85cc0..d62c85e439 100644
> --- a/cmd/mmc.c
> +++ b/cmd/mmc.c
> @@ -54,6 +54,8 @@ static void print_mmcinfo(struct mmc *mmc)
>  	if (!IS_SD(mmc) && mmc->version >= MMC_VERSION_4_41) {
>  		bool has_enh = (mmc->part_support & ENHNCD_SUPPORT) != 0;
>  		bool usr_enh = has_enh && (mmc->part_attr & EXT_CSD_ENH_USR);
> +		u8 wp, ext_csd[MMC_MAX_BLOCK_LEN];
> +		int ret;
> 
>  #if CONFIG_IS_ENABLED(MMC_HW_PARTITIONING)
>  		puts("HC WP Group Size: ");
> @@ -90,6 +92,28 @@ static void print_mmcinfo(struct mmc *mmc)
>  					putc('\n');
>  			}
>  		}
> +		ret = mmc_send_ext_csd(mmc, ext_csd);
> +		if (ret)
> +			return;

Is it really needed to call mmc_send_ext_csd() at here.
ext_csd register value was already read somewhere.

Best Regards,
Jaehoon Chung

> +		wp = ext_csd[EXT_CSD_BOOT_WP_STATUS];
> +		for (i = 0; i < 2; ++i) {
> +			printf("Boot area %d is ", i);
> +			switch (wp & 3) {
> +			case 0:
> +				printf("not write protected\n");
> +				break;
> +			case 1:
> +				printf("power on protected\n");
> +				break;
> +			case 2:
> +				printf("permanently protected\n");
> +				break;
> +			default:
> +				printf("in reserved protection state\n");
> +				break;
> +			}
> +			wp >>= 2;
> +		}
>  	}
>  }
>  static struct mmc *init_mmc_device(int dev, bool force_init)
> --
> 2.25.1
> 
> 
> 

[PATCH 3/4] cmd: mmc: display write protect state of boot partition

[Heinrich Schuchardt]

On 4/1/20 5:04 AM, Jaehoon Chung wrote:
> Hi,
>
> On 3/30/20 2:24 PM, Heinrich Schuchardt wrote:
>> Boot partitions of eMMC devices can be power on or permanently write
>> protected. Let the 'mmc info' command display the protection state.
>>
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>> ---
>>  cmd/mmc.c | 24 ++++++++++++++++++++++++
>>  1 file changed, 24 insertions(+)
>>
>> diff --git a/cmd/mmc.c b/cmd/mmc.c
>> index 6f3cb85cc0..d62c85e439 100644
>> --- a/cmd/mmc.c
>> +++ b/cmd/mmc.c
>> @@ -54,6 +54,8 @@ static void print_mmcinfo(struct mmc *mmc)
>>  	if (!IS_SD(mmc) && mmc->version >= MMC_VERSION_4_41) {
>>  		bool has_enh = (mmc->part_support & ENHNCD_SUPPORT) != 0;
>>  		bool usr_enh = has_enh && (mmc->part_attr & EXT_CSD_ENH_USR);
>> +		u8 wp, ext_csd[MMC_MAX_BLOCK_LEN];
>> +		int ret;
>>
>>  #if CONFIG_IS_ENABLED(MMC_HW_PARTITIONING)
>>  		puts("HC WP Group Size: ");
>> @@ -90,6 +92,28 @@ static void print_mmcinfo(struct mmc *mmc)
>>  					putc('\n');
>>  			}
>>  		}
>> +		ret = mmc_send_ext_csd(mmc, ext_csd);
>> +		if (ret)
>> +			return;
>
> Is it really needed to call mmc_send_ext_csd() at here.
> ext_csd register value was already read somewhere.

struct mmc does not contain the extended CSD. Performancewise it does
not make sense to carry a copy which would have to be updated after each
command that could potentially update it like SWITCH.

The call to mmc_send_ext_csd() does not lead to any visible performance
issue here.

Best regards

Heinrich

>
> Best Regards,
> Jaehoon Chung
>
>> +		wp = ext_csd[EXT_CSD_BOOT_WP_STATUS];
>> +		for (i = 0; i < 2; ++i) {
>> +			printf("Boot area %d is ", i);
>> +			switch (wp & 3) {
>> +			case 0:
>> +				printf("not write protected\n");
>> +				break;
>> +			case 1:
>> +				printf("power on protected\n");
>> +				break;
>> +			case 2:
>> +				printf("permanently protected\n");
>> +				break;
>> +			default:
>> +				printf("in reserved protection state\n");
>> +				break;
>> +			}
>> +			wp >>= 2;
>> +		}
>>  	}
>>  }
>>  static struct mmc *init_mmc_device(int dev, bool force_init)
>> --
>> 2.25.1
>>
>>
>>
>

[PATCH 3/4] cmd: mmc: display write protect state of boot partition

[Jaehoon Chung]

On 4/1/20 3:05 PM, Heinrich Schuchardt wrote:
> On 4/1/20 5:04 AM, Jaehoon Chung wrote:
>> Hi,
>>
>> On 3/30/20 2:24 PM, Heinrich Schuchardt wrote:
>>> Boot partitions of eMMC devices can be power on or permanently write
>>> protected. Let the 'mmc info' command display the protection state.
>>>
>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>>> ---
>>>  cmd/mmc.c | 24 ++++++++++++++++++++++++
>>>  1 file changed, 24 insertions(+)
>>>
>>> diff --git a/cmd/mmc.c b/cmd/mmc.c
>>> index 6f3cb85cc0..d62c85e439 100644
>>> --- a/cmd/mmc.c
>>> +++ b/cmd/mmc.c
>>> @@ -54,6 +54,8 @@ static void print_mmcinfo(struct mmc *mmc)
>>>  	if (!IS_SD(mmc) && mmc->version >= MMC_VERSION_4_41) {
>>>  		bool has_enh = (mmc->part_support & ENHNCD_SUPPORT) != 0;
>>>  		bool usr_enh = has_enh && (mmc->part_attr & EXT_CSD_ENH_USR);
>>> +		u8 wp, ext_csd[MMC_MAX_BLOCK_LEN];
>>> +		int ret;
>>>
>>>  #if CONFIG_IS_ENABLED(MMC_HW_PARTITIONING)
>>>  		puts("HC WP Group Size: ");
>>> @@ -90,6 +92,28 @@ static void print_mmcinfo(struct mmc *mmc)
>>>  					putc('\n');
>>>  			}
>>>  		}
>>> +		ret = mmc_send_ext_csd(mmc, ext_csd);
>>> +		if (ret)
>>> +			return;
>>
>> Is it really needed to call mmc_send_ext_csd() at here.
>> ext_csd register value was already read somewhere.
> 
> struct mmc does not contain the extended CSD. Performancewise it does
> not make sense to carry a copy which would have to be updated after each
> command that could potentially update it like SWITCH.

Why doesn't make sense to carry a copy? Its value can be updated whenever it's changed.
If someone want to set "write protect", then it has to follow sequence.
read ext_csd -> check value -> send_switch command 
Then it's possible to keep its value..

And some values are one-time programmable. it's not efficient about sending command to read again.

Best Regards,
Jaehoon Chung

> 
> The call to mmc_send_ext_csd() does not lead to any visible performance
> issue here.
> 
> Best regards
> 
> Heinrich
> 
>>
>> Best Regards,
>> Jaehoon Chung
>>
>>> +		wp = ext_csd[EXT_CSD_BOOT_WP_STATUS];
>>> +		for (i = 0; i < 2; ++i) {
>>> +			printf("Boot area %d is ", i);
>>> +			switch (wp & 3) {
>>> +			case 0:
>>> +				printf("not write protected\n");
>>> +				break;
>>> +			case 1:
>>> +				printf("power on protected\n");
>>> +				break;
>>> +			case 2:
>>> +				printf("permanently protected\n");
>>> +				break;
>>> +			default:
>>> +				printf("in reserved protection state\n");
>>> +				break;
>>> +			}
>>> +			wp >>= 2;
>>> +		}
>>>  	}
>>>  }
>>>  static struct mmc *init_mmc_device(int dev, bool force_init)
>>> --
>>> 2.25.1
>>>
>>>
>>>
>>
> 
> 
>