[PATCH v4 0/3] Inline Encryption Support for UFS

[PATCH v4 0/3] Inline Encryption Support for UFS

[Satya Tangirala]

This patch series adds support for inline encryption to UFS using
the inline encryption support in the block layer. It follows the JEDEC
UFSHCI v2.1 specification, which defines inline encryption for UFS.

This patch series previously went through a number of iterations as
part of the "Inline Encryption Support" patchset (last version was v13:
https://lkml.kernel.org/r/20200514003727.69001-1-satyat@google.com).
This patch series is rebased on v5.8-rc4.

Patch 1 introduces the crypto registers and struct definitions defined
in the UFSHCI v2.1 spec.

Patch 2 introduces functions to manipulate the UFS inline encryption
hardware (again in line with the UFSHCI v2.1 spec) via the block
layer keyslot manager. Device specific drivers must set the
UFSHCD_CAP_CRYPTO in hba->caps before ufshcd_hba_init_crypto is called
to opt-in to inline encryption support.

Patch 3 wires up ufshcd.c with the UFS crypto API introduced in Patch 2.

This patch series has been tested on some Qualcomm chipsets (on the
db845c, sm8150-mtp and sm8250-mtp) using some additional patches at
https://lkml.kernel.org/linux-scsi/20200501045111.665881-1-ebiggers@kernel.org/
and on some Mediatek chipsets using the additional patch in
https://lkml.kernel.org/linux-scsi/20200304022101.14165-1-stanley.chu@mediatek.com/.
These additional patches are required because these chipsets need certain
additional behaviour not specified within the UFSHCI v2.1 spec.

Thanks a lot to all the folks who tested this out!

Changes v3 => v4:
 - fix incorrect patch folding
 - some cleanups from Eric

Changes v2 => v3:
 - introduce ufshcd_prepare_req_desc_hdr_crypto to clean up code slightly
 - split up ufshcd_hba_init_crypto into ufshcd_hba_init_crypto_capabilities
   and ufshcd_init_crypto. The first function is called from
   ufshcd_hba_capabilities, and only reads crypto capabilities from device
   registers and sets up appropriate crypto structures. The second function
   is called from ufshcd_init, and actually initializes the inline crypto
   hardware.

Changes v1 => v2
 - handle OCS_DEVICE_FATAL_ERROR explicitly in ufshcd_transfer_rsp_status

Satya Tangirala (3):
  scsi: ufs: UFS driver v2.1 spec crypto additions
  scsi: ufs: UFS crypto API
  scsi: ufs: Add inline encryption support to UFS

 drivers/scsi/ufs/Kconfig         |   9 ++
 drivers/scsi/ufs/Makefile        |   1 +
 drivers/scsi/ufs/ufshcd-crypto.c | 238 +++++++++++++++++++++++++++++++
 drivers/scsi/ufs/ufshcd-crypto.h |  77 ++++++++++
 drivers/scsi/ufs/ufshcd.c        |  49 ++++++-
 drivers/scsi/ufs/ufshcd.h        |  24 ++++
 drivers/scsi/ufs/ufshci.h        |  67 ++++++++-
 7 files changed, 456 insertions(+), 9 deletions(-)
 create mode 100644 drivers/scsi/ufs/ufshcd-crypto.c
 create mode 100644 drivers/scsi/ufs/ufshcd-crypto.h

-- 
2.27.0.212.ge8ba1cc988-goog

[PATCH v4 1/3] scsi: ufs: UFS driver v2.1 spec crypto additions

[Satya Tangirala]

Add the crypto registers and structs defined in v2.1 of the JEDEC UFSHCI
specification in preparation to add support for inline encryption to
UFS.

Signed-off-by: Satya Tangirala <satyat@google.com>
Reviewed-by: Eric Biggers <ebiggers@google.com>
Reviewed-by: Stanley Chu <stanley.chu@mediatek.com>
---
 drivers/scsi/ufs/ufshcd.c |  3 ++
 drivers/scsi/ufs/ufshcd.h |  6 ++++
 drivers/scsi/ufs/ufshci.h | 67 +++++++++++++++++++++++++++++++++++++--
 3 files changed, 74 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index ad4fc829cbb2..4fdb200de46c 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -4792,6 +4792,9 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
 	case OCS_MISMATCH_RESP_UPIU_SIZE:
 	case OCS_PEER_COMM_FAILURE:
 	case OCS_FATAL_ERROR:
+	case OCS_DEVICE_FATAL_ERROR:
+	case OCS_INVALID_CRYPTO_CONFIG:
+	case OCS_GENERAL_CRYPTO_ERROR:
 	default:
 		result |= DID_ERROR << 16;
 		dev_err(hba->dev,
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index bf97d616e597..b4981f1c37a2 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -564,6 +564,12 @@ enum ufshcd_caps {
 	 * provisioned to be used. This would increase the write performance.
 	 */
 	UFSHCD_CAP_WB_EN				= 1 << 7,
+
+	/*
+	 * This capability allows the host controller driver to use the
+	 * inline crypto engine, if it is present
+	 */
+	UFSHCD_CAP_CRYPTO				= 1 << 8,
 };
 
 struct ufs_hba_variant_params {
diff --git a/drivers/scsi/ufs/ufshci.h b/drivers/scsi/ufs/ufshci.h
index c2961d37cc1c..c0651fe6dbbc 100644
--- a/drivers/scsi/ufs/ufshci.h
+++ b/drivers/scsi/ufs/ufshci.h
@@ -90,6 +90,7 @@ enum {
 	MASK_64_ADDRESSING_SUPPORT		= 0x01000000,
 	MASK_OUT_OF_ORDER_DATA_DELIVERY_SUPPORT	= 0x02000000,
 	MASK_UIC_DME_TEST_MODE_SUPPORT		= 0x04000000,
+	MASK_CRYPTO_SUPPORT			= 0x10000000,
 };
 
 #define UFS_MASK(mask, offset)		((mask) << (offset))
@@ -143,6 +144,7 @@ enum {
 #define DEVICE_FATAL_ERROR			0x800
 #define CONTROLLER_FATAL_ERROR			0x10000
 #define SYSTEM_BUS_FATAL_ERROR			0x20000
+#define CRYPTO_ENGINE_FATAL_ERROR		0x40000
 
 #define UFSHCD_UIC_HIBERN8_MASK	(UIC_HIBERNATE_ENTER |\
 				UIC_HIBERNATE_EXIT)
@@ -155,11 +157,13 @@ enum {
 #define UFSHCD_ERROR_MASK	(UIC_ERROR |\
 				DEVICE_FATAL_ERROR |\
 				CONTROLLER_FATAL_ERROR |\
-				SYSTEM_BUS_FATAL_ERROR)
+				SYSTEM_BUS_FATAL_ERROR |\
+				CRYPTO_ENGINE_FATAL_ERROR)
 
 #define INT_FATAL_ERRORS	(DEVICE_FATAL_ERROR |\
 				CONTROLLER_FATAL_ERROR |\
-				SYSTEM_BUS_FATAL_ERROR)
+				SYSTEM_BUS_FATAL_ERROR |\
+				CRYPTO_ENGINE_FATAL_ERROR)
 
 /* HCS - Host Controller Status 30h */
 #define DEVICE_PRESENT				0x1
