[PATCH v3] mmc: Avoid reprogram all keys to Inline Crypto Engine for MMC runtime suspend resume

Debraj Mukhopadhyay posted 1 patch 6 months, 3 weeks ago
There is a newer version of this series
drivers/mmc/core/crypto.c    | 2 +-
drivers/mmc/host/sdhci-msm.c | 6 ++++++
include/linux/mmc/host.h     | 5 +++++
3 files changed, 12 insertions(+), 1 deletion(-)
[PATCH v3] mmc: Avoid reprogram all keys to Inline Crypto Engine for MMC runtime suspend resume
Posted by Debraj Mukhopadhyay 6 months, 3 weeks ago
Crypto reprogram all keys is called for each MMC runtime
suspend/resume in current upstream design. If this is implemented
as a non-interruptible call to TEE for security, the cpu core is
blocked for execution while this call executes although the crypto
engine already has the keys. For example, glitches in audio/video
streaming applications have been observed due to this. Add the flag
MMC_CAP2_CRYPTO_NO_REPROG as part of host->caps2 to control reprogramming
keys to crypto engine for socs which dont require this feature.

Signed-off-by: Seshu Madhavi Puppala <quic_spuppala@quicinc.com>
Co-developed-by: Ram Prakash Gupta <quic_rampraka@quicinc.com>
Signed-off-by: Ram Prakash Gupta <quic_rampraka@quicinc.com>
Co-developed-by: Sarthak Garg <quic_sartgarg@quicinc.com>
Signed-off-by: Sarthak Garg <quic_sartgarg@quicinc.com>
Signed-off-by: Debraj Mukhopadhyay <quic_dmukhopa@quicinc.com>

---

Changes in v3:
- Renamed MMC_CAP2_DONT_REPROGRAM to MMC_CAP2_CRYPTO_NO_REPROG
  in the commit message for clarity.
- Added parentheses around the condition: (host->caps2 & MMC_CAP2_CRYPTO)
  to improve readability and correctness.
- Updated the comment associated with MMC_CAP2_CRYPTO_NO_REPROG 
  to better reflect its purpose.
  
Changes in v2:
- Renamed MMC_CAP2_DONT_REPROGRAM to MMC_CAP2_CRYPTO_NO_REPROG for
  improved clarity.
- Defined MMC_CAP2_CRYPTO_NO_REPROG for MMC targets that do not support
  a Crypto Engine.
- Restricted the usage of struct crypto_profile to MMC devices that
  support a Crypto Engine.

Changes in v1:
- Addressed the comments from:
  https://lore.kernel.org/lkml/20241006135530.17363-3-
  quic_spuppala@quicinc.com/T/#m69c9ab538bd9efd54515646952d0d7d1d7c17690
- Avoided reprogram of keys for Qualcomm SOCs only.
- Ensured reprogram of all keys on host controller reset.

---

 drivers/mmc/core/crypto.c    | 2 +-
 drivers/mmc/host/sdhci-msm.c | 6 ++++++
 include/linux/mmc/host.h     | 5 +++++
 3 files changed, 12 insertions(+), 1 deletion(-)

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);
 }
 
diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 66c0d1ba2a33..ee6783555f2e 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -1920,6 +1920,7 @@ static int sdhci_msm_ice_init(struct sdhci_msm_host *msm_host,
 	}
 
 	mmc->caps2 |= MMC_CAP2_CRYPTO;
+	mmc->caps2 |= MMC_CAP2_CRYPTO_NO_REPROG;
 	return 0;
 }
 
@@ -2497,6 +2498,11 @@ static int sdhci_msm_gcc_reset(struct device *dev, struct sdhci_host *host)
 	usleep_range(200, 210);
 	reset_control_put(reset);
 
