Re: [External] : Re: [PATCH v2] mmc-utils: Add eMMC erase command support
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, wrote: On 3/30/2021 6:39 AM, Ulf Hansson wrote: On Wed, 24 Mar 2021 at 17:45, Bean Huo wrote: From: Kimito Sakata 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 Signed-off-by: Bean Huo Reviewed-by: Kenneth Gibbons --- 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 on .\n", NULL }, + { do_erase, -4, + "erase", " " " " " " "\n" + "Send Erase CMD38 with specific argument to the \n\n" + "NOTE!: This will delete all user data in the specified region of the device\n" + " 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_END36/* 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_SUPPORT231 #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
Re: [External] : Re: [PATCH v2] mmc-utils: Add eMMC erase command support
On Tue, 30 Mar 2021 at 23:36, wrote: > > > > On 3/30/2021 6:39 AM, Ulf Hansson wrote: > > On Wed, 24 Mar 2021 at 17:45, Bean Huo wrote: > >> From: Kimito Sakata > >> > >> 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 > >> Signed-off-by: Bean Huo > >> Reviewed-by: Kenneth Gibbons > >> --- > >> > >> 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 on > >> .\n", > >>NULL > >> }, > >> + { do_erase, -4, > >> + "erase", " " " " " " "\n" > >> + "Send Erase CMD38 with specific argument to the > >> \n\n" > >> + "NOTE!: This will delete all user data in the specified > >> region of the device\n" > >> + " 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_END36/* 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_SUPPORT231 > >> #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
Re: [External] : Re: [PATCH v2] mmc-utils: Add eMMC erase command support
On 3/30/2021 6:39 AM, Ulf Hansson wrote: On Wed, 24 Mar 2021 at 17:45, Bean Huo wrote: From: Kimito Sakata 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 Signed-off-by: Bean Huo Reviewed-by: Kenneth Gibbons --- 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 on .\n", NULL }, + { do_erase, -4, + "erase", " " " " " " "\n" + "Send Erase CMD38 with specific argument to the \n\n" + "NOTE!: This will delete all user data in the specified region of the device\n" + " 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_END36/* 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_SUPPORT231 #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