Re: [PATCH v6] mmc: sdhci-msm: Enable ICE for CQE-capable controllers with non-CQE cards

[Md Sadre Alam <quic_mdalam@quicinc.com>]

Hi,

On 1/21/2026 6:14 PM, Ulf Hansson wrote:
> On Wed, 26 Nov 2025 at 07:43, Md Sadre Alam <quic_mdalam@quicinc.com> wrote:
>>
>> Enable Inline Crypto Engine (ICE) support for CQE-capable sdhci-msm
>> controllers when used with eMMC cards that do not support CQE.
>>
>> This addresses the scenario where:
>> - The host controller supports CQE (and has CQHCI crypto infrastructure)
>> - The eMMC card does not support CQE
>> - Standard (non-CMDQ) requests need crypto support
>>
>> This allows hardware-accelerated encryption and decryption for standard
>> requests on CQE-capable hardware by utilizing the existing CQHCI crypto
>> register space even when CQE functionality is not available due to card
>> limitations.
>>
>> The implementation:
>> - Adds ICE register definitions for non-CQE crypto configuration
>> - Implements per-request crypto setup via sdhci_msm_ice_cfg()
>> - Hooks into the request path via mmc_host_ops.request for non-CQE requests
>> - Uses CQHCI register space (NONCQ_CRYPTO_PARM/DUN) for crypto configuration
>>
>> With this, CQE-capable controllers can benefit from inline encryption
>> when paired with non-CQE cards, improving performance for encrypted I/O
>> while maintaining compatibility with existing CQE crypto support.
>>
>> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
>> Acked-by: Adrian Hunter <adrian.hunter@intel.com>
> 
> This has been applied for next since a few weeks, but I have a
> question. See more below.
> 
> [...]
> 
>>
>>   drivers/mmc/host/sdhci-msm.c | 77 ++++++++++++++++++++++++++++++++++++
>>   1 file changed, 77 insertions(+)
>>
>> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
>> index 3b85233131b3..da356627d9de 100644
>> --- a/drivers/mmc/host/sdhci-msm.c
>> +++ b/drivers/mmc/host/sdhci-msm.c
>> @@ -157,6 +157,17 @@
>>   #define CQHCI_VENDOR_CFG1      0xA00
>>   #define CQHCI_VENDOR_DIS_RST_ON_CQ_EN  (0x3 << 13)
>>
>> +/* non command queue crypto enable register*/
>> +#define NONCQ_CRYPTO_PARM              0x70
>> +#define NONCQ_CRYPTO_DUN               0x74
>> +
>> +#define DISABLE_CRYPTO                 BIT(15)
>> +#define CRYPTO_GENERAL_ENABLE          BIT(1)
>> +#define HC_VENDOR_SPECIFIC_FUNC4       0x260
>> +
>> +#define ICE_HCI_PARAM_CCI      GENMASK(7, 0)
>> +#define ICE_HCI_PARAM_CE       GENMASK(8, 8)
>> +
>>   struct sdhci_msm_offset {
>>          u32 core_hc_mode;
>>          u32 core_mci_data_cnt;
>> @@ -300,6 +311,7 @@ struct sdhci_msm_host {
>>          u32 dll_config;
>>          u32 ddr_config;
>>          bool vqmmc_enabled;
>> +       bool non_cqe_ice_init_done;
>>   };
>>
>>   static const struct sdhci_msm_offset *sdhci_priv_msm_offset(struct sdhci_host *host)
>> @@ -2012,6 +2024,68 @@ static int sdhci_msm_ice_keyslot_evict(struct blk_crypto_profile *profile,
>>          return qcom_ice_evict_key(msm_host->ice, slot);
>>   }
>>
>> +static void sdhci_msm_non_cqe_ice_init(struct sdhci_host *host)
>> +{
>> +       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> +       struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>> +       struct mmc_host *mmc = msm_host->mmc;
>> +       struct cqhci_host *cq_host = mmc->cqe_private;
>> +       u32 config;
>> +
>> +       config = sdhci_readl(host, HC_VENDOR_SPECIFIC_FUNC4);
>> +       config &= ~DISABLE_CRYPTO;
>> +       sdhci_writel(host, config, HC_VENDOR_SPECIFIC_FUNC4);
>> +       config = cqhci_readl(cq_host, CQHCI_CFG);
>> +       config |= CRYPTO_GENERAL_ENABLE;
>> +       cqhci_writel(cq_host, config, CQHCI_CFG);
>> +}
>> +
>> +static void sdhci_msm_ice_cfg(struct sdhci_host *host, struct mmc_request *mrq)
>> +{
>> +       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> +       struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>> +       struct mmc_host *mmc = msm_host->mmc;
>> +       struct cqhci_host *cq_host = mmc->cqe_private;
>> +       unsigned int crypto_params = 0;
>> +       int key_index;
>> +
>> +       if (mrq->crypto_ctx) {
>> +               if (!msm_host->non_cqe_ice_init_done) {
>> +                       sdhci_msm_non_cqe_ice_init(host);
>> +                       msm_host->non_cqe_ice_init_done = true;
>> +               }
>> +
>> +               key_index = mrq->crypto_key_slot;
>> +               crypto_params = FIELD_PREP(ICE_HCI_PARAM_CE, 1) |
>> +                               FIELD_PREP(ICE_HCI_PARAM_CCI, key_index);
>> +
>> +               cqhci_writel(cq_host, crypto_params, NONCQ_CRYPTO_PARM);
>> +               cqhci_writel(cq_host, lower_32_bits(mrq->crypto_ctx->bc_dun[0]),
>> +                            NONCQ_CRYPTO_DUN);
>> +       } else {
>> +               cqhci_writel(cq_host, crypto_params, NONCQ_CRYPTO_PARM);
> 
> Should we really call cqhci_writel() here, even if
> sdhci_msm_non_cqe_ice_init() has not been called yet?
> 
Thanks for the pointing this. The else branch is intentional.
for plain requests we must clear the crypto registers, otherwise
the ICE hardware would continue using the previous encryption context.
ICE initialization is only required when programming an encryption 
context, so we don’t call sdhci_msm_non_cqe_ice_init() in the plain 
path. Writing 0 to NONCQ_CRYPTO_PARM is safe without ICE init and 
ensures that subsequent plain requests are handled correctly

Thanks,
Alam.