Re: [PATCH] mmc: sdhci-msm: Enable ICE support for non-cmdq eMMC devices

From: Md Sadre Alam

Date: Tue Oct 14 2025 - 02:56:29 EST




On 10/13/2025 4:23 PM, Adrian Hunter wrote:
On 13/10/2025 12:09, Md Sadre Alam wrote:
Hi,

On 10/9/2025 5:59 PM, Adrian Hunter wrote:
On 08/10/2025 14:07, Md Sadre Alam wrote:
Enable Inline Crypto Engine (ICE) support for eMMC devices that don't
use command queuing (CQE). This allows hardware-accelerated encryption
and decryption for standard eMMC operations without command queuing.

The changes include:
- Add non-cmdq crypto register definitions
- Implement crypto configuration callback for non-cmdq operations
- Initialize ICE hardware during host setup for non-cmdq devices
- Integrate crypto configuration into the main request path

This enables non-cmdq eMMC devices to benefit from hardware crypto
acceleration, improving performance for encrypted storage operations
while maintaining compatibility with existing cmdq crypto support.

Signed-off-by: Md Sadre Alam <quic_mdalam@xxxxxxxxxxx>
---
  drivers/mmc/host/cqhci.h     |  4 ++
  drivers/mmc/host/sdhci-msm.c | 74 +++++++++++++++++++++++++++++++++++-
  drivers/mmc/host/sdhci.c     | 20 ++++++++++
  drivers/mmc/host/sdhci.h     |  2 +
  4 files changed, 99 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/cqhci.h b/drivers/mmc/host/cqhci.h
index ce189a1866b9..9bf236e27675 100644
--- a/drivers/mmc/host/cqhci.h
+++ b/drivers/mmc/host/cqhci.h
@@ -119,6 +119,10 @@
  /* command response argument */
  #define CQHCI_CRA            0x5C
  +/* non command queue crypto enable register*/
+#define NONCQ_CRYPTO_PARM        0x70
+#define NONCQ_CRYPTO_DUN        0x74

Since cqhci is not using these, they might be better in sdhci-msm.c
Ok

+
  /* crypto capabilities */
  #define CQHCI_CCAP            0x100
  #define CQHCI_CRYPTOCAP            0x104
diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 4e5edbf2fc9b..2204c6abb3fe 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -157,6 +157,23 @@
  #define CQHCI_VENDOR_CFG1    0xA00
  #define CQHCI_VENDOR_DIS_RST_ON_CQ_EN    (0x3 << 13)
  +#define DISABLE_CRYPTO            BIT(15)
+#define CRYPTO_GENERAL_ENABLE        BIT(1)
+#define HC_VENDOR_SPECIFIC_FUNC4    0x260
+#define ICE_HCI_SUPPORT            BIT(28)
+
+/* SDHCI MSM ICE CTRL Info register offset */
+enum {
+    OFFSET_SDHCI_MSM_ICE_HCI_PARAM_CCI    = 0,
+    OFFSET_SDHCI_MSM_ICE_HCI_PARAM_CE    = 8,
+};
+
+/* SDHCI MSM ICE CTRL Info register masks */
+enum {
+    MASK_SDHCI_MSM_ICE_HCI_PARAM_CE        = 0x1,
+    MASK_SDHCI_MSM_ICE_HCI_PARAM_CCI    = 0xff
+};

Preferably use GENMASK() and FIELD_PREP()
Ok

