[PATCH v2 0/3] Inline Encryption support for UFS

[PATCH v2 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).
There aren't any significant changes here from that version.
This patch series is based on v5.8-rc1.

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 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 | 226 +++++++++++++++++++++++++++++++
 drivers/scsi/ufs/ufshcd-crypto.h |  60 ++++++++
 drivers/scsi/ufs/ufshcd.c        |  47 ++++++-
 drivers/scsi/ufs/ufshcd.h        |  24 ++++
 drivers/scsi/ufs/ufshci.h        |  67 ++++++++-
 7 files changed, 427 insertions(+), 7 deletions(-)
 create mode 100644 drivers/scsi/ufs/ufshcd-crypto.c
 create mode 100644 drivers/scsi/ufs/ufshcd-crypto.h

-- 
2.27.0.290.gba653c62da-goog

[PATCH v2 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.290.gba653c62da-goog

[PATCH v2 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 | 226 +++++++++++++++++++++++++++++++
 drivers/scsi/ufs/ufshcd-crypto.h |  42 ++++++
 drivers/scsi/ufs/ufshcd.h        |  12 ++
 5 files changed, 290 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..65a3115d2a2d
--- /dev/null
+++ b/drivers/scsi/ufs/ufshcd-crypto.c
@@ -0,0 +1,226 @@
+// 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 - 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(struct ufs_hba *hba)
+{
+	int cap_idx = 0;
+	int err = 0;
+	enum blk_crypto_mode_num blk_mode_num;
+	int slot = 0;
+	int num_keyslots;
+
+	/*
+	 * 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) */
+	num_keyslots = hba->crypto_capabilities.config_count + 1;
+	err = blk_ksm_init(&hba->ksm, num_keyslots);
+	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;
+	}
+
+	for (slot = 0; slot < num_keyslots; slot++)
+		ufshcd_clear_keyslot(hba, slot);
+
+	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;
+}
+
+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..22677619de59
--- /dev/null
+++ b/drivers/scsi/ufs/ufshcd-crypto.h
@@ -0,0 +1,42 @@
+/* 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(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(struct ufs_hba *hba)
+{
+	return 0;
+}
+
+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.290.gba653c62da-goog

[PATCH v2 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 | 18 +++++++++++++
 drivers/scsi/ufs/ufshcd.c        | 44 ++++++++++++++++++++++++++++----
 drivers/scsi/ufs/ufshcd.h        |  6 +++++
 3 files changed, 63 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd-crypto.h b/drivers/scsi/ufs/ufshcd-crypto.h
index 22677619de59..9578edb63e7b 100644
--- a/drivers/scsi/ufs/ufshcd-crypto.h
+++ b/drivers/scsi/ufs/ufshcd-crypto.h
@@ -10,6 +10,20 @@
 #include "ufshcd.h"
 #include "ufshci.h"
 
+static inline void ufshcd_prepare_lrbp_crypto(struct ufs_hba *hba,
+					      struct scsi_cmnd *cmd,
+					      struct ufshcd_lrb *lrbp)
+{
+	struct request *rq = cmd->request;
+
+	if (rq->crypt_keyslot) {
+		lrbp->crypto_key_slot = blk_ksm_get_slot_idx(rq->crypt_keyslot);
+		lrbp->data_unit_num = rq->crypt_ctx->bc_dun[0];
+	} else {
+		lrbp->crypto_key_slot = -1;
+	}
+}
+
 bool ufshcd_crypto_enable(struct ufs_hba *hba);
 
 int ufshcd_hba_init_crypto(struct ufs_hba *hba);
@@ -21,6 +35,10 @@ void ufshcd_crypto_destroy_keyslot_manager(struct ufs_hba *hba);
 
 #else /* CONFIG_SCSI_UFS_CRYPTO */
 
+static inline void ufshcd_prepare_lrbp_crypto(struct ufs_hba *hba,
+					      struct scsi_cmnd *cmd,
+					      struct ufshcd_lrb *lrbp) { }
+
 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..266c1d7c9f02 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);
 }
 
 /**
@@ -2237,6 +2243,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;
@@ -2255,9 +2263,17 @@ static void ufshcd_prepare_req_desc_hdr(struct ufshcd_lrb *lrbp,
 		dword_0 |= UTP_REQ_DESC_INT_CMD;
 
 	/* Transfer request descriptor header fields */
