| 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
| ||
|
Message-ID: <CAPDyKFpcGvhp82MTcN3wTeLrxErQC6Cficva341VVLWBf6zL9w@mail.gmail.com>
Date: Fri, 23 Jan 2026 10:38:33 +0100
From: Ulf Hansson <ulf.hansson@...aro.org>
To: Md Sadre Alam <quic_mdalam@...cinc.com>
Cc: adrian.hunter@...el.com, linux-arm-msm@...r.kernel.org,
linux-mmc@...r.kernel.org, linux-kernel@...r.kernel.org, abel.vesa@...aro.org,
ebiggers@...nel.org
Subject: Re: [PATCH v6] mmc: sdhci-msm: Enable ICE for CQE-capable controllers
with non-CQE cards
On Fri, 23 Jan 2026 at 07:10, Md Sadre Alam <quic_mdalam@...cinc.com> wrote:
>
> Hi,
>
> On 1/21/2026 6:14 PM, Ulf Hansson wrote:
> > On Wed, 26 Nov 2025 at 07:43, Md Sadre Alam <quic_mdalam@...cinc.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@...cinc.com>
> >> Acked-by: Adrian Hunter <adrian.hunter@...el.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 for clarifying!
Kind regards
Uffe
Powered by blists - more mailing lists