+
  struct sdhci_msm_offset {
      u32 core_hc_mode;
      u32 core_mci_data_cnt;
@@ -1882,9 +1899,47 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
   * Inline Crypto Engine (ICE) support                                        *
   *                                                                           *
  \*****************************************************************************/
-

Unnecessary to delete this line
Ok

  #ifdef CONFIG_MMC_CRYPTO
  +static int sdhci_msm_ice_cfg(struct sdhci_host *host, struct mmc_request *mrq,
+                 u32 slot)
+{
+    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 = 0;
+    bool bypass = true;
+    u64 dun = 0;
+
+    if (!mrq || !cq_host)
+        return -EINVAL;

It should not be possible to get here if (!mrq || !cq_host)
Ok, will remove it in next revision.

+
+    if (mrq->crypto_ctx) {
+        dun = mrq->crypto_ctx->bc_dun[0];
+        bypass = false;
+        key_index = mrq->crypto_key_slot;
+    }
+
+    /* Configure ICE bypass mode */
+    crypto_params |= ((!bypass) & MASK_SDHCI_MSM_ICE_HCI_PARAM_CE)
+             << OFFSET_SDHCI_MSM_ICE_HCI_PARAM_CE;
+    /* Configure Crypto Configure Index (CCI) */
+    crypto_params |= (key_index & MASK_SDHCI_MSM_ICE_HCI_PARAM_CCI)
+             << OFFSET_SDHCI_MSM_ICE_HCI_PARAM_CCI;
+
+    cqhci_writel(cq_host, crypto_params, NONCQ_CRYPTO_PARM);
+
+    if (mrq->crypto_ctx)
+        cqhci_writel(cq_host, lower_32_bits(dun), NONCQ_CRYPTO_DUN);
+
+    /* Ensure crypto configuration is written before proceeding */
+    wmb();
+
+    return 0;
+}
+
  static const struct blk_crypto_ll_ops sdhci_msm_crypto_ops; /* forward decl */
    static int sdhci_msm_ice_init(struct sdhci_msm_host *msm_host,
@@ -2131,6 +2186,8 @@ static int sdhci_msm_cqe_add_host(struct sdhci_host *host,
      struct cqhci_host *cq_host;
      bool dma64;
      u32 cqcfg;
+    u32 config;
+    u32 ice_cap;
      int ret;
        /*
@@ -2185,6 +2242,18 @@ static int sdhci_msm_cqe_add_host(struct sdhci_host *host,
      if (ret)
          goto cleanup;
  +    /* Initialize ICE for non-CMDQ eMMC devices */
+    config = sdhci_readl(host, HC_VENDOR_SPECIFIC_FUNC4);
+    config &= ~DISABLE_CRYPTO;
+    sdhci_writel(host, config, HC_VENDOR_SPECIFIC_FUNC4);
+    ice_cap = cqhci_readl(cq_host, CQHCI_CAP);
+    if (ice_cap & ICE_HCI_SUPPORT) {
+        config = cqhci_readl(cq_host, CQHCI_CFG);
+        config |= CRYPTO_GENERAL_ENABLE;
+        cqhci_writel(cq_host, config, CQHCI_CFG);
+    }
+    sdhci_msm_ice_enable(msm_host);
+
      dev_info(&pdev->dev, "%s: CQE init: success\n",
              mmc_hostname(host->mmc));
      return ret;
@@ -2450,6 +2519,9 @@ static const struct of_device_id sdhci_msm_dt_match[] = {
  MODULE_DEVICE_TABLE(of, sdhci_msm_dt_match);
    static const struct sdhci_ops sdhci_msm_ops = {
+#ifdef CONFIG_MMC_CRYPTO
+    .crypto_engine_cfg = sdhci_msm_ice_cfg,
+#endif
      .reset = sdhci_and_cqhci_reset,
      .set_clock = sdhci_msm_set_clock,
      .get_min_clock = sdhci_msm_get_min_clock,
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index ac7e11f37af7..2d636a8ee452 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2202,6 +2202,21 @@ void sdhci_set_power_and_bus_voltage(struct sdhci_host *host,
  }
  EXPORT_SYMBOL_GPL(sdhci_set_power_and_bus_voltage);
  +static int sdhci_crypto_cfg(struct sdhci_host *host, struct mmc_request *mrq,
+                u32 slot)
+{
+    int err = 0;
+
+    if (host->ops->crypto_engine_cfg) {
+        err = host->ops->crypto_engine_cfg(host, mrq, slot);
+        if (err)
+            pr_err("%s: failed to configure crypto: %d\n",
+                   mmc_hostname(host->mmc), err);
+    }
+
+    return err;
+}
+
  /*****************************************************************************\
   *                                                                           *
   * MMC callbacks                                                             *
@@ -2227,6 +2242,11 @@ void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
        cmd = sdhci_manual_cmd23(host, mrq) ? mrq->sbc : mrq->cmd;
  +    if (mmc->caps2 & MMC_CAP2_CRYPTO) {
+        if (sdhci_crypto_cfg(host, mrq, 0))
+            goto out_finish;
+    }

It would be preferable to hook the >request() callback e.g.

    host->mmc_host_ops.request = sdhci_msm_request;

void sdhci_msm_request(struct mmc_host *mmc, struct mmc_request *mrq)
{
    if (mmc->caps2 & MMC_CAP2_CRYPTO) {
        etc
    }

    sdhci_request(mmc, mrq);
}
Thanks for the suggestion. I Will update the patch to override the mmc_host_ops.request callback in sdhci-msm.c via a platform-specific wrapper (sdhci_msm_request). Since mmc->ops is a const pointer, I Will clone the existing ops into a local copy

Can just update the sdhci ops directly:

host->mmc_host_ops.request = sdhci_msm_request;

(msm_mmc_ops) and replaced only the request field. This preserves all platform-specific callbacks like enable_sdio_irq and avoids probe failures. The change in probe function.
Thanks for the suggestion. Updating host->mmc_host_ops.request directly to sdhci_msm_request does indeed retain the platform-specific callbacks. I’ll incorporate this change in the next revision.

Thanks,
Alam.