Re: [External] : Re: [PATCH v2] mmc-utils: Add eMMC erase command support

kimito.sakata@xxxxxxxxxx · Wed, 31 Mar 2021 08:30:44 -0600

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@xxxxxxxxxx> wrote:


On 3/30/2021 6:39 AM, Ulf Hansson wrote:
On Wed, 24 Mar 2021 at 17:45, Bean Huo <huobean@xxxxxxxxx> wrote:
From: Kimito Sakata <kimito.sakata@xxxxxxxxxx>

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@xxxxxxxxxx>
Signed-off-by: Bean Huo <beanhuo@xxxxxxxxxx>
Reviewed-by: Kenneth Gibbons <kenny.gibbons@xxxxxxxxxx>
---

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







