linux-kernel - Re: [PATCH v4] mmc: Add quirk MMC_QUIRK_BROKEN_CACHE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <f9beb373-a6c8-ba07-f3d1-c72ecc058806@gmail.com>
Date:   Wed, 4 Oct 2023 21:07:17 +0200
From:   Bean Huo <huobean@...il.com>
To:     Bean Huo <beanhuo@...pp.de>, ulf.hansson@...aro.org,
        adrian.hunter@...el.com, beanhuo@...ron.com,
        jakub.kwapisz@...adex.com, rafael.beims@...adex.com
Cc:     linux-mmc@...r.kernel.org, linux-kernel@...r.kernel.org,
        stable@...r.kernel.org
Subject: Re: [PATCH v4] mmc: Add quirk MMC_QUIRK_BROKEN_CACHE_FLUSH for Micron
 eMMC Q2J54A

Hi Ulf,

Any comments on this version?

Kind regards,

Beam

On 01.10.23 9:49 PM, Bean Huo wrote:
> From: Bean Huo <beanhuo@...ron.com>
>
> Micron MTFC4GACAJCN eMMC supports cache but requires that flush cache
> operation be allowed only after a write has occurred. Otherwise, the
> cache flush command or subsequent commands will time out.
>
> Signed-off-by: Bean Huo <beanhuo@...ron.com>
> Signed-off-by: Rafael Beims <rafael.beims@...adex.com>
> Cc: stable@...r.kernel.org
>
> ---
> Changelog:
>
> v3--v4:
>      1. Add helper function for this quirk in drivers/mmc/core/card.h.
>      2. Set card->written_flag only for REQ_OP_WRITE.
> v2--v3:
>      1. Set card->written_flag in mmc_blk_mq_issue_rq().
> v1--v2:
>      1. Add Rafael's test-tag, and Co-developed-by.
>      2. Check host->card whether NULL or not in __mmc_start_request() before asserting host->card->->quirks
> ---
>   drivers/mmc/core/block.c  | 5 ++++-
>   drivers/mmc/core/card.h   | 4 ++++
>   drivers/mmc/core/mmc.c    | 5 +++++
>   drivers/mmc/core/quirks.h | 7 ++++---
>   include/linux/mmc/card.h  | 2 ++
>   5 files changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index 3a8f27c3e310..dfa67d9c80bb 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -2381,8 +2381,11 @@ enum mmc_issued mmc_blk_mq_issue_rq(struct mmc_queue *mq, struct request *req)
>   			}
>   			ret = mmc_blk_cqe_issue_flush(mq, req);
>   			break;
> -		case REQ_OP_READ:
>   		case REQ_OP_WRITE:
> +			if (mmc_card_broken_cache_flush(card) && !card->written_flag)
> +				card->written_flag = true;
> +			fallthrough;
> +		case REQ_OP_READ:
>   			if (host->cqe_enabled)
>   				ret = mmc_blk_cqe_issue_rw_rq(mq, req);
>   			else
> diff --git a/drivers/mmc/core/card.h b/drivers/mmc/core/card.h
> index 4edf9057fa79..b7754a1b8d97 100644
> --- a/drivers/mmc/core/card.h
> +++ b/drivers/mmc/core/card.h
> @@ -280,4 +280,8 @@ static inline int mmc_card_broken_sd_cache(const struct mmc_card *c)
>   	return c->quirks & MMC_QUIRK_BROKEN_SD_CACHE;
>   }
>   
> +static inline int mmc_card_broken_cache_flush(const struct mmc_card *c)
> +{
> +	return c->quirks & MMC_QUIRK_BROKEN_CACHE_FLUSH;
> +}
>   #endif
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index 89cd48fcec79..47896c32086e 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -1929,6 +1929,8 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
>   	if (!oldcard)
>   		host->card = card;
>   
> +	card->written_flag = false;
> +
>   	return 0;
>   
>   free_card:
> @@ -2081,6 +2083,9 @@ static int _mmc_flush_cache(struct mmc_host *host)
>   {
>   	int err = 0;
>   
> +	if (mmc_card_broken_cache_flush(host->card) && !host->card->written_flag)
> +		return err;
> +
>   	if (_mmc_cache_enabled(host)) {
>   		err = mmc_switch(host->card, EXT_CSD_CMD_SET_NORMAL,
>   				 EXT_CSD_FLUSH_CACHE, 1,
> diff --git a/drivers/mmc/core/quirks.h b/drivers/mmc/core/quirks.h
> index 32b64b564fb1..5e68c8b4cdca 100644
> --- a/drivers/mmc/core/quirks.h
> +++ b/drivers/mmc/core/quirks.h
> @@ -110,11 +110,12 @@ static const struct mmc_fixup __maybe_unused mmc_blk_fixups[] = {
>   		  MMC_QUIRK_TRIM_BROKEN),
>   
>   	/*
> -	 * Micron MTFC4GACAJCN-1M advertises TRIM but it does not seems to
> -	 * support being used to offload WRITE_ZEROES.
> +	 * Micron MTFC4GACAJCN-1M supports TRIM but does not appear to suppor
> +	 * WRITE_ZEROES offloading. It also supports caching, but the cache can
> +	 * only be flushed after a write has occurred.
>   	 */
>   	MMC_FIXUP("Q2J54A", CID_MANFID_MICRON, 0x014e, add_quirk_mmc,
> -		  MMC_QUIRK_TRIM_BROKEN),
> +		  MMC_QUIRK_TRIM_BROKEN | MMC_QUIRK_BROKEN_CACHE_FLUSH),
>   
>   	/*
>   	 * Kingston EMMC04G-M627 advertises TRIM but it does not seems to
> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
> index daa2f40d9ce6..7b12eebc5586 100644
> --- a/include/linux/mmc/card.h
> +++ b/include/linux/mmc/card.h
> @@ -295,7 +295,9 @@ struct mmc_card {
>   #define MMC_QUIRK_BROKEN_HPI	(1<<13)		/* Disable broken HPI support */
>   #define MMC_QUIRK_BROKEN_SD_DISCARD	(1<<14)	/* Disable broken SD discard support */
>   #define MMC_QUIRK_BROKEN_SD_CACHE	(1<<15)	/* Disable broken SD cache support */
> +#define MMC_QUIRK_BROKEN_CACHE_FLUSH	(1<<16)	/* Don't flush cache until the write has occurred */
>   
> +	bool			written_flag;	/* Indicates eMMC has been written since power on */
>   	bool			reenable_cmdq;	/* Re-enable Command Queue */
>   
>   	unsigned int		erase_size;	/* erase size in sectors */