Skip to contentSkip to conversation

eMMC inline encryption support

PATCH Superseded Eric Biggers 4 participants 17 messages Jan 21, 2021 – Jan 22, 2021

Subsystems (7)

v5 of 6 Superseded Latest is v6 →

lore
Share & tools
lore

Eric Biggers

author maintainer
#0 lore raw

v5 cover eMMC inline encryption support

Hello,

This patchset adds support for eMMC inline encryption, as specified by the upcoming version of the eMMC specification and as already implemented and used on many devices. Building on that, it then adds Qualcomm ICE support and wires it up for the Snapdragon 630 SoC.

Inline encryption hardware improves the performance of storage encryption and reduces power usage. See Documentation/block/inline-encryption.rst for more information about inline encryption and the blk-crypto framework (upstreamed in v5.8) which supports it. Most mobile devices already use UFS or eMMC inline encryption hardware; UFS support was already upstreamed in v5.9.

Patches 1-4 add support for the standard eMMC inline encryption.

However, as with UFS, host controller-specific patches are needed on top of the standard support. Therefore, patches 5-9 add Qualcomm ICE (Inline Crypto Engine) support and wire it up on the Snapdragon 630 SoC.

To test this I took advantage of the recently upstreamed support for the Snapdragon 630 SoC, plus work-in-progress patches from the SoMainline project (https://github.com/SoMainline/linux/tree/konrad/v5.10-rc3). In particular, I was able to run the fscrypt xfstests for ext4 and f2fs in a Debian chroot. Among other things, these tests verified that the correct ciphertext is written to disk (the same as software encryption).

It will also be possible to add support for Mediatek eMMC inline encryption hardware in mtk-sd, and it should be easier than the Qualcomm hardware since the Mediatek hardware follows the standard more closely. I.e., patches 1-4 should be almost enough for the Mediatek hardware.

This patchset is based on v5.11-rc2 plus the patch "block/keyslot-manager: introduce devm_blk_ksm_init()"
(https://lkml.kernel.org/r/[email protected]).
It can also be retrieved from tag "mmc-crypto-v5" of
https://git.kernel.org/pub/scm/linux/kernel/git/ebiggers/linux.git

Changed since v4:
- Use the proposed resource-managed variant of blk_ksm_init().
- Removed an unnecessary call to devm_kfree().

Changed since v3:
- Added Acked-by and Reviewed-and-tested-by tags.
- Rebased onto v5.11-rc2.

Changed since v2:
- Improved comment for sdhci_msm_ice_wait_bist_status()
- Removed an unhelpful comment in union cqhci_crypto_cfg_entry.
- Fixed the commit message of "mmc: cqhci: initialize upper 64 bits of 128-bit task descriptors".
- Added Reviewed-by's and Acked-by's.

Changed since v1:
- Only select QCOM_SCM if ARCH_QCOM. (Fixes a build break.)
- Split most of the cqhci_prep_task_desc() change into its own patch.
- Made sdhci_msm_ice_wait_bist_status() use readl_poll_timeout().
- Added a couple more comments.
- Added some Acked-by's.

Eric Biggers (9):
mmc: add basic support for inline encryption
mmc: cqhci: rename cqhci.c to cqhci-core.c
mmc: cqhci: initialize upper 64 bits of 128-bit task descriptors
mmc: cqhci: add support for inline encryption
mmc: cqhci: add cqhci_host_ops::program_key
firmware: qcom_scm: update comment for ICE-related functions
dt-bindings: mmc: sdhci-msm: add ICE registers and clock
arm64: dts: qcom: sdm630: add ICE registers and clocks
mmc: sdhci-msm: add Inline Crypto Engine support

 .../devicetree/bindings/mmc/sdhci-msm.txt     |   3 +
 arch/arm64/boot/dts/qcom/sdm630.dtsi          |  10 +-
 drivers/firmware/qcom_scm.c                   |  16 +-
 drivers/mmc/core/Kconfig                      |   8 +
 drivers/mmc/core/Makefile                     |   1 +
 drivers/mmc/core/block.c                      |   3 +
 drivers/mmc/core/core.c                       |   3 +
 drivers/mmc/core/crypto.c                     |  48 +++
 drivers/mmc/core/crypto.h                     |  40 +++
 drivers/mmc/core/host.c                       |   1 +
 drivers/mmc/core/queue.c                      |   3 +
 drivers/mmc/host/Kconfig                      |   1 +
 drivers/mmc/host/Makefile                     |   2 +
 drivers/mmc/host/{cqhci.c => cqhci-core.c}    |  69 ++++-
 drivers/mmc/host/cqhci-crypto.c               | 242 +++++++++++++++
 drivers/mmc/host/cqhci-crypto.h               |  47 +++
 drivers/mmc/host/cqhci.h                      |  84 +++++-
 drivers/mmc/host/sdhci-msm.c                  | 276 +++++++++++++++++-
 include/linux/mmc/core.h                      |   6 +
 include/linux/mmc/host.h                      |   7 +
 20 files changed, 845 insertions(+), 25 deletions(-)
 create mode 100644 drivers/mmc/core/crypto.c
 create mode 100644 drivers/mmc/core/crypto.h
 rename drivers/mmc/host/{cqhci.c => cqhci-core.c} (94%)
 create mode 100644 drivers/mmc/host/cqhci-crypto.c
 create mode 100644 drivers/mmc/host/cqhci-crypto.h
Signature · 1 line
2.30.0
author maintainer
#1 lore raw

PATCH v5 1/9 mmc: add basic support for inline encryption

From: Eric Biggers <[email protected]>

In preparation for adding CQHCI crypto engine (inline encryption) support, add the code required to make mmc_core and mmc_block aware of inline encryption. Specifically:

- Add a capability flag MMC_CAP2_CRYPTO to struct mmc_host. Drivers will set this if the host and driver support inline encryption.

- Embed a blk_keyslot_manager in struct mmc_host. Drivers will initialize this (as a device-managed resource) if the host and driver support inline encryption. mmc_block registers this keyslot manager with the request_queue of any MMC card attached to the host.

- Make mmc_block copy the crypto keyslot and crypto data unit number from struct request to struct mmc_request, so that drivers will have access to them.

- If the MMC host is reset, reprogram all the keyslots to ensure that the software state stays in sync with the hardware state.

---
 drivers/mmc/core/Kconfig  |  8 +++++++
 drivers/mmc/core/Makefile |  1 +
 drivers/mmc/core/block.c  |  3 +++
 drivers/mmc/core/core.c   |  3 +++
 drivers/mmc/core/crypto.c | 48 +++++++++++++++++++++++++++++++++++++++
 drivers/mmc/core/crypto.h | 40 ++++++++++++++++++++++++++++++++
 drivers/mmc/core/host.c   |  1 +
 drivers/mmc/core/queue.c  |  3 +++
 include/linux/mmc/core.h  |  6 +++++
 include/linux/mmc/host.h  |  7 ++++++
 10 files changed, 120 insertions(+)
 create mode 100644 drivers/mmc/core/crypto.c
 create mode 100644 drivers/mmc/core/crypto.h
drivers/mmc/core/Kconfig +8 -0
index c12fe13e4b147..ae8b69aee6190 100644
@@ -81,3 +81,11 @@ config MMC_TEST
 	  This driver is only of interest to those developing or
 	  testing a host driver. Most people should say N here.
 
+config MMC_CRYPTO
+	bool "MMC Crypto Engine Support"
+	depends on BLK_INLINE_ENCRYPTION
+	help
+	  Enable Crypto Engine Support in MMC.
+	  Enabling this makes it possible for the kernel to use the crypto
+	  capabilities of the MMC device (if present) to perform crypto
+	  operations on data being transferred to/from the device.
drivers/mmc/core/Makefile +1 -0
index 95ffe008ebdf8..6a907736cd7a5 100644
@@ -18,3 +18,4 @@ obj-$(CONFIG_MMC_BLOCK)		+= mmc_block.o
 mmc_block-objs			:= block.o queue.o
 obj-$(CONFIG_MMC_TEST)		+= mmc_test.o
 obj-$(CONFIG_SDIO_UART)		+= sdio_uart.o
+mmc_core-$(CONFIG_MMC_CRYPTO)	+= crypto.o
drivers/mmc/core/block.c +3 -0
index 42e27a2982180..b877f62df3660 100644
@@ -51,6 +51,7 @@
 #include "block.h"
 #include "core.h"
 #include "card.h"
+#include "crypto.h"
 #include "host.h"
 #include "bus.h"
 #include "mmc_ops.h"
@@ -1247,6 +1248,8 @@ static void mmc_blk_data_prep(struct mmc_queue *mq, struct mmc_queue_req *mqrq,
 
 	memset(brq, 0, sizeof(struct mmc_blk_request));
 
+	mmc_crypto_prepare_req(mqrq);
+
 	brq->mrq.data = &brq->data;
 	brq->mrq.tag = req->tag;
 
drivers/mmc/core/core.c +3 -0
index 19f1ee57fb345..bd4b557e68899 100644
@@ -37,6 +37,7 @@
 
 #include "core.h"
 #include "card.h"
+#include "crypto.h"
 #include "bus.h"
 #include "host.h"
 #include "sdio_bus.h"
@@ -992,6 +993,8 @@ void mmc_set_initial_state(struct mmc_host *host)
 		host->ops->hs400_enhanced_strobe(host, &host->ios);
 
 	mmc_set_ios(host);
+
+	mmc_crypto_set_initial_state(host);
 }
 
 /**
drivers/mmc/core/crypto.c new file +48 -0

48 lines not on this page · raw message

drivers/mmc/core/crypto.h new file +40 -0
new file mode 100644
index 0000000000000..fbe9a520bf90d
@@ -0,0 +1,40 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * MMC crypto engine (inline encryption) support
+ *
+ * Copyright 2020 Google LLC
+ */
+
+#ifndef _MMC_CORE_CRYPTO_H
+#define _MMC_CORE_CRYPTO_H
+
+struct mmc_host;
+struct mmc_queue_req;
+struct request_queue;
+
+#ifdef CONFIG_MMC_CRYPTO
+
+void mmc_crypto_set_initial_state(struct mmc_host *host);
+
+void mmc_crypto_setup_queue(struct request_queue *q, struct mmc_host *host);
+
+void mmc_crypto_prepare_req(struct mmc_queue_req *mqrq);
+
+#else /* CONFIG_MMC_CRYPTO */
+
+static inline void mmc_crypto_set_initial_state(struct mmc_host *host)
+{
+}
+
+static inline void mmc_crypto_setup_queue(struct request_queue *q,
+					  struct mmc_host *host)
+{
+}
+
+static inline void mmc_crypto_prepare_req(struct mmc_queue_req *mqrq)
+{
+}
+
+#endif /* !CONFIG_MMC_CRYPTO */
+
+#endif /* _MMC_CORE_CRYPTO_H */
drivers/mmc/core/host.c +1 -0
index 96b2ca1f1b06d..6be85e014be5d 100644
@@ -25,6 +25,7 @@
 #include <linux/mmc/slot-gpio.h>
 
 #include "core.h"
+#include "crypto.h"
 #include "host.h"
 #include "slot-gpio.h"
 #include "pwrseq.h"
drivers/mmc/core/queue.c +3 -0
index de7cb0369c308..d96db852bb91a 100644
@@ -19,6 +19,7 @@
 #include "block.h"
 #include "core.h"
 #include "card.h"
+#include "crypto.h"
 #include "host.h"
 
 #define MMC_DMA_MAP_MERGE_SEGMENTS	512
@@ -405,6 +406,8 @@ static void mmc_setup_queue(struct mmc_queue *mq, struct mmc_card *card)
 	mutex_init(&mq->complete_lock);
 
 	init_waitqueue_head(&mq->wait);
+
+	mmc_crypto_setup_queue(mq->queue, host);
 }
 
 static inline bool mmc_merge_capable(struct mmc_host *host)
include/linux/mmc/core.h +6 -0
index 29aa507116261..ab19245e99451 100644
@@ -162,6 +162,12 @@ struct mmc_request {
 	bool			cap_cmd_during_tfr;
 
 	int			tag;
+
+#ifdef CONFIG_MMC_CRYPTO
+	bool			crypto_enabled;
+	int			crypto_key_slot;
+	u32			data_unit_num;
+#endif
 };
 
 struct mmc_card;
include/linux/mmc/host.h +7 -0
index 01bba36545c54..6f86948f92caf 100644
@@ -15,6 +15,7 @@
 #include <linux/mmc/card.h>
 #include <linux/mmc/pm.h>
 #include <linux/dma-direction.h>
+#include <linux/keyslot-manager.h>
 
 struct mmc_ios {
 	unsigned int	clock;			/* clock rate */
@@ -384,6 +385,7 @@ struct mmc_host {
 #define MMC_CAP2_CQE_DCMD	(1 << 24)	/* CQE can issue a direct command */
 #define MMC_CAP2_AVOID_3_3V	(1 << 25)	/* Host must negotiate down from 3.3V */
 #define MMC_CAP2_MERGE_CAPABLE	(1 << 26)	/* Host can merge a segment over the segment size */
+#define MMC_CAP2_CRYPTO		(1 << 27)	/* Host supports inline encryption */
 
 	int			fixed_drv_type;	/* fixed driver type for non-removable media */
 
@@ -478,6 +480,11 @@ struct mmc_host {
 	bool			cqe_enabled;
 	bool			cqe_on;
 
+	/* Inline encryption support */
+#ifdef CONFIG_MMC_CRYPTO
+	struct blk_keyslot_manager ksm;
+#endif
+
 	/* Host Software Queue support */
 	bool			hsq_enabled;
 
Signature · 1 line
2.30.0
author maintainer
#2 lore raw

PATCH v5 2/9 mmc: cqhci: rename cqhci.c to cqhci-core.c

In reply to Eric Biggers

From: Eric Biggers <[email protected]>

Rename cqhci.c to cqhci-core.c so that another source file can be added to the cqhci module without having to rename the module.

---
 drivers/mmc/host/Makefile                  | 1 +
 drivers/mmc/host/{cqhci.c => cqhci-core.c} | 0
 2 files changed, 1 insertion(+)
 rename drivers/mmc/host/{cqhci.c => cqhci-core.c} (100%)
drivers/mmc/host/Makefile +1 -0
index 451c25fc2c692..20c2f9463d0dc 100644
@@ -104,6 +104,7 @@ obj-$(CONFIG_MMC_SDHCI_BRCMSTB)		+= sdhci-brcmstb.o
 obj-$(CONFIG_MMC_SDHCI_OMAP)		+= sdhci-omap.o
 obj-$(CONFIG_MMC_SDHCI_SPRD)		+= sdhci-sprd.o
 obj-$(CONFIG_MMC_CQHCI)			+= cqhci.o
+cqhci-y					+= cqhci-core.o
 obj-$(CONFIG_MMC_HSQ)			+= mmc_hsq.o
 
 ifeq ($(CONFIG_CB710_DEBUG),y)
drivers/mmc/host/cqhci-core.c renamed from drivers/mmc/host/cqhci.c +0 -0
similarity index 100%
rename from drivers/mmc/host/cqhci.c
rename to drivers/mmc/host/cqhci-core.c
Signature · 1 line
2.30.0
author maintainer
#3 lore raw

PATCH v5 3/9 mmc: cqhci: initialize upper 64 bits of 128-bit task descriptors

In reply to Eric Biggers

From: Eric Biggers <[email protected]>

Move the task descriptor initialization into cqhci_prep_task_desc(). In addition, make it explicitly initialize all 128 bits of the task descriptor if the host controller is using 128-bit task descriptors, rather than relying on the implicit zeroing from dmam_alloc_coherent().

This is needed to prepare for CQHCI inline encryption support, which requires 128-bit task descriptors and uses the upper 64 bits.

---
 drivers/mmc/host/cqhci-core.c | 30 ++++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)
drivers/mmc/host/cqhci-core.c +20 -10
index 697fe40756bf2..ad7c9acff1728 100644
@@ -408,13 +408,15 @@ static void cqhci_disable(struct mmc_host *mmc)
 }
 
 static void cqhci_prep_task_desc(struct mmc_request *mrq,
-					u64 *data, bool intr)
+				 struct cqhci_host *cq_host, int tag)
 {
+	__le64 *task_desc = (__le64 __force *)get_desc(cq_host, tag);
 	u32 req_flags = mrq->data->flags;
+	u64 desc0;
 
-	*data = CQHCI_VALID(1) |
+	desc0 = CQHCI_VALID(1) |
 		CQHCI_END(1) |
-		CQHCI_INT(intr) |
+		CQHCI_INT(1) |
 		CQHCI_ACT(0x5) |
 		CQHCI_FORCED_PROG(!!(req_flags & MMC_DATA_FORCED_PRG)) |
 		CQHCI_DATA_TAG(!!(req_flags & MMC_DATA_DAT_TAG)) |
@@ -425,8 +427,19 @@ static void cqhci_prep_task_desc(struct mmc_request *mrq,
 		CQHCI_BLK_COUNT(mrq->data->blocks) |
 		CQHCI_BLK_ADDR((u64)mrq->data->blk_addr);
 
-	pr_debug("%s: cqhci: tag %d task descriptor 0x%016llx\n",
-		 mmc_hostname(mrq->host), mrq->tag, (unsigned long long)*data);
+	task_desc[0] = cpu_to_le64(desc0);
+
+	if (cq_host->caps & CQHCI_TASK_DESC_SZ_128) {
+		u64 desc1 = 0;
+
+		task_desc[1] = cpu_to_le64(desc1);
+
+		pr_debug("%s: cqhci: tag %d task descriptor 0x%016llx%016llx\n",
+			 mmc_hostname(mrq->host), mrq->tag, desc1, desc0);
+	} else {
+		pr_debug("%s: cqhci: tag %d task descriptor 0x%016llx\n",
+			 mmc_hostname(mrq->host), mrq->tag, desc0);
+	}
 }
 
 static int cqhci_dma_map(struct mmc_host *host, struct mmc_request *mrq)
@@ -567,8 +580,6 @@ static inline int cqhci_tag(struct mmc_request *mrq)
 static int cqhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 {
 	int err = 0;
-	u64 data = 0;
-	u64 *task_desc = NULL;
 	int tag = cqhci_tag(mrq);
 	struct cqhci_host *cq_host = mmc->cqe_private;
 	unsigned long flags;
@@ -598,9 +609,8 @@ static int cqhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 	}
 
 	if (mrq->data) {
-		task_desc = (__le64 __force *)get_desc(cq_host, tag);
-		cqhci_prep_task_desc(mrq, &data, 1);
-		*task_desc = cpu_to_le64(data);
+		cqhci_prep_task_desc(mrq, cq_host, tag);
+
 		err = cqhci_prep_tran_desc(mrq, cq_host, tag);
 		if (err) {
 			pr_err("%s: cqhci: failed to setup tx desc: %d\n",
Signature · 1 line
2.30.0
author maintainer
#4 lore raw

PATCH v5 4/9 mmc: cqhci: add support for inline encryption

In reply to Eric Biggers

From: Eric Biggers <[email protected]>

Add support for eMMC inline encryption using the blk-crypto framework (Documentation/block/inline-encryption.rst).

eMMC inline encryption support is specified by the upcoming JEDEC eMMC v5.2 specification. It is only specified for the CQ interface, not the non-CQ interface. Although the eMMC v5.2 specification hasn't been officially released yet, the crypto support was already agreed on several years ago, and it was already implemented by at least two major hardware vendors. Lots of hardware in the field already supports and uses it, e.g. Snapdragon 630 to give one example.

eMMC inline encryption support is very similar to the UFS inline encryption support which was standardized in the UFS v2.1 specification and was already upstreamed. The only major difference is that eMMC limits data unit numbers to 32 bits, unlike UFS's 64 bits.

Like we did with UFS, make the crypto support opt-in by individual drivers; don't enable it automatically whenever the hardware declares crypto support. This is necessary because in every case we've seen, some extra vendor-specific logic is needed to use the crypto support.

---
 drivers/mmc/host/Makefile       |   1 +
 drivers/mmc/host/cqhci-core.c   |  41 +++++-
 drivers/mmc/host/cqhci-crypto.c | 238 ++++++++++++++++++++++++++++++++
 drivers/mmc/host/cqhci-crypto.h |  47 +++++++
 drivers/mmc/host/cqhci.h        |  80 ++++++++++-
 5 files changed, 404 insertions(+), 3 deletions(-)
 create mode 100644 drivers/mmc/host/cqhci-crypto.c
 create mode 100644 drivers/mmc/host/cqhci-crypto.h
drivers/mmc/host/Makefile +1 -0
index 20c2f9463d0dc..35158508ab63d 100644
@@ -105,6 +105,7 @@ obj-$(CONFIG_MMC_SDHCI_OMAP)		+= sdhci-omap.o
 obj-$(CONFIG_MMC_SDHCI_SPRD)		+= sdhci-sprd.o
 obj-$(CONFIG_MMC_CQHCI)			+= cqhci.o
 cqhci-y					+= cqhci-core.o
+cqhci-$(CONFIG_MMC_CRYPTO)		+= cqhci-crypto.o
 obj-$(CONFIG_MMC_HSQ)			+= mmc_hsq.o
 
 ifeq ($(CONFIG_CB710_DEBUG),y)
drivers/mmc/host/cqhci-core.c +39 -2

83 lines not on this page · raw message

drivers/mmc/host/cqhci-crypto.c new file +238 -0

238 lines not on this page · raw message

drivers/mmc/host/cqhci-crypto.h new file +47 -0

47 lines not on this page · raw message

drivers/mmc/host/cqhci.h +79 -1

126 lines not on this page · raw message

Signature · 1 line
2.30.0
#5 lore raw

Re: [PATCH v5 4/9] mmc: cqhci: add support for inline encryption

I love your patch! Yet something to improve:

Hi Eric,

I love your patch! Yet something to improve:

[auto build test ERROR on robh/for-next]
[also build test ERROR on linus/master v5.11-rc4 next-20210121] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Eric-Biggers/mmc-add-basic-support-for-inline-encryption/20210121-180505
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: arc-allyesconfig (attached as .config)
compiler: arceb-elf-gcc (GCC) 9.3.0

reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/c5658e96c3e056ebf966e683859fa165948d1660
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Eric-Biggers/mmc-add-basic-support-for-inline-encryption/20210121-180505 git checkout c5658e96c3e056ebf966e683859fa165948d1660
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arc

If you fix the issue, kindly add following tag as appropriate

All errors (new ones prefixed by >>):

drivers/mmc/host/cqhci-crypto.c: In function 'cqhci_crypto_init':

>> drivers/mmc/host/cqhci-crypto.c:198:8: error: implicit declaration of function 'devm_blk_ksm_init'; did you mean 'blk_ksm_init'? [-Werror=implicit-function-declaration]
     198 |  err = devm_blk_ksm_init(dev, ksm, num_keyslots);
         |        ^~~~~~~~~~~~~~~~~
         |        blk_ksm_init

cc1: some warnings being treated as errors

vim +198 drivers/mmc/host/cqhci-crypto.c

149
150 /**
151 * cqhci_crypto_init - initialize CQHCI crypto support
152 * @cq_host: a cqhci host
153 *
154 * If the driver previously set MMC_CAP2_CRYPTO and the CQE declares
155 * CQHCI_CAP_CS, initialize the crypto support. This involves reading the
156 * crypto capability registers, initializing the keyslot manager, clearing all
157 * keyslots, and enabling 128-bit task descriptors.
158 *
159 * Return: 0 if crypto was initialized or isn't supported; whether
160 * MMC_CAP2_CRYPTO remains set indicates which one of those cases it is.
161 * Also can return a negative errno value on unexpected error.
162 */
163 int cqhci_crypto_init(struct cqhci_host *cq_host)
164 {
165 struct mmc_host *mmc = cq_host->mmc;
166 struct device *dev = mmc_dev(mmc);
167 struct blk_keyslot_manager *ksm = &mmc->ksm;
168 unsigned int num_keyslots;
169 unsigned int cap_idx;
170 enum blk_crypto_mode_num blk_mode_num;
171 unsigned int slot;
172 int err = 0;
173
174 if (!(mmc->caps2 & MMC_CAP2_CRYPTO) ||
175 !(cqhci_readl(cq_host, CQHCI_CAP) & CQHCI_CAP_CS))
176 goto out;
177
178 cq_host->crypto_capabilities.reg_val =
179 cpu_to_le32(cqhci_readl(cq_host, CQHCI_CCAP));
180
181 cq_host->crypto_cfg_register =
182 (u32)cq_host->crypto_capabilities.config_array_ptr * 0x100;
183
184 cq_host->crypto_cap_array =
185 devm_kcalloc(dev, cq_host->crypto_capabilities.num_crypto_cap,
186 sizeof(cq_host->crypto_cap_array[0]), GFP_KERNEL);
187 if (!cq_host->crypto_cap_array) {
188 err = -ENOMEM;
189 goto out;
190 }
191
192 /*
193 * CCAP.CFGC is off by one, so the actual number of crypto
194 * configurations (a.k.a. keyslots) is CCAP.CFGC + 1.
195 */
196 num_keyslots = cq_host->crypto_capabilities.config_count + 1;
197

 > 198		err = devm_blk_ksm_init(dev, ksm, num_keyslots);

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

#6 lore raw

Re: [PATCH v5 4/9] mmc: cqhci: add support for inline encryption

In reply to Eric Biggers
Quoted 31 lines · Eric Biggers

From: Eric Biggers <[email protected]>

Add support for eMMC inline encryption using the blk-crypto framework (Documentation/block/inline-encryption.rst).

eMMC inline encryption support is specified by the upcoming JEDEC eMMC v5.2 specification. It is only specified for the CQ interface, not the non-CQ interface. Although the eMMC v5.2 specification hasn't been officially released yet, the crypto support was already agreed on several years ago, and it was already implemented by at least two major hardware vendors. Lots of hardware in the field already supports and uses it, e.g. Snapdragon 630 to give one example.

eMMC inline encryption support is very similar to the UFS inline encryption support which was standardized in the UFS v2.1 specification and was already upstreamed. The only major difference is that eMMC limits data unit numbers to 32 bits, unlike UFS's 64 bits.

Like we did with UFS, make the crypto support opt-in by individual drivers; don't enable it automatically whenever the hardware declares crypto support. This is necessary because in every case we've seen, some extra vendor-specific logic is needed to use the crypto support.

---

[...]

Quoted 122 lines
drivers/mmc/host/cqhci-crypto.c new file +89 -0
+/**
+ * cqhci_crypto_init - initialize CQHCI crypto support
+ * @cq_host: a cqhci host
+ *
+ * If the driver previously set MMC_CAP2_CRYPTO and the CQE declares
+ * CQHCI_CAP_CS, initialize the crypto support.  This involves reading the
+ * crypto capability registers, initializing the keyslot manager, clearing all
+ * keyslots, and enabling 128-bit task descriptors.
+ *
+ * Return: 0 if crypto was initialized or isn't supported; whether
+ *        MMC_CAP2_CRYPTO remains set indicates which one of those cases it is.
+ *        Also can return a negative errno value on unexpected error.
+ */
+int cqhci_crypto_init(struct cqhci_host *cq_host)
+{
+       struct mmc_host *mmc = cq_host->mmc;
+       struct device *dev = mmc_dev(mmc);
+       struct blk_keyslot_manager *ksm = &mmc->ksm;
+       unsigned int num_keyslots;
+       unsigned int cap_idx;
+       enum blk_crypto_mode_num blk_mode_num;
+       unsigned int slot;
+       int err = 0;
+
+       if (!(mmc->caps2 & MMC_CAP2_CRYPTO) ||
+           !(cqhci_readl(cq_host, CQHCI_CAP) & CQHCI_CAP_CS))
+               goto out;
+
+       cq_host->crypto_capabilities.reg_val =
+                       cpu_to_le32(cqhci_readl(cq_host, CQHCI_CCAP));
+
+       cq_host->crypto_cfg_register =
+               (u32)cq_host->crypto_capabilities.config_array_ptr * 0x100;
+
+       cq_host->crypto_cap_array =
+               devm_kcalloc(dev, cq_host->crypto_capabilities.num_crypto_cap,
+                            sizeof(cq_host->crypto_cap_array[0]), GFP_KERNEL);
+       if (!cq_host->crypto_cap_array) {
+               err = -ENOMEM;
+               goto out;
+       }
+
+       /*
+        * CCAP.CFGC is off by one, so the actual number of crypto
+        * configurations (a.k.a. keyslots) is CCAP.CFGC + 1.
+        */
+       num_keyslots = cq_host->crypto_capabilities.config_count + 1;
+
+       err = devm_blk_ksm_init(dev, ksm, num_keyslots);
+       if (err)
+               goto out;
+
+       ksm->ksm_ll_ops = cqhci_ksm_ops;
+       ksm->dev = dev;
+
+       /* Unfortunately, CQHCI crypto only supports 32 DUN bits. */
+       ksm->max_dun_bytes_supported = 4;
+
+       /*
+        * Cache all the crypto capabilities and advertise the supported crypto
+        * modes and data unit sizes to the block layer.
+        */
+       for (cap_idx = 0; cap_idx < cq_host->crypto_capabilities.num_crypto_cap;
+            cap_idx++) {
+               cq_host->crypto_cap_array[cap_idx].reg_val =
+                       cpu_to_le32(cqhci_readl(cq_host,
+                                               CQHCI_CRYPTOCAP +
+                                               cap_idx * sizeof(__le32)));
+               blk_mode_num = cqhci_find_blk_crypto_mode(
+                                       cq_host->crypto_cap_array[cap_idx]);
+               if (blk_mode_num == BLK_ENCRYPTION_MODE_INVALID)
+                       continue;
+               ksm->crypto_modes_supported[blk_mode_num] |=
+                       cq_host->crypto_cap_array[cap_idx].sdus_mask * 512;
+       }
+
+       /* Clear all the keyslots so that we start in a known state. */
+       for (slot = 0; slot < num_keyslots; slot++)
+               cqhci_crypto_clear_keyslot(cq_host, slot);
+
+       /* CQHCI crypto requires the use of 128-bit task descriptors. */
+       cq_host->caps |= CQHCI_TASK_DESC_SZ_128;
+
+       return 0;
+
+out:
+       mmc->caps2 &= ~MMC_CAP2_CRYPTO;
+       return err;
+}
drivers/mmc/host/cqhci-crypto.h new file +27 -0
new file mode 100644
index 0000000000000..60b58ee0e6256
@@ -0,0 +1,47 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * CQHCI crypto engine (inline encryption) support
+ *
+ * Copyright 2020 Google LLC
+ */
+
+#ifndef LINUX_MMC_CQHCI_CRYPTO_H
+#define LINUX_MMC_CQHCI_CRYPTO_H
+
+#include <linux/mmc/host.h>
+
+#include "cqhci.h"
+
+#ifdef CONFIG_MMC_CRYPTO
+
+int cqhci_crypto_init(struct cqhci_host *host);
+
+/*
+ * Returns the crypto bits that should be set in bits 64-127 of the
+ * task descriptor.
+ */
+static inline u64 cqhci_crypto_prep_task_desc(struct mmc_request *mrq)
+{
+       if (!mrq->crypto_enabled)
+               return 0;
+

drivers/mmc/host/cqhci-crypto.h line 37

Quoted 10 lines
drivers/mmc/host/cqhci-crypto.h new file +10 -0
+       return CQHCI_CRYPTO_ENABLE_BIT |
+              CQHCI_CRYPTO_KEYSLOT(mrq->crypto_key_slot) |
+              mrq->data_unit_num;
+}
+
+#else /* CONFIG_MMC_CRYPTO */
+
+static inline int cqhci_crypto_init(struct cqhci_host *host)
+{
+       return 0;

The host calling this function may have MMC_CAP2_CRYPTO set for it.

When CONFIG_MMC_CRYPTO is set, cqhci_crypto_init() may unset MMC_CAP2_CRYPTO if initialization fails. It seems like we should unset MMC_CAP2_CRYPTO in this stub function as well, right?

drivers/mmc/host/cqhci-crypto.h line 47

Quoted 10 lines
drivers/mmc/host/cqhci-crypto.h new file +10 -0
+}
+
+static inline u64 cqhci_crypto_prep_task_desc(struct mmc_request *mrq)
+{
+       return 0;
+}
+
+#endif /* !CONFIG_MMC_CRYPTO */
+
+#endif /* LINUX_MMC_CQHCI_CRYPTO_H */

[...]

Kind regards
Uffe

author maintainer
#7 lore raw

Re: [PATCH v5 4/9] mmc: cqhci: add support for inline encryption

Quoted 11 lines · Ulf Hansson

drivers/mmc/host/cqhci-crypto.h line 37

Quoted 5 lines
drivers/mmc/host/cqhci-crypto.h new file +5 -0
+#else /* CONFIG_MMC_CRYPTO */
+
+static inline int cqhci_crypto_init(struct cqhci_host *host)
+{
+       return 0;

The host calling this function may have MMC_CAP2_CRYPTO set for it.

When CONFIG_MMC_CRYPTO is set, cqhci_crypto_init() may unset MMC_CAP2_CRYPTO if initialization fails. It seems like we should unset MMC_CAP2_CRYPTO in this stub function as well, right?

The code in sdhci-msm.c that sets MMC_CAP2_CRYPTO is conditional on CONFIG_MMC_CRYPTO. So, MMC_CAP2_CRYPTO won't be set when !CONFIG_MMC_CRYPTO.

I suppose we might as well do something to stop other drivers from accidentally getting that wrong, though.

How about just defining the flag to 0 when !CONFIG_MMC_CRYPTO:

#ifdef CONFIG_MMC_CRYPTO
#define MMC_CAP2_CRYPTO (1 << 27) /* Host supports inline encryption */
#else
#define MMC_CAP2_CRYPTO 0
#endif

That would be more reliable than relying on the flag getting cleared by something.

- Eric

#8 lore raw

Re: [PATCH v5 4/9] mmc: cqhci: add support for inline encryption

Quoted 22 lines · Eric Biggers
Quoted 11 lines · Ulf Hansson

drivers/mmc/host/cqhci-crypto.h line 37

Quoted 5 lines
drivers/mmc/host/cqhci-crypto.h new file +5 -0
+#else /* CONFIG_MMC_CRYPTO */
+
+static inline int cqhci_crypto_init(struct cqhci_host *host)
+{
+       return 0;

The host calling this function may have MMC_CAP2_CRYPTO set for it.

When CONFIG_MMC_CRYPTO is set, cqhci_crypto_init() may unset MMC_CAP2_CRYPTO if initialization fails. It seems like we should unset MMC_CAP2_CRYPTO in this stub function as well, right?

The code in sdhci-msm.c that sets MMC_CAP2_CRYPTO is conditional on CONFIG_MMC_CRYPTO. So, MMC_CAP2_CRYPTO won't be set when !CONFIG_MMC_CRYPTO.

I suppose we might as well do something to stop other drivers from accidentally getting that wrong, though.

How about just defining the flag to 0 when !CONFIG_MMC_CRYPTO:

Quoted 8 lines

#ifdef CONFIG_MMC_CRYPTO
#define MMC_CAP2_CRYPTO (1 << 27) /* Host supports inline encryption */
#else
#define MMC_CAP2_CRYPTO 0
#endif

That would be more reliable than relying on the flag getting cleared by something.

Yep, this looks good to me!

Kind regards
Uffe

author maintainer
#9 lore raw

PATCH v5 5/9 mmc: cqhci: add cqhci_host_ops::program_key

In reply to Eric Biggers

From: Eric Biggers <[email protected]>

On Snapdragon SoCs, the Linux kernel isn't permitted to directly access the standard CQHCI crypto configuration registers. Instead, programming and evicting keys must be done through vendor-specific SMC calls.

To support this hardware, add a ->program_key() method to 'struct cqhci_host_ops'. This allows overriding the standard CQHCI crypto key programming / eviction procedure.

This is inspired by the corresponding UFS crypto support, which uses these same SMC calls. See commit 1bc726e26ef3 ("scsi: ufs: Add program_key() variant op").

---
 drivers/mmc/host/cqhci-crypto.c | 22 +++++++++++++---------
 drivers/mmc/host/cqhci.h        |  4 ++++
 2 files changed, 17 insertions(+), 9 deletions(-)
drivers/mmc/host/cqhci-crypto.c +13 -9
index 0e2a9dcac6308..6419cfbb4ab78 100644
@@ -30,13 +30,16 @@ cqhci_host_from_ksm(struct blk_keyslot_manager *ksm)
 	return mmc->cqe_private;
 }
 
-static void cqhci_crypto_program_key(struct cqhci_host *cq_host,
-				     const union cqhci_crypto_cfg_entry *cfg,
-				     int slot)
+static int cqhci_crypto_program_key(struct cqhci_host *cq_host,
+				    const union cqhci_crypto_cfg_entry *cfg,
+				    int slot)
 {
 	u32 slot_offset = cq_host->crypto_cfg_register + slot * sizeof(*cfg);
 	int i;
 
+	if (cq_host->ops->program_key)
+		return cq_host->ops->program_key(cq_host, cfg, slot);
+
 	/* Clear CFGE */
 	cqhci_writel(cq_host, 0, slot_offset + 16 * sizeof(cfg->reg_val[0]));
 
@@ -51,6 +54,7 @@ static void cqhci_crypto_program_key(struct cqhci_host *cq_host,
 	/* Write dword 16, which includes the new value of CFGE */
 	cqhci_writel(cq_host, le32_to_cpu(cfg->reg_val[16]),
 		     slot_offset + 16 * sizeof(cfg->reg_val[0]));
+	return 0;
 }
 
 static int cqhci_crypto_keyslot_program(struct blk_keyslot_manager *ksm,
@@ -67,6 +71,7 @@ static int cqhci_crypto_keyslot_program(struct blk_keyslot_manager *ksm,
 	int i;
 	int cap_idx = -1;
 	union cqhci_crypto_cfg_entry cfg = {};
+	int err;
 
 	BUILD_BUG_ON(CQHCI_CRYPTO_KEY_SIZE_INVALID != 0);
 	for (i = 0; i < cq_host->crypto_capabilities.num_crypto_cap; i++) {
@@ -93,13 +98,13 @@ static int cqhci_crypto_keyslot_program(struct blk_keyslot_manager *ksm,
 		memcpy(cfg.crypto_key, key->raw, key->size);
 	}
 
-	cqhci_crypto_program_key(cq_host, &cfg, slot);
+	err = cqhci_crypto_program_key(cq_host, &cfg, slot);
 
 	memzero_explicit(&cfg, sizeof(cfg));
-	return 0;
+	return err;
 }
 
-static void cqhci_crypto_clear_keyslot(struct cqhci_host *cq_host, int slot)
+static int cqhci_crypto_clear_keyslot(struct cqhci_host *cq_host, int slot)
 {
 	/*
 	 * Clear the crypto cfg on the device. Clearing CFGE
@@ -107,7 +112,7 @@ static void cqhci_crypto_clear_keyslot(struct cqhci_host *cq_host, int slot)
 	 */
 	union cqhci_crypto_cfg_entry cfg = {};
 
-	cqhci_crypto_program_key(cq_host, &cfg, slot);
+	return cqhci_crypto_program_key(cq_host, &cfg, slot);
 }
 
 static int cqhci_crypto_keyslot_evict(struct blk_keyslot_manager *ksm,
@@ -116,8 +121,7 @@ static int cqhci_crypto_keyslot_evict(struct blk_keyslot_manager *ksm,
 {
 	struct cqhci_host *cq_host = cqhci_host_from_ksm(ksm);
 
-	cqhci_crypto_clear_keyslot(cq_host, slot);
-	return 0;
+	return cqhci_crypto_clear_keyslot(cq_host, slot);
 }
 
 /*
drivers/mmc/host/cqhci.h +4 -0
index 8e9e8f5db5bcc..ba9387ed90eb6 100644
@@ -286,6 +286,10 @@ struct cqhci_host_ops {
 				 u64 *data);
 	void (*pre_enable)(struct mmc_host *mmc);
 	void (*post_disable)(struct mmc_host *mmc);
+#ifdef CONFIG_MMC_CRYPTO
+	int (*program_key)(struct cqhci_host *cq_host,
+			   const union cqhci_crypto_cfg_entry *cfg, int slot);
+#endif
 };
 
 static inline void cqhci_writel(struct cqhci_host *host, u32 val, int reg)
Signature · 1 line
2.30.0
author maintainer
#10 lore raw

PATCH v5 6/9 firmware: qcom_scm: update comment for ICE-related functions

In reply to Eric Biggers

From: Eric Biggers <[email protected]>

The SCM calls QCOM_SCM_ES_INVALIDATE_ICE_KEY and QCOM_SCM_ES_CONFIG_SET_ICE_KEY are also needed for eMMC inline encryption support, not just for UFS. Update the comments accordingly.

---
 drivers/firmware/qcom_scm.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)
drivers/firmware/qcom_scm.c +11 -5
index 7be48c1bec96d..f57779fc7ee93 100644
@@ -965,8 +965,11 @@ EXPORT_SYMBOL(qcom_scm_ice_available);
  * qcom_scm_ice_invalidate_key() - Invalidate an inline encryption key
  * @index: the keyslot to invalidate
  *
- * The UFSHCI standard defines a standard way to do this, but it doesn't work on
- * these SoCs; only this SCM call does.
+ * The UFSHCI and eMMC standards define a standard way to do this, but it
+ * doesn't work on these SoCs; only this SCM call does.
+ *
+ * It is assumed that the SoC has only one ICE instance being used, as this SCM
+ * call doesn't specify which ICE instance the keyslot belongs to.
  *
  * Return: 0 on success; -errno on failure.
  */
@@ -995,10 +998,13 @@ EXPORT_SYMBOL(qcom_scm_ice_invalidate_key);
  *		    units, e.g. 1 = 512 bytes, 8 = 4096 bytes, etc.
  *
  * Program a key into a keyslot of Qualcomm ICE (Inline Crypto Engine), where it
- * can then be used to encrypt/decrypt UFS I/O requests inline.
+ * can then be used to encrypt/decrypt UFS or eMMC I/O requests inline.
+ *
+ * The UFSHCI and eMMC standards define a standard way to do this, but it
+ * doesn't work on these SoCs; only this SCM call does.
  *
- * The UFSHCI standard defines a standard way to do this, but it doesn't work on
- * these SoCs; only this SCM call does.
+ * It is assumed that the SoC has only one ICE instance being used, as this SCM
+ * call doesn't specify which ICE instance the keyslot belongs to.
  *
  * Return: 0 on success; -errno on failure.
  */
Signature · 1 line
2.30.0
#11 lore raw

Re: [PATCH v5 6/9] firmware: qcom_scm: update comment for ICE-related functions

- trimmed cc-list

Quoted 9 lines · Eric Biggers

From: Eric Biggers <[email protected]>

The SCM calls QCOM_SCM_ES_INVALIDATE_ICE_KEY and QCOM_SCM_ES_CONFIG_SET_ICE_KEY are also needed for eMMC inline encryption support, not just for UFS. Update the comments accordingly.

Björn, may I have your ack on this one? I intend to queue this via my mmc tree.

Kind regards
Uffe

Quoted 42 lines
---
 drivers/firmware/qcom_scm.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)
drivers/firmware/qcom_scm.c +11 -6
index 7be48c1bec96d..f57779fc7ee93 100644
@@ -965,8 +965,11 @@ EXPORT_SYMBOL(qcom_scm_ice_available);
  * qcom_scm_ice_invalidate_key() - Invalidate an inline encryption key
  * @index: the keyslot to invalidate
  *
- * The UFSHCI standard defines a standard way to do this, but it doesn't work on
- * these SoCs; only this SCM call does.
+ * The UFSHCI and eMMC standards define a standard way to do this, but it
+ * doesn't work on these SoCs; only this SCM call does.
+ *
+ * It is assumed that the SoC has only one ICE instance being used, as this SCM
+ * call doesn't specify which ICE instance the keyslot belongs to.
  *
  * Return: 0 on success; -errno on failure.
  */
@@ -995,10 +998,13 @@ EXPORT_SYMBOL(qcom_scm_ice_invalidate_key);
  *                 units, e.g. 1 = 512 bytes, 8 = 4096 bytes, etc.
  *
  * Program a key into a keyslot of Qualcomm ICE (Inline Crypto Engine), where it
- * can then be used to encrypt/decrypt UFS I/O requests inline.
+ * can then be used to encrypt/decrypt UFS or eMMC I/O requests inline.
+ *
+ * The UFSHCI and eMMC standards define a standard way to do this, but it
+ * doesn't work on these SoCs; only this SCM call does.
  *
- * The UFSHCI standard defines a standard way to do this, but it doesn't work on
- * these SoCs; only this SCM call does.
+ * It is assumed that the SoC has only one ICE instance being used, as this SCM
+ * call doesn't specify which ICE instance the keyslot belongs to.
  *
  * Return: 0 on success; -errno on failure.
  */
--

2.30.0

#12 lore raw

Re: [PATCH v5 6/9] firmware: qcom_scm: update comment for ICE-related functions

Quoted 6 lines · Ulf Hansson

- trimmed cc-list

Quoted 3 lines · Eric Biggers

From: Eric Biggers <[email protected]>

Quoted 9 lines
Quoted 6 lines

The SCM calls QCOM_SCM_ES_INVALIDATE_ICE_KEY and QCOM_SCM_ES_CONFIG_SET_ICE_KEY are also needed for eMMC inline encryption support, not just for UFS. Update the comments accordingly.

Björn, may I have your ack on this one? I intend to queue this via my mmc tree.

Certainly, sorry for not paying attention.

And I presume I'll take the dts patch (8/9) through the Qcom tree...

Regards,
Bjorn

Quoted 45 lines

Kind regards
Uffe

Quoted 42 lines
---
 drivers/firmware/qcom_scm.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)
drivers/firmware/qcom_scm.c +11 -6
index 7be48c1bec96d..f57779fc7ee93 100644
@@ -965,8 +965,11 @@ EXPORT_SYMBOL(qcom_scm_ice_available);
  * qcom_scm_ice_invalidate_key() - Invalidate an inline encryption key
  * @index: the keyslot to invalidate
  *
- * The UFSHCI standard defines a standard way to do this, but it doesn't work on
- * these SoCs; only this SCM call does.
+ * The UFSHCI and eMMC standards define a standard way to do this, but it
+ * doesn't work on these SoCs; only this SCM call does.
+ *
+ * It is assumed that the SoC has only one ICE instance being used, as this SCM
+ * call doesn't specify which ICE instance the keyslot belongs to.
  *
  * Return: 0 on success; -errno on failure.
  */
@@ -995,10 +998,13 @@ EXPORT_SYMBOL(qcom_scm_ice_invalidate_key);
  *                 units, e.g. 1 = 512 bytes, 8 = 4096 bytes, etc.
  *
  * Program a key into a keyslot of Qualcomm ICE (Inline Crypto Engine), where it
- * can then be used to encrypt/decrypt UFS I/O requests inline.
+ * can then be used to encrypt/decrypt UFS or eMMC I/O requests inline.
+ *
+ * The UFSHCI and eMMC standards define a standard way to do this, but it
+ * doesn't work on these SoCs; only this SCM call does.
  *
- * The UFSHCI standard defines a standard way to do this, but it doesn't work on
- * these SoCs; only this SCM call does.
+ * It is assumed that the SoC has only one ICE instance being used, as this SCM
+ * call doesn't specify which ICE instance the keyslot belongs to.
  *
  * Return: 0 on success; -errno on failure.
  */
--

2.30.0

#13 lore raw

Re: [PATCH v5 6/9] firmware: qcom_scm: update comment for ICE-related functions

Quoted 16 lines · Bjorn Andersson
Quoted 13 lines · Ulf Hansson

- trimmed cc-list

Quoted 9 lines · Eric Biggers

From: Eric Biggers <[email protected]>

The SCM calls QCOM_SCM_ES_INVALIDATE_ICE_KEY and QCOM_SCM_ES_CONFIG_SET_ICE_KEY are also needed for eMMC inline encryption support, not just for UFS. Update the comments accordingly.

Quoted 6 lines
Quoted 2 lines

Björn, may I have your ack on this one? I intend to queue this via my mmc tree.

Certainly, sorry for not paying attention.

Thanks!

Quoted 3 lines

And I presume I'll take the dts patch (8/9) through the Qcom tree...

Yep, I leave that for you.

[...]

Kind regards
Uffe

author maintainer
#14 lore raw

PATCH v5 7/9 dt-bindings: mmc: sdhci-msm: add ICE registers and clock

In reply to Eric Biggers

From: Eric Biggers <[email protected]>

Document the bindings for the registers and clock for the MMC instance of the Inline Crypto Engine (ICE) on Snapdragon SoCs. These bindings are needed in order for sdhci-msm to support inline encryption.

---
 Documentation/devicetree/bindings/mmc/sdhci-msm.txt | 3 +++
 1 file changed, 3 insertions(+)
Documentation/devicetree/bindings/mmc/sdhci-msm.txt +3 -0
index 3b602fd6180bf..4f2e138439506 100644
@@ -30,10 +30,12 @@ Required properties:
 	- SD Core register map (required for controllers earlier than msm-v5)
 	- CQE register map (Optional, CQE support is present on SDHC instance meant
 	                    for eMMC and version v4.2 and above)
+	- Inline Crypto Engine register map (optional)
 - reg-names: When CQE register map is supplied, below reg-names are required
 	- "hc" for Host controller register map
 	- "core" for SD core register map
 	- "cqhci" for CQE register map
+	- "ice" for Inline Crypto Engine register map (optional)
 - interrupts: Should contain an interrupt-specifiers for the interrupts:
 	- Host controller interrupt (required)
 - pinctrl-names: Should contain only one value - "default".
@@ -46,6 +48,7 @@ Required properties:
 	"xo"	- TCXO clock (optional)
 	"cal"	- reference clock for RCLK delay calibration (optional)
 	"sleep"	- sleep clock for RCLK delay calibration (optional)
+	"ice" - clock for Inline Crypto Engine (optional)
 
 - qcom,ddr-config: Certain chipsets and platforms require particular settings
 	for the DDR_CONFIG register. Use this field to specify the register
Signature · 1 line
2.30.0
author maintainer
#15 lore raw

PATCH v5 8/9 arm64: dts: qcom: sdm630: add ICE registers and clocks

In reply to Eric Biggers

From: Eric Biggers <[email protected]>

Add the registers and clock for the Inline Crypto Engine (ICE) to the device tree node for the sdhci-msm host controller on sdm630. This allows sdhci-msm to support inline encryption on sdm630.

---
 arch/arm64/boot/dts/qcom/sdm630.dtsi | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)
arch/arm64/boot/dts/qcom/sdm630.dtsi +6 -4
index 37d5cc32f6b62..afb3d20c31fa0 100644
@@ -808,8 +808,9 @@ spmi_bus: spmi@800f000 {
 		sdhc_1: sdhci@c0c4000 {
 			compatible = "qcom,sdm630-sdhci", "qcom,sdhci-msm-v5";
 			reg = <0x0c0c4000 0x1000>,
-				<0x0c0c5000 0x1000>;
-			reg-names = "hc", "cqhci";
+				<0x0c0c5000 0x1000>,
+				<0x0c0c8000 0x8000>;
+			reg-names = "hc", "cqhci", "ice";
 
 			interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>,
 					<GIC_SPI 112 IRQ_TYPE_LEVEL_HIGH>;
@@ -817,8 +818,9 @@ sdhc_1: sdhci@c0c4000 {
 
 			clocks = <&gcc GCC_SDCC1_APPS_CLK>,
 					<&gcc GCC_SDCC1_AHB_CLK>,
-					<&xo_board>;
-			clock-names = "core", "iface", "xo";
+					<&xo_board>,
+					<&gcc GCC_SDCC1_ICE_CORE_CLK>;
+			clock-names = "core", "iface", "xo", "ice";
 
 			pinctrl-names = "default", "sleep";
 			pinctrl-0 = <&sdc1_clk_on &sdc1_cmd_on &sdc1_data_on &sdc1_rclk_on>;
Signature · 1 line
2.30.0
author maintainer
#16 lore raw

PATCH v5 9/9 mmc: sdhci-msm: add Inline Crypto Engine support

In reply to Eric Biggers

From: Eric Biggers <[email protected]>

Add support for Qualcomm Inline Crypto Engine (ICE) to sdhci-msm.

The standard-compliant parts, such as querying the crypto capabilities and enabling crypto for individual MMC requests, are already handled by cqhci-crypto.c, which itself is wired into the blk-crypto framework. However, ICE requires vendor-specific init, enable, and resume logic, and it requires that keys be programmed and evicted by vendor-specific SMC calls. Make the sdhci-msm driver handle these details.

This is heavily inspired by the similar changes made for UFS, since the UFS and eMMC ICE instances are very similar. See commit df4ec2fa7a4d ("scsi: ufs-qcom: Add Inline Crypto Engine support").

I tested this on a Sony Xperia 10, which uses the Snapdragon 630 SoC, which has basic upstream support. Mainly, I used android-xfstests
(https://github.com/tytso/xfstests-bld/blob/master/Documentation/android-xfstests.md)
to run the ext4 and f2fs encryption tests in a Debian chroot:

android-xfstests -c ext4,f2fs -g encrypt -m inlinecrypt

These tests included tests which verify that the on-disk ciphertext is identical to that produced by a software implementation. I also verified that ICE was actually being used.

---
 drivers/mmc/host/Kconfig     |   1 +
 drivers/mmc/host/sdhci-msm.c | 276 ++++++++++++++++++++++++++++++++++-
 2 files changed, 273 insertions(+), 4 deletions(-)
drivers/mmc/host/Kconfig +1 -0
index 596f326373153..d57720ee70b0c 100644
@@ -544,6 +544,7 @@ config MMC_SDHCI_MSM
 	depends on MMC_SDHCI_PLTFM
 	select MMC_SDHCI_IO_ACCESSORS
 	select MMC_CQHCI
+	select QCOM_SCM if MMC_CRYPTO && ARCH_QCOM
 	help
 	  This selects the Secure Digital Host Controller Interface (SDHCI)
 	  support present in Qualcomm SOCs. The controller supports
drivers/mmc/host/sdhci-msm.c +272 -4

332 lines not on this page · raw message

Signature · 1 line
2.30.0