@@ -318,6 +322,61 @@ enum {
 	INTERRUPT_MASK_ALL_VER_21	= 0x71FFF,
 };
 
+/* CCAP - Crypto Capability 100h */
+union ufs_crypto_capabilities {
+	__le32 reg_val;
+	struct {
+		u8 num_crypto_cap;
+		u8 config_count;
+		u8 reserved;
+		u8 config_array_ptr;
+	};
+};
+
+enum ufs_crypto_key_size {
+	UFS_CRYPTO_KEY_SIZE_INVALID	= 0x0,
+	UFS_CRYPTO_KEY_SIZE_128		= 0x1,
+	UFS_CRYPTO_KEY_SIZE_192		= 0x2,
+	UFS_CRYPTO_KEY_SIZE_256		= 0x3,
+	UFS_CRYPTO_KEY_SIZE_512		= 0x4,
+};
+
+enum ufs_crypto_alg {
+	UFS_CRYPTO_ALG_AES_XTS			= 0x0,
+	UFS_CRYPTO_ALG_BITLOCKER_AES_CBC	= 0x1,
+	UFS_CRYPTO_ALG_AES_ECB			= 0x2,
+	UFS_CRYPTO_ALG_ESSIV_AES_CBC		= 0x3,
+};
+
+/* x-CRYPTOCAP - Crypto Capability X */
+union ufs_crypto_cap_entry {
+	__le32 reg_val;
+	struct {
+		u8 algorithm_id;
+		u8 sdus_mask; /* Supported data unit size mask */
+		u8 key_size;
+		u8 reserved;
+	};
+};
+
+#define UFS_CRYPTO_CONFIGURATION_ENABLE (1 << 7)
+#define UFS_CRYPTO_KEY_MAX_SIZE 64
+/* x-CRYPTOCFG - Crypto Configuration X */
+union ufs_crypto_cfg_entry {
+	__le32 reg_val[32];
+	struct {
+		u8 crypto_key[UFS_CRYPTO_KEY_MAX_SIZE];
+		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];
+	};
+};
+
 /*
  * Request Descriptor Definitions
  */
@@ -339,6 +398,7 @@ enum {
 	UTP_NATIVE_UFS_COMMAND		= 0x10000000,
 	UTP_DEVICE_MANAGEMENT_FUNCTION	= 0x20000000,
 	UTP_REQ_DESC_INT_CMD		= 0x01000000,
+	UTP_REQ_DESC_CRYPTO_ENABLE_CMD	= 0x00800000,
 };
 
 /* UTP Transfer Request Data Direction (DD) */
@@ -358,6 +418,9 @@ enum {
 	OCS_PEER_COMM_FAILURE		= 0x5,
 	OCS_ABORTED			= 0x6,
 	OCS_FATAL_ERROR			= 0x7,
+	OCS_DEVICE_FATAL_ERROR		= 0x8,
+	OCS_INVALID_CRYPTO_CONFIG	= 0x9,
+	OCS_GENERAL_CRYPTO_ERROR	= 0xA,
 	OCS_INVALID_COMMAND_STATUS	= 0x0F,
 	MASK_OCS			= 0x0F,
 };
-- 
2.27.0.212.ge8ba1cc988-goog

[PATCH v4 2/3] scsi: ufs: UFS crypto API

[Satya Tangirala]

Introduce functions to manipulate UFS inline encryption hardware
in line with the JEDEC UFSHCI v2.1 specification and to work with the
block keyslot manager.

The UFS crypto API will assume by default that a vendor driver doesn't
support UFS crypto, even if the hardware advertises the capability, because
a lot of hardware requires some special handling that's not specified in
the aforementioned JEDEC spec. Each vendor driver must explicitly set
hba->caps |= UFSHCD_CAP_CRYPTO before ufshcd_hba_init_crypto is called to
opt-in to UFS crypto support.

Signed-off-by: Satya Tangirala <satyat@google.com>
Reviewed-by: Eric Biggers <ebiggers@google.com>
Reviewed-by: Stanley Chu <stanley.chu@mediatek.com>
---
 drivers/scsi/ufs/Kconfig         |   9 ++
 drivers/scsi/ufs/Makefile        |   1 +
 drivers/scsi/ufs/ufshcd-crypto.c | 238 +++++++++++++++++++++++++++++++
 drivers/scsi/ufs/ufshcd-crypto.h |  46 ++++++
 drivers/scsi/ufs/ufshcd.h        |  12 ++
 5 files changed, 306 insertions(+)
 create mode 100644 drivers/scsi/ufs/ufshcd-crypto.c
 create mode 100644 drivers/scsi/ufs/ufshcd-crypto.h

diff --git a/drivers/scsi/ufs/Kconfig b/drivers/scsi/ufs/Kconfig
index d35378be89e8..4c0a9661049a 100644
--- a/drivers/scsi/ufs/Kconfig
+++ b/drivers/scsi/ufs/Kconfig
@@ -160,3 +160,12 @@ config SCSI_UFS_BSG
 
 	  Select this if you need a bsg device node for your UFS controller.
 	  If unsure, say N.
+
+config SCSI_UFS_CRYPTO
+	bool "UFS Crypto Engine Support"
+	depends on SCSI_UFSHCD && BLK_INLINE_ENCRYPTION
+	help
+	  Enable Crypto Engine Support in UFS.
+	  Enabling this makes it possible for the kernel to use the crypto
+	  capabilities of the UFS device (if present) to perform crypto
+	  operations on data being transferred to/from the device.
diff --git a/drivers/scsi/ufs/Makefile b/drivers/scsi/ufs/Makefile
index 94c6c5d7334b..197e178f44bc 100644
--- a/drivers/scsi/ufs/Makefile
+++ b/drivers/scsi/ufs/Makefile
@@ -7,6 +7,7 @@ obj-$(CONFIG_SCSI_UFS_QCOM) += ufs-qcom.o
 obj-$(CONFIG_SCSI_UFSHCD) += ufshcd-core.o
 ufshcd-core-y				+= ufshcd.o ufs-sysfs.o
 ufshcd-core-$(CONFIG_SCSI_UFS_BSG)	+= ufs_bsg.o
+ufshcd-core-$(CONFIG_SCSI_UFS_CRYPTO) += ufshcd-crypto.o
 obj-$(CONFIG_SCSI_UFSHCD_PCI) += ufshcd-pci.o
 obj-$(CONFIG_SCSI_UFSHCD_PLATFORM) += ufshcd-pltfrm.o
 obj-$(CONFIG_SCSI_UFS_HISI) += ufs-hisi.o
