* [PATCH] drivers: mmc: support secure erase.
@ 2020-12-18 13:21 ` dennis
2020-12-21 23:05 ` Jaehoon Chung
0 siblings, 1 reply; 6+ messages in thread
From: dennis @ 2020-12-18 13:21 UTC (permalink / raw)
To: u-boot
According to the JEDEC Standard, eMMC v4.4 cards can supoort secure erase.
the first bit in ext_csd[EXT_CSD_SEC_FEATURE_SUPPORT] - EXT_CSD_SEC_ER_EN
is used to check if secure erase supoort by the device.
Signed-off-by: dennis <dennislaplacian1@gmail.com>
---
drivers/mmc/Kconfig | 8 ++++++++
drivers/mmc/mmc.c | 4 ++++
drivers/mmc/mmc_write.c | 16 +++++++++++++---
include/mmc.h | 5 ++++-
4 files changed, 29 insertions(+), 4 deletions(-)
diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
index 14d7913986..6105cbe960 100644
--- a/drivers/mmc/Kconfig
+++ b/drivers/mmc/Kconfig
@@ -18,6 +18,14 @@ config MMC_WRITE
help
Enable write access to MMC and SD Cards
+config MMC_SECURE_ERASE
+ bool "Enable secure erase for eMMC"
+ depends on MMC_WRITE
+ default n
+ help
+ This option replace the default erase with secure erase in eMMC v4.4
+ and above cards.
+
config MMC_BROKEN_CD
bool "Poll for broken card detection case"
help
diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index a6394bcf30..56b17070e7 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -2272,6 +2272,10 @@ static int mmc_startup_v4(struct mmc *mmc)
if ((capacity >> 20) > 2 * 1024)
mmc->capacity_user = capacity;
}
+#if CONFIG_IS_ENABLED(MMC_SECURE_ERASE)
+ if (mmc->version >= MMC_VERSION_4_4 && IS_MMC(mmc))
+ mmc->sec_feature_support = ext_csd[EXT_CSD_SEC_FEATURE_SUPPORT];
+#endif
if (mmc->version >= MMC_VERSION_4_5)
mmc->gen_cmd6_time = ext_csd[EXT_CSD_GENERIC_CMD6_TIME];
diff --git a/drivers/mmc/mmc_write.c b/drivers/mmc/mmc_write.c
index 6a4453ca02..0b720ccf7e 100644
--- a/drivers/mmc/mmc_write.c
+++ b/drivers/mmc/mmc_write.c
@@ -15,7 +15,7 @@
#include <linux/math64.h>
#include "mmc_private.h"
-static ulong mmc_erase_t(struct mmc *mmc, ulong start, lbaint_t blkcnt)
+static ulong mmc_erase_t(struct mmc *mmc, ulong start, lbaint_t blkcnt, uint arg)
{
struct mmc_cmd cmd;
ulong end;
@@ -52,7 +52,7 @@ static ulong mmc_erase_t(struct mmc *mmc, ulong start, lbaint_t blkcnt)
goto err_out;
cmd.cmdidx = MMC_CMD_ERASE;
- cmd.cmdarg = MMC_ERASE_ARG;
+ cmd.cmdarg = arg;
cmd.resp_type = MMC_RSP_R1b;
err = mmc_send_cmd(mmc, &cmd, NULL);
@@ -81,6 +81,7 @@ ulong mmc_berase(struct blk_desc *block_dev, lbaint_t start, lbaint_t blkcnt)
struct mmc *mmc = find_mmc_device(dev_num);
lbaint_t blk = 0, blk_r = 0;
int timeout_ms = 1000;
+ uint arg = MMC_ERASE_ARG;
if (!mmc)
return -1;
@@ -105,6 +106,15 @@ ulong mmc_berase(struct blk_desc *block_dev, lbaint_t start, lbaint_t blkcnt)
((start + blkcnt + mmc->erase_grp_size)
& ~(mmc->erase_grp_size - 1)) - 1);
+#if CONFIG_IS_ENABLED(MMC_SECURE_ERASE)
+ if (!(mmc->sec_feature_support & EXT_CSD_SEC_ER_EN)) {
+ printf("secure erase not supported on device\n"
+ "perform insecure erase\n");
+ } else {
+ arg = MMC_SECURE_ERASE_ARG;
+ }
+#endif
+
while (blk < blkcnt) {
if (IS_SD(mmc) && mmc->ssr.au) {
blk_r = ((blkcnt - blk) > mmc->ssr.au) ?
@@ -113,7 +123,7 @@ ulong mmc_berase(struct blk_desc *block_dev, lbaint_t start, lbaint_t blkcnt)
blk_r = ((blkcnt - blk) > mmc->erase_grp_size) ?
mmc->erase_grp_size : (blkcnt - blk);
}
- err = mmc_erase_t(mmc, start + blk, blk_r);
+ err = mmc_erase_t(mmc, start + blk, blk_r, arg);
if (err)
break;
diff --git a/include/mmc.h b/include/mmc.h
index 1d377e0281..bfdc712bdd 100644
--- a/include/mmc.h
+++ b/include/mmc.h
@@ -80,7 +80,6 @@ struct bd_info;
#define MMC_MODE_1BIT BIT(28)
#define MMC_MODE_SPI BIT(27)
-
#define SD_DATA_4BIT 0x00040000
#define IS_SD(x) ((x)->version & SD_VERSION_SD)
@@ -240,6 +239,7 @@ static inline bool mmc_is_tuning_cmd(uint cmdidx)
#define EXT_CSD_HC_WP_GRP_SIZE 221 /* RO */
#define EXT_CSD_HC_ERASE_GRP_SIZE 224 /* RO */
#define EXT_CSD_BOOT_MULT 226 /* RO */
+#define EXT_CSD_SEC_FEATURE_SUPPORT 231 /* RO */
#define EXT_CSD_GENERIC_CMD6_TIME 248 /* RO */
#define EXT_CSD_BKOPS_SUPPORT 502 /* RO */
@@ -247,6 +247,8 @@ static inline bool mmc_is_tuning_cmd(uint cmdidx)
* EXT_CSD field definitions
*/
+#define EXT_CSD_SEC_ER_EN BIT(0)
+
#define EXT_CSD_CMD_SET_NORMAL (1 << 0)
#define EXT_CSD_CMD_SET_SECURE (1 << 1)
#define EXT_CSD_CMD_SET_CPSECURE (1 << 2)
@@ -690,6 +692,7 @@ struct mmc {
uint tran_speed;
uint legacy_speed; /* speed for the legacy mode provided by the card */
uint read_bl_len;
+ u8 sec_feature_support;
#if CONFIG_IS_ENABLED(MMC_WRITE)
uint write_bl_len;
uint erase_grp_size; /* in 512-byte sectors */
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH] drivers: mmc: support secure erase.
2020-12-18 13:21 ` [PATCH] drivers: mmc: support secure erase dennis
@ 2020-12-21 23:05 ` Jaehoon Chung
2020-12-26 8:44 ` dennis laplacian1
0 siblings, 1 reply; 6+ messages in thread
From: Jaehoon Chung @ 2020-12-21 23:05 UTC (permalink / raw)
To: u-boot
Hi,
CC'd Peng (mmc maintainer).
On 12/18/20 10:21 PM, dennis wrote:
> According to the JEDEC Standard, eMMC v4.4 cards can supoort secure erase.
> the first bit in ext_csd[EXT_CSD_SEC_FEATURE_SUPPORT] - EXT_CSD_SEC_ER_EN
> is used to check if secure erase supoort by the device.
%s/supoort/supports/
Anyway, i don't know what
>
> Signed-off-by: dennis <dennislaplacian1@gmail.com>
> ---
> drivers/mmc/Kconfig | 8 ++++++++
> drivers/mmc/mmc.c | 4 ++++
> drivers/mmc/mmc_write.c | 16 +++++++++++++---
> include/mmc.h | 5 ++++-
> 4 files changed, 29 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
> index 14d7913986..6105cbe960 100644
> --- a/drivers/mmc/Kconfig
> +++ b/drivers/mmc/Kconfig
> @@ -18,6 +18,14 @@ config MMC_WRITE
> help
> Enable write access to MMC and SD Cards
>
> +config MMC_SECURE_ERASE
> + bool "Enable secure erase for eMMC"
> + depends on MMC_WRITE
> + default n
> + help
> + This option replace the default erase with secure erase in eMMC v4.4
> + and above cards.
> +
> config MMC_BROKEN_CD
> bool "Poll for broken card detection case"
> help
> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
> index a6394bcf30..56b17070e7 100644
> --- a/drivers/mmc/mmc.c
> +++ b/drivers/mmc/mmc.c
> @@ -2272,6 +2272,10 @@ static int mmc_startup_v4(struct mmc *mmc)
> if ((capacity >> 20) > 2 * 1024)
> mmc->capacity_user = capacity;
> }
> +#if CONFIG_IS_ENABLED(MMC_SECURE_ERASE)
> + if (mmc->version >= MMC_VERSION_4_4 && IS_MMC(mmc))
if (IS_ENABLED())?
> + mmc->sec_feature_support = ext_csd[EXT_CSD_SEC_FEATURE_SUPPORT];
> +#endif
>
> if (mmc->version >= MMC_VERSION_4_5)
> mmc->gen_cmd6_time = ext_csd[EXT_CSD_GENERIC_CMD6_TIME];
> diff --git a/drivers/mmc/mmc_write.c b/drivers/mmc/mmc_write.c
> index 6a4453ca02..0b720ccf7e 100644
> --- a/drivers/mmc/mmc_write.c
> +++ b/drivers/mmc/mmc_write.c
> @@ -15,7 +15,7 @@
> #include <linux/math64.h>
> #include "mmc_private.h"
>
> -static ulong mmc_erase_t(struct mmc *mmc, ulong start, lbaint_t blkcnt)
> +static ulong mmc_erase_t(struct mmc *mmc, ulong start, lbaint_t blkcnt, uint arg)
> {
> struct mmc_cmd cmd;
> ulong end;
> @@ -52,7 +52,7 @@ static ulong mmc_erase_t(struct mmc *mmc, ulong start, lbaint_t blkcnt)
> goto err_out;
>
> cmd.cmdidx = MMC_CMD_ERASE;
> - cmd.cmdarg = MMC_ERASE_ARG;
> + cmd.cmdarg = arg;
> cmd.resp_type = MMC_RSP_R1b;
>
> err = mmc_send_cmd(mmc, &cmd, NULL);
> @@ -81,6 +81,7 @@ ulong mmc_berase(struct blk_desc *block_dev, lbaint_t start, lbaint_t blkcnt)
> struct mmc *mmc = find_mmc_device(dev_num);
> lbaint_t blk = 0, blk_r = 0;
> int timeout_ms = 1000;
> + uint arg = MMC_ERASE_ARG;
>
> if (!mmc)
> return -1;
> @@ -105,6 +106,15 @@ ulong mmc_berase(struct blk_desc *block_dev, lbaint_t start, lbaint_t blkcnt)
> ((start + blkcnt + mmc->erase_grp_size)
> & ~(mmc->erase_grp_size - 1)) - 1);
>
> +#if CONFIG_IS_ENABLED(MMC_SECURE_ERASE)
> + if (!(mmc->sec_feature_support & EXT_CSD_SEC_ER_EN)) {
ditto. if (IS_ENABLED()) instead of #if.
And i don't know why doesn't check this in mmc_erase_t().
Then it doesn't need to pass arg as argument.
> + printf("secure erase not supported on device\n"
> + "perform insecure erase\n");
> + } else {
> + arg = MMC_SECURE_ERASE_ARG;
> + }
> +#endif
> +
> while (blk < blkcnt) {
> if (IS_SD(mmc) && mmc->ssr.au) {
> blk_r = ((blkcnt - blk) > mmc->ssr.au) ?
> @@ -113,7 +123,7 @@ ulong mmc_berase(struct blk_desc *block_dev, lbaint_t start, lbaint_t blkcnt)
> blk_r = ((blkcnt - blk) > mmc->erase_grp_size) ?
> mmc->erase_grp_size : (blkcnt - blk);
> }
> - err = mmc_erase_t(mmc, start + blk, blk_r);
> + err = mmc_erase_t(mmc, start + blk, blk_r, arg);
> if (err)
> break;
>
> diff --git a/include/mmc.h b/include/mmc.h
> index 1d377e0281..bfdc712bdd 100644
> --- a/include/mmc.h
> +++ b/include/mmc.h
> @@ -80,7 +80,6 @@ struct bd_info;
> #define MMC_MODE_1BIT BIT(28)
> #define MMC_MODE_SPI BIT(27)
>
> -
Not need to touch this.
> #define SD_DATA_4BIT 0x00040000
>
> #define IS_SD(x) ((x)->version & SD_VERSION_SD)
> @@ -240,6 +239,7 @@ static inline bool mmc_is_tuning_cmd(uint cmdidx)
> #define EXT_CSD_HC_WP_GRP_SIZE 221 /* RO */
> #define EXT_CSD_HC_ERASE_GRP_SIZE 224 /* RO */
> #define EXT_CSD_BOOT_MULT 226 /* RO */
> +#define EXT_CSD_SEC_FEATURE_SUPPORT 231 /* RO */
> #define EXT_CSD_GENERIC_CMD6_TIME 248 /* RO */
> #define EXT_CSD_BKOPS_SUPPORT 502 /* RO */
>
> @@ -247,6 +247,8 @@ static inline bool mmc_is_tuning_cmd(uint cmdidx)
> * EXT_CSD field definitions
> */
>
> +#define EXT_CSD_SEC_ER_EN BIT(0)
> +
> #define EXT_CSD_CMD_SET_NORMAL (1 << 0)
> #define EXT_CSD_CMD_SET_SECURE (1 << 1)
> #define EXT_CSD_CMD_SET_CPSECURE (1 << 2)
> @@ -690,6 +692,7 @@ struct mmc {
> uint tran_speed;
> uint legacy_speed; /* speed for the legacy mode provided by the card */
> uint read_bl_len;
> + u8 sec_feature_support;
This is located at below under MMC_WRITE and it's only used when MMC_SECURE_ERASE is enabled.
Best Regards,
Jaehoon Chung
> #if CONFIG_IS_ENABLED(MMC_WRITE)
> uint write_bl_len;
> uint erase_grp_size; /* in 512-byte sectors */
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] drivers: mmc: support secure erase.
2020-12-21 23:05 ` Jaehoon Chung
@ 2020-12-26 8:44 ` dennis laplacian1
2020-12-27 22:40 ` Jaehoon Chung
0 siblings, 1 reply; 6+ messages in thread
From: dennis laplacian1 @ 2020-12-26 8:44 UTC (permalink / raw)
To: u-boot
Hi Jaehoon Chung,
for your comment:
"ditto. if (IS_ENABLED()) instead of #if.
And i don't know why doesn't check this in mmc_erase_t().
Then it doesn't need to pass arg as argument."
I check the support of secure erase in the mmc_berase and not in
mmc_erase_t because mmc_erase_t is called multiple times for each block.
So to prevent unnecessary checks, I check the support in mmc_berase
function.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] drivers: mmc: support secure erase.
2020-12-26 8:44 ` dennis laplacian1
@ 2020-12-27 22:40 ` Jaehoon Chung
2021-01-01 11:42 ` dennis laplacian1
0 siblings, 1 reply; 6+ messages in thread
From: Jaehoon Chung @ 2020-12-27 22:40 UTC (permalink / raw)
To: u-boot
Hi Dennis,
On 12/26/20 5:44 PM, dennis laplacian1 wrote:
> Hi Jaehoon Chung,
> for your comment:
> "ditto. if (IS_ENABLED()) instead of #if.
> And i don't know why doesn't check this in mmc_erase_t().
> Then it doesn't need to pass arg as argument."
>
> I check the support of secure erase in the mmc_berase and not in
> mmc_erase_t because mmc_erase_t is called multiple times for each block.
> So to prevent unnecessary checks, I check the support in mmc_berase
> function.
It there any problem to check a condition in mmc_erase_t()? or any dramatic degradation?
And CONFIG_MMC_SECURE_ERASE is not enabled by default. So It doesn't need to add a variable.
Best Regards,
Jaehoon Chung
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] drivers: mmc: support secure erase.
2020-12-27 22:40 ` Jaehoon Chung
@ 2021-01-01 11:42 ` dennis laplacian1
2021-01-04 23:44 ` Jaehoon Chung
0 siblings, 1 reply; 6+ messages in thread
From: dennis laplacian1 @ 2021-01-01 11:42 UTC (permalink / raw)
To: u-boot
Hi Jaehoon.
The only degradation is that the check will be for every block in the
deleted range and not for the range.
I can move the check to mmc_erase_t, but then if the device is not
supporting secure erase the error message will be printed for every block.
I can check the support also in mmc_berase and print an error message only
in this function.
What do you think?
??????? ??? ??, 28 ????? 2020 ?-0:39 ??? ?Jaehoon Chung?? <?
jh80.chung@samsung.com??>:?
> Hi Dennis,
>
> On 12/26/20 5:44 PM, dennis laplacian1 wrote:
> > Hi Jaehoon Chung,
> > for your comment:
> > "ditto. if (IS_ENABLED()) instead of #if.
> > And i don't know why doesn't check this in mmc_erase_t().
> > Then it doesn't need to pass arg as argument."
> >
> > I check the support of secure erase in the mmc_berase and not in
> > mmc_erase_t because mmc_erase_t is called multiple times for each block.
> > So to prevent unnecessary checks, I check the support in mmc_berase
> > function.
>
> It there any problem to check a condition in mmc_erase_t()? or any
> dramatic degradation?
> And CONFIG_MMC_SECURE_ERASE is not enabled by default. So It doesn't need
> to add a variable.
>
> Best Regards,
> Jaehoon Chung
>
> >
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] drivers: mmc: support secure erase.
2021-01-01 11:42 ` dennis laplacian1
@ 2021-01-04 23:44 ` Jaehoon Chung
0 siblings, 0 replies; 6+ messages in thread
From: Jaehoon Chung @ 2021-01-04 23:44 UTC (permalink / raw)
To: u-boot
Hi Dennis,
On 1/1/21 8:42 PM, dennis laplacian1 wrote:
> Hi Jaehoon.
> The only degradation is that the check will be for every block in the
> deleted range and not for the range.
> I can move the check to mmc_erase_t, but then if the device is not
> supporting secure erase the error message will be printed for every block.
> I can check the support also in mmc_berase and print an error message only
> in this function.
> What do you think?
I understood what you said. Your code is displaying "secure erase not supported..".
So it will be displayed spamming when card doesn't support secure erase.
In this case, your mention is right.
Well, if you're ok, it can be changed from printf to debug.
And I think that it can be located to mmc->sec_feature_support is assigned.
If it needs to know whether card is sec_feature_support or not, i think that it can be implemented into mmcinfo.
I don't think what's correct or wrong. :) It's just my preference.
Best Regards,
Jaehoon Chung
>
> ??????? ??? ??, 28 ????? 2020 ?-0:39 ??? ?Jaehoon Chung?? <?
> jh80.chung at samsung.com??>:?
>
>> Hi Dennis,
>>
>> On 12/26/20 5:44 PM, dennis laplacian1 wrote:
>>> Hi Jaehoon Chung,
>>> for your comment:
>>> "ditto. if (IS_ENABLED()) instead of #if.
>>> And i don't know why doesn't check this in mmc_erase_t().
>>> Then it doesn't need to pass arg as argument."
>>>
>>> I check the support of secure erase in the mmc_berase and not in
>>> mmc_erase_t because mmc_erase_t is called multiple times for each block.
>>> So to prevent unnecessary checks, I check the support in mmc_berase
>>> function.
>>
>> It there any problem to check a condition in mmc_erase_t()? or any
>> dramatic degradation?
>> And CONFIG_MMC_SECURE_ERASE is not enabled by default. So It doesn't need
>> to add a variable.
>>
>> Best Regards,
>> Jaehoon Chung
>>
>>>
>>
>>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-01-04 23:44 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <CGME20201218170158epcas1p4076c9b81d5eda9864a9998282ed8ea47@epcas1p4.samsung.com>
2020-12-18 13:21 ` [PATCH] drivers: mmc: support secure erase dennis
2020-12-21 23:05 ` Jaehoon Chung
2020-12-26 8:44 ` dennis laplacian1
2020-12-27 22:40 ` Jaehoon Chung
2021-01-01 11:42 ` dennis laplacian1
2021-01-04 23:44 ` Jaehoon Chung
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.