linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] mmc-utils: Add eMMC erase command support
@ 2021-03-24 16:44 Bean Huo
  2021-03-30 12:39 ` Ulf Hansson
  0 siblings, 1 reply; 5+ messages in thread
From: Bean Huo @ 2021-03-24 16:44 UTC (permalink / raw)
  To: beanhuo, kenny.gibbons, kimito.sakata, rkamdar, chris, ulf.hansson
  Cc: linux-mmc, linux-kernel

From: Kimito Sakata <kimito.sakata@oracle.com>

we have been using this erase feature for a while, but it is
still not merged into the upstream mmc-utils. Especially, for
the customer, every time when they update the mmc-utils, they
should re-install this patch again, let's try to make this
erase command upstreamed in the mmc-utils.

Co-developed-by: Bean Huo <beanhuo@micron.com>
Signed-off-by: Bean Huo <beanhuo@micron.com>
Reviewed-by: Kenneth Gibbons <kenny.gibbons@oracle.com>
---

Hi Ulf,
Please help us review this mmc-utils patch, and if agree, it is
possible to make it merged in the official mmc-utils.

Changelog:

V1--V2:
    1. refactor Kimito's original patch
    2. change to use MMC_IOC_MULTI_CMD
    3. add checkup if eMMC devie supports secure erase/trim

---
 mmc.c      |   8 ++++
 mmc.h      |  13 +++++-
 mmc_cmds.c | 135 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 mmc_cmds.h |   1 +
 4 files changed, 156 insertions(+), 1 deletion(-)

diff --git a/mmc.c b/mmc.c
index 50c9c9e..cb29a65 100644
--- a/mmc.c
+++ b/mmc.c
@@ -215,6 +215,14 @@ static struct Command commands[] = {
 		"Run Field Firmware Update with <image name> on <device>.\n",
 	  NULL
 	},
+	{ do_erase, -4,
+	"erase", "<type> " "<start address> " "<end address> " "<device>\n"
+		"Send Erase CMD38 with specific argument to the <device>\n\n"
+		"NOTE!: This will delete all user data in the specified region of the device\n"
+		"<type> must be: legacy | discard | secure-erase | "
+		"secure-trim1 | secure-trim2 | trim \n",
+	NULL
+	},
 	{ 0, 0, 0, 0 }
 };
 
diff --git a/mmc.h b/mmc.h
index 648fb26..90b7fb5 100644
--- a/mmc.h
+++ b/mmc.h
@@ -34,7 +34,15 @@
 #define MMC_SET_WRITE_PROT	28    /* ac   [31:0] data addr   R1b */
 #define MMC_CLEAR_WRITE_PROT	29    /* ac   [31:0] data addr   R1b */
 #define MMC_SEND_WRITE_PROT_TYPE 31   /* ac   [31:0] data addr   R1  */
-
+#define MMC_ERASE_GROUP_START	35    /* ac   [31:0] data addr   R1  */
+#define MMC_ERASE_GROUP_END	36    /* ac   [31:0] data addr   R1  */
+#define MMC_ERASE		38    /* ac   [31] Secure request
+					      [30:16] set to 0
+					      [15] Force Garbage Collect request
+					      [14:2] set to 0
+					      [1] Discard Enable
+					      [0] Identify Write Blocks for
+					      Erase (or TRIM Enable)  R1b */
 /*
  * EXT_CSD fields
  */
@@ -61,6 +69,7 @@
 #define EXT_CSD_CACHE_SIZE_2		251
 #define EXT_CSD_CACHE_SIZE_1		250
 #define EXT_CSD_CACHE_SIZE_0		249
+#define EXT_CSD_SEC_FEATURE_SUPPORT	231
 #define EXT_CSD_BOOT_INFO		228	/* R/W */
 #define EXT_CSD_HC_ERASE_GRP_SIZE	224
 #define EXT_CSD_HC_WP_GRP_SIZE		221
@@ -177,6 +186,8 @@
 #define EXT_CSD_REV_V4_2		2
 #define EXT_CSD_REV_V4_1		1
 #define EXT_CSD_REV_V4_0		0
+#define EXT_CSD_SEC_GB_CL_EN		(1<<4)
+#define EXT_CSD_SEC_ER_EN		(1<<0)
 
 
 /* From kernel linux/mmc/core.h */
diff --git a/mmc_cmds.c b/mmc_cmds.c
index fb37189..17986e3 100644
--- a/mmc_cmds.c
+++ b/mmc_cmds.c
@@ -2435,6 +2435,141 @@ int do_cache_dis(int nargs, char **argv)
 	return do_cache_ctrl(0, nargs, argv);
 }
 
+static int erase(int dev_fd, __u32 argin, __u32 start, __u32 end)
+{
+#ifndef MMC_IOC_MULTI_CMD
+	fprintf(stderr, "mmc-utils has been compiled without MMC_IOC_MULTI_CMD"
+			" support, needed by erase.\n");
+	return -ENOTSUP;
+#else
+	int ret = 0;
+	struct mmc_ioc_multi_cmd *multi_cmd;
+
+	multi_cmd = calloc(1, sizeof(struct mmc_ioc_multi_cmd) +
+			   3 * sizeof(struct mmc_ioc_cmd));
+	if (!multi_cmd) {
+		perror("Failed to allocate memory");
+		return -ENOMEM;
+	}
+
+	multi_cmd->num_of_cmds = 3;
+	/* Set erase start address */
+	multi_cmd->cmds[0].opcode = MMC_ERASE_GROUP_START;
+	multi_cmd->cmds[0].arg = start;
+	multi_cmd->cmds[0].flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_AC;
+	multi_cmd->cmds[0].write_flag = 1;
+
+	/* Set erase end address */
+	multi_cmd->cmds[1].opcode = MMC_ERASE_GROUP_END;
+	multi_cmd->cmds[1].arg = end;
+	multi_cmd->cmds[1].flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_AC;
+	multi_cmd->cmds[1].write_flag = 1;
+
+	/* Send Erase Command */
+	multi_cmd->cmds[2].opcode = MMC_ERASE;
+	multi_cmd->cmds[2].arg = argin;
+	multi_cmd->cmds[2].cmd_timeout_ms = 300*255*255;
+	multi_cmd->cmds[2].flags = MMC_RSP_SPI_R1B | MMC_RSP_R1B | MMC_CMD_AC;
+	multi_cmd->cmds[2].write_flag = 1;
+
+	/* send erase cmd with multi-cmd */
+	ret = ioctl(dev_fd, MMC_IOC_MULTI_CMD, multi_cmd);
+	if (ret)
+		perror("Erase multi-cmd ioctl");
+
+	free(multi_cmd);
+	return ret;
+#endif
+}
+
+int do_erase(int nargs, char **argv)
+{
+	int dev_fd, ret;
+	char *print_str;
+	char **eptr = NULL;
+	__u8 ext_csd[512], checkup_mask = 0;
+	__u32 arg, start, end;
+
+	if (nargs != 5) {
+		fprintf(stderr, "Usage: erase <type> <start addr> <end addr> </path/to/mmcblkX>\n");
+		exit(1);
+	}
+
+	if (strstr(argv[2], "0x") || strstr(argv[2], "0X"))
+		start = strtol(argv[2], eptr, 16);
+	else
+		start = strtol(argv[2], eptr, 10);
+
+	if (strstr(argv[3], "0x") || strstr(argv[3], "0X"))
+		end = strtol(argv[3], eptr, 16);
+	else
+		end = strtol(argv[3], eptr, 10);
+
+	if (end < start) {
+		fprintf(stderr, "erase start [0x%08x] > erase end [0x%08x]\n",
+			start, end);
+		exit(1);
+	}
+
+	if (strcmp(argv[1], "legacy") == 0) {
+		arg = 0x00000000;
+		print_str = "Legacy Erase";
+	} else if (strcmp(argv[1], "discard") == 0) {
+		arg = 0x00000003;
+		print_str = "Discard";
+	} else if (strcmp(argv[1], "secure-erase") == 0) {
+		print_str = "Secure Erase";
+		checkup_mask = EXT_CSD_SEC_ER_EN;
+		arg = 0x80000000;
+	} else if (strcmp(argv[1], "secure-trim1") == 0) {
+		print_str = "Secure Trim Step 1";
+		checkup_mask = EXT_CSD_SEC_ER_EN | EXT_CSD_SEC_GB_CL_EN;
+		arg = 0x80000001;
+	} else if (strcmp(argv[1], "secure-trim2") == 0) {
+		print_str = "Secure Trim Step 2";
+		checkup_mask = EXT_CSD_SEC_ER_EN | EXT_CSD_SEC_GB_CL_EN;
+		arg = 0x80008000;
+	} else if (strcmp(argv[1], "trim") == 0) {
+		print_str = "Trim";
+		checkup_mask = EXT_CSD_SEC_GB_CL_EN;
+		arg = 0x00000001;
+	} else {
+		fprintf(stderr, "Unknown erase type: %s\n", argv[1]);
+		exit(1);
+	}
+
+	dev_fd = open(argv[4], O_RDWR);
+	if (dev_fd < 0) {
+		perror(argv[4]);
+		exit(1);
+	}
+
+	if (checkup_mask) {
+		ret = read_extcsd(dev_fd, ext_csd);
+		if (ret) {
+			fprintf(stderr, "Could not read EXT_CSD from %s\n",
+				argv[4]);
+			goto out;
+		}
+		if ((checkup_mask & ext_csd[EXT_CSD_SEC_FEATURE_SUPPORT]) !=
+								checkup_mask) {
+			fprintf(stderr, "%s is not supported in %s\n",
+				print_str, argv[4]);
+			ret = -ENOTSUP;
+			goto out;
+		}
+
+	}
+	printf("Executing %s from 0x%08x to 0x%08x\n", print_str, start, end);
+
+	ret = erase(dev_fd, arg, start, end);
+out:
+	printf(" %s %s!\n\n", print_str, ret ? "Failed" : "Succeed");
+	close(dev_fd);
+	return ret;
+}
+
+
 int do_ffu(int nargs, char **argv)
 {
 #ifndef MMC_IOC_MULTI_CMD
diff --git a/mmc_cmds.h b/mmc_cmds.h
index 9d3246c..8331ab2 100644
--- a/mmc_cmds.h
+++ b/mmc_cmds.h
@@ -45,3 +45,4 @@ int do_ffu(int nargs, char **argv);
 int do_read_scr(int argc, char **argv);
 int do_read_cid(int argc, char **argv);
 int do_read_csd(int argc, char **argv);
+int do_erase(int nargs, char **argv);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] mmc-utils: Add eMMC erase command support
  2021-03-24 16:44 [PATCH v2] mmc-utils: Add eMMC erase command support Bean Huo
@ 2021-03-30 12:39 ` Ulf Hansson
  2021-03-30 21:36   ` [External] : " kimito.sakata
  0 siblings, 1 reply; 5+ messages in thread
From: Ulf Hansson @ 2021-03-30 12:39 UTC (permalink / raw)
  To: Bean Huo
  Cc: Bean Huo (beanhuo),
	kenny.gibbons, kimito.sakata, rkamdar, Chris Ball, linux-mmc,
	Linux Kernel Mailing List

On Wed, 24 Mar 2021 at 17:45, Bean Huo <huobean@gmail.com> wrote:
>
> From: Kimito Sakata <kimito.sakata@oracle.com>
>
> we have been using this erase feature for a while, but it is
> still not merged into the upstream mmc-utils. Especially, for
> the customer, every time when they update the mmc-utils, they
> should re-install this patch again, let's try to make this
> erase command upstreamed in the mmc-utils.
>
> Co-developed-by: Bean Huo <beanhuo@micron.com>
> Signed-off-by: Bean Huo <beanhuo@micron.com>
> Reviewed-by: Kenneth Gibbons <kenny.gibbons@oracle.com>
> ---
>
> Hi Ulf,
> Please help us review this mmc-utils patch, and if agree, it is
> possible to make it merged in the official mmc-utils.
>
> Changelog:
>
> V1--V2:
>     1. refactor Kimito's original patch
>     2. change to use MMC_IOC_MULTI_CMD
>     3. add checkup if eMMC devie supports secure erase/trim
>
> ---
>  mmc.c      |   8 ++++
>  mmc.h      |  13 +++++-
>  mmc_cmds.c | 135 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  mmc_cmds.h |   1 +
>  4 files changed, 156 insertions(+), 1 deletion(-)
>
> diff --git a/mmc.c b/mmc.c
> index 50c9c9e..cb29a65 100644
> --- a/mmc.c
> +++ b/mmc.c
> @@ -215,6 +215,14 @@ static struct Command commands[] = {
>                 "Run Field Firmware Update with <image name> on <device>.\n",
>           NULL
>         },
> +       { do_erase, -4,
> +       "erase", "<type> " "<start address> " "<end address> " "<device>\n"
> +               "Send Erase CMD38 with specific argument to the <device>\n\n"
> +               "NOTE!: This will delete all user data in the specified region of the device\n"
> +               "<type> must be: legacy | discard | secure-erase | "
> +               "secure-trim1 | secure-trim2 | trim \n",
> +       NULL
> +       },
>         { 0, 0, 0, 0 }
>  };
>
> diff --git a/mmc.h b/mmc.h
> index 648fb26..90b7fb5 100644
> --- a/mmc.h
> +++ b/mmc.h
> @@ -34,7 +34,15 @@
>  #define MMC_SET_WRITE_PROT     28    /* ac   [31:0] data addr   R1b */
>  #define MMC_CLEAR_WRITE_PROT   29    /* ac   [31:0] data addr   R1b */
>  #define MMC_SEND_WRITE_PROT_TYPE 31   /* ac   [31:0] data addr   R1  */
> -
> +#define MMC_ERASE_GROUP_START  35    /* ac   [31:0] data addr   R1  */
> +#define MMC_ERASE_GROUP_END    36    /* ac   [31:0] data addr   R1  */
> +#define MMC_ERASE              38    /* ac   [31] Secure request
> +                                             [30:16] set to 0
> +                                             [15] Force Garbage Collect request
> +                                             [14:2] set to 0
> +                                             [1] Discard Enable
> +                                             [0] Identify Write Blocks for
> +                                             Erase (or TRIM Enable)  R1b */
>  /*
>   * EXT_CSD fields
>   */
> @@ -61,6 +69,7 @@
>  #define EXT_CSD_CACHE_SIZE_2           251
>  #define EXT_CSD_CACHE_SIZE_1           250
>  #define EXT_CSD_CACHE_SIZE_0           249
> +#define EXT_CSD_SEC_FEATURE_SUPPORT    231
>  #define EXT_CSD_BOOT_INFO              228     /* R/W */
>  #define EXT_CSD_HC_ERASE_GRP_SIZE      224
>  #define EXT_CSD_HC_WP_GRP_SIZE         221
> @@ -177,6 +186,8 @@
>  #define EXT_CSD_REV_V4_2               2
>  #define EXT_CSD_REV_V4_1               1
>  #define EXT_CSD_REV_V4_0               0
> +#define EXT_CSD_SEC_GB_CL_EN           (1<<4)
> +#define EXT_CSD_SEC_ER_EN              (1<<0)
>
>
>  /* From kernel linux/mmc/core.h */
> diff --git a/mmc_cmds.c b/mmc_cmds.c
> index fb37189..17986e3 100644
> --- a/mmc_cmds.c
> +++ b/mmc_cmds.c
> @@ -2435,6 +2435,141 @@ int do_cache_dis(int nargs, char **argv)
>         return do_cache_ctrl(0, nargs, argv);
>  }
>
> +static int erase(int dev_fd, __u32 argin, __u32 start, __u32 end)
> +{
> +#ifndef MMC_IOC_MULTI_CMD

In kernel v4.4 we added the multi cmd support, which is quite some
time ago. So, I think it's time to drop these ifdef hackary from the
userland tool. At least, we shouldn't need it for new kinds of
features that we add.

> +       fprintf(stderr, "mmc-utils has been compiled without MMC_IOC_MULTI_CMD"
> +                       " support, needed by erase.\n");
> +       return -ENOTSUP;
> +#else
> +       int ret = 0;
> +       struct mmc_ioc_multi_cmd *multi_cmd;
> +

[...]

Kind regards
Uffe

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [External] : Re: [PATCH v2] mmc-utils: Add eMMC erase command support
  2021-03-30 12:39 ` Ulf Hansson