+#ifdef CONFIG_SCSI_UFS_CRYPTO
+	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);
+	}
+#endif /* CONFIG_SCSI_UFS_CRYPTO */
+
 	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 +2281,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 +2536,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(hba, cmd, lrbp);
+
 	lrbp->req_abort_skip = false;
 
 	ufshcd_comp_scsi_upiu(hba, lrbp);
@@ -2554,6 +2572,9 @@ 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 */
+#ifdef CONFIG_SCSI_UFS_CRYPTO
+	lrbp->crypto_key_slot = -1; /* No crypto operations */
+#endif
 	hba->dev_cmd.type = cmd_type;
 
 	return ufshcd_comp_devman_upiu(hba, lrbp);
@@ -4650,6 +4671,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 +6138,9 @@ static int ufshcd_issue_devman_upiu_cmd(struct ufs_hba *hba,
 	lrbp->task_tag = tag;
 	lrbp->lun = 0;
 	lrbp->intr_cmd = true;
+#ifdef CONFIG_SCSI_UFS_CRYPTO
+	lrbp->crypto_key_slot = -1; /* No crypto operations */
+#endif
 	hba->dev_cmd.type = cmd_type;
 
 	switch (hba->ufs_version) {
@@ -8662,6 +8688,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);
@@ -8872,6 +8899,13 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
 	/* Reset the attached device */
 	ufshcd_vops_device_reset(hba);
 
+	/* Init crypto */
+	err = ufshcd_hba_init_crypto(hba);
+	if (err) {
+		dev_err(hba->dev, "crypto setup failed\n");
+		goto free_tmf_queue;
+	}
+
 	/* 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.290.gba653c62da-goog

Re: [PATCH v2 0/3] Inline Encryption support for UFS

[Martin K. Petersen]

Avri,

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

I'd appreciate it if you could review this series.

Thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering

RE: [PATCH v2 0/3] Inline Encryption support for UFS

[Avri Altman]

> Avri,
> 
> > 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.
> 
> I'd appreciate it if you could review this series.
I need help doing it.  I will ask around.
Thanks,
Avri

RE: [PATCH v2 0/3] Inline Encryption support for UFS

[Avri Altman]

+Alim & Asutosh

Hi Satya,

> 
> Avri,
> 
> > 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.
> 
> I'd appreciate it if you could review this series.
> 
> Thanks!
> 
> --
> Martin K. Petersen      Oracle Linux Engineering
A quick question and a comment:

Does the IE infrastructure that you've added to the block layer invented for ufs?
Do you see other devices using it in the future?

Today, chipset vendors are using a different scheme for their IE.
Need their ack before reviewing your patches.

Thanks,
Avri

RE: [PATCH v2 0/3] Inline Encryption support for UFS

[Avri Altman]

 
> +Alim & Asutosh
> 
> Hi Satya,
> 
> >
> > Avri,
> >
> > > 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.
> >
> > I'd appreciate it if you could review this series.
> >
> > Thanks!
> >
> > --
> > Martin K. Petersen      Oracle Linux Engineering
> A quick question and a comment:
> 
> Does the IE infrastructure that you've added to the block layer invented for
> ufs?
And how this infrastructure relates to Eric's RFC: Inline crypto support on DragonBoard 845c
https://www.spinics.net/lists/linux-scsi/msg141472.html 

> Do you see other devices using it in the future?
> 
> Today, chipset vendors are using a different scheme for their IE.
> Need their ack before reviewing your patches.
> 
> Thanks,
> Avri

Re: [PATCH v2 0/3] Inline Encryption support for UFS

[Eric Biggers]

On Sun, Jun 21, 2020 at 12:47:11PM +0000, Avri Altman wrote:
>  
> > +Alim & Asutosh
> > 
> > Hi Satya,
> > 
> > >
> > > Avri,
> > >
> > > > 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.
> > >
> > > I'd appreciate it if you could review this series.
> > >
> > > Thanks!
> > >
> > > --
> > > Martin K. Petersen      Oracle Linux Engineering
> > A quick question and a comment:
> > 
> > Does the IE infrastructure that you've added to the block layer invented for
> > ufs?
> And how this infrastructure relates to Eric's RFC: Inline crypto support on DragonBoard 845c
> https://www.spinics.net/lists/linux-scsi/msg141472.html 
> 

My patches are based on top of Satya's patches.  My patches just introduce the
quirks needed for Qualcomm ICE to work, since it doesn't follow the UFS standard
exactly.

- Eric

RE: [PATCH v2 0/3] Inline Encryption support for UFS

[Alim Akhtar]

Thanks Avri for CCing me.

> -----Original Message-----
> From: Avri Altman <Avri.Altman@wdc.com>
> Sent: 21 June 2020 18:05
> To: Martin K. Petersen <martin.petersen@oracle.com>; Satya Tangirala
> <satyat@google.com>; alim.akhtar@samsung.com; asutoshd@codeaurora.org
> Cc: linux-scsi@vger.kernel.org; Barani Muthukumaran
> <bmuthuku@qti.qualcomm.com>; Kuohong Wang
> <kuohong.wang@mediatek.com>; Kim Boojin <boojin.kim@samsung.com>
> Subject: RE: [PATCH v2 0/3] Inline Encryption support for UFS
> 
> +Alim & Asutosh
> 
> Hi Satya,
> 
> >
> > Avri,
> >
> > > 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.
> >
> > I'd appreciate it if you could review this series.
> >
> > Thanks!
> >
> > --
> > Martin K. Petersen      Oracle Linux Engineering
> A quick question and a comment:
> 
> Does the IE infrastructure that you've added to the block layer invented
for ufs?
> Do you see other devices using it in the future?
> 
> Today, chipset vendors are using a different scheme for their IE.
> Need their ack before reviewing your patches.
> 
Yes, as of today at least in Samsung HCI, we use additional HW blocks to
handle all the crypto part.
(Though I need to check the status on the recent SoCs).
However given the fact that UFSHCI 2.1 spec does includes Crypto support,
and going by threads that you shared, looks  like other 
Vendors does uses IE. I am inclined toward getting this reviewed. 
> Thanks,
> Avri

Re: [PATCH v2 0/3] Inline Encryption support for UFS

[Satya Tangirala]

On Sun, Jun 21, 2020 at 12:35:13PM +0000, Avri Altman wrote:
> +Alim & Asutosh
> 
> Hi Satya,
> 
> > 
> > Avri,
> > 
> > > 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.
> > 
> > I'd appreciate it if you could review this series.
> > 
> > Thanks!
> > 
> > --
> > Martin K. Petersen      Oracle Linux Engineering
> A quick question and a comment:
> 
> Does the IE infrastructure that you've added to the block layer invented for ufs?
It wasn't invented "just" for UFS, but it was guided by how UFS inline
crypto works. It is meant to be generic, and should be how we support
any arbitrary IE hardware in the kernel.

> Do you see other devices using it in the future?
> 
Certainly - at the very least, I think it's likely we'll add support
for eMMC inline crypto (although inline encryption support for eMMC was
only added in an unreleased/yet to be released spec). It's certainly my
hope that we add IE support for other classes of devices too.

> Today, chipset vendors are using a different scheme for their IE.
> Need their ack before reviewing your patches.
> 
Sure :). The Inline Encryption patches (the block layer, fscrypt, f2fs,
ext4 and these UFS patches) have been part of the Android common kernel
https://android.googlesource.com/kernel/common/
for quite a while now, and chipset vendors have been working with us
on ensuring that their UFS cards are supported by these patches (by adding
code similar to Eric's RFC: Inline crypto support on DragonBoard 845c at
https://www.spinics.net/lists/linux-scsi/msg141472.html
or Mediatek's "scsi: ufs-mediatek: add inline encryption support" at
https://lkml.kernel.org/linux-scsi/20200304022101.14165-1-stanley.chu@mediatek.com/
for their individual chipsets).
> Thanks,
> Avri

Re: [PATCH v2 0/3] Inline Encryption support for UFS

[Eric Biggers]

On Sun, Jun 21, 2020 at 06:24:36PM +0000, Satya Tangirala wrote:
> 
> > Today, chipset vendors are using a different scheme for their IE.
> > Need their ack before reviewing your patches.
> > 
> Sure :). The Inline Encryption patches (the block layer, fscrypt, f2fs,
> ext4 and these UFS patches) have been part of the Android common kernel
> https://android.googlesource.com/kernel/common/
> for quite a while now, and chipset vendors have been working with us
> on ensuring that their UFS cards are supported by these patches (by adding
> code similar to Eric's RFC: Inline crypto support on DragonBoard 845c at
> https://www.spinics.net/lists/linux-scsi/msg141472.html
> or Mediatek's "scsi: ufs-mediatek: add inline encryption support" at
> https://lkml.kernel.org/linux-scsi/20200304022101.14165-1-stanley.chu@mediatek.com/
> for their individual chipsets).

Just as a practical matter, I don't think this patchset should be blocked
waiting for a formal Acked-by from every vendor, as some of them don't to be
participating in LKML at all despite being Cc'ed on these patches for a year and
working with us outside LKML to support their hardware using the new framework.
So I think a formal Acked-by on LKML is not always guaranteed.  (It would
certainly be preferable if everyone would participate though!)

- Eric

Re: [PATCH v2 0/3] Inline Encryption support for UFS

[Eric Biggers]

On Sun, Jun 21, 2020 at 11:24:30PM +0530, Alim Akhtar wrote:
> Thanks Avri for CCing me.
> 
> > -----Original Message-----
> > From: Avri Altman <Avri.Altman@wdc.com>
> > Sent: 21 June 2020 18:05
> > To: Martin K. Petersen <martin.petersen@oracle.com>; Satya Tangirala
> > <satyat@google.com>; alim.akhtar@samsung.com; asutoshd@codeaurora.org
> > Cc: linux-scsi@vger.kernel.org; Barani Muthukumaran
> > <bmuthuku@qti.qualcomm.com>; Kuohong Wang
> > <kuohong.wang@mediatek.com>; Kim Boojin <boojin.kim@samsung.com>
> > Subject: RE: [PATCH v2 0/3] Inline Encryption support for UFS
> > 
> > +Alim & Asutosh
> > 
> > Hi Satya,
> > 
> > >
> > > Avri,
> > >
> > > > 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.
> > >
> > > I'd appreciate it if you could review this series.
> > >
> > > Thanks!
> > >
> > > --
> > > Martin K. Petersen      Oracle Linux Engineering
> > A quick question and a comment:
> > 
> > Does the IE infrastructure that you've added to the block layer invented
> for ufs?
> > Do you see other devices using it in the future?
> > 
> > Today, chipset vendors are using a different scheme for their IE.
> > Need their ack before reviewing your patches.
> > 
> Yes, as of today at least in Samsung HCI, we use additional HW blocks to
> handle all the crypto part.
> (Though I need to check the status on the recent SoCs).
> However given the fact that UFSHCI 2.1 spec does includes Crypto support,
> and going by threads that you shared, looks  like other 
> Vendors does uses IE. I am inclined toward getting this reviewed. 

Note that Boojin Kim, who has been Cc'ed on all these patches, has already been
working on replacing Samsung's legacy inline encryption implementation with one
using the new framework.

Unfortunately, Samsung's UFS inline encryption hardware doesn't follow the UFS
specification, so it needs custom driver code and doesn't take much advantage of
ufshcd-crypto (this patchset).  However, it can still use the blk-crypto
framework.  So only the driver needs to differ, not the rest of the storage
stack.  This differs from the "old world" where every vendor had to customize
the entire storage stack to support their inline encryption hardware.

Anyway, ufshcd-crypto (this patchset) is still needed for all vendors who did
mostly/fully follow the UFS specification, e.g. Mediatek
(https://lkml.kernel.org/linux-scsi/20200304022101.14165-1-stanley.chu@mediatek.com)
and Qualcomm
(https://lkml.kernel.org/linux-scsi/20200621173713.132879-1-ebiggers@kernel.org).

More reviews are always appreciated, though note that this patchset has already
been out for review for over a year.  (This is really v15; Satya started the
numbering over after blk-crypto was merged in v5.8-rc1.)  So I'm not sure we
should count on many more formal reviews.  

- Eric

RE: [PATCH v2 0/3] Inline Encryption support for UFS

[Avri Altman]

> > > A quick question and a comment:
> > >
> > > Does the IE infrastructure that you've added to the block layer invented
> > for ufs?
> > > Do you see other devices using it in the future?
> > >
> > > Today, chipset vendors are using a different scheme for their IE.
> > > Need their ack before reviewing your patches.
> > >
> > Yes, as of today at least in Samsung HCI, we use additional HW blocks to
> > handle all the crypto part.
> > (Though I need to check the status on the recent SoCs).
> > However given the fact that UFSHCI 2.1 spec does includes Crypto support,
> > and going by threads that you shared, looks  like other
> > Vendors does uses IE. I am inclined toward getting this reviewed.
> 
> Note that Boojin Kim, who has been Cc'ed on all these patches, has already
> been
> working on replacing Samsung's legacy inline encryption implementation with
> one
> using the new framework.
> 
> Unfortunately, Samsung's UFS inline encryption hardware doesn't follow the
> UFS
> specification, so it needs custom driver code and doesn't take much
> advantage of
> ufshcd-crypto (this patchset).  However, it can still use the blk-crypto
> framework.  So only the driver needs to differ, not the rest of the storage
> stack.  This differs from the "old world" where every vendor had to customize
> the entire storage stack to support their inline encryption hardware.
> 
> Anyway, ufshcd-crypto (this patchset) is still needed for all vendors who did
> mostly/fully follow the UFS specification, e.g. Mediatek
> (https://lkml.kernel.org/linux-scsi/20200304022101.14165-1-
> stanley.chu@mediatek.com)
> and Qualcomm
> (https://lkml.kernel.org/linux-scsi/20200621173713.132879-1-
> ebiggers@kernel.org).
> 
> More reviews are always appreciated, though note that this patchset has
> already
> been out for review for over a year.  (This is really v15; Satya started the
> numbering over after blk-crypto was merged in v5.8-rc1.)  So I'm not sure we
> should count on many more formal reviews.
Ok then.
This works as a practical ack by most of the large chipset vendors.

Thanks,
Avri

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

[Avri Altman]

>ufshcd_prepare_req_desc_hdr
maybe pack all your additions to ufshcd_prepare_req_desc_hdr into a
ufshcd_prepare_req_desc_hdr_crypto helper, so the ifdefs can be avoided?

>ufshcd_compose_dev_cmd
same goes to ufshcd_compose_dev_cmd and ufshcd_issue_devman_upiu_cmd -
maybe just call ufshcd_prepare_lrbp_crypto (with some minor changes)
instead of those ifdefs?

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

[Alim Akhtar]

Hi Satya

On Thu, Jun 18, 2020 at 8:18 AM Satya Tangirala <satyat@google.com> wrote:
>
> Wire up ufshcd.c with the UFS Crypto API, the block layer inline
> encryption additions and the keyslot manager.
PORT_SYMBOL_GPL(ufshcd_dealloc_host);
> @@ -8872,6 +8899,13 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
>         /* Reset the attached device */
>         ufshcd_vops_device_reset(hba);
>
> +       /* Init crypto */
> +       err = ufshcd_hba_init_crypto(hba);
How about checking the capabilities of host controller in
ufshcd_hba_capabilities() itself and then proceed?

> +       if (err) {
> +               dev_err(hba->dev, "crypto setup failed\n");
> +               goto free_tmf_queue;
> +       }
> +
>         /* 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.290.gba653c62da-goog
>


-- 
Regards,
Alim