lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <f4363815-a5bc-4f5a-80a1-7d4a17ad539b@intel.com>
Date: Thu, 9 Oct 2025 15:29:20 +0300
From: Adrian Hunter <adrian.hunter@...el.com>
To: Md Sadre Alam <quic_mdalam@...cinc.com>, <quic_asutoshd@...cinc.com>,
	<ulf.hansson@...aro.org>, <linux-mmc@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, <linux-arm-msm@...r.kernel.org>,
	<quic_varada@...cinc.com>
Subject: Re: [PATCH] mmc: sdhci-msm: Enable ICE support for non-cmdq eMMC
 devices

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@...cinc.com>
> ---
>  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

> +
>  /* 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()

> +
>  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

>  #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)

> +
> +	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);
}

> +
>  	if (!sdhci_send_command_retry(host, cmd, flags))
>  		goto out_finish;
>  
> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> index b6a571d866fa..9ac32a787270 100644
> --- a/drivers/mmc/host/sdhci.h
> +++ b/drivers/mmc/host/sdhci.h
> @@ -709,6 +709,8 @@ struct sdhci_ops {
>  	unsigned int    (*get_ro)(struct sdhci_host *host);
>  	void		(*reset)(struct sdhci_host *host, u8 mask);
>  	int	(*platform_execute_tuning)(struct sdhci_host *host, u32 opcode);
> +	int	(*crypto_engine_cfg)(struct sdhci_host *host,
> +				     struct mmc_request *mrq, u32 slot);
>  	void	(*set_uhs_signaling)(struct sdhci_host *host, unsigned int uhs);
>  	void	(*hw_reset)(struct sdhci_host *host);
>  	void    (*adma_workaround)(struct sdhci_host *host, u32 intmask);


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