diff --git a/drivers/scsi/ufs/ufshcd-crypto.c b/drivers/scsi/ufs/ufshcd-crypto.c
new file mode 100644
index 000000000000..98ff87c38aa7
--- /dev/null
+++ b/drivers/scsi/ufs/ufshcd-crypto.c
@@ -0,0 +1,238 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2019 Google LLC
+ */
+
+#include "ufshcd.h"
+#include "ufshcd-crypto.h"
+
+/* Blk-crypto modes supported by UFS crypto */
+static const struct ufs_crypto_alg_entry {
+	enum ufs_crypto_alg ufs_alg;
+	enum ufs_crypto_key_size ufs_key_size;
+} ufs_crypto_algs[BLK_ENCRYPTION_MODE_MAX] = {
+	[BLK_ENCRYPTION_MODE_AES_256_XTS] = {
+		.ufs_alg = UFS_CRYPTO_ALG_AES_XTS,
+		.ufs_key_size = UFS_CRYPTO_KEY_SIZE_256,
+	},
+};
+
+static void ufshcd_program_key(struct ufs_hba *hba,
+			       const union ufs_crypto_cfg_entry *cfg,
+			       int slot)
+{
+	int i;
+	u32 slot_offset = hba->crypto_cfg_register + slot * sizeof(*cfg);
+
+	ufshcd_hold(hba, false);
+	/* Ensure that CFGE is cleared before programming the key */
+	ufshcd_writel(hba, 0, slot_offset + 16 * sizeof(cfg->reg_val[0]));
+	for (i = 0; i < 16; i++) {
+		ufshcd_writel(hba, le32_to_cpu(cfg->reg_val[i]),
+			      slot_offset + i * sizeof(cfg->reg_val[0]));
+	}
+	/* Write dword 17 */
+	ufshcd_writel(hba, le32_to_cpu(cfg->reg_val[17]),
+		      slot_offset + 17 * sizeof(cfg->reg_val[0]));
+	/* Dword 16 must be written last */
+	ufshcd_writel(hba, le32_to_cpu(cfg->reg_val[16]),
+		      slot_offset + 16 * sizeof(cfg->reg_val[0]));
+	ufshcd_release(hba);
+}
+
+static int ufshcd_crypto_keyslot_program(struct blk_keyslot_manager *ksm,
+					 const struct blk_crypto_key *key,
+					 unsigned int slot)
+{
+	struct ufs_hba *hba = container_of(ksm, struct ufs_hba, ksm);
+	const union ufs_crypto_cap_entry *ccap_array = hba->crypto_cap_array;
+	const struct ufs_crypto_alg_entry *alg =
+			&ufs_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 ufs_crypto_cfg_entry cfg = { 0 };
+
+	BUILD_BUG_ON(UFS_CRYPTO_KEY_SIZE_INVALID != 0);
+	for (i = 0; i < hba->crypto_capabilities.num_crypto_cap; i++) {
+		if (ccap_array[i].algorithm_id == alg->ufs_alg &&
+		    ccap_array[i].key_size == alg->ufs_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 = UFS_CRYPTO_CONFIGURATION_ENABLE;
+
+	if (ccap_array[cap_idx].algorithm_id == UFS_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 + UFS_CRYPTO_KEY_MAX_SIZE/2,
+		       key->raw + key->size/2, key->size/2);
+	} else {
+		memcpy(cfg.crypto_key, key->raw, key->size);
+	}
+
+	ufshcd_program_key(hba, &cfg, slot);
+
+	memzero_explicit(&cfg, sizeof(cfg));
+	return 0;
+}
+
+static void ufshcd_clear_keyslot(struct ufs_hba *hba, int slot)
+{
+	/*
+	 * Clear the crypto cfg on the device. Clearing CFGE
+	 * might not be sufficient, so just clear the entire cfg.
+	 */
+	union ufs_crypto_cfg_entry cfg = { 0 };
+
+	ufshcd_program_key(hba, &cfg, slot);
+}
+
+static int ufshcd_crypto_keyslot_evict(struct blk_keyslot_manager *ksm,
+				       const struct blk_crypto_key *key,
+				       unsigned int slot)
+{
+	struct ufs_hba *hba = container_of(ksm, struct ufs_hba, ksm);
+
+	ufshcd_clear_keyslot(hba, slot);
+
+	return 0;
+}
+
+bool ufshcd_crypto_enable(struct ufs_hba *hba)
+{
+	if (!(hba->caps & UFSHCD_CAP_CRYPTO))
+		return false;
+
+	/* Reset might clear all keys, so reprogram all the keys. */
+	blk_ksm_reprogram_all_keys(&hba->ksm);
+	return true;
+}
+
+static const struct blk_ksm_ll_ops ufshcd_ksm_ops = {
+	.keyslot_program	= ufshcd_crypto_keyslot_program,
+	.keyslot_evict		= ufshcd_crypto_keyslot_evict,
+};
+
+static enum blk_crypto_mode_num
+ufshcd_find_blk_crypto_mode(union ufs_crypto_cap_entry cap)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(ufs_crypto_algs); i++) {
+		BUILD_BUG_ON(UFS_CRYPTO_KEY_SIZE_INVALID != 0);
+		if (ufs_crypto_algs[i].ufs_alg == cap.algorithm_id &&
+		    ufs_crypto_algs[i].ufs_key_size == cap.key_size) {
+			return i;
+		}
+	}
+	return BLK_ENCRYPTION_MODE_INVALID;
+}
+
+/**
+ * ufshcd_hba_init_crypto_capabilities - Read crypto capabilities, init crypto
+ *					 fields in hba
+ * @hba: Per adapter instance
+ *
+ * Return: 0 if crypto was initialized or is not supported, else a -errno value.
+ */
+int ufshcd_hba_init_crypto_capabilities(struct ufs_hba *hba)
+{
+	int cap_idx;
+	int err = 0;
+	enum blk_crypto_mode_num blk_mode_num;
+
+	/*
+	 * Don't use crypto if either the hardware doesn't advertise the
+	 * standard crypto capability bit *or* if the vendor specific driver
+	 * hasn't advertised that crypto is supported.
+	 */
+	if (!(hba->capabilities & MASK_CRYPTO_SUPPORT) ||
+	    !(hba->caps & UFSHCD_CAP_CRYPTO))
+		goto out;
+
+	hba->crypto_capabilities.reg_val =
+			cpu_to_le32(ufshcd_readl(hba, REG_UFS_CCAP));
+	hba->crypto_cfg_register =
+		(u32)hba->crypto_capabilities.config_array_ptr * 0x100;
+	hba->crypto_cap_array =
+		devm_kcalloc(hba->dev, hba->crypto_capabilities.num_crypto_cap,
+			     sizeof(hba->crypto_cap_array[0]), GFP_KERNEL);
+	if (!hba->crypto_cap_array) {
+		err = -ENOMEM;
+		goto out;
+	}
+
+	/* The actual number of configurations supported is (CFGC+1) */
+	err = blk_ksm_init(&hba->ksm,
+			   hba->crypto_capabilities.config_count + 1);
+	if (err)
+		goto out_free_caps;
+
+	hba->ksm.ksm_ll_ops = ufshcd_ksm_ops;
+	/* UFS only supports 8 bytes for any DUN */
+	hba->ksm.max_dun_bytes_supported = 8;
+	hba->ksm.dev = hba->dev;
+
+	/*
+	 * Cache all the UFS crypto capabilities and advertise the supported
+	 * crypto modes and data unit sizes to the block layer.
+	 */
+	for (cap_idx = 0; cap_idx < hba->crypto_capabilities.num_crypto_cap;
+	     cap_idx++) {
+		hba->crypto_cap_array[cap_idx].reg_val =
+			cpu_to_le32(ufshcd_readl(hba,
+						 REG_UFS_CRYPTOCAP +
+						 cap_idx * sizeof(__le32)));
+		blk_mode_num = ufshcd_find_blk_crypto_mode(
+						hba->crypto_cap_array[cap_idx]);
+		if (blk_mode_num != BLK_ENCRYPTION_MODE_INVALID)
+			hba->ksm.crypto_modes_supported[blk_mode_num] |=
+				hba->crypto_cap_array[cap_idx].sdus_mask * 512;
+	}
+
+	return 0;
+
+out_free_caps:
+	devm_kfree(hba->dev, hba->crypto_cap_array);
+out:
+	/* Indicate that init failed by clearing UFSHCD_CAP_CRYPTO */
+	hba->caps &= ~UFSHCD_CAP_CRYPTO;
+	return err;
+}
+
+/**
+ * ufshcd_init_crypto - Initialize crypto hardware
+ * @hba: Per adapter instance
+ */
+void ufshcd_init_crypto(struct ufs_hba *hba)
+{
+	int slot;
+
+	if (!(hba->caps & UFSHCD_CAP_CRYPTO))
+		return;
+
+	/* Clear all keyslots - the number of keyslots is (CFGC + 1) */
+	for (slot = 0; slot < hba->crypto_capabilities.config_count + 1; slot++)
+		ufshcd_clear_keyslot(hba, slot);
+}
+
+void ufshcd_crypto_setup_rq_keyslot_manager(struct ufs_hba *hba,
+					    struct request_queue *q)
+{
+	if (hba->caps & UFSHCD_CAP_CRYPTO)
+		blk_ksm_register(&hba->ksm, q);
+}
+
+void ufshcd_crypto_destroy_keyslot_manager(struct ufs_hba *hba)
+{
+	blk_ksm_destroy(&hba->ksm);
+}
diff --git a/drivers/scsi/ufs/ufshcd-crypto.h b/drivers/scsi/ufs/ufshcd-crypto.h
new file mode 100644
index 000000000000..cbc58b4f5df7
--- /dev/null
+++ b/drivers/scsi/ufs/ufshcd-crypto.h
@@ -0,0 +1,46 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright 2019 Google LLC
+ */
+
+#ifndef _UFSHCD_CRYPTO_H
+#define _UFSHCD_CRYPTO_H
+
+#ifdef CONFIG_SCSI_UFS_CRYPTO
+#include "ufshcd.h"
+#include "ufshci.h"
+
+bool ufshcd_crypto_enable(struct ufs_hba *hba);
+
+int ufshcd_hba_init_crypto_capabilities(struct ufs_hba *hba);
+
+void ufshcd_init_crypto(struct ufs_hba *hba);
+
+void ufshcd_crypto_setup_rq_keyslot_manager(struct ufs_hba *hba,
+					    struct request_queue *q);
+
+void ufshcd_crypto_destroy_keyslot_manager(struct ufs_hba *hba);
+
+#else /* CONFIG_SCSI_UFS_CRYPTO */
+
+static inline bool ufshcd_crypto_enable(struct ufs_hba *hba)
+{
+	return false;
+}
+
+static inline int ufshcd_hba_init_crypto_capabilities(struct ufs_hba *hba)
+{
+	return 0;
+}
+
+static inline void ufshcd_init_crypto(struct ufs_hba *hba) { }
+
+static inline void ufshcd_crypto_setup_rq_keyslot_manager(struct ufs_hba *hba,
+						struct request_queue *q) { }
+
+static inline void ufshcd_crypto_destroy_keyslot_manager(struct ufs_hba *hba)
+{ }
+
+#endif /* CONFIG_SCSI_UFS_CRYPTO */
+
+#endif /* _UFSHCD_CRYPTO_H */
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index b4981f1c37a2..271fc19f8002 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -57,6 +57,7 @@
 #include <linux/regulator/consumer.h>
 #include <linux/bitfield.h>
 #include <linux/devfreq.h>
