* [PATCH v2 1/9] mmc: add basic support for inline encryption
2020-12-03 2:05 [PATCH v2 0/9] eMMC inline encryption support Eric Biggers
@ 2020-12-03 2:05 ` Eric Biggers
2020-12-08 23:40 ` Satya Tangirala
2020-12-03 2:05 ` [PATCH v2 2/9] mmc: cqhci: rename cqhci.c to cqhci-core.c Eric Biggers
` (7 subsequent siblings)
8 siblings, 1 reply; 28+ messages in thread
From: Eric Biggers @ 2020-12-03 2:05 UTC (permalink / raw)
To: linux-mmc
Cc: linux-arm-msm, devicetree, linux-fscrypt, Satya Tangirala,
Ulf Hansson, Andy Gross, Bjorn Andersson, Adrian Hunter,
Asutosh Das, Rob Herring, Neeraj Soni, Barani Muthukumaran,
Peng Zhou, Stanley Chu, Konrad Dybcio
From: Eric Biggers <ebiggers@google.com>
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 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. mmc_core destroys this keyslot manager
when freeing the mmc_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.
Co-developed-by: Satya Tangirala <satyat@google.com>
Signed-off-by: Satya Tangirala <satyat@google.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
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 | 54 +++++++++++++++++++++++++++++++++++++++
drivers/mmc/core/crypto.h | 46 +++++++++++++++++++++++++++++++++
drivers/mmc/core/host.c | 2 ++
drivers/mmc/core/queue.c | 3 +++
include/linux/mmc/core.h | 6 +++++
include/linux/mmc/host.h | 7 +++++
10 files changed, 133 insertions(+)
create mode 100644 drivers/mmc/core/crypto.c
create mode 100644 drivers/mmc/core/crypto.h
diff --git a/drivers/mmc/core/Kconfig b/drivers/mmc/core/Kconfig
index c12fe13e4b147..ae8b69aee6190 100644
--- a/drivers/mmc/core/Kconfig
+++ b/drivers/mmc/core/Kconfig
@@ -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.
diff --git a/drivers/mmc/core/Makefile b/drivers/mmc/core/Makefile
index 95ffe008ebdf8..6a907736cd7a5 100644
--- a/drivers/mmc/core/Makefile
+++ b/drivers/mmc/core/Makefile
@@ -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
diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index 8d3df0be0355c..eaf2f10743260 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -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;
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index d42037f0f10d7..275de270232b3 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -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);
}
/**
diff --git a/drivers/mmc/core/crypto.c b/drivers/mmc/core/crypto.c
new file mode 100644
index 0000000000000..4f47eb4740db0
--- /dev/null
+++ b/drivers/mmc/core/crypto.c
@@ -0,0 +1,54 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * MMC crypto engine (inline encryption) support
+ *
+ * Copyright 2020 Google LLC
+ */
+
+#include <linux/blk-crypto.h>
+#include <linux/mmc/host.h>
+
+#include "core.h"
+#include "crypto.h"
+#include "queue.h"
+
+void mmc_crypto_set_initial_state(struct mmc_host *host)
+{
+ /* Reset might clear all keys, so reprogram all the keys. */
+ if (host->caps2 & MMC_CAP2_CRYPTO)
+ blk_ksm_reprogram_all_keys(&host->ksm);
+}
+
+void mmc_crypto_free_host(struct mmc_host *host)
+{
+ if (host->caps2 & MMC_CAP2_CRYPTO)
+ blk_ksm_destroy(&host->ksm);
+}
+
+void mmc_crypto_setup_queue(struct request_queue *q, struct mmc_host *host)
+{
+ if (host->caps2 & MMC_CAP2_CRYPTO)
+ blk_ksm_register(&host->ksm, q);
+}
+EXPORT_SYMBOL_GPL(mmc_crypto_setup_queue);
+
+void mmc_crypto_prepare_req(struct mmc_queue_req *mqrq)
+{
+ struct request *req = mmc_queue_req_to_req(mqrq);
+ struct mmc_request *mrq = &mqrq->brq.mrq;
+
+ if (!req->crypt_keyslot)
+ return;
+
+ mrq->crypto_enabled = true;
+ mrq->crypto_key_slot = blk_ksm_get_slot_idx(req->crypt_keyslot);
+
+ /*
+ * For now we assume that all MMC drivers set max_dun_bytes_supported=4,
+ * which is the limit for CQHCI crypto. So all DUNs should be 32-bit.
+ */
+ WARN_ON_ONCE(req->crypt_ctx->bc_dun[0] > U32_MAX);
+
+ mrq->data_unit_num = req->crypt_ctx->bc_dun[0];
+}
+EXPORT_SYMBOL_GPL(mmc_crypto_prepare_req);
diff --git a/drivers/mmc/core/crypto.h b/drivers/mmc/core/crypto.h
new file mode 100644
index 0000000000000..4780639b832f4
--- /dev/null
+++ b/drivers/mmc/core/crypto.h
@@ -0,0 +1,46 @@
+/* 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_free_host(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_free_host(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 */
diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index 96b2ca1f1b06d..d962b9ca0e37a 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -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"
@@ -532,6 +533,7 @@ EXPORT_SYMBOL(mmc_remove_host);
*/
void mmc_free_host(struct mmc_host *host)
{
+ mmc_crypto_free_host(host);
mmc_pwrseq_free(host);
put_device(&host->class_dev);
}
diff --git a/drivers/mmc/core/queue.c b/drivers/mmc/core/queue.c
index de7cb0369c308..d96db852bb91a 100644
--- a/drivers/mmc/core/queue.c
+++ b/drivers/mmc/core/queue.c
@@ -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)
diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
index 29aa507116261..ab19245e99451 100644
--- a/include/linux/mmc/core.h
+++ b/include/linux/mmc/core.h
@@ -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;
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index c079b932330f2..550460bf1b37c 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -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 */
@@ -377,6 +378,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 */
@@ -471,6 +473,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;
--
2.29.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v2 1/9] mmc: add basic support for inline encryption
2020-12-03 2:05 ` [PATCH v2 1/9] mmc: add basic support for inline encryption Eric Biggers
@ 2020-12-08 23:40 ` Satya Tangirala
0 siblings, 0 replies; 28+ messages in thread
From: Satya Tangirala @ 2020-12-08 23:40 UTC (permalink / raw)
To: Eric Biggers
Cc: linux-mmc, linux-arm-msm, devicetree, linux-fscrypt, Ulf Hansson,
Andy Gross, Bjorn Andersson, Adrian Hunter, Asutosh Das,
Rob Herring, Neeraj Soni, Barani Muthukumaran, Peng Zhou,
Stanley Chu, Konrad Dybcio
On Wed, Dec 02, 2020 at 06:05:08PM -0800, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
>
> 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 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. mmc_core destroys this keyslot manager
> when freeing the mmc_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.
>
> Co-developed-by: Satya Tangirala <satyat@google.com>
> Signed-off-by: Satya Tangirala <satyat@google.com>
> Acked-by: Adrian Hunter <adrian.hunter@intel.com>
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
> 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 | 54 +++++++++++++++++++++++++++++++++++++++
> drivers/mmc/core/crypto.h | 46 +++++++++++++++++++++++++++++++++
> drivers/mmc/core/host.c | 2 ++
> drivers/mmc/core/queue.c | 3 +++
> include/linux/mmc/core.h | 6 +++++
> include/linux/mmc/host.h | 7 +++++
> 10 files changed, 133 insertions(+)
> create mode 100644 drivers/mmc/core/crypto.c
> create mode 100644 drivers/mmc/core/crypto.h
>
> diff --git a/drivers/mmc/core/Kconfig b/drivers/mmc/core/Kconfig
> index c12fe13e4b147..ae8b69aee6190 100644
> --- a/drivers/mmc/core/Kconfig
> +++ b/drivers/mmc/core/Kconfig
> @@ -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.
> diff --git a/drivers/mmc/core/Makefile b/drivers/mmc/core/Makefile
> index 95ffe008ebdf8..6a907736cd7a5 100644
> --- a/drivers/mmc/core/Makefile
> +++ b/drivers/mmc/core/Makefile
> @@ -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
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index 8d3df0be0355c..eaf2f10743260 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -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;
>
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index d42037f0f10d7..275de270232b3 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -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);
> }
>
> /**
> diff --git a/drivers/mmc/core/crypto.c b/drivers/mmc/core/crypto.c
> new file mode 100644
> index 0000000000000..4f47eb4740db0
> --- /dev/null
> +++ b/drivers/mmc/core/crypto.c
> @@ -0,0 +1,54 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * MMC crypto engine (inline encryption) support
> + *
> + * Copyright 2020 Google LLC
> + */
> +
> +#include <linux/blk-crypto.h>
> +#include <linux/mmc/host.h>
> +
> +#include "core.h"
> +#include "crypto.h"
> +#include "queue.h"
> +
> +void mmc_crypto_set_initial_state(struct mmc_host *host)
> +{
> + /* Reset might clear all keys, so reprogram all the keys. */
> + if (host->caps2 & MMC_CAP2_CRYPTO)
> + blk_ksm_reprogram_all_keys(&host->ksm);
> +}
> +
> +void mmc_crypto_free_host(struct mmc_host *host)
> +{
> + if (host->caps2 & MMC_CAP2_CRYPTO)
> + blk_ksm_destroy(&host->ksm);
> +}
> +
> +void mmc_crypto_setup_queue(struct request_queue *q, struct mmc_host *host)
> +{
> + if (host->caps2 & MMC_CAP2_CRYPTO)
> + blk_ksm_register(&host->ksm, q);
> +}
> +EXPORT_SYMBOL_GPL(mmc_crypto_setup_queue);
> +
> +void mmc_crypto_prepare_req(struct mmc_queue_req *mqrq)
> +{
> + struct request *req = mmc_queue_req_to_req(mqrq);
> + struct mmc_request *mrq = &mqrq->brq.mrq;
> +
> + if (!req->crypt_keyslot)
> + return;
> +
> + mrq->crypto_enabled = true;
> + mrq->crypto_key_slot = blk_ksm_get_slot_idx(req->crypt_keyslot);
> +
> + /*
> + * For now we assume that all MMC drivers set max_dun_bytes_supported=4,
> + * which is the limit for CQHCI crypto. So all DUNs should be 32-bit.
> + */
> + WARN_ON_ONCE(req->crypt_ctx->bc_dun[0] > U32_MAX);
> +
> + mrq->data_unit_num = req->crypt_ctx->bc_dun[0];
> +}
> +EXPORT_SYMBOL_GPL(mmc_crypto_prepare_req);
> diff --git a/drivers/mmc/core/crypto.h b/drivers/mmc/core/crypto.h
> new file mode 100644
> index 0000000000000..4780639b832f4
> --- /dev/null
> +++ b/drivers/mmc/core/crypto.h
> @@ -0,0 +1,46 @@
> +/* 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_free_host(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_free_host(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 */
> diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
> index 96b2ca1f1b06d..d962b9ca0e37a 100644
> --- a/drivers/mmc/core/host.c
> +++ b/drivers/mmc/core/host.c
> @@ -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"
> @@ -532,6 +533,7 @@ EXPORT_SYMBOL(mmc_remove_host);
> */
> void mmc_free_host(struct mmc_host *host)
> {
> + mmc_crypto_free_host(host);
> mmc_pwrseq_free(host);
> put_device(&host->class_dev);
> }
> diff --git a/drivers/mmc/core/queue.c b/drivers/mmc/core/queue.c
> index de7cb0369c308..d96db852bb91a 100644
> --- a/drivers/mmc/core/queue.c
> +++ b/drivers/mmc/core/queue.c
> @@ -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)
> diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
> index 29aa507116261..ab19245e99451 100644
> --- a/include/linux/mmc/core.h
> +++ b/include/linux/mmc/core.h
> @@ -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;
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index c079b932330f2..550460bf1b37c 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -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 */
> @@ -377,6 +378,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 */
>
> @@ -471,6 +473,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;
>
> --
> 2.29.2
>
Looks good to me. Please feel free to add
Reviewed-by: Satya Tangirala <satyat@google.com>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 2/9] mmc: cqhci: rename cqhci.c to cqhci-core.c
2020-12-03 2:05 [PATCH v2 0/9] eMMC inline encryption support Eric Biggers
2020-12-03 2:05 ` [PATCH v2 1/9] mmc: add basic support for inline encryption Eric Biggers
@ 2020-12-03 2:05 ` Eric Biggers
2020-12-03 2:05 ` [PATCH v2 3/9] mmc: cqhci: initialize upper 64 bits of 128-bit task descriptors Eric Biggers
` (6 subsequent siblings)
8 siblings, 0 replies; 28+ messages in thread
From: Eric Biggers @ 2020-12-03 2:05 UTC (permalink / raw)
To: linux-mmc
Cc: linux-arm-msm, devicetree, linux-fscrypt, Satya Tangirala,
Ulf Hansson, Andy Gross, Bjorn Andersson, Adrian Hunter,
Asutosh Das, Rob Herring, Neeraj Soni, Barani Muthukumaran,
Peng Zhou, Stanley Chu, Konrad Dybcio
From: Eric Biggers <ebiggers@google.com>
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.
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
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%)
diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
index 451c25fc2c692..20c2f9463d0dc 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -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)
diff --git a/drivers/mmc/host/cqhci.c b/drivers/mmc/host/cqhci-core.c
similarity index 100%
rename from drivers/mmc/host/cqhci.c
rename to drivers/mmc/host/cqhci-core.c
--
2.29.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v2 3/9] mmc: cqhci: initialize upper 64 bits of 128-bit task descriptors
2020-12-03 2:05 [PATCH v2 0/9] eMMC inline encryption support Eric Biggers
2020-12-03 2:05 ` [PATCH v2 1/9] mmc: add basic support for inline encryption Eric Biggers
2020-12-03 2:05 ` [PATCH v2 2/9] mmc: cqhci: rename cqhci.c to cqhci-core.c Eric Biggers
@ 2020-12-03 2:05 ` Eric Biggers
2020-12-03 6:45 ` Adrian Hunter
2020-12-08 23:45 ` Satya Tangirala
2020-12-03 2:05 ` [PATCH v2 4/9] mmc: cqhci: add support for inline encryption Eric Biggers
` (5 subsequent siblings)
8 siblings, 2 replies; 28+ messages in thread
From: Eric Biggers @ 2020-12-03 2:05 UTC (permalink / raw)
To: linux-mmc
Cc: linux-arm-msm, devicetree, linux-fscrypt, Satya Tangirala,
Ulf Hansson, Andy Gross, Bjorn Andersson, Adrian Hunter,
Asutosh Das, Rob Herring, Neeraj Soni, Barani Muthukumaran,
Peng Zhou, Stanley Chu, Konrad Dybcio
From: Eric Biggers <ebiggers@google.com>
Move the task descriptor initialization into cqhci_prep_task_desc(), and
make it initialize all 128 bits of the task descriptor if the host
controller is using 128-bit task descriptors.
This is needed to prepare for CQHCI inline encryption support, which
requires 128-bit task descriptors and uses the upper 64 bits.
Note: since some host controllers already enable 128-bit task
descriptors, it's unclear why the previous code worked when it wasn't
initializing the upper 64 bits. One possibility is that the bits are
being ignored because the features that use them aren't enabled yet.
In any case, setting them to 0 won't hurt.
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
drivers/mmc/host/cqhci-core.c | 30 ++++++++++++++++++++----------
1 file changed, 20 insertions(+), 10 deletions(-)
diff --git a/drivers/mmc/host/cqhci-core.c b/drivers/mmc/host/cqhci-core.c
index 697fe40756bf2..ad7c9acff1728 100644
--- a/drivers/mmc/host/cqhci-core.c
+++ b/drivers/mmc/host/cqhci-core.c
@@ -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",
--
2.29.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v2 3/9] mmc: cqhci: initialize upper 64 bits of 128-bit task descriptors
2020-12-03 2:05 ` [PATCH v2 3/9] mmc: cqhci: initialize upper 64 bits of 128-bit task descriptors Eric Biggers
@ 2020-12-03 6:45 ` Adrian Hunter
2020-12-03 19:23 ` Eric Biggers
2020-12-08 23:45 ` Satya Tangirala
1 sibling, 1 reply; 28+ messages in thread
From: Adrian Hunter @ 2020-12-03 6:45 UTC (permalink / raw)
To: Eric Biggers, linux-mmc
Cc: linux-arm-msm, devicetree, linux-fscrypt, Satya Tangirala,
Ulf Hansson, Andy Gross, Bjorn Andersson, Asutosh Das,
Rob Herring, Neeraj Soni, Barani Muthukumaran, Peng Zhou,
Stanley Chu, Konrad Dybcio
On 3/12/20 4:05 am, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
>
> Move the task descriptor initialization into cqhci_prep_task_desc(), and
> make it initialize all 128 bits of the task descriptor if the host
> controller is using 128-bit task descriptors.
>
> This is needed to prepare for CQHCI inline encryption support, which
> requires 128-bit task descriptors and uses the upper 64 bits.
>
> Note: since some host controllers already enable 128-bit task
> descriptors, it's unclear why the previous code worked when it wasn't
> initializing the upper 64 bits. One possibility is that the bits are
> being ignored because the features that use them aren't enabled yet.
> In any case, setting them to 0 won't hurt.
Coherent allocations are zero-initialized. So the upper 64-bits stay zero.
People set 128-bit anyway because the hardware needs it.
>
> Signed-off-by: Eric Biggers <ebiggers@google.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
> drivers/mmc/host/cqhci-core.c | 30 ++++++++++++++++++++----------
> 1 file changed, 20 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/mmc/host/cqhci-core.c b/drivers/mmc/host/cqhci-core.c
> index 697fe40756bf2..ad7c9acff1728 100644
> --- a/drivers/mmc/host/cqhci-core.c
> +++ b/drivers/mmc/host/cqhci-core.c
> @@ -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",
>
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 3/9] mmc: cqhci: initialize upper 64 bits of 128-bit task descriptors
2020-12-03 6:45 ` Adrian Hunter
@ 2020-12-03 19:23 ` Eric Biggers
0 siblings, 0 replies; 28+ messages in thread
From: Eric Biggers @ 2020-12-03 19:23 UTC (permalink / raw)
To: Adrian Hunter
Cc: linux-mmc, linux-arm-msm, devicetree, linux-fscrypt,
Satya Tangirala, Ulf Hansson, Andy Gross, Bjorn Andersson,
Asutosh Das, Rob Herring, Neeraj Soni, Barani Muthukumaran,
Peng Zhou, Stanley Chu, Konrad Dybcio
On Thu, Dec 03, 2020 at 08:45:15AM +0200, Adrian Hunter wrote:
> On 3/12/20 4:05 am, Eric Biggers wrote:
> > From: Eric Biggers <ebiggers@google.com>
> >
> > Move the task descriptor initialization into cqhci_prep_task_desc(), and
> > make it initialize all 128 bits of the task descriptor if the host
> > controller is using 128-bit task descriptors.
> >
> > This is needed to prepare for CQHCI inline encryption support, which
> > requires 128-bit task descriptors and uses the upper 64 bits.
> >
> > Note: since some host controllers already enable 128-bit task
> > descriptors, it's unclear why the previous code worked when it wasn't
> > initializing the upper 64 bits. One possibility is that the bits are
> > being ignored because the features that use them aren't enabled yet.
> > In any case, setting them to 0 won't hurt.
>
> Coherent allocations are zero-initialized. So the upper 64-bits stay zero.
> People set 128-bit anyway because the hardware needs it.
Okay, that explains it then -- I didn't realize that dma_alloc_coherent() always
returns zeroed memory. It isn't mentioned in
Documentation/core-api/dma-api.rst, and there's no kerneldoc comment, so it
wasn't clear. But apparently it's intentional; see commit 518a2f1925c3
("dma-mapping: zero memory returned from dma_alloc_*").
I'll fix this commit message in the next version.
- Eric
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 3/9] mmc: cqhci: initialize upper 64 bits of 128-bit task descriptors
2020-12-03 2:05 ` [PATCH v2 3/9] mmc: cqhci: initialize upper 64 bits of 128-bit task descriptors Eric Biggers
2020-12-03 6:45 ` Adrian Hunter
@ 2020-12-08 23:45 ` Satya Tangirala
1 sibling, 0 replies; 28+ messages in thread
From: Satya Tangirala @ 2020-12-08 23:45 UTC (permalink / raw)
To: Eric Biggers
Cc: linux-mmc, linux-arm-msm, devicetree, linux-fscrypt, Ulf Hansson,
Andy Gross, Bjorn Andersson, Adrian Hunter, Asutosh Das,
Rob Herring, Neeraj Soni, Barani Muthukumaran, Peng Zhou,
Stanley Chu, Konrad Dybcio
On Wed, Dec 02, 2020 at 06:05:10PM -0800, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
>
> Move the task descriptor initialization into cqhci_prep_task_desc(), and
> make it initialize all 128 bits of the task descriptor if the host
> controller is using 128-bit task descriptors.
>
> This is needed to prepare for CQHCI inline encryption support, which
> requires 128-bit task descriptors and uses the upper 64 bits.
>
> Note: since some host controllers already enable 128-bit task
> descriptors, it's unclear why the previous code worked when it wasn't
> initializing the upper 64 bits. One possibility is that the bits are
> being ignored because the features that use them aren't enabled yet.
> In any case, setting them to 0 won't hurt.
>
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
> drivers/mmc/host/cqhci-core.c | 30 ++++++++++++++++++++----------
> 1 file changed, 20 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/mmc/host/cqhci-core.c b/drivers/mmc/host/cqhci-core.c
> index 697fe40756bf2..ad7c9acff1728 100644
> --- a/drivers/mmc/host/cqhci-core.c
> +++ b/drivers/mmc/host/cqhci-core.c
> @@ -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",
> --
> 2.29.2
>
Looks good to me. Please feel free to add
Reviewed-by: Satya Tangirala <satyat@google.com>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 4/9] mmc: cqhci: add support for inline encryption
2020-12-03 2:05 [PATCH v2 0/9] eMMC inline encryption support Eric Biggers
` (2 preceding siblings ...)
2020-12-03 2:05 ` [PATCH v2 3/9] mmc: cqhci: initialize upper 64 bits of 128-bit task descriptors Eric Biggers
@ 2020-12-03 2:05 ` Eric Biggers
2020-12-03 6:47 ` Adrian Hunter
` (2 more replies)
2020-12-03 2:05 ` [PATCH v2 5/9] mmc: cqhci: add cqhci_host_ops::program_key Eric Biggers
` (4 subsequent siblings)
8 siblings, 3 replies; 28+ messages in thread
From: Eric Biggers @ 2020-12-03 2:05 UTC (permalink / raw)
To: linux-mmc
Cc: linux-arm-msm, devicetree, linux-fscrypt, Satya Tangirala,
Ulf Hansson, Andy Gross, Bjorn Andersson, Adrian Hunter,
Asutosh Das, Rob Herring, Neeraj Soni, Barani Muthukumaran,
Peng Zhou, Stanley Chu, Konrad Dybcio
From: Eric Biggers <ebiggers@google.com>
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.
Co-developed-by: Satya Tangirala <satyat@google.com>
Signed-off-by: Satya Tangirala <satyat@google.com>
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
drivers/mmc/host/Makefile | 1 +
drivers/mmc/host/cqhci-core.c | 41 +++++-
drivers/mmc/host/cqhci-crypto.c | 241 ++++++++++++++++++++++++++++++++
drivers/mmc/host/cqhci-crypto.h | 47 +++++++
drivers/mmc/host/cqhci.h | 81 ++++++++++-
5 files changed, 408 insertions(+), 3 deletions(-)
create mode 100644 drivers/mmc/host/cqhci-crypto.c
create mode 100644 drivers/mmc/host/cqhci-crypto.h
diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
index 20c2f9463d0dc..35158508ab63d 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -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)
diff --git a/drivers/mmc/host/cqhci-core.c b/drivers/mmc/host/cqhci-core.c
index ad7c9acff1728..93b0432bb6011 100644
--- a/drivers/mmc/host/cqhci-core.c
+++ b/drivers/mmc/host/cqhci-core.c
@@ -18,6 +18,7 @@
#include <linux/mmc/card.h>
#include "cqhci.h"
+#include "cqhci-crypto.h"
#define DCMD_SLOT 31
#define NUM_SLOTS 32
@@ -258,6 +259,9 @@ static void __cqhci_enable(struct cqhci_host *cq_host)
if (cq_host->caps & CQHCI_TASK_DESC_SZ_128)
cqcfg |= CQHCI_TASK_DESC_SZ;
+ if (mmc->caps2 & MMC_CAP2_CRYPTO)
+ cqcfg |= CQHCI_CRYPTO_GENERAL_ENABLE;
+
cqhci_writel(cq_host, cqcfg, CQHCI_CFG);
cqhci_writel(cq_host, lower_32_bits(cq_host->desc_dma_base),
@@ -430,7 +434,7 @@ static void cqhci_prep_task_desc(struct mmc_request *mrq,
task_desc[0] = cpu_to_le64(desc0);
if (cq_host->caps & CQHCI_TASK_DESC_SZ_128) {
- u64 desc1 = 0;
+ u64 desc1 = cqhci_crypto_prep_task_desc(mrq);
task_desc[1] = cpu_to_le64(desc1);
@@ -681,6 +685,7 @@ static void cqhci_error_irq(struct mmc_host *mmc, u32 status, int cmd_error,
struct cqhci_host *cq_host = mmc->cqe_private;
struct cqhci_slot *slot;
u32 terri;
+ u32 tdpe;
int tag;
spin_lock(&cq_host->lock);
@@ -719,6 +724,30 @@ static void cqhci_error_irq(struct mmc_host *mmc, u32 status, int cmd_error,
}
}
+ /*
+ * Handle ICCE ("Invalid Crypto Configuration Error"). This should
+ * never happen, since the block layer ensures that all crypto-enabled
+ * I/O requests have a valid keyslot before they reach the driver.
+ *
+ * Note that GCE ("General Crypto Error") is different; it already got
+ * handled above by checking TERRI.
+ */
+ if (status & CQHCI_IS_ICCE) {
+ tdpe = cqhci_readl(cq_host, CQHCI_TDPE);
+ WARN_ONCE(1,
+ "%s: cqhci: invalid crypto configuration error. IRQ status: 0x%08x TDPE: 0x%08x\n",
+ mmc_hostname(mmc), status, tdpe);
+ while (tdpe != 0) {
+ tag = __ffs(tdpe);
+ tdpe &= ~(1 << tag);
+ slot = &cq_host->slot[tag];
+ if (!slot->mrq)
+ continue;
+ slot->flags = cqhci_error_flags(data_error, cmd_error);
+ cqhci_recovery_needed(mmc, slot->mrq, true);
+ }
+ }
+
if (!cq_host->recovery_halt) {
/*
* The only way to guarantee forward progress is to mark at
@@ -784,7 +813,8 @@ irqreturn_t cqhci_irq(struct mmc_host *mmc, u32 intmask, int cmd_error,
pr_debug("%s: cqhci: IRQ status: 0x%08x\n", mmc_hostname(mmc), status);
- if ((status & CQHCI_IS_RED) || cmd_error || data_error)
+ if ((status & (CQHCI_IS_RED | CQHCI_IS_GCE | CQHCI_IS_ICCE)) ||
+ cmd_error || data_error)
cqhci_error_irq(mmc, status, cmd_error, data_error);
if (status & CQHCI_IS_TCC) {
@@ -1151,6 +1181,13 @@ int cqhci_init(struct cqhci_host *cq_host, struct mmc_host *mmc,
goto out_err;
}
+ err = cqhci_crypto_init(cq_host);
+ if (err) {
+ pr_err("%s: CQHCI crypto initialization failed\n",
+ mmc_hostname(mmc));
+ goto out_err;
+ }
+
spin_lock_init(&cq_host->lock);
init_completion(&cq_host->halt_comp);
diff --git a/drivers/mmc/host/cqhci-crypto.c b/drivers/mmc/host/cqhci-crypto.c
new file mode 100644
index 0000000000000..98f141c8480ce
--- /dev/null
+++ b/drivers/mmc/host/cqhci-crypto.c
@@ -0,0 +1,241 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * CQHCI crypto engine (inline encryption) support
+ *
+ * Copyright 2020 Google LLC
+ */
+
+#include <linux/blk-crypto.h>
+#include <linux/keyslot-manager.h>
+#include <linux/mmc/host.h>
+
+#include "cqhci-crypto.h"
+
+/* Map from blk-crypto modes to CQHCI crypto algorithm IDs and key sizes */
+static const struct cqhci_crypto_alg_entry {
+ enum cqhci_crypto_alg alg;
+ enum cqhci_crypto_key_size key_size;
+} cqhci_crypto_algs[BLK_ENCRYPTION_MODE_MAX] = {
+ [BLK_ENCRYPTION_MODE_AES_256_XTS] = {
+ .alg = CQHCI_CRYPTO_ALG_AES_XTS,
+ .key_size = CQHCI_CRYPTO_KEY_SIZE_256,
+ },
+};
+
+static inline struct cqhci_host *
+cqhci_host_from_ksm(struct blk_keyslot_manager *ksm)
+{
+ struct mmc_host *mmc = container_of(ksm, struct mmc_host, 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)
+{
+ u32 slot_offset = cq_host->crypto_cfg_register + slot * sizeof(*cfg);
+ int i;
+
+ /* Clear CFGE */
+ cqhci_writel(cq_host, 0, slot_offset + 16 * sizeof(cfg->reg_val[0]));
+
+ /* Write the key */
+ for (i = 0; i < 16; i++) {
+ cqhci_writel(cq_host, le32_to_cpu(cfg->reg_val[i]),
+ slot_offset + i * sizeof(cfg->reg_val[0]));
+ }
+ /* Write dword 17 */
+ cqhci_writel(cq_host, le32_to_cpu(cfg->reg_val[17]),
+ slot_offset + 17 * sizeof(cfg->reg_val[0]));
+ /* 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]));
+}
+
+static int cqhci_crypto_keyslot_program(struct blk_keyslot_manager *ksm,
+ const struct blk_crypto_key *key,
+ unsigned int slot)
+
+{
+ struct cqhci_host *cq_host = cqhci_host_from_ksm(ksm);
+ const union cqhci_crypto_cap_entry *ccap_array =
+ cq_host->crypto_cap_array;
+ const struct cqhci_crypto_alg_entry *alg =
+ &cqhci_crypto_algs[key->crypto_cfg.crypto_mode];
+ u8 data_unit_mask = key->crypto_cfg.data_unit_size / 512;
+ int i;
+ int cap_idx = -1;
+ union cqhci_crypto_cfg_entry cfg = {};
+
+ BUILD_BUG_ON(CQHCI_CRYPTO_KEY_SIZE_INVALID != 0);
+ for (i = 0; i < cq_host->crypto_capabilities.num_crypto_cap; i++) {
+ if (ccap_array[i].algorithm_id == alg->alg &&
+ ccap_array[i].key_size == alg->key_size &&
+ (ccap_array[i].sdus_mask & data_unit_mask)) {
+ cap_idx = i;
+ break;
+ }
+ }
+ if (WARN_ON(cap_idx < 0))
+ return -EOPNOTSUPP;
+
+ cfg.data_unit_size = data_unit_mask;
+ cfg.crypto_cap_idx = cap_idx;
+ cfg.config_enable = CQHCI_CRYPTO_CONFIGURATION_ENABLE;
+
+ if (ccap_array[cap_idx].algorithm_id == CQHCI_CRYPTO_ALG_AES_XTS) {
+ /* In XTS mode, the blk_crypto_key's size is already doubled */
+ memcpy(cfg.crypto_key, key->raw, key->size/2);
+ memcpy(cfg.crypto_key + CQHCI_CRYPTO_KEY_MAX_SIZE/2,
+ key->raw + key->size/2, key->size/2);
+ } else {
+ memcpy(cfg.crypto_key, key->raw, key->size);
+ }
+
+ cqhci_crypto_program_key(cq_host, &cfg, slot);
+
+ memzero_explicit(&cfg, sizeof(cfg));
+ return 0;
+}
+
+static void cqhci_crypto_clear_keyslot(struct cqhci_host *cq_host, int slot)
+{
+ /*
+ * Clear the crypto cfg on the device. Clearing CFGE
+ * might not be sufficient, so just clear the entire cfg.
+ */
+ union cqhci_crypto_cfg_entry cfg = {};
+
+ cqhci_crypto_program_key(cq_host, &cfg, slot);
+}
+
+static int cqhci_crypto_keyslot_evict(struct blk_keyslot_manager *ksm,
+ const struct blk_crypto_key *key,
+ unsigned int slot)
+{
+ struct cqhci_host *cq_host = cqhci_host_from_ksm(ksm);
+
+ cqhci_crypto_clear_keyslot(cq_host, slot);
+ return 0;
+}
+
+/*
+ * The keyslot management operations for CQHCI crypto.
+ *
+ * Note that the block layer ensures that these are never called while the host
+ * controller is runtime-suspended. However, the CQE won't necessarily be
+ * "enabled" when these are called, i.e. CQHCI_ENABLE might not be set in the
+ * CQHCI_CFG register. But the hardware allows that.
+ */
+static const struct blk_ksm_ll_ops cqhci_ksm_ops = {
+ .keyslot_program = cqhci_crypto_keyslot_program,
+ .keyslot_evict = cqhci_crypto_keyslot_evict,
+};
+
+static enum blk_crypto_mode_num
+cqhci_find_blk_crypto_mode(union cqhci_crypto_cap_entry cap)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(cqhci_crypto_algs); i++) {
+ BUILD_BUG_ON(CQHCI_CRYPTO_KEY_SIZE_INVALID != 0);
+ if (cqhci_crypto_algs[i].alg == cap.algorithm_id &&
+ cqhci_crypto_algs[i].key_size == cap.key_size)
+ return i;
+ }
+ return BLK_ENCRYPTION_MODE_INVALID;
+}
+
+/**
+ * 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 = blk_ksm_init(ksm, num_keyslots);
+ if (err)
+ goto out_free_caps;
+
+ 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_free_caps:
+ devm_kfree(dev, cq_host->crypto_cap_array);
+ cq_host->crypto_cap_array = NULL;
+out:
+ mmc->caps2 &= ~MMC_CAP2_CRYPTO;
+ return err;
+}
diff --git a/drivers/mmc/host/cqhci-crypto.h b/drivers/mmc/host/cqhci-crypto.h
new file mode 100644
index 0000000000000..60b58ee0e6256
--- /dev/null
+++ b/drivers/mmc/host/cqhci-crypto.h
@@ -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;
+
+ 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;
+}
+
+static inline u64 cqhci_crypto_prep_task_desc(struct mmc_request *mrq)
+{
+ return 0;
+}
+
+#endif /* !CONFIG_MMC_CRYPTO */
+
+#endif /* LINUX_MMC_CQHCI_CRYPTO_H */
diff --git a/drivers/mmc/host/cqhci.h b/drivers/mmc/host/cqhci.h
index 89bf6adbce8ca..5c18734624fea 100644
--- a/drivers/mmc/host/cqhci.h
+++ b/drivers/mmc/host/cqhci.h
@@ -22,10 +22,13 @@
/* capabilities */
#define CQHCI_CAP 0x04
+#define CQHCI_CAP_CS 0x10000000 /* Crypto Support */
+
/* configuration */
#define CQHCI_CFG 0x08
#define CQHCI_DCMD 0x00001000
#define CQHCI_TASK_DESC_SZ 0x00000100
+#define CQHCI_CRYPTO_GENERAL_ENABLE 0x00000002
#define CQHCI_ENABLE 0x00000001
/* control */
@@ -39,8 +42,11 @@
#define CQHCI_IS_TCC BIT(1)
#define CQHCI_IS_RED BIT(2)
#define CQHCI_IS_TCL BIT(3)
+#define CQHCI_IS_GCE BIT(4) /* General Crypto Error */
+#define CQHCI_IS_ICCE BIT(5) /* Invalid Crypto Config Error */
-#define CQHCI_IS_MASK (CQHCI_IS_TCC | CQHCI_IS_RED)
+#define CQHCI_IS_MASK (CQHCI_IS_TCC | CQHCI_IS_RED | \
+ CQHCI_IS_GCE | CQHCI_IS_ICCE)
/* interrupt status enable */
#define CQHCI_ISTE 0x14
@@ -78,6 +84,9 @@
/* task clear */
#define CQHCI_TCLR 0x38
+/* task descriptor processing error */
+#define CQHCI_TDPE 0x3c
+
/* send status config 1 */
#define CQHCI_SSC1 0x40
#define CQHCI_SSC1_CBC_MASK GENMASK(19, 16)
@@ -107,6 +116,10 @@
/* command response argument */
#define CQHCI_CRA 0x5C
+/* crypto capabilities */
+#define CQHCI_CCAP 0x100
+#define CQHCI_CRYPTOCAP 0x104
+
#define CQHCI_INT_ALL 0xF
#define CQHCI_IC_DEFAULT_ICCTH 31
#define CQHCI_IC_DEFAULT_ICTOVAL 1
@@ -133,11 +146,71 @@
#define CQHCI_CMD_TIMING(x) (((x) & 1) << 22)
#define CQHCI_RESP_TYPE(x) (((x) & 0x3) << 23)
+/* crypto task descriptor fields (for bits 64-127 of task descriptor) */
+#define CQHCI_CRYPTO_ENABLE_BIT (1ULL << 47)
+#define CQHCI_CRYPTO_KEYSLOT(x) ((u64)(x) << 32)
+
/* transfer descriptor fields */
#define CQHCI_DAT_LENGTH(x) (((x) & 0xFFFF) << 16)
#define CQHCI_DAT_ADDR_LO(x) (((x) & 0xFFFFFFFF) << 32)
#define CQHCI_DAT_ADDR_HI(x) (((x) & 0xFFFFFFFF) << 0)
+/* CCAP - Crypto Capability 100h */
+union cqhci_crypto_capabilities {
+ __le32 reg_val;
+ struct {
+ u8 num_crypto_cap;
+ u8 config_count;
+ u8 reserved;
+ u8 config_array_ptr;
+ };
+};
+
+enum cqhci_crypto_key_size {
+ CQHCI_CRYPTO_KEY_SIZE_INVALID = 0,
+ CQHCI_CRYPTO_KEY_SIZE_128 = 1,
+ CQHCI_CRYPTO_KEY_SIZE_192 = 2,
+ CQHCI_CRYPTO_KEY_SIZE_256 = 3,
+ CQHCI_CRYPTO_KEY_SIZE_512 = 4,
+};
+
+enum cqhci_crypto_alg {
+ CQHCI_CRYPTO_ALG_AES_XTS = 0,
+ CQHCI_CRYPTO_ALG_BITLOCKER_AES_CBC = 1,
+ CQHCI_CRYPTO_ALG_AES_ECB = 2,
+ CQHCI_CRYPTO_ALG_ESSIV_AES_CBC = 3,
+};
+
+/* x-CRYPTOCAP - Crypto Capability X */
+union cqhci_crypto_cap_entry {
+ __le32 reg_val;
+ struct {
+ u8 algorithm_id;
+ u8 sdus_mask; /* Supported data unit size mask */
+ u8 key_size;
+ u8 reserved;
+ };
+};
+
+#define CQHCI_CRYPTO_CONFIGURATION_ENABLE (1 << 7)
+#define CQHCI_CRYPTO_KEY_MAX_SIZE 64
+/* x-CRYPTOCFG - Crypto Configuration X */
+union cqhci_crypto_cfg_entry {
+ __le32 reg_val[32];
+ struct {
+ u8 crypto_key[CQHCI_CRYPTO_KEY_MAX_SIZE];
+ /* 4KB/512 = 8 */
+ u8 data_unit_size;
+ u8 crypto_cap_idx;
+ u8 reserved_1;
+ u8 config_enable;
+ u8 reserved_multi_host;
+ u8 reserved_2;
+ u8 vsb[2];
+ u8 reserved_3[56];
+ };
+};
+
struct cqhci_host_ops;
struct mmc_host;
struct mmc_request;
@@ -196,6 +269,12 @@ struct cqhci_host {
struct completion halt_comp;
wait_queue_head_t wait_queue;
struct cqhci_slot *slot;
+
+#ifdef CONFIG_MMC_CRYPTO
+ union cqhci_crypto_capabilities crypto_capabilities;
+ union cqhci_crypto_cap_entry *crypto_cap_array;
+ u32 crypto_cfg_register;
+#endif
};
struct cqhci_host_ops {
--
2.29.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v2 4/9] mmc: cqhci: add support for inline encryption
2020-12-03 2:05 ` [PATCH v2 4/9] mmc: cqhci: add support for inline encryption Eric Biggers
@ 2020-12-03 6:47 ` Adrian Hunter
2020-12-05 10:59 ` Satya Tangirala
2020-12-05 12:28 ` Satya Tangirala
2 siblings, 0 replies; 28+ messages in thread
From: Adrian Hunter @ 2020-12-03 6:47 UTC (permalink / raw)
To: Eric Biggers, linux-mmc
Cc: linux-arm-msm, devicetree, linux-fscrypt, Satya Tangirala,
Ulf Hansson, Andy Gross, Bjorn Andersson, Asutosh Das,
Rob Herring, Neeraj Soni, Barani Muthukumaran, Peng Zhou,
Stanley Chu, Konrad Dybcio
On 3/12/20 4:05 am, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
>
> 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.
>
> Co-developed-by: Satya Tangirala <satyat@google.com>
> Signed-off-by: Satya Tangirala <satyat@google.com>
> Signed-off-by: Eric Biggers <ebiggers@google.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
> drivers/mmc/host/Makefile | 1 +
> drivers/mmc/host/cqhci-core.c | 41 +++++-
> drivers/mmc/host/cqhci-crypto.c | 241 ++++++++++++++++++++++++++++++++
> drivers/mmc/host/cqhci-crypto.h | 47 +++++++
> drivers/mmc/host/cqhci.h | 81 ++++++++++-
> 5 files changed, 408 insertions(+), 3 deletions(-)
> create mode 100644 drivers/mmc/host/cqhci-crypto.c
> create mode 100644 drivers/mmc/host/cqhci-crypto.h
>
> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
> index 20c2f9463d0dc..35158508ab63d 100644
> --- a/drivers/mmc/host/Makefile
> +++ b/drivers/mmc/host/Makefile
> @@ -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)
> diff --git a/drivers/mmc/host/cqhci-core.c b/drivers/mmc/host/cqhci-core.c
> index ad7c9acff1728..93b0432bb6011 100644
> --- a/drivers/mmc/host/cqhci-core.c
> +++ b/drivers/mmc/host/cqhci-core.c
> @@ -18,6 +18,7 @@
> #include <linux/mmc/card.h>
>
> #include "cqhci.h"
> +#include "cqhci-crypto.h"
>
> #define DCMD_SLOT 31
> #define NUM_SLOTS 32
> @@ -258,6 +259,9 @@ static void __cqhci_enable(struct cqhci_host *cq_host)
> if (cq_host->caps & CQHCI_TASK_DESC_SZ_128)
> cqcfg |= CQHCI_TASK_DESC_SZ;
>
> + if (mmc->caps2 & MMC_CAP2_CRYPTO)
> + cqcfg |= CQHCI_CRYPTO_GENERAL_ENABLE;
> +
> cqhci_writel(cq_host, cqcfg, CQHCI_CFG);
>
> cqhci_writel(cq_host, lower_32_bits(cq_host->desc_dma_base),
> @@ -430,7 +434,7 @@ static void cqhci_prep_task_desc(struct mmc_request *mrq,
> task_desc[0] = cpu_to_le64(desc0);
>
> if (cq_host->caps & CQHCI_TASK_DESC_SZ_128) {
> - u64 desc1 = 0;
> + u64 desc1 = cqhci_crypto_prep_task_desc(mrq);
>
> task_desc[1] = cpu_to_le64(desc1);
>
> @@ -681,6 +685,7 @@ static void cqhci_error_irq(struct mmc_host *mmc, u32 status, int cmd_error,
> struct cqhci_host *cq_host = mmc->cqe_private;
> struct cqhci_slot *slot;
> u32 terri;
> + u32 tdpe;
> int tag;
>
> spin_lock(&cq_host->lock);
> @@ -719,6 +724,30 @@ static void cqhci_error_irq(struct mmc_host *mmc, u32 status, int cmd_error,
> }
> }
>
> + /*
> + * Handle ICCE ("Invalid Crypto Configuration Error"). This should
> + * never happen, since the block layer ensures that all crypto-enabled
> + * I/O requests have a valid keyslot before they reach the driver.
> + *
> + * Note that GCE ("General Crypto Error") is different; it already got
> + * handled above by checking TERRI.
> + */
> + if (status & CQHCI_IS_ICCE) {
> + tdpe = cqhci_readl(cq_host, CQHCI_TDPE);
> + WARN_ONCE(1,
> + "%s: cqhci: invalid crypto configuration error. IRQ status: 0x%08x TDPE: 0x%08x\n",
> + mmc_hostname(mmc), status, tdpe);
> + while (tdpe != 0) {
> + tag = __ffs(tdpe);
> + tdpe &= ~(1 << tag);
> + slot = &cq_host->slot[tag];
> + if (!slot->mrq)
> + continue;
> + slot->flags = cqhci_error_flags(data_error, cmd_error);
> + cqhci_recovery_needed(mmc, slot->mrq, true);
> + }
> + }
> +
> if (!cq_host->recovery_halt) {
> /*
> * The only way to guarantee forward progress is to mark at
> @@ -784,7 +813,8 @@ irqreturn_t cqhci_irq(struct mmc_host *mmc, u32 intmask, int cmd_error,
>
> pr_debug("%s: cqhci: IRQ status: 0x%08x\n", mmc_hostname(mmc), status);
>
> - if ((status & CQHCI_IS_RED) || cmd_error || data_error)
> + if ((status & (CQHCI_IS_RED | CQHCI_IS_GCE | CQHCI_IS_ICCE)) ||
> + cmd_error || data_error)
> cqhci_error_irq(mmc, status, cmd_error, data_error);
>
> if (status & CQHCI_IS_TCC) {
> @@ -1151,6 +1181,13 @@ int cqhci_init(struct cqhci_host *cq_host, struct mmc_host *mmc,
> goto out_err;
> }
>
> + err = cqhci_crypto_init(cq_host);
> + if (err) {
> + pr_err("%s: CQHCI crypto initialization failed\n",
> + mmc_hostname(mmc));
> + goto out_err;
> + }
> +
> spin_lock_init(&cq_host->lock);
>
> init_completion(&cq_host->halt_comp);
> diff --git a/drivers/mmc/host/cqhci-crypto.c b/drivers/mmc/host/cqhci-crypto.c
> new file mode 100644
> index 0000000000000..98f141c8480ce
> --- /dev/null
> +++ b/drivers/mmc/host/cqhci-crypto.c
> @@ -0,0 +1,241 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * CQHCI crypto engine (inline encryption) support
> + *
> + * Copyright 2020 Google LLC
> + */
> +
> +#include <linux/blk-crypto.h>
> +#include <linux/keyslot-manager.h>
> +#include <linux/mmc/host.h>
> +
> +#include "cqhci-crypto.h"
> +
> +/* Map from blk-crypto modes to CQHCI crypto algorithm IDs and key sizes */
> +static const struct cqhci_crypto_alg_entry {
> + enum cqhci_crypto_alg alg;
> + enum cqhci_crypto_key_size key_size;
> +} cqhci_crypto_algs[BLK_ENCRYPTION_MODE_MAX] = {
> + [BLK_ENCRYPTION_MODE_AES_256_XTS] = {
> + .alg = CQHCI_CRYPTO_ALG_AES_XTS,
> + .key_size = CQHCI_CRYPTO_KEY_SIZE_256,
> + },
> +};
> +
> +static inline struct cqhci_host *
> +cqhci_host_from_ksm(struct blk_keyslot_manager *ksm)
> +{
> + struct mmc_host *mmc = container_of(ksm, struct mmc_host, 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)
> +{
> + u32 slot_offset = cq_host->crypto_cfg_register + slot * sizeof(*cfg);
> + int i;
> +
> + /* Clear CFGE */
> + cqhci_writel(cq_host, 0, slot_offset + 16 * sizeof(cfg->reg_val[0]));
> +
> + /* Write the key */
> + for (i = 0; i < 16; i++) {
> + cqhci_writel(cq_host, le32_to_cpu(cfg->reg_val[i]),
> + slot_offset + i * sizeof(cfg->reg_val[0]));
> + }
> + /* Write dword 17 */
> + cqhci_writel(cq_host, le32_to_cpu(cfg->reg_val[17]),
> + slot_offset + 17 * sizeof(cfg->reg_val[0]));
> + /* 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]));
> +}
> +
> +static int cqhci_crypto_keyslot_program(struct blk_keyslot_manager *ksm,
> + const struct blk_crypto_key *key,
> + unsigned int slot)
> +
> +{
> + struct cqhci_host *cq_host = cqhci_host_from_ksm(ksm);
> + const union cqhci_crypto_cap_entry *ccap_array =
> + cq_host->crypto_cap_array;
> + const struct cqhci_crypto_alg_entry *alg =
> + &cqhci_crypto_algs[key->crypto_cfg.crypto_mode];
> + u8 data_unit_mask = key->crypto_cfg.data_unit_size / 512;
> + int i;
> + int cap_idx = -1;
> + union cqhci_crypto_cfg_entry cfg = {};
> +
> + BUILD_BUG_ON(CQHCI_CRYPTO_KEY_SIZE_INVALID != 0);
> + for (i = 0; i < cq_host->crypto_capabilities.num_crypto_cap; i++) {
> + if (ccap_array[i].algorithm_id == alg->alg &&
> + ccap_array[i].key_size == alg->key_size &&
> + (ccap_array[i].sdus_mask & data_unit_mask)) {
> + cap_idx = i;
> + break;
> + }
> + }
> + if (WARN_ON(cap_idx < 0))
> + return -EOPNOTSUPP;
> +
> + cfg.data_unit_size = data_unit_mask;
> + cfg.crypto_cap_idx = cap_idx;
> + cfg.config_enable = CQHCI_CRYPTO_CONFIGURATION_ENABLE;
> +
> + if (ccap_array[cap_idx].algorithm_id == CQHCI_CRYPTO_ALG_AES_XTS) {
> + /* In XTS mode, the blk_crypto_key's size is already doubled */
> + memcpy(cfg.crypto_key, key->raw, key->size/2);
> + memcpy(cfg.crypto_key + CQHCI_CRYPTO_KEY_MAX_SIZE/2,
> + key->raw + key->size/2, key->size/2);
> + } else {
> + memcpy(cfg.crypto_key, key->raw, key->size);
> + }
> +
> + cqhci_crypto_program_key(cq_host, &cfg, slot);
> +
> + memzero_explicit(&cfg, sizeof(cfg));
> + return 0;
> +}
> +
> +static void cqhci_crypto_clear_keyslot(struct cqhci_host *cq_host, int slot)
> +{
> + /*
> + * Clear the crypto cfg on the device. Clearing CFGE
> + * might not be sufficient, so just clear the entire cfg.
> + */
> + union cqhci_crypto_cfg_entry cfg = {};
> +
> + cqhci_crypto_program_key(cq_host, &cfg, slot);
> +}
> +
> +static int cqhci_crypto_keyslot_evict(struct blk_keyslot_manager *ksm,
> + const struct blk_crypto_key *key,
> + unsigned int slot)
> +{
> + struct cqhci_host *cq_host = cqhci_host_from_ksm(ksm);
> +
> + cqhci_crypto_clear_keyslot(cq_host, slot);
> + return 0;
> +}
> +
> +/*
> + * The keyslot management operations for CQHCI crypto.
> + *
> + * Note that the block layer ensures that these are never called while the host
> + * controller is runtime-suspended. However, the CQE won't necessarily be
> + * "enabled" when these are called, i.e. CQHCI_ENABLE might not be set in the
> + * CQHCI_CFG register. But the hardware allows that.
> + */
> +static const struct blk_ksm_ll_ops cqhci_ksm_ops = {
> + .keyslot_program = cqhci_crypto_keyslot_program,
> + .keyslot_evict = cqhci_crypto_keyslot_evict,
> +};
> +
> +static enum blk_crypto_mode_num
> +cqhci_find_blk_crypto_mode(union cqhci_crypto_cap_entry cap)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(cqhci_crypto_algs); i++) {
> + BUILD_BUG_ON(CQHCI_CRYPTO_KEY_SIZE_INVALID != 0);
> + if (cqhci_crypto_algs[i].alg == cap.algorithm_id &&
> + cqhci_crypto_algs[i].key_size == cap.key_size)
> + return i;
> + }
> + return BLK_ENCRYPTION_MODE_INVALID;
> +}
> +
> +/**
> + * 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 = blk_ksm_init(ksm, num_keyslots);
> + if (err)
> + goto out_free_caps;
> +
> + 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_free_caps:
> + devm_kfree(dev, cq_host->crypto_cap_array);
> + cq_host->crypto_cap_array = NULL;
> +out:
> + mmc->caps2 &= ~MMC_CAP2_CRYPTO;
> + return err;
> +}
> diff --git a/drivers/mmc/host/cqhci-crypto.h b/drivers/mmc/host/cqhci-crypto.h
> new file mode 100644
> index 0000000000000..60b58ee0e6256
> --- /dev/null
> +++ b/drivers/mmc/host/cqhci-crypto.h
> @@ -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;
> +
> + 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;
> +}
> +
> +static inline u64 cqhci_crypto_prep_task_desc(struct mmc_request *mrq)
> +{
> + return 0;
> +}
> +
> +#endif /* !CONFIG_MMC_CRYPTO */
> +
> +#endif /* LINUX_MMC_CQHCI_CRYPTO_H */
> diff --git a/drivers/mmc/host/cqhci.h b/drivers/mmc/host/cqhci.h
> index 89bf6adbce8ca..5c18734624fea 100644
> --- a/drivers/mmc/host/cqhci.h
> +++ b/drivers/mmc/host/cqhci.h
> @@ -22,10 +22,13 @@
>
> /* capabilities */
> #define CQHCI_CAP 0x04
> +#define CQHCI_CAP_CS 0x10000000 /* Crypto Support */
> +
> /* configuration */
> #define CQHCI_CFG 0x08
> #define CQHCI_DCMD 0x00001000
> #define CQHCI_TASK_DESC_SZ 0x00000100
> +#define CQHCI_CRYPTO_GENERAL_ENABLE 0x00000002
> #define CQHCI_ENABLE 0x00000001
>
> /* control */
> @@ -39,8 +42,11 @@
> #define CQHCI_IS_TCC BIT(1)
> #define CQHCI_IS_RED BIT(2)
> #define CQHCI_IS_TCL BIT(3)
> +#define CQHCI_IS_GCE BIT(4) /* General Crypto Error */
> +#define CQHCI_IS_ICCE BIT(5) /* Invalid Crypto Config Error */
>
> -#define CQHCI_IS_MASK (CQHCI_IS_TCC | CQHCI_IS_RED)
> +#define CQHCI_IS_MASK (CQHCI_IS_TCC | CQHCI_IS_RED | \
> + CQHCI_IS_GCE | CQHCI_IS_ICCE)
>
> /* interrupt status enable */
> #define CQHCI_ISTE 0x14
> @@ -78,6 +84,9 @@
> /* task clear */
> #define CQHCI_TCLR 0x38
>
> +/* task descriptor processing error */
> +#define CQHCI_TDPE 0x3c
> +
> /* send status config 1 */
> #define CQHCI_SSC1 0x40
> #define CQHCI_SSC1_CBC_MASK GENMASK(19, 16)
> @@ -107,6 +116,10 @@
> /* command response argument */
> #define CQHCI_CRA 0x5C
>
> +/* crypto capabilities */
> +#define CQHCI_CCAP 0x100
> +#define CQHCI_CRYPTOCAP 0x104
> +
> #define CQHCI_INT_ALL 0xF
> #define CQHCI_IC_DEFAULT_ICCTH 31
> #define CQHCI_IC_DEFAULT_ICTOVAL 1
> @@ -133,11 +146,71 @@
> #define CQHCI_CMD_TIMING(x) (((x) & 1) << 22)
> #define CQHCI_RESP_TYPE(x) (((x) & 0x3) << 23)
>
> +/* crypto task descriptor fields (for bits 64-127 of task descriptor) */
> +#define CQHCI_CRYPTO_ENABLE_BIT (1ULL << 47)
> +#define CQHCI_CRYPTO_KEYSLOT(x) ((u64)(x) << 32)
> +
> /* transfer descriptor fields */
> #define CQHCI_DAT_LENGTH(x) (((x) & 0xFFFF) << 16)
> #define CQHCI_DAT_ADDR_LO(x) (((x) & 0xFFFFFFFF) << 32)
> #define CQHCI_DAT_ADDR_HI(x) (((x) & 0xFFFFFFFF) << 0)
>
> +/* CCAP - Crypto Capability 100h */
> +union cqhci_crypto_capabilities {
> + __le32 reg_val;
> + struct {
> + u8 num_crypto_cap;
> + u8 config_count;
> + u8 reserved;
> + u8 config_array_ptr;
> + };
> +};
> +
> +enum cqhci_crypto_key_size {
> + CQHCI_CRYPTO_KEY_SIZE_INVALID = 0,
> + CQHCI_CRYPTO_KEY_SIZE_128 = 1,
> + CQHCI_CRYPTO_KEY_SIZE_192 = 2,
> + CQHCI_CRYPTO_KEY_SIZE_256 = 3,
> + CQHCI_CRYPTO_KEY_SIZE_512 = 4,
> +};
> +
> +enum cqhci_crypto_alg {
> + CQHCI_CRYPTO_ALG_AES_XTS = 0,
> + CQHCI_CRYPTO_ALG_BITLOCKER_AES_CBC = 1,
> + CQHCI_CRYPTO_ALG_AES_ECB = 2,
> + CQHCI_CRYPTO_ALG_ESSIV_AES_CBC = 3,
> +};
> +
> +/* x-CRYPTOCAP - Crypto Capability X */
> +union cqhci_crypto_cap_entry {
> + __le32 reg_val;
> + struct {
> + u8 algorithm_id;
> + u8 sdus_mask; /* Supported data unit size mask */
> + u8 key_size;
> + u8 reserved;
> + };
> +};
> +
> +#define CQHCI_CRYPTO_CONFIGURATION_ENABLE (1 << 7)
> +#define CQHCI_CRYPTO_KEY_MAX_SIZE 64
> +/* x-CRYPTOCFG - Crypto Configuration X */
> +union cqhci_crypto_cfg_entry {
> + __le32 reg_val[32];
> + struct {
> + u8 crypto_key[CQHCI_CRYPTO_KEY_MAX_SIZE];
> + /* 4KB/512 = 8 */
> + u8 data_unit_size;
> + u8 crypto_cap_idx;
> + u8 reserved_1;
> + u8 config_enable;
> + u8 reserved_multi_host;
> + u8 reserved_2;
> + u8 vsb[2];
> + u8 reserved_3[56];
> + };
> +};
> +
> struct cqhci_host_ops;
> struct mmc_host;
> struct mmc_request;
> @@ -196,6 +269,12 @@ struct cqhci_host {
> struct completion halt_comp;
> wait_queue_head_t wait_queue;
> struct cqhci_slot *slot;
> +
> +#ifdef CONFIG_MMC_CRYPTO
> + union cqhci_crypto_capabilities crypto_capabilities;
> + union cqhci_crypto_cap_entry *crypto_cap_array;
> + u32 crypto_cfg_register;
> +#endif
> };
>
> struct cqhci_host_ops {
>
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 4/9] mmc: cqhci: add support for inline encryption
2020-12-03 2:05 ` [PATCH v2 4/9] mmc: cqhci: add support for inline encryption Eric Biggers
2020-12-03 6:47 ` Adrian Hunter
@ 2020-12-05 10:59 ` Satya Tangirala
2020-12-05 12:33 ` Satya Tangirala
2020-12-05 12:28 ` Satya Tangirala
2 siblings, 1 reply; 28+ messages in thread
From: Satya Tangirala @ 2020-12-05 10:59 UTC (permalink / raw)
To: Eric Biggers
Cc: linux-mmc, linux-arm-msm, devicetree, linux-fscrypt, Ulf Hansson,
Andy Gross, Bjorn Andersson, Adrian Hunter, Asutosh Das,
Rob Herring, Neeraj Soni, Barani Muthukumaran, Peng Zhou,
Stanley Chu, Konrad Dybcio
On Wed, Dec 02, 2020 at 06:05:11PM -0800, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
>
> 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.
>
> Co-developed-by: Satya Tangirala <satyat@google.com>
> Signed-off-by: Satya Tangirala <satyat@google.com>
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
> drivers/mmc/host/Makefile | 1 +
> drivers/mmc/host/cqhci-core.c | 41 +++++-
> drivers/mmc/host/cqhci-crypto.c | 241 ++++++++++++++++++++++++++++++++
> drivers/mmc/host/cqhci-crypto.h | 47 +++++++
> drivers/mmc/host/cqhci.h | 81 ++++++++++-
> 5 files changed, 408 insertions(+), 3 deletions(-)
> create mode 100644 drivers/mmc/host/cqhci-crypto.c
> create mode 100644 drivers/mmc/host/cqhci-crypto.h
>
> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
> index 20c2f9463d0dc..35158508ab63d 100644
> --- a/drivers/mmc/host/Makefile
> +++ b/drivers/mmc/host/Makefile
> @@ -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)
> diff --git a/drivers/mmc/host/cqhci-core.c b/drivers/mmc/host/cqhci-core.c
> index ad7c9acff1728..93b0432bb6011 100644
> --- a/drivers/mmc/host/cqhci-core.c
> +++ b/drivers/mmc/host/cqhci-core.c
> @@ -18,6 +18,7 @@
> #include <linux/mmc/card.h>
>
> #include "cqhci.h"
> +#include "cqhci-crypto.h"
>
> #define DCMD_SLOT 31
> #define NUM_SLOTS 32
> @@ -258,6 +259,9 @@ static void __cqhci_enable(struct cqhci_host *cq_host)
> if (cq_host->caps & CQHCI_TASK_DESC_SZ_128)
> cqcfg |= CQHCI_TASK_DESC_SZ;
>
> + if (mmc->caps2 & MMC_CAP2_CRYPTO)
> + cqcfg |= CQHCI_CRYPTO_GENERAL_ENABLE;
> +
> cqhci_writel(cq_host, cqcfg, CQHCI_CFG);
>
> cqhci_writel(cq_host, lower_32_bits(cq_host->desc_dma_base),
> @@ -430,7 +434,7 @@ static void cqhci_prep_task_desc(struct mmc_request *mrq,
> task_desc[0] = cpu_to_le64(desc0);
>
> if (cq_host->caps & CQHCI_TASK_DESC_SZ_128) {
> - u64 desc1 = 0;
> + u64 desc1 = cqhci_crypto_prep_task_desc(mrq);
>
> task_desc[1] = cpu_to_le64(desc1);
>
> @@ -681,6 +685,7 @@ static void cqhci_error_irq(struct mmc_host *mmc, u32 status, int cmd_error,
> struct cqhci_host *cq_host = mmc->cqe_private;
> struct cqhci_slot *slot;
> u32 terri;
> + u32 tdpe;
> int tag;
>
> spin_lock(&cq_host->lock);
> @@ -719,6 +724,30 @@ static void cqhci_error_irq(struct mmc_host *mmc, u32 status, int cmd_error,
> }
> }
>
> + /*
> + * Handle ICCE ("Invalid Crypto Configuration Error"). This should
> + * never happen, since the block layer ensures that all crypto-enabled
> + * I/O requests have a valid keyslot before they reach the driver.
> + *
> + * Note that GCE ("General Crypto Error") is different; it already got
> + * handled above by checking TERRI.
> + */
> + if (status & CQHCI_IS_ICCE) {
> + tdpe = cqhci_readl(cq_host, CQHCI_TDPE);
> + WARN_ONCE(1,
> + "%s: cqhci: invalid crypto configuration error. IRQ status: 0x%08x TDPE: 0x%08x\n",
> + mmc_hostname(mmc), status, tdpe);
> + while (tdpe != 0) {
> + tag = __ffs(tdpe);
> + tdpe &= ~(1 << tag);
> + slot = &cq_host->slot[tag];
> + if (!slot->mrq)
> + continue;
> + slot->flags = cqhci_error_flags(data_error, cmd_error);
> + cqhci_recovery_needed(mmc, slot->mrq, true);
> + }
> + }
> +
> if (!cq_host->recovery_halt) {
> /*
> * The only way to guarantee forward progress is to mark at
> @@ -784,7 +813,8 @@ irqreturn_t cqhci_irq(struct mmc_host *mmc, u32 intmask, int cmd_error,
>
> pr_debug("%s: cqhci: IRQ status: 0x%08x\n", mmc_hostname(mmc), status);
>
> - if ((status & CQHCI_IS_RED) || cmd_error || data_error)
> + if ((status & (CQHCI_IS_RED | CQHCI_IS_GCE | CQHCI_IS_ICCE)) ||
> + cmd_error || data_error)
> cqhci_error_irq(mmc, status, cmd_error, data_error);
>
> if (status & CQHCI_IS_TCC) {
> @@ -1151,6 +1181,13 @@ int cqhci_init(struct cqhci_host *cq_host, struct mmc_host *mmc,
> goto out_err;
> }
>
> + err = cqhci_crypto_init(cq_host);
> + if (err) {
> + pr_err("%s: CQHCI crypto initialization failed\n",
> + mmc_hostname(mmc));
> + goto out_err;
> + }
> +
> spin_lock_init(&cq_host->lock);
>
> init_completion(&cq_host->halt_comp);
> diff --git a/drivers/mmc/host/cqhci-crypto.c b/drivers/mmc/host/cqhci-crypto.c
> new file mode 100644
> index 0000000000000..98f141c8480ce
> --- /dev/null
> +++ b/drivers/mmc/host/cqhci-crypto.c
> @@ -0,0 +1,241 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * CQHCI crypto engine (inline encryption) support
> + *
> + * Copyright 2020 Google LLC
> + */
> +
> +#include <linux/blk-crypto.h>
> +#include <linux/keyslot-manager.h>
> +#include <linux/mmc/host.h>
> +
> +#include "cqhci-crypto.h"
> +
> +/* Map from blk-crypto modes to CQHCI crypto algorithm IDs and key sizes */
> +static const struct cqhci_crypto_alg_entry {
> + enum cqhci_crypto_alg alg;
> + enum cqhci_crypto_key_size key_size;
> +} cqhci_crypto_algs[BLK_ENCRYPTION_MODE_MAX] = {
> + [BLK_ENCRYPTION_MODE_AES_256_XTS] = {
> + .alg = CQHCI_CRYPTO_ALG_AES_XTS,
> + .key_size = CQHCI_CRYPTO_KEY_SIZE_256,
> + },
> +};
> +
> +static inline struct cqhci_host *
> +cqhci_host_from_ksm(struct blk_keyslot_manager *ksm)
> +{
> + struct mmc_host *mmc = container_of(ksm, struct mmc_host, 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)
> +{
> + u32 slot_offset = cq_host->crypto_cfg_register + slot * sizeof(*cfg);
> + int i;
> +
> + /* Clear CFGE */
> + cqhci_writel(cq_host, 0, slot_offset + 16 * sizeof(cfg->reg_val[0]));
> +
> + /* Write the key */
> + for (i = 0; i < 16; i++) {
> + cqhci_writel(cq_host, le32_to_cpu(cfg->reg_val[i]),
> + slot_offset + i * sizeof(cfg->reg_val[0]));
> + }
> + /* Write dword 17 */
> + cqhci_writel(cq_host, le32_to_cpu(cfg->reg_val[17]),
> + slot_offset + 17 * sizeof(cfg->reg_val[0]));
> + /* 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]));
> +}
> +
> +static int cqhci_crypto_keyslot_program(struct blk_keyslot_manager *ksm,
> + const struct blk_crypto_key *key,
> + unsigned int slot)
> +
> +{
> + struct cqhci_host *cq_host = cqhci_host_from_ksm(ksm);
> + const union cqhci_crypto_cap_entry *ccap_array =
> + cq_host->crypto_cap_array;
> + const struct cqhci_crypto_alg_entry *alg =
> + &cqhci_crypto_algs[key->crypto_cfg.crypto_mode];
> + u8 data_unit_mask = key->crypto_cfg.data_unit_size / 512;
> + int i;
> + int cap_idx = -1;
> + union cqhci_crypto_cfg_entry cfg = {};
> +
> + BUILD_BUG_ON(CQHCI_CRYPTO_KEY_SIZE_INVALID != 0);
> + for (i = 0; i < cq_host->crypto_capabilities.num_crypto_cap; i++) {
> + if (ccap_array[i].algorithm_id == alg->alg &&
> + ccap_array[i].key_size == alg->key_size &&
> + (ccap_array[i].sdus_mask & data_unit_mask)) {
> + cap_idx = i;
> + break;
> + }
> + }
> + if (WARN_ON(cap_idx < 0))
> + return -EOPNOTSUPP;
> +
> + cfg.data_unit_size = data_unit_mask;
> + cfg.crypto_cap_idx = cap_idx;
> + cfg.config_enable = CQHCI_CRYPTO_CONFIGURATION_ENABLE;
> +
> + if (ccap_array[cap_idx].algorithm_id == CQHCI_CRYPTO_ALG_AES_XTS) {
> + /* In XTS mode, the blk_crypto_key's size is already doubled */
> + memcpy(cfg.crypto_key, key->raw, key->size/2);
> + memcpy(cfg.crypto_key + CQHCI_CRYPTO_KEY_MAX_SIZE/2,
> + key->raw + key->size/2, key->size/2);
> + } else {
> + memcpy(cfg.crypto_key, key->raw, key->size);
> + }
> +
> + cqhci_crypto_program_key(cq_host, &cfg, slot);
> +
> + memzero_explicit(&cfg, sizeof(cfg));
> + return 0;
> +}
> +
> +static void cqhci_crypto_clear_keyslot(struct cqhci_host *cq_host, int slot)
> +{
> + /*
> + * Clear the crypto cfg on the device. Clearing CFGE
> + * might not be sufficient, so just clear the entire cfg.
> + */
> + union cqhci_crypto_cfg_entry cfg = {};
> +
> + cqhci_crypto_program_key(cq_host, &cfg, slot);
> +}
> +
> +static int cqhci_crypto_keyslot_evict(struct blk_keyslot_manager *ksm,
> + const struct blk_crypto_key *key,
> + unsigned int slot)
> +{
> + struct cqhci_host *cq_host = cqhci_host_from_ksm(ksm);
> +
> + cqhci_crypto_clear_keyslot(cq_host, slot);
> + return 0;
> +}
> +
> +/*
> + * The keyslot management operations for CQHCI crypto.
> + *
> + * Note that the block layer ensures that these are never called while the host
> + * controller is runtime-suspended. However, the CQE won't necessarily be
> + * "enabled" when these are called, i.e. CQHCI_ENABLE might not be set in the
> + * CQHCI_CFG register. But the hardware allows that.
> + */
> +static const struct blk_ksm_ll_ops cqhci_ksm_ops = {
> + .keyslot_program = cqhci_crypto_keyslot_program,
> + .keyslot_evict = cqhci_crypto_keyslot_evict,
> +};
> +
> +static enum blk_crypto_mode_num
> +cqhci_find_blk_crypto_mode(union cqhci_crypto_cap_entry cap)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(cqhci_crypto_algs); i++) {
> + BUILD_BUG_ON(CQHCI_CRYPTO_KEY_SIZE_INVALID != 0);
> + if (cqhci_crypto_algs[i].alg == cap.algorithm_id &&
> + cqhci_crypto_algs[i].key_size == cap.key_size)
> + return i;
> + }
> + return BLK_ENCRYPTION_MODE_INVALID;
> +}
> +
> +/**
> + * 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 = blk_ksm_init(ksm, num_keyslots);
> + if (err)
> + goto out_free_caps;
> +
> + 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_free_caps:
> + devm_kfree(dev, cq_host->crypto_cap_array);
> + cq_host->crypto_cap_array = NULL;
> +out:
> + mmc->caps2 &= ~MMC_CAP2_CRYPTO;
> + return err;
> +}
> diff --git a/drivers/mmc/host/cqhci-crypto.h b/drivers/mmc/host/cqhci-crypto.h
> new file mode 100644
> index 0000000000000..60b58ee0e6256
> --- /dev/null
> +++ b/drivers/mmc/host/cqhci-crypto.h
> @@ -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;
> +
> + 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;
> +}
> +
> +static inline u64 cqhci_crypto_prep_task_desc(struct mmc_request *mrq)
> +{
> + return 0;
> +}
> +
> +#endif /* !CONFIG_MMC_CRYPTO */
> +
> +#endif /* LINUX_MMC_CQHCI_CRYPTO_H */
> diff --git a/drivers/mmc/host/cqhci.h b/drivers/mmc/host/cqhci.h
> index 89bf6adbce8ca..5c18734624fea 100644
> --- a/drivers/mmc/host/cqhci.h
> +++ b/drivers/mmc/host/cqhci.h
> @@ -22,10 +22,13 @@
>
> /* capabilities */
> #define CQHCI_CAP 0x04
> +#define CQHCI_CAP_CS 0x10000000 /* Crypto Support */
> +
> /* configuration */
> #define CQHCI_CFG 0x08
> #define CQHCI_DCMD 0x00001000
> #define CQHCI_TASK_DESC_SZ 0x00000100
> +#define CQHCI_CRYPTO_GENERAL_ENABLE 0x00000002
> #define CQHCI_ENABLE 0x00000001
>
> /* control */
> @@ -39,8 +42,11 @@
> #define CQHCI_IS_TCC BIT(1)
> #define CQHCI_IS_RED BIT(2)
> #define CQHCI_IS_TCL BIT(3)
> +#define CQHCI_IS_GCE BIT(4) /* General Crypto Error */
> +#define CQHCI_IS_ICCE BIT(5) /* Invalid Crypto Config Error */
>
> -#define CQHCI_IS_MASK (CQHCI_IS_TCC | CQHCI_IS_RED)
> +#define CQHCI_IS_MASK (CQHCI_IS_TCC | CQHCI_IS_RED | \
> + CQHCI_IS_GCE | CQHCI_IS_ICCE)
>
> /* interrupt status enable */
> #define CQHCI_ISTE 0x14
> @@ -78,6 +84,9 @@
> /* task clear */
> #define CQHCI_TCLR 0x38
>
> +/* task descriptor processing error */
> +#define CQHCI_TDPE 0x3c
> +
> /* send status config 1 */
> #define CQHCI_SSC1 0x40
> #define CQHCI_SSC1_CBC_MASK GENMASK(19, 16)
> @@ -107,6 +116,10 @@
> /* command response argument */
> #define CQHCI_CRA 0x5C
>
> +/* crypto capabilities */
> +#define CQHCI_CCAP 0x100
> +#define CQHCI_CRYPTOCAP 0x104
> +
> #define CQHCI_INT_ALL 0xF
> #define CQHCI_IC_DEFAULT_ICCTH 31
> #define CQHCI_IC_DEFAULT_ICTOVAL 1
> @@ -133,11 +146,71 @@
> #define CQHCI_CMD_TIMING(x) (((x) & 1) << 22)
> #define CQHCI_RESP_TYPE(x) (((x) & 0x3) << 23)
>
> +/* crypto task descriptor fields (for bits 64-127 of task descriptor) */
> +#define CQHCI_CRYPTO_ENABLE_BIT (1ULL << 47)
> +#define CQHCI_CRYPTO_KEYSLOT(x) ((u64)(x) << 32)
> +
> /* transfer descriptor fields */
> #define CQHCI_DAT_LENGTH(x) (((x) & 0xFFFF) << 16)
> #define CQHCI_DAT_ADDR_LO(x) (((x) & 0xFFFFFFFF) << 32)
> #define CQHCI_DAT_ADDR_HI(x) (((x) & 0xFFFFFFFF) << 0)
>
> +/* CCAP - Crypto Capability 100h */
> +union cqhci_crypto_capabilities {
> + __le32 reg_val;
> + struct {
> + u8 num_crypto_cap;
> + u8 config_count;
> + u8 reserved;
> + u8 config_array_ptr;
> + };
> +};
> +
> +enum cqhci_crypto_key_size {
> + CQHCI_CRYPTO_KEY_SIZE_INVALID = 0,
> + CQHCI_CRYPTO_KEY_SIZE_128 = 1,
> + CQHCI_CRYPTO_KEY_SIZE_192 = 2,
> + CQHCI_CRYPTO_KEY_SIZE_256 = 3,
> + CQHCI_CRYPTO_KEY_SIZE_512 = 4,
> +};
> +
> +enum cqhci_crypto_alg {
> + CQHCI_CRYPTO_ALG_AES_XTS = 0,
> + CQHCI_CRYPTO_ALG_BITLOCKER_AES_CBC = 1,
> + CQHCI_CRYPTO_ALG_AES_ECB = 2,
> + CQHCI_CRYPTO_ALG_ESSIV_AES_CBC = 3,
> +};
> +
> +/* x-CRYPTOCAP - Crypto Capability X */
> +union cqhci_crypto_cap_entry {
> + __le32 reg_val;
> + struct {
> + u8 algorithm_id;
> + u8 sdus_mask; /* Supported data unit size mask */
> + u8 key_size;
> + u8 reserved;
> + };
> +};
> +
> +#define CQHCI_CRYPTO_CONFIGURATION_ENABLE (1 << 7)
> +#define CQHCI_CRYPTO_KEY_MAX_SIZE 64
> +/* x-CRYPTOCFG - Crypto Configuration X */
> +union cqhci_crypto_cfg_entry {
> + __le32 reg_val[32];
> + struct {
> + u8 crypto_key[CQHCI_CRYPTO_KEY_MAX_SIZE];
> + /* 4KB/512 = 8 */
> + u8 data_unit_size;
> + u8 crypto_cap_idx;
> + u8 reserved_1;
> + u8 config_enable;
> + u8 reserved_multi_host;
> + u8 reserved_2;
> + u8 vsb[2];
> + u8 reserved_3[56];
> + };
> +};
> +
> struct cqhci_host_ops;
> struct mmc_host;
> struct mmc_request;
> @@ -196,6 +269,12 @@ struct cqhci_host {
> struct completion halt_comp;
> wait_queue_head_t wait_queue;
> struct cqhci_slot *slot;
> +
> +#ifdef CONFIG_MMC_CRYPTO
> + union cqhci_crypto_capabilities crypto_capabilities;
> + union cqhci_crypto_cap_entry *crypto_cap_array;
> + u32 crypto_cfg_register;
> +#endif
> };
>
> struct cqhci_host_ops {
> --
> 2.29.2
>
The version of code from Qualcomm on AOSP reprogrammed all keyslots from
cqhci_recovery_finish(). I notice that this patch drops that - I'm guessing
that was intentional? Other than that, this patch looks good to me :).
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 4/9] mmc: cqhci: add support for inline encryption
2020-12-05 10:59 ` Satya Tangirala
@ 2020-12-05 12:33 ` Satya Tangirala
2020-12-05 18:20 ` Eric Biggers
0 siblings, 1 reply; 28+ messages in thread
From: Satya Tangirala @ 2020-12-05 12:33 UTC (permalink / raw)
To: Eric Biggers
Cc: linux-mmc, linux-arm-msm, devicetree, linux-fscrypt, Ulf Hansson,
Andy Gross, Bjorn Andersson, Adrian Hunter, Asutosh Das,
Rob Herring, Neeraj Soni, Barani Muthukumaran, Peng Zhou,
Stanley Chu, Konrad Dybcio
On Sat, Dec 05, 2020 at 10:59:27AM +0000, Satya Tangirala wrote:
> On Wed, Dec 02, 2020 at 06:05:11PM -0800, Eric Biggers wrote:
> > From: Eric Biggers <ebiggers@google.com>
> >
> > 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.
> >
> > Co-developed-by: Satya Tangirala <satyat@google.com>
> > Signed-off-by: Satya Tangirala <satyat@google.com>
> > Signed-off-by: Eric Biggers <ebiggers@google.com>
> > ---
> > drivers/mmc/host/Makefile | 1 +
> > drivers/mmc/host/cqhci-core.c | 41 +++++-
> > drivers/mmc/host/cqhci-crypto.c | 241 ++++++++++++++++++++++++++++++++
> > drivers/mmc/host/cqhci-crypto.h | 47 +++++++
> > drivers/mmc/host/cqhci.h | 81 ++++++++++-
> > 5 files changed, 408 insertions(+), 3 deletions(-)
> > create mode 100644 drivers/mmc/host/cqhci-crypto.c
> > create mode 100644 drivers/mmc/host/cqhci-crypto.h
> >
> > diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
> > index 20c2f9463d0dc..35158508ab63d 100644
> > --- a/drivers/mmc/host/Makefile
> > +++ b/drivers/mmc/host/Makefile
> > @@ -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)
> > diff --git a/drivers/mmc/host/cqhci-core.c b/drivers/mmc/host/cqhci-core.c
> > index ad7c9acff1728..93b0432bb6011 100644
> > --- a/drivers/mmc/host/cqhci-core.c
> > +++ b/drivers/mmc/host/cqhci-core.c
> > @@ -18,6 +18,7 @@
> > #include <linux/mmc/card.h>
> >
> > #include "cqhci.h"
> > +#include "cqhci-crypto.h"
> >
> > #define DCMD_SLOT 31
> > #define NUM_SLOTS 32
> > @@ -258,6 +259,9 @@ static void __cqhci_enable(struct cqhci_host *cq_host)
> > if (cq_host->caps & CQHCI_TASK_DESC_SZ_128)
> > cqcfg |= CQHCI_TASK_DESC_SZ;
> >
> > + if (mmc->caps2 & MMC_CAP2_CRYPTO)
> > + cqcfg |= CQHCI_CRYPTO_GENERAL_ENABLE;
> > +
> > cqhci_writel(cq_host, cqcfg, CQHCI_CFG);
> >
> > cqhci_writel(cq_host, lower_32_bits(cq_host->desc_dma_base),
> > @@ -430,7 +434,7 @@ static void cqhci_prep_task_desc(struct mmc_request *mrq,
> > task_desc[0] = cpu_to_le64(desc0);
> >
> > if (cq_host->caps & CQHCI_TASK_DESC_SZ_128) {
> > - u64 desc1 = 0;
> > + u64 desc1 = cqhci_crypto_prep_task_desc(mrq);
> >
> > task_desc[1] = cpu_to_le64(desc1);
> >
> > @@ -681,6 +685,7 @@ static void cqhci_error_irq(struct mmc_host *mmc, u32 status, int cmd_error,
> > struct cqhci_host *cq_host = mmc->cqe_private;
> > struct cqhci_slot *slot;
> > u32 terri;
> > + u32 tdpe;
> > int tag;
> >
> > spin_lock(&cq_host->lock);
> > @@ -719,6 +724,30 @@ static void cqhci_error_irq(struct mmc_host *mmc, u32 status, int cmd_error,
> > }
> > }
> >
> > + /*
> > + * Handle ICCE ("Invalid Crypto Configuration Error"). This should
> > + * never happen, since the block layer ensures that all crypto-enabled
> > + * I/O requests have a valid keyslot before they reach the driver.
> > + *
> > + * Note that GCE ("General Crypto Error") is different; it already got
> > + * handled above by checking TERRI.
> > + */
> > + if (status & CQHCI_IS_ICCE) {
> > + tdpe = cqhci_readl(cq_host, CQHCI_TDPE);
> > + WARN_ONCE(1,
> > + "%s: cqhci: invalid crypto configuration error. IRQ status: 0x%08x TDPE: 0x%08x\n",
> > + mmc_hostname(mmc), status, tdpe);
> > + while (tdpe != 0) {
> > + tag = __ffs(tdpe);
> > + tdpe &= ~(1 << tag);
> > + slot = &cq_host->slot[tag];
> > + if (!slot->mrq)
> > + continue;
> > + slot->flags = cqhci_error_flags(data_error, cmd_error);
> > + cqhci_recovery_needed(mmc, slot->mrq, true);
> > + }
> > + }
> > +
> > if (!cq_host->recovery_halt) {
> > /*
> > * The only way to guarantee forward progress is to mark at
> > @@ -784,7 +813,8 @@ irqreturn_t cqhci_irq(struct mmc_host *mmc, u32 intmask, int cmd_error,
> >
> > pr_debug("%s: cqhci: IRQ status: 0x%08x\n", mmc_hostname(mmc), status);
> >
> > - if ((status & CQHCI_IS_RED) || cmd_error || data_error)
> > + if ((status & (CQHCI_IS_RED | CQHCI_IS_GCE | CQHCI_IS_ICCE)) ||
> > + cmd_error || data_error)
> > cqhci_error_irq(mmc, status, cmd_error, data_error);
> >
> > if (status & CQHCI_IS_TCC) {
> > @@ -1151,6 +1181,13 @@ int cqhci_init(struct cqhci_host *cq_host, struct mmc_host *mmc,
> > goto out_err;
> > }
> >
> > + err = cqhci_crypto_init(cq_host);
> > + if (err) {
> > + pr_err("%s: CQHCI crypto initialization failed\n",
> > + mmc_hostname(mmc));
> > + goto out_err;
> > + }
> > +
> > spin_lock_init(&cq_host->lock);
> >
> > init_completion(&cq_host->halt_comp);
> > diff --git a/drivers/mmc/host/cqhci-crypto.c b/drivers/mmc/host/cqhci-crypto.c
> > new file mode 100644
> > index 0000000000000..98f141c8480ce
> > --- /dev/null
> > +++ b/drivers/mmc/host/cqhci-crypto.c
> > @@ -0,0 +1,241 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * CQHCI crypto engine (inline encryption) support
> > + *
> > + * Copyright 2020 Google LLC
> > + */
> > +
> > +#include <linux/blk-crypto.h>
> > +#include <linux/keyslot-manager.h>
> > +#include <linux/mmc/host.h>
> > +
> > +#include "cqhci-crypto.h"
> > +
> > +/* Map from blk-crypto modes to CQHCI crypto algorithm IDs and key sizes */
> > +static const struct cqhci_crypto_alg_entry {
> > + enum cqhci_crypto_alg alg;
> > + enum cqhci_crypto_key_size key_size;
> > +} cqhci_crypto_algs[BLK_ENCRYPTION_MODE_MAX] = {
> > + [BLK_ENCRYPTION_MODE_AES_256_XTS] = {
> > + .alg = CQHCI_CRYPTO_ALG_AES_XTS,
> > + .key_size = CQHCI_CRYPTO_KEY_SIZE_256,
> > + },
> > +};
> > +
> > +static inline struct cqhci_host *
> > +cqhci_host_from_ksm(struct blk_keyslot_manager *ksm)
> > +{
> > + struct mmc_host *mmc = container_of(ksm, struct mmc_host, 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)
> > +{
> > + u32 slot_offset = cq_host->crypto_cfg_register + slot * sizeof(*cfg);
> > + int i;
> > +
> > + /* Clear CFGE */
> > + cqhci_writel(cq_host, 0, slot_offset + 16 * sizeof(cfg->reg_val[0]));
> > +
> > + /* Write the key */
> > + for (i = 0; i < 16; i++) {
> > + cqhci_writel(cq_host, le32_to_cpu(cfg->reg_val[i]),
> > + slot_offset + i * sizeof(cfg->reg_val[0]));
> > + }
> > + /* Write dword 17 */
> > + cqhci_writel(cq_host, le32_to_cpu(cfg->reg_val[17]),
> > + slot_offset + 17 * sizeof(cfg->reg_val[0]));
> > + /* 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]));
> > +}
> > +
> > +static int cqhci_crypto_keyslot_program(struct blk_keyslot_manager *ksm,
> > + const struct blk_crypto_key *key,
> > + unsigned int slot)
> > +
> > +{
> > + struct cqhci_host *cq_host = cqhci_host_from_ksm(ksm);
> > + const union cqhci_crypto_cap_entry *ccap_array =
> > + cq_host->crypto_cap_array;
> > + const struct cqhci_crypto_alg_entry *alg =
> > + &cqhci_crypto_algs[key->crypto_cfg.crypto_mode];
> > + u8 data_unit_mask = key->crypto_cfg.data_unit_size / 512;
> > + int i;
> > + int cap_idx = -1;
> > + union cqhci_crypto_cfg_entry cfg = {};
> > +
> > + BUILD_BUG_ON(CQHCI_CRYPTO_KEY_SIZE_INVALID != 0);
> > + for (i = 0; i < cq_host->crypto_capabilities.num_crypto_cap; i++) {
> > + if (ccap_array[i].algorithm_id == alg->alg &&
> > + ccap_array[i].key_size == alg->key_size &&
> > + (ccap_array[i].sdus_mask & data_unit_mask)) {
> > + cap_idx = i;
> > + break;
> > + }
> > + }
> > + if (WARN_ON(cap_idx < 0))
> > + return -EOPNOTSUPP;
> > +
> > + cfg.data_unit_size = data_unit_mask;
> > + cfg.crypto_cap_idx = cap_idx;
> > + cfg.config_enable = CQHCI_CRYPTO_CONFIGURATION_ENABLE;
> > +
> > + if (ccap_array[cap_idx].algorithm_id == CQHCI_CRYPTO_ALG_AES_XTS) {
> > + /* In XTS mode, the blk_crypto_key's size is already doubled */
> > + memcpy(cfg.crypto_key, key->raw, key->size/2);
> > + memcpy(cfg.crypto_key + CQHCI_CRYPTO_KEY_MAX_SIZE/2,
> > + key->raw + key->size/2, key->size/2);
> > + } else {
> > + memcpy(cfg.crypto_key, key->raw, key->size);
> > + }
> > +
> > + cqhci_crypto_program_key(cq_host, &cfg, slot);
> > +
> > + memzero_explicit(&cfg, sizeof(cfg));
> > + return 0;
> > +}
> > +
> > +static void cqhci_crypto_clear_keyslot(struct cqhci_host *cq_host, int slot)
> > +{
> > + /*
> > + * Clear the crypto cfg on the device. Clearing CFGE
> > + * might not be sufficient, so just clear the entire cfg.
> > + */
> > + union cqhci_crypto_cfg_entry cfg = {};
> > +
> > + cqhci_crypto_program_key(cq_host, &cfg, slot);
> > +}
> > +
> > +static int cqhci_crypto_keyslot_evict(struct blk_keyslot_manager *ksm,
> > + const struct blk_crypto_key *key,
> > + unsigned int slot)
> > +{
> > + struct cqhci_host *cq_host = cqhci_host_from_ksm(ksm);
> > +
> > + cqhci_crypto_clear_keyslot(cq_host, slot);
> > + return 0;
> > +}
> > +
> > +/*
> > + * The keyslot management operations for CQHCI crypto.
> > + *
> > + * Note that the block layer ensures that these are never called while the host
> > + * controller is runtime-suspended. However, the CQE won't necessarily be
> > + * "enabled" when these are called, i.e. CQHCI_ENABLE might not be set in the
> > + * CQHCI_CFG register. But the hardware allows that.
> > + */
> > +static const struct blk_ksm_ll_ops cqhci_ksm_ops = {
> > + .keyslot_program = cqhci_crypto_keyslot_program,
> > + .keyslot_evict = cqhci_crypto_keyslot_evict,
> > +};
> > +
> > +static enum blk_crypto_mode_num
> > +cqhci_find_blk_crypto_mode(union cqhci_crypto_cap_entry cap)
> > +{
> > + int i;
> > +
> > + for (i = 0; i < ARRAY_SIZE(cqhci_crypto_algs); i++) {
> > + BUILD_BUG_ON(CQHCI_CRYPTO_KEY_SIZE_INVALID != 0);
> > + if (cqhci_crypto_algs[i].alg == cap.algorithm_id &&
> > + cqhci_crypto_algs[i].key_size == cap.key_size)
> > + return i;
> > + }
> > + return BLK_ENCRYPTION_MODE_INVALID;
> > +}
> > +
> > +/**
> > + * 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 = blk_ksm_init(ksm, num_keyslots);
> > + if (err)
> > + goto out_free_caps;
> > +
> > + 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_free_caps:
> > + devm_kfree(dev, cq_host->crypto_cap_array);
> > + cq_host->crypto_cap_array = NULL;
> > +out:
> > + mmc->caps2 &= ~MMC_CAP2_CRYPTO;
> > + return err;
> > +}
> > diff --git a/drivers/mmc/host/cqhci-crypto.h b/drivers/mmc/host/cqhci-crypto.h
> > new file mode 100644
> > index 0000000000000..60b58ee0e6256
> > --- /dev/null
> > +++ b/drivers/mmc/host/cqhci-crypto.h
> > @@ -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;
> > +
> > + 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;
> > +}
> > +
> > +static inline u64 cqhci_crypto_prep_task_desc(struct mmc_request *mrq)
> > +{
> > + return 0;
> > +}
> > +
> > +#endif /* !CONFIG_MMC_CRYPTO */
> > +
> > +#endif /* LINUX_MMC_CQHCI_CRYPTO_H */
> > diff --git a/drivers/mmc/host/cqhci.h b/drivers/mmc/host/cqhci.h
> > index 89bf6adbce8ca..5c18734624fea 100644
> > --- a/drivers/mmc/host/cqhci.h
> > +++ b/drivers/mmc/host/cqhci.h
> > @@ -22,10 +22,13 @@
> >
> > /* capabilities */
> > #define CQHCI_CAP 0x04
> > +#define CQHCI_CAP_CS 0x10000000 /* Crypto Support */
> > +
> > /* configuration */
> > #define CQHCI_CFG 0x08
> > #define CQHCI_DCMD 0x00001000
> > #define CQHCI_TASK_DESC_SZ 0x00000100
> > +#define CQHCI_CRYPTO_GENERAL_ENABLE 0x00000002
> > #define CQHCI_ENABLE 0x00000001
> >
> > /* control */
> > @@ -39,8 +42,11 @@
> > #define CQHCI_IS_TCC BIT(1)
> > #define CQHCI_IS_RED BIT(2)
> > #define CQHCI_IS_TCL BIT(3)
> > +#define CQHCI_IS_GCE BIT(4) /* General Crypto Error */
> > +#define CQHCI_IS_ICCE BIT(5) /* Invalid Crypto Config Error */
> >
> > -#define CQHCI_IS_MASK (CQHCI_IS_TCC | CQHCI_IS_RED)
> > +#define CQHCI_IS_MASK (CQHCI_IS_TCC | CQHCI_IS_RED | \
> > + CQHCI_IS_GCE | CQHCI_IS_ICCE)
> >
> > /* interrupt status enable */
> > #define CQHCI_ISTE 0x14
> > @@ -78,6 +84,9 @@
> > /* task clear */
> > #define CQHCI_TCLR 0x38
> >
> > +/* task descriptor processing error */
> > +#define CQHCI_TDPE 0x3c
> > +
> > /* send status config 1 */
> > #define CQHCI_SSC1 0x40
> > #define CQHCI_SSC1_CBC_MASK GENMASK(19, 16)
> > @@ -107,6 +116,10 @@
> > /* command response argument */
> > #define CQHCI_CRA 0x5C
> >
> > +/* crypto capabilities */
> > +#define CQHCI_CCAP 0x100
> > +#define CQHCI_CRYPTOCAP 0x104
> > +
> > #define CQHCI_INT_ALL 0xF
> > #define CQHCI_IC_DEFAULT_ICCTH 31
> > #define CQHCI_IC_DEFAULT_ICTOVAL 1
> > @@ -133,11 +146,71 @@
> > #define CQHCI_CMD_TIMING(x) (((x) & 1) << 22)
> > #define CQHCI_RESP_TYPE(x) (((x) & 0x3) << 23)
> >
> > +/* crypto task descriptor fields (for bits 64-127 of task descriptor) */
> > +#define CQHCI_CRYPTO_ENABLE_BIT (1ULL << 47)
> > +#define CQHCI_CRYPTO_KEYSLOT(x) ((u64)(x) << 32)
> > +
> > /* transfer descriptor fields */
> > #define CQHCI_DAT_LENGTH(x) (((x) & 0xFFFF) << 16)
> > #define CQHCI_DAT_ADDR_LO(x) (((x) & 0xFFFFFFFF) << 32)
> > #define CQHCI_DAT_ADDR_HI(x) (((x) & 0xFFFFFFFF) << 0)
> >
> > +/* CCAP - Crypto Capability 100h */
> > +union cqhci_crypto_capabilities {
> > + __le32 reg_val;
> > + struct {
> > + u8 num_crypto_cap;
> > + u8 config_count;
> > + u8 reserved;
> > + u8 config_array_ptr;
> > + };
> > +};
> > +
> > +enum cqhci_crypto_key_size {
> > + CQHCI_CRYPTO_KEY_SIZE_INVALID = 0,
> > + CQHCI_CRYPTO_KEY_SIZE_128 = 1,
> > + CQHCI_CRYPTO_KEY_SIZE_192 = 2,
> > + CQHCI_CRYPTO_KEY_SIZE_256 = 3,
> > + CQHCI_CRYPTO_KEY_SIZE_512 = 4,
> > +};
> > +
> > +enum cqhci_crypto_alg {
> > + CQHCI_CRYPTO_ALG_AES_XTS = 0,
> > + CQHCI_CRYPTO_ALG_BITLOCKER_AES_CBC = 1,
> > + CQHCI_CRYPTO_ALG_AES_ECB = 2,
> > + CQHCI_CRYPTO_ALG_ESSIV_AES_CBC = 3,
> > +};
> > +
> > +/* x-CRYPTOCAP - Crypto Capability X */
> > +union cqhci_crypto_cap_entry {
> > + __le32 reg_val;
> > + struct {
> > + u8 algorithm_id;
> > + u8 sdus_mask; /* Supported data unit size mask */
> > + u8 key_size;
> > + u8 reserved;
> > + };
> > +};
> > +
> > +#define CQHCI_CRYPTO_CONFIGURATION_ENABLE (1 << 7)
> > +#define CQHCI_CRYPTO_KEY_MAX_SIZE 64
> > +/* x-CRYPTOCFG - Crypto Configuration X */
> > +union cqhci_crypto_cfg_entry {
> > + __le32 reg_val[32];
> > + struct {
> > + u8 crypto_key[CQHCI_CRYPTO_KEY_MAX_SIZE];
> > + /* 4KB/512 = 8 */
> > + u8 data_unit_size;
> > + u8 crypto_cap_idx;
> > + u8 reserved_1;
> > + u8 config_enable;
> > + u8 reserved_multi_host;
> > + u8 reserved_2;
> > + u8 vsb[2];
> > + u8 reserved_3[56];
> > + };
> > +};
> > +
> > struct cqhci_host_ops;
> > struct mmc_host;
> > struct mmc_request;
> > @@ -196,6 +269,12 @@ struct cqhci_host {
> > struct completion halt_comp;
> > wait_queue_head_t wait_queue;
> > struct cqhci_slot *slot;
> > +
> > +#ifdef CONFIG_MMC_CRYPTO
> > + union cqhci_crypto_capabilities crypto_capabilities;
> > + union cqhci_crypto_cap_entry *crypto_cap_array;
> > + u32 crypto_cfg_register;
> > +#endif
> > };
> >
> > struct cqhci_host_ops {
> > --
> > 2.29.2
> >
> The version of code from Qualcomm on AOSP reprogrammed all keyslots from
> cqhci_recovery_finish(). I notice that this patch drops that - I'm guessing
> that was intentional? Other than that, this patch looks good to me :).
For reference, the code I'm referring to is at
https://android-review.googlesource.com/c/kernel/common/+/1269702/9
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 4/9] mmc: cqhci: add support for inline encryption
2020-12-05 12:33 ` Satya Tangirala
@ 2020-12-05 18:20 ` Eric Biggers
0 siblings, 0 replies; 28+ messages in thread
From: Eric Biggers @ 2020-12-05 18:20 UTC (permalink / raw)
To: Satya Tangirala
Cc: linux-mmc, linux-arm-msm, devicetree, linux-fscrypt, Ulf Hansson,
Andy Gross, Bjorn Andersson, Adrian Hunter, Asutosh Das,
Rob Herring, Neeraj Soni, Barani Muthukumaran, Peng Zhou,
Stanley Chu, Konrad Dybcio
On Sat, Dec 05, 2020 at 12:33:40PM +0000, Satya Tangirala wrote:
> > The version of code from Qualcomm on AOSP reprogrammed all keyslots from
> > cqhci_recovery_finish(). I notice that this patch drops that - I'm guessing
> > that was intentional? Other than that, this patch looks good to me :).
> For reference, the code I'm referring to is at
> https://android-review.googlesource.com/c/kernel/common/+/1269702/9
Yes, it's intentional. Reprogramming the keys should only be needed when the
hardware is reset, which patch 1 handles via mmc_set_initial_state(). The CQE
recovery procedure normally just "stops" the hardware, not reset it. So
cqhci_recovery_finish() doesn't seem like the right place to reprogram the keys.
- Eric
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 4/9] mmc: cqhci: add support for inline encryption
2020-12-03 2:05 ` [PATCH v2 4/9] mmc: cqhci: add support for inline encryption Eric Biggers
2020-12-03 6:47 ` Adrian Hunter
2020-12-05 10:59 ` Satya Tangirala
@ 2020-12-05 12:28 ` Satya Tangirala
2020-12-05 18:07 ` Eric Biggers
2 siblings, 1 reply; 28+ messages in thread
From: Satya Tangirala @ 2020-12-05 12:28 UTC (permalink / raw)
To: Eric Biggers
Cc: linux-mmc, linux-arm-msm, devicetree, linux-fscrypt, Ulf Hansson,
Andy Gross, Bjorn Andersson, Adrian Hunter, Asutosh Das,
Rob Herring, Neeraj Soni, Barani Muthukumaran, Peng Zhou,
Stanley Chu, Konrad Dybcio
On Wed, Dec 02, 2020 at 06:05:11PM -0800, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
>
> 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.
>
> Co-developed-by: Satya Tangirala <satyat@google.com>
> Signed-off-by: Satya Tangirala <satyat@google.com>
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
> drivers/mmc/host/Makefile | 1 +
> drivers/mmc/host/cqhci-core.c | 41 +++++-
> drivers/mmc/host/cqhci-crypto.c | 241 ++++++++++++++++++++++++++++++++
> drivers/mmc/host/cqhci-crypto.h | 47 +++++++
> drivers/mmc/host/cqhci.h | 81 ++++++++++-
> 5 files changed, 408 insertions(+), 3 deletions(-)
> create mode 100644 drivers/mmc/host/cqhci-crypto.c
> create mode 100644 drivers/mmc/host/cqhci-crypto.h
>
> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
> index 20c2f9463d0dc..35158508ab63d 100644
> --- a/drivers/mmc/host/Makefile
> +++ b/drivers/mmc/host/Makefile
> @@ -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)
> diff --git a/drivers/mmc/host/cqhci-core.c b/drivers/mmc/host/cqhci-core.c
> index ad7c9acff1728..93b0432bb6011 100644
> --- a/drivers/mmc/host/cqhci-core.c
> +++ b/drivers/mmc/host/cqhci-core.c
> @@ -18,6 +18,7 @@
> #include <linux/mmc/card.h>
>
> #include "cqhci.h"
> +#include "cqhci-crypto.h"
>
> #define DCMD_SLOT 31
> #define NUM_SLOTS 32
> @@ -258,6 +259,9 @@ static void __cqhci_enable(struct cqhci_host *cq_host)
> if (cq_host->caps & CQHCI_TASK_DESC_SZ_128)
> cqcfg |= CQHCI_TASK_DESC_SZ;
>
> + if (mmc->caps2 & MMC_CAP2_CRYPTO)
> + cqcfg |= CQHCI_CRYPTO_GENERAL_ENABLE;
> +
> cqhci_writel(cq_host, cqcfg, CQHCI_CFG);
>
> cqhci_writel(cq_host, lower_32_bits(cq_host->desc_dma_base),
> @@ -430,7 +434,7 @@ static void cqhci_prep_task_desc(struct mmc_request *mrq,
> task_desc[0] = cpu_to_le64(desc0);
>
> if (cq_host->caps & CQHCI_TASK_DESC_SZ_128) {
> - u64 desc1 = 0;
> + u64 desc1 = cqhci_crypto_prep_task_desc(mrq);
>
> task_desc[1] = cpu_to_le64(desc1);
>
> @@ -681,6 +685,7 @@ static void cqhci_error_irq(struct mmc_host *mmc, u32 status, int cmd_error,
> struct cqhci_host *cq_host = mmc->cqe_private;
> struct cqhci_slot *slot;
> u32 terri;
> + u32 tdpe;
> int tag;
>
> spin_lock(&cq_host->lock);
> @@ -719,6 +724,30 @@ static void cqhci_error_irq(struct mmc_host *mmc, u32 status, int cmd_error,
> }
> }
>
> + /*
> + * Handle ICCE ("Invalid Crypto Configuration Error"). This should
> + * never happen, since the block layer ensures that all crypto-enabled
> + * I/O requests have a valid keyslot before they reach the driver.
> + *
> + * Note that GCE ("General Crypto Error") is different; it already got
> + * handled above by checking TERRI.
> + */
> + if (status & CQHCI_IS_ICCE) {
> + tdpe = cqhci_readl(cq_host, CQHCI_TDPE);
> + WARN_ONCE(1,
> + "%s: cqhci: invalid crypto configuration error. IRQ status: 0x%08x TDPE: 0x%08x\n",
> + mmc_hostname(mmc), status, tdpe);
> + while (tdpe != 0) {
> + tag = __ffs(tdpe);
> + tdpe &= ~(1 << tag);
> + slot = &cq_host->slot[tag];
> + if (!slot->mrq)
> + continue;
> + slot->flags = cqhci_error_flags(data_error, cmd_error);
> + cqhci_recovery_needed(mmc, slot->mrq, true);
> + }
> + }
> +
> if (!cq_host->recovery_halt) {
> /*
> * The only way to guarantee forward progress is to mark at
> @@ -784,7 +813,8 @@ irqreturn_t cqhci_irq(struct mmc_host *mmc, u32 intmask, int cmd_error,
>
> pr_debug("%s: cqhci: IRQ status: 0x%08x\n", mmc_hostname(mmc), status);
>
> - if ((status & CQHCI_IS_RED) || cmd_error || data_error)
> + if ((status & (CQHCI_IS_RED | CQHCI_IS_GCE | CQHCI_IS_ICCE)) ||
> + cmd_error || data_error)
> cqhci_error_irq(mmc, status, cmd_error, data_error);
>
> if (status & CQHCI_IS_TCC) {
> @@ -1151,6 +1181,13 @@ int cqhci_init(struct cqhci_host *cq_host, struct mmc_host *mmc,
> goto out_err;
> }
>
> + err = cqhci_crypto_init(cq_host);
> + if (err) {
> + pr_err("%s: CQHCI crypto initialization failed\n",
> + mmc_hostname(mmc));
> + goto out_err;
> + }
> +
> spin_lock_init(&cq_host->lock);
>
> init_completion(&cq_host->halt_comp);
> diff --git a/drivers/mmc/host/cqhci-crypto.c b/drivers/mmc/host/cqhci-crypto.c
> new file mode 100644
> index 0000000000000..98f141c8480ce
> --- /dev/null
> +++ b/drivers/mmc/host/cqhci-crypto.c
> @@ -0,0 +1,241 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * CQHCI crypto engine (inline encryption) support
> + *
> + * Copyright 2020 Google LLC
> + */
> +
> +#include <linux/blk-crypto.h>
> +#include <linux/keyslot-manager.h>
> +#include <linux/mmc/host.h>
> +
> +#include "cqhci-crypto.h"
> +
> +/* Map from blk-crypto modes to CQHCI crypto algorithm IDs and key sizes */
> +static const struct cqhci_crypto_alg_entry {
> + enum cqhci_crypto_alg alg;
> + enum cqhci_crypto_key_size key_size;
> +} cqhci_crypto_algs[BLK_ENCRYPTION_MODE_MAX] = {
> + [BLK_ENCRYPTION_MODE_AES_256_XTS] = {
> + .alg = CQHCI_CRYPTO_ALG_AES_XTS,
> + .key_size = CQHCI_CRYPTO_KEY_SIZE_256,
> + },
> +};
> +
> +static inline struct cqhci_host *
> +cqhci_host_from_ksm(struct blk_keyslot_manager *ksm)
> +{
> + struct mmc_host *mmc = container_of(ksm, struct mmc_host, 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)
> +{
> + u32 slot_offset = cq_host->crypto_cfg_register + slot * sizeof(*cfg);
> + int i;
> +
> + /* Clear CFGE */
> + cqhci_writel(cq_host, 0, slot_offset + 16 * sizeof(cfg->reg_val[0]));
> +
> + /* Write the key */
> + for (i = 0; i < 16; i++) {
> + cqhci_writel(cq_host, le32_to_cpu(cfg->reg_val[i]),
> + slot_offset + i * sizeof(cfg->reg_val[0]));
> + }
> + /* Write dword 17 */
> + cqhci_writel(cq_host, le32_to_cpu(cfg->reg_val[17]),
> + slot_offset + 17 * sizeof(cfg->reg_val[0]));
> + /* 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]));
> +}
> +
> +static int cqhci_crypto_keyslot_program(struct blk_keyslot_manager *ksm,
> + const struct blk_crypto_key *key,
> + unsigned int slot)
> +
> +{
> + struct cqhci_host *cq_host = cqhci_host_from_ksm(ksm);
> + const union cqhci_crypto_cap_entry *ccap_array =
> + cq_host->crypto_cap_array;
> + const struct cqhci_crypto_alg_entry *alg =
> + &cqhci_crypto_algs[key->crypto_cfg.crypto_mode];
> + u8 data_unit_mask = key->crypto_cfg.data_unit_size / 512;
> + int i;
> + int cap_idx = -1;
> + union cqhci_crypto_cfg_entry cfg = {};
> +
> + BUILD_BUG_ON(CQHCI_CRYPTO_KEY_SIZE_INVALID != 0);
> + for (i = 0; i < cq_host->crypto_capabilities.num_crypto_cap; i++) {
> + if (ccap_array[i].algorithm_id == alg->alg &&
> + ccap_array[i].key_size == alg->key_size &&
> + (ccap_array[i].sdus_mask & data_unit_mask)) {
> + cap_idx = i;
> + break;
> + }
> + }
> + if (WARN_ON(cap_idx < 0))
> + return -EOPNOTSUPP;
> +
> + cfg.data_unit_size = data_unit_mask;
> + cfg.crypto_cap_idx = cap_idx;
> + cfg.config_enable = CQHCI_CRYPTO_CONFIGURATION_ENABLE;
> +
> + if (ccap_array[cap_idx].algorithm_id == CQHCI_CRYPTO_ALG_AES_XTS) {
> + /* In XTS mode, the blk_crypto_key's size is already doubled */
> + memcpy(cfg.crypto_key, key->raw, key->size/2);
> + memcpy(cfg.crypto_key + CQHCI_CRYPTO_KEY_MAX_SIZE/2,
> + key->raw + key->size/2, key->size/2);
> + } else {
> + memcpy(cfg.crypto_key, key->raw, key->size);
> + }
> +
> + cqhci_crypto_program_key(cq_host, &cfg, slot);
> +
> + memzero_explicit(&cfg, sizeof(cfg));
> + return 0;
> +}
> +
> +static void cqhci_crypto_clear_keyslot(struct cqhci_host *cq_host, int slot)
> +{
> + /*
> + * Clear the crypto cfg on the device. Clearing CFGE
> + * might not be sufficient, so just clear the entire cfg.
> + */
> + union cqhci_crypto_cfg_entry cfg = {};
> +
> + cqhci_crypto_program_key(cq_host, &cfg, slot);
> +}
> +
> +static int cqhci_crypto_keyslot_evict(struct blk_keyslot_manager *ksm,
> + const struct blk_crypto_key *key,
> + unsigned int slot)
> +{
> + struct cqhci_host *cq_host = cqhci_host_from_ksm(ksm);
> +
> + cqhci_crypto_clear_keyslot(cq_host, slot);
> + return 0;
> +}
> +
> +/*
> + * The keyslot management operations for CQHCI crypto.
> + *
> + * Note that the block layer ensures that these are never called while the host
> + * controller is runtime-suspended. However, the CQE won't necessarily be
> + * "enabled" when these are called, i.e. CQHCI_ENABLE might not be set in the
> + * CQHCI_CFG register. But the hardware allows that.
> + */
> +static const struct blk_ksm_ll_ops cqhci_ksm_ops = {
> + .keyslot_program = cqhci_crypto_keyslot_program,
> + .keyslot_evict = cqhci_crypto_keyslot_evict,
> +};
> +
> +static enum blk_crypto_mode_num
> +cqhci_find_blk_crypto_mode(union cqhci_crypto_cap_entry cap)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(cqhci_crypto_algs); i++) {
> + BUILD_BUG_ON(CQHCI_CRYPTO_KEY_SIZE_INVALID != 0);
> + if (cqhci_crypto_algs[i].alg == cap.algorithm_id &&
> + cqhci_crypto_algs[i].key_size == cap.key_size)
> + return i;
> + }
> + return BLK_ENCRYPTION_MODE_INVALID;
> +}
> +
> +/**
> + * 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 = blk_ksm_init(ksm, num_keyslots);
> + if (err)
> + goto out_free_caps;
> +
> + 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_free_caps:
> + devm_kfree(dev, cq_host->crypto_cap_array);
> + cq_host->crypto_cap_array = NULL;
> +out:
> + mmc->caps2 &= ~MMC_CAP2_CRYPTO;
> + return err;
> +}
> diff --git a/drivers/mmc/host/cqhci-crypto.h b/drivers/mmc/host/cqhci-crypto.h
> new file mode 100644
> index 0000000000000..60b58ee0e6256
> --- /dev/null
> +++ b/drivers/mmc/host/cqhci-crypto.h
> @@ -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;
> +
> + 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;
> +}
> +
> +static inline u64 cqhci_crypto_prep_task_desc(struct mmc_request *mrq)
> +{
> + return 0;
> +}
> +
> +#endif /* !CONFIG_MMC_CRYPTO */
> +
> +#endif /* LINUX_MMC_CQHCI_CRYPTO_H */
> diff --git a/drivers/mmc/host/cqhci.h b/drivers/mmc/host/cqhci.h
> index 89bf6adbce8ca..5c18734624fea 100644
> --- a/drivers/mmc/host/cqhci.h
> +++ b/drivers/mmc/host/cqhci.h
> @@ -22,10 +22,13 @@
>
> /* capabilities */
> #define CQHCI_CAP 0x04
> +#define CQHCI_CAP_CS 0x10000000 /* Crypto Support */
> +
> /* configuration */
> #define CQHCI_CFG 0x08
> #define CQHCI_DCMD 0x00001000
> #define CQHCI_TASK_DESC_SZ 0x00000100
> +#define CQHCI_CRYPTO_GENERAL_ENABLE 0x00000002
> #define CQHCI_ENABLE 0x00000001
>
> /* control */
> @@ -39,8 +42,11 @@
> #define CQHCI_IS_TCC BIT(1)
> #define CQHCI_IS_RED BIT(2)
> #define CQHCI_IS_TCL BIT(3)
> +#define CQHCI_IS_GCE BIT(4) /* General Crypto Error */
> +#define CQHCI_IS_ICCE BIT(5) /* Invalid Crypto Config Error */
>
> -#define CQHCI_IS_MASK (CQHCI_IS_TCC | CQHCI_IS_RED)
> +#define CQHCI_IS_MASK (CQHCI_IS_TCC | CQHCI_IS_RED | \
> + CQHCI_IS_GCE | CQHCI_IS_ICCE)
>
> /* interrupt status enable */
> #define CQHCI_ISTE 0x14
> @@ -78,6 +84,9 @@
> /* task clear */
> #define CQHCI_TCLR 0x38
>
> +/* task descriptor processing error */
> +#define CQHCI_TDPE 0x3c
> +
> /* send status config 1 */
> #define CQHCI_SSC1 0x40
> #define CQHCI_SSC1_CBC_MASK GENMASK(19, 16)
> @@ -107,6 +116,10 @@
> /* command response argument */
> #define CQHCI_CRA 0x5C
>
> +/* crypto capabilities */
> +#define CQHCI_CCAP 0x100
> +#define CQHCI_CRYPTOCAP 0x104
> +
> #define CQHCI_INT_ALL 0xF
> #define CQHCI_IC_DEFAULT_ICCTH 31
> #define CQHCI_IC_DEFAULT_ICTOVAL 1
> @@ -133,11 +146,71 @@
> #define CQHCI_CMD_TIMING(x) (((x) & 1) << 22)
> #define CQHCI_RESP_TYPE(x) (((x) & 0x3) << 23)
>
> +/* crypto task descriptor fields (for bits 64-127 of task descriptor) */
> +#define CQHCI_CRYPTO_ENABLE_BIT (1ULL << 47)
> +#define CQHCI_CRYPTO_KEYSLOT(x) ((u64)(x) << 32)
> +
> /* transfer descriptor fields */
> #define CQHCI_DAT_LENGTH(x) (((x) & 0xFFFF) << 16)
> #define CQHCI_DAT_ADDR_LO(x) (((x) & 0xFFFFFFFF) << 32)
> #define CQHCI_DAT_ADDR_HI(x) (((x) & 0xFFFFFFFF) << 0)
>
> +/* CCAP - Crypto Capability 100h */
> +union cqhci_crypto_capabilities {
> + __le32 reg_val;
> + struct {
> + u8 num_crypto_cap;
> + u8 config_count;
> + u8 reserved;
> + u8 config_array_ptr;
> + };
> +};
> +
> +enum cqhci_crypto_key_size {
> + CQHCI_CRYPTO_KEY_SIZE_INVALID = 0,
> + CQHCI_CRYPTO_KEY_SIZE_128 = 1,
> + CQHCI_CRYPTO_KEY_SIZE_192 = 2,
> + CQHCI_CRYPTO_KEY_SIZE_256 = 3,
> + CQHCI_CRYPTO_KEY_SIZE_512 = 4,
> +};
> +
> +enum cqhci_crypto_alg {
> + CQHCI_CRYPTO_ALG_AES_XTS = 0,
> + CQHCI_CRYPTO_ALG_BITLOCKER_AES_CBC = 1,
> + CQHCI_CRYPTO_ALG_AES_ECB = 2,
> + CQHCI_CRYPTO_ALG_ESSIV_AES_CBC = 3,
> +};
> +
> +/* x-CRYPTOCAP - Crypto Capability X */
> +union cqhci_crypto_cap_entry {
> + __le32 reg_val;
> + struct {
> + u8 algorithm_id;
> + u8 sdus_mask; /* Supported data unit size mask */
> + u8 key_size;
> + u8 reserved;
> + };
> +};
> +
> +#define CQHCI_CRYPTO_CONFIGURATION_ENABLE (1 << 7)
> +#define CQHCI_CRYPTO_KEY_MAX_SIZE 64
> +/* x-CRYPTOCFG - Crypto Configuration X */
> +union cqhci_crypto_cfg_entry {
> + __le32 reg_val[32];
> + struct {
> + u8 crypto_key[CQHCI_CRYPTO_KEY_MAX_SIZE];
> + /* 4KB/512 = 8 */
I'm not sure what this comment is for (admittedly, it seems I introduced
this line into AOSP, and that seems to have made it here haha) - I think
we should just remove it.
> + u8 data_unit_size;
> + u8 crypto_cap_idx;
> + u8 reserved_1;
> + u8 config_enable;
> + u8 reserved_multi_host;
> + u8 reserved_2;
> + u8 vsb[2];
> + u8 reserved_3[56];
> + };
> +};
> +
> struct cqhci_host_ops;
> struct mmc_host;
> struct mmc_request;
> @@ -196,6 +269,12 @@ struct cqhci_host {
> struct completion halt_comp;
> wait_queue_head_t wait_queue;
> struct cqhci_slot *slot;
> +
> +#ifdef CONFIG_MMC_CRYPTO
> + union cqhci_crypto_capabilities crypto_capabilities;
> + union cqhci_crypto_cap_entry *crypto_cap_array;
> + u32 crypto_cfg_register;
> +#endif
> };
>
> struct cqhci_host_ops {
> --
> 2.29.2
>
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 4/9] mmc: cqhci: add support for inline encryption
2020-12-05 12:28 ` Satya Tangirala
@ 2020-12-05 18:07 ` Eric Biggers
2020-12-09 0:01 ` Satya Tangirala
0 siblings, 1 reply; 28+ messages in thread
From: Eric Biggers @ 2020-12-05 18:07 UTC (permalink / raw)
To: Satya Tangirala
Cc: linux-mmc, linux-arm-msm, devicetree, linux-fscrypt, Ulf Hansson,
Andy Gross, Bjorn Andersson, Adrian Hunter, Asutosh Das,
Rob Herring, Neeraj Soni, Barani Muthukumaran, Peng Zhou,
Stanley Chu, Konrad Dybcio
On Sat, Dec 05, 2020 at 12:28:08PM +0000, Satya Tangirala wrote:
> > +#define CQHCI_CRYPTO_CONFIGURATION_ENABLE (1 << 7)
> > +#define CQHCI_CRYPTO_KEY_MAX_SIZE 64
> > +/* x-CRYPTOCFG - Crypto Configuration X */
> > +union cqhci_crypto_cfg_entry {
> > + __le32 reg_val[32];
> > + struct {
> > + u8 crypto_key[CQHCI_CRYPTO_KEY_MAX_SIZE];
> > + /* 4KB/512 = 8 */
> I'm not sure what this comment is for (admittedly, it seems I introduced
> this line into AOSP, and that seems to have made it here haha) - I think
> we should just remove it.
> > + u8 data_unit_size;
> > + u8 crypto_cap_idx;
> > + u8 reserved_1;
> > + u8 config_enable;
> > + u8 reserved_multi_host;
> > + u8 reserved_2;
> > + u8 vsb[2];
> > + u8 reserved_3[56];
> > + };
> > +};
(Please quote just the part that you're actually replying to -- thanks!)
The comment gives the typical value that is stored in data_unit_size,
but yeah it's a bad comment. I'll just remove it.
- Eric
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 4/9] mmc: cqhci: add support for inline encryption
2020-12-05 18:07 ` Eric Biggers
@ 2020-12-09 0:01 ` Satya Tangirala
0 siblings, 0 replies; 28+ messages in thread
From: Satya Tangirala @ 2020-12-09 0:01 UTC (permalink / raw)
To: Eric Biggers
Cc: linux-mmc, linux-arm-msm, devicetree, linux-fscrypt, Ulf Hansson,
Andy Gross, Bjorn Andersson, Adrian Hunter, Asutosh Das,
Rob Herring, Neeraj Soni, Barani Muthukumaran, Peng Zhou,
Stanley Chu, Konrad Dybcio
On Sat, Dec 05, 2020 at 10:07:32AM -0800, Eric Biggers wrote:
> (Please quote just the part that you're actually replying to -- thanks!)
>
Sorry about that. Will do so in future :)
> The comment gives the typical value that is stored in data_unit_size,
> but yeah it's a bad comment. I'll just remove it.
>
Cool. Please feel free to add
Reviewed-by: Satya Tangirala <satyat@google.com>
> - Eric
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 5/9] mmc: cqhci: add cqhci_host_ops::program_key
2020-12-03 2:05 [PATCH v2 0/9] eMMC inline encryption support Eric Biggers
` (3 preceding siblings ...)
2020-12-03 2:05 ` [PATCH v2 4/9] mmc: cqhci: add support for inline encryption Eric Biggers
@ 2020-12-03 2:05 ` Eric Biggers
2020-12-08 23:48 ` Satya Tangirala
2020-12-03 2:05 ` [PATCH v2 6/9] firmware: qcom_scm: update comment for ICE-related functions Eric Biggers
` (3 subsequent siblings)
8 siblings, 1 reply; 28+ messages in thread
From: Eric Biggers @ 2020-12-03 2:05 UTC (permalink / raw)
To: linux-mmc
Cc: linux-arm-msm, devicetree, linux-fscrypt, Satya Tangirala,
Ulf Hansson, Andy Gross, Bjorn Andersson, Adrian Hunter,
Asutosh Das, Rob Herring, Neeraj Soni, Barani Muthukumaran,
Peng Zhou, Stanley Chu, Konrad Dybcio
From: Eric Biggers <ebiggers@google.com>
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").
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
drivers/mmc/host/cqhci-crypto.c | 22 +++++++++++++---------
drivers/mmc/host/cqhci.h | 4 ++++
2 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/drivers/mmc/host/cqhci-crypto.c b/drivers/mmc/host/cqhci-crypto.c
index 98f141c8480ce..0aaa948d240b1 100644
--- a/drivers/mmc/host/cqhci-crypto.c
+++ b/drivers/mmc/host/cqhci-crypto.c
@@ -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);
}
/*
diff --git a/drivers/mmc/host/cqhci.h b/drivers/mmc/host/cqhci.h
index 5c18734624fea..ece997dd8bcc7 100644
--- a/drivers/mmc/host/cqhci.h
+++ b/drivers/mmc/host/cqhci.h
@@ -287,6 +287,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)
--
2.29.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v2 5/9] mmc: cqhci: add cqhci_host_ops::program_key
2020-12-03 2:05 ` [PATCH v2 5/9] mmc: cqhci: add cqhci_host_ops::program_key Eric Biggers
@ 2020-12-08 23:48 ` Satya Tangirala
0 siblings, 0 replies; 28+ messages in thread
From: Satya Tangirala @ 2020-12-08 23:48 UTC (permalink / raw)
To: Eric Biggers
Cc: linux-mmc, linux-arm-msm, devicetree, linux-fscrypt, Ulf Hansson,
Andy Gross, Bjorn Andersson, Adrian Hunter, Asutosh Das,
Rob Herring, Neeraj Soni, Barani Muthukumaran, Peng Zhou,
Stanley Chu, Konrad Dybcio
On Wed, Dec 02, 2020 at 06:05:12PM -0800, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
>
> 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").
>
> Acked-by: Adrian Hunter <adrian.hunter@intel.com>
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
> drivers/mmc/host/cqhci-crypto.c | 22 +++++++++++++---------
> drivers/mmc/host/cqhci.h | 4 ++++
> 2 files changed, 17 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/mmc/host/cqhci-crypto.c b/drivers/mmc/host/cqhci-crypto.c
> index 98f141c8480ce..0aaa948d240b1 100644
> --- a/drivers/mmc/host/cqhci-crypto.c
> +++ b/drivers/mmc/host/cqhci-crypto.c
> @@ -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);
> }
>
> /*
> diff --git a/drivers/mmc/host/cqhci.h b/drivers/mmc/host/cqhci.h
> index 5c18734624fea..ece997dd8bcc7 100644
> --- a/drivers/mmc/host/cqhci.h
> +++ b/drivers/mmc/host/cqhci.h
> @@ -287,6 +287,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)
> --
> 2.29.2
>
Looks good to me. Please feel free to add
Reviewed-by: Satya Tangirala <satyat@google.com>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 6/9] firmware: qcom_scm: update comment for ICE-related functions
2020-12-03 2:05 [PATCH v2 0/9] eMMC inline encryption support Eric Biggers
` (4 preceding siblings ...)
2020-12-03 2:05 ` [PATCH v2 5/9] mmc: cqhci: add cqhci_host_ops::program_key Eric Biggers
@ 2020-12-03 2:05 ` Eric Biggers
2020-12-08 23:52 ` Satya Tangirala
2020-12-03 2:05 ` [PATCH v2 7/9] dt-bindings: mmc: sdhci-msm: add ICE registers and clock Eric Biggers
` (2 subsequent siblings)
8 siblings, 1 reply; 28+ messages in thread
From: Eric Biggers @ 2020-12-03 2:05 UTC (permalink / raw)
To: linux-mmc
Cc: linux-arm-msm, devicetree, linux-fscrypt, Satya Tangirala,
Ulf Hansson, Andy Gross, Bjorn Andersson, Adrian Hunter,
Asutosh Das, Rob Herring, Neeraj Soni, Barani Muthukumaran,
Peng Zhou, Stanley Chu, Konrad Dybcio
From: Eric Biggers <ebiggers@google.com>
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.
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
drivers/firmware/qcom_scm.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
index 7be48c1bec96d..f57779fc7ee93 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -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.29.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v2 6/9] firmware: qcom_scm: update comment for ICE-related functions
2020-12-03 2:05 ` [PATCH v2 6/9] firmware: qcom_scm: update comment for ICE-related functions Eric Biggers
@ 2020-12-08 23:52 ` Satya Tangirala
0 siblings, 0 replies; 28+ messages in thread
From: Satya Tangirala @ 2020-12-08 23:52 UTC (permalink / raw)
To: Eric Biggers
Cc: linux-mmc, linux-arm-msm, devicetree, linux-fscrypt, Ulf Hansson,
Andy Gross, Bjorn Andersson, Adrian Hunter, Asutosh Das,
Rob Herring, Neeraj Soni, Barani Muthukumaran, Peng Zhou,
Stanley Chu, Konrad Dybcio
On Wed, Dec 02, 2020 at 06:05:13PM -0800, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
>
> 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.
>
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
> drivers/firmware/qcom_scm.c | 16 +++++++++++-----
> 1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
> index 7be48c1bec96d..f57779fc7ee93 100644
> --- a/drivers/firmware/qcom_scm.c
> +++ b/drivers/firmware/qcom_scm.c
> @@ -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.29.2
>
Looks good to me. Please feel free to add
Reviewed-by: Satya Tangirala <satyat@google.com>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 7/9] dt-bindings: mmc: sdhci-msm: add ICE registers and clock
2020-12-03 2:05 [PATCH v2 0/9] eMMC inline encryption support Eric Biggers
` (5 preceding siblings ...)
2020-12-03 2:05 ` [PATCH v2 6/9] firmware: qcom_scm: update comment for ICE-related functions Eric Biggers
@ 2020-12-03 2:05 ` Eric Biggers
2020-12-08 23:54 ` Satya Tangirala
2020-12-03 2:05 ` [PATCH v2 8/9] arm64: dts: qcom: sdm630: add ICE registers and clocks Eric Biggers
2020-12-03 2:05 ` [PATCH v2 9/9] mmc: sdhci-msm: add Inline Crypto Engine support Eric Biggers
8 siblings, 1 reply; 28+ messages in thread
From: Eric Biggers @ 2020-12-03 2:05 UTC (permalink / raw)
To: linux-mmc
Cc: linux-arm-msm, devicetree, linux-fscrypt, Satya Tangirala,
Ulf Hansson, Andy Gross, Bjorn Andersson, Adrian Hunter,
Asutosh Das, Rob Herring, Neeraj Soni, Barani Muthukumaran,
Peng Zhou, Stanley Chu, Konrad Dybcio
From: Eric Biggers <ebiggers@google.com>
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.
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
Documentation/devicetree/bindings/mmc/sdhci-msm.txt | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
index 3b602fd6180bf..4f2e138439506 100644
--- a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
+++ b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
@@ -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
--
2.29.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v2 7/9] dt-bindings: mmc: sdhci-msm: add ICE registers and clock
2020-12-03 2:05 ` [PATCH v2 7/9] dt-bindings: mmc: sdhci-msm: add ICE registers and clock Eric Biggers
@ 2020-12-08 23:54 ` Satya Tangirala
0 siblings, 0 replies; 28+ messages in thread
From: Satya Tangirala @ 2020-12-08 23:54 UTC (permalink / raw)
To: Eric Biggers
Cc: linux-mmc, linux-arm-msm, devicetree, linux-fscrypt, Ulf Hansson,
Andy Gross, Bjorn Andersson, Adrian Hunter, Asutosh Das,
Rob Herring, Neeraj Soni, Barani Muthukumaran, Peng Zhou,
Stanley Chu, Konrad Dybcio
On Wed, Dec 02, 2020 at 06:05:14PM -0800, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
>
> 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.
>
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
> Documentation/devicetree/bindings/mmc/sdhci-msm.txt | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
> index 3b602fd6180bf..4f2e138439506 100644
> --- a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
> +++ b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
> @@ -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
> --
> 2.29.2
>
Looks good to me. Please feel free to add
Reviewed-by: Satya Tangirala <satyat@google.com>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 8/9] arm64: dts: qcom: sdm630: add ICE registers and clocks
2020-12-03 2:05 [PATCH v2 0/9] eMMC inline encryption support Eric Biggers
` (6 preceding siblings ...)
2020-12-03 2:05 ` [PATCH v2 7/9] dt-bindings: mmc: sdhci-msm: add ICE registers and clock Eric Biggers
@ 2020-12-03 2:05 ` Eric Biggers
2020-12-03 2:05 ` [PATCH v2 9/9] mmc: sdhci-msm: add Inline Crypto Engine support Eric Biggers
8 siblings, 0 replies; 28+ messages in thread
From: Eric Biggers @ 2020-12-03 2:05 UTC (permalink / raw)
To: linux-mmc
Cc: linux-arm-msm, devicetree, linux-fscrypt, Satya Tangirala,
Ulf Hansson, Andy Gross, Bjorn Andersson, Adrian Hunter,
Asutosh Das, Rob Herring, Neeraj Soni, Barani Muthukumaran,
Peng Zhou, Stanley Chu, Konrad Dybcio
From: Eric Biggers <ebiggers@google.com>
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.
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
arch/arm64/boot/dts/qcom/sdm630.dtsi | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sdm630.dtsi b/arch/arm64/boot/dts/qcom/sdm630.dtsi
index deb928d303c22..21aee33518b54 100644
--- a/arch/arm64/boot/dts/qcom/sdm630.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm630.dtsi
@@ -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>;
--
2.29.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v2 9/9] mmc: sdhci-msm: add Inline Crypto Engine support
2020-12-03 2:05 [PATCH v2 0/9] eMMC inline encryption support Eric Biggers
` (7 preceding siblings ...)
2020-12-03 2:05 ` [PATCH v2 8/9] arm64: dts: qcom: sdm630: add ICE registers and clocks Eric Biggers
@ 2020-12-03 2:05 ` Eric Biggers
2020-12-03 6:51 ` Adrian Hunter
2020-12-05 12:09 ` Satya Tangirala
8 siblings, 2 replies; 28+ messages in thread
From: Eric Biggers @ 2020-12-03 2:05 UTC (permalink / raw)
To: linux-mmc
Cc: linux-arm-msm, devicetree, linux-fscrypt, Satya Tangirala,
Ulf Hansson, Andy Gross, Bjorn Andersson, Adrian Hunter,
Asutosh Das, Rob Herring, Neeraj Soni, Barani Muthukumaran,
Peng Zhou, Stanley Chu, Konrad Dybcio
From: Eric Biggers <ebiggers@google.com>
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.
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
drivers/mmc/host/Kconfig | 1 +
drivers/mmc/host/sdhci-msm.c | 265 ++++++++++++++++++++++++++++++++++-
2 files changed, 262 insertions(+), 4 deletions(-)
diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 31481c9fcc2ec..4f8ff5a690fba 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -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
diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 3451eb3255135..ce6c3edbef530 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -13,6 +13,7 @@
#include <linux/pm_opp.h>
#include <linux/slab.h>
#include <linux/iopoll.h>
+#include <linux/qcom_scm.h>
#include <linux/regulator/consumer.h>
#include <linux/interconnect.h>
#include <linux/pinctrl/consumer.h>
@@ -256,10 +257,12 @@ struct sdhci_msm_variant_info {
struct sdhci_msm_host {
struct platform_device *pdev;
void __iomem *core_mem; /* MSM SDCC mapped address */
+ void __iomem *ice_mem; /* MSM ICE mapped address (if available) */
int pwr_irq; /* power irq */
struct clk *bus_clk; /* SDHC bus voter clock */
struct clk *xo_clk; /* TCXO clk needed for FLL feature of cm_dll*/
- struct clk_bulk_data bulk_clks[4]; /* core, iface, cal, sleep clocks */
+ /* core, iface, cal, sleep, and ice clocks */
+ struct clk_bulk_data bulk_clks[5];
unsigned long clk_rate;
struct mmc_host *mmc;
struct opp_table *opp_table;
@@ -1785,6 +1788,235 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
__sdhci_msm_set_clock(host, clock);
}
+/*****************************************************************************\
+ * *
+ * Inline Crypto Engine (ICE) support *
+ * *
+\*****************************************************************************/
+
+#ifdef CONFIG_MMC_CRYPTO
+
+#define AES_256_XTS_KEY_SIZE 64
+
+/* QCOM ICE registers */
+
+#define QCOM_ICE_REG_VERSION 0x0008
+
+#define QCOM_ICE_REG_FUSE_SETTING 0x0010
+#define QCOM_ICE_FUSE_SETTING_MASK 0x1
+#define QCOM_ICE_FORCE_HW_KEY0_SETTING_MASK 0x2
+#define QCOM_ICE_FORCE_HW_KEY1_SETTING_MASK 0x4
+
+#define QCOM_ICE_REG_BIST_STATUS 0x0070
+#define QCOM_ICE_BIST_STATUS_MASK 0xF0000000
+
+#define QCOM_ICE_REG_ADVANCED_CONTROL 0x1000
+
+#define sdhci_msm_ice_writel(host, val, reg) \
+ writel((val), (host)->ice_mem + (reg))
+#define sdhci_msm_ice_readl(host, reg) \
+ readl((host)->ice_mem + (reg))
+
+static bool sdhci_msm_ice_supported(struct sdhci_msm_host *msm_host)
+{
+ struct device *dev = mmc_dev(msm_host->mmc);
+ u32 regval = sdhci_msm_ice_readl(msm_host, QCOM_ICE_REG_VERSION);
+ int major = regval >> 24;
+ int minor = (regval >> 16) & 0xFF;
+ int step = regval & 0xFFFF;
+
+ /* For now this driver only supports ICE version 3. */
+ if (major != 3) {
+ dev_warn(dev, "Unsupported ICE version: v%d.%d.%d\n",
+ major, minor, step);
+ return false;
+ }
+
+ dev_info(dev, "Found QC Inline Crypto Engine (ICE) v%d.%d.%d\n",
+ major, minor, step);
+
+ /* If fuses are blown, ICE might not work in the standard way. */
+ regval = sdhci_msm_ice_readl(msm_host, QCOM_ICE_REG_FUSE_SETTING);
+ if (regval & (QCOM_ICE_FUSE_SETTING_MASK |
+ QCOM_ICE_FORCE_HW_KEY0_SETTING_MASK |
+ QCOM_ICE_FORCE_HW_KEY1_SETTING_MASK)) {
+ dev_warn(dev, "Fuses are blown; ICE is unusable!\n");
+ return false;
+ }
+ return true;
+}
+
+static inline struct clk *sdhci_msm_ice_get_clk(struct device *dev)
+{
+ return devm_clk_get(dev, "ice");
+}
+
+static int sdhci_msm_ice_init(struct sdhci_msm_host *msm_host,
+ struct cqhci_host *cq_host)
+{
+ struct mmc_host *mmc = msm_host->mmc;
+ struct device *dev = mmc_dev(mmc);
+ struct resource *res;
+ int err;
+
+ if (!(cqhci_readl(cq_host, CQHCI_CAP) & CQHCI_CAP_CS))
+ return 0;
+
+ res = platform_get_resource_byname(msm_host->pdev, IORESOURCE_MEM,
+ "ice");
+ if (!res) {
+ dev_warn(dev, "ICE registers not found\n");
+ goto disable;
+ }
+
+ if (!qcom_scm_ice_available()) {
+ dev_warn(dev, "ICE SCM interface not found\n");
+ goto disable;
+ }
+
+ msm_host->ice_mem = devm_ioremap_resource(dev, res);
+ if (IS_ERR(msm_host->ice_mem)) {
+ err = PTR_ERR(msm_host->ice_mem);
+ dev_err(dev, "Failed to map ICE registers; err=%d\n", err);
+ return err;
+ }
+
+ if (!sdhci_msm_ice_supported(msm_host))
+ goto disable;
+
+ mmc->caps2 |= MMC_CAP2_CRYPTO;
+ return 0;
+
+disable:
+ dev_warn(dev, "Disabling inline encryption support\n");
+ return 0;
+}
+
+static void sdhci_msm_ice_low_power_mode_enable(struct sdhci_msm_host *msm_host)
+{
+ u32 regval;
+
+ regval = sdhci_msm_ice_readl(msm_host, QCOM_ICE_REG_ADVANCED_CONTROL);
+ /*
+ * Enable low power mode sequence
+ * [0]-0, [1]-0, [2]-0, [3]-E, [4]-0, [5]-0, [6]-0, [7]-0
+ */
+ regval |= 0x7000;
+ sdhci_msm_ice_writel(msm_host, regval, QCOM_ICE_REG_ADVANCED_CONTROL);
+}
+
+static void sdhci_msm_ice_optimization_enable(struct sdhci_msm_host *msm_host)
+{
+ u32 regval;
+
+ /* ICE Optimizations Enable Sequence */
+ regval = sdhci_msm_ice_readl(msm_host, QCOM_ICE_REG_ADVANCED_CONTROL);
+ regval |= 0xD807100;
+ /* ICE HPG requires delay before writing */
+ udelay(5);
+ sdhci_msm_ice_writel(msm_host, regval, QCOM_ICE_REG_ADVANCED_CONTROL);
+ udelay(5);
+}
+
+/* Poll until all BIST (built-in self test) bits are reset */
+static int sdhci_msm_ice_wait_bist_status(struct sdhci_msm_host *msm_host)
+{
+ u32 regval;
+ int err;
+
+ err = readl_poll_timeout(msm_host->ice_mem + QCOM_ICE_REG_BIST_STATUS,
+ regval, !(regval & QCOM_ICE_BIST_STATUS_MASK),
+ 50, 5000);
+ if (err)
+ dev_err(mmc_dev(msm_host->mmc),
+ "Timed out waiting for ICE self-test to complete\n");
+ return err;
+}
+
+static void sdhci_msm_ice_enable(struct sdhci_msm_host *msm_host)
+{
+ if (!(msm_host->mmc->caps2 & MMC_CAP2_CRYPTO))
+ return;
+ sdhci_msm_ice_low_power_mode_enable(msm_host);
+ sdhci_msm_ice_optimization_enable(msm_host);
+ sdhci_msm_ice_wait_bist_status(msm_host);
+}
+
+static int __maybe_unused sdhci_msm_ice_resume(struct sdhci_msm_host *msm_host)
+{
+ if (!(msm_host->mmc->caps2 & MMC_CAP2_CRYPTO))
+ return 0;
+ return sdhci_msm_ice_wait_bist_status(msm_host);
+}
+
+/*
+ * Program a key into a QC ICE keyslot, or evict a keyslot. QC ICE requires
+ * vendor-specific SCM calls for this; it doesn't support the standard way.
+ */
+static int sdhci_msm_program_key(struct cqhci_host *cq_host,
+ const union cqhci_crypto_cfg_entry *cfg,
+ int slot)
+{
+ struct device *dev = mmc_dev(cq_host->mmc);
+ union cqhci_crypto_cap_entry cap;
+ union {
+ u8 bytes[AES_256_XTS_KEY_SIZE];
+ u32 words[AES_256_XTS_KEY_SIZE / sizeof(u32)];
+ } key;
+ int i;
+ int err;
+
+ if (!(cfg->config_enable & CQHCI_CRYPTO_CONFIGURATION_ENABLE))
+ return qcom_scm_ice_invalidate_key(slot);
+
+ /* Only AES-256-XTS has been tested so far. */
+ cap = cq_host->crypto_cap_array[cfg->crypto_cap_idx];
+ if (cap.algorithm_id != CQHCI_CRYPTO_ALG_AES_XTS ||
+ cap.key_size != CQHCI_CRYPTO_KEY_SIZE_256) {
+ dev_err_ratelimited(dev,
+ "Unhandled crypto capability; algorithm_id=%d, key_size=%d\n",
+ cap.algorithm_id, cap.key_size);
+ return -EINVAL;
+ }
+
+ memcpy(key.bytes, cfg->crypto_key, AES_256_XTS_KEY_SIZE);
+
+ /*
+ * The SCM call byte-swaps the 32-bit words of the key. So we have to
+ * do the same, in order for the final key be correct.
+ */
+ for (i = 0; i < ARRAY_SIZE(key.words); i++)
+ __cpu_to_be32s(&key.words[i]);
+
+ err = qcom_scm_ice_set_key(slot, key.bytes, AES_256_XTS_KEY_SIZE,
+ QCOM_SCM_ICE_CIPHER_AES_256_XTS,
+ cfg->data_unit_size);
+ memzero_explicit(&key, sizeof(key));
+ return err;
+}
+#else /* CONFIG_MMC_CRYPTO */
+static inline struct clk *sdhci_msm_ice_get_clk(struct device *dev)
+{
+ return NULL;
+}
+
+static inline int sdhci_msm_ice_init(struct sdhci_msm_host *msm_host,
+ struct cqhci_host *cq_host)
+{
+ return 0;
+}
+
+static inline void sdhci_msm_ice_enable(struct sdhci_msm_host *msm_host)
+{
+}
+
+static inline int __maybe_unused
+sdhci_msm_ice_resume(struct sdhci_msm_host *msm_host)
+{
+ return 0;
+}
+#endif /* !CONFIG_MMC_CRYPTO */
+
/*****************************************************************************\
* *
* MSM Command Queue Engine (CQE) *
@@ -1803,6 +2035,16 @@ static u32 sdhci_msm_cqe_irq(struct sdhci_host *host, u32 intmask)
return 0;
}
+static void sdhci_msm_cqe_enable(struct mmc_host *mmc)
+{
+ struct sdhci_host *host = mmc_priv(mmc);
+ struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+ struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
+
+ sdhci_cqe_enable(mmc);
+ sdhci_msm_ice_enable(msm_host);
+}
+
static void sdhci_msm_cqe_disable(struct mmc_host *mmc, bool recovery)
{
struct sdhci_host *host = mmc_priv(mmc);
@@ -1835,8 +2077,11 @@ static void sdhci_msm_cqe_disable(struct mmc_host *mmc, bool recovery)
}
static const struct cqhci_host_ops sdhci_msm_cqhci_ops = {
- .enable = sdhci_cqe_enable,
+ .enable = sdhci_msm_cqe_enable,
.disable = sdhci_msm_cqe_disable,
+#ifdef CONFIG_MMC_CRYPTO
+ .program_key = sdhci_msm_program_key,
+#endif
};
static int sdhci_msm_cqe_add_host(struct sdhci_host *host,
@@ -1872,6 +2117,10 @@ static int sdhci_msm_cqe_add_host(struct sdhci_host *host,
dma64 = host->flags & SDHCI_USE_64_BIT_DMA;
+ ret = sdhci_msm_ice_init(msm_host, cq_host);
+ if (ret)
+ goto cleanup;
+
ret = cqhci_init(cq_host, host->mmc, dma64);
if (ret) {
dev_err(&pdev->dev, "%s: CQE init: failed (%d)\n",
@@ -2321,6 +2570,11 @@ static int sdhci_msm_probe(struct platform_device *pdev)
clk = NULL;
msm_host->bulk_clks[3].clk = clk;
+ clk = sdhci_msm_ice_get_clk(&pdev->dev);
+ if (IS_ERR(clk))
+ clk = NULL;
+ msm_host->bulk_clks[4].clk = clk;
+
ret = clk_bulk_prepare_enable(ARRAY_SIZE(msm_host->bulk_clks),
msm_host->bulk_clks);
if (ret)
@@ -2531,12 +2785,15 @@ static __maybe_unused int sdhci_msm_runtime_resume(struct device *dev)
* Whenever core-clock is gated dynamically, it's needed to
* restore the SDR DLL settings when the clock is ungated.
*/
- if (msm_host->restore_dll_config && msm_host->clk_rate)
+ if (msm_host->restore_dll_config && msm_host->clk_rate) {
ret = sdhci_msm_restore_sdr_dll_config(host);
+ if (ret)
+ return ret;
+ }
dev_pm_opp_set_rate(dev, msm_host->clk_rate);
- return ret;
+ return sdhci_msm_ice_resume(msm_host);
}
static const struct dev_pm_ops sdhci_msm_pm_ops = {
--
2.29.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v2 9/9] mmc: sdhci-msm: add Inline Crypto Engine support
2020-12-03 2:05 ` [PATCH v2 9/9] mmc: sdhci-msm: add Inline Crypto Engine support Eric Biggers
@ 2020-12-03 6:51 ` Adrian Hunter
2020-12-05 12:09 ` Satya Tangirala
1 sibling, 0 replies; 28+ messages in thread
From: Adrian Hunter @ 2020-12-03 6:51 UTC (permalink / raw)
To: Eric Biggers, linux-mmc
Cc: linux-arm-msm, devicetree, linux-fscrypt, Satya Tangirala,
Ulf Hansson, Andy Gross, Bjorn Andersson, Asutosh Das,
Rob Herring, Neeraj Soni, Barani Muthukumaran, Peng Zhou,
Stanley Chu, Konrad Dybcio
On 3/12/20 4:05 am, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
>
> 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.
>
> Signed-off-by: Eric Biggers <ebiggers@google.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
> drivers/mmc/host/Kconfig | 1 +
> drivers/mmc/host/sdhci-msm.c | 265 ++++++++++++++++++++++++++++++++++-
> 2 files changed, 262 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index 31481c9fcc2ec..4f8ff5a690fba 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -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
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index 3451eb3255135..ce6c3edbef530 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -13,6 +13,7 @@
> #include <linux/pm_opp.h>
> #include <linux/slab.h>
> #include <linux/iopoll.h>
> +#include <linux/qcom_scm.h>
> #include <linux/regulator/consumer.h>
> #include <linux/interconnect.h>
> #include <linux/pinctrl/consumer.h>
> @@ -256,10 +257,12 @@ struct sdhci_msm_variant_info {
> struct sdhci_msm_host {
> struct platform_device *pdev;
> void __iomem *core_mem; /* MSM SDCC mapped address */
> + void __iomem *ice_mem; /* MSM ICE mapped address (if available) */
> int pwr_irq; /* power irq */
> struct clk *bus_clk; /* SDHC bus voter clock */
> struct clk *xo_clk; /* TCXO clk needed for FLL feature of cm_dll*/
> - struct clk_bulk_data bulk_clks[4]; /* core, iface, cal, sleep clocks */
> + /* core, iface, cal, sleep, and ice clocks */
> + struct clk_bulk_data bulk_clks[5];
> unsigned long clk_rate;
> struct mmc_host *mmc;
> struct opp_table *opp_table;
> @@ -1785,6 +1788,235 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
> __sdhci_msm_set_clock(host, clock);
> }
>
> +/*****************************************************************************\
> + * *
> + * Inline Crypto Engine (ICE) support *
> + * *
> +\*****************************************************************************/
> +
> +#ifdef CONFIG_MMC_CRYPTO
> +
> +#define AES_256_XTS_KEY_SIZE 64
> +
> +/* QCOM ICE registers */
> +
> +#define QCOM_ICE_REG_VERSION 0x0008
> +
> +#define QCOM_ICE_REG_FUSE_SETTING 0x0010
> +#define QCOM_ICE_FUSE_SETTING_MASK 0x1
> +#define QCOM_ICE_FORCE_HW_KEY0_SETTING_MASK 0x2
> +#define QCOM_ICE_FORCE_HW_KEY1_SETTING_MASK 0x4
> +
> +#define QCOM_ICE_REG_BIST_STATUS 0x0070
> +#define QCOM_ICE_BIST_STATUS_MASK 0xF0000000
> +
> +#define QCOM_ICE_REG_ADVANCED_CONTROL 0x1000
> +
> +#define sdhci_msm_ice_writel(host, val, reg) \
> + writel((val), (host)->ice_mem + (reg))
> +#define sdhci_msm_ice_readl(host, reg) \
> + readl((host)->ice_mem + (reg))
> +
> +static bool sdhci_msm_ice_supported(struct sdhci_msm_host *msm_host)
> +{
> + struct device *dev = mmc_dev(msm_host->mmc);
> + u32 regval = sdhci_msm_ice_readl(msm_host, QCOM_ICE_REG_VERSION);
> + int major = regval >> 24;
> + int minor = (regval >> 16) & 0xFF;
> + int step = regval & 0xFFFF;
> +
> + /* For now this driver only supports ICE version 3. */
> + if (major != 3) {
> + dev_warn(dev, "Unsupported ICE version: v%d.%d.%d\n",
> + major, minor, step);
> + return false;
> + }
> +
> + dev_info(dev, "Found QC Inline Crypto Engine (ICE) v%d.%d.%d\n",
> + major, minor, step);
> +
> + /* If fuses are blown, ICE might not work in the standard way. */
> + regval = sdhci_msm_ice_readl(msm_host, QCOM_ICE_REG_FUSE_SETTING);
> + if (regval & (QCOM_ICE_FUSE_SETTING_MASK |
> + QCOM_ICE_FORCE_HW_KEY0_SETTING_MASK |
> + QCOM_ICE_FORCE_HW_KEY1_SETTING_MASK)) {
> + dev_warn(dev, "Fuses are blown; ICE is unusable!\n");
> + return false;
> + }
> + return true;
> +}
> +
> +static inline struct clk *sdhci_msm_ice_get_clk(struct device *dev)
> +{
> + return devm_clk_get(dev, "ice");
> +}
> +
> +static int sdhci_msm_ice_init(struct sdhci_msm_host *msm_host,
> + struct cqhci_host *cq_host)
> +{
> + struct mmc_host *mmc = msm_host->mmc;
> + struct device *dev = mmc_dev(mmc);
> + struct resource *res;
> + int err;
> +
> + if (!(cqhci_readl(cq_host, CQHCI_CAP) & CQHCI_CAP_CS))
> + return 0;
> +
> + res = platform_get_resource_byname(msm_host->pdev, IORESOURCE_MEM,
> + "ice");
> + if (!res) {
> + dev_warn(dev, "ICE registers not found\n");
> + goto disable;
> + }
> +
> + if (!qcom_scm_ice_available()) {
> + dev_warn(dev, "ICE SCM interface not found\n");
> + goto disable;
> + }
> +
> + msm_host->ice_mem = devm_ioremap_resource(dev, res);
> + if (IS_ERR(msm_host->ice_mem)) {
> + err = PTR_ERR(msm_host->ice_mem);
> + dev_err(dev, "Failed to map ICE registers; err=%d\n", err);
> + return err;
> + }
> +
> + if (!sdhci_msm_ice_supported(msm_host))
> + goto disable;
> +
> + mmc->caps2 |= MMC_CAP2_CRYPTO;
> + return 0;
> +
> +disable:
> + dev_warn(dev, "Disabling inline encryption support\n");
> + return 0;
> +}
> +
> +static void sdhci_msm_ice_low_power_mode_enable(struct sdhci_msm_host *msm_host)
> +{
> + u32 regval;
> +
> + regval = sdhci_msm_ice_readl(msm_host, QCOM_ICE_REG_ADVANCED_CONTROL);
> + /*
> + * Enable low power mode sequence
> + * [0]-0, [1]-0, [2]-0, [3]-E, [4]-0, [5]-0, [6]-0, [7]-0
> + */
> + regval |= 0x7000;
> + sdhci_msm_ice_writel(msm_host, regval, QCOM_ICE_REG_ADVANCED_CONTROL);
> +}
> +
> +static void sdhci_msm_ice_optimization_enable(struct sdhci_msm_host *msm_host)
> +{
> + u32 regval;
> +
> + /* ICE Optimizations Enable Sequence */
> + regval = sdhci_msm_ice_readl(msm_host, QCOM_ICE_REG_ADVANCED_CONTROL);
> + regval |= 0xD807100;
> + /* ICE HPG requires delay before writing */
> + udelay(5);
> + sdhci_msm_ice_writel(msm_host, regval, QCOM_ICE_REG_ADVANCED_CONTROL);
> + udelay(5);
> +}
> +
> +/* Poll until all BIST (built-in self test) bits are reset */
> +static int sdhci_msm_ice_wait_bist_status(struct sdhci_msm_host *msm_host)
> +{
> + u32 regval;
> + int err;
> +
> + err = readl_poll_timeout(msm_host->ice_mem + QCOM_ICE_REG_BIST_STATUS,
> + regval, !(regval & QCOM_ICE_BIST_STATUS_MASK),
> + 50, 5000);
> + if (err)
> + dev_err(mmc_dev(msm_host->mmc),
> + "Timed out waiting for ICE self-test to complete\n");
> + return err;
> +}
> +
> +static void sdhci_msm_ice_enable(struct sdhci_msm_host *msm_host)
> +{
> + if (!(msm_host->mmc->caps2 & MMC_CAP2_CRYPTO))
> + return;
> + sdhci_msm_ice_low_power_mode_enable(msm_host);
> + sdhci_msm_ice_optimization_enable(msm_host);
> + sdhci_msm_ice_wait_bist_status(msm_host);
> +}
> +
> +static int __maybe_unused sdhci_msm_ice_resume(struct sdhci_msm_host *msm_host)
> +{
> + if (!(msm_host->mmc->caps2 & MMC_CAP2_CRYPTO))
> + return 0;
> + return sdhci_msm_ice_wait_bist_status(msm_host);
> +}
> +
> +/*
> + * Program a key into a QC ICE keyslot, or evict a keyslot. QC ICE requires
> + * vendor-specific SCM calls for this; it doesn't support the standard way.
> + */
> +static int sdhci_msm_program_key(struct cqhci_host *cq_host,
> + const union cqhci_crypto_cfg_entry *cfg,
> + int slot)
> +{
> + struct device *dev = mmc_dev(cq_host->mmc);
> + union cqhci_crypto_cap_entry cap;
> + union {
> + u8 bytes[AES_256_XTS_KEY_SIZE];
> + u32 words[AES_256_XTS_KEY_SIZE / sizeof(u32)];
> + } key;
> + int i;
> + int err;
> +
> + if (!(cfg->config_enable & CQHCI_CRYPTO_CONFIGURATION_ENABLE))
> + return qcom_scm_ice_invalidate_key(slot);
> +
> + /* Only AES-256-XTS has been tested so far. */
> + cap = cq_host->crypto_cap_array[cfg->crypto_cap_idx];
> + if (cap.algorithm_id != CQHCI_CRYPTO_ALG_AES_XTS ||
> + cap.key_size != CQHCI_CRYPTO_KEY_SIZE_256) {
> + dev_err_ratelimited(dev,
> + "Unhandled crypto capability; algorithm_id=%d, key_size=%d\n",
> + cap.algorithm_id, cap.key_size);
> + return -EINVAL;
> + }
> +
> + memcpy(key.bytes, cfg->crypto_key, AES_256_XTS_KEY_SIZE);
> +
> + /*
> + * The SCM call byte-swaps the 32-bit words of the key. So we have to
> + * do the same, in order for the final key be correct.
> + */
> + for (i = 0; i < ARRAY_SIZE(key.words); i++)
> + __cpu_to_be32s(&key.words[i]);
> +
> + err = qcom_scm_ice_set_key(slot, key.bytes, AES_256_XTS_KEY_SIZE,
> + QCOM_SCM_ICE_CIPHER_AES_256_XTS,
> + cfg->data_unit_size);
> + memzero_explicit(&key, sizeof(key));
> + return err;
> +}
> +#else /* CONFIG_MMC_CRYPTO */
> +static inline struct clk *sdhci_msm_ice_get_clk(struct device *dev)
> +{
> + return NULL;
> +}
> +
> +static inline int sdhci_msm_ice_init(struct sdhci_msm_host *msm_host,
> + struct cqhci_host *cq_host)
> +{
> + return 0;
> +}
> +
> +static inline void sdhci_msm_ice_enable(struct sdhci_msm_host *msm_host)
> +{
> +}
> +
> +static inline int __maybe_unused
> +sdhci_msm_ice_resume(struct sdhci_msm_host *msm_host)
> +{
> + return 0;
> +}
> +#endif /* !CONFIG_MMC_CRYPTO */
> +
> /*****************************************************************************\
> * *
> * MSM Command Queue Engine (CQE) *
> @@ -1803,6 +2035,16 @@ static u32 sdhci_msm_cqe_irq(struct sdhci_host *host, u32 intmask)
> return 0;
> }
>
> +static void sdhci_msm_cqe_enable(struct mmc_host *mmc)
> +{
> + struct sdhci_host *host = mmc_priv(mmc);
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> +
> + sdhci_cqe_enable(mmc);
> + sdhci_msm_ice_enable(msm_host);
> +}
> +
> static void sdhci_msm_cqe_disable(struct mmc_host *mmc, bool recovery)
> {
> struct sdhci_host *host = mmc_priv(mmc);
> @@ -1835,8 +2077,11 @@ static void sdhci_msm_cqe_disable(struct mmc_host *mmc, bool recovery)
> }
>
> static const struct cqhci_host_ops sdhci_msm_cqhci_ops = {
> - .enable = sdhci_cqe_enable,
> + .enable = sdhci_msm_cqe_enable,
> .disable = sdhci_msm_cqe_disable,
> +#ifdef CONFIG_MMC_CRYPTO
> + .program_key = sdhci_msm_program_key,
> +#endif
> };
>
> static int sdhci_msm_cqe_add_host(struct sdhci_host *host,
> @@ -1872,6 +2117,10 @@ static int sdhci_msm_cqe_add_host(struct sdhci_host *host,
>
> dma64 = host->flags & SDHCI_USE_64_BIT_DMA;
>
> + ret = sdhci_msm_ice_init(msm_host, cq_host);
> + if (ret)
> + goto cleanup;
> +
> ret = cqhci_init(cq_host, host->mmc, dma64);
> if (ret) {
> dev_err(&pdev->dev, "%s: CQE init: failed (%d)\n",
> @@ -2321,6 +2570,11 @@ static int sdhci_msm_probe(struct platform_device *pdev)
> clk = NULL;
> msm_host->bulk_clks[3].clk = clk;
>
> + clk = sdhci_msm_ice_get_clk(&pdev->dev);
> + if (IS_ERR(clk))
> + clk = NULL;
> + msm_host->bulk_clks[4].clk = clk;
> +
> ret = clk_bulk_prepare_enable(ARRAY_SIZE(msm_host->bulk_clks),
> msm_host->bulk_clks);
> if (ret)
> @@ -2531,12 +2785,15 @@ static __maybe_unused int sdhci_msm_runtime_resume(struct device *dev)
> * Whenever core-clock is gated dynamically, it's needed to
> * restore the SDR DLL settings when the clock is ungated.
> */
> - if (msm_host->restore_dll_config && msm_host->clk_rate)
> + if (msm_host->restore_dll_config && msm_host->clk_rate) {
> ret = sdhci_msm_restore_sdr_dll_config(host);
> + if (ret)
> + return ret;
> + }
>
> dev_pm_opp_set_rate(dev, msm_host->clk_rate);
>
> - return ret;
> + return sdhci_msm_ice_resume(msm_host);
> }
>
> static const struct dev_pm_ops sdhci_msm_pm_ops = {
>
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 9/9] mmc: sdhci-msm: add Inline Crypto Engine support
2020-12-03 2:05 ` [PATCH v2 9/9] mmc: sdhci-msm: add Inline Crypto Engine support Eric Biggers
2020-12-03 6:51 ` Adrian Hunter
@ 2020-12-05 12:09 ` Satya Tangirala
2020-12-05 19:43 ` Eric Biggers
1 sibling, 1 reply; 28+ messages in thread
From: Satya Tangirala @ 2020-12-05 12:09 UTC (permalink / raw)
To: Eric Biggers
Cc: linux-mmc, linux-arm-msm, devicetree, linux-fscrypt, Ulf Hansson,
Andy Gross, Bjorn Andersson, Adrian Hunter, Asutosh Das,
Rob Herring, Neeraj Soni, Barani Muthukumaran, Peng Zhou,
Stanley Chu, Konrad Dybcio
On Wed, Dec 02, 2020 at 06:05:16PM -0800, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
>
> 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.
>
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
> drivers/mmc/host/Kconfig | 1 +
> drivers/mmc/host/sdhci-msm.c | 265 ++++++++++++++++++++++++++++++++++-
> 2 files changed, 262 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index 31481c9fcc2ec..4f8ff5a690fba 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -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
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index 3451eb3255135..ce6c3edbef530 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -13,6 +13,7 @@
> #include <linux/pm_opp.h>
> #include <linux/slab.h>
> #include <linux/iopoll.h>
> +#include <linux/qcom_scm.h>
> #include <linux/regulator/consumer.h>
> #include <linux/interconnect.h>
> #include <linux/pinctrl/consumer.h>
> @@ -256,10 +257,12 @@ struct sdhci_msm_variant_info {
> struct sdhci_msm_host {
> struct platform_device *pdev;
> void __iomem *core_mem; /* MSM SDCC mapped address */
> + void __iomem *ice_mem; /* MSM ICE mapped address (if available) */
> int pwr_irq; /* power irq */
> struct clk *bus_clk; /* SDHC bus voter clock */
> struct clk *xo_clk; /* TCXO clk needed for FLL feature of cm_dll*/
> - struct clk_bulk_data bulk_clks[4]; /* core, iface, cal, sleep clocks */
> + /* core, iface, cal, sleep, and ice clocks */
> + struct clk_bulk_data bulk_clks[5];
> unsigned long clk_rate;
> struct mmc_host *mmc;
> struct opp_table *opp_table;
> @@ -1785,6 +1788,235 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
> __sdhci_msm_set_clock(host, clock);
> }
>
> +/*****************************************************************************\
> + * *
> + * Inline Crypto Engine (ICE) support *
> + * *
> +\*****************************************************************************/
> +
> +#ifdef CONFIG_MMC_CRYPTO
> +
> +#define AES_256_XTS_KEY_SIZE 64
> +
> +/* QCOM ICE registers */
> +
> +#define QCOM_ICE_REG_VERSION 0x0008
> +
> +#define QCOM_ICE_REG_FUSE_SETTING 0x0010
> +#define QCOM_ICE_FUSE_SETTING_MASK 0x1
> +#define QCOM_ICE_FORCE_HW_KEY0_SETTING_MASK 0x2
> +#define QCOM_ICE_FORCE_HW_KEY1_SETTING_MASK 0x4
> +
> +#define QCOM_ICE_REG_BIST_STATUS 0x0070
> +#define QCOM_ICE_BIST_STATUS_MASK 0xF0000000
> +
> +#define QCOM_ICE_REG_ADVANCED_CONTROL 0x1000
> +
> +#define sdhci_msm_ice_writel(host, val, reg) \
> + writel((val), (host)->ice_mem + (reg))
> +#define sdhci_msm_ice_readl(host, reg) \
> + readl((host)->ice_mem + (reg))
> +
> +static bool sdhci_msm_ice_supported(struct sdhci_msm_host *msm_host)
> +{
> + struct device *dev = mmc_dev(msm_host->mmc);
> + u32 regval = sdhci_msm_ice_readl(msm_host, QCOM_ICE_REG_VERSION);
> + int major = regval >> 24;
> + int minor = (regval >> 16) & 0xFF;
> + int step = regval & 0xFFFF;
> +
> + /* For now this driver only supports ICE version 3. */
> + if (major != 3) {
> + dev_warn(dev, "Unsupported ICE version: v%d.%d.%d\n",
> + major, minor, step);
> + return false;
> + }
> +
> + dev_info(dev, "Found QC Inline Crypto Engine (ICE) v%d.%d.%d\n",
> + major, minor, step);
> +
> + /* If fuses are blown, ICE might not work in the standard way. */
> + regval = sdhci_msm_ice_readl(msm_host, QCOM_ICE_REG_FUSE_SETTING);
> + if (regval & (QCOM_ICE_FUSE_SETTING_MASK |
> + QCOM_ICE_FORCE_HW_KEY0_SETTING_MASK |
> + QCOM_ICE_FORCE_HW_KEY1_SETTING_MASK)) {
> + dev_warn(dev, "Fuses are blown; ICE is unusable!\n");
> + return false;
> + }
> + return true;
> +}
> +
> +static inline struct clk *sdhci_msm_ice_get_clk(struct device *dev)
> +{
> + return devm_clk_get(dev, "ice");
> +}
> +
> +static int sdhci_msm_ice_init(struct sdhci_msm_host *msm_host,
> + struct cqhci_host *cq_host)
> +{
> + struct mmc_host *mmc = msm_host->mmc;
> + struct device *dev = mmc_dev(mmc);
> + struct resource *res;
> + int err;
> +
> + if (!(cqhci_readl(cq_host, CQHCI_CAP) & CQHCI_CAP_CS))
> + return 0;
> +
> + res = platform_get_resource_byname(msm_host->pdev, IORESOURCE_MEM,
> + "ice");
> + if (!res) {
> + dev_warn(dev, "ICE registers not found\n");
> + goto disable;
> + }
> +
> + if (!qcom_scm_ice_available()) {
> + dev_warn(dev, "ICE SCM interface not found\n");
> + goto disable;
> + }
> +
> + msm_host->ice_mem = devm_ioremap_resource(dev, res);
> + if (IS_ERR(msm_host->ice_mem)) {
> + err = PTR_ERR(msm_host->ice_mem);
> + dev_err(dev, "Failed to map ICE registers; err=%d\n", err);
> + return err;
> + }
> +
> + if (!sdhci_msm_ice_supported(msm_host))
> + goto disable;
> +
> + mmc->caps2 |= MMC_CAP2_CRYPTO;
> + return 0;
> +
> +disable:
> + dev_warn(dev, "Disabling inline encryption support\n");
> + return 0;
> +}
> +
> +static void sdhci_msm_ice_low_power_mode_enable(struct sdhci_msm_host *msm_host)
> +{
> + u32 regval;
> +
> + regval = sdhci_msm_ice_readl(msm_host, QCOM_ICE_REG_ADVANCED_CONTROL);
> + /*
> + * Enable low power mode sequence
> + * [0]-0, [1]-0, [2]-0, [3]-E, [4]-0, [5]-0, [6]-0, [7]-0
> + */
> + regval |= 0x7000;
> + sdhci_msm_ice_writel(msm_host, regval, QCOM_ICE_REG_ADVANCED_CONTROL);
> +}
> +
> +static void sdhci_msm_ice_optimization_enable(struct sdhci_msm_host *msm_host)
> +{
> + u32 regval;
> +
> + /* ICE Optimizations Enable Sequence */
> + regval = sdhci_msm_ice_readl(msm_host, QCOM_ICE_REG_ADVANCED_CONTROL);
> + regval |= 0xD807100;
> + /* ICE HPG requires delay before writing */
> + udelay(5);
> + sdhci_msm_ice_writel(msm_host, regval, QCOM_ICE_REG_ADVANCED_CONTROL);
> + udelay(5);
> +}
> +
> +/* Poll until all BIST (built-in self test) bits are reset */
> +static int sdhci_msm_ice_wait_bist_status(struct sdhci_msm_host *msm_host)
> +{
> + u32 regval;
> + int err;
> +
> + err = readl_poll_timeout(msm_host->ice_mem + QCOM_ICE_REG_BIST_STATUS,
> + regval, !(regval & QCOM_ICE_BIST_STATUS_MASK),
> + 50, 5000);
> + if (err)
> + dev_err(mmc_dev(msm_host->mmc),
> + "Timed out waiting for ICE self-test to complete\n");
> + return err;
> +}
> +
> +static void sdhci_msm_ice_enable(struct sdhci_msm_host *msm_host)
> +{
> + if (!(msm_host->mmc->caps2 & MMC_CAP2_CRYPTO))
> + return;
> + sdhci_msm_ice_low_power_mode_enable(msm_host);
> + sdhci_msm_ice_optimization_enable(msm_host);
> + sdhci_msm_ice_wait_bist_status(msm_host);
If sdhci_msm_ice_wait_bist_status() fails, should we really ignore the
error and continue en/decrypting with ICE? I'm not sure what the BIST
failing might really mean, but if it means it's possible that the ICE
en/decrypts incorrectly it would be bad to continue to use it.....
> +}
> +
> +static int __maybe_unused sdhci_msm_ice_resume(struct sdhci_msm_host *msm_host)
> +{
> + if (!(msm_host->mmc->caps2 & MMC_CAP2_CRYPTO))
> + return 0;
> + return sdhci_msm_ice_wait_bist_status(msm_host);
> +}
> +
> +/*
> + * Program a key into a QC ICE keyslot, or evict a keyslot. QC ICE requires
> + * vendor-specific SCM calls for this; it doesn't support the standard way.
> + */
> +static int sdhci_msm_program_key(struct cqhci_host *cq_host,
> + const union cqhci_crypto_cfg_entry *cfg,
> + int slot)
> +{
> + struct device *dev = mmc_dev(cq_host->mmc);
> + union cqhci_crypto_cap_entry cap;
> + union {
> + u8 bytes[AES_256_XTS_KEY_SIZE];
> + u32 words[AES_256_XTS_KEY_SIZE / sizeof(u32)];
> + } key;
> + int i;
> + int err;
> +
> + if (!(cfg->config_enable & CQHCI_CRYPTO_CONFIGURATION_ENABLE))
> + return qcom_scm_ice_invalidate_key(slot);
> +
> + /* Only AES-256-XTS has been tested so far. */
> + cap = cq_host->crypto_cap_array[cfg->crypto_cap_idx];
> + if (cap.algorithm_id != CQHCI_CRYPTO_ALG_AES_XTS ||
> + cap.key_size != CQHCI_CRYPTO_KEY_SIZE_256) {
> + dev_err_ratelimited(dev,
> + "Unhandled crypto capability; algorithm_id=%d, key_size=%d\n",
> + cap.algorithm_id, cap.key_size);
> + return -EINVAL;
> + }
> +
> + memcpy(key.bytes, cfg->crypto_key, AES_256_XTS_KEY_SIZE);
> +
> + /*
> + * The SCM call byte-swaps the 32-bit words of the key. So we have to
> + * do the same, in order for the final key be correct.
> + */
> + for (i = 0; i < ARRAY_SIZE(key.words); i++)
> + __cpu_to_be32s(&key.words[i]);
> +
> + err = qcom_scm_ice_set_key(slot, key.bytes, AES_256_XTS_KEY_SIZE,
> + QCOM_SCM_ICE_CIPHER_AES_256_XTS,
> + cfg->data_unit_size);
> + memzero_explicit(&key, sizeof(key));
> + return err;
> +}
> +#else /* CONFIG_MMC_CRYPTO */
> +static inline struct clk *sdhci_msm_ice_get_clk(struct device *dev)
> +{
> + return NULL;
> +}
> +
> +static inline int sdhci_msm_ice_init(struct sdhci_msm_host *msm_host,
> + struct cqhci_host *cq_host)
> +{
> + return 0;
> +}
> +
> +static inline void sdhci_msm_ice_enable(struct sdhci_msm_host *msm_host)
> +{
> +}
> +
> +static inline int __maybe_unused
> +sdhci_msm_ice_resume(struct sdhci_msm_host *msm_host)
> +{
> + return 0;
> +}
> +#endif /* !CONFIG_MMC_CRYPTO */
> +
> /*****************************************************************************\
> * *
> * MSM Command Queue Engine (CQE) *
> @@ -1803,6 +2035,16 @@ static u32 sdhci_msm_cqe_irq(struct sdhci_host *host, u32 intmask)
> return 0;
> }
>
> +static void sdhci_msm_cqe_enable(struct mmc_host *mmc)
> +{
> + struct sdhci_host *host = mmc_priv(mmc);
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> +
> + sdhci_cqe_enable(mmc);
> + sdhci_msm_ice_enable(msm_host);
> +}
> +
> static void sdhci_msm_cqe_disable(struct mmc_host *mmc, bool recovery)
> {
> struct sdhci_host *host = mmc_priv(mmc);
> @@ -1835,8 +2077,11 @@ static void sdhci_msm_cqe_disable(struct mmc_host *mmc, bool recovery)
> }
>
> static const struct cqhci_host_ops sdhci_msm_cqhci_ops = {
> - .enable = sdhci_cqe_enable,
> + .enable = sdhci_msm_cqe_enable,
> .disable = sdhci_msm_cqe_disable,
> +#ifdef CONFIG_MMC_CRYPTO
> + .program_key = sdhci_msm_program_key,
> +#endif
> };
>
> static int sdhci_msm_cqe_add_host(struct sdhci_host *host,
> @@ -1872,6 +2117,10 @@ static int sdhci_msm_cqe_add_host(struct sdhci_host *host,
>
> dma64 = host->flags & SDHCI_USE_64_BIT_DMA;
>
> + ret = sdhci_msm_ice_init(msm_host, cq_host);
> + if (ret)
> + goto cleanup;
> +
> ret = cqhci_init(cq_host, host->mmc, dma64);
> if (ret) {
> dev_err(&pdev->dev, "%s: CQE init: failed (%d)\n",
> @@ -2321,6 +2570,11 @@ static int sdhci_msm_probe(struct platform_device *pdev)
> clk = NULL;
> msm_host->bulk_clks[3].clk = clk;
>
> + clk = sdhci_msm_ice_get_clk(&pdev->dev);
> + if (IS_ERR(clk))
> + clk = NULL;
> + msm_host->bulk_clks[4].clk = clk;
> +
> ret = clk_bulk_prepare_enable(ARRAY_SIZE(msm_host->bulk_clks),
> msm_host->bulk_clks);
> if (ret)
> @@ -2531,12 +2785,15 @@ static __maybe_unused int sdhci_msm_runtime_resume(struct device *dev)
> * Whenever core-clock is gated dynamically, it's needed to
> * restore the SDR DLL settings when the clock is ungated.
> */
> - if (msm_host->restore_dll_config && msm_host->clk_rate)
> + if (msm_host->restore_dll_config && msm_host->clk_rate) {
> ret = sdhci_msm_restore_sdr_dll_config(host);
> + if (ret)
> + return ret;
> + }
>
> dev_pm_opp_set_rate(dev, msm_host->clk_rate);
>
> - return ret;
> + return sdhci_msm_ice_resume(msm_host);
> }
Doesn't this modify existing behaviour if
sdhci_msm_restore_sdr_dll_config() returns a non-zero value? Previously,
dev_pm_opp_set_rate() would always be called regardless of ret, but now
it's not called on non-zero ret value.
>
> static const struct dev_pm_ops sdhci_msm_pm_ops = {
> --
> 2.29.2
>
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 9/9] mmc: sdhci-msm: add Inline Crypto Engine support
2020-12-05 12:09 ` Satya Tangirala
@ 2020-12-05 19:43 ` Eric Biggers
2020-12-08 23:58 ` Satya Tangirala
0 siblings, 1 reply; 28+ messages in thread
From: Eric Biggers @ 2020-12-05 19:43 UTC (permalink / raw)
To: Satya Tangirala
Cc: linux-mmc, linux-arm-msm, devicetree, linux-fscrypt, Ulf Hansson,
Andy Gross, Bjorn Andersson, Adrian Hunter, Asutosh Das,
Rob Herring, Neeraj Soni, Barani Muthukumaran, Peng Zhou,
Stanley Chu, Konrad Dybcio
On Sat, Dec 05, 2020 at 12:09:16PM +0000, Satya Tangirala wrote:
> > +static void sdhci_msm_ice_enable(struct sdhci_msm_host *msm_host)
> > +{
> > + if (!(msm_host->mmc->caps2 & MMC_CAP2_CRYPTO))
> > + return;
> > + sdhci_msm_ice_low_power_mode_enable(msm_host);
> > + sdhci_msm_ice_optimization_enable(msm_host);
> > + sdhci_msm_ice_wait_bist_status(msm_host);
> If sdhci_msm_ice_wait_bist_status() fails, should we really ignore the
> error and continue en/decrypting with ICE? I'm not sure what the BIST
> failing might really mean, but if it means it's possible that the ICE
> en/decrypts incorrectly it would be bad to continue to use it.....
The "built-in self-test" that the ICE hardware does seems to be a FIPS
compliance thing which never actually fails in practice.
If it does fail, then according to
https://csrc.nist.gov/CSRC/media/projects/cryptographic-module-validation-program/documents/security-policies/140sp2588.pdf
(which is the closest thing I have to any documentation for ICE, other than the
eMMC standard), then the hardware itself will reject any crypto requests. So
rejecting them in software too should be redundant.
It's also worth noting that just because a hardware-level self-test passes
doesn't mean that the actual end-to-end storage encryption is working correctly.
To verify that you need to run something like Android's
vts_kernel_encryption_test, or the ciphertext verification tests in xfstests.
The hardware itself is really the wrong place to be testing the encryption.
It would be possible to add some code that sets a flag in the cqhci_host if the
ICE hardware test fails, and make cqhci_request() fail any crypto-enabled
requests if that flag is set. It just doesn't seem necessary, and I think we
should error on the side of less complexity for now.
What I was actually worried about is what happens if ICE needs to be used but
its self-test is still running, so it doesn't want to accept requests yet. I'm
not sure that's really a thing or not (one might hope the MMC host doesn't say
it's done resetting until the ICE tests are done), but that's why I left in the
code that waits for the tests to complete, which the downstream driver had.
Neeraj and Barani, if you have any additional insight or suggestions on this, or
know of anything I may be overlooking, that would be greatly appreciated.
Otherwise I just plan to add a comment that summarizes what I said above.
> > @@ -2531,12 +2785,15 @@ static __maybe_unused int sdhci_msm_runtime_resume(struct device *dev)
> > * Whenever core-clock is gated dynamically, it's needed to
> > * restore the SDR DLL settings when the clock is ungated.
> > */
> > - if (msm_host->restore_dll_config && msm_host->clk_rate)
> > + if (msm_host->restore_dll_config && msm_host->clk_rate) {
> > ret = sdhci_msm_restore_sdr_dll_config(host);
> > + if (ret)
> > + return ret;
> > + }
> >
> > dev_pm_opp_set_rate(dev, msm_host->clk_rate);
> >
> > - return ret;
> > + return sdhci_msm_ice_resume(msm_host);
> > }
> Doesn't this modify existing behaviour if
> sdhci_msm_restore_sdr_dll_config() returns a non-zero value? Previously,
> dev_pm_opp_set_rate() would always be called regardless of ret, but now
> it's not called on non-zero ret value.
Yes but I don't think it matters. IIUC, if a device's ->runtime_resume()
callback fails, then Linux's runtime power management framework keeps the device
in an error state and doesn't consider it to be resumed.
So if resuming a device involves N different things, and one of them fails, I
don't think we need to worry about trying to still do the other N-1 things; we
can just return an error on the first failure.
- Eric
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 9/9] mmc: sdhci-msm: add Inline Crypto Engine support
2020-12-05 19:43 ` Eric Biggers
@ 2020-12-08 23:58 ` Satya Tangirala
0 siblings, 0 replies; 28+ messages in thread
From: Satya Tangirala @ 2020-12-08 23:58 UTC (permalink / raw)
To: Eric Biggers
Cc: linux-mmc, linux-arm-msm, devicetree, linux-fscrypt, Ulf Hansson,
Andy Gross, Bjorn Andersson, Adrian Hunter, Asutosh Das,
Rob Herring, Neeraj Soni, Barani Muthukumaran, Peng Zhou,
Stanley Chu, Konrad Dybcio
On Sat, Dec 05, 2020 at 11:43:21AM -0800, Eric Biggers wrote:
> On Sat, Dec 05, 2020 at 12:09:16PM +0000, Satya Tangirala wrote:
> > > +static void sdhci_msm_ice_enable(struct sdhci_msm_host *msm_host)
> > > +{
> > > + if (!(msm_host->mmc->caps2 & MMC_CAP2_CRYPTO))
> > > + return;
> > > + sdhci_msm_ice_low_power_mode_enable(msm_host);
> > > + sdhci_msm_ice_optimization_enable(msm_host);
> > > + sdhci_msm_ice_wait_bist_status(msm_host);
> > If sdhci_msm_ice_wait_bist_status() fails, should we really ignore the
> > error and continue en/decrypting with ICE? I'm not sure what the BIST
> > failing might really mean, but if it means it's possible that the ICE
> > en/decrypts incorrectly it would be bad to continue to use it.....
>
> The "built-in self-test" that the ICE hardware does seems to be a FIPS
> compliance thing which never actually fails in practice.
>
> If it does fail, then according to
> https://csrc.nist.gov/CSRC/media/projects/cryptographic-module-validation-program/documents/security-policies/140sp2588.pdf
> (which is the closest thing I have to any documentation for ICE, other than the
> eMMC standard), then the hardware itself will reject any crypto requests. So
> rejecting them in software too should be redundant.
>
> It's also worth noting that just because a hardware-level self-test passes
> doesn't mean that the actual end-to-end storage encryption is working correctly.
> To verify that you need to run something like Android's
> vts_kernel_encryption_test, or the ciphertext verification tests in xfstests.
> The hardware itself is really the wrong place to be testing the encryption.
>
> It would be possible to add some code that sets a flag in the cqhci_host if the
> ICE hardware test fails, and make cqhci_request() fail any crypto-enabled
> requests if that flag is set. It just doesn't seem necessary, and I think we
> should error on the side of less complexity for now.
>
> What I was actually worried about is what happens if ICE needs to be used but
> its self-test is still running, so it doesn't want to accept requests yet. I'm
> not sure that's really a thing or not (one might hope the MMC host doesn't say
> it's done resetting until the ICE tests are done), but that's why I left in the
> code that waits for the tests to complete, which the downstream driver had.
>
> Neeraj and Barani, if you have any additional insight or suggestions on this, or
> know of anything I may be overlooking, that would be greatly appreciated.
>
> Otherwise I just plan to add a comment that summarizes what I said above.
>
Sure, sounds good to me :).
> > > @@ -2531,12 +2785,15 @@ static __maybe_unused int sdhci_msm_runtime_resume(struct device *dev)
> > > * Whenever core-clock is gated dynamically, it's needed to
> > > * restore the SDR DLL settings when the clock is ungated.
> > > */
> > > - if (msm_host->restore_dll_config && msm_host->clk_rate)
> > > + if (msm_host->restore_dll_config && msm_host->clk_rate) {
> > > ret = sdhci_msm_restore_sdr_dll_config(host);
> > > + if (ret)
> > > + return ret;
> > > + }
> > >
> > > dev_pm_opp_set_rate(dev, msm_host->clk_rate);
> > >
> > > - return ret;
> > > + return sdhci_msm_ice_resume(msm_host);
> > > }
> > Doesn't this modify existing behaviour if
> > sdhci_msm_restore_sdr_dll_config() returns a non-zero value? Previously,
> > dev_pm_opp_set_rate() would always be called regardless of ret, but now
> > it's not called on non-zero ret value.
>
> Yes but I don't think it matters. IIUC, if a device's ->runtime_resume()
> callback fails, then Linux's runtime power management framework keeps the device
> in an error state and doesn't consider it to be resumed.
>
> So if resuming a device involves N different things, and one of them fails, I
> don't think we need to worry about trying to still do the other N-1 things; we
> can just return an error on the first failure.
>
Ah, alright. Once you do add the comment you mentioned above, please
feel free to add
Reviewed-by: Satya Tangirala <satyat@google.com>
> - Eric
^ permalink raw reply [flat|nested] 28+ messages in thread