Re: [PATCH RESEND v8 2/2] mmc: card: Adding support for sanitize in eMMC 4.5

S, Venkatraman
2012-07-26T16:36:49Z

On Wed, Jul 25, 2012 at 5:01 PM, Yaniv Gardi <ygardi@codeaurora.org> wrote:
> This feature delete the unmap memory region of the eMMC card,
> by writing to a specific register in the EXT_CSD
> unmap region is the memory region that were previously deleted
> (by erase, trim or discard operation)
>
> Signed-off-by: Yaniv Gardi <ygardi@codeaurora.org>
>
> ---
> drivers/mmc/card/block.c | 72 +++++++++++++++++++++++++++++++++-------------
> drivers/mmc/card/queue.c | 10 ++++++-
> include/linux/mmc/host.h | 1 +
> 3 files changed, 62 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
> index f1c84de..c45ec9f 100644
> --- a/drivers/mmc/card/block.c
> +++ b/drivers/mmc/card/block.c
> @@ -860,10 +860,10 @@ static int mmc_blk_issue_secdiscard_rq(struct mmc_queue *mq,
> {
> struct mmc_blk_data *md = mq->data;
> struct mmc_card *card = md->queue.card;
> - unsigned int from, nr, arg, trim_arg, erase_arg;
> + unsigned int from, nr, arg;
> int err = 0, type = MMC_BLK_SECDISCARD;
>
> - if (!(mmc_can_secure_erase_trim(card) || mmc_can_sanitize(card))) {
> + if (!(mmc_can_secure_erase_trim(card))) {
> err = -EOPNOTSUPP;
> goto out;
> }
> @@ -871,23 +871,10 @@ static int mmc_blk_issue_secdiscard_rq(struct mmc_queue *mq,
> from = blk_rq_pos(req);
> nr = blk_rq_sectors(req);
>
> - /* The sanitize operation is supported at v4.5 only */
> - if (mmc_can_sanitize(card)) {
> - erase_arg = MMC_ERASE_ARG;
> - trim_arg = MMC_TRIM_ARG;
> - } else {
> - erase_arg = MMC_SECURE_ERASE_ARG;
> - trim_arg = MMC_SECURE_TRIM1_ARG;
> - }
> -
> - if (mmc_erase_group_aligned(card, from, nr))
> - arg = erase_arg;
> - else if (mmc_can_trim(card))
> - arg = trim_arg;
> - else {
> - err = -EINVAL;
> - goto out;
> - }
> + if (mmc_can_trim(card) && !mmc_erase_group_aligned(card, from, nr))
> + arg = MMC_SECURE_TRIM1_ARG;
> + else
> + arg = MMC_SECURE_ERASE_ARG;
> retry:
> if (card->quirks & MMC_QUIRK_INAND_CMD38) {
> err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
> @@ -937,6 +924,46 @@ out:
> return err ? 0 : 1;
> }
>
> +static int mmc_blk_issue_sanitize_rq(struct mmc_queue *mq,
> + struct request *req)
> +{
> + struct mmc_blk_data *md = mq->data;
> + struct mmc_card *card = md->queue.card;
> + int err = 0;
> +
> + BUG_ON(!card);
> + BUG_ON(!card->host);
> +
> + if (!(mmc_can_sanitize(card) &&
> + (card->host->caps2 & MMC_CAP2_SANITIZE))) {
> + pr_warning("%s: %s - SANITIZE is not supported\n",
> + mmc_hostname(card->host), __func__);
> + err = -EOPNOTSUPP;
> + goto out;
> + }
> +
> + pr_debug("%s: %s - SANITIZE IN PROGRESS...\n",
> + mmc_hostname(card->host), __func__);
> +
> + err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
> + EXT_CSD_SANITIZE_START, 1, 0);
> +
Everything else seems to be good overall. But you need some sort of timeout
implementation and handling for SANTIZE. It's not interrupted to
provide way for other commands,
as generic CMD6 timeout doesn't apply.
Currently, the device can be easily DOS'ed by just issuing the IOCTL repeatedly.