+#ifdef CONFIG_MMC_CRYPTO
+	if (host->mmc->caps2 & MMC_CAP2_CRYPTO)
+		blk_crypto_reprogram_all_keys(&host->mmc->crypto_profile);
+#endif
+
 	return ret;
 }
 
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 68f09a955a90..4a80da07cf39 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -459,6 +459,11 @@ struct mmc_host {
 #define MMC_CAP2_CRYPTO		0
 #endif
 #define MMC_CAP2_ALT_GPT_TEGRA	(1 << 28)	/* Host with eMMC that has GPT entry at a non-standard location */
+#ifdef CONFIG_MMC_CRYPTO
+#define MMC_CAP2_CRYPTO_NO_REPROG	(1 << 29)	/* Host handles inline crypto key reprogramming */
+#else
+#define MMC_CAP2_CRYPTO_NO_REPROG	0
+#endif
 
 	bool			uhs2_sd_tran;	/* UHS-II flag for SD_TRAN state */
 	bool			uhs2_app_cmd;	/* UHS-II flag for APP command */
-- 
2.34.1
Re: [PATCH v3] mmc: Avoid reprogram all keys to Inline Crypto Engine for MMC runtime suspend resume
Posted by Konrad Dybcio 6 months, 3 weeks ago
On 7/18/25 1:02 PM, Debraj Mukhopadhyay wrote:
> Crypto reprogram all keys is called for each MMC runtime
> suspend/resume in current upstream design. If this is implemented
> as a non-interruptible call to TEE for security, the cpu core is
> blocked for execution while this call executes although the crypto
> engine already has the keys. For example, glitches in audio/video
> streaming applications have been observed due to this. Add the flag
> MMC_CAP2_CRYPTO_NO_REPROG as part of host->caps2 to control reprogramming
> keys to crypto engine for socs which dont require this feature.
> 
> Signed-off-by: Seshu Madhavi Puppala <quic_spuppala@quicinc.com>
> Co-developed-by: Ram Prakash Gupta <quic_rampraka@quicinc.com>
> Signed-off-by: Ram Prakash Gupta <quic_rampraka@quicinc.com>
> Co-developed-by: Sarthak Garg <quic_sartgarg@quicinc.com>
> Signed-off-by: Sarthak Garg <quic_sartgarg@quicinc.com>
> Signed-off-by: Debraj Mukhopadhyay <quic_dmukhopa@quicinc.com>
> 
> ---

Let's take a step back - do we need to ever program this more than
once on QC? What about other devices (e.g. the generic cqhci-crypto)?
Do they also lose the crypto context over a runtime pm cycle?

If our hardware is fine with set-it-and-forget-it approach, maybe
we could limit this to a small if-condition sdhci-msm.c

[...]

> @@ -459,6 +459,11 @@ struct mmc_host {
>  #define MMC_CAP2_CRYPTO		0
>  #endif
>  #define MMC_CAP2_ALT_GPT_TEGRA	(1 << 28)	/* Host with eMMC that has GPT entry at a non-standard location */
> +#ifdef CONFIG_MMC_CRYPTO
> +#define MMC_CAP2_CRYPTO_NO_REPROG	(1 << 29)	/* Host handles inline crypto key reprogramming */
> +#else
> +#define MMC_CAP2_CRYPTO_NO_REPROG	0
> +#endif
This (and the crypto ifdef for MMC_CAP2_CRYPTO) looks like unnecessary
churn - crypto functions should never be called if the config is disabled
in the first place

Konrad
Re: [PATCH v3] mmc: Avoid reprogram all keys to Inline Crypto Engine for MMC runtime suspend resume
Posted by Neeraj Soni 1 month ago
Hi,

Aplogies for replying so late to the queries.

On 7/18/2025 5:35 PM, Konrad Dybcio wrote:
> On 7/18/25 1:02 PM, Debraj Mukhopadhyay wrote:
>> Crypto reprogram all keys is called for each MMC runtime
>> suspend/resume in current upstream design. If this is implemented
>> as a non-interruptible call to TEE for security, the cpu core is
>> blocked for execution while this call executes although the crypto
>> engine already has the keys. For example, glitches in audio/video
>> streaming applications have been observed due to this. Add the flag
>> MMC_CAP2_CRYPTO_NO_REPROG as part of host->caps2 to control reprogramming
>> keys to crypto engine for socs which dont require this feature.
>>
>> Signed-off-by: Seshu Madhavi Puppala <quic_spuppala@quicinc.com>
>> Co-developed-by: Ram Prakash Gupta <quic_rampraka@quicinc.com>
>> Signed-off-by: Ram Prakash Gupta <quic_rampraka@quicinc.com>
>> Co-developed-by: Sarthak Garg <quic_sartgarg@quicinc.com>
>> Signed-off-by: Sarthak Garg <quic_sartgarg@quicinc.com>
>> Signed-off-by: Debraj Mukhopadhyay <quic_dmukhopa@quicinc.com>
>>
>> ---
> 
> Let's take a step back - do we need to ever program this more than
> once on QC? What about other devices (e.g. the generic cqhci-crypto)?
> Do they also lose the crypto context over a runtime pm cycle?
> 
Yes crypto context is not lost during runtime pm as crypto context is tied to the host
where the context is preesrved in runtime pm cycle.
This needs to be programmed whenever host looses the crypto context like during power on
or device power reset.
 
> If our hardware is fine with set-it-and-forget-it approach, maybe
> we could limit this to a small if-condition sdhci-msm.c
QC hardware will be fine with this approach and we are doing so in this patch in
sdhci_msm_gcc_reset() API but it is not clear how other devices behave during runtime pm
hence a quirk is added in mmc_crypto_set_initial_state() to prevent reprogramming of
crypto context only for QC hardware.
> 
> [...]
> 
>> @@ -459,6 +459,11 @@ struct mmc_host {
>>  #define MMC_CAP2_CRYPTO		0
>>  #endif
>>  #define MMC_CAP2_ALT_GPT_TEGRA	(1 << 28)	/* Host with eMMC that has GPT entry at a non-standard location */
>> +#ifdef CONFIG_MMC_CRYPTO
>> +#define MMC_CAP2_CRYPTO_NO_REPROG	(1 << 29)	/* Host handles inline crypto key reprogramming */
>> +#else
>> +#define MMC_CAP2_CRYPTO_NO_REPROG	0
>> +#endif
> This (and the crypto ifdef for MMC_CAP2_CRYPTO) looks like unnecessary
> churn - crypto functions should never be called if the config is disabled
> in the first place
Do you suggest to remove #ifdef? I am not sure why #ifdef is used with MMC_CAP2_CRYPTO
(may be to reuse the bits if config is not defined) but for MMC_CAP2_CRYPTO_NO_REPROG we
followed the approach used for MMC_CAP2_CRYPTO.
> 
> Konrad
> 
Thanks
Neeraj
Re: [PATCH v3] mmc: Avoid reprogram all keys to Inline Crypto Engine for MMC runtime suspend resume
Posted by Konrad Dybcio 1 month ago
On 1/6/26 7:49 AM, Neeraj Soni wrote:
> Hi,
> 
> Aplogies for replying so late to the queries.
> 
> On 7/18/2025 5:35 PM, Konrad Dybcio wrote:
>> On 7/18/25 1:02 PM, Debraj Mukhopadhyay wrote:
>>> Crypto reprogram all keys is called for each MMC runtime
>>> suspend/resume in current upstream design. If this is implemented
>>> as a non-interruptible call to TEE for security, the cpu core is
>>> blocked for execution while this call executes although the crypto
>>> engine already has the keys. For example, glitches in audio/video
>>> streaming applications have been observed due to this. Add the flag
>>> MMC_CAP2_CRYPTO_NO_REPROG as part of host->caps2 to control reprogramming
>>> keys to crypto engine for socs which dont require this feature.
>>>
>>> Signed-off-by: Seshu Madhavi Puppala <quic_spuppala@quicinc.com>
>>> Co-developed-by: Ram Prakash Gupta <quic_rampraka@quicinc.com>
>>> Signed-off-by: Ram Prakash Gupta <quic_rampraka@quicinc.com>
>>> Co-developed-by: Sarthak Garg <quic_sartgarg@quicinc.com>
>>> Signed-off-by: Sarthak Garg <quic_sartgarg@quicinc.com>
>>> Signed-off-by: Debraj Mukhopadhyay <quic_dmukhopa@quicinc.com>
>>>
>>> ---

[...]

>>> @@ -459,6 +459,11 @@ struct mmc_host {
>>>  #define MMC_CAP2_CRYPTO		0
>>>  #endif
>>>  #define MMC_CAP2_ALT_GPT_TEGRA	(1 << 28)	/* Host with eMMC that has GPT entry at a non-standard location */
>>> +#ifdef CONFIG_MMC_CRYPTO
>>> +#define MMC_CAP2_CRYPTO_NO_REPROG	(1 << 29)	/* Host handles inline crypto key reprogramming */
>>> +#else
>>> +#define MMC_CAP2_CRYPTO_NO_REPROG	0
>>> +#endif
>> This (and the crypto ifdef for MMC_CAP2_CRYPTO) looks like unnecessary
>> churn - crypto functions should never be called if the config is disabled
>> in the first place
> Do you suggest to remove #ifdef? I am not sure why #ifdef is used with MMC_CAP2_CRYPTO
> (may be to reuse the bits if config is not defined) but for MMC_CAP2_CRYPTO_NO_REPROG we
> followed the approach used for MMC_CAP2_CRYPTO.

We got plenty of bits, let's not waste the time of a programmer trying
to figure out whether his particular combination of configs is going to
collide with the reuse, I'd argue the ifdef is unnecessary

Konrad
Re: [PATCH v3] mmc: Avoid reprogram all keys to Inline Crypto Engine for MMC runtime suspend resume
Posted by Neeraj Soni 1 month ago
Hi,

On 1/8/2026 3:13 PM, Konrad Dybcio wrote:
> On 1/6/26 7:49 AM, Neeraj Soni wrote:
>> Hi,
>>
>> Aplogies for replying so late to the queries.
>>
>> On 7/18/2025 5:35 PM, Konrad Dybcio wrote:
>>> On 7/18/25 1:02 PM, Debraj Mukhopadhyay wrote:
>>>> Crypto reprogram all keys is called for each MMC runtime
>>>> suspend/resume in current upstream design. If this is implemented
>>>> as a non-interruptible call to TEE for security, the cpu core is
>>>> blocked for execution while this call executes although the crypto
>>>> engine already has the keys. For example, glitches in audio/video
>>>> streaming applications have been observed due to this. Add the flag
>>>> MMC_CAP2_CRYPTO_NO_REPROG as part of host->caps2 to control reprogramming
>>>> keys to crypto engine for socs which dont require this feature.
>>>>
>>>> Signed-off-by: Seshu Madhavi Puppala <quic_spuppala@quicinc.com>
>>>> Co-developed-by: Ram Prakash Gupta <quic_rampraka@quicinc.com>
>>>> Signed-off-by: Ram Prakash Gupta <quic_rampraka@quicinc.com>
>>>> Co-developed-by: Sarthak Garg <quic_sartgarg@quicinc.com>
>>>> Signed-off-by: Sarthak Garg <quic_sartgarg@quicinc.com>
>>>> Signed-off-by: Debraj Mukhopadhyay <quic_dmukhopa@quicinc.com>
>>>>
>>>> ---
> 
> [...]
> 
>>>> @@ -459,6 +459,11 @@ struct mmc_host {
>>>>  #define MMC_CAP2_CRYPTO		0
>>>>  #endif
>>>>  #define MMC_CAP2_ALT_GPT_TEGRA	(1 << 28)	/* Host with eMMC that has GPT entry at a non-standard location */
>>>> +#ifdef CONFIG_MMC_CRYPTO
>>>> +#define MMC_CAP2_CRYPTO_NO_REPROG	(1 << 29)	/* Host handles inline crypto key reprogramming */
>>>> +#else
>>>> +#define MMC_CAP2_CRYPTO_NO_REPROG	0
>>>> +#endif
>>> This (and the crypto ifdef for MMC_CAP2_CRYPTO) looks like unnecessary
>>> churn - crypto functions should never be called if the config is disabled
>>> in the first place
>> Do you suggest to remove #ifdef? I am not sure why #ifdef is used with MMC_CAP2_CRYPTO
>> (may be to reuse the bits if config is not defined) but for MMC_CAP2_CRYPTO_NO_REPROG we
>> followed the approach used for MMC_CAP2_CRYPTO.
> 
> We got plenty of bits, let's not waste the time of a programmer trying
> to figure out whether his particular combination of configs is going to
> collide with the reuse, I'd argue the ifdef is unnecessary
>
Sure. I will remove the #ifdef from MMC_CAP2_CRYPTO and MMC_CAP2_CRYPTO_NO_REPROG in next patch.
 
> Konrad
> 
Regards
Neeraj
Re: [PATCH v3] mmc: Avoid reprogram all keys to Inline Crypto Engine for MMC runtime suspend resume
Posted by Ulf Hansson 6 months, 3 weeks ago
On Fri, 18 Jul 2025 at 14:05, Konrad Dybcio
<konrad.dybcio@oss.qualcomm.com> wrote:
>
> On 7/18/25 1:02 PM, Debraj Mukhopadhyay wrote:
> > Crypto reprogram all keys is called for each MMC runtime
> > suspend/resume in current upstream design. If this is implemented
> > as a non-interruptible call to TEE for security, the cpu core is
> > blocked for execution while this call executes although the crypto
> > engine already has the keys. For example, glitches in audio/video
> > streaming applications have been observed due to this. Add the flag
> > MMC_CAP2_CRYPTO_NO_REPROG as part of host->caps2 to control reprogramming
> > keys to crypto engine for socs which dont require this feature.
> >
> > Signed-off-by: Seshu Madhavi Puppala <quic_spuppala@quicinc.com>
> > Co-developed-by: Ram Prakash Gupta <quic_rampraka@quicinc.com>
> > Signed-off-by: Ram Prakash Gupta <quic_rampraka@quicinc.com>
> > Co-developed-by: Sarthak Garg <quic_sartgarg@quicinc.com>
> > Signed-off-by: Sarthak Garg <quic_sartgarg@quicinc.com>
> > Signed-off-by: Debraj Mukhopadhyay <quic_dmukhopa@quicinc.com>
> >
> > ---
>
> Let's take a step back - do we need to ever program this more than
> once on QC? What about other devices (e.g. the generic cqhci-crypto)?
> Do they also lose the crypto context over a runtime pm cycle?

I agree!

I also think it's important to also understand what runtime PM cycle
we are discussing here. It's a bit blurry for me currently, can we
please clarify this.

A runtime PM cycle of the card, means that the eMMC card is
power-cycled and re-initialized (assuming MMC_CAP_AGGRESSIVE_PM is
set, which I guess is a downstream patch as the upstream sdhci-msm
driver doesn't have this bit set, at least not yet). The mmc host is
probably also runtime PM power-cycled when the card is, but it's
orthogonal to the runtime PM cycle of the card - that's a really
important point here, I think.

As I understand it, the crypto context is not tied to the card, but to
the mmc host. What happens with the crypto context when the mmc host
is runtime PM cycled? Is the context preserved? I assume so, or?

>
> If our hardware is fine with set-it-and-forget-it approach, maybe
> we could limit this to a small if-condition sdhci-msm.c

Yes, maybe. Let's see.

[...]

Kind regards
Uffe
Re: [PATCH v3] mmc: Avoid reprogram all keys to Inline Crypto Engine for MMC runtime suspend resume
Posted by Neeraj Soni 1 month ago
Hi,

Apologies on replying so late to the queries.

On 7/18/2025 6:27 PM, Ulf Hansson wrote:
> On Fri, 18 Jul 2025 at 14:05, Konrad Dybcio
> <konrad.dybcio@oss.qualcomm.com> wrote:
>>
>> On 7/18/25 1:02 PM, Debraj Mukhopadhyay wrote:
>>> Crypto reprogram all keys is called for each MMC runtime
>>> suspend/resume in current upstream design. If this is implemented
>>> as a non-interruptible call to TEE for security, the cpu core is
>>> blocked for execution while this call executes although the crypto
>>> engine already has the keys. For example, glitches in audio/video
>>> streaming applications have been observed due to this. Add the flag
>>> MMC_CAP2_CRYPTO_NO_REPROG as part of host->caps2 to control reprogramming
>>> keys to crypto engine for socs which dont require this feature.
>>>
>>> Signed-off-by: Seshu Madhavi Puppala <quic_spuppala@quicinc.com>
>>> Co-developed-by: Ram Prakash Gupta <quic_rampraka@quicinc.com>
>>> Signed-off-by: Ram Prakash Gupta <quic_rampraka@quicinc.com>
>>> Co-developed-by: Sarthak Garg <quic_sartgarg@quicinc.com>
>>> Signed-off-by: Sarthak Garg <quic_sartgarg@quicinc.com>
>>> Signed-off-by: Debraj Mukhopadhyay <quic_dmukhopa@quicinc.com>
>>>
>>> ---
>>
>> Let's take a step back - do we need to ever program this more than
>> once on QC? What about other devices (e.g. the generic cqhci-crypto)?
>> Do they also lose the crypto context over a runtime pm cycle?
> 
> I agree!
> 
> I also think it's important to also understand what runtime PM cycle
> we are discussing here. It's a bit blurry for me currently, can we
> please clarify this.
> 
> A runtime PM cycle of the card, means that the eMMC card is
> power-cycled and re-initialized (assuming MMC_CAP_AGGRESSIVE_PM is
> set, which I guess is a downstream patch as the upstream sdhci-msm
> driver doesn't have this bit set, at least not yet). The mmc host is
> probably also runtime PM power-cycled when the card is, but it's
> orthogonal to the runtime PM cycle of the card - that's a really
> important point here, I think.
> 
> As I understand it, the crypto context is not tied to the card, but to
> the mmc host. What happens with the crypto context when the mmc host
> is runtime PM cycled? Is the context preserved? I assume so, or?
Yes the crypto context is tied to the host not to the card. In QC case when 
the host is runtime pm cycled the crypto contexts are preserved hence reprogramming
is not needed.
> 
>>
>> If our hardware is fine with set-it-and-forget-it approach, maybe
>> we could limit this to a small if-condition sdhci-msm.c
> 
> Yes, maybe. Let's see> 
> [...]
> 
> Kind regards
> Uffe
> 
>