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

[Eric Biggers <ebiggers@kernel.org>]

On Tue, Nov 11, 2025 at 04:16:04PM +0530, Md Sadre Alam wrote:
> Enable Inline Crypto Engine (ICE) support for eMMC devices that operate
> without Command Queue Engine (CQE).This allows hardware-accelerated
> encryption and decryption for standard (non-CMDQ) requests.
> 
> This patch:
> - Adds ICE register definitions for non-CMDQ crypto configuration
> - Implements a per-request crypto setup via sdhci_msm_ice_cfg()
> - Hooks into the request path via mmc_host_ops.request
> 
> With this, non-CMDQ eMMC devices can benefit from inline encryption,
> improving performance for encrypted I/O while maintaining compatibility
> with existing CQE crypto support.

This really should explain that this patch actually applies only to host
controllers that *do* support CQE.  Just they are using a card that
doesn't support CQE or CQE was explicitly disabled.  Right?

> +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;
> +	u32 ice_cap;
> +
> +	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);
> +}
> +
> +static int 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;
> +	bool crypto_enable;
> +	u64 dun = 0;
> +
> +	if (mrq->crypto_ctx) {
> +		if (!msm_host->ice_init_done) {
> +			sdhci_msm_non_cqe_ice_init(host);
> +			msm_host->ice_init_done = true;
> +		}

This means sdhci_msm_ice_enable() is called only once per host
controller.  It looks like the existing call to sdhci_msm_ice_enable()
happens each time after the host controller is resumed.  So there seems
to be an inconsistency there.  Which way is correct?

> +	} else {
> +		crypto_enable = false;
> +		key_index = 0;
> +		cqhci_writel(cq_host, crypto_params, NONCQ_CRYPTO_PARM);

The values assigned to 'crypto_enable' and 'key_index' are never used.

> +static void sdhci_msm_request(struct mmc_host *mmc, struct mmc_request *mrq)
> +{

Could you leave a comment here that notes this is used only for non-CQE
requests and that crypto on CQE requests is handled elsewhere?

- Eric