@ 2021-03-30 21:36   ` kimito.sakata
  2021-03-31  9:30     ` Ulf Hansson
  0 siblings, 1 reply; 5+ messages in thread
From: kimito.sakata @ 2021-03-30 21:36 UTC (permalink / raw)
  To: Ulf Hansson, Bean Huo
  Cc: Bean Huo (beanhuo),
	kenny.gibbons, rkamdar, Chris Ball, linux-mmc,
	Linux Kernel Mailing List



On 3/30/2021 6:39 AM, Ulf Hansson wrote:
> On Wed, 24 Mar 2021 at 17:45, Bean Huo <huobean@gmail.com> wrote:
>> From: Kimito Sakata <kimito.sakata@oracle.com>
>>
>> we have been using this erase feature for a while, but it is
>> still not merged into the upstream mmc-utils. Especially, for
>> the customer, every time when they update the mmc-utils, they
>> should re-install this patch again, let's try to make this
>> erase command upstreamed in the mmc-utils.
>>
>> Co-developed-by: Bean Huo <beanhuo@micron.com>
>> Signed-off-by: Bean Huo <beanhuo@micron.com>
>> Reviewed-by: Kenneth Gibbons <kenny.gibbons@oracle.com>
>> ---
>>
>> Hi Ulf,
>> Please help us review this mmc-utils patch, and if agree, it is
>> possible to make it merged in the official mmc-utils.
>>
>> Changelog:
>>
>> V1--V2:
>>      1. refactor Kimito's original patch
>>      2. change to use MMC_IOC_MULTI_CMD
>>      3. add checkup if eMMC devie supports secure erase/trim
>>
>> ---
>>   mmc.c      |   8 ++++
>>   mmc.h      |  13 +++++-
>>   mmc_cmds.c | 135 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   mmc_cmds.h |   1 +
>>   4 files changed, 156 insertions(+), 1 deletion(-)
>>
>> diff --git a/mmc.c b/mmc.c
>> index 50c9c9e..cb29a65 100644
>> --- a/mmc.c
>> +++ b/mmc.c
>> @@ -215,6 +215,14 @@ static struct Command commands[] = {
>>                  "Run Field Firmware Update with <image name> on <device>.\n",
>>            NULL
>>          },
>> +       { do_erase, -4,
>> +       "erase", "<type> " "<start address> " "<end address> " "<device>\n"
>> +               "Send Erase CMD38 with specific argument to the <device>\n\n"
>> +               "NOTE!: This will delete all user data in the specified region of the device\n"
>> +               "<type> must be: legacy | discard | secure-erase | "
>> +               "secure-trim1 | secure-trim2 | trim \n",
>> +       NULL
>> +       },
>>          { 0, 0, 0, 0 }
>>   };
>>
>> diff --git a/mmc.h b/mmc.h
>> index 648fb26..90b7fb5 100644
>> --- a/mmc.h
>> +++ b/mmc.h
>> @@ -34,7 +34,15 @@
>>   #define MMC_SET_WRITE_PROT     28    /* ac   [31:0] data addr   R1b */
>>   #define MMC_CLEAR_WRITE_PROT   29    /* ac   [31:0] data addr   R1b */
>>   #define MMC_SEND_WRITE_PROT_TYPE 31   /* ac   [31:0] data addr   R1  */
>> -
>> +#define MMC_ERASE_GROUP_START  35    /* ac   [31:0] data addr   R1  */
>> +#define MMC_ERASE_GROUP_END    36    /* ac   [31:0] data addr   R1  */
>> +#define MMC_ERASE              38    /* ac   [31] Secure request
>> +                                             [30:16] set to 0
>> +                                             [15] Force Garbage Collect request
>> +                                             [14:2] set to 0
>> +                                             [1] Discard Enable
>> +                                             [0] Identify Write Blocks for
>> +                                             Erase (or TRIM Enable)  R1b */
>>   /*
>>    * EXT_CSD fields
>>    */
>> @@ -61,6 +69,7 @@
>>   #define EXT_CSD_CACHE_SIZE_2           251
>>   #define EXT_CSD_CACHE_SIZE_1           250
>>   #define EXT_CSD_CACHE_SIZE_0           249
>> +#define EXT_CSD_SEC_FEATURE_SUPPORT    231
>>   #define EXT_CSD_BOOT_INFO              228     /* R/W */
>>   #define EXT_CSD_HC_ERASE_GRP_SIZE      224
>>   #define EXT_CSD_HC_WP_GRP_SIZE         221
>> @@ -177,6 +186,8 @@
>>   #define EXT_CSD_REV_V4_2               2
>>   #define EXT_CSD_REV_V4_1               1
>>   #define EXT_CSD_REV_V4_0               0
>> +#define EXT_CSD_SEC_GB_CL_EN           (1<<4)
>> +#define EXT_CSD_SEC_ER_EN              (1<<0)
>>
>>
>>   /* From kernel linux/mmc/core.h */
>> diff --git a/mmc_cmds.c b/mmc_cmds.c
>> index fb37189..17986e3 100644
>> --- a/mmc_cmds.c
>> +++ b/mmc_cmds.c
>> @@ -2435,6 +2435,141 @@ int do_cache_dis(int nargs, char **argv)
>>          return do_cache_ctrl(0, nargs, argv);
>>   }
>>
>> +static int erase(int dev_fd, __u32 argin, __u32 start, __u32 end)
>> +{
>> +#ifndef MMC_IOC_MULTI_CMD
> In kernel v4.4 we added the multi cmd support, which is quite some
> time ago. So, I think it's time to drop these ifdef hackary from the
> userland tool. At least, we shouldn't need it for new kinds of
> features that we add.
Ulf
Do you want us to take out the  MMC_IOC_MULTI_CMD ifdef and resubmit?
Kimito

>
>> +       fprintf(stderr, "mmc-utils has been compiled without MMC_IOC_MULTI_CMD"
>> +                       " support, needed by erase.\n");
>> +       return -ENOTSUP;
>> +#else
>> +       int ret = 0;
>> +       struct mmc_ioc_multi_cmd *multi_cmd;
>> +
> [...]
>
> Kind regards
> Uffe


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [External] : Re: [PATCH v2] mmc-utils: Add eMMC erase command support
  2021-03-30 21:36   ` [External] : " kimito.sakata
@ 2021-03-31  9:30     ` Ulf Hansson
  2021-03-31 14:30       ` kimito.sakata
  0 siblings, 1 reply; 5+ messages in thread
From: Ulf Hansson @ 2021-03-31  9:30 UTC (permalink / raw)
  To: kimito.sakata
  Cc: Bean Huo, Bean Huo (beanhuo),
	kenny.gibbons, rkamdar, Chris Ball, linux-mmc,
	Linux Kernel Mailing List

On Tue, 30 Mar 2021 at 23:36, <kimito.sakata@oracle.com> wrote:
>
>
>
> On 3/30/2021 6:39 AM, Ulf Hansson wrote:
> > On Wed, 24 Mar 2021 at 17:45, Bean Huo <huobean@gmail.com> wrote:
> >> From: Kimito Sakata <kimito.sakata@oracle.com>
> >>
> >> we have been using this erase feature for a while, but it is
> >> still not merged into the upstream mmc-utils. Especially, for
> >> the customer, every time when they update the mmc-utils, they
> >> should re-install this patch again, let's try to make this
> >> erase command upstreamed in the mmc-utils.
> >>
> >> Co-developed-by: Bean Huo <beanhuo@micron.com>
> >> Signed-off-by: Bean Huo <beanhuo@micron.com>
> >> Reviewed-by: Kenneth Gibbons <kenny.gibbons@oracle.com>
> >> ---
> >>
> >> Hi Ulf,
> >> Please help us review this mmc-utils patch, and if agree, it is
> >> possible to make it merged in the official mmc-utils.
> >>
> >> Changelog:
> >>
> >> V1--V2:
> >>      1. refactor Kimito's original patch
> >>      2. change to use MMC_IOC_MULTI_CMD
> >>      3. add checkup if eMMC devie supports secure erase/trim
> >>
> >> ---
> >>   mmc.c      |   8 ++++
> >>   mmc.h      |  13 +++++-
> >>   mmc_cmds.c | 135 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> >>   mmc_cmds.h |   1 +
> >>   4 files changed, 156 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/mmc.c b/mmc.c
> >> index 50c9c9e..cb29a65 100644
> >> --- a/mmc.c
> >> +++ b/mmc.c
> >> @@ -215,6 +215,14 @@ static struct Command commands[] = {
> >>                  "Run Field Firmware Update with <image name> on <device>.\n",
> >>            NULL
> >>          },
> >> +       { do_erase, -4,
> >> +       "erase", "<type> " "<start address> " "<end address> " "<device>\n"
> >> +               "Send Erase CMD38 with specific argument to the <device>\n\n"
> >> +               "NOTE!: This will delete all user data in the specified region of the device\n"
> >> +               "<type> must be: legacy | discard | secure-erase | "
> >> +               "secure-trim1 | secure-trim2 | trim \n",
> >> +       NULL
> >> +       },
> >>          { 0, 0, 0, 0 }
> >>   };
> >>
> >> diff --git a/mmc.h b/mmc.h
> >> index 648fb26..90b7fb5 100644
> >> --- a/mmc.h
> >> +++ b/mmc.h
> >> @@ -34,7 +34,15 @@
> >>   #define MMC_SET_WRITE_PROT     28    /* ac   [31:0] data addr   R1b */
> >>   #define MMC_CLEAR_WRITE_PROT   29    /* ac   [31:0] data addr   R1b */
> >>   #define MMC_SEND_WRITE_PROT_TYPE 31   /* ac   [31:0] data addr   R1  */
> >> -
> >> +#define MMC_ERASE_GROUP_START  35    /* ac   [31:0] data addr   R1  */
> >> +#define MMC_ERASE_GROUP_END    36    /* ac   [31:0] data addr   R1  */
> >> +#define MMC_ERASE              38    /* ac   [31] Secure request
> >> +                                             [30:16] set to 0
> >> +                                             [15] Force Garbage Collect request
> >> +                                             [14:2] set to 0
> >> +                                             [1] Discard Enable
> >> +                                             [0] Identify Write Blocks for
> >> +                                             Erase (or TRIM Enable)  R1b */
> >>   /*
> >>    * EXT_CSD fields
> >>    */
> >> @@ -61,6 +69,7 @@
> >>   #define EXT_CSD_CACHE_SIZE_2           251
> >>   #define EXT_CSD_CACHE_SIZE_1           250
> >>   #define EXT_CSD_CACHE_SIZE_0           249
> >> +#define EXT_CSD_SEC_FEATURE_SUPPORT    231
> >>   #define EXT_CSD_BOOT_INFO              228     /* R/W */
> >>   #define EXT_CSD_HC_ERASE_GRP_SIZE      224
> >>   #define EXT_CSD_HC_WP_GRP_SIZE         221
> >> @@ -177,6 +186,8 @@
> >>   #define EXT_CSD_REV_V4_2               2
> >>   #define EXT_CSD_REV_V4_1               1
> >>   #define EXT_CSD_REV_V4_0               0
> >> +#define EXT_CSD_SEC_GB_CL_EN           (1<<4)
> >> +#define EXT_CSD_SEC_ER_EN              (1<<0)
> >>
> >>
> >>   /* From kernel linux/mmc/core.h */
> >> diff --git a/mmc_cmds.c b/mmc_cmds.c
> >> index fb37189..17986e3 100644
> >> --- a/mmc_cmds.c
> >> +++ b/mmc_cmds.c
> >> @@ -2435,6 +2435,141 @@ int do_cache_dis(int nargs, char **argv)
> >>          return do_cache_ctrl(0, nargs, argv);
> >>   }
> >>
> >> +static int erase(int dev_fd, __u32 argin, __u32 start, __u32 end)
> >> +{
> >> +#ifndef MMC_IOC_MULTI_CMD
> > In kernel v4.4 we added the multi cmd support, which is quite some
> > time ago. So, I think it's time to drop these ifdef hackary from the
> > userland tool. At least, we shouldn't need it for new kinds of
> > features that we add.
> Ulf
> Do you want us to take out the  MMC_IOC_MULTI_CMD ifdef and resubmit?

Yes, please.

Moreover, we should probably also remove all the other #ifndef
MMC_IOC_MULTI_CMD hacks that we currently have in the code. But that's
a separate patch.

> Kimito
>
> >
> >> +       fprintf(stderr, "mmc-utils has been compiled without MMC_IOC_MULTI_CMD"
> >> +                       " support, needed by erase.\n");
> >> +       return -ENOTSUP;
> >> +#else
> >> +       int ret = 0;
> >> +       struct mmc_ioc_multi_cmd *multi_cmd;
> >> +
> > [...]

Kind regards
Uffe

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [External] : Re: [PATCH v2] mmc-utils: Add eMMC erase command support
  2021-03-31  9:30     ` Ulf Hansson
@ 2021-03-31 14:30       ` kimito.sakata
  0 siblings, 0 replies; 5+ messages in thread
From: kimito.sakata @ 2021-03-31 14:30 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Bean Huo, Bean Huo (beanhuo),
	kenny.gibbons, rkamdar, Chris Ball, linux-mmc,
	Linux Kernel Mailing List

OK Ulf. I'll make the changes and resubmit.
Thanks
Kimito

On 3/31/2021 3:30 AM, Ulf Hansson wrote:
> On Tue, 30 Mar 2021 at 23:36, <kimito.sakata@oracle.com> wrote:
>>
>>
>> On 3/30/2021 6:39 AM, Ulf Hansson wrote:
>>> On Wed, 24 Mar 2021 at 17:45, Bean Huo <huobean@gmail.com> wrote:
>>>> From: Kimito Sakata <kimito.sakata@oracle.com>
>>>>
>>>> we have been using this erase feature for a while, but it is
>>>> still not merged into the upstream mmc-utils. Especially, for
>>>> the customer, every time when they update the mmc-utils, they
>>>> should re-install this patch again, let's try to make this
>>>> erase command upstreamed in the mmc-utils.
>>>>
>>>> Co-developed-by: Bean Huo <beanhuo@micron.com>
>>>> Signed-off-by: Bean Huo <beanhuo@micron.com>
>>>> Reviewed-by: Kenneth Gibbons <kenny.gibbons@oracle.com>
>>>> ---
>>>>
>>>> Hi Ulf,
>>>> Please help us review this mmc-utils patch, and if agree, it is
>>>> possible to make it merged in the official mmc-utils.
>>>>
>>>> Changelog:
>>>>
>>>> V1--V2:
>>>>       1. refactor Kimito's original patch
>>>>       2. change to use MMC_IOC_MULTI_CMD
>>>>       3. add checkup if eMMC devie supports secure erase/trim
>>>>
>>>> ---
>>>>    mmc.c      |   8 ++++
>>>>    mmc.h      |  13 +++++-
>>>>    mmc_cmds.c | 135 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>    mmc_cmds.h |   1 +
>>>>    4 files changed, 156 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/mmc.c b/mmc.c
>>>> index 50c9c9e..cb29a65 100644
>>>> --- a/mmc.c
>>>> +++ b/mmc.c
>>>> @@ -215,6 +215,14 @@ static struct Command commands[] = {
>>>>                   "Run Field Firmware Update with <image name> on <device>.\n",
>>>>             NULL
>>>>           },
>>>> +       { do_erase, -4,
>>>> +       "erase", "<type> " "<start address> " "<end address> " "<device>\n"
>>>> +               "Send Erase CMD38 with specific argument to the <device>\n\n"
>>>> +               "NOTE!: This will delete all user data in the specified region of the device\n"
>>>> +               "<type> must be: legacy | discard | secure-erase | "
>>>> +               "secure-trim1 | secure-trim2 | trim \n",
>>>> +       NULL
>>>> +       },
>>>>           { 0, 0, 0, 0 }
>>>>    };
>>>>
>>>> diff --git a/mmc.h b/mmc.h
>>>> index 648fb26..90b7fb5 100644
>>>> --- a/mmc.h
>>>> +++ b/mmc.h
>>>> @@ -34,7 +34,15 @@
>>>>    #define MMC_SET_WRITE_PROT     28    /* ac   [31:0] data addr   R1b */
>>>>    #define MMC_CLEAR_WRITE_PROT   29    /* ac   [31:0] data addr   R1b */
>>>>    #define MMC_SEND_WRITE_PROT_TYPE 31   /* ac   [31:0] data addr   R1  */
>>>> -
>>>> +#define MMC_ERASE_GROUP_START  35    /* ac   [31:0] data addr   R1  */
>>>> +#define MMC_ERASE_GROUP_END    36    /* ac   [31:0] data addr   R1  */
>>>> +#define MMC_ERASE              38    /* ac   [31] Secure request
>>>> +                                             [30:16] set to 0
>>>> +                                             [15] Force Garbage Collect request
>>>> +                                             [14:2] set to 0
>>>> +                                             [1] Discard Enable
>>>> +                                             [0] Identify Write Blocks for
>>>> +                                             Erase (or TRIM Enable)  R1b */
>>>>    /*
>>>>     * EXT_CSD fields
>>>>     */
>>>> @@ -61,6 +69,7 @@
>>>>    #define EXT_CSD_CACHE_SIZE_2           251
>>>>    #define EXT_CSD_CACHE_SIZE_1           250
>>>>    #define EXT_CSD_CACHE_SIZE_0           249
>>>> +#define EXT_CSD_SEC_FEATURE_SUPPORT    231
>>>>    #define EXT_CSD_BOOT_INFO              228     /* R/W */
>>>>    #define EXT_CSD_HC_ERASE_GRP_SIZE      224
>>>>    #define EXT_CSD_HC_WP_GRP_SIZE         221
>>>> @@ -177,6 +186,8 @@
>>>>    #define EXT_CSD_REV_V4_2               2
>>>>    #define EXT_CSD_REV_V4_1               1
>>>>    #define EXT_CSD_REV_V4_0               0
>>>> +#define EXT_CSD_SEC_GB_CL_EN           (1<<4)
>>>> +#define EXT_CSD_SEC_ER_EN              (1<<0)
>>>>
>>>>
>>>>    /* From kernel linux/mmc/core.h */
>>>> diff --git a/mmc_cmds.c b/mmc_cmds.c
>>>> index fb37189..17986e3 100644
>>>> --- a/mmc_cmds.c
>>>> +++ b/mmc_cmds.c
>>>> @@ -2435,6 +2435,141 @@ int do_cache_dis(int nargs, char **argv)
>>>>           return do_cache_ctrl(0, nargs, argv);
>>>>    }
>>>>
>>>> +static int erase(int dev_fd, __u32 argin, __u32 start, __u32 end)
>>>> +{
>>>> +#ifndef MMC_IOC_MULTI_CMD
>>> In kernel v4.4 we added the multi cmd support, which is quite some
>>> time ago. So, I think it's time to drop these ifdef hackary from the
>>> userland tool. At least, we shouldn't need it for new kinds of
>>> features that we add.
>> Ulf
>> Do you want us to take out the  MMC_IOC_MULTI_CMD ifdef and resubmit?
> Yes, please.
>
> Moreover, we should probably also remove all the other #ifndef
> MMC_IOC_MULTI_CMD hacks that we currently have in the code. But that's
> a separate patch.
>
>> Kimito
>>
>>>> +       fprintf(stderr, "mmc-utils has been compiled without MMC_IOC_MULTI_CMD"
>>>> +                       " support, needed by erase.\n");
>>>> +       return -ENOTSUP;
>>>> +#else
>>>> +       int ret = 0;
>>>> +       struct mmc_ioc_multi_cmd *multi_cmd;
>>>> +
>>> [...]
> Kind regards
> Uffe


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-03-31 14:31 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-24 16:44 [PATCH v2] mmc-utils: Add eMMC erase command support Bean Huo
2021-03-30 12:39 ` Ulf Hansson
2021-03-30 21:36   ` [External] : " kimito.sakata
2021-03-31  9:30     ` Ulf Hansson
2021-03-31 14:30       ` kimito.sakata

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).