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

[Ulf Hansson]

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.

> +       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