Hi Adrian,
> The sanitize logic looks wrong to me. I would expect it to look
> like this:
>
>
> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
> index b180965..f5e0534 100644
> --- a/drivers/mmc/card/block.c
> +++ b/drivers/mmc/card/block.c
> @@ -881,17 +881,12 @@ static int mmc_blk_issue_secdiscard_rq(struct mmc_queue
> *mq,
> goto out;
> }
>
> - /* The sanitize operation is supported at v4.5 only */
> - if (mmc_can_sanitize(card)) {
> - err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
> - EXT_CSD_SANITIZE_START, 1, 0);
> - goto out;
> - }
> -
> from = blk_rq_pos(req);
> nr = blk_rq_sectors(req);
>
> - if (mmc_can_trim(card) && !mmc_erase_group_aligned(card, from, nr))
> + if (mmc_can_sanitize(card))
> + arg = MMC_DISCARD_ARG;
The sanitize and discard are not coupled functionalities.
Discard is a hint for performance meant to replace trim and erase
where performance matters.
Sanitize is a security operation meant to clear all unmapped contents.
Jedec 4.5 spec does not guarantee that a discarded sector will be sanitized.
This patch, if applied, will expose the kernel to a potential security
risk (retrieve old contents not wiped by a sanitize)
> + else if (mmc_can_trim(card) && !mmc_erase_group_aligned(card, from, nr))
> arg = MMC_SECURE_TRIM1_ARG;
> else
> arg = MMC_SECURE_ERASE_ARG;
> @@ -918,6 +913,12 @@ retry:
> }
> err = mmc_erase(card, from, nr, MMC_SECURE_TRIM2_ARG);
> }
> +
> + /* The sanitize operation is supported at v4.5 only */
> + if (!err && mmc_can_sanitize(card)) {
> + err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
> + EXT_CSD_SANITIZE_START, 1, 0);
> + }
> out:
> if (err == -EIO && !mmc_blk_reset(md, card->host, type))
> goto retry;
>
>
>
> Also the timeout for eMMC v4.5 DISCARD looks wrong. It should be
> the same as TRIM:
>
>
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index 14f262e..00fd7db 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -1407,7 +1407,7 @@ static unsigned int mmc_mmc_erase_timeout(struct
> mmc_card *card,
>
> if (card->ext_csd.erase_group_def & 1) {
> /* High Capacity Erase Group Size uses HC timeouts */
> - if (arg == MMC_TRIM_ARG)
> + if (arg == MMC_TRIM_ARG || arg == MMC_DISCARD_ARG)
> erase_timeout = card->ext_csd.trim_timeout;
> else
> erase_timeout = card->ext_csd.hc_erase_timeout;
>
>
Although I suspect that the discard cmd will be much faster than the
Trim on most devices, there is no such info available as of today in ext csd.
As such I agree with Adrian, discard timeout is nearer to trim than erase.
>
> In addition eMMC v4.5 seems to indicate the use of the trim timeout
> irrespective of the setting of erase_group_def, so maybe it should be
> like this:
>
>
>
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index 14f262e..4691a23 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -1405,7 +1405,10 @@ static unsigned int mmc_mmc_erase_timeout(struct
> mmc_card *card,
> {
> unsigned int erase_timeout;
>
> - if (card->ext_csd.erase_group_def & 1) {
> + if (arg == MMC_DISCARD_ARG ||
> + (arg == MMC_TRIM_ARG && card->ext_csd.rev >= 6)) {
> + erase_timeout = card->ext_csd.trim_timeout;
> + } else if (card->ext_csd.erase_group_def & 1) {
> /* High Capacity Erase Group Size uses HC timeouts */
> if (arg == MMC_TRIM_ARG)
> erase_timeout = card->ext_csd.trim_timeout;
>
>
>
>
> Alternatively, maybe it would be better to switch to HC erase size for all
> eMMC v4.5 cards?
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
