2012-08-29 06:09:35

by IAN CHEN

[permalink] [raw]
Subject: [PATCH v6] mmc: card: Skip secure option for MoviNAND.

For several MoviNAND, there are some known issue with secure option.
For these specific MoviNAND device, we skip secure option.

You could refer this discussion in XDA developers.
http://forum.xda-developers.com/showthread.php?t=1644364
https://plus.google.com/111398485184813224730/posts/21pTYfTsCkB#111398485184813224730/posts/21pTYfTsCkB

Signed-off-by: Ian Chen <[email protected]>
Reviewed-by: Namjae Jeon <[email protected]>
Acked-by: Jaehoon Chung <[email protected]>
Reviewed-by: Linus Walleij <[email protected]>
---
drivers/mmc/card/block.c | 24 +++++++++++++++++++++++-
include/linux/mmc/card.h | 1 +
2 files changed, 24 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index f1c84de..0377885 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -1411,7 +1411,8 @@ static int mmc_blk_issue_rq(struct mmc_queue *mq, struct request *req)
/* complete ongoing async transfer before issuing discard */
if (card->host->areq)
mmc_blk_issue_rw_rq(mq, NULL);
- if (req->cmd_flags & REQ_SECURE)
+ if (req->cmd_flags & REQ_SECURE &&
+ !(card->quirks & MMC_QUIRK_SEC_ERASE_TRIM_BROKEN))
ret = mmc_blk_issue_secdiscard_rq(mq, req);
else
ret = mmc_blk_issue_discard_rq(mq, req);
@@ -1716,6 +1717,7 @@ force_ro_fail:
#define CID_MANFID_SANDISK 0x2
#define CID_MANFID_TOSHIBA 0x11
#define CID_MANFID_MICRON 0x13
+#define CID_MANFID_SAMSUNG 0x15

static const struct mmc_fixup blk_fixups[] =
{
@@ -1752,6 +1754,26 @@ static const struct mmc_fixup blk_fixups[] =
MMC_FIXUP(CID_NAME_ANY, CID_MANFID_MICRON, 0x200, add_quirk_mmc,
MMC_QUIRK_LONG_READ_TIME),

+ /*
+ * Some issue about secure erase/secure trim for Samsung MoviNAND
+ */
+ MMC_FIXUP("M8G2FA", CID_MANFID_SAMSUNG, CID_OEMID_ANY, add_quirk_mmc,
+ MMC_QUIRK_SEC_ERASE_TRIM_BROKEN),
+ MMC_FIXUP("MAG4FA", CID_MANFID_SAMSUNG, CID_OEMID_ANY, add_quirk_mmc,
+ MMC_QUIRK_SEC_ERASE_TRIM_BROKEN),
+ MMC_FIXUP("MBG8FA", CID_MANFID_SAMSUNG, CID_OEMID_ANY, add_quirk_mmc,
+ MMC_QUIRK_SEC_ERASE_TRIM_BROKEN),
+ MMC_FIXUP("MCGAFA", CID_MANFID_SAMSUNG, CID_OEMID_ANY, add_quirk_mmc,
+ MMC_QUIRK_SEC_ERASE_TRIM_BROKEN),
+ MMC_FIXUP("VAL00M", CID_MANFID_SAMSUNG, CID_OEMID_ANY, add_quirk_mmc,
+ MMC_QUIRK_SEC_ERASE_TRIM_BROKEN),
+ MMC_FIXUP("VYL00M", CID_MANFID_SAMSUNG, CID_OEMID_ANY, add_quirk_mmc,
+ MMC_QUIRK_SEC_ERASE_TRIM_BROKEN),
+ MMC_FIXUP("KYL00M", CID_MANFID_SAMSUNG, CID_OEMID_ANY, add_quirk_mmc,
+ MMC_QUIRK_SEC_ERASE_TRIM_BROKEN),
+ MMC_FIXUP("VZL00M", CID_MANFID_SAMSUNG, CID_OEMID_ANY, add_quirk_mmc,
+ MMC_QUIRK_SEC_ERASE_TRIM_BROKEN),
+
END_FIXUP
};

diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index 111aca5..4b27f9f 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -239,6 +239,7 @@ struct mmc_card {
#define MMC_QUIRK_BLK_NO_CMD23 (1<<7) /* Avoid CMD23 for regular multiblock */
#define MMC_QUIRK_BROKEN_BYTE_MODE_512 (1<<8) /* Avoid sending 512 bytes in */
#define MMC_QUIRK_LONG_READ_TIME (1<<9) /* Data read time > CSD says */
+#define MMC_QUIRK_SEC_ERASE_TRIM_BROKEN (1<<10) /* Skip secure for erase/trim */
/* byte mode */
unsigned int poweroff_notify_state; /* eMMC4.5 notify feature */
#define MMC_NO_POWER_NOTIFICATION 0
--
1.7.0.4


2012-08-29 07:27:34

by Jaehoon Chung

[permalink] [raw]
Subject: Re: [PATCH v6] mmc: card: Skip secure option for MoviNAND.

Hi Chris,

Could you merge this patch?
We want to prevent this problem at mainline kernel.

Best Regards,
Jaehoon Chung

On 08/29/2012 03:05 PM, IAN CHEN wrote:
> For several MoviNAND, there are some known issue with secure option.
> For these specific MoviNAND device, we skip secure option.
>
> You could refer this discussion in XDA developers.
> http://forum.xda-developers.com/showthread.php?t=1644364
> https://plus.google.com/111398485184813224730/posts/21pTYfTsCkB#111398485184813224730/posts/21pTYfTsCkB
>
> Signed-off-by: Ian Chen <[email protected]>
> Reviewed-by: Namjae Jeon <[email protected]>
> Acked-by: Jaehoon Chung <[email protected]>
> Reviewed-by: Linus Walleij <[email protected]>
> ---
> drivers/mmc/card/block.c | 24 +++++++++++++++++++++++-
> include/linux/mmc/card.h | 1 +
> 2 files changed, 24 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
> index f1c84de..0377885 100644
> --- a/drivers/mmc/card/block.c
> +++ b/drivers/mmc/card/block.c
> @@ -1411,7 +1411,8 @@ static int mmc_blk_issue_rq(struct mmc_queue *mq, struct request *req)
> /* complete ongoing async transfer before issuing discard */
> if (card->host->areq)
> mmc_blk_issue_rw_rq(mq, NULL);
> - if (req->cmd_flags & REQ_SECURE)
> + if (req->cmd_flags & REQ_SECURE &&
> + !(card->quirks & MMC_QUIRK_SEC_ERASE_TRIM_BROKEN))
> ret = mmc_blk_issue_secdiscard_rq(mq, req);
> else
> ret = mmc_blk_issue_discard_rq(mq, req);
> @@ -1716,6 +1717,7 @@ force_ro_fail:
> #define CID_MANFID_SANDISK 0x2
> #define CID_MANFID_TOSHIBA 0x11
> #define CID_MANFID_MICRON 0x13
> +#define CID_MANFID_SAMSUNG 0x15
>
> static const struct mmc_fixup blk_fixups[] =
> {
> @@ -1752,6 +1754,26 @@ static const struct mmc_fixup blk_fixups[] =
> MMC_FIXUP(CID_NAME_ANY, CID_MANFID_MICRON, 0x200, add_quirk_mmc,
> MMC_QUIRK_LONG_READ_TIME),
>
> + /*
> + * Some issue about secure erase/secure trim for Samsung MoviNAND
> + */
> + MMC_FIXUP("M8G2FA", CID_MANFID_SAMSUNG, CID_OEMID_ANY, add_quirk_mmc,
> + MMC_QUIRK_SEC_ERASE_TRIM_BROKEN),
> + MMC_FIXUP("MAG4FA", CID_MANFID_SAMSUNG, CID_OEMID_ANY, add_quirk_mmc,
> + MMC_QUIRK_SEC_ERASE_TRIM_BROKEN),
> + MMC_FIXUP("MBG8FA", CID_MANFID_SAMSUNG, CID_OEMID_ANY, add_quirk_mmc,
> + MMC_QUIRK_SEC_ERASE_TRIM_BROKEN),
> + MMC_FIXUP("MCGAFA", CID_MANFID_SAMSUNG, CID_OEMID_ANY, add_quirk_mmc,
> + MMC_QUIRK_SEC_ERASE_TRIM_BROKEN),
> + MMC_FIXUP("VAL00M", CID_MANFID_SAMSUNG, CID_OEMID_ANY, add_quirk_mmc,
> + MMC_QUIRK_SEC_ERASE_TRIM_BROKEN),
> + MMC_FIXUP("VYL00M", CID_MANFID_SAMSUNG, CID_OEMID_ANY, add_quirk_mmc,
> + MMC_QUIRK_SEC_ERASE_TRIM_BROKEN),
> + MMC_FIXUP("KYL00M", CID_MANFID_SAMSUNG, CID_OEMID_ANY, add_quirk_mmc,
> + MMC_QUIRK_SEC_ERASE_TRIM_BROKEN),
> + MMC_FIXUP("VZL00M", CID_MANFID_SAMSUNG, CID_OEMID_ANY, add_quirk_mmc,
> + MMC_QUIRK_SEC_ERASE_TRIM_BROKEN),
> +
> END_FIXUP
> };
>
> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
> index 111aca5..4b27f9f 100644
> --- a/include/linux/mmc/card.h
> +++ b/include/linux/mmc/card.h
> @@ -239,6 +239,7 @@ struct mmc_card {
> #define MMC_QUIRK_BLK_NO_CMD23 (1<<7) /* Avoid CMD23 for regular multiblock */
> #define MMC_QUIRK_BROKEN_BYTE_MODE_512 (1<<8) /* Avoid sending 512 bytes in */
> #define MMC_QUIRK_LONG_READ_TIME (1<<9) /* Data read time > CSD says */
> +#define MMC_QUIRK_SEC_ERASE_TRIM_BROKEN (1<<10) /* Skip secure for erase/trim */
> /* byte mode */
> unsigned int poweroff_notify_state; /* eMMC4.5 notify feature */
> #define MMC_NO_POWER_NOTIFICATION 0
>

2012-08-29 10:27:11

by Chris Ball

[permalink] [raw]
Subject: Re: [PATCH v6] mmc: card: Skip secure option for MoviNAND.

Hi,

On Wed, Aug 29 2012, IAN CHEN wrote:
> For several MoviNAND, there are some known issue with secure option.
> For these specific MoviNAND device, we skip secure option.
>
> You could refer this discussion in XDA developers.
> http://forum.xda-developers.com/showthread.php?t=1644364
> https://plus.google.com/111398485184813224730/posts/21pTYfTsCkB#111398485184813224730/posts/21pTYfTsCkB
>
> Signed-off-by: Ian Chen <[email protected]>
> Reviewed-by: Namjae Jeon <[email protected]>
> Acked-by: Jaehoon Chung <[email protected]>
> Reviewed-by: Linus Walleij <[email protected]>

Thanks. We should explain the nature of the bug in the commit message,
so that users and the stable@ maintainers know how bad the problem is
by reading the patch. (This should go to stable@, right? Do you know
which kernel versions are affected?)

I'll add something like this, unless you have a better proposal:

==
For several MoviNAND, there are some known issues with secure option.
For these specific MoviNAND device, we skip secure option.

Specifically, there is a bug in the eMMC firmware that causes
unrecoverable corruption when the MMC is erased with MMC_CAP_ERASE
enabled.

You could refer this discussion in XDA developers.
http://forum.xda-developers.com/showthread.php?t=1644364
https://plus.google.com/111398485184813224730/posts/21pTYfTsCkB#111398485184813224730/posts/21pTYfTsCkB
==

- Chris.
--
Chris Ball <[email protected]> <http://printf.net/>
One Laptop Per Child

2012-08-30 01:36:03

by IAN CHEN

[permalink] [raw]
Subject: RE: [PATCH v6] mmc: card: Skip secure option for MoviNAND.

Hi,

Thanks for your help.
The effected kernel will be all the version with MMC_CAP_ERASE & SECURE
option enabled.
As commit content mentioned, the problem is unrecoverable corruption after
SECURE ERASE or SECURE TRIM.
For devices using defective eMMC with kernel supporting MMC_CAP_ERASE &
SECURE option, this issue might cause damage.
Therefore, we skip secure for these known devices.

Regards,
Ian

-----Original Message-----
From: Chris Ball [mailto:[email protected]]
Sent: Wednesday, August 29, 2012 7:27 PM

Hi,

On Wed, Aug 29 2012, IAN CHEN wrote:
> For several MoviNAND, there are some known issue with secure option.
> For these specific MoviNAND device, we skip secure option.
>
> You could refer this discussion in XDA developers.
> http://forum.xda-developers.com/showthread.php?t=1644364
>
https://plus.google.com/111398485184813224730/posts/21pTYfTsCkB#111398485184
813224730/posts/21pTYfTsCkB
>
> Signed-off-by: Ian Chen <[email protected]>
> Reviewed-by: Namjae Jeon <[email protected]>
> Acked-by: Jaehoon Chung <[email protected]>
> Reviewed-by: Linus Walleij <[email protected]>

Thanks. We should explain the nature of the bug in the commit message,
so that users and the stable@ maintainers know how bad the problem is
by reading the patch. (This should go to stable@, right? Do you know
which kernel versions are affected?)

I'll add something like this, unless you have a better proposal:

==
For several MoviNAND, there are some known issues with secure option.
For these specific MoviNAND device, we skip secure option.

Specifically, there is a bug in the eMMC firmware that causes
unrecoverable corruption when the MMC is erased with MMC_CAP_ERASE
enabled.

You could refer this discussion in XDA developers.
http://forum.xda-developers.com/showthread.php?t=1644364
https://plus.google.com/111398485184813224730/posts/21pTYfTsCkB#111398485184
813224730/posts/21pTYfTsCkB
==

- Chris.
--
Chris Ball <[email protected]> <http://printf.net/>
One Laptop Per Child

2012-08-30 01:52:42

by Chris Ball

[permalink] [raw]
Subject: Re: [PATCH v6] mmc: card: Skip secure option for MoviNAND.

Hi,

On Wed, Aug 29 2012, IAN CHEN wrote:
> Thanks for your help.
> The effected kernel will be all the version with MMC_CAP_ERASE & SECURE
> option enabled.
> As commit content mentioned, the problem is unrecoverable corruption after
> SECURE ERASE or SECURE TRIM.
> For devices using defective eMMC with kernel supporting MMC_CAP_ERASE &
> SECURE option, this issue might cause damage.
> Therefore, we skip secure for these known devices.

Thanks, pushed to mmc-next for 3.6 with a stable@ tag.

- Chris.
--
Chris Ball <[email protected]> <http://printf.net/>
One Laptop Per Child