+#include <linux/keyslot-manager.h>
 #include "unipro.h"
 
 #include <asm/irq.h>
@@ -630,6 +631,10 @@ struct ufs_hba_variant_params {
  * @is_urgent_bkops_lvl_checked: keeps track if the urgent bkops level for
  *  device is known or not.
  * @scsi_block_reqs_cnt: reference counting for scsi block requests
+ * @crypto_capabilities: Content of crypto capabilities register (0x100)
+ * @crypto_cap_array: Array of crypto capabilities
+ * @crypto_cfg_register: Start of the crypto cfg array
+ * @ksm: the keyslot manager tied to this hba
  */
 struct ufs_hba {
 	void __iomem *mmio_base;
@@ -752,6 +757,13 @@ struct ufs_hba {
 	bool wb_buf_flush_enabled;
 	bool wb_enabled;
 	struct delayed_work rpm_dev_flush_recheck_work;
+
+#ifdef CONFIG_SCSI_UFS_CRYPTO
+	union ufs_crypto_capabilities crypto_capabilities;
+	union ufs_crypto_cap_entry *crypto_cap_array;
+	u32 crypto_cfg_register;
+	struct blk_keyslot_manager ksm;
+#endif
 };
 
 /* Returns true if clocks can be gated. Otherwise false */
-- 
2.27.0.212.ge8ba1cc988-goog

[PATCH v4 3/3] scsi: ufs: Add inline encryption support to UFS

[Satya Tangirala]

Wire up ufshcd.c with the UFS Crypto API, the block layer inline
encryption additions and the keyslot manager.

Many existing inline crypto devices require some additional behaviour not
specified in the UFSHCI v2.1 specification - as such the vendor specific
drivers will need to be updated where necessary to make it possible to use
those devices. Some of these changes have already been proposed upstream,
such as for the Qualcomm 845 SoC at
https://lkml.kernel.org/linux-scsi/20200501045111.665881-1-ebiggers@kernel.org/
and for ufs-mediatek at
https://lkml.kernel.org/linux-scsi/20200304022101.14165-1-stanley.chu@mediatek.com/

This patch has been tested on the db845c, sm8150-mtp and sm8250-mtp
(which have Qualcomm chipsets) and on some mediatek chipsets using these
aforementioned vendor specific driver updates.

Signed-off-by: Satya Tangirala <satyat@google.com>
Reviewed-by: Eric Biggers <ebiggers@google.com>
Reviewed-by: Stanley Chu <stanley.chu@mediatek.com>
---
 drivers/scsi/ufs/ufshcd-crypto.h | 31 +++++++++++++++++++++
 drivers/scsi/ufs/ufshcd.c        | 46 +++++++++++++++++++++++++++-----
 drivers/scsi/ufs/ufshcd.h        |  6 +++++
 3 files changed, 76 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd-crypto.h b/drivers/scsi/ufs/ufshcd-crypto.h
index cbc58b4f5df7..d53851be5541 100644
--- a/drivers/scsi/ufs/ufshcd-crypto.h
+++ b/drivers/scsi/ufs/ufshcd-crypto.h
@@ -10,6 +10,30 @@
 #include "ufshcd.h"
 #include "ufshci.h"
 
+static inline void ufshcd_prepare_lrbp_crypto(struct request *rq,
+					      struct ufshcd_lrb *lrbp)
+{
+	if (!rq || !rq->crypt_keyslot) {
+		lrbp->crypto_key_slot = -1;
+		return;
+	}
+
+	lrbp->crypto_key_slot = blk_ksm_get_slot_idx(rq->crypt_keyslot);
+	lrbp->data_unit_num = rq->crypt_ctx->bc_dun[0];
+}
+
+static inline void
+ufshcd_prepare_req_desc_hdr_crypto(struct ufshcd_lrb *lrbp, u32 *dword_0,
+				   u32 *dword_1, u32 *dword_3)
+{
+	if (lrbp->crypto_key_slot >= 0) {
+		*dword_0 |= UTP_REQ_DESC_CRYPTO_ENABLE_CMD;
+		*dword_0 |= lrbp->crypto_key_slot;
+		*dword_1 = lower_32_bits(lrbp->data_unit_num);
+		*dword_3 = upper_32_bits(lrbp->data_unit_num);
+	}
+}
+
 bool ufshcd_crypto_enable(struct ufs_hba *hba);
 
 int ufshcd_hba_init_crypto_capabilities(struct ufs_hba *hba);
@@ -23,6 +47,13 @@ void ufshcd_crypto_destroy_keyslot_manager(struct ufs_hba *hba);
 
 #else /* CONFIG_SCSI_UFS_CRYPTO */
 
+static inline void ufshcd_prepare_lrbp_crypto(struct request *rq,
+					      struct ufshcd_lrb *lrbp) { }
+
+static inline void
+ufshcd_prepare_req_desc_hdr_crypto(struct ufshcd_lrb *lrbp, u32 *dword_0,
+				   u32 *dword_1, u32 *dword_3) { }
+
 static inline bool ufshcd_crypto_enable(struct ufs_hba *hba)
 {
 	return false;
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 4fdb200de46c..c0259ebb4685 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -48,6 +48,7 @@
 #include "unipro.h"
 #include "ufs-sysfs.h"
 #include "ufs_bsg.h"
+#include "ufshcd-crypto.h"
 #include <asm/unaligned.h>
 #include <linux/blkdev.h>
 
@@ -839,7 +840,12 @@ static void ufshcd_enable_run_stop_reg(struct ufs_hba *hba)
  */
 static inline void ufshcd_hba_start(struct ufs_hba *hba)
 {
-	ufshcd_writel(hba, CONTROLLER_ENABLE, REG_CONTROLLER_ENABLE);
+	u32 val = CONTROLLER_ENABLE;
+
+	if (ufshcd_crypto_enable(hba))
+		val |= CRYPTO_GENERAL_ENABLE;
+
+	ufshcd_writel(hba, val, REG_CONTROLLER_ENABLE);
 }
 
 /**
@@ -1996,15 +2002,26 @@ int ufshcd_copy_query_response(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
 /**
  * ufshcd_hba_capabilities - Read controller capabilities
  * @hba: per adapter instance
+ *
+ * Return: 0 on success, negative on error.
  */
-static inline void ufshcd_hba_capabilities(struct ufs_hba *hba)
+static inline int ufshcd_hba_capabilities(struct ufs_hba *hba)
 {
+	int err;
+
 	hba->capabilities = ufshcd_readl(hba, REG_CONTROLLER_CAPABILITIES);
 
 	/* nutrs and nutmrs are 0 based values */
 	hba->nutrs = (hba->capabilities & MASK_TRANSFER_REQUESTS_SLOTS) + 1;
 	hba->nutmrs =
 	((hba->capabilities & MASK_TASK_MANAGEMENT_REQUEST_SLOTS) >> 16) + 1;
+
+	/* Read crypto capabilities */
+	err = ufshcd_hba_init_crypto_capabilities(hba);
+	if (err)
+		dev_err(hba->dev, "crypto setup failed\n");
+
+	return err;
 }
 
 /**
@@ -2237,6 +2254,8 @@ static void ufshcd_prepare_req_desc_hdr(struct ufshcd_lrb *lrbp,
 	struct utp_transfer_req_desc *req_desc = lrbp->utr_descriptor_ptr;
 	u32 data_direction;
 	u32 dword_0;
+	u32 dword_1 = 0;
+	u32 dword_3 = 0;
 
 	if (cmd_dir == DMA_FROM_DEVICE) {
 		data_direction = UTP_DEVICE_TO_HOST;
@@ -2254,10 +2273,12 @@ static void ufshcd_prepare_req_desc_hdr(struct ufshcd_lrb *lrbp,
 	if (lrbp->intr_cmd)
 		dword_0 |= UTP_REQ_DESC_INT_CMD;
 
+	/* Prepare crypto related dwords */
+	ufshcd_prepare_req_desc_hdr_crypto(lrbp, &dword_0, &dword_1, &dword_3);
+
 	/* Transfer request descriptor header fields */
 	req_desc->header.dword_0 = cpu_to_le32(dword_0);
-	/* dword_1 is reserved, hence it is set to 0 */
-	req_desc->header.dword_1 = 0;
+	req_desc->header.dword_1 = cpu_to_le32(dword_1);
 	/*
 	 * assigning invalid value for command status. Controller
 	 * updates OCS on command completion, with the command
@@ -2265,8 +2286,7 @@ static void ufshcd_prepare_req_desc_hdr(struct ufshcd_lrb *lrbp,
 	 */
 	req_desc->header.dword_2 =
 		cpu_to_le32(OCS_INVALID_COMMAND_STATUS);
-	/* dword_3 is reserved, hence it is set to 0 */
-	req_desc->header.dword_3 = 0;
+	req_desc->header.dword_3 = cpu_to_le32(dword_3);
 
 	req_desc->prd_table_length = 0;
 }
@@ -2521,6 +2541,9 @@ static int ufshcd_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd)
 	lrbp->task_tag = tag;
 	lrbp->lun = ufshcd_scsi_to_upiu_lun(cmd->device->lun);
 	lrbp->intr_cmd = !ufshcd_is_intr_aggr_allowed(hba) ? true : false;
+
+	ufshcd_prepare_lrbp_crypto(cmd->request, lrbp);
+
 	lrbp->req_abort_skip = false;
 
 	ufshcd_comp_scsi_upiu(hba, lrbp);
@@ -2554,6 +2577,7 @@ static int ufshcd_compose_dev_cmd(struct ufs_hba *hba,
 	lrbp->task_tag = tag;
 	lrbp->lun = 0; /* device management cmd is not specific to any LUN */
 	lrbp->intr_cmd = true; /* No interrupt aggregation */
+	ufshcd_prepare_lrbp_crypto(NULL, lrbp);
 	hba->dev_cmd.type = cmd_type;
 
 	return ufshcd_comp_devman_upiu(hba, lrbp);
@@ -4650,6 +4674,8 @@ static int ufshcd_slave_configure(struct scsi_device *sdev)
 	if (ufshcd_is_rpm_autosuspend_allowed(hba))
 		sdev->rpm_autosuspend = 1;
 
+	ufshcd_crypto_setup_rq_keyslot_manager(hba, q);
+
 	return 0;
 }
 
@@ -6115,6 +6141,7 @@ static int ufshcd_issue_devman_upiu_cmd(struct ufs_hba *hba,
 	lrbp->task_tag = tag;
 	lrbp->lun = 0;
 	lrbp->intr_cmd = true;
+	ufshcd_prepare_lrbp_crypto(NULL, lrbp);
 	hba->dev_cmd.type = cmd_type;
 
 	switch (hba->ufs_version) {
@@ -8662,6 +8689,7 @@ EXPORT_SYMBOL_GPL(ufshcd_remove);
  */
 void ufshcd_dealloc_host(struct ufs_hba *hba)
 {
+	ufshcd_crypto_destroy_keyslot_manager(hba);
 	scsi_host_put(hba->host);
 }
 EXPORT_SYMBOL_GPL(ufshcd_dealloc_host);
@@ -8762,7 +8790,9 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
 		goto out_error;
 
 	/* Read capabilities registers */
-	ufshcd_hba_capabilities(hba);
+	err = ufshcd_hba_capabilities(hba);
+	if (err)
+		goto out_disable;
 
 	/* Get UFS version supported by the controller */
 	hba->ufs_version = ufshcd_get_ufs_version(hba);
@@ -8872,6 +8902,8 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
 	/* Reset the attached device */
 	ufshcd_vops_device_reset(hba);
 
+	ufshcd_init_crypto(hba);
+
 	/* Host controller enable */
 	err = ufshcd_hba_enable(hba);
 	if (err) {
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 271fc19f8002..1cb0fde5772c 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -184,6 +184,8 @@ struct ufs_pm_lvl_states {
  * @intr_cmd: Interrupt command (doesn't participate in interrupt aggregation)
  * @issue_time_stamp: time stamp for debug purposes
  * @compl_time_stamp: time stamp for statistics
+ * @crypto_key_slot: the key slot to use for inline crypto (-1 if none)
+ * @data_unit_num: the data unit number for the first block for inline crypto
  * @req_abort_skip: skip request abort task flag
  */
 struct ufshcd_lrb {
@@ -208,6 +210,10 @@ struct ufshcd_lrb {
 	bool intr_cmd;
 	ktime_t issue_time_stamp;
 	ktime_t compl_time_stamp;
+#ifdef CONFIG_SCSI_UFS_CRYPTO
+	int crypto_key_slot;
+	u64 data_unit_num;
+#endif
 
 	bool req_abort_skip;
 };
-- 
2.27.0.212.ge8ba1cc988-goog

Re: [PATCH v4 0/3] Inline Encryption Support for UFS

[Eric Biggers]

On Mon, Jul 06, 2020 at 08:04:11PM +0000, Satya Tangirala wrote:
> This patch series adds support for inline encryption to UFS using
> the inline encryption support in the block layer. It follows the JEDEC
> UFSHCI v2.1 specification, which defines inline encryption for UFS.
> 
> This patch series previously went through a number of iterations as
> part of the "Inline Encryption Support" patchset (last version was v13:
> https://lkml.kernel.org/r/20200514003727.69001-1-satyat@google.com).
> This patch series is rebased on v5.8-rc4.
> 
> Patch 1 introduces the crypto registers and struct definitions defined
> in the UFSHCI v2.1 spec.
> 
> Patch 2 introduces functions to manipulate the UFS inline encryption
> hardware (again in line with the UFSHCI v2.1 spec) via the block
> layer keyslot manager. Device specific drivers must set the
> UFSHCD_CAP_CRYPTO in hba->caps before ufshcd_hba_init_crypto is called
> to opt-in to inline encryption support.

Note that it's now ufshcd_hba_init_crypto_capabilities(), not
ufshcd_hba_init_crypto().

> 
> Patch 3 wires up ufshcd.c with the UFS crypto API introduced in Patch 2.
> 
> This patch series has been tested on some Qualcomm chipsets (on the
> db845c, sm8150-mtp and sm8250-mtp) using some additional patches at
> https://lkml.kernel.org/linux-scsi/20200501045111.665881-1-ebiggers@kernel.org/
> and on some Mediatek chipsets using the additional patch in
> https://lkml.kernel.org/linux-scsi/20200304022101.14165-1-stanley.chu@mediatek.com/.
> These additional patches are required because these chipsets need certain
> additional behaviour not specified within the UFSHCI v2.1 spec.
> 
> Thanks a lot to all the folks who tested this out!
> 
> Changes v3 => v4:
>  - fix incorrect patch folding
>  - some cleanups from Eric
> 
> Changes v2 => v3:
>  - introduce ufshcd_prepare_req_desc_hdr_crypto to clean up code slightly
>  - split up ufshcd_hba_init_crypto into ufshcd_hba_init_crypto_capabilities
>    and ufshcd_init_crypto. The first function is called from
>    ufshcd_hba_capabilities, and only reads crypto capabilities from device
>    registers and sets up appropriate crypto structures. The second function
>    is called from ufshcd_init, and actually initializes the inline crypto
>    hardware.
> 
> Changes v1 => v2
>  - handle OCS_DEVICE_FATAL_ERROR explicitly in ufshcd_transfer_rsp_status
> 
> Satya Tangirala (3):
>   scsi: ufs: UFS driver v2.1 spec crypto additions
>   scsi: ufs: UFS crypto API
>   scsi: ufs: Add inline encryption support to UFS

These patches look good to me.  Avri and Alim, what do you think?
We'd like these to be applied for 5.9.

- Eric

RE: [PATCH v4 0/3] Inline Encryption Support for UFS

[Alim Akhtar]

Hi Satya,

> -----Original Message-----
> From: Satya Tangirala <satyat@google.com>
> Sent: 07 July 2020 01:34
> To: linux-scsi@vger.kernel.org; Avri Altman <avri.altman@wdc.com>; Alim
> Akhtar <alim.akhtar@samsung.com>
> Cc: Barani Muthukumaran <bmuthuku@qti.qualcomm.com>; Kuohong Wang
> <kuohong.wang@mediatek.com>; Kim Boojin <boojin.kim@samsung.com>;
> Satya Tangirala <satyat@google.com>
> Subject: [PATCH v4 0/3] Inline Encryption Support for UFS
> 
> This patch series adds support for inline encryption to UFS using the inline
> encryption support in the block layer. It follows the JEDEC UFSHCI v2.1
> specification, which defines inline encryption for UFS.
> 
> This patch series previously went through a number of iterations as part of the
> "Inline Encryption Support" patchset (last version was v13:
> https://lkml.kernel.org/r/20200514003727.69001-1-satyat@google.com).
> This patch series is rebased on v5.8-rc4.
> 
> Patch 1 introduces the crypto registers and struct definitions defined in the
> UFSHCI v2.1 spec.
> 
> Patch 2 introduces functions to manipulate the UFS inline encryption hardware
> (again in line with the UFSHCI v2.1 spec) via the block layer keyslot manager.
> Device specific drivers must set the UFSHCD_CAP_CRYPTO in hba->caps before
> ufshcd_hba_init_crypto is called to opt-in to inline encryption support.
> 
> Patch 3 wires up ufshcd.c with the UFS crypto API introduced in Patch 2.
> 
> This patch series has been tested on some Qualcomm chipsets (on the db845c,
> sm8150-mtp and sm8250-mtp) using some additional patches at
> https://lkml.kernel.org/linux-scsi/20200501045111.665881-1-
> ebiggers@kernel.org/
> and on some Mediatek chipsets using the additional patch in
> https://lkml.kernel.org/linux-scsi/20200304022101.14165-1-
> stanley.chu@mediatek.com/.
> These additional patches are required because these chipsets need certain
> additional behaviour not specified within the UFSHCI v2.1 spec.
> 
> Thanks a lot to all the folks who tested this out!
> 
> Changes v3 => v4:
>  - fix incorrect patch folding
>  - some cleanups from Eric
> 
> Changes v2 => v3:
>  - introduce ufshcd_prepare_req_desc_hdr_crypto to clean up code slightly
>  - split up ufshcd_hba_init_crypto into ufshcd_hba_init_crypto_capabilities
>    and ufshcd_init_crypto. The first function is called from
>    ufshcd_hba_capabilities, and only reads crypto capabilities from device
>    registers and sets up appropriate crypto structures. The second function
>    is called from ufshcd_init, and actually initializes the inline crypto
>    hardware.
> 
> Changes v1 => v2
>  - handle OCS_DEVICE_FATAL_ERROR explicitly in ufshcd_transfer_rsp_status
> 
> Satya Tangirala (3):
>   scsi: ufs: UFS driver v2.1 spec crypto additions
>   scsi: ufs: UFS crypto API
>   scsi: ufs: Add inline encryption support to UFS
> 
>  drivers/scsi/ufs/Kconfig         |   9 ++
>  drivers/scsi/ufs/Makefile        |   1 +
>  drivers/scsi/ufs/ufshcd-crypto.c | 238 +++++++++++++++++++++++++++++++
> drivers/scsi/ufs/ufshcd-crypto.h |  77 ++++++++++
>  drivers/scsi/ufs/ufshcd.c        |  49 ++++++-
>  drivers/scsi/ufs/ufshcd.h        |  24 ++++
>  drivers/scsi/ufs/ufshci.h        |  67 ++++++++-
>  7 files changed, 456 insertions(+), 9 deletions(-)  create mode 100644
> drivers/scsi/ufs/ufshcd-crypto.c  create mode 100644 drivers/scsi/ufs/ufshcd-
> crypto.h
> 
Looks Good to me.
I don’t have a platform to test this series though.
It will be good to get a Tested-by tags for this series.

Reviewed-by: Alim Akhtar <alim.akhtar@samsung.com>

> --
> 2.27.0.212.ge8ba1cc988-goog

Re: [PATCH v4 0/3] Inline Encryption Support for UFS

[Eric Biggers]

On Tue, Jul 07, 2020 at 11:06:14PM +0530, Alim Akhtar wrote:
> Hi Satya,
> 
> > -----Original Message-----
> > From: Satya Tangirala <satyat@google.com>
> > Sent: 07 July 2020 01:34
> > To: linux-scsi@vger.kernel.org; Avri Altman <avri.altman@wdc.com>; Alim
> > Akhtar <alim.akhtar@samsung.com>
> > Cc: Barani Muthukumaran <bmuthuku@qti.qualcomm.com>; Kuohong Wang
> > <kuohong.wang@mediatek.com>; Kim Boojin <boojin.kim@samsung.com>;
> > Satya Tangirala <satyat@google.com>
> > Subject: [PATCH v4 0/3] Inline Encryption Support for UFS
> > 
> > This patch series adds support for inline encryption to UFS using the inline
> > encryption support in the block layer. It follows the JEDEC UFSHCI v2.1
> > specification, which defines inline encryption for UFS.
> > 
> > This patch series previously went through a number of iterations as part of the
> > "Inline Encryption Support" patchset (last version was v13:
> > https://lkml.kernel.org/r/20200514003727.69001-1-satyat@google.com).
> > This patch series is rebased on v5.8-rc4.
> > 
> > Patch 1 introduces the crypto registers and struct definitions defined in the
> > UFSHCI v2.1 spec.
> > 
> > Patch 2 introduces functions to manipulate the UFS inline encryption hardware
> > (again in line with the UFSHCI v2.1 spec) via the block layer keyslot manager.
> > Device specific drivers must set the UFSHCD_CAP_CRYPTO in hba->caps before
> > ufshcd_hba_init_crypto is called to opt-in to inline encryption support.
> > 
> > Patch 3 wires up ufshcd.c with the UFS crypto API introduced in Patch 2.
> > 
> > This patch series has been tested on some Qualcomm chipsets (on the db845c,
> > sm8150-mtp and sm8250-mtp) using some additional patches at
> > https://lkml.kernel.org/linux-scsi/20200501045111.665881-1-
> > ebiggers@kernel.org/
> > and on some Mediatek chipsets using the additional patch in
> > https://lkml.kernel.org/linux-scsi/20200304022101.14165-1-
> > stanley.chu@mediatek.com/.
> > These additional patches are required because these chipsets need certain
> > additional behaviour not specified within the UFSHCI v2.1 spec.
> > 
> > Thanks a lot to all the folks who tested this out!
> > 
> > Changes v3 => v4:
> >  - fix incorrect patch folding
> >  - some cleanups from Eric
> > 
> > Changes v2 => v3:
> >  - introduce ufshcd_prepare_req_desc_hdr_crypto to clean up code slightly
> >  - split up ufshcd_hba_init_crypto into ufshcd_hba_init_crypto_capabilities
> >    and ufshcd_init_crypto. The first function is called from
> >    ufshcd_hba_capabilities, and only reads crypto capabilities from device
> >    registers and sets up appropriate crypto structures. The second function
> >    is called from ufshcd_init, and actually initializes the inline crypto
> >    hardware.
> > 
> > Changes v1 => v2
> >  - handle OCS_DEVICE_FATAL_ERROR explicitly in ufshcd_transfer_rsp_status
> > 
> > Satya Tangirala (3):
> >   scsi: ufs: UFS driver v2.1 spec crypto additions
> >   scsi: ufs: UFS crypto API
> >   scsi: ufs: Add inline encryption support to UFS
> > 
> >  drivers/scsi/ufs/Kconfig         |   9 ++
> >  drivers/scsi/ufs/Makefile        |   1 +
> >  drivers/scsi/ufs/ufshcd-crypto.c | 238 +++++++++++++++++++++++++++++++
> > drivers/scsi/ufs/ufshcd-crypto.h |  77 ++++++++++
> >  drivers/scsi/ufs/ufshcd.c        |  49 ++++++-
> >  drivers/scsi/ufs/ufshcd.h        |  24 ++++
> >  drivers/scsi/ufs/ufshci.h        |  67 ++++++++-
> >  7 files changed, 456 insertions(+), 9 deletions(-)  create mode 100644
> > drivers/scsi/ufs/ufshcd-crypto.c  create mode 100644 drivers/scsi/ufs/ufshcd-
> > crypto.h
> > 
> Looks Good to me.
> I don’t have a platform to test this series though.
> It will be good to get a Tested-by tags for this series.
> 
> Reviewed-by: Alim Akhtar <alim.akhtar@samsung.com>
> 

There's information about testing in the cover letter and commit messages
already.  But feel free to also add my Tested-by to all three patches:

Tested-by: Eric Biggers <ebiggers@google.com> # db845c

- Eric

Re: [PATCH v4 0/3] Inline Encryption Support for UFS

[Martin K. Petersen]

On Mon, 6 Jul 2020 20:04:11 +0000, Satya Tangirala wrote:

> This patch series adds support for inline encryption to UFS using
> the inline encryption support in the block layer. It follows the JEDEC
> UFSHCI v2.1 specification, which defines inline encryption for UFS.
> 
> This patch series previously went through a number of iterations as
> part of the "Inline Encryption Support" patchset (last version was v13:
> https://lkml.kernel.org/r/20200514003727.69001-1-satyat@google.com).
> This patch series is rebased on v5.8-rc4.
> 
> [...]

Applied to 5.9/scsi-queue, thanks!

[1/3] scsi: ufs: UFS driver v2.1 spec crypto additions
      https://git.kernel.org/mkp/scsi/c/5e7341e1f9ec
[2/3] scsi: ufs: UFS crypto API
      https://git.kernel.org/mkp/scsi/c/70297a8ac7a7
[3/3] scsi: ufs: Add inline encryption support to UFS
      https://git.kernel.org/mkp/scsi/c/df043c745ea1

-- 
Martin K. Petersen	Oracle Linux Engineering

RE: [PATCH v4 2/3] scsi: ufs: UFS crypto API

[Avri Altman]

> +
> +static enum blk_crypto_mode_num
> +ufshcd_find_blk_crypto_mode(union ufs_crypto_cap_entry cap)
> +{
> +       int i;
> +
> +       for (i = 0; i < ARRAY_SIZE(ufs_crypto_algs); i++) {
> +               BUILD_BUG_ON(UFS_CRYPTO_KEY_SIZE_INVALID != 0);
> +               if (ufs_crypto_algs[i].ufs_alg == cap.algorithm_id &&
> +                   ufs_crypto_algs[i].ufs_key_size == cap.key_size) {
> +                       return i;
> +               }
> +       }
> +       return BLK_ENCRYPTION_MODE_INVALID;
BLK_ENCRYPTION_MODE_INVALID is 0, but 0 is a valid mode num?

> +}
> +
> +/**
> + * ufshcd_hba_init_crypto_capabilities - Read crypto capabilities, init crypto
> + *                                      fields in hba
> + * @hba: Per adapter instance
> + *
> + * Return: 0 if crypto was initialized or is not supported, else a -errno value.
> + */
> +int ufshcd_hba_init_crypto_capabilities(struct ufs_hba *hba)
> +{
> +       int cap_idx;
> +       int err = 0;
> +       enum blk_crypto_mode_num blk_mode_num;
> +
> +       /*
> +        * Don't use crypto if either the hardware doesn't advertise the
> +        * standard crypto capability bit *or* if the vendor specific driver
> +        * hasn't advertised that crypto is supported.
> +        */
> +       if (!(hba->capabilities & MASK_CRYPTO_SUPPORT) ||
> +           !(hba->caps & UFSHCD_CAP_CRYPTO))
> +               goto out;
> +
> +       hba->crypto_capabilities.reg_val =
> +                       cpu_to_le32(ufshcd_readl(hba, REG_UFS_CCAP));
> +       hba->crypto_cfg_register =
> +               (u32)hba->crypto_capabilities.config_array_ptr * 0x100;
This deserve a comment, e.g. 
UFSHCI says:
The address for entry x of the x-CRYPTOCFG array is calculated as follows:
ADDR (x-CRYPTOCFG) = UFS_HCI_BASE + CFGPTR*100h + x*80h

RE: [PATCH v4 0/3] Inline Encryption Support for UFS

[Avri Altman]

> 
> On Mon, Jul 06, 2020 at 08:04:11PM +0000, Satya Tangirala wrote:
> > This patch series adds support for inline encryption to UFS using
> > the inline encryption support in the block layer. It follows the JEDEC
> > UFSHCI v2.1 specification, which defines inline encryption for UFS.
> >
> > This patch series previously went through a number of iterations as
> > part of the "Inline Encryption Support" patchset (last version was v13:
> > https://lkml.kernel.org/r/20200514003727.69001-1-satyat@google.com).
> > This patch series is rebased on v5.8-rc4.
> >
> > Patch 1 introduces the crypto registers and struct definitions defined
> > in the UFSHCI v2.1 spec.
> >
> > Patch 2 introduces functions to manipulate the UFS inline encryption
> > hardware (again in line with the UFSHCI v2.1 spec) via the block
> > layer keyslot manager. Device specific drivers must set the
> > UFSHCD_CAP_CRYPTO in hba->caps before ufshcd_hba_init_crypto is called
> > to opt-in to inline encryption support.
> 
> Note that it's now ufshcd_hba_init_crypto_capabilities(), not
> ufshcd_hba_init_crypto().
> 
> >
> > Patch 3 wires up ufshcd.c with the UFS crypto API introduced in Patch 2.
> >
> > This patch series has been tested on some Qualcomm chipsets (on the
> > db845c, sm8150-mtp and sm8250-mtp) using some additional patches at
> > https://lkml.kernel.org/linux-scsi/20200501045111.665881-1-
> ebiggers@kernel.org/
> > and on some Mediatek chipsets using the additional patch in
> > https://lkml.kernel.org/linux-scsi/20200304022101.14165-1-
> stanley.chu@mediatek.com/.
> > These additional patches are required because these chipsets need certain
> > additional behaviour not specified within the UFSHCI v2.1 spec.
> >
> > Thanks a lot to all the folks who tested this out!
> >
> > Changes v3 => v4:
> >  - fix incorrect patch folding
> >  - some cleanups from Eric
> >
> > Changes v2 => v3:
> >  - introduce ufshcd_prepare_req_desc_hdr_crypto to clean up code slightly
> >  - split up ufshcd_hba_init_crypto into ufshcd_hba_init_crypto_capabilities
> >    and ufshcd_init_crypto. The first function is called from
> >    ufshcd_hba_capabilities, and only reads crypto capabilities from device
> >    registers and sets up appropriate crypto structures. The second function
> >    is called from ufshcd_init, and actually initializes the inline crypto
> >    hardware.
> >
> > Changes v1 => v2
> >  - handle OCS_DEVICE_FATAL_ERROR explicitly in ufshcd_transfer_rsp_status
> >
> > Satya Tangirala (3):
> >   scsi: ufs: UFS driver v2.1 spec crypto additions
> >   scsi: ufs: UFS crypto API
> >   scsi: ufs: Add inline encryption support to UFS
> 
> These patches look good to me.  Avri and Alim, what do you think?
> We'd like these to be applied for 5.9.
Yes.  It looks good to me too.
I've added 2 nits to your 2nd patch - please feel free to ignore them.

Thanks,
Avri

> 
> - Eric

Re: [PATCH v4 2/3] scsi: ufs: UFS crypto API

[Eric Biggers]

On Wed, Jul 08, 2020 at 07:44:28AM +0000, Avri Altman wrote:
> > +
> > +static enum blk_crypto_mode_num
> > +ufshcd_find_blk_crypto_mode(union ufs_crypto_cap_entry cap)
> > +{
> > +       int i;
> > +
> > +       for (i = 0; i < ARRAY_SIZE(ufs_crypto_algs); i++) {
> > +               BUILD_BUG_ON(UFS_CRYPTO_KEY_SIZE_INVALID != 0);
> > +               if (ufs_crypto_algs[i].ufs_alg == cap.algorithm_id &&
> > +                   ufs_crypto_algs[i].ufs_key_size == cap.key_size) {
> > +                       return i;
> > +               }
> > +       }
> > +       return BLK_ENCRYPTION_MODE_INVALID;
> BLK_ENCRYPTION_MODE_INVALID is 0, but 0 is a valid mode num?

ufs_crypto_algs[] does have to contain an entry for BLK_ENCRYPTION_MODE_INVALID,
but it's an empty entry with ufs_key_size == UFS_CRYPTO_KEY_SIZE_INVALID (0).
So it will never be selected and the code is correct.

We should probably start iterating at 1 to make this clearer, though note that
the code still needs to handle empty entries anyway in case any gaps are ever
introduced into the BLK_ENCRYPTION_MODE_* values.

- Eric