[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAPDyKFoPVeu3xe7ZAxBTNFfHmf=f7OZ3iUG=pRG8mD=UOGNVPA@mail.gmail.com>
Date: Thu, 22 Jan 2026 11:14:08 +0100
From: Ulf Hansson <ulf.hansson@...aro.org>
To: Eric Biggers <ebiggers@...nel.org>
Cc: Neeraj Soni <neeraj.soni@....qualcomm.com>, adrian.hunter@...el.com,
quic_dmukhopa@...cinc.com, quic_rampraka@...cinc.com,
quic_nitirawa@...cinc.com, quic_sachgupt@...cinc.com,
quic_bhaskarv@...cinc.com, quic_gaurkash@...cinc.com,
quic_sartgarg@...cinc.com, linux-mmc@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org
Subject: Re: [PATCH v4] mmc: Avoid reprogram all keys to Inline Crypto Engine
for MMC runtime suspend resume
On Thu, 22 Jan 2026 at 02:14, Eric Biggers <ebiggers@...nel.org> wrote:
>
> On Wed, Jan 21, 2026 at 03:12:43PM +0100, Ulf Hansson wrote:
> > > diff --git a/drivers/mmc/core/crypto.c b/drivers/mmc/core/crypto.c
> > > index fec4fbf16a5b..a5a90bfc634e 100644
> > > --- a/drivers/mmc/core/crypto.c
> > > +++ b/drivers/mmc/core/crypto.c
> > > @@ -15,7 +15,7 @@
> > > void mmc_crypto_set_initial_state(struct mmc_host *host)
> > > {
> > > /* Reset might clear all keys, so reprogram all the keys. */
> > > - if (host->caps2 & MMC_CAP2_CRYPTO)
> > > + if ((host->caps2 & MMC_CAP2_CRYPTO) && !(host->caps2 & MMC_CAP2_CRYPTO_NO_REPROG))
> > > blk_crypto_reprogram_all_keys(&host->crypto_profile);
> >
> > As far as I understand, calling blk_crypto_reprogram_all_keys() would
> > only be needed for those mmc hosts that lose their corresponding ICE
> > context during runtime+system suspend, reset and possibly during
> > ->probe().
> >
> > In other words, calling mmc_crypto_set_initial_state() from
> > mmc_set_initial_state() looks like it's a mistake, as it has really
> > nothing to do with the card's initialization, unless I have understood
> > this wrong!?
> >
> > That said, I would rather make the mtk-sd and sdhci-msm drivers to
> > handle this themselves, by explicitly calling
> > blk_crypto_reprogram_all_keys() when needed - and drop
> > mmc_crypto_set_initial_state() altogether.
> >
> > For the sdhci-msm case, it seems like the only case we need to care
> > about is for the reset.
> >
> > For mtk-sd I don't know what is needed, but possibly Eric can help out here?
>
> The comment for mmc_set_initial_state() says "Set initial state after a
> power cycle or a hw_reset." I relied on that when I added the call to
> mmc_crypto_set_initial_state() back in 2020. In the following thread it
> was also discussed that the code was intended to reprogram the keys on
> reset, not runtime suspend as that shouldn't be needed:
> https://lore.kernel.org/linux-mmc/X7gQ9Y44iIgkiM64@sol.localdomain/T/#u
The comment in the mmc_set_initial_state() is referring to the card
and not the host controller. There have been some similar
misunderstandings in the past for other functions in the core, sorry.
In any case, I have been trying to understand where the ICE context
really belongs and recently Neeraj answered that question [1].
>
> If that is not what it actually does, it probably would be appropriate
> to replace it with something else.
I agree, the comment(s) could deserve some clarifications.
>
> - Eric
Kind regards
Uffe
[1]
https://lore.kernel.org/all/e1f689d0-523b-5842-3a6e-10b431d617ce@oss.qualcomm.com/
Powered by blists - more mailing lists