* [PATCH v3 01/12] ice, ufs, mmc: use blk_crypto_key for program_key
2023-11-22 5:38 [PATCH v3 00/12] Hardware wrapped key support for qcom ice and ufs Gaurav Kashyap
@ 2023-11-22 5:38 ` Gaurav Kashyap
2023-12-08 6:22 ` Om Prakash Singh
2023-11-22 5:38 ` [PATCH v3 02/12] qcom_scm: scm call for deriving a software secret Gaurav Kashyap
` (12 subsequent siblings)
13 siblings, 1 reply; 39+ messages in thread
From: Gaurav Kashyap @ 2023-11-22 5:38 UTC (permalink / raw)
To: linux-scsi, linux-arm-msm, ebiggers, neil.armstrong, srinivas.kandagatla
Cc: linux-mmc, linux-block, linux-fscrypt, omprsing, quic_psodagud,
abel.vesa, quic_spuppala, kernel, Gaurav Kashyap
The program key ops in the storage controller does not
pass on the blk crypto key structure to ice, this is okay
when wrapped keys are not supported and keys are standard
AES XTS sizes. However, wrapped keyblobs can be of any size
and in preparation for that, modify the ICE and storage
controller APIs to accept blk_crypto_key.
Signed-off-by: Gaurav Kashyap <quic_gaurkash@quicinc.com>
---
drivers/mmc/host/cqhci-crypto.c | 7 ++++---
drivers/mmc/host/cqhci.h | 2 ++
drivers/mmc/host/sdhci-msm.c | 6 ++++--
drivers/soc/qcom/ice.c | 6 +++---
drivers/ufs/core/ufshcd-crypto.c | 7 ++++---
drivers/ufs/host/ufs-qcom.c | 6 ++++--
include/soc/qcom/ice.h | 5 +++--
include/ufs/ufshcd.h | 1 +
8 files changed, 25 insertions(+), 15 deletions(-)
diff --git a/drivers/mmc/host/cqhci-crypto.c b/drivers/mmc/host/cqhci-crypto.c
index 6652982410ec..91da6de1d650 100644
--- a/drivers/mmc/host/cqhci-crypto.c
+++ b/drivers/mmc/host/cqhci-crypto.c
@@ -32,6 +32,7 @@ cqhci_host_from_crypto_profile(struct blk_crypto_profile *profile)
}
static int cqhci_crypto_program_key(struct cqhci_host *cq_host,
+ const struct blk_crypto_key *bkey,
const union cqhci_crypto_cfg_entry *cfg,
int slot)
{
@@ -39,7 +40,7 @@ static int cqhci_crypto_program_key(struct cqhci_host *cq_host,
int i;
if (cq_host->ops->program_key)
- return cq_host->ops->program_key(cq_host, cfg, slot);
+ return cq_host->ops->program_key(cq_host, bkey, cfg, slot);
/* Clear CFGE */
cqhci_writel(cq_host, 0, slot_offset + 16 * sizeof(cfg->reg_val[0]));
@@ -99,7 +100,7 @@ static int cqhci_crypto_keyslot_program(struct blk_crypto_profile *profile,
memcpy(cfg.crypto_key, key->raw, key->size);
}
- err = cqhci_crypto_program_key(cq_host, &cfg, slot);
+ err = cqhci_crypto_program_key(cq_host, key, &cfg, slot);
memzero_explicit(&cfg, sizeof(cfg));
return err;
@@ -113,7 +114,7 @@ static int cqhci_crypto_clear_keyslot(struct cqhci_host *cq_host, int slot)
*/
union cqhci_crypto_cfg_entry cfg = {};
- return cqhci_crypto_program_key(cq_host, &cfg, slot);
+ return cqhci_crypto_program_key(cq_host, NULL, &cfg, slot);
}
static int cqhci_crypto_keyslot_evict(struct blk_crypto_profile *profile,
diff --git a/drivers/mmc/host/cqhci.h b/drivers/mmc/host/cqhci.h
index 1a12e40a02e6..949ebbe05773 100644
--- a/drivers/mmc/host/cqhci.h
+++ b/drivers/mmc/host/cqhci.h
@@ -12,6 +12,7 @@
#include <linux/completion.h>
#include <linux/wait.h>
#include <linux/irqreturn.h>
+#include <linux/blk-crypto.h>
#include <asm/io.h>
/* registers */
@@ -291,6 +292,7 @@ struct cqhci_host_ops {
void (*post_disable)(struct mmc_host *mmc);
#ifdef CONFIG_MMC_CRYPTO
int (*program_key)(struct cqhci_host *cq_host,
+ const struct blk_crypto_key *bkey,
const union cqhci_crypto_cfg_entry *cfg, int slot);
#endif
};
diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 668e0aceeeba..7d956fad27c7 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -1859,6 +1859,7 @@ static __maybe_unused int sdhci_msm_ice_suspend(struct sdhci_msm_host *msm_host)
* 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 struct blk_crypto_key_type *bkey,
const union cqhci_crypto_cfg_entry *cfg,
int slot)
{
@@ -1866,6 +1867,7 @@ static int sdhci_msm_program_key(struct cqhci_host *cq_host,
struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
union cqhci_crypto_cap_entry cap;
+ u8 ice_key_size;
/* Only AES-256-XTS has been tested so far. */
cap = cq_host->crypto_cap_array[cfg->crypto_cap_idx];
@@ -1873,11 +1875,11 @@ static int sdhci_msm_program_key(struct cqhci_host *cq_host,
cap.key_size != CQHCI_CRYPTO_KEY_SIZE_256)
return -EINVAL;
+ ice_key_size = QCOM_ICE_CRYPTO_KEY_SIZE_256;
if (cfg->config_enable & CQHCI_CRYPTO_CONFIGURATION_ENABLE)
return qcom_ice_program_key(msm_host->ice,
QCOM_ICE_CRYPTO_ALG_AES_XTS,
- QCOM_ICE_CRYPTO_KEY_SIZE_256,
- cfg->crypto_key,
+ ice_key_size, bkey,
cfg->data_unit_size, slot);
else
return qcom_ice_evict_key(msm_host->ice, slot);
diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c
index fbab7fe5c652..6f941d32fffb 100644
--- a/drivers/soc/qcom/ice.c
+++ b/drivers/soc/qcom/ice.c
@@ -163,8 +163,8 @@ EXPORT_SYMBOL_GPL(qcom_ice_suspend);
int qcom_ice_program_key(struct qcom_ice *ice,
u8 algorithm_id, u8 key_size,
- const u8 crypto_key[], u8 data_unit_size,
- int slot)
+ const struct blk_crypto_key *bkey,
+ u8 data_unit_size, int slot)
{
struct device *dev = ice->dev;
union {
@@ -183,7 +183,7 @@ int qcom_ice_program_key(struct qcom_ice *ice,
return -EINVAL;
}
- memcpy(key.bytes, crypto_key, AES_256_XTS_KEY_SIZE);
+ memcpy(key.bytes, bkey->raw, AES_256_XTS_KEY_SIZE);
/* The SCM call requires that the key words are encoded in big endian */
for (i = 0; i < ARRAY_SIZE(key.words); i++)
diff --git a/drivers/ufs/core/ufshcd-crypto.c b/drivers/ufs/core/ufshcd-crypto.c
index f4cc54d82281..34537cbac622 100644
--- a/drivers/ufs/core/ufshcd-crypto.c
+++ b/drivers/ufs/core/ufshcd-crypto.c
@@ -18,6 +18,7 @@ static const struct ufs_crypto_alg_entry {
};
static int ufshcd_program_key(struct ufs_hba *hba,
+ const struct blk_crypto_key *bkey,
const union ufs_crypto_cfg_entry *cfg, int slot)
{
int i;
@@ -27,7 +28,7 @@ static int ufshcd_program_key(struct ufs_hba *hba,
ufshcd_hold(hba);
if (hba->vops && hba->vops->program_key) {
- err = hba->vops->program_key(hba, cfg, slot);
+ err = hba->vops->program_key(hba, bkey, cfg, slot);
goto out;
}
@@ -89,7 +90,7 @@ static int ufshcd_crypto_keyslot_program(struct blk_crypto_profile *profile,
memcpy(cfg.crypto_key, key->raw, key->size);
}
- err = ufshcd_program_key(hba, &cfg, slot);
+ err = ufshcd_program_key(hba, key, &cfg, slot);
memzero_explicit(&cfg, sizeof(cfg));
return err;
@@ -103,7 +104,7 @@ static int ufshcd_clear_keyslot(struct ufs_hba *hba, int slot)
*/
union ufs_crypto_cfg_entry cfg = {};
- return ufshcd_program_key(hba, &cfg, slot);
+ return ufshcd_program_key(hba, NULL, &cfg, slot);
}
static int ufshcd_crypto_keyslot_evict(struct blk_crypto_profile *profile,
diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
index 96cb8b5b4e66..7bec2b99b1df 100644
--- a/drivers/ufs/host/ufs-qcom.c
+++ b/drivers/ufs/host/ufs-qcom.c
@@ -146,6 +146,7 @@ static inline int ufs_qcom_ice_suspend(struct ufs_qcom_host *host)
}
static int ufs_qcom_ice_program_key(struct ufs_hba *hba,
+ const struct blk_crypto_key *bkey,
const union ufs_crypto_cfg_entry *cfg,
int slot)
{
@@ -153,6 +154,7 @@ static int ufs_qcom_ice_program_key(struct ufs_hba *hba,
union ufs_crypto_cap_entry cap;
bool config_enable =
cfg->config_enable & UFS_CRYPTO_CONFIGURATION_ENABLE;
+ u8 ice_key_size;
/* Only AES-256-XTS has been tested so far. */
cap = hba->crypto_cap_array[cfg->crypto_cap_idx];
@@ -160,11 +162,11 @@ static int ufs_qcom_ice_program_key(struct ufs_hba *hba,
cap.key_size != UFS_CRYPTO_KEY_SIZE_256)
return -EINVAL;
+ ice_key_size = QCOM_ICE_CRYPTO_KEY_SIZE_256;
if (config_enable)
return qcom_ice_program_key(host->ice,
QCOM_ICE_CRYPTO_ALG_AES_XTS,
- QCOM_ICE_CRYPTO_KEY_SIZE_256,
- cfg->crypto_key,
+ ice_key_size, bkey,
cfg->data_unit_size, slot);
else
return qcom_ice_evict_key(host->ice, slot);
diff --git a/include/soc/qcom/ice.h b/include/soc/qcom/ice.h
index 5870a94599a2..9dd835dba2a7 100644
--- a/include/soc/qcom/ice.h
+++ b/include/soc/qcom/ice.h
@@ -7,6 +7,7 @@
#define __QCOM_ICE_H__
#include <linux/types.h>
+#include <linux/blk-crypto.h>
struct qcom_ice;
@@ -30,8 +31,8 @@ int qcom_ice_resume(struct qcom_ice *ice);
int qcom_ice_suspend(struct qcom_ice *ice);
int qcom_ice_program_key(struct qcom_ice *ice,
u8 algorithm_id, u8 key_size,
- const u8 crypto_key[], u8 data_unit_size,
- int slot);
+ const struct blk_crypto_key *bkey,
+ u8 data_unit_size, int slot);
int qcom_ice_evict_key(struct qcom_ice *ice, int slot);
struct qcom_ice *of_qcom_ice_get(struct device *dev);
#endif /* __QCOM_ICE_H__ */
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index 7f0b2c5599cd..e1184ccc0508 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -362,6 +362,7 @@ struct ufs_hba_variant_ops {
struct devfreq_dev_profile *profile,
struct devfreq_simple_ondemand_data *data);
int (*program_key)(struct ufs_hba *hba,
+ const struct blk_crypto_key *bkey,
const union ufs_crypto_cfg_entry *cfg, int slot);
void (*event_notify)(struct ufs_hba *hba,
enum ufs_event_type evt, void *data);
--
2.25.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [PATCH v3 01/12] ice, ufs, mmc: use blk_crypto_key for program_key
2023-11-22 5:38 ` [PATCH v3 01/12] ice, ufs, mmc: use blk_crypto_key for program_key Gaurav Kashyap
@ 2023-12-08 6:22 ` Om Prakash Singh
0 siblings, 0 replies; 39+ messages in thread
From: Om Prakash Singh @ 2023-12-08 6:22 UTC (permalink / raw)
To: Gaurav Kashyap, linux-scsi, linux-arm-msm, ebiggers,
neil.armstrong, srinivas.kandagatla
Cc: linux-mmc, linux-block, linux-fscrypt, omprsing, quic_psodagud,
abel.vesa, quic_spuppala, kernel
On 11/22/2023 11:08 AM, Gaurav Kashyap wrote:
> The program key ops in the storage controller does not pass on the blk
> crypto key structure to ice, this is okay when wrapped keys are not
> supported and keys are standard AES XTS sizes. However, wrapped keyblobs
> can be of any size and in preparation for that, modify the ICE and
> storage controller APIs to accept blk_crypto_key. Signed-off-by: Gaurav
> Kashyap <quic_gaurkash@quicinc.com> ---
Reviewed-by: Om Prakash Singh <quic_omprsing@quicinc.com>
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v3 02/12] qcom_scm: scm call for deriving a software secret
2023-11-22 5:38 [PATCH v3 00/12] Hardware wrapped key support for qcom ice and ufs Gaurav Kashyap
2023-11-22 5:38 ` [PATCH v3 01/12] ice, ufs, mmc: use blk_crypto_key for program_key Gaurav Kashyap
@ 2023-11-22 5:38 ` Gaurav Kashyap
2023-11-22 17:43 ` Trilok Soni
2023-12-08 6:38 ` Om Prakash Singh
2023-11-22 5:38 ` [PATCH v3 03/12] soc: qcom: ice: add hwkm support in ice Gaurav Kashyap
` (11 subsequent siblings)
13 siblings, 2 replies; 39+ messages in thread
From: Gaurav Kashyap @ 2023-11-22 5:38 UTC (permalink / raw)
To: linux-scsi, linux-arm-msm, ebiggers, neil.armstrong, srinivas.kandagatla
Cc: linux-mmc, linux-block, linux-fscrypt, omprsing, quic_psodagud,
abel.vesa, quic_spuppala, kernel, Gaurav Kashyap
Inline storage encryption requires deriving a sw secret from
the hardware wrapped keys. For non-wrapped keys, this can be
directly done as keys are in the clear.
However, when keys are hardware wrapped, it can be unwrapped
by HWKM (Hardware Key Manager) which is accessible only from Qualcomm
Trustzone. Hence, it also makes sense that the software secret is also
derived there and returned to the linux kernel . This can be invoked by
using the crypto profile APIs provided by the block layer.
Signed-off-by: Gaurav Kashyap <quic_gaurkash@quicinc.com>
---
drivers/firmware/qcom/qcom_scm.c | 71 ++++++++++++++++++++++++++
drivers/firmware/qcom/qcom_scm.h | 1 +
include/linux/firmware/qcom/qcom_scm.h | 2 +
3 files changed, 74 insertions(+)
diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
index 520de9b5633a..6dfb913f3e33 100644
--- a/drivers/firmware/qcom/qcom_scm.c
+++ b/drivers/firmware/qcom/qcom_scm.c
@@ -1214,6 +1214,77 @@ int qcom_scm_ice_set_key(u32 index, const u8 *key, u32 key_size,
}
EXPORT_SYMBOL_GPL(qcom_scm_ice_set_key);
+/**
+ * qcom_scm_derive_sw_secret() - Derive software secret from wrapped key
+ * @wkey: the hardware wrapped key inaccessible to software
+ * @wkey_size: size of the wrapped key
+ * @sw_secret: the secret to be derived which is exactly the secret size
+ * @sw_secret_size: size of the sw_secret
+ *
+ * Derive a software secret from a hardware wrapped key for software crypto
+ * operations.
+ * For wrapped keys, the key needs to be unwrapped, in order to derive a
+ * software secret, which can be done in the hardware from a secure execution
+ * environment.
+ *
+ * For more information on sw secret, please refer to "Hardware-wrapped keys"
+ * section of Documentation/block/inline-encryption.rst.
+ *
+ * Return: 0 on success; -errno on failure.
+ */
+int qcom_scm_derive_sw_secret(const u8 *wkey, size_t wkey_size,
+ u8 *sw_secret, size_t sw_secret_size)
+{
+ struct qcom_scm_desc desc = {
+ .svc = QCOM_SCM_SVC_ES,
+ .cmd = QCOM_SCM_ES_DERIVE_SW_SECRET,
+ .arginfo = QCOM_SCM_ARGS(4, QCOM_SCM_RW,
+ QCOM_SCM_VAL, QCOM_SCM_RW,
+ QCOM_SCM_VAL),
+ .args[1] = wkey_size,
+ .args[3] = sw_secret_size,
+ .owner = ARM_SMCCC_OWNER_SIP,
+ };
+
+ void *wkey_buf, *secret_buf;
+ dma_addr_t wkey_phys, secret_phys;
+ int ret;
+
+ /*
+ * Like qcom_scm_ice_set_key(), we use dma_alloc_coherent() to properly
+ * get a physical address, while guaranteeing that we can zeroize the
+ * key material later using memzero_explicit().
+ */
+ wkey_buf = dma_alloc_coherent(__scm->dev, wkey_size, &wkey_phys, GFP_KERNEL);
+ if (!wkey_buf)
+ return -ENOMEM;
+ secret_buf = dma_alloc_coherent(__scm->dev, sw_secret_size, &secret_phys, GFP_KERNEL);
+ if (!secret_buf) {
+ ret = -ENOMEM;
+ goto err_free_wrapped;
+ }
+
+ memcpy(wkey_buf, wkey, wkey_size);
+ desc.args[0] = wkey_phys;
+ desc.args[2] = secret_phys;
+
+ ret = qcom_scm_call(__scm->dev, &desc, NULL);
+ if (!ret)
+ memcpy(sw_secret, secret_buf, sw_secret_size);
+
+ memzero_explicit(secret_buf, sw_secret_size);
+
+ dma_free_coherent(__scm->dev, sw_secret_size, secret_buf, secret_phys);
+
+err_free_wrapped:
+ memzero_explicit(wkey_buf, wkey_size);
+
+ dma_free_coherent(__scm->dev, wkey_size, wkey_buf, wkey_phys);
+
+ return ret;
+}
+EXPORT_SYMBOL(qcom_scm_derive_sw_secret);
+
/**
* qcom_scm_hdcp_available() - Check if secure environment supports HDCP.
*
diff --git a/drivers/firmware/qcom/qcom_scm.h b/drivers/firmware/qcom/qcom_scm.h
index 4532907e8489..c75456aa6ac5 100644
--- a/drivers/firmware/qcom/qcom_scm.h
+++ b/drivers/firmware/qcom/qcom_scm.h
@@ -121,6 +121,7 @@ int scm_legacy_call(struct device *dev, const struct qcom_scm_desc *desc,
#define QCOM_SCM_SVC_ES 0x10 /* Enterprise Security */
#define QCOM_SCM_ES_INVALIDATE_ICE_KEY 0x03
#define QCOM_SCM_ES_CONFIG_SET_ICE_KEY 0x04
+#define QCOM_SCM_ES_DERIVE_SW_SECRET 0x07
#define QCOM_SCM_SVC_HDCP 0x11
#define QCOM_SCM_HDCP_INVOKE 0x01
diff --git a/include/linux/firmware/qcom/qcom_scm.h b/include/linux/firmware/qcom/qcom_scm.h
index ccaf28846054..c65f2d61492d 100644
--- a/include/linux/firmware/qcom/qcom_scm.h
+++ b/include/linux/firmware/qcom/qcom_scm.h
@@ -103,6 +103,8 @@ bool qcom_scm_ice_available(void);
int qcom_scm_ice_invalidate_key(u32 index);
int qcom_scm_ice_set_key(u32 index, const u8 *key, u32 key_size,
enum qcom_scm_ice_cipher cipher, u32 data_unit_size);
+int qcom_scm_derive_sw_secret(const u8 *wkey, size_t wkey_size,
+ u8 *sw_secret, size_t sw_secret_size);
bool qcom_scm_hdcp_available(void);
int qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt, u32 *resp);
--
2.25.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [PATCH v3 02/12] qcom_scm: scm call for deriving a software secret
2023-11-22 5:38 ` [PATCH v3 02/12] qcom_scm: scm call for deriving a software secret Gaurav Kashyap
@ 2023-11-22 17:43 ` Trilok Soni
2023-12-08 6:38 ` Om Prakash Singh
1 sibling, 0 replies; 39+ messages in thread
From: Trilok Soni @ 2023-11-22 17:43 UTC (permalink / raw)
To: Gaurav Kashyap, linux-scsi, linux-arm-msm, ebiggers,
neil.armstrong, srinivas.kandagatla
Cc: linux-mmc, linux-block, linux-fscrypt, omprsing, quic_psodagud,
abel.vesa, quic_spuppala, kernel
On 11/21/2023 9:38 PM, Gaurav Kashyap wrote:
> +
> + dma_free_coherent(__scm->dev, wkey_size, wkey_buf, wkey_phys);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(qcom_scm_derive_sw_secret);
GPL please.
--
---Trilok Soni
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v3 02/12] qcom_scm: scm call for deriving a software secret
2023-11-22 5:38 ` [PATCH v3 02/12] qcom_scm: scm call for deriving a software secret Gaurav Kashyap
2023-11-22 17:43 ` Trilok Soni
@ 2023-12-08 6:38 ` Om Prakash Singh
2023-12-12 4:09 ` Gaurav Kashyap
1 sibling, 1 reply; 39+ messages in thread
From: Om Prakash Singh @ 2023-12-08 6:38 UTC (permalink / raw)
To: Gaurav Kashyap, linux-scsi, linux-arm-msm, ebiggers,
neil.armstrong, srinivas.kandagatla
Cc: linux-mmc, linux-block, linux-fscrypt, omprsing, quic_psodagud,
abel.vesa, quic_spuppala, kernel
On 11/22/2023 11:08 AM, Gaurav Kashyap wrote:
> Inline storage encryption requires deriving a sw secret from
> the hardware wrapped keys. For non-wrapped keys, this can be
> directly done as keys are in the clear.
>
> However, when keys are hardware wrapped, it can be unwrapped
> by HWKM (Hardware Key Manager) which is accessible only from Qualcomm
> Trustzone. Hence, it also makes sense that the software secret is also
> derived there and returned to the linux kernel . This can be invoked by
> using the crypto profile APIs provided by the block layer.
>
> Signed-off-by: Gaurav Kashyap <quic_gaurkash@quicinc.com>
> ---
> drivers/firmware/qcom/qcom_scm.c | 71 ++++++++++++++++++++++++++
> drivers/firmware/qcom/qcom_scm.h | 1 +
> include/linux/firmware/qcom/qcom_scm.h | 2 +
> 3 files changed, 74 insertions(+)
>
> diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
> index 520de9b5633a..6dfb913f3e33 100644
> --- a/drivers/firmware/qcom/qcom_scm.c
> +++ b/drivers/firmware/qcom/qcom_scm.c
> @@ -1214,6 +1214,77 @@ int qcom_scm_ice_set_key(u32 index, const u8 *key, u32 key_size,
> }
> EXPORT_SYMBOL_GPL(qcom_scm_ice_set_key);
>
> +/**
> + * qcom_scm_derive_sw_secret() - Derive software secret from wrapped key
> + * @wkey: the hardware wrapped key inaccessible to software
> + * @wkey_size: size of the wrapped key
> + * @sw_secret: the secret to be derived which is exactly the secret size
> + * @sw_secret_size: size of the sw_secret
> + *
> + * Derive a software secret from a hardware wrapped key for software crypto
> + * operations.
> + * For wrapped keys, the key needs to be unwrapped, in order to derive a
> + * software secret, which can be done in the hardware from a secure execution
> + * environment.
> + *
> + * For more information on sw secret, please refer to "Hardware-wrapped keys"
> + * section of Documentation/block/inline-encryption.rst.
> + *
> + * Return: 0 on success; -errno on failure.
> + */
> +int qcom_scm_derive_sw_secret(const u8 *wkey, size_t wkey_size,
> + u8 *sw_secret, size_t sw_secret_size)
> +{
> + struct qcom_scm_desc desc = {
> + .svc = QCOM_SCM_SVC_ES,
> + .cmd = QCOM_SCM_ES_DERIVE_SW_SECRET,
> + .arginfo = QCOM_SCM_ARGS(4, QCOM_SCM_RW,
> + QCOM_SCM_VAL, QCOM_SCM_RW,
> + QCOM_SCM_VAL),
> + .args[1] = wkey_size,
> + .args[3] = sw_secret_size,
> + .owner = ARM_SMCCC_OWNER_SIP,
> + };
> +
> + void *wkey_buf, *secret_buf;
> + dma_addr_t wkey_phys, secret_phys;
> + int ret;
> +
> + /*
> + * Like qcom_scm_ice_set_key(), we use dma_alloc_coherent() to properly
> + * get a physical address, while guaranteeing that we can zeroize the
> + * key material later using memzero_explicit().
> + */
> + wkey_buf = dma_alloc_coherent(__scm->dev, wkey_size, &wkey_phys, GFP_KERNEL);
> + if (!wkey_buf)
> + return -ENOMEM;
> + secret_buf = dma_alloc_coherent(__scm->dev, sw_secret_size, &secret_phys, GFP_KERNEL);
> + if (!secret_buf) {
> + ret = -ENOMEM;
> + goto err_free_wrapped;
> + }
> +
> + memcpy(wkey_buf, wkey, wkey_size);
> + desc.args[0] = wkey_phys;
> + desc.args[2] = secret_phys;
> +
> + ret = qcom_scm_call(__scm->dev, &desc, NULL);
> + if (!ret)
> + memcpy(sw_secret, secret_buf, sw_secret_size);
> +
> + memzero_explicit(secret_buf, sw_secret_size);
> +
> + dma_free_coherent(__scm->dev, sw_secret_size, secret_buf, secret_phys);
> +
> +err_free_wrapped:
> + memzero_explicit(wkey_buf, wkey_size);
In error handling case the operation is being performed on unallocated
memory.
> +
> + dma_free_coherent(__scm->dev, wkey_size, wkey_buf, wkey_phys);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(qcom_scm_derive_sw_secret);
> +
> /**
> * qcom_scm_hdcp_available() - Check if secure environment supports HDCP.
> *
> diff --git a/drivers/firmware/qcom/qcom_scm.h b/drivers/firmware/qcom/qcom_scm.h
> index 4532907e8489..c75456aa6ac5 100644
> --- a/drivers/firmware/qcom/qcom_scm.h
> +++ b/drivers/firmware/qcom/qcom_scm.h
> @@ -121,6 +121,7 @@ int scm_legacy_call(struct device *dev, const struct qcom_scm_desc *desc,
> #define QCOM_SCM_SVC_ES 0x10 /* Enterprise Security */
> #define QCOM_SCM_ES_INVALIDATE_ICE_KEY 0x03
> #define QCOM_SCM_ES_CONFIG_SET_ICE_KEY 0x04
> +#define QCOM_SCM_ES_DERIVE_SW_SECRET 0x07
>
> #define QCOM_SCM_SVC_HDCP 0x11
> #define QCOM_SCM_HDCP_INVOKE 0x01
> diff --git a/include/linux/firmware/qcom/qcom_scm.h b/include/linux/firmware/qcom/qcom_scm.h
> index ccaf28846054..c65f2d61492d 100644
> --- a/include/linux/firmware/qcom/qcom_scm.h
> +++ b/include/linux/firmware/qcom/qcom_scm.h
> @@ -103,6 +103,8 @@ bool qcom_scm_ice_available(void);
> int qcom_scm_ice_invalidate_key(u32 index);
> int qcom_scm_ice_set_key(u32 index, const u8 *key, u32 key_size,
> enum qcom_scm_ice_cipher cipher, u32 data_unit_size);
> +int qcom_scm_derive_sw_secret(const u8 *wkey, size_t wkey_size,
> + u8 *sw_secret, size_t sw_secret_size);
>
> bool qcom_scm_hdcp_available(void);
> int qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt, u32 *resp);
^ permalink raw reply [flat|nested] 39+ messages in thread* RE: [PATCH v3 02/12] qcom_scm: scm call for deriving a software secret
2023-12-08 6:38 ` Om Prakash Singh
@ 2023-12-12 4:09 ` Gaurav Kashyap
0 siblings, 0 replies; 39+ messages in thread
From: Gaurav Kashyap @ 2023-12-12 4:09 UTC (permalink / raw)
To: Om Prakash Singh (QUIC), Gaurav Kashyap (QUIC),
linux-scsi, linux-arm-msm, ebiggers, neil.armstrong,
srinivas.kandagatla
Cc: linux-mmc, linux-block, linux-fscrypt, Om Prakash Singh,
Prasad Sodagudi (QUIC), abel.vesa, Seshu Madhavi Puppala (QUIC),
kernel
Hello Om,
On 12/07/2023, Om Prakash Singh wrote:
> On 11/22/2023 11:08 AM, Gaurav Kashyap wrote:
> > Inline storage encryption requires deriving a sw secret from the
> > hardware wrapped keys. For non-wrapped keys, this can be directly done
> > as keys are in the clear.
> >
> > However, when keys are hardware wrapped, it can be unwrapped by
> HWKM
> > (Hardware Key Manager) which is accessible only from Qualcomm
> > Trustzone. Hence, it also makes sense that the software secret is also
> > derived there and returned to the linux kernel . This can be invoked
> > by using the crypto profile APIs provided by the block layer.
> >
> > Signed-off-by: Gaurav Kashyap <quic_gaurkash@quicinc.com>
> > ---
> > drivers/firmware/qcom/qcom_scm.c | 71
> ++++++++++++++++++++++++++
> > drivers/firmware/qcom/qcom_scm.h | 1 +
> > include/linux/firmware/qcom/qcom_scm.h | 2 +
> > 3 files changed, 74 insertions(+)
> >
> > diff --git a/drivers/firmware/qcom/qcom_scm.c
> > b/drivers/firmware/qcom/qcom_scm.c
> > index 520de9b5633a..6dfb913f3e33 100644
> > --- a/drivers/firmware/qcom/qcom_scm.c
> > +++ b/drivers/firmware/qcom/qcom_scm.c
> > @@ -1214,6 +1214,77 @@ int qcom_scm_ice_set_key(u32 index, const u8
> *key, u32 key_size,
> > }
> > EXPORT_SYMBOL_GPL(qcom_scm_ice_set_key);
> >
> > +/**
> > + * qcom_scm_derive_sw_secret() - Derive software secret from wrapped
> > +key
> > + * @wkey: the hardware wrapped key inaccessible to software
> > + * @wkey_size: size of the wrapped key
> > + * @sw_secret: the secret to be derived which is exactly the secret
> > +size
> > + * @sw_secret_size: size of the sw_secret
> > + *
> > + * Derive a software secret from a hardware wrapped key for software
> > +crypto
> > + * operations.
> > + * For wrapped keys, the key needs to be unwrapped, in order to
> > +derive a
> > + * software secret, which can be done in the hardware from a secure
> > +execution
> > + * environment.
> > + *
> > + * For more information on sw secret, please refer to "Hardware-wrapped
> keys"
> > + * section of Documentation/block/inline-encryption.rst.
> > + *
> > + * Return: 0 on success; -errno on failure.
> > + */
> > +int qcom_scm_derive_sw_secret(const u8 *wkey, size_t wkey_size,
> > + u8 *sw_secret, size_t sw_secret_size) {
> > + struct qcom_scm_desc desc = {
> > + .svc = QCOM_SCM_SVC_ES,
> > + .cmd = QCOM_SCM_ES_DERIVE_SW_SECRET,
> > + .arginfo = QCOM_SCM_ARGS(4, QCOM_SCM_RW,
> > + QCOM_SCM_VAL, QCOM_SCM_RW,
> > + QCOM_SCM_VAL),
> > + .args[1] = wkey_size,
> > + .args[3] = sw_secret_size,
> > + .owner = ARM_SMCCC_OWNER_SIP,
> > + };
> > +
> > + void *wkey_buf, *secret_buf;
> > + dma_addr_t wkey_phys, secret_phys;
> > + int ret;
> > +
> > + /*
> > + * Like qcom_scm_ice_set_key(), we use dma_alloc_coherent() to
> properly
> > + * get a physical address, while guaranteeing that we can zeroize the
> > + * key material later using memzero_explicit().
> > + */
> > + wkey_buf = dma_alloc_coherent(__scm->dev, wkey_size,
> &wkey_phys, GFP_KERNEL);
> > + if (!wkey_buf)
> > + return -ENOMEM;
> > + secret_buf = dma_alloc_coherent(__scm->dev, sw_secret_size,
> &secret_phys, GFP_KERNEL);
> > + if (!secret_buf) {
> > + ret = -ENOMEM;
> > + goto err_free_wrapped;
> > + }
> > +
> > + memcpy(wkey_buf, wkey, wkey_size);
> > + desc.args[0] = wkey_phys;
> > + desc.args[2] = secret_phys;
> > +
> > + ret = qcom_scm_call(__scm->dev, &desc, NULL);
> > + if (!ret)
> > + memcpy(sw_secret, secret_buf, sw_secret_size);
> > +
> > + memzero_explicit(secret_buf, sw_secret_size);
> > +
> > + dma_free_coherent(__scm->dev, sw_secret_size, secret_buf,
> > +secret_phys);
> > +
> > +err_free_wrapped:
> > + memzero_explicit(wkey_buf, wkey_size);
> In error handling case the operation is being performed on unallocated
> memory.
The flow comes here only when secret_buf allocation fails.
And at that point, wkey_buf is already allocated.
Wkey_buf failure errors out directly after allocation failure.
> > +
> > + dma_free_coherent(__scm->dev, wkey_size, wkey_buf,
> wkey_phys);
> > +
> > + return ret;
> > +}
> > +EXPORT_SYMBOL(qcom_scm_derive_sw_secret);
> > +
> > /**
> > * qcom_scm_hdcp_available() - Check if secure environment supports
> HDCP.
> > *
> > diff --git a/drivers/firmware/qcom/qcom_scm.h
> > b/drivers/firmware/qcom/qcom_scm.h
> > index 4532907e8489..c75456aa6ac5 100644
> > --- a/drivers/firmware/qcom/qcom_scm.h
> > +++ b/drivers/firmware/qcom/qcom_scm.h
> > @@ -121,6 +121,7 @@ int scm_legacy_call(struct device *dev, const struct
> qcom_scm_desc *desc,
> > #define QCOM_SCM_SVC_ES 0x10 /* Enterprise
> Security */
> > #define QCOM_SCM_ES_INVALIDATE_ICE_KEY 0x03
> > #define QCOM_SCM_ES_CONFIG_SET_ICE_KEY 0x04
> > +#define QCOM_SCM_ES_DERIVE_SW_SECRET 0x07
> >
> > #define QCOM_SCM_SVC_HDCP 0x11
> > #define QCOM_SCM_HDCP_INVOKE 0x01
> > diff --git a/include/linux/firmware/qcom/qcom_scm.h
> > b/include/linux/firmware/qcom/qcom_scm.h
> > index ccaf28846054..c65f2d61492d 100644
> > --- a/include/linux/firmware/qcom/qcom_scm.h
> > +++ b/include/linux/firmware/qcom/qcom_scm.h
> > @@ -103,6 +103,8 @@ bool qcom_scm_ice_available(void);
> > int qcom_scm_ice_invalidate_key(u32 index);
> > int qcom_scm_ice_set_key(u32 index, const u8 *key, u32 key_size,
> > enum qcom_scm_ice_cipher cipher, u32
> data_unit_size);
> > +int qcom_scm_derive_sw_secret(const u8 *wkey, size_t wkey_size,
> > + u8 *sw_secret, size_t sw_secret_size);
> >
> > bool qcom_scm_hdcp_available(void);
> > int qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt,
> > u32 *resp);
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v3 03/12] soc: qcom: ice: add hwkm support in ice
2023-11-22 5:38 [PATCH v3 00/12] Hardware wrapped key support for qcom ice and ufs Gaurav Kashyap
2023-11-22 5:38 ` [PATCH v3 01/12] ice, ufs, mmc: use blk_crypto_key for program_key Gaurav Kashyap
2023-11-22 5:38 ` [PATCH v3 02/12] qcom_scm: scm call for deriving a software secret Gaurav Kashyap
@ 2023-11-22 5:38 ` Gaurav Kashyap
2023-12-08 4:11 ` Bjorn Andersson
` (3 more replies)
2023-11-22 5:38 ` [PATCH v3 04/12] soc: qcom: ice: support for hardware wrapped keys Gaurav Kashyap
` (10 subsequent siblings)
13 siblings, 4 replies; 39+ messages in thread
From: Gaurav Kashyap @ 2023-11-22 5:38 UTC (permalink / raw)
To: linux-scsi, linux-arm-msm, ebiggers, neil.armstrong, srinivas.kandagatla
Cc: linux-mmc, linux-block, linux-fscrypt, omprsing, quic_psodagud,
abel.vesa, quic_spuppala, kernel, Gaurav Kashyap
Qualcomm's ICE (Inline Crypto Engine) contains a proprietary
key management hardware called Hardware Key Manager (HWKM).
This patch integrates HWKM support in ICE when it is
available. HWKM primarily provides hardware wrapped key support
where the ICE (storage) keys are not available in software and
protected in hardware.
Signed-off-by: Gaurav Kashyap <quic_gaurkash@quicinc.com>
---
drivers/soc/qcom/ice.c | 133 ++++++++++++++++++++++++++++++++++++++++-
include/soc/qcom/ice.h | 1 +
2 files changed, 133 insertions(+), 1 deletion(-)
diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c
index 6f941d32fffb..adf9cab848fa 100644
--- a/drivers/soc/qcom/ice.c
+++ b/drivers/soc/qcom/ice.c
@@ -26,6 +26,19 @@
#define QCOM_ICE_REG_FUSE_SETTING 0x0010
#define QCOM_ICE_REG_BIST_STATUS 0x0070
#define QCOM_ICE_REG_ADVANCED_CONTROL 0x1000
+#define QCOM_ICE_REG_CONTROL 0x0
+/* QCOM ICE HWKM registers */
+#define QCOM_ICE_REG_HWKM_TZ_KM_CTL 0x1000
+#define QCOM_ICE_REG_HWKM_TZ_KM_STATUS 0x1004
+#define QCOM_ICE_REG_HWKM_BANK0_BANKN_IRQ_STATUS 0x2008
+#define QCOM_ICE_REG_HWKM_BANK0_BBAC_0 0x5000
+#define QCOM_ICE_REG_HWKM_BANK0_BBAC_1 0x5004
+#define QCOM_ICE_REG_HWKM_BANK0_BBAC_2 0x5008
+#define QCOM_ICE_REG_HWKM_BANK0_BBAC_3 0x500C
+#define QCOM_ICE_REG_HWKM_BANK0_BBAC_4 0x5010
+
+#define QCOM_ICE_HWKM_BIST_DONE_V1_VAL 0x11
+#define QCOM_ICE_HWKM_BIST_DONE_V2_VAL 0x287
/* BIST ("built-in self-test") status flags */
#define QCOM_ICE_BIST_STATUS_MASK GENMASK(31, 28)
@@ -34,6 +47,9 @@
#define QCOM_ICE_FORCE_HW_KEY0_SETTING_MASK 0x2
#define QCOM_ICE_FORCE_HW_KEY1_SETTING_MASK 0x4
+#define QCOM_ICE_HWKM_REG_OFFSET 0x8000
+#define HWKM_OFFSET(reg) ((reg) + QCOM_ICE_HWKM_REG_OFFSET)
+
#define qcom_ice_writel(engine, val, reg) \
writel((val), (engine)->base + (reg))
@@ -46,6 +62,9 @@ struct qcom_ice {
struct device_link *link;
struct clk *core_clk;
+ u8 hwkm_version;
+ bool use_hwkm;
+ bool hwkm_init_complete;
};
static bool qcom_ice_check_supported(struct qcom_ice *ice)
@@ -63,8 +82,26 @@ static bool qcom_ice_check_supported(struct qcom_ice *ice)
return false;
}
+ if (major >= 4 || (major == 3 && minor == 2 && step >= 1))
+ ice->hwkm_version = 2;
+ else if (major == 3 && minor == 2)
+ ice->hwkm_version = 1;
+ else
+ ice->hwkm_version = 0;
+
+ if (ice->hwkm_version == 0)
+ ice->use_hwkm = false;
+
dev_info(dev, "Found QC Inline Crypto Engine (ICE) v%d.%d.%d\n",
major, minor, step);
+ if (!ice->hwkm_version)
+ dev_info(dev, "QC ICE HWKM (Hardware Key Manager) not supported");
+ else
+ dev_info(dev, "QC ICE HWKM (Hardware Key Manager) version = %d",
+ ice->hwkm_version);
+
+ if (!ice->use_hwkm)
+ dev_info(dev, "QC ICE HWKM (Hardware Key Manager) not used");
/* If fuses are blown, ICE might not work in the standard way. */
regval = qcom_ice_readl(ice, QCOM_ICE_REG_FUSE_SETTING);
@@ -113,10 +150,14 @@ static void qcom_ice_optimization_enable(struct qcom_ice *ice)
* fails, so we needn't do it in software too, and (c) properly testing
* storage encryption requires testing the full storage stack anyway,
* and not relying on hardware-level self-tests.
+ *
+ * However, we still care about if HWKM BIST failed (when supported) as
+ * important functionality would fail later, so disable hwkm on failure.
*/
static int qcom_ice_wait_bist_status(struct qcom_ice *ice)
{
u32 regval;
+ u32 bist_done_val;
int err;
err = readl_poll_timeout(ice->base + QCOM_ICE_REG_BIST_STATUS,
@@ -125,15 +166,95 @@ static int qcom_ice_wait_bist_status(struct qcom_ice *ice)
if (err)
dev_err(ice->dev, "Timed out waiting for ICE self-test to complete\n");
+ if (ice->use_hwkm) {
+ bist_done_val = (ice->hwkm_version == 1) ?
+ QCOM_ICE_HWKM_BIST_DONE_V1_VAL :
+ QCOM_ICE_HWKM_BIST_DONE_V2_VAL;
+ if (qcom_ice_readl(ice,
+ HWKM_OFFSET(QCOM_ICE_REG_HWKM_TZ_KM_STATUS)) !=
+ bist_done_val) {
+ dev_warn(ice->dev, "HWKM BIST error\n");
+ ice->use_hwkm = false;
+ }
+ }
return err;
}
+static void qcom_ice_enable_standard_mode(struct qcom_ice *ice)
+{
+ u32 val = 0;
+
+ if (!ice->use_hwkm)
+ return;
+
+ /*
+ * When ICE is in standard (hwkm) mode, it supports HW wrapped
+ * keys, and when it is in legacy mode, it only supports standard
+ * (non HW wrapped) keys.
+ *
+ * Put ICE in standard mode, ICE defaults to legacy mode.
+ * Legacy mode - ICE HWKM slave not supported.
+ * Standard mode - ICE HWKM slave supported.
+ *
+ * Depending on the version of HWKM, it is controlled by different
+ * registers in ICE.
+ */
+ if (ice->hwkm_version >= 2) {
+ val = qcom_ice_readl(ice, QCOM_ICE_REG_CONTROL);
+ val = val & 0xFFFFFFFE;
+ qcom_ice_writel(ice, val, QCOM_ICE_REG_CONTROL);
+ } else {
+ qcom_ice_writel(ice, 0x7,
+ HWKM_OFFSET(QCOM_ICE_REG_HWKM_TZ_KM_CTL));
+ }
+}
+
+static void qcom_ice_hwkm_init(struct qcom_ice *ice)
+{
+ if (!ice->use_hwkm)
+ return;
+
+ /* Disable CRC checks. This HWKM feature is not used. */
+ qcom_ice_writel(ice, 0x6,
+ HWKM_OFFSET(QCOM_ICE_REG_HWKM_TZ_KM_CTL));
+
+ /*
+ * Give register bank of the HWKM slave access to read and modify
+ * the keyslots in ICE HWKM slave. Without this, trustzone will not
+ * be able to program keys into ICE.
+ */
+ qcom_ice_writel(ice, 0xFFFFFFFF,
+ HWKM_OFFSET(QCOM_ICE_REG_HWKM_BANK0_BBAC_0));
+ qcom_ice_writel(ice, 0xFFFFFFFF,
+ HWKM_OFFSET(QCOM_ICE_REG_HWKM_BANK0_BBAC_1));
+ qcom_ice_writel(ice, 0xFFFFFFFF,
+ HWKM_OFFSET(QCOM_ICE_REG_HWKM_BANK0_BBAC_2));
+ qcom_ice_writel(ice, 0xFFFFFFFF,
+ HWKM_OFFSET(QCOM_ICE_REG_HWKM_BANK0_BBAC_3));
+ qcom_ice_writel(ice, 0xFFFFFFFF,
+ HWKM_OFFSET(QCOM_ICE_REG_HWKM_BANK0_BBAC_4));
+
+ /* Clear HWKM response FIFO before doing anything */
+ qcom_ice_writel(ice, 0x8,
+ HWKM_OFFSET(QCOM_ICE_REG_HWKM_BANK0_BANKN_IRQ_STATUS));
+}
+
int qcom_ice_enable(struct qcom_ice *ice)
{
+ int err;
+
qcom_ice_low_power_mode_enable(ice);
qcom_ice_optimization_enable(ice);
- return qcom_ice_wait_bist_status(ice);
+ qcom_ice_enable_standard_mode(ice);
+
+ err = qcom_ice_wait_bist_status(ice);
+ if (err)
+ return err;
+
+ qcom_ice_hwkm_init(ice);
+
+ return err;
}
EXPORT_SYMBOL_GPL(qcom_ice_enable);
@@ -149,6 +270,8 @@ int qcom_ice_resume(struct qcom_ice *ice)
return err;
}
+ qcom_ice_enable_standard_mode(ice);
+ qcom_ice_hwkm_init(ice);
return qcom_ice_wait_bist_status(ice);
}
EXPORT_SYMBOL_GPL(qcom_ice_resume);
@@ -205,6 +328,12 @@ int qcom_ice_evict_key(struct qcom_ice *ice, int slot)
}
EXPORT_SYMBOL_GPL(qcom_ice_evict_key);
+bool qcom_ice_hwkm_supported(struct qcom_ice *ice)
+{
+ return ice->use_hwkm;
+}
+EXPORT_SYMBOL_GPL(qcom_ice_hwkm_supported);
+
static struct qcom_ice *qcom_ice_create(struct device *dev,
void __iomem *base)
{
@@ -239,6 +368,8 @@ static struct qcom_ice *qcom_ice_create(struct device *dev,
engine->core_clk = devm_clk_get_enabled(dev, NULL);
if (IS_ERR(engine->core_clk))
return ERR_CAST(engine->core_clk);
+ engine->use_hwkm = of_property_read_bool(dev->of_node,
+ "qcom,ice-use-hwkm");
if (!qcom_ice_check_supported(engine))
return ERR_PTR(-EOPNOTSUPP);
diff --git a/include/soc/qcom/ice.h b/include/soc/qcom/ice.h
index 9dd835dba2a7..1f52e82e3e1c 100644
--- a/include/soc/qcom/ice.h
+++ b/include/soc/qcom/ice.h
@@ -34,5 +34,6 @@ int qcom_ice_program_key(struct qcom_ice *ice,
const struct blk_crypto_key *bkey,
u8 data_unit_size, int slot);
int qcom_ice_evict_key(struct qcom_ice *ice, int slot);
+bool qcom_ice_hwkm_supported(struct qcom_ice *ice);
struct qcom_ice *of_qcom_ice_get(struct device *dev);
#endif /* __QCOM_ICE_H__ */
--
2.25.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [PATCH v3 03/12] soc: qcom: ice: add hwkm support in ice
2023-11-22 5:38 ` [PATCH v3 03/12] soc: qcom: ice: add hwkm support in ice Gaurav Kashyap
@ 2023-12-08 4:11 ` Bjorn Andersson
2023-12-12 3:53 ` Gaurav Kashyap
2023-12-08 6:04 ` Om Prakash Singh
` (2 subsequent siblings)
3 siblings, 1 reply; 39+ messages in thread
From: Bjorn Andersson @ 2023-12-08 4:11 UTC (permalink / raw)
To: Gaurav Kashyap
Cc: linux-scsi, linux-arm-msm, ebiggers, neil.armstrong,
srinivas.kandagatla, linux-mmc, linux-block, linux-fscrypt,
omprsing, quic_psodagud, abel.vesa, quic_spuppala, kernel
On Tue, Nov 21, 2023 at 09:38:08PM -0800, Gaurav Kashyap wrote:
> Qualcomm's ICE (Inline Crypto Engine) contains a proprietary
> key management hardware called Hardware Key Manager (HWKM).
> This patch integrates HWKM support in ICE when it is
> available. HWKM primarily provides hardware wrapped key support
> where the ICE (storage) keys are not available in software and
> protected in hardware.
>
> Signed-off-by: Gaurav Kashyap <quic_gaurkash@quicinc.com>
> ---
> drivers/soc/qcom/ice.c | 133 ++++++++++++++++++++++++++++++++++++++++-
> include/soc/qcom/ice.h | 1 +
> 2 files changed, 133 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c
> index 6f941d32fffb..adf9cab848fa 100644
> --- a/drivers/soc/qcom/ice.c
> +++ b/drivers/soc/qcom/ice.c
> @@ -26,6 +26,19 @@
> #define QCOM_ICE_REG_FUSE_SETTING 0x0010
> #define QCOM_ICE_REG_BIST_STATUS 0x0070
> #define QCOM_ICE_REG_ADVANCED_CONTROL 0x1000
> +#define QCOM_ICE_REG_CONTROL 0x0
> +/* QCOM ICE HWKM registers */
> +#define QCOM_ICE_REG_HWKM_TZ_KM_CTL 0x1000
> +#define QCOM_ICE_REG_HWKM_TZ_KM_STATUS 0x1004
> +#define QCOM_ICE_REG_HWKM_BANK0_BANKN_IRQ_STATUS 0x2008
> +#define QCOM_ICE_REG_HWKM_BANK0_BBAC_0 0x5000
> +#define QCOM_ICE_REG_HWKM_BANK0_BBAC_1 0x5004
> +#define QCOM_ICE_REG_HWKM_BANK0_BBAC_2 0x5008
> +#define QCOM_ICE_REG_HWKM_BANK0_BBAC_3 0x500C
> +#define QCOM_ICE_REG_HWKM_BANK0_BBAC_4 0x5010
> +
> +#define QCOM_ICE_HWKM_BIST_DONE_V1_VAL 0x11
> +#define QCOM_ICE_HWKM_BIST_DONE_V2_VAL 0x287
>
> /* BIST ("built-in self-test") status flags */
> #define QCOM_ICE_BIST_STATUS_MASK GENMASK(31, 28)
> @@ -34,6 +47,9 @@
> #define QCOM_ICE_FORCE_HW_KEY0_SETTING_MASK 0x2
> #define QCOM_ICE_FORCE_HW_KEY1_SETTING_MASK 0x4
>
> +#define QCOM_ICE_HWKM_REG_OFFSET 0x8000
> +#define HWKM_OFFSET(reg) ((reg) + QCOM_ICE_HWKM_REG_OFFSET)
> +
> #define qcom_ice_writel(engine, val, reg) \
> writel((val), (engine)->base + (reg))
>
> @@ -46,6 +62,9 @@ struct qcom_ice {
> struct device_link *link;
>
> struct clk *core_clk;
> + u8 hwkm_version;
> + bool use_hwkm;
> + bool hwkm_init_complete;
> };
>
> static bool qcom_ice_check_supported(struct qcom_ice *ice)
> @@ -63,8 +82,26 @@ static bool qcom_ice_check_supported(struct qcom_ice *ice)
> return false;
> }
>
> + if (major >= 4 || (major == 3 && minor == 2 && step >= 1))
> + ice->hwkm_version = 2;
> + else if (major == 3 && minor == 2)
> + ice->hwkm_version = 1;
> + else
> + ice->hwkm_version = 0;
> +
> + if (ice->hwkm_version == 0)
> + ice->use_hwkm = false;
> +
> dev_info(dev, "Found QC Inline Crypto Engine (ICE) v%d.%d.%d\n",
> major, minor, step);
> + if (!ice->hwkm_version)
> + dev_info(dev, "QC ICE HWKM (Hardware Key Manager) not supported");
So for a version < 3.2.0, we will dev_info() three times, one stating
the version found, one stating that HWKM is not supported, and then
below one saying that HWKM is not used.
> + else
> + dev_info(dev, "QC ICE HWKM (Hardware Key Manager) version = %d",
> + ice->hwkm_version);
And for version >= 3.2.0 we will dev_info() two times.
To the vast majority of readers of the kernel log none of these
info-prints are useful - it's just spam.
I'd prefer that it was turned into dev_dbg(), which those who want to
know (e.g. during bringup) can enable. But that's a separate change,
please start by consolidating your information into a single line
printed in the log.
> +
> + if (!ice->use_hwkm)
> + dev_info(dev, "QC ICE HWKM (Hardware Key Manager) not used");
>
> /* If fuses are blown, ICE might not work in the standard way. */
> regval = qcom_ice_readl(ice, QCOM_ICE_REG_FUSE_SETTING);
> @@ -113,10 +150,14 @@ static void qcom_ice_optimization_enable(struct qcom_ice *ice)
> * fails, so we needn't do it in software too, and (c) properly testing
> * storage encryption requires testing the full storage stack anyway,
> * and not relying on hardware-level self-tests.
> + *
> + * However, we still care about if HWKM BIST failed (when supported) as
> + * important functionality would fail later, so disable hwkm on failure.
> */
> static int qcom_ice_wait_bist_status(struct qcom_ice *ice)
> {
> u32 regval;
> + u32 bist_done_val;
The "val" suffix indicates that this would be a "value", but it's
actually a register offset. "bist_done_reg" would be better.
> int err;
>
> err = readl_poll_timeout(ice->base + QCOM_ICE_REG_BIST_STATUS,
> @@ -125,15 +166,95 @@ static int qcom_ice_wait_bist_status(struct qcom_ice *ice)
> if (err)
> dev_err(ice->dev, "Timed out waiting for ICE self-test to complete\n");
>
> + if (ice->use_hwkm) {
> + bist_done_val = (ice->hwkm_version == 1) ?
> + QCOM_ICE_HWKM_BIST_DONE_V1_VAL :
> + QCOM_ICE_HWKM_BIST_DONE_V2_VAL;
> + if (qcom_ice_readl(ice,
> + HWKM_OFFSET(QCOM_ICE_REG_HWKM_TZ_KM_STATUS)) !=
> + bist_done_val) {
> + dev_warn(ice->dev, "HWKM BIST error\n");
Sounds like a error to me, wouldn't dev_err() be suitable?
> + ice->use_hwkm = false;
> + }
> + }
> return err;
> }
>
> +static void qcom_ice_enable_standard_mode(struct qcom_ice *ice)
> +{
> + u32 val = 0;
> +
> + if (!ice->use_hwkm)
> + return;
> +
> + /*
> + * When ICE is in standard (hwkm) mode, it supports HW wrapped
> + * keys, and when it is in legacy mode, it only supports standard
> + * (non HW wrapped) keys.
> + *
> + * Put ICE in standard mode, ICE defaults to legacy mode.
> + * Legacy mode - ICE HWKM slave not supported.
> + * Standard mode - ICE HWKM slave supported.
> + *
> + * Depending on the version of HWKM, it is controlled by different
> + * registers in ICE.
> + */
> + if (ice->hwkm_version >= 2) {
> + val = qcom_ice_readl(ice, QCOM_ICE_REG_CONTROL);
> + val = val & 0xFFFFFFFE;
> + qcom_ice_writel(ice, val, QCOM_ICE_REG_CONTROL);
> + } else {
> + qcom_ice_writel(ice, 0x7,
> + HWKM_OFFSET(QCOM_ICE_REG_HWKM_TZ_KM_CTL));
> + }
> +}
> +
> +static void qcom_ice_hwkm_init(struct qcom_ice *ice)
> +{
> + if (!ice->use_hwkm)
> + return;
> +
> + /* Disable CRC checks. This HWKM feature is not used. */
> + qcom_ice_writel(ice, 0x6,
> + HWKM_OFFSET(QCOM_ICE_REG_HWKM_TZ_KM_CTL));
> +
> + /*
> + * Give register bank of the HWKM slave access to read and modify
> + * the keyslots in ICE HWKM slave. Without this, trustzone will not
> + * be able to program keys into ICE.
> + */
> + qcom_ice_writel(ice, 0xFFFFFFFF,
> + HWKM_OFFSET(QCOM_ICE_REG_HWKM_BANK0_BBAC_0));
This line is 86 characters long if left unwrapped. You're allowed to go
over 80 characters if it makes the code more readable, so please do so
for these and below.
> + qcom_ice_writel(ice, 0xFFFFFFFF,
> + HWKM_OFFSET(QCOM_ICE_REG_HWKM_BANK0_BBAC_1));
> + qcom_ice_writel(ice, 0xFFFFFFFF,
> + HWKM_OFFSET(QCOM_ICE_REG_HWKM_BANK0_BBAC_2));
> + qcom_ice_writel(ice, 0xFFFFFFFF,
> + HWKM_OFFSET(QCOM_ICE_REG_HWKM_BANK0_BBAC_3));
> + qcom_ice_writel(ice, 0xFFFFFFFF,
> + HWKM_OFFSET(QCOM_ICE_REG_HWKM_BANK0_BBAC_4));
> +
> + /* Clear HWKM response FIFO before doing anything */
> + qcom_ice_writel(ice, 0x8,
> + HWKM_OFFSET(QCOM_ICE_REG_HWKM_BANK0_BANKN_IRQ_STATUS));
> +}
> +
> int qcom_ice_enable(struct qcom_ice *ice)
> {
> + int err;
> +
> qcom_ice_low_power_mode_enable(ice);
> qcom_ice_optimization_enable(ice);
>
> - return qcom_ice_wait_bist_status(ice);
> + qcom_ice_enable_standard_mode(ice);
> +
> + err = qcom_ice_wait_bist_status(ice);
> + if (err)
> + return err;
> +
> + qcom_ice_hwkm_init(ice);
> +
> + return err;
> }
> EXPORT_SYMBOL_GPL(qcom_ice_enable);
>
> @@ -149,6 +270,8 @@ int qcom_ice_resume(struct qcom_ice *ice)
> return err;
> }
>
> + qcom_ice_enable_standard_mode(ice);
> + qcom_ice_hwkm_init(ice);
> return qcom_ice_wait_bist_status(ice);
> }
> EXPORT_SYMBOL_GPL(qcom_ice_resume);
> @@ -205,6 +328,12 @@ int qcom_ice_evict_key(struct qcom_ice *ice, int slot)
> }
> EXPORT_SYMBOL_GPL(qcom_ice_evict_key);
>
> +bool qcom_ice_hwkm_supported(struct qcom_ice *ice)
> +{
> + return ice->use_hwkm;
> +}
> +EXPORT_SYMBOL_GPL(qcom_ice_hwkm_supported);
> +
> static struct qcom_ice *qcom_ice_create(struct device *dev,
> void __iomem *base)
> {
> @@ -239,6 +368,8 @@ static struct qcom_ice *qcom_ice_create(struct device *dev,
> engine->core_clk = devm_clk_get_enabled(dev, NULL);
> if (IS_ERR(engine->core_clk))
> return ERR_CAST(engine->core_clk);
> + engine->use_hwkm = of_property_read_bool(dev->of_node,
> + "qcom,ice-use-hwkm");
Under what circumstances would we, with version >= 3.2, not specify this
flag?
Thanks,
Bjorn
>
> if (!qcom_ice_check_supported(engine))
> return ERR_PTR(-EOPNOTSUPP);
> diff --git a/include/soc/qcom/ice.h b/include/soc/qcom/ice.h
> index 9dd835dba2a7..1f52e82e3e1c 100644
> --- a/include/soc/qcom/ice.h
> +++ b/include/soc/qcom/ice.h
> @@ -34,5 +34,6 @@ int qcom_ice_program_key(struct qcom_ice *ice,
> const struct blk_crypto_key *bkey,
> u8 data_unit_size, int slot);
> int qcom_ice_evict_key(struct qcom_ice *ice, int slot);
> +bool qcom_ice_hwkm_supported(struct qcom_ice *ice);
> struct qcom_ice *of_qcom_ice_get(struct device *dev);
> #endif /* __QCOM_ICE_H__ */
> --
> 2.25.1
>
>
^ permalink raw reply [flat|nested] 39+ messages in thread* RE: [PATCH v3 03/12] soc: qcom: ice: add hwkm support in ice
2023-12-08 4:11 ` Bjorn Andersson
@ 2023-12-12 3:53 ` Gaurav Kashyap
0 siblings, 0 replies; 39+ messages in thread
From: Gaurav Kashyap @ 2023-12-12 3:53 UTC (permalink / raw)
To: Bjorn Andersson, Gaurav Kashyap (QUIC)
Cc: linux-scsi, linux-arm-msm, ebiggers, neil.armstrong,
srinivas.kandagatla, linux-mmc, linux-block, linux-fscrypt,
Om Prakash Singh, Prasad Sodagudi (QUIC),
abel.vesa, Seshu Madhavi Puppala (QUIC),
kernel
Hello Bjorn,
On 12/07/2023, Bjorn Andersson wrote:
> On Tue, Nov 21, 2023 at 09:38:08PM -0800, Gaurav Kashyap wrote:
> > Qualcomm's ICE (Inline Crypto Engine) contains a proprietary key
> > management hardware called Hardware Key Manager (HWKM).
> > This patch integrates HWKM support in ICE when it is available. HWKM
> > primarily provides hardware wrapped key support where the ICE
> > (storage) keys are not available in software and protected in
> > hardware.
> >
> > Signed-off-by: Gaurav Kashyap <quic_gaurkash@quicinc.com>
> > ---
> > drivers/soc/qcom/ice.c | 133
> ++++++++++++++++++++++++++++++++++++++++-
> > include/soc/qcom/ice.h | 1 +
> > 2 files changed, 133 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c index
> > 6f941d32fffb..adf9cab848fa 100644
> > --- a/drivers/soc/qcom/ice.c
> > +++ b/drivers/soc/qcom/ice.c
> > @@ -26,6 +26,19 @@
> > #define QCOM_ICE_REG_FUSE_SETTING 0x0010
> > #define QCOM_ICE_REG_BIST_STATUS 0x0070
> > #define QCOM_ICE_REG_ADVANCED_CONTROL 0x1000
> > +#define QCOM_ICE_REG_CONTROL 0x0
> > +/* QCOM ICE HWKM registers */
> > +#define QCOM_ICE_REG_HWKM_TZ_KM_CTL 0x1000
> > +#define QCOM_ICE_REG_HWKM_TZ_KM_STATUS 0x1004
> > +#define QCOM_ICE_REG_HWKM_BANK0_BANKN_IRQ_STATUS 0x2008
> > +#define QCOM_ICE_REG_HWKM_BANK0_BBAC_0 0x5000
> > +#define QCOM_ICE_REG_HWKM_BANK0_BBAC_1 0x5004
> > +#define QCOM_ICE_REG_HWKM_BANK0_BBAC_2 0x5008
> > +#define QCOM_ICE_REG_HWKM_BANK0_BBAC_3 0x500C
> > +#define QCOM_ICE_REG_HWKM_BANK0_BBAC_4 0x5010
> > +
> > +#define QCOM_ICE_HWKM_BIST_DONE_V1_VAL 0x11
> > +#define QCOM_ICE_HWKM_BIST_DONE_V2_VAL 0x287
> >
> > /* BIST ("built-in self-test") status flags */
> > #define QCOM_ICE_BIST_STATUS_MASK GENMASK(31, 28)
> > @@ -34,6 +47,9 @@
> > #define QCOM_ICE_FORCE_HW_KEY0_SETTING_MASK 0x2 #define
> > QCOM_ICE_FORCE_HW_KEY1_SETTING_MASK 0x4
> >
> > +#define QCOM_ICE_HWKM_REG_OFFSET 0x8000
> > +#define HWKM_OFFSET(reg) ((reg) +
> QCOM_ICE_HWKM_REG_OFFSET)
> > +
> > #define qcom_ice_writel(engine, val, reg) \
> > writel((val), (engine)->base + (reg))
> >
> > @@ -46,6 +62,9 @@ struct qcom_ice {
> > struct device_link *link;
> >
> > struct clk *core_clk;
> > + u8 hwkm_version;
> > + bool use_hwkm;
> > + bool hwkm_init_complete;
> > };
> >
> > static bool qcom_ice_check_supported(struct qcom_ice *ice) @@ -63,8
> > +82,26 @@ static bool qcom_ice_check_supported(struct qcom_ice *ice)
> > return false;
> > }
> >
> > + if (major >= 4 || (major == 3 && minor == 2 && step >= 1))
> > + ice->hwkm_version = 2;
> > + else if (major == 3 && minor == 2)
> > + ice->hwkm_version = 1;
> > + else
> > + ice->hwkm_version = 0;
> > +
> > + if (ice->hwkm_version == 0)
> > + ice->use_hwkm = false;
> > +
> > dev_info(dev, "Found QC Inline Crypto Engine (ICE) v%d.%d.%d\n",
> > major, minor, step);
> > + if (!ice->hwkm_version)
> > + dev_info(dev, "QC ICE HWKM (Hardware Key Manager) not
> > + supported");
>
> So for a version < 3.2.0, we will dev_info() three times, one stating the
> version found, one stating that HWKM is not supported, and then below one
> saying that HWKM is not used.
>
> > + else
> > + dev_info(dev, "QC ICE HWKM (Hardware Key Manager) version =
> %d",
> > + ice->hwkm_version);
>
> And for version >= 3.2.0 we will dev_info() two times.
>
>
> To the vast majority of readers of the kernel log none of these info-prints are
> useful - it's just spam.
>
> I'd prefer that it was turned into dev_dbg(), which those who want to know
> (e.g. during bringup) can enable. But that's a separate change, please start by
> consolidating your information into a single line printed in the log.
Noted for next patch.
>
> > +
> > + if (!ice->use_hwkm)
> > + dev_info(dev, "QC ICE HWKM (Hardware Key Manager) not
> > + used");
> >
> > /* If fuses are blown, ICE might not work in the standard way. */
> > regval = qcom_ice_readl(ice, QCOM_ICE_REG_FUSE_SETTING); @@
> > -113,10 +150,14 @@ static void qcom_ice_optimization_enable(struct
> qcom_ice *ice)
> > * fails, so we needn't do it in software too, and (c) properly testing
> > * storage encryption requires testing the full storage stack anyway,
> > * and not relying on hardware-level self-tests.
> > + *
> > + * However, we still care about if HWKM BIST failed (when supported)
> > + as
> > + * important functionality would fail later, so disable hwkm on failure.
> > */
> > static int qcom_ice_wait_bist_status(struct qcom_ice *ice) {
> > u32 regval;
> > + u32 bist_done_val;
>
> The "val" suffix indicates that this would be a "value", but it's actually a
> register offset. "bist_done_reg" would be better.
>
Noted for next patch.
> > int err;
> >
> > err = readl_poll_timeout(ice->base + QCOM_ICE_REG_BIST_STATUS,
> > @@ -125,15 +166,95 @@ static int qcom_ice_wait_bist_status(struct
> qcom_ice *ice)
> > if (err)
> > dev_err(ice->dev, "Timed out waiting for ICE self-test
> > to complete\n");
> >
> > + if (ice->use_hwkm) {
> > + bist_done_val = (ice->hwkm_version == 1) ?
> > + QCOM_ICE_HWKM_BIST_DONE_V1_VAL :
> > + QCOM_ICE_HWKM_BIST_DONE_V2_VAL;
> > + if (qcom_ice_readl(ice,
> > +
> HWKM_OFFSET(QCOM_ICE_REG_HWKM_TZ_KM_STATUS)) !=
> > + bist_done_val) {
> > + dev_warn(ice->dev, "HWKM BIST error\n");
>
> Sounds like a error to me, wouldn't dev_err() be suitable?
>
Yes, noted for next patch.
> > + ice->use_hwkm = false;
> > + }
> > + }
> > return err;
> > }
> >
> > +static void qcom_ice_enable_standard_mode(struct qcom_ice *ice) {
> > + u32 val = 0;
> > +
> > + if (!ice->use_hwkm)
> > + return;
> > +
> > + /*
> > + * When ICE is in standard (hwkm) mode, it supports HW wrapped
> > + * keys, and when it is in legacy mode, it only supports standard
> > + * (non HW wrapped) keys.
> > + *
> > + * Put ICE in standard mode, ICE defaults to legacy mode.
> > + * Legacy mode - ICE HWKM slave not supported.
> > + * Standard mode - ICE HWKM slave supported.
> > + *
> > + * Depending on the version of HWKM, it is controlled by different
> > + * registers in ICE.
> > + */
> > + if (ice->hwkm_version >= 2) {
> > + val = qcom_ice_readl(ice, QCOM_ICE_REG_CONTROL);
> > + val = val & 0xFFFFFFFE;
> > + qcom_ice_writel(ice, val, QCOM_ICE_REG_CONTROL);
> > + } else {
> > + qcom_ice_writel(ice, 0x7,
> > + HWKM_OFFSET(QCOM_ICE_REG_HWKM_TZ_KM_CTL));
> > + }
> > +}
> > +
> > +static void qcom_ice_hwkm_init(struct qcom_ice *ice) {
> > + if (!ice->use_hwkm)
> > + return;
> > +
> > + /* Disable CRC checks. This HWKM feature is not used. */
> > + qcom_ice_writel(ice, 0x6,
> > + HWKM_OFFSET(QCOM_ICE_REG_HWKM_TZ_KM_CTL));
> > +
> > + /*
> > + * Give register bank of the HWKM slave access to read and modify
> > + * the keyslots in ICE HWKM slave. Without this, trustzone will not
> > + * be able to program keys into ICE.
> > + */
> > + qcom_ice_writel(ice, 0xFFFFFFFF,
> > + HWKM_OFFSET(QCOM_ICE_REG_HWKM_BANK0_BBAC_0));
>
> This line is 86 characters long if left unwrapped. You're allowed to go over 80
> characters if it makes the code more readable, so please do so for these and
> below.
>
Noted for next patch.
> > + qcom_ice_writel(ice, 0xFFFFFFFF,
> > + HWKM_OFFSET(QCOM_ICE_REG_HWKM_BANK0_BBAC_1));
> > + qcom_ice_writel(ice, 0xFFFFFFFF,
> > + HWKM_OFFSET(QCOM_ICE_REG_HWKM_BANK0_BBAC_2));
> > + qcom_ice_writel(ice, 0xFFFFFFFF,
> > + HWKM_OFFSET(QCOM_ICE_REG_HWKM_BANK0_BBAC_3));
> > + qcom_ice_writel(ice, 0xFFFFFFFF,
> > + HWKM_OFFSET(QCOM_ICE_REG_HWKM_BANK0_BBAC_4));
> > +
> > + /* Clear HWKM response FIFO before doing anything */
> > + qcom_ice_writel(ice, 0x8,
> > +
> >
> +HWKM_OFFSET(QCOM_ICE_REG_HWKM_BANK0_BANKN_IRQ_STATUS));
> > +}
> > +
> > int qcom_ice_enable(struct qcom_ice *ice) {
> > + int err;
> > +
> > qcom_ice_low_power_mode_enable(ice);
> > qcom_ice_optimization_enable(ice);
> >
> > - return qcom_ice_wait_bist_status(ice);
> > + qcom_ice_enable_standard_mode(ice);
> > +
> > + err = qcom_ice_wait_bist_status(ice);
> > + if (err)
> > + return err;
> > +
> > + qcom_ice_hwkm_init(ice);
> > +
> > + return err;
> > }
> > EXPORT_SYMBOL_GPL(qcom_ice_enable);
> >
> > @@ -149,6 +270,8 @@ int qcom_ice_resume(struct qcom_ice *ice)
> > return err;
> > }
> >
> > + qcom_ice_enable_standard_mode(ice);
> > + qcom_ice_hwkm_init(ice);
> > return qcom_ice_wait_bist_status(ice); }
> > EXPORT_SYMBOL_GPL(qcom_ice_resume);
> > @@ -205,6 +328,12 @@ int qcom_ice_evict_key(struct qcom_ice *ice, int
> > slot) } EXPORT_SYMBOL_GPL(qcom_ice_evict_key);
> >
> > +bool qcom_ice_hwkm_supported(struct qcom_ice *ice) {
> > + return ice->use_hwkm;
> > +}
> > +EXPORT_SYMBOL_GPL(qcom_ice_hwkm_supported);
> > +
> > static struct qcom_ice *qcom_ice_create(struct device *dev,
> > void __iomem *base) { @@ -239,6
> > +368,8 @@ static struct qcom_ice *qcom_ice_create(struct device *dev,
> > engine->core_clk = devm_clk_get_enabled(dev, NULL);
> > if (IS_ERR(engine->core_clk))
> > return ERR_CAST(engine->core_clk);
> > + engine->use_hwkm = of_property_read_bool(dev->of_node,
> > + "qcom,ice-use-hwkm");
>
> Under what circumstances would we, with version >= 3.2, not specify this
> flag?
>
> Thanks,
> Bjorn
>
For 3.2.0 versions and above where all the Trustzone support is not present for wrapped keys,
using Qualcomm ICE means using standard (non-wrapped) keys. This cannot work in conjunction
with "HWKM mode" being enabled, and ICE needs to be in "Legacy Mode". HWKM mode is
basically a bunch of register initializations.
Ideally, there should not be any SoC supporting HWKM which does not have all the support, with
a pure hardware version based decision. But unfortunately, we need an explicit switch to
support the above scenario.
> >
> > if (!qcom_ice_check_supported(engine))
> > return ERR_PTR(-EOPNOTSUPP); diff --git
> > a/include/soc/qcom/ice.h b/include/soc/qcom/ice.h index
> > 9dd835dba2a7..1f52e82e3e1c 100644
> > --- a/include/soc/qcom/ice.h
> > +++ b/include/soc/qcom/ice.h
> > @@ -34,5 +34,6 @@ int qcom_ice_program_key(struct qcom_ice *ice,
> > const struct blk_crypto_key *bkey,
> > u8 data_unit_size, int slot); int
> > qcom_ice_evict_key(struct qcom_ice *ice, int slot);
> > +bool qcom_ice_hwkm_supported(struct qcom_ice *ice);
> > struct qcom_ice *of_qcom_ice_get(struct device *dev); #endif /*
> > __QCOM_ICE_H__ */
> > --
> > 2.25.1
> >
> >
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v3 03/12] soc: qcom: ice: add hwkm support in ice
2023-11-22 5:38 ` [PATCH v3 03/12] soc: qcom: ice: add hwkm support in ice Gaurav Kashyap
2023-12-08 4:11 ` Bjorn Andersson
@ 2023-12-08 6:04 ` Om Prakash Singh
2023-12-12 3:58 ` Gaurav Kashyap
2023-12-08 6:06 ` Om Prakash Singh
2023-12-08 6:11 ` Om Prakash Singh
3 siblings, 1 reply; 39+ messages in thread
From: Om Prakash Singh @ 2023-12-08 6:04 UTC (permalink / raw)
To: Gaurav Kashyap, linux-scsi, linux-arm-msm, ebiggers,
neil.armstrong, srinivas.kandagatla
Cc: linux-mmc, linux-block, linux-fscrypt, omprsing, quic_psodagud,
abel.vesa, quic_spuppala, kernel
On 11/22/2023 11:08 AM, Gaurav Kashyap wrote:
> Qualcomm's ICE (Inline Crypto Engine) contains a proprietary
> key management hardware called Hardware Key Manager (HWKM).
> This patch integrates HWKM support in ICE when it is
> available. HWKM primarily provides hardware wrapped key support
> where the ICE (storage) keys are not available in software and
> protected in hardware.
>
> Signed-off-by: Gaurav Kashyap <quic_gaurkash@quicinc.com>
> ---
> drivers/soc/qcom/ice.c | 133 ++++++++++++++++++++++++++++++++++++++++-
> include/soc/qcom/ice.h | 1 +
> 2 files changed, 133 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c
> index 6f941d32fffb..adf9cab848fa 100644
> --- a/drivers/soc/qcom/ice.c
> +++ b/drivers/soc/qcom/ice.c
> @@ -26,6 +26,19 @@
> #define QCOM_ICE_REG_FUSE_SETTING 0x0010
> #define QCOM_ICE_REG_BIST_STATUS 0x0070
> #define QCOM_ICE_REG_ADVANCED_CONTROL 0x1000
> +#define QCOM_ICE_REG_CONTROL 0x0
> +/* QCOM ICE HWKM registers */
> +#define QCOM_ICE_REG_HWKM_TZ_KM_CTL 0x1000
> +#define QCOM_ICE_REG_HWKM_TZ_KM_STATUS 0x1004
> +#define QCOM_ICE_REG_HWKM_BANK0_BANKN_IRQ_STATUS 0x2008
> +#define QCOM_ICE_REG_HWKM_BANK0_BBAC_0 0x5000
> +#define QCOM_ICE_REG_HWKM_BANK0_BBAC_1 0x5004
> +#define QCOM_ICE_REG_HWKM_BANK0_BBAC_2 0x5008
> +#define QCOM_ICE_REG_HWKM_BANK0_BBAC_3 0x500C
> +#define QCOM_ICE_REG_HWKM_BANK0_BBAC_4 0x5010
> +
> +#define QCOM_ICE_HWKM_BIST_DONE_V1_VAL 0x11
> +#define QCOM_ICE_HWKM_BIST_DONE_V2_VAL 0x287
>
> /* BIST ("built-in self-test") status flags */
> #define QCOM_ICE_BIST_STATUS_MASK GENMASK(31, 28)
> @@ -34,6 +47,9 @@
> #define QCOM_ICE_FORCE_HW_KEY0_SETTING_MASK 0x2
> #define QCOM_ICE_FORCE_HW_KEY1_SETTING_MASK 0x4
>
> +#define QCOM_ICE_HWKM_REG_OFFSET 0x8000
> +#define HWKM_OFFSET(reg) ((reg) + QCOM_ICE_HWKM_REG_OFFSET)
> +
> #define qcom_ice_writel(engine, val, reg) \
> writel((val), (engine)->base + (reg))
>
> @@ -46,6 +62,9 @@ struct qcom_ice {
> struct device_link *link;
>
> struct clk *core_clk;
> + u8 hwkm_version;
> + bool use_hwkm;
we can rely on hwkm_version alone to determine if hwkm support is
available or not.
if hwkm_version = 0 (default) consider hwkm is not enabled
> + bool hwkm_init_complete;
> };
>
> static bool qcom_ice_check_supported(struct qcom_ice *ice)
> @@ -63,8 +82,26 @@ static bool qcom_ice_check_supported(struct qcom_ice *ice)
> return false;
> }
>
> + if (major >= 4 || (major == 3 && minor == 2 && step >= 1))
> + ice->hwkm_version = 2;
> + else if (major == 3 && minor == 2)
> + ice->hwkm_version = 1;
> + else
> + ice->hwkm_version = 0;
> +
> + if (ice->hwkm_version == 0)
> + ice->use_hwkm = false;
> +
hwkm version should pass from device-tree property instead of current
complex logic. This will be helpful in support future hwkm version also.
ice->hwkm_version == 0, condition can be use for determining if hwkm is
enabled or not.
> dev_info(dev, "Found QC Inline Crypto Engine (ICE) v%d.%d.%d\n",
> major, minor, step);
> + if (!ice->hwkm_version)
> + dev_info(dev, "QC ICE HWKM (Hardware Key Manager) not supported");
> + else
> + dev_info(dev, "QC ICE HWKM (Hardware Key Manager) version = %d",
> + ice->hwkm_version);
> +
> + if (!ice->use_hwkm)
> + dev_info(dev, "QC ICE HWKM (Hardware Key Manager) not used");
>
> /* If fuses are blown, ICE might not work in the standard way. */
> regval = qcom_ice_readl(ice, QCOM_ICE_REG_FUSE_SETTING);
> @@ -113,10 +150,14 @@ static void qcom_ice_optimization_enable(struct qcom_ice *ice)
> * fails, so we needn't do it in software too, and (c) properly testing
> * storage encryption requires testing the full storage stack anyway,
> * and not relying on hardware-level self-tests.
> + *
> + * However, we still care about if HWKM BIST failed (when supported) as
> + * important functionality would fail later, so disable hwkm on failure.
> */
> static int qcom_ice_wait_bist_status(struct qcom_ice *ice)
> {
> u32 regval;
> + u32 bist_done_val;
> int err;
>
> err = readl_poll_timeout(ice->base + QCOM_ICE_REG_BIST_STATUS,
> @@ -125,15 +166,95 @@ static int qcom_ice_wait_bist_status(struct qcom_ice *ice)
> if (err)
> dev_err(ice->dev, "Timed out waiting for ICE self-test to complete\n");
>
> + if (ice->use_hwkm) {
> + bist_done_val = (ice->hwkm_version == 1) ?
> + QCOM_ICE_HWKM_BIST_DONE_V1_VAL :
> + QCOM_ICE_HWKM_BIST_DONE_V2_VAL;
> + if (qcom_ice_readl(ice,
> + HWKM_OFFSET(QCOM_ICE_REG_HWKM_TZ_KM_STATUS)) !=
> + bist_done_val) {
> + dev_warn(ice->dev, "HWKM BIST error\n");
> + ice->use_hwkm = false;
error is not passed to caller. if HWKM is enabled, and BIST failed, ICE
init also should be considered has failure.
Any reason considering it as warning only ?
> + }
> + }
> return err;
> }
>
> +static void qcom_ice_enable_standard_mode(struct qcom_ice *ice)
> +{
> + u32 val = 0;
> +
> + if (!ice->use_hwkm)
> + return;
> +
> + /*
> + * When ICE is in standard (hwkm) mode, it supports HW wrapped
> + * keys, and when it is in legacy mode, it only supports standard
> + * (non HW wrapped) keys.
> + *
> + * Put ICE in standard mode, ICE defaults to legacy mode.
> + * Legacy mode - ICE HWKM slave not supported.
> + * Standard mode - ICE HWKM slave supported.
> + *
> + * Depending on the version of HWKM, it is controlled by different
> + * registers in ICE.
> + */
> + if (ice->hwkm_version >= 2) {
hwkm_version version >= 2 is not exist. This programming instruction may
create problem in future.
> + val = qcom_ice_readl(ice, QCOM_ICE_REG_CONTROL);
> + val = val & 0xFFFFFFFE;
> + qcom_ice_writel(ice, val, QCOM_ICE_REG_CONTROL);
> + } else {
> + qcom_ice_writel(ice, 0x7,
void using constant value like "0x7" in code use #define with meaningful
name that can indicate what operation is being performed.
This comment is applicable for many places.
> + HWKM_OFFSET(QCOM_ICE_REG_HWKM_TZ_KM_CTL));
> + }
> +}
> +
> +static void qcom_ice_hwkm_init(struct qcom_ice *ice)
> +{
> + if (!ice->use_hwkm)
> + return;
> +
> + /* Disable CRC checks. This HWKM feature is not used. */
> + qcom_ice_writel(ice, 0x6,
> + HWKM_OFFSET(QCOM_ICE_REG_HWKM_TZ_KM_CTL));
> +
> + /*
> + * Give register bank of the HWKM slave access to read and modify
> + * the keyslots in ICE HWKM slave. Without this, trustzone will not
> + * be able to program keys into ICE.
> + */
> + qcom_ice_writel(ice, 0xFFFFFFFF,
> + HWKM_OFFSET(QCOM_ICE_REG_HWKM_BANK0_BBAC_0));
> + qcom_ice_writel(ice, 0xFFFFFFFF,
> + HWKM_OFFSET(QCOM_ICE_REG_HWKM_BANK0_BBAC_1));
> + qcom_ice_writel(ice, 0xFFFFFFFF,
> + HWKM_OFFSET(QCOM_ICE_REG_HWKM_BANK0_BBAC_2));
> + qcom_ice_writel(ice, 0xFFFFFFFF,
> + HWKM_OFFSET(QCOM_ICE_REG_HWKM_BANK0_BBAC_3));
> + qcom_ice_writel(ice, 0xFFFFFFFF,
> + HWKM_OFFSET(QCOM_ICE_REG_HWKM_BANK0_BBAC_4));
> +
> + /* Clear HWKM response FIFO before doing anything */
> + qcom_ice_writel(ice, 0x8,
> + HWKM_OFFSET(QCOM_ICE_REG_HWKM_BANK0_BANKN_IRQ_STATUS));
> +}
> +
> int qcom_ice_enable(struct qcom_ice *ice)
> {
> + int err;
> +
> qcom_ice_low_power_mode_enable(ice);
> qcom_ice_optimization_enable(ice);
>
> - return qcom_ice_wait_bist_status(ice);
> + qcom_ice_enable_standard_mode(ice);
> +
> + err = qcom_ice_wait_bist_status(ice);
> + if (err)
> + return err;
> +
> + qcom_ice_hwkm_init(ice);
new code added in this section is only application when hwkm is enabled.
if (!ice->hwkm_version) {
qcom_ice_enable_standard_mode(ice);
qcom_ice_hwkm_init(ice);
}
> +
> + return err;
> }
> EXPORT_SYMBOL_GPL(qcom_ice_enable);
>
> @@ -149,6 +270,8 @@ int qcom_ice_resume(struct qcom_ice *ice)
> return err;
> }
>
> + qcom_ice_enable_standard_mode(ice);
> + qcom_ice_hwkm_init(ice);
> return qcom_ice_wait_bist_status(ice);
> }
> EXPORT_SYMBOL_GPL(qcom_ice_resume);
> @@ -205,6 +328,12 @@ int qcom_ice_evict_key(struct qcom_ice *ice, int slot)
> }
> EXPORT_SYMBOL_GPL(qcom_ice_evict_key);
>
> +bool qcom_ice_hwkm_supported(struct qcom_ice *ice)
> +{
> + return ice->use_hwkm;
> +}
> +EXPORT_SYMBOL_GPL(qcom_ice_hwkm_supported);
> +
> static struct qcom_ice *qcom_ice_create(struct device *dev,
> void __iomem *base)
> {
> @@ -239,6 +368,8 @@ static struct qcom_ice *qcom_ice_create(struct device *dev,
> engine->core_clk = devm_clk_get_enabled(dev, NULL);
> if (IS_ERR(engine->core_clk))
> return ERR_CAST(engine->core_clk);
> + engine->use_hwkm = of_property_read_bool(dev->of_node,
> + "qcom,ice-use-hwkm");
>
> if (!qcom_ice_check_supported(engine))
> return ERR_PTR(-EOPNOTSUPP);
> diff --git a/include/soc/qcom/ice.h b/include/soc/qcom/ice.h
> index 9dd835dba2a7..1f52e82e3e1c 100644
> --- a/include/soc/qcom/ice.h
> +++ b/include/soc/qcom/ice.h
> @@ -34,5 +34,6 @@ int qcom_ice_program_key(struct qcom_ice *ice,
> const struct blk_crypto_key *bkey,
> u8 data_unit_size, int slot);
> int qcom_ice_evict_key(struct qcom_ice *ice, int slot);
> +bool qcom_ice_hwkm_supported(struct qcom_ice *ice);
> struct qcom_ice *of_qcom_ice_get(struct device *dev);
> #endif /* __QCOM_ICE_H__ */
^ permalink raw reply [flat|nested] 39+ messages in thread* RE: [PATCH v3 03/12] soc: qcom: ice: add hwkm support in ice
2023-12-08 6:04 ` Om Prakash Singh
@ 2023-12-12 3:58 ` Gaurav Kashyap
0 siblings, 0 replies; 39+ messages in thread
From: Gaurav Kashyap @ 2023-12-12 3:58 UTC (permalink / raw)
To: Om Prakash Singh (QUIC), Gaurav Kashyap (QUIC),
linux-scsi, linux-arm-msm, ebiggers, neil.armstrong,
srinivas.kandagatla
Cc: linux-mmc, linux-block, linux-fscrypt, Om Prakash Singh,
Prasad Sodagudi (QUIC), abel.vesa, Seshu Madhavi Puppala (QUIC),
kernel
Hello Om
On 12/07/2023, Om Prakash Singh wrote:
> On 11/22/2023 11:08 AM, Gaurav Kashyap wrote:
> > Qualcomm's ICE (Inline Crypto Engine) contains a proprietary key
> > management hardware called Hardware Key Manager (HWKM).
> > This patch integrates HWKM support in ICE when it is available. HWKM
> > primarily provides hardware wrapped key support where the ICE
> > (storage) keys are not available in software and protected in
> > hardware.
> >
> > Signed-off-by: Gaurav Kashyap <quic_gaurkash@quicinc.com>
> > ---
> > drivers/soc/qcom/ice.c | 133
> ++++++++++++++++++++++++++++++++++++++++-
> > include/soc/qcom/ice.h | 1 +
> > 2 files changed, 133 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c index
> > 6f941d32fffb..adf9cab848fa 100644
> > --- a/drivers/soc/qcom/ice.c
> > +++ b/drivers/soc/qcom/ice.c
> > @@ -26,6 +26,19 @@
> > #define QCOM_ICE_REG_FUSE_SETTING 0x0010
> > #define QCOM_ICE_REG_BIST_STATUS 0x0070
> > #define QCOM_ICE_REG_ADVANCED_CONTROL 0x1000
> > +#define QCOM_ICE_REG_CONTROL 0x0
> > +/* QCOM ICE HWKM registers */
> > +#define QCOM_ICE_REG_HWKM_TZ_KM_CTL
> 0x1000
> > +#define QCOM_ICE_REG_HWKM_TZ_KM_STATUS
> 0x1004
> > +#define QCOM_ICE_REG_HWKM_BANK0_BANKN_IRQ_STATUS
> 0x2008
> > +#define QCOM_ICE_REG_HWKM_BANK0_BBAC_0
> 0x5000
> > +#define QCOM_ICE_REG_HWKM_BANK0_BBAC_1
> 0x5004
> > +#define QCOM_ICE_REG_HWKM_BANK0_BBAC_2
> 0x5008
> > +#define QCOM_ICE_REG_HWKM_BANK0_BBAC_3
> 0x500C
> > +#define QCOM_ICE_REG_HWKM_BANK0_BBAC_4
> 0x5010
> > +
> > +#define QCOM_ICE_HWKM_BIST_DONE_V1_VAL 0x11
> > +#define QCOM_ICE_HWKM_BIST_DONE_V2_VAL 0x287
> >
> > /* BIST ("built-in self-test") status flags */
> > #define QCOM_ICE_BIST_STATUS_MASK GENMASK(31, 28)
> > @@ -34,6 +47,9 @@
> > #define QCOM_ICE_FORCE_HW_KEY0_SETTING_MASK 0x2
> > #define QCOM_ICE_FORCE_HW_KEY1_SETTING_MASK 0x4
> >
> > +#define QCOM_ICE_HWKM_REG_OFFSET 0x8000
> > +#define HWKM_OFFSET(reg) ((reg) +
> QCOM_ICE_HWKM_REG_OFFSET)
> > +
> > #define qcom_ice_writel(engine, val, reg) \
> > writel((val), (engine)->base + (reg))
> >
> > @@ -46,6 +62,9 @@ struct qcom_ice {
> > struct device_link *link;
> >
> > struct clk *core_clk;
> > + u8 hwkm_version;
> > + bool use_hwkm;
>
> we can rely on hwkm_version alone to determine if hwkm support is
> available or not.
> if hwkm_version = 0 (default) consider hwkm is not enabled
>
The reason this is being added is to support standard keys on chipsets which contain a HWKM version, but
does not have the capability in Trustzone (for example) to completely use wrapped keys.
Using a HWKM DTSI entry will let you explicitly enable/disable wrapped key (or hwkm) support.
In general too, I think it is good to allow support for both standard and wrapped keys.
> > + bool hwkm_init_complete;
> > };
> >
> > static bool qcom_ice_check_supported(struct qcom_ice *ice) @@ -63,8
> > +82,26 @@ static bool qcom_ice_check_supported(struct qcom_ice *ice)
> > return false;
> > }
> >
> > + if (major >= 4 || (major == 3 && minor == 2 && step >= 1))
> > + ice->hwkm_version = 2;
> > + else if (major == 3 && minor == 2)
> > + ice->hwkm_version = 1;
> > + else
> > + ice->hwkm_version = 0;
> > +
> > + if (ice->hwkm_version == 0)
> > + ice->use_hwkm = false;
> > +
> hwkm version should pass from device-tree property instead of current
> complex logic. This will be helpful in support future hwkm version also.
> ice->hwkm_version == 0, condition can be use for determining if hwkm is
> enabled or not.
>
Shouldn't the hardware version be read from the hardware?
> > dev_info(dev, "Found QC Inline Crypto Engine (ICE) v%d.%d.%d\n",
> > major, minor, step);
> > + if (!ice->hwkm_version)
> > + dev_info(dev, "QC ICE HWKM (Hardware Key Manager) not
> supported");
> > + else
> > + dev_info(dev, "QC ICE HWKM (Hardware Key Manager)
> version = %d",
> > + ice->hwkm_version);
> > +
> > + if (!ice->use_hwkm)
> > + dev_info(dev, "QC ICE HWKM (Hardware Key Manager) not
> used");
> >
> > /* If fuses are blown, ICE might not work in the standard way. */
> > regval = qcom_ice_readl(ice, QCOM_ICE_REG_FUSE_SETTING);
> > @@ -113,10 +150,14 @@ static void qcom_ice_optimization_enable(struct
> qcom_ice *ice)
> > * fails, so we needn't do it in software too, and (c) properly testing
> > * storage encryption requires testing the full storage stack anyway,
> > * and not relying on hardware-level self-tests.
> > + *
> > + * However, we still care about if HWKM BIST failed (when supported) as
> > + * important functionality would fail later, so disable hwkm on failure.
> > */
> > static int qcom_ice_wait_bist_status(struct qcom_ice *ice)
> > {
> > u32 regval;
> > + u32 bist_done_val;
> > int err;
> >
> > err = readl_poll_timeout(ice->base + QCOM_ICE_REG_BIST_STATUS,
> > @@ -125,15 +166,95 @@ static int qcom_ice_wait_bist_status(struct
> qcom_ice *ice)
> > if (err)
> > dev_err(ice->dev, "Timed out waiting for ICE self-test to
> complete\n");
> >
> > + if (ice->use_hwkm) {
> > + bist_done_val = (ice->hwkm_version == 1) ?
> > + QCOM_ICE_HWKM_BIST_DONE_V1_VAL :
> > + QCOM_ICE_HWKM_BIST_DONE_V2_VAL;
> > + if (qcom_ice_readl(ice,
> > +
> HWKM_OFFSET(QCOM_ICE_REG_HWKM_TZ_KM_STATUS)) !=
> > + bist_done_val) {
> > + dev_warn(ice->dev, "HWKM BIST error\n");
> > + ice->use_hwkm = false;
> error is not passed to caller. if HWKM is enabled, and BIST failed, ICE
> init also should be considered has failure.
>
> Any reason considering it as warning only ?
Noted for the next patch.
> > + }
> > + }
> > return err;
> > }
> >
> > +static void qcom_ice_enable_standard_mode(struct qcom_ice *ice)
> > +{
> > + u32 val = 0;
> > +
> > + if (!ice->use_hwkm)
> > + return;
> > +
> > + /*
> > + * When ICE is in standard (hwkm) mode, it supports HW wrapped
> > + * keys, and when it is in legacy mode, it only supports standard
> > + * (non HW wrapped) keys.
> > + *
> > + * Put ICE in standard mode, ICE defaults to legacy mode.
> > + * Legacy mode - ICE HWKM slave not supported.
> > + * Standard mode - ICE HWKM slave supported.
> > + *
> > + * Depending on the version of HWKM, it is controlled by different
> > + * registers in ICE.
> > + */
> > + if (ice->hwkm_version >= 2) {
>
> hwkm_version version >= 2 is not exist. This programming instruction may
> create problem in future.
>
We currently use HWKM version 2 on SM8650.
If you are specifying only versions greater than 2,
At best, we won't need any changes to support let's say a version 3
At worst, changes need to be made to support version specific changes,
which we would have had to make anyway.
> > + val = qcom_ice_readl(ice, QCOM_ICE_REG_CONTROL);
> > + val = val & 0xFFFFFFFE;
> > + qcom_ice_writel(ice, val, QCOM_ICE_REG_CONTROL);
> > + } else {
> > + qcom_ice_writel(ice, 0x7,
> void using constant value like "0x7" in code use #define with meaningful
> name that can indicate what operation is being performed.
> This comment is applicable for many places.
I agree, noted for the next patch.
> > +
> HWKM_OFFSET(QCOM_ICE_REG_HWKM_TZ_KM_CTL));
> > + }
> > +}
> > +
> > +static void qcom_ice_hwkm_init(struct qcom_ice *ice)
> > +{
> > + if (!ice->use_hwkm)
> > + return;
> > +
> > + /* Disable CRC checks. This HWKM feature is not used. */
> > + qcom_ice_writel(ice, 0x6,
> > +
> HWKM_OFFSET(QCOM_ICE_REG_HWKM_TZ_KM_CTL));
> > +
> > + /*
> > + * Give register bank of the HWKM slave access to read and modify
> > + * the keyslots in ICE HWKM slave. Without this, trustzone will not
> > + * be able to program keys into ICE.
> > + */
> > + qcom_ice_writel(ice, 0xFFFFFFFF,
> > +
> HWKM_OFFSET(QCOM_ICE_REG_HWKM_BANK0_BBAC_0));
> > + qcom_ice_writel(ice, 0xFFFFFFFF,
> > +
> HWKM_OFFSET(QCOM_ICE_REG_HWKM_BANK0_BBAC_1));
> > + qcom_ice_writel(ice, 0xFFFFFFFF,
> > +
> HWKM_OFFSET(QCOM_ICE_REG_HWKM_BANK0_BBAC_2));
> > + qcom_ice_writel(ice, 0xFFFFFFFF,
> > +
> HWKM_OFFSET(QCOM_ICE_REG_HWKM_BANK0_BBAC_3));
> > + qcom_ice_writel(ice, 0xFFFFFFFF,
> > +
> HWKM_OFFSET(QCOM_ICE_REG_HWKM_BANK0_BBAC_4));
> > +
> > + /* Clear HWKM response FIFO before doing anything */
> > + qcom_ice_writel(ice, 0x8,
> > +
> HWKM_OFFSET(QCOM_ICE_REG_HWKM_BANK0_BANKN_IRQ_STA
> TUS));
> > +}
> > +
> > int qcom_ice_enable(struct qcom_ice *ice)
> > {
> > + int err;
> > +
> > qcom_ice_low_power_mode_enable(ice);
> > qcom_ice_optimization_enable(ice);
> >
> > - return qcom_ice_wait_bist_status(ice);
> > + qcom_ice_enable_standard_mode(ice);
> > +
> > + err = qcom_ice_wait_bist_status(ice);
> > + if (err)
> > + return err;
> > +
> > + qcom_ice_hwkm_init(ice);
> new code added in this section is only application when hwkm is enabled.
> if (!ice->hwkm_version) {
> qcom_ice_enable_standard_mode(ice);
> qcom_ice_hwkm_init(ice);
> }
It is already gated within the new APIs
> > +
> > + return err;
> > }
> > EXPORT_SYMBOL_GPL(qcom_ice_enable);
> >
> > @@ -149,6 +270,8 @@ int qcom_ice_resume(struct qcom_ice *ice)
> > return err;
> > }
> >
> > + qcom_ice_enable_standard_mode(ice);
> > + qcom_ice_hwkm_init(ice);
> > return qcom_ice_wait_bist_status(ice);
> > }
> > EXPORT_SYMBOL_GPL(qcom_ice_resume);
> > @@ -205,6 +328,12 @@ int qcom_ice_evict_key(struct qcom_ice *ice, int
> slot)
> > }
> > EXPORT_SYMBOL_GPL(qcom_ice_evict_key);
> >
> > +bool qcom_ice_hwkm_supported(struct qcom_ice *ice)
> > +{
> > + return ice->use_hwkm;
> > +}
> > +EXPORT_SYMBOL_GPL(qcom_ice_hwkm_supported);
> > +
> > static struct qcom_ice *qcom_ice_create(struct device *dev,
> > void __iomem *base)
> > {
> > @@ -239,6 +368,8 @@ static struct qcom_ice *qcom_ice_create(struct
> device *dev,
> > engine->core_clk = devm_clk_get_enabled(dev, NULL);
> > if (IS_ERR(engine->core_clk))
> > return ERR_CAST(engine->core_clk);
> > + engine->use_hwkm = of_property_read_bool(dev->of_node,
> > + "qcom,ice-use-hwkm");
> >
> > if (!qcom_ice_check_supported(engine))
> > return ERR_PTR(-EOPNOTSUPP);
> > diff --git a/include/soc/qcom/ice.h b/include/soc/qcom/ice.h
> > index 9dd835dba2a7..1f52e82e3e1c 100644
> > --- a/include/soc/qcom/ice.h
> > +++ b/include/soc/qcom/ice.h
> > @@ -34,5 +34,6 @@ int qcom_ice_program_key(struct qcom_ice *ice,
> > const struct blk_crypto_key *bkey,
> > u8 data_unit_size, int slot);
> > int qcom_ice_evict_key(struct qcom_ice *ice, int slot);
> > +bool qcom_ice_hwkm_supported(struct qcom_ice *ice);
> > struct qcom_ice *of_qcom_ice_get(struct device *dev);
> > #endif /* __QCOM_ICE_H__ */
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v3 03/12] soc: qcom: ice: add hwkm support in ice
2023-11-22 5:38 ` [PATCH v3 03/12] soc: qcom: ice: add hwkm support in ice Gaurav Kashyap
2023-12-08 4:11 ` Bjorn Andersson
2023-12-08 6:04 ` Om Prakash Singh
@ 2023-12-08 6:06 ` Om Prakash Singh
2023-12-08 6:11 ` Om Prakash Singh
3 siblings, 0 replies; 39+ messages in thread
From: Om Prakash Singh @ 2023-12-08 6:06 UTC (permalink / raw)
To: Gaurav Kashyap, linux-scsi, linux-arm-msm, ebiggers,
neil.armstrong, srinivas.kandagatla
Cc: linux-mmc, linux-block, linux-fscrypt, omprsing, quic_psodagud,
abel.vesa, quic_spuppala, kernel
On 11/22/2023 11:08 AM, Gaurav Kashyap wrote:
> Qualcomm's ICE (Inline Crypto Engine) contains a proprietary
> key management hardware called Hardware Key Manager (HWKM).
> This patch integrates HWKM support in ICE when it is
> available. HWKM primarily provides hardware wrapped key support
> where the ICE (storage) keys are not available in software and
> protected in hardware.
>
> Signed-off-by: Gaurav Kashyap <quic_gaurkash@quicinc.com>
> ---
> drivers/soc/qcom/ice.c | 133 ++++++++++++++++++++++++++++++++++++++++-
> include/soc/qcom/ice.h | 1 +
> 2 files changed, 133 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c
> index 6f941d32fffb..adf9cab848fa 100644
> --- a/drivers/soc/qcom/ice.c
> +++ b/drivers/soc/qcom/ice.c
> @@ -26,6 +26,19 @@
> #define QCOM_ICE_REG_FUSE_SETTING 0x0010
> #define QCOM_ICE_REG_BIST_STATUS 0x0070
> #define QCOM_ICE_REG_ADVANCED_CONTROL 0x1000
> +#define QCOM_ICE_REG_CONTROL 0x0
> +/* QCOM ICE HWKM registers */
> +#define QCOM_ICE_REG_HWKM_TZ_KM_CTL 0x1000
> +#define QCOM_ICE_REG_HWKM_TZ_KM_STATUS 0x1004
> +#define QCOM_ICE_REG_HWKM_BANK0_BANKN_IRQ_STATUS 0x2008
> +#define QCOM_ICE_REG_HWKM_BANK0_BBAC_0 0x5000
> +#define QCOM_ICE_REG_HWKM_BANK0_BBAC_1 0x5004
> +#define QCOM_ICE_REG_HWKM_BANK0_BBAC_2 0x5008
> +#define QCOM_ICE_REG_HWKM_BANK0_BBAC_3 0x500C
> +#define QCOM_ICE_REG_HWKM_BANK0_BBAC_4 0x5010
> +
> +#define QCOM_ICE_HWKM_BIST_DONE_V1_VAL 0x11
> +#define QCOM_ICE_HWKM_BIST_DONE_V2_VAL 0x287
>
> /* BIST ("built-in self-test") status flags */
> #define QCOM_ICE_BIST_STATUS_MASK GENMASK(31, 28)
> @@ -34,6 +47,9 @@
> #define QCOM_ICE_FORCE_HW_KEY0_SETTING_MASK 0x2
> #define QCOM_ICE_FORCE_HW_KEY1_SETTING_MASK 0x4
>
> +#define QCOM_ICE_HWKM_REG_OFFSET 0x8000
> +#define HWKM_OFFSET(reg) ((reg) + QCOM_ICE_HWKM_REG_OFFSET)
> +
> #define qcom_ice_writel(engine, val, reg) \
> writel((val), (engine)->base + (reg))
>
> @@ -46,6 +62,9 @@ struct qcom_ice {
> struct device_link *link;
>
> struct clk *core_clk;
> + u8 hwkm_version;
> + bool use_hwkm;
we can rely on hwkm_version alone to determine if hwkm support is
available or not.
if hwkm_version = 0 (default) consider hwkm is not enabled
> + bool hwkm_init_complete;
> };
>
> static bool qcom_ice_check_supported(struct qcom_ice *ice)
> @@ -63,8 +82,26 @@ static bool qcom_ice_check_supported(struct qcom_ice *ice)
> return false;
> }
>
> + if (major >= 4 || (major == 3 && minor == 2 && step >= 1))
> + ice->hwkm_version = 2;
> + else if (major == 3 && minor == 2)
> + ice->hwkm_version = 1;
> + else
> + ice->hwkm_version = 0;
> +
> + if (ice->hwkm_version == 0)
> + ice->use_hwkm = false;
> +
hwkm version should pass from device-tree property instead of current
complex logic. This will be helpful in support future hwkm version also.
ice->hwkm_version == 0, condition can be use for determining if hwkm is
enabled or not.
> dev_info(dev, "Found QC Inline Crypto Engine (ICE) v%d.%d.%d\n",
> major, minor, step);
> + if (!ice->hwkm_version)
> + dev_info(dev, "QC ICE HWKM (Hardware Key Manager) not supported");
> + else
> + dev_info(dev, "QC ICE HWKM (Hardware Key Manager) version = %d",
> + ice->hwkm_version);
> +
> + if (!ice->use_hwkm)
> + dev_info(dev, "QC ICE HWKM (Hardware Key Manager) not used");
>
> /* If fuses are blown, ICE might not work in the standard way. */
> regval = qcom_ice_readl(ice, QCOM_ICE_REG_FUSE_SETTING);
> @@ -113,10 +150,14 @@ static void qcom_ice_optimization_enable(struct qcom_ice *ice)
> * fails, so we needn't do it in software too, and (c) properly testing
> * storage encryption requires testing the full storage stack anyway,
> * and not relying on hardware-level self-tests.
> + *
> + * However, we still care about if HWKM BIST failed (when supported) as
> + * important functionality would fail later, so disable hwkm on failure.
> */
> static int qcom_ice_wait_bist_status(struct qcom_ice *ice)
> {
> u32 regval;
> + u32 bist_done_val;
> int err;
>
> err = readl_poll_timeout(ice->base + QCOM_ICE_REG_BIST_STATUS,
> @@ -125,15 +166,95 @@ static int qcom_ice_wait_bist_status(struct qcom_ice *ice)
> if (err)
> dev_err(ice->dev, "Timed out waiting for ICE self-test to complete\n");
>
> + if (ice->use_hwkm) {
> + bist_done_val = (ice->hwkm_version == 1) ?
> + QCOM_ICE_HWKM_BIST_DONE_V1_VAL :
> + QCOM_ICE_HWKM_BIST_DONE_V2_VAL;
> + if (qcom_ice_readl(ice,
> + HWKM_OFFSET(QCOM_ICE_REG_HWKM_TZ_KM_STATUS)) !=
> + bist_done_val) {
> + dev_warn(ice->dev, "HWKM BIST error\n");
> + ice->use_hwkm = false;
error is not passed to caller. if HWKM is enabled, and BIST failed, ICE
init also should be considered has failure.
Any reason considering it as warning only ?
> + }
> + }
> return err;
> }
>
> +static void qcom_ice_enable_standard_mode(struct qcom_ice *ice)
> +{
> + u32 val = 0;
> +
> + if (!ice->use_hwkm)
> + return;
> +
> + /*
> + * When ICE is in standard (hwkm) mode, it supports HW wrapped
> + * keys, and when it is in legacy mode, it only supports standard
> + * (non HW wrapped) keys.
> + *
> + * Put ICE in standard mode, ICE defaults to legacy mode.
> + * Legacy mode - ICE HWKM slave not supported.
> + * Standard mode - ICE HWKM slave supported.
> + *
> + * Depending on the version of HWKM, it is controlled by different
> + * registers in ICE.
> + */
> + if (ice->hwkm_version >= 2) {
hwkm_version version >= 2 is not exist. This programming instruction may
create problem in future.
> + val = qcom_ice_readl(ice, QCOM_ICE_REG_CONTROL);
> + val = val & 0xFFFFFFFE;
> + qcom_ice_writel(ice, val, QCOM_ICE_REG_CONTROL);
> + } else {
> + qcom_ice_writel(ice, 0x7,
void using constant value like "0x7" in code use #define with meaningful
name that can indicate what operation is being performed.
This comment is applicable for many places.
> + HWKM_OFFSET(QCOM_ICE_REG_HWKM_TZ_KM_CTL));
> + }
> +}
> +
> +static void qcom_ice_hwkm_init(struct qcom_ice *ice)
> +{
> + if (!ice->use_hwkm)
> + return;
> +
> + /* Disable CRC checks. This HWKM feature is not used. */
> + qcom_ice_writel(ice, 0x6,
> + HWKM_OFFSET(QCOM_ICE_REG_HWKM_TZ_KM_CTL));
> +
> + /*
> + * Give register bank of the HWKM slave access to read and modify
> + * the keyslots in ICE HWKM slave. Without this, trustzone will not
> + * be able to program keys into ICE.
> + */
> + qcom_ice_writel(ice, 0xFFFFFFFF,
> + HWKM_OFFSET(QCOM_ICE_REG_HWKM_BANK0_BBAC_0));
> + qcom_ice_writel(ice, 0xFFFFFFFF,
> + HWKM_OFFSET(QCOM_ICE_REG_HWKM_BANK0_BBAC_1));
> + qcom_ice_writel(ice, 0xFFFFFFFF,
> + HWKM_OFFSET(QCOM_ICE_REG_HWKM_BANK0_BBAC_2));
> + qcom_ice_writel(ice, 0xFFFFFFFF,
> + HWKM_OFFSET(QCOM_ICE_REG_HWKM_BANK0_BBAC_3));
> + qcom_ice_writel(ice, 0xFFFFFFFF,
> + HWKM_OFFSET(QCOM_ICE_REG_HWKM_BANK0_BBAC_4));
> +
> + /* Clear HWKM response FIFO before doing anything */
> + qcom_ice_writel(ice, 0x8,
> + HWKM_OFFSET(QCOM_ICE_REG_HWKM_BANK0_BANKN_IRQ_STATUS));
> +}
> +
> int qcom_ice_enable(struct qcom_ice *ice)
> {
> + int err;
> +
> qcom_ice_low_power_mode_enable(ice);
> qcom_ice_optimization_enable(ice);
>
> - return qcom_ice_wait_bist_status(ice);
> + qcom_ice_enable_standard_mode(ice);
> +
> + err = qcom_ice_wait_bist_status(ice);
> + if (err)
> + return err;
> +
> + qcom_ice_hwkm_init(ice);
new code added in this section is only application when hwkm is enabled.
if (!ice->hwkm_version) {
qcom_ice_enable_standard_mode(ice);
qcom_ice_hwkm_init(ice);
}
> +
> + return err;
> }
> EXPORT_SYMBOL_GPL(qcom_ice_enable);
>
> @@ -149,6 +270,8 @@ int qcom_ice_resume(struct qcom_ice *ice)
> return err;
> }
>
> + qcom_ice_enable_standard_mode(ice);
> + qcom_ice_hwkm_init(ice);
> return qcom_ice_wait_bist_status(ice);
> }
> EXPORT_SYMBOL_GPL(qcom_ice_resume);
> @@ -205,6 +328,12 @@ int qcom_ice_evict_key(struct qcom_ice *ice, int slot)
> }
> EXPORT_SYMBOL_GPL(qcom_ice_evict_key);
>
> +bool qcom_ice_hwkm_supported(struct qcom_ice *ice)
> +{
> + return ice->use_hwkm;
> +}
> +EXPORT_SYMBOL_GPL(qcom_ice_hwkm_supported);
> +
> static struct qcom_ice *qcom_ice_create(struct device *dev,
> void __iomem *base)
> {
> @@ -239,6 +368,8 @@ static struct qcom_ice *qcom_ice_create(struct device *dev,
> engine->core_clk = devm_clk_get_enabled(dev, NULL);
> if (IS_ERR(engine->core_clk))
> return ERR_CAST(engine->core_clk);
> + engine->use_hwkm = of_property_read_bool(dev->of_node,
> + "qcom,ice-use-hwkm");
>
> if (!qcom_ice_check_supported(engine))
> return ERR_PTR(-EOPNOTSUPP);
> diff --git a/include/soc/qcom/ice.h b/include/soc/qcom/ice.h
> index 9dd835dba2a7..1f52e82e3e1c 100644
> --- a/include/soc/qcom/ice.h
> +++ b/include/soc/qcom/ice.h
> @@ -34,5 +34,6 @@ int qcom_ice_program_key(struct qcom_ice *ice,
> const struct blk_crypto_key *bkey,
> u8 data_unit_size, int slot);
> int qcom_ice_evict_key(struct qcom_ice *ice, int slot);
> +bool qcom_ice_hwkm_supported(struct qcom_ice *ice);
> struct qcom_ice *of_qcom_ice_get(struct device *dev);
> #endif /* __QCOM_ICE_H__ */
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [PATCH v3 03/12] soc: qcom: ice: add hwkm support in ice
2023-11-22 5:38 ` [PATCH v3 03/12] soc: qcom: ice: add hwkm support in ice Gaurav Kashyap
` (2 preceding siblings ...)
2023-12-08 6:06 ` Om Prakash Singh
@ 2023-12-08 6:11 ` Om Prakash Singh
3 siblings, 0 replies; 39+ messages in thread
From: Om Prakash Singh @ 2023-12-08 6:11 UTC (permalink / raw)
To: Gaurav Kashyap, linux-scsi, linux-arm-msm, ebiggers,
neil.armstrong, srinivas.kandagatla
Cc: linux-mmc, linux-block, linux-fscrypt, omprsing, quic_psodagud,
abel.vesa, quic_spuppala, kernel
On 11/22/2023 11:08 AM, Gaurav Kashyap wrote:
> Qualcomm's ICE (Inline Crypto Engine) contains a proprietary
> key management hardware called Hardware Key Manager (HWKM).
> This patch integrates HWKM support in ICE when it is
> available. HWKM primarily provides hardware wrapped key support
> where the ICE (storage) keys are not available in software and
> protected in hardware.
>
> Signed-off-by: Gaurav Kashyap <quic_gaurkash@quicinc.com>
> ---
> drivers/soc/qcom/ice.c | 133 ++++++++++++++++++++++++++++++++++++++++-
> include/soc/qcom/ice.h | 1 +
> 2 files changed, 133 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c
> index 6f941d32fffb..adf9cab848fa 100644
> --- a/drivers/soc/qcom/ice.c
> +++ b/drivers/soc/qcom/ice.c
> @@ -26,6 +26,19 @@
> #define QCOM_ICE_REG_FUSE_SETTING 0x0010
> #define QCOM_ICE_REG_BIST_STATUS 0x0070
> #define QCOM_ICE_REG_ADVANCED_CONTROL 0x1000
> +#define QCOM_ICE_REG_CONTROL 0x0
> +/* QCOM ICE HWKM registers */
> +#define QCOM_ICE_REG_HWKM_TZ_KM_CTL 0x1000
> +#define QCOM_ICE_REG_HWKM_TZ_KM_STATUS 0x1004
> +#define QCOM_ICE_REG_HWKM_BANK0_BANKN_IRQ_STATUS 0x2008
> +#define QCOM_ICE_REG_HWKM_BANK0_BBAC_0 0x5000
> +#define QCOM_ICE_REG_HWKM_BANK0_BBAC_1 0x5004
> +#define QCOM_ICE_REG_HWKM_BANK0_BBAC_2 0x5008
> +#define QCOM_ICE_REG_HWKM_BANK0_BBAC_3 0x500C
> +#define QCOM_ICE_REG_HWKM_BANK0_BBAC_4 0x5010
> +
> +#define QCOM_ICE_HWKM_BIST_DONE_V1_VAL 0x11
> +#define QCOM_ICE_HWKM_BIST_DONE_V2_VAL 0x287
>
> /* BIST ("built-in self-test") status flags */
> #define QCOM_ICE_BIST_STATUS_MASK GENMASK(31, 28)
> @@ -34,6 +47,9 @@
> #define QCOM_ICE_FORCE_HW_KEY0_SETTING_MASK 0x2
> #define QCOM_ICE_FORCE_HW_KEY1_SETTING_MASK 0x4
>
> +#define QCOM_ICE_HWKM_REG_OFFSET 0x8000
> +#define HWKM_OFFSET(reg) ((reg) + QCOM_ICE_HWKM_REG_OFFSET)
> +
> #define qcom_ice_writel(engine, val, reg) \
> writel((val), (engine)->base + (reg))
>
> @@ -46,6 +62,9 @@ struct qcom_ice {
> struct device_link *link;
>
> struct clk *core_clk;
> + u8 hwkm_version;
> + bool use_hwkm;
we can rely on hwkm_version alone to determine if hwkm support is
available or not.
if hwkm_version = 0 (default) consider hwkm is not enabled
> + bool hwkm_init_complete;
> };
>
> static bool qcom_ice_check_supported(struct qcom_ice *ice)
> @@ -63,8 +82,26 @@ static bool qcom_ice_check_supported(struct qcom_ice *ice)
> return false;
> }
>
> + if (major >= 4 || (major == 3 && minor == 2 && step >= 1))
> + ice->hwkm_version = 2;
> + else if (major == 3 && minor == 2)
> + ice->hwkm_version = 1;
> + else
> + ice->hwkm_version = 0;
> +
> + if (ice->hwkm_version == 0)
> + ice->use_hwkm = false;
> +
hwkm version should pass from device-tree property instead of current
complex logic. This will be helpful in support future hwkm version also.
ice->hwkm_version == 0, condition can be use for determining if hwkm is
enabled or not.
> dev_info(dev, "Found QC Inline Crypto Engine (ICE) v%d.%d.%d\n",
> major, minor, step);
> + if (!ice->hwkm_version)
> + dev_info(dev, "QC ICE HWKM (Hardware Key Manager) not supported");
> + else
> + dev_info(dev, "QC ICE HWKM (Hardware Key Manager) version = %d",
> + ice->hwkm_version);
> +
> + if (!ice->use_hwkm)
> + dev_info(dev, "QC ICE HWKM (Hardware Key Manager) not used");
>
> /* If fuses are blown, ICE might not work in the standard way. */
> regval = qcom_ice_readl(ice, QCOM_ICE_REG_FUSE_SETTING);
> @@ -113,10 +150,14 @@ static void qcom_ice_optimization_enable(struct qcom_ice *ice)
> * fails, so we needn't do it in software too, and (c) properly testing
> * storage encryption requires testing the full storage stack anyway,
> * and not relying on hardware-level self-tests.
> + *
> + * However, we still care about if HWKM BIST failed (when supported) as
> + * important functionality would fail later, so disable hwkm on failure.
> */
> static int qcom_ice_wait_bist_status(struct qcom_ice *ice)
> {
> u32 regval;
> + u32 bist_done_val;
> int err;
>
> err = readl_poll_timeout(ice->base + QCOM_ICE_REG_BIST_STATUS,
> @@ -125,15 +166,95 @@ static int qcom_ice_wait_bist_status(struct qcom_ice *ice)
> if (err)
> dev_err(ice->dev, "Timed out waiting for ICE self-test to complete\n");
>
> + if (ice->use_hwkm) {
> + bist_done_val = (ice->hwkm_version == 1) ?
> + QCOM_ICE_HWKM_BIST_DONE_V1_VAL :
> + QCOM_ICE_HWKM_BIST_DONE_V2_VAL;
> + if (qcom_ice_readl(ice,
> + HWKM_OFFSET(QCOM_ICE_REG_HWKM_TZ_KM_STATUS)) !=
> + bist_done_val) {
> + dev_warn(ice->dev, "HWKM BIST error\n");
> + ice->use_hwkm = false;
error is not passed to caller. if HWKM is enabled, and BIST failed, ICE
init also should be considered has failure.
Any reason considering it as warning only ?
> + }
> + }
> return err;
> }
>
> +static void qcom_ice_enable_standard_mode(struct qcom_ice *ice)
> +{
> + u32 val = 0;
> +
> + if (!ice->use_hwkm)
> + return;
> +
> + /*
> + * When ICE is in standard (hwkm) mode, it supports HW wrapped
> + * keys, and when it is in legacy mode, it only supports standard
> + * (non HW wrapped) keys.
> + *
> + * Put ICE in standard mode, ICE defaults to legacy mode.
> + * Legacy mode - ICE HWKM slave not supported.
> + * Standard mode - ICE HWKM slave supported.
> + *
> + * Depending on the version of HWKM, it is controlled by different
> + * registers in ICE.
> + */
> + if (ice->hwkm_version >= 2) {
This programming instruction may create problem fn future of hwkm
version is not compatible. Better way is:
if (ice->hwkm_version == 2 && ice->hwkm_version == 3)
> + val = qcom_ice_readl(ice, QCOM_ICE_REG_CONTROL);
> + val = val & 0xFFFFFFFE;
> + qcom_ice_writel(ice, val, QCOM_ICE_REG_CONTROL);
> + } else {
> + qcom_ice_writel(ice, 0x7,
void using constant value like "0x7" in code use #define with meaningful
name that can indicate what operation is being performed.
This comment is applicable for many places.
> + HWKM_OFFSET(QCOM_ICE_REG_HWKM_TZ_KM_CTL));
> + }
> +}
> +
> +static void qcom_ice_hwkm_init(struct qcom_ice *ice)
> +{
> + if (!ice->use_hwkm)
> + return;
> +
> + /* Disable CRC checks. This HWKM feature is not used. */
> + qcom_ice_writel(ice, 0x6,
> + HWKM_OFFSET(QCOM_ICE_REG_HWKM_TZ_KM_CTL));
> +
> + /*
> + * Give register bank of the HWKM slave access to read and modify
> + * the keyslots in ICE HWKM slave. Without this, trustzone will not
> + * be able to program keys into ICE.
> + */
> + qcom_ice_writel(ice, 0xFFFFFFFF,
> + HWKM_OFFSET(QCOM_ICE_REG_HWKM_BANK0_BBAC_0));
> + qcom_ice_writel(ice, 0xFFFFFFFF,
> + HWKM_OFFSET(QCOM_ICE_REG_HWKM_BANK0_BBAC_1));
> + qcom_ice_writel(ice, 0xFFFFFFFF,
> + HWKM_OFFSET(QCOM_ICE_REG_HWKM_BANK0_BBAC_2));
> + qcom_ice_writel(ice, 0xFFFFFFFF,
> + HWKM_OFFSET(QCOM_ICE_REG_HWKM_BANK0_BBAC_3));
> + qcom_ice_writel(ice, 0xFFFFFFFF,
> + HWKM_OFFSET(QCOM_ICE_REG_HWKM_BANK0_BBAC_4));
> +
> + /* Clear HWKM response FIFO before doing anything */
> + qcom_ice_writel(ice, 0x8,
> + HWKM_OFFSET(QCOM_ICE_REG_HWKM_BANK0_BANKN_IRQ_STATUS));
> +}
> +
> int qcom_ice_enable(struct qcom_ice *ice)
> {
> + int err;
> +
> qcom_ice_low_power_mode_enable(ice);
> qcom_ice_optimization_enable(ice);
>
> - return qcom_ice_wait_bist_status(ice);
> + qcom_ice_enable_standard_mode(ice);
> +
> + err = qcom_ice_wait_bist_status(ice);
> + if (err)
> + return err;
> +
> + qcom_ice_hwkm_init(ice);
new code added in this section is only application when hwkm is enabled.
if (!ice->hwkm_version) {
qcom_ice_enable_standard_mode(ice);
qcom_ice_hwkm_init(ice);
}
> +
> + return err;
> }
> EXPORT_SYMBOL_GPL(qcom_ice_enable);
>
> @@ -149,6 +270,8 @@ int qcom_ice_resume(struct qcom_ice *ice)
> return err;
> }
>
> + qcom_ice_enable_standard_mode(ice);
> + qcom_ice_hwkm_init(ice);
> return qcom_ice_wait_bist_status(ice);
> }
> EXPORT_SYMBOL_GPL(qcom_ice_resume);
> @@ -205,6 +328,12 @@ int qcom_ice_evict_key(struct qcom_ice *ice, int slot)
> }
> EXPORT_SYMBOL_GPL(qcom_ice_evict_key);
>
> +bool qcom_ice_hwkm_supported(struct qcom_ice *ice)
> +{
> + return ice->use_hwkm;
> +}
> +EXPORT_SYMBOL_GPL(qcom_ice_hwkm_supported);
> +
> static struct qcom_ice *qcom_ice_create(struct device *dev,
> void __iomem *base)
> {
> @@ -239,6 +368,8 @@ static struct qcom_ice *qcom_ice_create(struct device *dev,
> engine->core_clk = devm_clk_get_enabled(dev, NULL);
> if (IS_ERR(engine->core_clk))
> return ERR_CAST(engine->core_clk);
> + engine->use_hwkm = of_property_read_bool(dev->of_node,
> + "qcom,ice-use-hwkm");
>
> if (!qcom_ice_check_supported(engine))
> return ERR_PTR(-EOPNOTSUPP);
> diff --git a/include/soc/qcom/ice.h b/include/soc/qcom/ice.h
> index 9dd835dba2a7..1f52e82e3e1c 100644
> --- a/include/soc/qcom/ice.h
> +++ b/include/soc/qcom/ice.h
> @@ -34,5 +34,6 @@ int qcom_ice_program_key(struct qcom_ice *ice,
> const struct blk_crypto_key *bkey,
> u8 data_unit_size, int slot);
> int qcom_ice_evict_key(struct qcom_ice *ice, int slot);
> +bool qcom_ice_hwkm_supported(struct qcom_ice *ice);
> struct qcom_ice *of_qcom_ice_get(struct device *dev);
> #endif /* __QCOM_ICE_H__ */
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v3 04/12] soc: qcom: ice: support for hardware wrapped keys
2023-11-22 5:38 [PATCH v3 00/12] Hardware wrapped key support for qcom ice and ufs Gaurav Kashyap
` (2 preceding siblings ...)
2023-11-22 5:38 ` [PATCH v3 03/12] soc: qcom: ice: add hwkm support in ice Gaurav Kashyap
@ 2023-11-22 5:38 ` Gaurav Kashyap
2023-12-08 7:45 ` Om Prakash Singh
2023-11-22 5:38 ` [PATCH v3 05/12] ufs: core: support wrapped keys in ufs core Gaurav Kashyap
` (9 subsequent siblings)
13 siblings, 1 reply; 39+ messages in thread
From: Gaurav Kashyap @ 2023-11-22 5:38 UTC (permalink / raw)
To: linux-scsi, linux-arm-msm, ebiggers, neil.armstrong, srinivas.kandagatla
Cc: linux-mmc, linux-block, linux-fscrypt, omprsing, quic_psodagud,
abel.vesa, quic_spuppala, kernel, Gaurav Kashyap
Now that HWKM support is added to ICE, extend the ICE
driver to support hardware wrapped keys programming coming
in from the storage controllers (ufs and emmc). The patches that follow
will add ufs and emmc support.
Derive software secret support is also added by forwarding the
call the corresponding scm api.
Signed-off-by: Gaurav Kashyap <quic_gaurkash@quicinc.com>
---
drivers/soc/qcom/ice.c | 114 +++++++++++++++++++++++++++++++++++++----
include/soc/qcom/ice.h | 4 ++
2 files changed, 107 insertions(+), 11 deletions(-)
diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c
index adf9cab848fa..ee7c0beef3d2 100644
--- a/drivers/soc/qcom/ice.c
+++ b/drivers/soc/qcom/ice.c
@@ -27,6 +27,8 @@
#define QCOM_ICE_REG_BIST_STATUS 0x0070
#define QCOM_ICE_REG_ADVANCED_CONTROL 0x1000
#define QCOM_ICE_REG_CONTROL 0x0
+#define QCOM_ICE_LUT_KEYS_CRYPTOCFG_R16 0x4040
+
/* QCOM ICE HWKM registers */
#define QCOM_ICE_REG_HWKM_TZ_KM_CTL 0x1000
#define QCOM_ICE_REG_HWKM_TZ_KM_STATUS 0x1004
@@ -37,6 +39,7 @@
#define QCOM_ICE_REG_HWKM_BANK0_BBAC_3 0x500C
#define QCOM_ICE_REG_HWKM_BANK0_BBAC_4 0x5010
+/* QCOM ICE HWKM BIST vals */
#define QCOM_ICE_HWKM_BIST_DONE_V1_VAL 0x11
#define QCOM_ICE_HWKM_BIST_DONE_V2_VAL 0x287
@@ -47,6 +50,8 @@
#define QCOM_ICE_FORCE_HW_KEY0_SETTING_MASK 0x2
#define QCOM_ICE_FORCE_HW_KEY1_SETTING_MASK 0x4
+#define QCOM_ICE_LUT_KEYS_CRYPTOCFG_OFFSET 0x80
+
#define QCOM_ICE_HWKM_REG_OFFSET 0x8000
#define HWKM_OFFSET(reg) ((reg) + QCOM_ICE_HWKM_REG_OFFSET)
@@ -67,6 +72,16 @@ struct qcom_ice {
bool hwkm_init_complete;
};
+union crypto_cfg {
+ __le32 regval;
+ struct {
+ u8 dusize;
+ u8 capidx;
+ u8 reserved;
+ u8 cfge;
+ };
+};
+
static bool qcom_ice_check_supported(struct qcom_ice *ice)
{
u32 regval = qcom_ice_readl(ice, QCOM_ICE_REG_VERSION);
@@ -237,6 +252,8 @@ static void qcom_ice_hwkm_init(struct qcom_ice *ice)
/* Clear HWKM response FIFO before doing anything */
qcom_ice_writel(ice, 0x8,
HWKM_OFFSET(QCOM_ICE_REG_HWKM_BANK0_BANKN_IRQ_STATUS));
+
+ ice->hwkm_init_complete = true;
}
int qcom_ice_enable(struct qcom_ice *ice)
@@ -284,6 +301,51 @@ int qcom_ice_suspend(struct qcom_ice *ice)
}
EXPORT_SYMBOL_GPL(qcom_ice_suspend);
+/*
+ * HW dictates the internal mapping between the ICE and HWKM slots,
+ * which are different for different versions, make the translation
+ * here. For v1 however, the translation is done in trustzone.
+ */
+static int translate_hwkm_slot(struct qcom_ice *ice, int slot)
+{
+ return (ice->hwkm_version == 1) ? slot : (slot * 2);
+}
+
+static int qcom_ice_program_wrapped_key(struct qcom_ice *ice,
+ const struct blk_crypto_key *key,
+ u8 data_unit_size, int slot)
+{
+ int hwkm_slot;
+ int err;
+ union crypto_cfg cfg;
+
+ hwkm_slot = translate_hwkm_slot(ice, slot);
+
+ memset(&cfg, 0, sizeof(cfg));
+ cfg.dusize = data_unit_size;
+ cfg.capidx = QCOM_SCM_ICE_CIPHER_AES_256_XTS;
+ cfg.cfge = 0x80;
+
+ /* Clear CFGE */
+ qcom_ice_writel(ice, 0x0, QCOM_ICE_LUT_KEYS_CRYPTOCFG_R16 +
+ QCOM_ICE_LUT_KEYS_CRYPTOCFG_OFFSET * slot);
+
+ /* Call trustzone to program the wrapped key using hwkm */
+ err = qcom_scm_ice_set_key(hwkm_slot, key->raw, key->size,
+ QCOM_SCM_ICE_CIPHER_AES_256_XTS, data_unit_size);
+ if (err) {
+ pr_err("%s:SCM call Error: 0x%x slot %d\n", __func__, err,
+ slot);
+ return err;
+ }
+
+ /* Enable CFGE after programming key */
+ qcom_ice_writel(ice, cfg.regval, QCOM_ICE_LUT_KEYS_CRYPTOCFG_R16 +
+ QCOM_ICE_LUT_KEYS_CRYPTOCFG_OFFSET * slot);
+
+ return err;
+}
+
int qcom_ice_program_key(struct qcom_ice *ice,
u8 algorithm_id, u8 key_size,
const struct blk_crypto_key *bkey,
@@ -299,24 +361,31 @@ int qcom_ice_program_key(struct qcom_ice *ice,
/* Only AES-256-XTS has been tested so far. */
if (algorithm_id != QCOM_ICE_CRYPTO_ALG_AES_XTS ||
- key_size != QCOM_ICE_CRYPTO_KEY_SIZE_256) {
+ (key_size != QCOM_ICE_CRYPTO_KEY_SIZE_256 &&
+ key_size != QCOM_ICE_CRYPTO_KEY_SIZE_WRAPPED)) {
dev_err_ratelimited(dev,
"Unhandled crypto capability; algorithm_id=%d, key_size=%d\n",
algorithm_id, key_size);
return -EINVAL;
}
- memcpy(key.bytes, bkey->raw, AES_256_XTS_KEY_SIZE);
-
- /* The SCM call requires that the key words are encoded in big endian */
- for (i = 0; i < ARRAY_SIZE(key.words); i++)
- __cpu_to_be32s(&key.words[i]);
+ if (bkey->crypto_cfg.key_type == BLK_CRYPTO_KEY_TYPE_HW_WRAPPED) {
+ if (!ice->use_hwkm)
+ return -EINVAL;
+ err = qcom_ice_program_wrapped_key(ice, bkey, data_unit_size,
+ slot);
+ } else {
+ memcpy(key.bytes, bkey->raw, AES_256_XTS_KEY_SIZE);
- err = qcom_scm_ice_set_key(slot, key.bytes, AES_256_XTS_KEY_SIZE,
- QCOM_SCM_ICE_CIPHER_AES_256_XTS,
- data_unit_size);
+ /* The SCM call requires that the key words are encoded in big endian */
+ for (i = 0; i < ARRAY_SIZE(key.words); i++)
+ __cpu_to_be32s(&key.words[i]);
- memzero_explicit(&key, sizeof(key));
+ err = qcom_scm_ice_set_key(slot, key.bytes, AES_256_XTS_KEY_SIZE,
+ QCOM_SCM_ICE_CIPHER_AES_256_XTS,
+ data_unit_size);
+ memzero_explicit(&key, sizeof(key));
+ }
return err;
}
@@ -324,7 +393,21 @@ EXPORT_SYMBOL_GPL(qcom_ice_program_key);
int qcom_ice_evict_key(struct qcom_ice *ice, int slot)
{
- return qcom_scm_ice_invalidate_key(slot);
+ int hwkm_slot = slot;
+
+ if (ice->use_hwkm) {
+ hwkm_slot = translate_hwkm_slot(ice, slot);
+ /*
+ * Ignore calls to evict key when HWKM is supported and hwkm init
+ * is not yet done. This is to avoid the clearing all slots call
+ * during a storage reset when ICE is still in legacy mode. HWKM slave
+ * in ICE takes care of zeroing out the keytable on reset.
+ */
+ if (!ice->hwkm_init_complete)
+ return 0;
+ }
+
+ return qcom_scm_ice_invalidate_key(hwkm_slot);
}
EXPORT_SYMBOL_GPL(qcom_ice_evict_key);
@@ -334,6 +417,15 @@ bool qcom_ice_hwkm_supported(struct qcom_ice *ice)
}
EXPORT_SYMBOL_GPL(qcom_ice_hwkm_supported);
+int qcom_ice_derive_sw_secret(struct qcom_ice *ice, const u8 wkey[],
+ unsigned int wkey_size,
+ u8 sw_secret[BLK_CRYPTO_SW_SECRET_SIZE])
+{
+ return qcom_scm_derive_sw_secret(wkey, wkey_size,
+ sw_secret, BLK_CRYPTO_SW_SECRET_SIZE);
+}
+EXPORT_SYMBOL_GPL(qcom_ice_derive_sw_secret);
+
static struct qcom_ice *qcom_ice_create(struct device *dev,
void __iomem *base)
{
diff --git a/include/soc/qcom/ice.h b/include/soc/qcom/ice.h
index 1f52e82e3e1c..dabe0d3a1fd0 100644
--- a/include/soc/qcom/ice.h
+++ b/include/soc/qcom/ice.h
@@ -17,6 +17,7 @@ enum qcom_ice_crypto_key_size {
QCOM_ICE_CRYPTO_KEY_SIZE_192 = 0x2,
QCOM_ICE_CRYPTO_KEY_SIZE_256 = 0x3,
QCOM_ICE_CRYPTO_KEY_SIZE_512 = 0x4,
+ QCOM_ICE_CRYPTO_KEY_SIZE_WRAPPED = 0x5,
};
enum qcom_ice_crypto_alg {
@@ -35,5 +36,8 @@ int qcom_ice_program_key(struct qcom_ice *ice,
u8 data_unit_size, int slot);
int qcom_ice_evict_key(struct qcom_ice *ice, int slot);
bool qcom_ice_hwkm_supported(struct qcom_ice *ice);
+int qcom_ice_derive_sw_secret(struct qcom_ice *ice, const u8 wkey[],
+ unsigned int wkey_size,
+ u8 sw_secret[BLK_CRYPTO_SW_SECRET_SIZE]);
struct qcom_ice *of_qcom_ice_get(struct device *dev);
#endif /* __QCOM_ICE_H__ */
--
2.25.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [PATCH v3 04/12] soc: qcom: ice: support for hardware wrapped keys
2023-11-22 5:38 ` [PATCH v3 04/12] soc: qcom: ice: support for hardware wrapped keys Gaurav Kashyap
@ 2023-12-08 7:45 ` Om Prakash Singh
2023-12-12 4:04 ` Gaurav Kashyap
0 siblings, 1 reply; 39+ messages in thread
From: Om Prakash Singh @ 2023-12-08 7:45 UTC (permalink / raw)
To: Gaurav Kashyap, linux-scsi, linux-arm-msm, ebiggers,
neil.armstrong, srinivas.kandagatla
Cc: linux-mmc, linux-block, linux-fscrypt, omprsing, quic_psodagud,
abel.vesa, quic_spuppala, kernel
On 11/22/2023 11:08 AM, Gaurav Kashyap wrote:
> Now that HWKM support is added to ICE, extend the ICE
> driver to support hardware wrapped keys programming coming
> in from the storage controllers (ufs and emmc). The patches that follow
> will add ufs and emmc support.
>
> Derive software secret support is also added by forwarding the
> call the corresponding scm api.
>
> Signed-off-by: Gaurav Kashyap <quic_gaurkash@quicinc.com>
> ---
> drivers/soc/qcom/ice.c | 114 +++++++++++++++++++++++++++++++++++++----
> include/soc/qcom/ice.h | 4 ++
> 2 files changed, 107 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c
> index adf9cab848fa..ee7c0beef3d2 100644
> --- a/drivers/soc/qcom/ice.c
> +++ b/drivers/soc/qcom/ice.c
> @@ -27,6 +27,8 @@
> #define QCOM_ICE_REG_BIST_STATUS 0x0070
> #define QCOM_ICE_REG_ADVANCED_CONTROL 0x1000
> #define QCOM_ICE_REG_CONTROL 0x0
> +#define QCOM_ICE_LUT_KEYS_CRYPTOCFG_R16 0x4040
> +
> /* QCOM ICE HWKM registers */
> #define QCOM_ICE_REG_HWKM_TZ_KM_CTL 0x1000
> #define QCOM_ICE_REG_HWKM_TZ_KM_STATUS 0x1004
> @@ -37,6 +39,7 @@
> #define QCOM_ICE_REG_HWKM_BANK0_BBAC_3 0x500C
> #define QCOM_ICE_REG_HWKM_BANK0_BBAC_4 0x5010
>
> +/* QCOM ICE HWKM BIST vals */
> #define QCOM_ICE_HWKM_BIST_DONE_V1_VAL 0x11
> #define QCOM_ICE_HWKM_BIST_DONE_V2_VAL 0x287
>
> @@ -47,6 +50,8 @@
> #define QCOM_ICE_FORCE_HW_KEY0_SETTING_MASK 0x2
> #define QCOM_ICE_FORCE_HW_KEY1_SETTING_MASK 0x4
>
> +#define QCOM_ICE_LUT_KEYS_CRYPTOCFG_OFFSET 0x80
> +
> #define QCOM_ICE_HWKM_REG_OFFSET 0x8000
> #define HWKM_OFFSET(reg) ((reg) + QCOM_ICE_HWKM_REG_OFFSET)
>
> @@ -67,6 +72,16 @@ struct qcom_ice {
> bool hwkm_init_complete;
> };
>
> +union crypto_cfg {
> + __le32 regval;
> + struct {
> + u8 dusize;
> + u8 capidx;
> + u8 reserved;
> + u8 cfge;
> + };
> +};
> +
> static bool qcom_ice_check_supported(struct qcom_ice *ice)
> {
> u32 regval = qcom_ice_readl(ice, QCOM_ICE_REG_VERSION);
> @@ -237,6 +252,8 @@ static void qcom_ice_hwkm_init(struct qcom_ice *ice)
> /* Clear HWKM response FIFO before doing anything */
> qcom_ice_writel(ice, 0x8,
> HWKM_OFFSET(QCOM_ICE_REG_HWKM_BANK0_BANKN_IRQ_STATUS));
> +
> + ice->hwkm_init_complete = true;
This this change should go with previous patch 3/12.
> }
>
> int qcom_ice_enable(struct qcom_ice *ice)
> @@ -284,6 +301,51 @@ int qcom_ice_suspend(struct qcom_ice *ice)
> }
> EXPORT_SYMBOL_GPL(qcom_ice_suspend);
>
> +/*
> + * HW dictates the internal mapping between the ICE and HWKM slots,
> + * which are different for different versions, make the translation
> + * here. For v1 however, the translation is done in trustzone.
> + */
> +static int translate_hwkm_slot(struct qcom_ice *ice, int slot)
> +{
> + return (ice->hwkm_version == 1) ? slot : (slot * 2);
> +}
> +
> +static int qcom_ice_program_wrapped_key(struct qcom_ice *ice,
> + const struct blk_crypto_key *key,
> + u8 data_unit_size, int slot)
> +{
> + int hwkm_slot;
> + int err;
> + union crypto_cfg cfg;
> +
> + hwkm_slot = translate_hwkm_slot(ice, slot);
> +
> + memset(&cfg, 0, sizeof(cfg));
> + cfg.dusize = data_unit_size;
> + cfg.capidx = QCOM_SCM_ICE_CIPHER_AES_256_XTS;
> + cfg.cfge = 0x80;
use macro for constant value "0x80"
> +
> + /* Clear CFGE */
> + qcom_ice_writel(ice, 0x0, QCOM_ICE_LUT_KEYS_CRYPTOCFG_R16 +
> + QCOM_ICE_LUT_KEYS_CRYPTOCFG_OFFSET * slot);
> +
> + /* Call trustzone to program the wrapped key using hwkm */
> + err = qcom_scm_ice_set_key(hwkm_slot, key->raw, key->size,
> + QCOM_SCM_ICE_CIPHER_AES_256_XTS, data_unit_size);
> + if (err) {
> + pr_err("%s:SCM call Error: 0x%x slot %d\n", __func__, err,
> + slot);
> + return err;
> + }
> +
> + /* Enable CFGE after programming key */
> + qcom_ice_writel(ice, cfg.regval, QCOM_ICE_LUT_KEYS_CRYPTOCFG_R16 +
> + QCOM_ICE_LUT_KEYS_CRYPTOCFG_OFFSET * slot);
> +
> + return err;
> +}
> +
> int qcom_ice_program_key(struct qcom_ice *ice,
> u8 algorithm_id, u8 key_size,
> const struct blk_crypto_key *bkey,
> @@ -299,24 +361,31 @@ int qcom_ice_program_key(struct qcom_ice *ice,
>
> /* Only AES-256-XTS has been tested so far. */
> if (algorithm_id != QCOM_ICE_CRYPTO_ALG_AES_XTS ||
> - key_size != QCOM_ICE_CRYPTO_KEY_SIZE_256) {
> + (key_size != QCOM_ICE_CRYPTO_KEY_SIZE_256 &&
> + key_size != QCOM_ICE_CRYPTO_KEY_SIZE_WRAPPED)) {
Can you please check the logic with && operation. the condition will
always be false.
> dev_err_ratelimited(dev,
> "Unhandled crypto capability; algorithm_id=%d, key_size=%d\n",
> algorithm_id, key_size);
> return -EINVAL;
> }
>
> - memcpy(key.bytes, bkey->raw, AES_256_XTS_KEY_SIZE);
> -
> - /* The SCM call requires that the key words are encoded in big endian */
> - for (i = 0; i < ARRAY_SIZE(key.words); i++)
> - __cpu_to_be32s(&key.words[i]);
> + if (bkey->crypto_cfg.key_type == BLK_CRYPTO_KEY_TYPE_HW_WRAPPED) {
> + if (!ice->use_hwkm)
> + return -EINVAL;
having error log in failure case would help in debugging.
> + err = qcom_ice_program_wrapped_key(ice, bkey, data_unit_size,
> + slot);
> + } else {
> + memcpy(key.bytes, bkey->raw, AES_256_XTS_KEY_SIZE);
>
> - err = qcom_scm_ice_set_key(slot, key.bytes, AES_256_XTS_KEY_SIZE,
> - QCOM_SCM_ICE_CIPHER_AES_256_XTS,
> - data_unit_size);
> + /* The SCM call requires that the key words are encoded in big endian */
> + for (i = 0; i < ARRAY_SIZE(key.words); i++)
> + __cpu_to_be32s(&key.words[i]);
>
> - memzero_explicit(&key, sizeof(key));
> + err = qcom_scm_ice_set_key(slot, key.bytes, AES_256_XTS_KEY_SIZE,
> + QCOM_SCM_ICE_CIPHER_AES_256_XTS,
> + data_unit_size);
> + memzero_explicit(&key, sizeof(key));
> + }
>
> return err;
> }
> @@ -324,7 +393,21 @@ EXPORT_SYMBOL_GPL(qcom_ice_program_key);
>
> int qcom_ice_evict_key(struct qcom_ice *ice, int slot)
> {
> - return qcom_scm_ice_invalidate_key(slot);
> + int hwkm_slot = slot;
> +
> + if (ice->use_hwkm) {
> + hwkm_slot = translate_hwkm_slot(ice, slot);
> + /*
> + * Ignore calls to evict key when HWKM is supported and hwkm init
> + * is not yet done. This is to avoid the clearing all slots call
> + * during a storage reset when ICE is still in legacy mode. HWKM slave
> + * in ICE takes care of zeroing out the keytable on reset.
> + */
> + if (!ice->hwkm_init_complete)
> + return 0;
> + }
> +
> + return qcom_scm_ice_invalidate_key(hwkm_slot);
> }
> EXPORT_SYMBOL_GPL(qcom_ice_evict_key);
>
> @@ -334,6 +417,15 @@ bool qcom_ice_hwkm_supported(struct qcom_ice *ice)
> }
> EXPORT_SYMBOL_GPL(qcom_ice_hwkm_supported);
>
> +int qcom_ice_derive_sw_secret(struct qcom_ice *ice, const u8 wkey[],
> + unsigned int wkey_size,
> + u8 sw_secret[BLK_CRYPTO_SW_SECRET_SIZE])
> +{
> + return qcom_scm_derive_sw_secret(wkey, wkey_size,
> + sw_secret, BLK_CRYPTO_SW_SECRET_SIZE);
> +}
> +EXPORT_SYMBOL_GPL(qcom_ice_derive_sw_secret);
> +
> static struct qcom_ice *qcom_ice_create(struct device *dev,
> void __iomem *base)
> {
> diff --git a/include/soc/qcom/ice.h b/include/soc/qcom/ice.h
> index 1f52e82e3e1c..dabe0d3a1fd0 100644
> --- a/include/soc/qcom/ice.h
> +++ b/include/soc/qcom/ice.h
> @@ -17,6 +17,7 @@ enum qcom_ice_crypto_key_size {
> QCOM_ICE_CRYPTO_KEY_SIZE_192 = 0x2,
> QCOM_ICE_CRYPTO_KEY_SIZE_256 = 0x3,
> QCOM_ICE_CRYPTO_KEY_SIZE_512 = 0x4,
> + QCOM_ICE_CRYPTO_KEY_SIZE_WRAPPED = 0x5,
> };
>
> enum qcom_ice_crypto_alg {
> @@ -35,5 +36,8 @@ int qcom_ice_program_key(struct qcom_ice *ice,
> u8 data_unit_size, int slot);
> int qcom_ice_evict_key(struct qcom_ice *ice, int slot);
> bool qcom_ice_hwkm_supported(struct qcom_ice *ice);
> +int qcom_ice_derive_sw_secret(struct qcom_ice *ice, const u8 wkey[],
> + unsigned int wkey_size,
> + u8 sw_secret[BLK_CRYPTO_SW_SECRET_SIZE]);
> struct qcom_ice *of_qcom_ice_get(struct device *dev);
> #endif /* __QCOM_ICE_H__ */
^ permalink raw reply [flat|nested] 39+ messages in thread* RE: [PATCH v3 04/12] soc: qcom: ice: support for hardware wrapped keys
2023-12-08 7:45 ` Om Prakash Singh
@ 2023-12-12 4:04 ` Gaurav Kashyap
0 siblings, 0 replies; 39+ messages in thread
From: Gaurav Kashyap @ 2023-12-12 4:04 UTC (permalink / raw)
To: Om Prakash Singh (QUIC), Gaurav Kashyap (QUIC),
linux-scsi, linux-arm-msm, ebiggers, neil.armstrong,
srinivas.kandagatla
Cc: linux-mmc, linux-block, linux-fscrypt, Om Prakash Singh,
Prasad Sodagudi (QUIC), abel.vesa, Seshu Madhavi Puppala (QUIC),
kernel
Hello Om
On 12/07/2023, Om Prakash Singh wrote:
> On 11/22/2023 11:08 AM, Gaurav Kashyap wrote:
> > Now that HWKM support is added to ICE, extend the ICE driver to
> > support hardware wrapped keys programming coming in from the storage
> > controllers (ufs and emmc). The patches that follow will add ufs and
> > emmc support.
> >
> > Derive software secret support is also added by forwarding the call
> > the corresponding scm api.
> >
> > Signed-off-by: Gaurav Kashyap <quic_gaurkash@quicinc.com>
> > ---
> > drivers/soc/qcom/ice.c | 114
> +++++++++++++++++++++++++++++++++++++----
> > include/soc/qcom/ice.h | 4 ++
> > 2 files changed, 107 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c index
> > adf9cab848fa..ee7c0beef3d2 100644
> > --- a/drivers/soc/qcom/ice.c
> > +++ b/drivers/soc/qcom/ice.c
> > @@ -27,6 +27,8 @@
> > #define QCOM_ICE_REG_BIST_STATUS 0x0070
> > #define QCOM_ICE_REG_ADVANCED_CONTROL 0x1000
> > #define QCOM_ICE_REG_CONTROL 0x0
> > +#define QCOM_ICE_LUT_KEYS_CRYPTOCFG_R16 0x4040
> > +
> > /* QCOM ICE HWKM registers */
> > #define QCOM_ICE_REG_HWKM_TZ_KM_CTL
> 0x1000
> > #define QCOM_ICE_REG_HWKM_TZ_KM_STATUS
> 0x1004
> > @@ -37,6 +39,7 @@
> > #define QCOM_ICE_REG_HWKM_BANK0_BBAC_3
> 0x500C
> > #define QCOM_ICE_REG_HWKM_BANK0_BBAC_4
> 0x5010
> >
> > +/* QCOM ICE HWKM BIST vals */
> > #define QCOM_ICE_HWKM_BIST_DONE_V1_VAL 0x11
> > #define QCOM_ICE_HWKM_BIST_DONE_V2_VAL 0x287
> >
> > @@ -47,6 +50,8 @@
> > #define QCOM_ICE_FORCE_HW_KEY0_SETTING_MASK 0x2
> > #define QCOM_ICE_FORCE_HW_KEY1_SETTING_MASK 0x4
> >
> > +#define QCOM_ICE_LUT_KEYS_CRYPTOCFG_OFFSET 0x80
> > +
> > #define QCOM_ICE_HWKM_REG_OFFSET 0x8000
> > #define HWKM_OFFSET(reg) ((reg) +
> QCOM_ICE_HWKM_REG_OFFSET)
> >
> > @@ -67,6 +72,16 @@ struct qcom_ice {
> > bool hwkm_init_complete;
> > };
> >
> > +union crypto_cfg {
> > + __le32 regval;
> > + struct {
> > + u8 dusize;
> > + u8 capidx;
> > + u8 reserved;
> > + u8 cfge;
> > + };
> > +};
> > +
> > static bool qcom_ice_check_supported(struct qcom_ice *ice)
> > {
> > u32 regval = qcom_ice_readl(ice, QCOM_ICE_REG_VERSION); @@ -
> 237,6
> > +252,8 @@ static void qcom_ice_hwkm_init(struct qcom_ice *ice)
> > /* Clear HWKM response FIFO before doing anything */
> > qcom_ice_writel(ice, 0x8,
> >
> HWKM_OFFSET(QCOM_ICE_REG_HWKM_BANK0_BANKN_IRQ_STA
> TUS));
> > +
> > + ice->hwkm_init_complete = true;
> This this change should go with previous patch 3/12.
Okay, makes sense.
> > }
> >
> > int qcom_ice_enable(struct qcom_ice *ice) @@ -284,6 +301,51 @@ int
> > qcom_ice_suspend(struct qcom_ice *ice)
> > }
> > EXPORT_SYMBOL_GPL(qcom_ice_suspend);
> >
> > +/*
> > + * HW dictates the internal mapping between the ICE and HWKM slots,
> > + * which are different for different versions, make the translation
> > + * here. For v1 however, the translation is done in trustzone.
> > + */
> > +static int translate_hwkm_slot(struct qcom_ice *ice, int slot) {
> > + return (ice->hwkm_version == 1) ? slot : (slot * 2); }
> > +
> > +static int qcom_ice_program_wrapped_key(struct qcom_ice *ice,
> > + const struct blk_crypto_key *key,
> > + u8 data_unit_size, int slot)
> > +{
> > + int hwkm_slot;
> > + int err;
> > + union crypto_cfg cfg;
> > +
> > + hwkm_slot = translate_hwkm_slot(ice, slot);
> > +
> > + memset(&cfg, 0, sizeof(cfg));
> > + cfg.dusize = data_unit_size;
> > + cfg.capidx = QCOM_SCM_ICE_CIPHER_AES_256_XTS;
> > + cfg.cfge = 0x80;
> use macro for constant value "0x80"
> > +
> > + /* Clear CFGE */
> > + qcom_ice_writel(ice, 0x0, QCOM_ICE_LUT_KEYS_CRYPTOCFG_R16 +
> > + QCOM_ICE_LUT_KEYS_CRYPTOCFG_OFFSET
> * slot);
> > +
> > + /* Call trustzone to program the wrapped key using hwkm */
> > + err = qcom_scm_ice_set_key(hwkm_slot, key->raw, key->size,
> > + QCOM_SCM_ICE_CIPHER_AES_256_XTS,
> data_unit_size);
> > + if (err) {
> > + pr_err("%s:SCM call Error: 0x%x slot %d\n", __func__, err,
> > + slot);
> > + return err;
> > + }
> > +
> > + /* Enable CFGE after programming key */
> > + qcom_ice_writel(ice, cfg.regval,
> QCOM_ICE_LUT_KEYS_CRYPTOCFG_R16 +
> > +
> QCOM_ICE_LUT_KEYS_CRYPTOCFG_OFFSET * slot);
> > +
> > + return err;
> > +}
> > +
> > int qcom_ice_program_key(struct qcom_ice *ice,
> > u8 algorithm_id, u8 key_size,
> > const struct blk_crypto_key *bkey, @@ -299,24
> +361,31 @@ int
> > qcom_ice_program_key(struct qcom_ice *ice,
> >
> > /* Only AES-256-XTS has been tested so far. */
> > if (algorithm_id != QCOM_ICE_CRYPTO_ALG_AES_XTS ||
> > - key_size != QCOM_ICE_CRYPTO_KEY_SIZE_256) {
> > + (key_size != QCOM_ICE_CRYPTO_KEY_SIZE_256 &&
> > + key_size != QCOM_ICE_CRYPTO_KEY_SIZE_WRAPPED)) {
> Can you please check the logic with && operation. the condition will always
> be false.
No, the condition won't always be false, my v2 patches were wrong which was pointed out by Neil
https://lore.kernel.org/all/fcbf6dee-aa6a-4af8-9ff1-495adbcb5a57@linaro.org/
The condition would check if the size passed is either 256 or wrapped.
> > dev_err_ratelimited(dev,
> > "Unhandled crypto capability;
> algorithm_id=%d, key_size=%d\n",
> > algorithm_id, key_size);
> > return -EINVAL;
> > }
> >
> > - memcpy(key.bytes, bkey->raw, AES_256_XTS_KEY_SIZE);
> > -
> > - /* The SCM call requires that the key words are encoded in big
> endian */
> > - for (i = 0; i < ARRAY_SIZE(key.words); i++)
> > - __cpu_to_be32s(&key.words[i]);
> > + if (bkey->crypto_cfg.key_type ==
> BLK_CRYPTO_KEY_TYPE_HW_WRAPPED) {
> > + if (!ice->use_hwkm)
> > + return -EINVAL;
> having error log in failure case would help in debugging.
> > + err = qcom_ice_program_wrapped_key(ice, bkey,
> data_unit_size,
> > + slot);
> > + } else {
> > + memcpy(key.bytes, bkey->raw, AES_256_XTS_KEY_SIZE);
> >
> > - err = qcom_scm_ice_set_key(slot, key.bytes,
> AES_256_XTS_KEY_SIZE,
> > - QCOM_SCM_ICE_CIPHER_AES_256_XTS,
> > - data_unit_size);
> > + /* The SCM call requires that the key words are encoded in
> big endian */
> > + for (i = 0; i < ARRAY_SIZE(key.words); i++)
> > + __cpu_to_be32s(&key.words[i]);
> >
> > - memzero_explicit(&key, sizeof(key));
> > + err = qcom_scm_ice_set_key(slot, key.bytes,
> AES_256_XTS_KEY_SIZE,
> > +
> QCOM_SCM_ICE_CIPHER_AES_256_XTS,
> > + data_unit_size);
> > + memzero_explicit(&key, sizeof(key));
> > + }
> >
> > return err;
> > }
> > @@ -324,7 +393,21 @@ EXPORT_SYMBOL_GPL(qcom_ice_program_key);
> >
> > int qcom_ice_evict_key(struct qcom_ice *ice, int slot)
> > {
> > - return qcom_scm_ice_invalidate_key(slot);
> > + int hwkm_slot = slot;
> > +
> > + if (ice->use_hwkm) {
> > + hwkm_slot = translate_hwkm_slot(ice, slot);
> > + /*
> > + * Ignore calls to evict key when HWKM is supported and hwkm init
> > + * is not yet done. This is to avoid the clearing all slots call
> > + * during a storage reset when ICE is still in legacy mode. HWKM slave
> > + * in ICE takes care of zeroing out the keytable on reset.
> > + */
> > + if (!ice->hwkm_init_complete)
> > + return 0;
> > + }
> > +
> > + return qcom_scm_ice_invalidate_key(hwkm_slot);
> > }
> > EXPORT_SYMBOL_GPL(qcom_ice_evict_key);
> >
> > @@ -334,6 +417,15 @@ bool qcom_ice_hwkm_supported(struct qcom_ice
> *ice)
> > }
> > EXPORT_SYMBOL_GPL(qcom_ice_hwkm_supported);
> >
> > +int qcom_ice_derive_sw_secret(struct qcom_ice *ice, const u8 wkey[],
> > + unsigned int wkey_size,
> > + u8 sw_secret[BLK_CRYPTO_SW_SECRET_SIZE])
> > +{
> > + return qcom_scm_derive_sw_secret(wkey, wkey_size,
> > + sw_secret,
> BLK_CRYPTO_SW_SECRET_SIZE); }
> > +EXPORT_SYMBOL_GPL(qcom_ice_derive_sw_secret);
> > +
> > static struct qcom_ice *qcom_ice_create(struct device *dev,
> > void __iomem *base)
> > {
> > diff --git a/include/soc/qcom/ice.h b/include/soc/qcom/ice.h index
> > 1f52e82e3e1c..dabe0d3a1fd0 100644
> > --- a/include/soc/qcom/ice.h
> > +++ b/include/soc/qcom/ice.h
> > @@ -17,6 +17,7 @@ enum qcom_ice_crypto_key_size {
> > QCOM_ICE_CRYPTO_KEY_SIZE_192 = 0x2,
> > QCOM_ICE_CRYPTO_KEY_SIZE_256 = 0x3,
> > QCOM_ICE_CRYPTO_KEY_SIZE_512 = 0x4,
> > + QCOM_ICE_CRYPTO_KEY_SIZE_WRAPPED = 0x5,
> > };
> >
> > enum qcom_ice_crypto_alg {
> > @@ -35,5 +36,8 @@ int qcom_ice_program_key(struct qcom_ice *ice,
> > u8 data_unit_size, int slot);
> > int qcom_ice_evict_key(struct qcom_ice *ice, int slot);
> > bool qcom_ice_hwkm_supported(struct qcom_ice *ice);
> > +int qcom_ice_derive_sw_secret(struct qcom_ice *ice, const u8 wkey[],
> > + unsigned int wkey_size,
> > + u8 sw_secret[BLK_CRYPTO_SW_SECRET_SIZE]);
> > struct qcom_ice *of_qcom_ice_get(struct device *dev);
> > #endif /* __QCOM_ICE_H__ */
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v3 05/12] ufs: core: support wrapped keys in ufs core
2023-11-22 5:38 [PATCH v3 00/12] Hardware wrapped key support for qcom ice and ufs Gaurav Kashyap
` (3 preceding siblings ...)
2023-11-22 5:38 ` [PATCH v3 04/12] soc: qcom: ice: support for hardware wrapped keys Gaurav Kashyap
@ 2023-11-22 5:38 ` Gaurav Kashyap
2023-12-08 3:42 ` Bjorn Andersson
2023-11-22 5:38 ` [PATCH v3 06/12] ufs: host: wrapped keys support in ufs qcom Gaurav Kashyap
` (8 subsequent siblings)
13 siblings, 1 reply; 39+ messages in thread
From: Gaurav Kashyap @ 2023-11-22 5:38 UTC (permalink / raw)
To: linux-scsi, linux-arm-msm, ebiggers, neil.armstrong, srinivas.kandagatla
Cc: linux-mmc, linux-block, linux-fscrypt, omprsing, quic_psodagud,
abel.vesa, quic_spuppala, kernel, Gaurav Kashyap
1. Add a new quirk in UFS when wrapped keys are supported. Since
wrapped keys are not part of the UFS spec, treat it as a
supported quirk.
2. Add derive_sw_secret crypto profile op implementation in ufshcd
crypto.
Signed-off-by: Gaurav Kashyap <quic_gaurkash@quicinc.com>
---
drivers/ufs/core/ufshcd-crypto.c | 39 +++++++++++++++++++++++++-------
include/ufs/ufshcd.h | 10 ++++++++
2 files changed, 41 insertions(+), 8 deletions(-)
diff --git a/drivers/ufs/core/ufshcd-crypto.c b/drivers/ufs/core/ufshcd-crypto.c
index 34537cbac622..3edbca87c322 100644
--- a/drivers/ufs/core/ufshcd-crypto.c
+++ b/drivers/ufs/core/ufshcd-crypto.c
@@ -81,13 +81,15 @@ static int ufshcd_crypto_keyslot_program(struct blk_crypto_profile *profile,
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);
+ if (key->crypto_cfg.key_type != BLK_CRYPTO_KEY_TYPE_HW_WRAPPED) {
+ 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);
+ }
}
err = ufshcd_program_key(hba, key, &cfg, slot);
@@ -127,9 +129,24 @@ bool ufshcd_crypto_enable(struct ufs_hba *hba)
return true;
}
+static int ufshcd_crypto_derive_sw_secret(struct blk_crypto_profile *profile,
+ const u8 wkey[], size_t wkey_size,
+ u8 sw_secret[BLK_CRYPTO_SW_SECRET_SIZE])
+{
+ struct ufs_hba *hba =
+ container_of(profile, struct ufs_hba, crypto_profile);
+
+ if (hba->vops && hba->vops->derive_sw_secret)
+ return hba->vops->derive_sw_secret(hba, wkey, wkey_size,
+ sw_secret);
+
+ return -EOPNOTSUPP;
+}
+
static const struct blk_crypto_ll_ops ufshcd_crypto_ops = {
.keyslot_program = ufshcd_crypto_keyslot_program,
.keyslot_evict = ufshcd_crypto_keyslot_evict,
+ .derive_sw_secret = ufshcd_crypto_derive_sw_secret,
};
static enum blk_crypto_mode_num
@@ -191,7 +208,13 @@ int ufshcd_hba_init_crypto_capabilities(struct ufs_hba *hba)
hba->crypto_profile.ll_ops = ufshcd_crypto_ops;
/* UFS only supports 8 bytes for any DUN */
hba->crypto_profile.max_dun_bytes_supported = 8;
- hba->crypto_profile.key_types_supported = BLK_CRYPTO_KEY_TYPE_STANDARD;
+ if (hba->quirks & UFSHCD_QUIRK_USES_WRAPPED_CRYPTO_KEYS)
+ hba->crypto_profile.key_types_supported =
+ BLK_CRYPTO_KEY_TYPE_HW_WRAPPED;
+ else
+ hba->crypto_profile.key_types_supported =
+ BLK_CRYPTO_KEY_TYPE_STANDARD;
+
hba->crypto_profile.dev = hba->dev;
/*
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index e1184ccc0508..86677788b5bd 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -320,6 +320,7 @@ struct ufs_pwr_mode_info {
* @device_reset: called to issue a reset pulse on the UFS device
* @config_scaling_param: called to configure clock scaling parameters
* @program_key: program or evict an inline encryption key
+ * @derive_sw_secret: derive sw secret from a wrapped key
* @event_notify: called to notify important events
* @reinit_notify: called to notify reinit of UFSHCD during max gear switch
* @mcq_config_resource: called to configure MCQ platform resources
@@ -364,6 +365,9 @@ struct ufs_hba_variant_ops {
int (*program_key)(struct ufs_hba *hba,
const struct blk_crypto_key *bkey,
const union ufs_crypto_cfg_entry *cfg, int slot);
+ int (*derive_sw_secret)(struct ufs_hba *hba, const u8 wkey[],
+ unsigned int wkey_size,
+ u8 sw_secret[BLK_CRYPTO_SW_SECRET_SIZE]);
void (*event_notify)(struct ufs_hba *hba,
enum ufs_event_type evt, void *data);
void (*reinit_notify)(struct ufs_hba *);
@@ -643,6 +647,12 @@ enum ufshcd_quirks {
* thus need this quirk to skip related flow.
*/
UFSHCD_QUIRK_MCQ_BROKEN_RTC = 1 << 21,
+
+ /*
+ * This quirk indicates that UFS will be using HW wrapped keys
+ * when using inline encryption.
+ */
+ UFSHCD_QUIRK_USES_WRAPPED_CRYPTO_KEYS = 1 << 22,
};
enum ufshcd_caps {
--
2.25.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [PATCH v3 05/12] ufs: core: support wrapped keys in ufs core
2023-11-22 5:38 ` [PATCH v3 05/12] ufs: core: support wrapped keys in ufs core Gaurav Kashyap
@ 2023-12-08 3:42 ` Bjorn Andersson
0 siblings, 0 replies; 39+ messages in thread
From: Bjorn Andersson @ 2023-12-08 3:42 UTC (permalink / raw)
To: Gaurav Kashyap
Cc: linux-scsi, linux-arm-msm, ebiggers, neil.armstrong,
srinivas.kandagatla, linux-mmc, linux-block, linux-fscrypt,
omprsing, quic_psodagud, abel.vesa, quic_spuppala, kernel
On Tue, Nov 21, 2023 at 09:38:10PM -0800, Gaurav Kashyap wrote:
> 1. Add a new quirk in UFS when wrapped keys are supported. Since
> wrapped keys are not part of the UFS spec, treat it as a
> supported quirk.
> 2. Add derive_sw_secret crypto profile op implementation in ufshcd
> crypto.
Please read:
https://docs.kernel.org/process/submitting-patches.html#separate-your-changes
https://docs.kernel.org/process/submitting-patches.html#describe-your-changes
Thank you,
Bjorn
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v3 06/12] ufs: host: wrapped keys support in ufs qcom
2023-11-22 5:38 [PATCH v3 00/12] Hardware wrapped key support for qcom ice and ufs Gaurav Kashyap
` (4 preceding siblings ...)
2023-11-22 5:38 ` [PATCH v3 05/12] ufs: core: support wrapped keys in ufs core Gaurav Kashyap
@ 2023-11-22 5:38 ` Gaurav Kashyap
2023-12-08 7:54 ` Om Prakash Singh
2023-11-22 5:38 ` [PATCH v3 07/12] qcom_scm: scm call for create, prepare and import keys Gaurav Kashyap
` (7 subsequent siblings)
13 siblings, 1 reply; 39+ messages in thread
From: Gaurav Kashyap @ 2023-11-22 5:38 UTC (permalink / raw)
To: linux-scsi, linux-arm-msm, ebiggers, neil.armstrong, srinivas.kandagatla
Cc: linux-mmc, linux-block, linux-fscrypt, omprsing, quic_psodagud,
abel.vesa, quic_spuppala, kernel, Gaurav Kashyap
1. Implement derive software secret defined in ufs core.
2. Use the wrapped keys quirk when hwkm is supported. The assumption
here is that if Qualcomm ICE supports HWKM, then all ICE keys
will be treated as hardware wrapped keys.
Signed-off-by: Gaurav Kashyap <quic_gaurkash@quicinc.com>
---
drivers/ufs/host/ufs-qcom.c | 23 ++++++++++++++++++++++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
index 7bec2b99b1df..6b09e09b1a30 100644
--- a/drivers/ufs/host/ufs-qcom.c
+++ b/drivers/ufs/host/ufs-qcom.c
@@ -125,6 +125,8 @@ static int ufs_qcom_ice_init(struct ufs_qcom_host *host)
host->ice = ice;
hba->caps |= UFSHCD_CAP_CRYPTO;
+ if (qcom_ice_hwkm_supported(host->ice))
+ hba->quirks |= UFSHCD_QUIRK_USES_WRAPPED_CRYPTO_KEYS;
return 0;
}
@@ -162,7 +164,11 @@ static int ufs_qcom_ice_program_key(struct ufs_hba *hba,
cap.key_size != UFS_CRYPTO_KEY_SIZE_256)
return -EINVAL;
- ice_key_size = QCOM_ICE_CRYPTO_KEY_SIZE_256;
+ if (bkey->crypto_cfg.key_type == BLK_CRYPTO_KEY_TYPE_HW_WRAPPED)
+ ice_key_size = QCOM_ICE_CRYPTO_KEY_SIZE_WRAPPED;
+ else
+ ice_key_size = QCOM_ICE_CRYPTO_KEY_SIZE_256;
+
if (config_enable)
return qcom_ice_program_key(host->ice,
QCOM_ICE_CRYPTO_ALG_AES_XTS,
@@ -172,9 +178,23 @@ static int ufs_qcom_ice_program_key(struct ufs_hba *hba,
return qcom_ice_evict_key(host->ice, slot);
}
+/*
+ * Derive a software secret from a hardware wrapped key. The key is unwrapped in
+ * hardware from trustzone and a software key/secret is then derived from it.
+ */
+int ufs_qcom_ice_derive_sw_secret(struct ufs_hba *hba, const u8 wkey[],
+ unsigned int wkey_size,
+ u8 sw_secret[BLK_CRYPTO_SW_SECRET_SIZE])
+{
+ struct ufs_qcom_host *host = ufshcd_get_variant(hba);
+
+ return qcom_ice_derive_sw_secret(host->ice, wkey, wkey_size, sw_secret);
+}
+
#else
#define ufs_qcom_ice_program_key NULL
+#define ufs_qcom_ice_derive_sw_secret NULL
static inline void ufs_qcom_ice_enable(struct ufs_qcom_host *host)
{
@@ -1996,6 +2016,7 @@ static const struct ufs_hba_variant_ops ufs_hba_qcom_vops = {
.device_reset = ufs_qcom_device_reset,
.config_scaling_param = ufs_qcom_config_scaling_param,
.program_key = ufs_qcom_ice_program_key,
+ .derive_sw_secret = ufs_qcom_ice_derive_sw_secret,
.reinit_notify = ufs_qcom_reinit_notify,
.mcq_config_resource = ufs_qcom_mcq_config_resource,
.get_hba_mac = ufs_qcom_get_hba_mac,
--
2.25.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [PATCH v3 06/12] ufs: host: wrapped keys support in ufs qcom
2023-11-22 5:38 ` [PATCH v3 06/12] ufs: host: wrapped keys support in ufs qcom Gaurav Kashyap
@ 2023-12-08 7:54 ` Om Prakash Singh
0 siblings, 0 replies; 39+ messages in thread
From: Om Prakash Singh @ 2023-12-08 7:54 UTC (permalink / raw)
To: Gaurav Kashyap, linux-scsi, linux-arm-msm, ebiggers,
neil.armstrong, srinivas.kandagatla
Cc: linux-mmc, linux-block, linux-fscrypt, omprsing, quic_psodagud,
abel.vesa, quic_spuppala, kernel
On 11/22/2023 11:08 AM, Gaurav Kashyap wrote:
> 1. Implement derive software secret defined in ufs core. 2. Use the
> wrapped keys quirk when hwkm is supported. The assumption here is that
> if Qualcomm ICE supports HWKM, then all ICE keys will be treated as
> hardware wrapped keys.
As per convention need to submit this in two separate patch
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v3 07/12] qcom_scm: scm call for create, prepare and import keys
2023-11-22 5:38 [PATCH v3 00/12] Hardware wrapped key support for qcom ice and ufs Gaurav Kashyap
` (5 preceding siblings ...)
2023-11-22 5:38 ` [PATCH v3 06/12] ufs: host: wrapped keys support in ufs qcom Gaurav Kashyap
@ 2023-11-22 5:38 ` Gaurav Kashyap
2023-12-13 8:11 ` Mukesh Ojha
2023-11-22 5:38 ` [PATCH v3 08/12] ufs: core: add support for generate, import and prepare keys Gaurav Kashyap
` (6 subsequent siblings)
13 siblings, 1 reply; 39+ messages in thread
From: Gaurav Kashyap @ 2023-11-22 5:38 UTC (permalink / raw)
To: linux-scsi, linux-arm-msm, ebiggers, neil.armstrong, srinivas.kandagatla
Cc: linux-mmc, linux-block, linux-fscrypt, omprsing, quic_psodagud,
abel.vesa, quic_spuppala, kernel, Gaurav Kashyap
Storage encryption has two IOCTLs for creating, importing
and preparing keys for encryption. For wrapped keys, these
IOCTLs need to interface with the secure environment, which
require these SCM calls.
generate_key: This is used to generate and return a longterm
wrapped key. Trustzone achieves this by generating
a key and then wrapping it using hwkm, returning
a wrapped keyblob.
import_key: The functionality is similar to generate, but here,
a raw key is imported into hwkm and a longterm wrapped
keyblob is returned.
prepare_key: The longterm wrapped key from import or generate
is made further secure by rewrapping it with a per-boot
ephemeral wrapped key before installing it to the linux
kernel for programming to ICE.
Signed-off-by: Gaurav Kashyap <quic_gaurkash@quicinc.com>
---
drivers/firmware/qcom/qcom_scm.c | 205 +++++++++++++++++++++++++
drivers/firmware/qcom/qcom_scm.h | 3 +
include/linux/firmware/qcom/qcom_scm.h | 5 +
3 files changed, 213 insertions(+)
diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
index 6dfb913f3e33..259b3c316019 100644
--- a/drivers/firmware/qcom/qcom_scm.c
+++ b/drivers/firmware/qcom/qcom_scm.c
@@ -1285,6 +1285,211 @@ int qcom_scm_derive_sw_secret(const u8 *wkey, size_t wkey_size,
}
EXPORT_SYMBOL(qcom_scm_derive_sw_secret);
+/**
+ * qcom_scm_generate_ice_key() - Generate a wrapped key for encryption.
+ * @lt_key: the wrapped key returned after key generation
+ * @lt_key_size: size of the wrapped key to be returned.
+ *
+ * Qualcomm wrapped keys need to be generated in a trusted environment.
+ * A generate key IOCTL call is used to achieve this. These are longterm
+ * in nature as they need to be generated and wrapped only once per
+ * requirement.
+ *
+ * This SCM calls adds support for the create key IOCTL to interface
+ * with the secure environment to generate and return a wrapped key..
+ *
+ * Return: 0 on success; -errno on failure.
+ */
+int qcom_scm_generate_ice_key(u8 *lt_key, size_t lt_key_size)
+{
+ struct qcom_scm_desc desc = {
+ .svc = QCOM_SCM_SVC_ES,
+ .cmd = QCOM_SCM_ES_GENERATE_ICE_KEY,
+ .arginfo = QCOM_SCM_ARGS(2, QCOM_SCM_RW,
+ QCOM_SCM_VAL),
+ .args[1] = lt_key_size,
+ .owner = ARM_SMCCC_OWNER_SIP,
+ };
+
+ void *lt_key_buf;
+ dma_addr_t lt_key_phys;
+ int ret;
+
+ /*
+ * Like qcom_scm_ice_set_key(), we use dma_alloc_coherent() to properly
+ * get a physical address, while guaranteeing that we can zeroize the
+ * key material later using memzero_explicit().
+ *
+ */
+ lt_key_buf = dma_alloc_coherent(__scm->dev, lt_key_size, <_key_phys, GFP_KERNEL);
+ if (!lt_key_buf)
+ return -ENOMEM;
+
+ desc.args[0] = lt_key_phys;
+
+ ret = qcom_scm_call(__scm->dev, &desc, NULL);
+ memcpy(lt_key, lt_key_buf, lt_key_size);
+
+ memzero_explicit(lt_key_buf, lt_key_size);
+
+ dma_free_coherent(__scm->dev, lt_key_size, lt_key_buf, lt_key_phys);
+
+ if (!ret)
+ return lt_key_size;
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(qcom_scm_generate_ice_key);
+
+/**
+ * qcom_scm_prepare_ice_key() - Get per boot ephemeral wrapped key
+ * @lt_key: the longterm wrapped key
+ * @lt_key_size: size of the wrapped key
+ * @eph_key: ephemeral wrapped key to be returned
+ * @eph_key_size: size of the ephemeral wrapped key
+ *
+ * Qualcomm wrapped keys (longterm keys) are rewrapped with a per-boot
+ * ephemeral key for added protection. These are ephemeral in nature as
+ * they are valid only for that boot. A create key IOCTL is used to
+ * achieve this. These are the keys that are installed into the kernel
+ * to be then unwrapped and programmed into ICE.
+ *
+ * This SCM call adds support for the create key IOCTL to interface
+ * with the secure environment to rewrap the wrapped key with an
+ * ephemeral wrapping key.
+ *
+ * Return: 0 on success; -errno on failure.
+ */
+int qcom_scm_prepare_ice_key(const u8 *lt_key, size_t lt_key_size,
+ u8 *eph_key, size_t eph_key_size)
+{
+ struct qcom_scm_desc desc = {
+ .svc = QCOM_SCM_SVC_ES,
+ .cmd = QCOM_SCM_ES_PREPARE_ICE_KEY,
+ .arginfo = QCOM_SCM_ARGS(4, QCOM_SCM_RO,
+ QCOM_SCM_VAL, QCOM_SCM_RW,
+ QCOM_SCM_VAL),
+ .args[1] = lt_key_size,
+ .args[3] = eph_key_size,
+ .owner = ARM_SMCCC_OWNER_SIP,
+ };
+
+ void *lt_key_buf, *eph_key_buf;
+ dma_addr_t lt_key_phys, eph_key_phys;
+ int ret;
+
+ /*
+ * Like qcom_scm_ice_set_key(), we use dma_alloc_coherent() to properly
+ * get a physical address, while guaranteeing that we can zeroize the
+ * key material later using memzero_explicit().
+ *
+ */
+ lt_key_buf = dma_alloc_coherent(__scm->dev, lt_key_size, <_key_phys, GFP_KERNEL);
+ if (!lt_key_buf)
+ return -ENOMEM;
+ eph_key_buf = dma_alloc_coherent(__scm->dev, eph_key_size, &eph_key_phys, GFP_KERNEL);
+ if (!eph_key_buf) {
+ ret = -ENOMEM;
+ goto err_free_longterm;
+ }
+
+ memcpy(lt_key_buf, lt_key, lt_key_size);
+ desc.args[0] = lt_key_phys;
+ desc.args[2] = eph_key_phys;
+
+ ret = qcom_scm_call(__scm->dev, &desc, NULL);
+ if (!ret)
+ memcpy(eph_key, eph_key_buf, eph_key_size);
+
+ memzero_explicit(eph_key_buf, eph_key_size);
+
+ dma_free_coherent(__scm->dev, eph_key_size, eph_key_buf, eph_key_phys);
+
+err_free_longterm:
+ memzero_explicit(lt_key_buf, lt_key_size);
+
+ dma_free_coherent(__scm->dev, lt_key_size, lt_key_buf, lt_key_phys);
+
+ if (!ret)
+ return eph_key_size;
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(qcom_scm_prepare_ice_key);
+
+/**
+ * qcom_scm_import_ice_key() - Import a wrapped key for encryption
+ * @imp_key: the raw key that is imported
+ * @imp_key_size: size of the key to be imported
+ * @lt_key: the wrapped key to be returned
+ * @lt_key_size: size of the wrapped key
+ *
+ * Conceptually, this is very similar to generate, the difference being,
+ * here we want to import a raw key and return a longterm wrapped key
+ * from it. The same create key IOCTL is used to achieve this.
+ *
+ * This SCM call adds support for the create key IOCTL to interface with
+ * the secure environment to import a raw key and generate a longterm
+ * wrapped key.
+ *
+ * Return: 0 on success; -errno on failure.
+ */
+int qcom_scm_import_ice_key(const u8 *imp_key, size_t imp_key_size,
+ u8 *lt_key, size_t lt_key_size)
+{
+ struct qcom_scm_desc desc = {
+ .svc = QCOM_SCM_SVC_ES,
+ .cmd = QCOM_SCM_ES_IMPORT_ICE_KEY,
+ .arginfo = QCOM_SCM_ARGS(4, QCOM_SCM_RO,
+ QCOM_SCM_VAL, QCOM_SCM_RW,
+ QCOM_SCM_VAL),
+ .args[1] = imp_key_size,
+ .args[3] = lt_key_size,
+ .owner = ARM_SMCCC_OWNER_SIP,
+ };
+
+ void *imp_key_buf, *lt_key_buf;
+ dma_addr_t imp_key_phys, lt_key_phys;
+ int ret;
+ /*
+ * Like qcom_scm_ice_set_key(), we use dma_alloc_coherent() to properly
+ * get a physical address, while guaranteeing that we can zeroize the
+ * key material later using memzero_explicit().
+ *
+ */
+ imp_key_buf = dma_alloc_coherent(__scm->dev, imp_key_size, &imp_key_phys, GFP_KERNEL);
+ if (!imp_key_buf)
+ return -ENOMEM;
+ lt_key_buf = dma_alloc_coherent(__scm->dev, lt_key_size, <_key_phys, GFP_KERNEL);
+ if (!lt_key_buf) {
+ ret = -ENOMEM;
+ goto err_free_longterm;
+ }
+
+ memcpy(imp_key_buf, imp_key, imp_key_size);
+ desc.args[0] = imp_key_phys;
+ desc.args[2] = lt_key_phys;
+
+ ret = qcom_scm_call(__scm->dev, &desc, NULL);
+ if (!ret)
+ memcpy(lt_key, lt_key_buf, lt_key_size);
+
+ memzero_explicit(lt_key_buf, lt_key_size);
+
+ dma_free_coherent(__scm->dev, lt_key_size, lt_key_buf, lt_key_phys);
+
+err_free_longterm:
+ memzero_explicit(imp_key_buf, imp_key_size);
+
+ dma_free_coherent(__scm->dev, imp_key_size, imp_key_buf, imp_key_phys);
+
+ if (!ret)
+ return lt_key_size;
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(qcom_scm_import_ice_key);
+
/**
* qcom_scm_hdcp_available() - Check if secure environment supports HDCP.
*
diff --git a/drivers/firmware/qcom/qcom_scm.h b/drivers/firmware/qcom/qcom_scm.h
index c75456aa6ac5..d89ab5446ba5 100644
--- a/drivers/firmware/qcom/qcom_scm.h
+++ b/drivers/firmware/qcom/qcom_scm.h
@@ -122,6 +122,9 @@ int scm_legacy_call(struct device *dev, const struct qcom_scm_desc *desc,
#define QCOM_SCM_ES_INVALIDATE_ICE_KEY 0x03
#define QCOM_SCM_ES_CONFIG_SET_ICE_KEY 0x04
#define QCOM_SCM_ES_DERIVE_SW_SECRET 0x07
+#define QCOM_SCM_ES_GENERATE_ICE_KEY 0x08
+#define QCOM_SCM_ES_PREPARE_ICE_KEY 0x09
+#define QCOM_SCM_ES_IMPORT_ICE_KEY 0xA
#define QCOM_SCM_SVC_HDCP 0x11
#define QCOM_SCM_HDCP_INVOKE 0x01
diff --git a/include/linux/firmware/qcom/qcom_scm.h b/include/linux/firmware/qcom/qcom_scm.h
index c65f2d61492d..477aeec6255e 100644
--- a/include/linux/firmware/qcom/qcom_scm.h
+++ b/include/linux/firmware/qcom/qcom_scm.h
@@ -105,6 +105,11 @@ int qcom_scm_ice_set_key(u32 index, const u8 *key, u32 key_size,
enum qcom_scm_ice_cipher cipher, u32 data_unit_size);
int qcom_scm_derive_sw_secret(const u8 *wkey, size_t wkey_size,
u8 *sw_secret, size_t sw_secret_size);
+int qcom_scm_generate_ice_key(u8 *lt_key, size_t lt_key_size);
+int qcom_scm_prepare_ice_key(const u8 *lt_key, size_t lt_key_size,
+ u8 *eph_key, size_t eph_size);
+int qcom_scm_import_ice_key(const u8 *imp_key, size_t imp_size,
+ u8 *lt_key, size_t lt_key_size);
bool qcom_scm_hdcp_available(void);
int qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt, u32 *resp);
--
2.25.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [PATCH v3 07/12] qcom_scm: scm call for create, prepare and import keys
2023-11-22 5:38 ` [PATCH v3 07/12] qcom_scm: scm call for create, prepare and import keys Gaurav Kashyap
@ 2023-12-13 8:11 ` Mukesh Ojha
0 siblings, 0 replies; 39+ messages in thread
From: Mukesh Ojha @ 2023-12-13 8:11 UTC (permalink / raw)
To: Gaurav Kashyap, linux-scsi, linux-arm-msm, ebiggers,
neil.armstrong, srinivas.kandagatla
Cc: linux-mmc, linux-block, linux-fscrypt, omprsing, quic_psodagud,
abel.vesa, quic_spuppala, kernel
On 11/22/2023 11:08 AM, Gaurav Kashyap wrote:
> Storage encryption has two IOCTLs for creating, importing
> and preparing keys for encryption. For wrapped keys, these
> IOCTLs need to interface with the secure environment, which
> require these SCM calls.
>
> generate_key: This is used to generate and return a longterm
> wrapped key. Trustzone achieves this by generating
> a key and then wrapping it using hwkm, returning
> a wrapped keyblob.
> import_key: The functionality is similar to generate, but here,
> a raw key is imported into hwkm and a longterm wrapped
> keyblob is returned.
> prepare_key: The longterm wrapped key from import or generate
> is made further secure by rewrapping it with a per-boot
> ephemeral wrapped key before installing it to the linux
> kernel for programming to ICE.
>
> Signed-off-by: Gaurav Kashyap <quic_gaurkash@quicinc.com>
[..]
> ---
> drivers/firmware/qcom/qcom_scm.c | 205 +++++++++++++++++++++++++
> drivers/firmware/qcom/qcom_scm.h | 3 +
> include/linux/firmware/qcom/qcom_scm.h | 5 +
> 3 files changed, 213 insertions(+)
>
> diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
> index 6dfb913f3e33..259b3c316019 100644
> --- a/drivers/firmware/qcom/qcom_scm.c
> +++ b/drivers/firmware/qcom/qcom_scm.c
> @@ -1285,6 +1285,211 @@ int qcom_scm_derive_sw_secret(const u8 *wkey, size_t wkey_size,
> }
> EXPORT_SYMBOL(qcom_scm_derive_sw_secret);
>
> +/**
> + * qcom_scm_generate_ice_key() - Generate a wrapped key for encryption.
> + * @lt_key: the wrapped key returned after key generation
> + * @lt_key_size: size of the wrapped key to be returned.
> + *
> + * Qualcomm wrapped keys need to be generated in a trusted environment.
> + * A generate key IOCTL call is used to achieve this. These are longterm
nit: remove space after key
> + * in nature as they need to be generated and wrapped only once per
> + * requirement.
> + *
> + * This SCM calls adds support for the create key IOCTL to interface
Just starting with "Adds support... " would do.
> + * with the secure environment to generate and return a wrapped key..
> + *
> + * Return: 0 on success; -errno on failure.
> + */
> +int qcom_scm_generate_ice_key(u8 *lt_key, size_t lt_key_size)
> +{
> + struct qcom_scm_desc desc = {
> + .svc = QCOM_SCM_SVC_ES,
> + .cmd = QCOM_SCM_ES_GENERATE_ICE_KEY,
> + .arginfo = QCOM_SCM_ARGS(2, QCOM_SCM_RW,
> + QCOM_SCM_VAL),
Keep this in one line.
> + .args[1] = lt_key_size,
> + .owner = ARM_SMCCC_OWNER_SIP,
> + };
> +
> + void *lt_key_buf;
> + dma_addr_t lt_key_phys; > + int ret;
reverse x-mas tree is the everyone looks to be following..
dma_addr_t lt_key_phys;
void *lt_key_buf;
int ret;
> +
> + /*
> + * Like qcom_scm_ice_set_key(), we use dma_alloc_coherent() to properly
> + * get a physical address, while guaranteeing that we can zeroize the
> + * key material later using memzero_explicit().
> + *
Extra *
> + */
> + lt_key_buf = dma_alloc_coherent(__scm->dev, lt_key_size, <_key_phys, GFP_KERNEL);
> + if (!lt_key_buf)
> + return -ENOMEM;
> +
> + desc.args[0] = lt_key_phys;
> +
> + ret = qcom_scm_call(__scm->dev, &desc, NULL);
> + memcpy(lt_key, lt_key_buf, lt_key_size);
Do you really want to copy the key buf if the scm call fails ?
> +
> + memzero_explicit(lt_key_buf, lt_key_size);
> +
> + dma_free_coherent(__scm->dev, lt_key_size, lt_key_buf, lt_key_phys);
> +
> + if (!ret)
why this is here instead of just after qcom_scm_call() ?
> + return lt_key_size;
You said, you will return 0 on success.
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(qcom_scm_generate_ice_key);
> +
> +/**
> + * qcom_scm_prepare_ice_key() - Get per boot ephemeral wrapped key
> + * @lt_key: the longterm wrapped key
> + * @lt_key_size: size of the wrapped key
> + * @eph_key: ephemeral wrapped key to be returned
> + * @eph_key_size: size of the ephemeral wrapped key
> + *
> + * Qualcomm wrapped keys (longterm keys) are rewrapped with a per-boot
> + * ephemeral key for added protection. These are ephemeral in nature as
> + * they are valid only for that boot. A create key IOCTL is used to
> + * achieve this. These are the keys that are installed into the kernel
> + * to be then unwrapped and programmed into ICE.
> + *
> + * This SCM call adds support for the create key IOCTL to interface
> + * with the secure environment to rewrap the wrapped key with an
> + * ephemeral wrapping key.
> + *
> + * Return: 0 on success; -errno on failure.
> + */
> +int qcom_scm_prepare_ice_key(const u8 *lt_key, size_t lt_key_size,
> + u8 *eph_key, size_t eph_key_size)
> +{
> + struct qcom_scm_desc desc = {
> + .svc = QCOM_SCM_SVC_ES,
> + .cmd = QCOM_SCM_ES_PREPARE_ICE_KEY,
> + .arginfo = QCOM_SCM_ARGS(4, QCOM_SCM_RO,
> + QCOM_SCM_VAL, QCOM_SCM_RW,
> + QCOM_SCM_VAL),
> + .args[1] = lt_key_size,
> + .args[3] = eph_key_size,
> + .owner = ARM_SMCCC_OWNER_SIP,
> + };
> +
> + void *lt_key_buf, *eph_key_buf;
> + dma_addr_t lt_key_phys, eph_key_phys;
> + int ret;
One variable and R-XMAS is the norm..
> +
> + /*
> + * Like qcom_scm_ice_set_key(), we use dma_alloc_coherent() to properly
> + * get a physical address, while guaranteeing that we can zeroize the
> + * key material later using memzero_explicit().
> + *
extra *
> + */
> + lt_key_buf = dma_alloc_coherent(__scm->dev, lt_key_size, <_key_phys, GFP_KERNEL);
> + if (!lt_key_buf)
> + return -ENOMEM;
> + eph_key_buf = dma_alloc_coherent(__scm->dev, eph_key_size, &eph_key_phys, GFP_KERNEL);
> + if (!eph_key_buf) {
> + ret = -ENOMEM;
> + goto err_free_longterm;
> + }
> +
> + memcpy(lt_key_buf, lt_key, lt_key_size);
> + desc.args[0] = lt_key_phys;
> + desc.args[2] = eph_key_phys;
> +
> + ret = qcom_scm_call(__scm->dev, &desc, NULL);
> + if (!ret)
> + memcpy(eph_key, eph_key_buf, eph_key_size);
> +
> + memzero_explicit(eph_key_buf, eph_key_size);
> +
> + dma_free_coherent(__scm->dev, eph_key_size, eph_key_buf, eph_key_phys);
> +
> +err_free_longterm:
> + memzero_explicit(lt_key_buf, lt_key_size);
> +
> + dma_free_coherent(__scm->dev, lt_key_size, lt_key_buf, lt_key_phys);
> +
> + if (!ret)
> + return eph_key_size;
you said, this will return 0 on success..
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(qcom_scm_prepare_ice_key);
> +
> +/**
> + * qcom_scm_import_ice_key() - Import a wrapped key for encryption
> + * @imp_key: the raw key that is imported
> + * @imp_key_size: size of the key to be imported
> + * @lt_key: the wrapped key to be returned
> + * @lt_key_size: size of the wrapped key
> + *
> + * Conceptually, this is very similar to generate, the difference being,
> + * here we want to import a raw key and return a longterm wrapped key
> + * from it. The same create key IOCTL is used to achieve this.
> + *
> + * This SCM call adds support for the create key IOCTL to interface with
> + * the secure environment to import a raw key and generate a longterm
> + * wrapped key.
> + *
> + * Return: 0 on success; -errno on failure.
> + */
> +int qcom_scm_import_ice_key(const u8 *imp_key, size_t imp_key_size,
> + u8 *lt_key, size_t lt_key_size)
> +{
> + struct qcom_scm_desc desc = {
> + .svc = QCOM_SCM_SVC_ES,
> + .cmd = QCOM_SCM_ES_IMPORT_ICE_KEY,
> + .arginfo = QCOM_SCM_ARGS(4, QCOM_SCM_RO,
> + QCOM_SCM_VAL, QCOM_SCM_RW,
> + QCOM_SCM_VAL),
> + .args[1] = imp_key_size,
> + .args[3] = lt_key_size,
> + .owner = ARM_SMCCC_OWNER_SIP,
> + };
> +
> + void *imp_key_buf, *lt_key_buf;
> + dma_addr_t imp_key_phys, lt_key_phys;
> + int ret;
> + /*
> + * Like qcom_scm_ice_set_key(), we use dma_alloc_coherent() to properly
> + * get a physical address, while guaranteeing that we can zeroize the
> + * key material later using memzero_explicit().
> + *
Extra *
> + */
> + imp_key_buf = dma_alloc_coherent(__scm->dev, imp_key_size, &imp_key_phys, GFP_KERNEL);
> + if (!imp_key_buf)
> + return -ENOMEM;
> + lt_key_buf = dma_alloc_coherent(__scm->dev, lt_key_size, <_key_phys, GFP_KERNEL);
> + if (!lt_key_buf) {
> + ret = -ENOMEM;
> + goto err_free_longterm;
> + }
> +
> + memcpy(imp_key_buf, imp_key, imp_key_size);
> + desc.args[0] = imp_key_phys;
> + desc.args[2] = lt_key_phys;
> +
> + ret = qcom_scm_call(__scm->dev, &desc, NULL);
> + if (!ret)
> + memcpy(lt_key, lt_key_buf, lt_key_size);
> +
> + memzero_explicit(lt_key_buf, lt_key_size);
> +
Why there is unnecessary line gap everywhere between lines ?
> + dma_free_coherent(__scm->dev, lt_key_size, lt_key_buf, lt_key_phys);
> +
> +err_free_longterm:
> + memzero_explicit(imp_key_buf, imp_key_size);
> +
> + dma_free_coherent(__scm->dev, imp_key_size, imp_key_buf, imp_key_phys);
> +
> + if (!ret)
> + return lt_key_size;
same as above comment on return value..
-Mukesh
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(qcom_scm_import_ice_key);
> +
> /**
> * qcom_scm_hdcp_available() - Check if secure environment supports HDCP.
> *
> diff --git a/drivers/firmware/qcom/qcom_scm.h b/drivers/firmware/qcom/qcom_scm.h
> index c75456aa6ac5..d89ab5446ba5 100644
> --- a/drivers/firmware/qcom/qcom_scm.h
> +++ b/drivers/firmware/qcom/qcom_scm.h
> @@ -122,6 +122,9 @@ int scm_legacy_call(struct device *dev, const struct qcom_scm_desc *desc,
> #define QCOM_SCM_ES_INVALIDATE_ICE_KEY 0x03
> #define QCOM_SCM_ES_CONFIG_SET_ICE_KEY 0x04
> #define QCOM_SCM_ES_DERIVE_SW_SECRET 0x07
> +#define QCOM_SCM_ES_GENERATE_ICE_KEY 0x08
> +#define QCOM_SCM_ES_PREPARE_ICE_KEY 0x09
> +#define QCOM_SCM_ES_IMPORT_ICE_KEY 0xA
>
> #define QCOM_SCM_SVC_HDCP 0x11
> #define QCOM_SCM_HDCP_INVOKE 0x01
> diff --git a/include/linux/firmware/qcom/qcom_scm.h b/include/linux/firmware/qcom/qcom_scm.h
> index c65f2d61492d..477aeec6255e 100644
> --- a/include/linux/firmware/qcom/qcom_scm.h
> +++ b/include/linux/firmware/qcom/qcom_scm.h
> @@ -105,6 +105,11 @@ int qcom_scm_ice_set_key(u32 index, const u8 *key, u32 key_size,
> enum qcom_scm_ice_cipher cipher, u32 data_unit_size);
> int qcom_scm_derive_sw_secret(const u8 *wkey, size_t wkey_size,
> u8 *sw_secret, size_t sw_secret_size);
> +int qcom_scm_generate_ice_key(u8 *lt_key, size_t lt_key_size);
> +int qcom_scm_prepare_ice_key(const u8 *lt_key, size_t lt_key_size,
> + u8 *eph_key, size_t eph_size);
> +int qcom_scm_import_ice_key(const u8 *imp_key, size_t imp_size,
> + u8 *lt_key, size_t lt_key_size);
>
> bool qcom_scm_hdcp_available(void);
> int qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt, u32 *resp);
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v3 08/12] ufs: core: add support for generate, import and prepare keys
2023-11-22 5:38 [PATCH v3 00/12] Hardware wrapped key support for qcom ice and ufs Gaurav Kashyap
` (6 preceding siblings ...)
2023-11-22 5:38 ` [PATCH v3 07/12] qcom_scm: scm call for create, prepare and import keys Gaurav Kashyap
@ 2023-11-22 5:38 ` Gaurav Kashyap
2023-12-08 3:49 ` Bjorn Andersson
2023-12-08 8:17 ` Om Prakash Singh
2023-11-22 5:38 ` [PATCH v3 09/12] soc: qcom: support for generate, import and prepare key Gaurav Kashyap
` (5 subsequent siblings)
13 siblings, 2 replies; 39+ messages in thread
From: Gaurav Kashyap @ 2023-11-22 5:38 UTC (permalink / raw)
To: linux-scsi, linux-arm-msm, ebiggers, neil.armstrong, srinivas.kandagatla
Cc: linux-mmc, linux-block, linux-fscrypt, omprsing, quic_psodagud,
abel.vesa, quic_spuppala, kernel, Gaurav Kashyap
This patch contains two changes in UFS for wrapped keys.
1. Implements the blk_crypto_profile ops for generate, import
and prepare key apis.
2. Defines UFS vops for generate, import and prepare keys so
that vendors can hook into them.
Signed-off-by: Gaurav Kashyap <quic_gaurkash@quicinc.com>
---
drivers/ufs/core/ufshcd-crypto.c | 41 ++++++++++++++++++++++++++++++++
include/ufs/ufshcd.h | 11 +++++++++
2 files changed, 52 insertions(+)
diff --git a/drivers/ufs/core/ufshcd-crypto.c b/drivers/ufs/core/ufshcd-crypto.c
index 3edbca87c322..cf34f4a9cda8 100644
--- a/drivers/ufs/core/ufshcd-crypto.c
+++ b/drivers/ufs/core/ufshcd-crypto.c
@@ -143,10 +143,51 @@ static int ufshcd_crypto_derive_sw_secret(struct blk_crypto_profile *profile,
return -EOPNOTSUPP;
}
+static int ufshcd_crypto_generate_key(struct blk_crypto_profile *profile,
+ u8 lt_key[BLK_CRYPTO_MAX_HW_WRAPPED_KEY_SIZE])
+{
+ struct ufs_hba *hba =
+ container_of(profile, struct ufs_hba, crypto_profile);
+
+ if (hba->vops && hba->vops->generate_key)
+ return hba->vops->generate_key(hba, lt_key);
+
+ return -EOPNOTSUPP;
+}
+
+static int ufshcd_crypto_prepare_key(struct blk_crypto_profile *profile,
+ const u8 *lt_key, size_t lt_key_size,
+ u8 eph_key[BLK_CRYPTO_MAX_HW_WRAPPED_KEY_SIZE])
+{
+ struct ufs_hba *hba =
+ container_of(profile, struct ufs_hba, crypto_profile);
+
+ if (hba->vops && hba->vops->prepare_key)
+ return hba->vops->prepare_key(hba, lt_key, lt_key_size, eph_key);
+
+ return -EOPNOTSUPP;
+}
+
+static int ufshcd_crypto_import_key(struct blk_crypto_profile *profile,
+ const u8 *imp_key, size_t imp_key_size,
+ u8 lt_key[BLK_CRYPTO_MAX_HW_WRAPPED_KEY_SIZE])
+{
+ struct ufs_hba *hba =
+ container_of(profile, struct ufs_hba, crypto_profile);
+
+ if (hba->vops && hba->vops->import_key)
+ return hba->vops->import_key(hba, imp_key, imp_key_size, lt_key);
+
+ return -EOPNOTSUPP;
+}
+
static const struct blk_crypto_ll_ops ufshcd_crypto_ops = {
.keyslot_program = ufshcd_crypto_keyslot_program,
.keyslot_evict = ufshcd_crypto_keyslot_evict,
.derive_sw_secret = ufshcd_crypto_derive_sw_secret,
+ .generate_key = ufshcd_crypto_generate_key,
+ .prepare_key = ufshcd_crypto_prepare_key,
+ .import_key = ufshcd_crypto_import_key,
};
static enum blk_crypto_mode_num
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index 86677788b5bd..49657a5d1e34 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -321,6 +321,9 @@ struct ufs_pwr_mode_info {
* @config_scaling_param: called to configure clock scaling parameters
* @program_key: program or evict an inline encryption key
* @derive_sw_secret: derive sw secret from a wrapped key
+ * @generate_key: generate a storage key and return longterm wrapped key
+ * @prepare_key: unwrap longterm key and return ephemeral wrapped key
+ * @import_key: import sw storage key and return longterm wrapped key
* @event_notify: called to notify important events
* @reinit_notify: called to notify reinit of UFSHCD during max gear switch
* @mcq_config_resource: called to configure MCQ platform resources
@@ -368,6 +371,14 @@ struct ufs_hba_variant_ops {
int (*derive_sw_secret)(struct ufs_hba *hba, const u8 wkey[],
unsigned int wkey_size,
u8 sw_secret[BLK_CRYPTO_SW_SECRET_SIZE]);
+ int (*generate_key)(struct ufs_hba *hba,
+ u8 lt_key[BLK_CRYPTO_MAX_HW_WRAPPED_KEY_SIZE]);
+ int (*prepare_key)(struct ufs_hba *hba,
+ const u8 *lt_key, size_t lt_key_size,
+ u8 eph_key[BLK_CRYPTO_MAX_HW_WRAPPED_KEY_SIZE]);
+ int (*import_key)(struct ufs_hba *hba,
+ const u8 *imp_key, size_t imp_key_size,
+ u8 lt_key[BLK_CRYPTO_MAX_HW_WRAPPED_KEY_SIZE]);
void (*event_notify)(struct ufs_hba *hba,
enum ufs_event_type evt, void *data);
void (*reinit_notify)(struct ufs_hba *);
--
2.25.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [PATCH v3 08/12] ufs: core: add support for generate, import and prepare keys
2023-11-22 5:38 ` [PATCH v3 08/12] ufs: core: add support for generate, import and prepare keys Gaurav Kashyap
@ 2023-12-08 3:49 ` Bjorn Andersson
2023-12-08 8:17 ` Om Prakash Singh
1 sibling, 0 replies; 39+ messages in thread
From: Bjorn Andersson @ 2023-12-08 3:49 UTC (permalink / raw)
To: Gaurav Kashyap
Cc: linux-scsi, linux-arm-msm, ebiggers, neil.armstrong,
srinivas.kandagatla, linux-mmc, linux-block, linux-fscrypt,
omprsing, quic_psodagud, abel.vesa, quic_spuppala, kernel
On Tue, Nov 21, 2023 at 09:38:13PM -0800, Gaurav Kashyap wrote:
> This patch contains two changes in UFS for wrapped keys.
The code in this patch isn't two different changes, it just add the new
variant_ops and tie these to the blk crypto ops. That is one logical
change.
> 1. Implements the blk_crypto_profile ops for generate, import
> and prepare key apis.
> 2. Defines UFS vops for generate, import and prepare keys so
> that vendors can hook into them.
Please describe why this code is needed. Something like "Key management
is vendor specific, so add new variant ops and tie these to the block
crypto ops"...
Thanks,
Bjorn
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v3 08/12] ufs: core: add support for generate, import and prepare keys
2023-11-22 5:38 ` [PATCH v3 08/12] ufs: core: add support for generate, import and prepare keys Gaurav Kashyap
2023-12-08 3:49 ` Bjorn Andersson
@ 2023-12-08 8:17 ` Om Prakash Singh
1 sibling, 0 replies; 39+ messages in thread
From: Om Prakash Singh @ 2023-12-08 8:17 UTC (permalink / raw)
To: Gaurav Kashyap, linux-scsi, linux-arm-msm, ebiggers,
neil.armstrong, srinivas.kandagatla
Cc: linux-mmc, linux-block, linux-fscrypt, omprsing, quic_psodagud,
abel.vesa, quic_spuppala, kernel
On 11/22/2023 11:08 AM, Gaurav Kashyap wrote:
> This patch contains two changes in UFS for wrapped keys.
> 1. Implements the blk_crypto_profile ops for generate, import
> and prepare key apis.
> 2. Defines UFS vops for generate, import and prepare keys so
> that vendors can hook into them.
re-write commit message as it is single change.
using numbering in commit message is indication of making multiple
independent changes in single patch, which should be avoided.
>
> Signed-off-by: Gaurav Kashyap <quic_gaurkash@quicinc.com>
> ---
> drivers/ufs/core/ufshcd-crypto.c | 41 ++++++++++++++++++++++++++++++++
> include/ufs/ufshcd.h | 11 +++++++++
> 2 files changed, 52 insertions(+)
>
> diff --git a/drivers/ufs/core/ufshcd-crypto.c b/drivers/ufs/core/ufshcd-crypto.c
> index 3edbca87c322..cf34f4a9cda8 100644
> --- a/drivers/ufs/core/ufshcd-crypto.c
> +++ b/drivers/ufs/core/ufshcd-crypto.c
> @@ -143,10 +143,51 @@ static int ufshcd_crypto_derive_sw_secret(struct blk_crypto_profile *profile,
> return -EOPNOTSUPP;
> }
>
> +static int ufshcd_crypto_generate_key(struct blk_crypto_profile *profile,
> + u8 lt_key[BLK_CRYPTO_MAX_HW_WRAPPED_KEY_SIZE])
> +{
> + struct ufs_hba *hba =
> + container_of(profile, struct ufs_hba, crypto_profile);
> +
> + if (hba->vops && hba->vops->generate_key)
> + return hba->vops->generate_key(hba, lt_key);
Please fix double space.
> +
> + return -EOPNOTSUPP;
> +}
> +
> +static int ufshcd_crypto_prepare_key(struct blk_crypto_profile *profile,
> + const u8 *lt_key, size_t lt_key_size,
> + u8 eph_key[BLK_CRYPTO_MAX_HW_WRAPPED_KEY_SIZE])
> +{
> + struct ufs_hba *hba =
> + container_of(profile, struct ufs_hba, crypto_profile);
> +
> + if (hba->vops && hba->vops->prepare_key)
> + return hba->vops->prepare_key(hba, lt_key, lt_key_size, eph_key);
Please fix double space.
> +
> + return -EOPNOTSUPP;
> +}
> +
> +static int ufshcd_crypto_import_key(struct blk_crypto_profile *profile,
> + const u8 *imp_key, size_t imp_key_size,
> + u8 lt_key[BLK_CRYPTO_MAX_HW_WRAPPED_KEY_SIZE])
> +{
> + struct ufs_hba *hba =
> + container_of(profile, struct ufs_hba, crypto_profile);
> +
> + if (hba->vops && hba->vops->import_key)
> + return hba->vops->import_key(hba, imp_key, imp_key_size, lt_key);
Please fix double space.
> +
> + return -EOPNOTSUPP;
> +}
> +
> static const struct blk_crypto_ll_ops ufshcd_crypto_ops = {
> .keyslot_program = ufshcd_crypto_keyslot_program,
> .keyslot_evict = ufshcd_crypto_keyslot_evict,
> .derive_sw_secret = ufshcd_crypto_derive_sw_secret,
> + .generate_key = ufshcd_crypto_generate_key,
> + .prepare_key = ufshcd_crypto_prepare_key,
> + .import_key = ufshcd_crypto_import_key,
> };
>
> static enum blk_crypto_mode_num
> diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
> index 86677788b5bd..49657a5d1e34 100644
> --- a/include/ufs/ufshcd.h
> +++ b/include/ufs/ufshcd.h
> @@ -321,6 +321,9 @@ struct ufs_pwr_mode_info {
> * @config_scaling_param: called to configure clock scaling parameters
> * @program_key: program or evict an inline encryption key
> * @derive_sw_secret: derive sw secret from a wrapped key
> + * @generate_key: generate a storage key and return longterm wrapped key
> + * @prepare_key: unwrap longterm key and return ephemeral wrapped key
> + * @import_key: import sw storage key and return longterm wrapped key
> * @event_notify: called to notify important events
> * @reinit_notify: called to notify reinit of UFSHCD during max gear switch
> * @mcq_config_resource: called to configure MCQ platform resources
> @@ -368,6 +371,14 @@ struct ufs_hba_variant_ops {
> int (*derive_sw_secret)(struct ufs_hba *hba, const u8 wkey[],
> unsigned int wkey_size,
> u8 sw_secret[BLK_CRYPTO_SW_SECRET_SIZE]);
> + int (*generate_key)(struct ufs_hba *hba,
> + u8 lt_key[BLK_CRYPTO_MAX_HW_WRAPPED_KEY_SIZE]);
> + int (*prepare_key)(struct ufs_hba *hba,
> + const u8 *lt_key, size_t lt_key_size,
> + u8 eph_key[BLK_CRYPTO_MAX_HW_WRAPPED_KEY_SIZE]);
> + int (*import_key)(struct ufs_hba *hba,
> + const u8 *imp_key, size_t imp_key_size,
> + u8 lt_key[BLK_CRYPTO_MAX_HW_WRAPPED_KEY_SIZE]);
> void (*event_notify)(struct ufs_hba *hba,
> enum ufs_event_type evt, void *data);
> void (*reinit_notify)(struct ufs_hba *);
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v3 09/12] soc: qcom: support for generate, import and prepare key
2023-11-22 5:38 [PATCH v3 00/12] Hardware wrapped key support for qcom ice and ufs Gaurav Kashyap
` (7 preceding siblings ...)
2023-11-22 5:38 ` [PATCH v3 08/12] ufs: core: add support for generate, import and prepare keys Gaurav Kashyap
@ 2023-11-22 5:38 ` Gaurav Kashyap
2023-12-08 8:26 ` Om Prakash Singh
2023-11-22 5:38 ` [PATCH v3 10/12] ufs: host: " Gaurav Kashyap
` (4 subsequent siblings)
13 siblings, 1 reply; 39+ messages in thread
From: Gaurav Kashyap @ 2023-11-22 5:38 UTC (permalink / raw)
To: linux-scsi, linux-arm-msm, ebiggers, neil.armstrong, srinivas.kandagatla
Cc: linux-mmc, linux-block, linux-fscrypt, omprsing, quic_psodagud,
abel.vesa, quic_spuppala, kernel, Gaurav Kashyap
Implements the ICE apis for generate, prepare and import key
apis and hooks it up the scm calls defined for them.
Key management has to be done from Qualcomm Trustzone as only
it can interface with HWKM.
Signed-off-by: Gaurav Kashyap <quic_gaurkash@quicinc.com>
---
drivers/soc/qcom/ice.c | 66 ++++++++++++++++++++++++++++++++++++++++++
include/soc/qcom/ice.h | 8 +++++
2 files changed, 74 insertions(+)
diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c
index ee7c0beef3d2..b00f314521ca 100644
--- a/drivers/soc/qcom/ice.c
+++ b/drivers/soc/qcom/ice.c
@@ -21,6 +21,13 @@
#define AES_256_XTS_KEY_SIZE 64
+/*
+ * Wrapped key sizes from HWKm is different for different versions of
+ * HW. It is not expected to change again in the future.
+ */
+#define QCOM_ICE_HWKM_WRAPPED_KEY_SIZE(v) \
+ ((v) == 1 ? 68 : 100)
+
/* QCOM ICE registers */
#define QCOM_ICE_REG_VERSION 0x0008
#define QCOM_ICE_REG_FUSE_SETTING 0x0010
@@ -426,6 +433,65 @@ int qcom_ice_derive_sw_secret(struct qcom_ice *ice, const u8 wkey[],
}
EXPORT_SYMBOL_GPL(qcom_ice_derive_sw_secret);
+/**
+ * qcom_ice_generate_key() - Generate a wrapped key for inline encryption
+ * @lt_key: longterm wrapped key that is generated, which is
+ * BLK_CRYPTO_MAX_HW_WRAPPED_KEY_SIZE in size.
+ *
+ * Make a scm call into trustzone to generate a wrapped key for storage
+ * encryption using hwkm.
+ *
+ * Return: 0 on success; err on failure.
+ */
+int qcom_ice_generate_key(struct qcom_ice *ice,
+ u8 lt_key[BLK_CRYPTO_MAX_HW_WRAPPED_KEY_SIZE])
+{
+ return qcom_scm_generate_ice_key(lt_key,
+ QCOM_ICE_HWKM_WRAPPED_KEY_SIZE(ice->hwkm_version));
+}
+EXPORT_SYMBOL_GPL(qcom_ice_generate_key);
+
+/**
+ * qcom_ice_prepare_key() - Prepare a longterm wrapped key for inline encryption
+ * @lt_key: longterm wrapped key that is generated or imported.
+ * @lt_key_size: size of the longterm wrapped_key
+ * @eph_key: wrapped key returned which has been wrapped with a per-boot ephemeral key,
+ * size of which is BLK_CRYPTO_MAX_HW_WRAPPED_KEY_SIZE in size.
+ *
+ * Make a scm call into trustzone to prepare a wrapped key for storage
+ * encryption by rewrapping the longterm wrapped key with a per boot ephemeral
+ * key using hwkm.
+ *
+ * Return: 0 on success; err on failure.
+ */
+int qcom_ice_prepare_key(struct qcom_ice *ice, const u8 *lt_key, size_t lt_key_size,
+ u8 eph_key[BLK_CRYPTO_MAX_HW_WRAPPED_KEY_SIZE])
+{
+ return qcom_scm_prepare_ice_key(lt_key, lt_key_size, eph_key,
+ QCOM_ICE_HWKM_WRAPPED_KEY_SIZE(ice->hwkm_version));
+}
+EXPORT_SYMBOL_GPL(qcom_ice_prepare_key);
+
+/**
+ * qcom_ice_import_key() - Import a raw key for inline encryption
+ * @imp_key: raw key that has to be imported
+ * @imp_key_size: size of the imported key
+ * @lt_key: longterm wrapped key that is imported, which is
+ * BLK_CRYPTO_MAX_HW_WRAPPED_KEY_SIZE in size.
+ *
+ * Make a scm call into trustzone to import a raw key for storage encryption
+ * and generate a longterm wrapped key using hwkm.
+ *
+ * Return: 0 on success; err on failure.
+ */
+int qcom_ice_import_key(struct qcom_ice *ice, const u8 *imp_key, size_t imp_key_size,
+ u8 lt_key[BLK_CRYPTO_MAX_HW_WRAPPED_KEY_SIZE])
+{
+ return qcom_scm_import_ice_key(imp_key, imp_key_size, lt_key,
+ QCOM_ICE_HWKM_WRAPPED_KEY_SIZE(ice->hwkm_version));
+}
+EXPORT_SYMBOL_GPL(qcom_ice_import_key);
+
static struct qcom_ice *qcom_ice_create(struct device *dev,
void __iomem *base)
{
diff --git a/include/soc/qcom/ice.h b/include/soc/qcom/ice.h
index dabe0d3a1fd0..dcf277d196ff 100644
--- a/include/soc/qcom/ice.h
+++ b/include/soc/qcom/ice.h
@@ -39,5 +39,13 @@ bool qcom_ice_hwkm_supported(struct qcom_ice *ice);
int qcom_ice_derive_sw_secret(struct qcom_ice *ice, const u8 wkey[],
unsigned int wkey_size,
u8 sw_secret[BLK_CRYPTO_SW_SECRET_SIZE]);
+int qcom_ice_generate_key(struct qcom_ice *ice,
+ u8 lt_key[BLK_CRYPTO_MAX_HW_WRAPPED_KEY_SIZE]);
+int qcom_ice_prepare_key(struct qcom_ice *ice,
+ const u8 *lt_key, size_t lt_key_size,
+ u8 eph_key[BLK_CRYPTO_MAX_HW_WRAPPED_KEY_SIZE]);
+int qcom_ice_import_key(struct qcom_ice *ice,
+ const u8 *imp_key, size_t imp_key_size,
+ u8 lt_key[BLK_CRYPTO_MAX_HW_WRAPPED_KEY_SIZE]);
struct qcom_ice *of_qcom_ice_get(struct device *dev);
#endif /* __QCOM_ICE_H__ */
--
2.25.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [PATCH v3 09/12] soc: qcom: support for generate, import and prepare key
2023-11-22 5:38 ` [PATCH v3 09/12] soc: qcom: support for generate, import and prepare key Gaurav Kashyap
@ 2023-12-08 8:26 ` Om Prakash Singh
0 siblings, 0 replies; 39+ messages in thread
From: Om Prakash Singh @ 2023-12-08 8:26 UTC (permalink / raw)
To: Gaurav Kashyap, linux-scsi, linux-arm-msm, ebiggers,
neil.armstrong, srinivas.kandagatla
Cc: linux-mmc, linux-block, linux-fscrypt, omprsing, quic_psodagud,
abel.vesa, quic_spuppala, kernel
On 11/22/2023 11:08 AM, Gaurav Kashyap wrote:
> Implements the ICE apis for generate, prepare and import key
> apis and hooks it up the scm calls defined for them.
We can avoid mentioning below text as it is also possibility to have
HWKM Linux driver.
> Key management has to be done from Qualcomm Trustzone as only
> it can interface with HWKM.
>
> Signed-off-by: Gaurav Kashyap <quic_gaurkash@quicinc.com>
> ---
> drivers/soc/qcom/ice.c | 66 ++++++++++++++++++++++++++++++++++++++++++
> include/soc/qcom/ice.h | 8 +++++
> 2 files changed, 74 insertions(+)
>
> diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c
> index ee7c0beef3d2..b00f314521ca 100644
> --- a/drivers/soc/qcom/ice.c
> +++ b/drivers/soc/qcom/ice.c
> @@ -21,6 +21,13 @@
>
> #define AES_256_XTS_KEY_SIZE 64
>
> +/*
> + * Wrapped key sizes from HWKm is different for different versions of
"HWKM"
> + * HW. It is not expected to change again in the future.
we can avoid mentioning "It is not expected to change again in the future"
> + */
> +#define QCOM_ICE_HWKM_WRAPPED_KEY_SIZE(v) \
> + ((v) == 1 ? 68 : 100)
> +
> /* QCOM ICE registers */
> #define QCOM_ICE_REG_VERSION 0x0008
> #define QCOM_ICE_REG_FUSE_SETTING 0x0010
> @@ -426,6 +433,65 @@ int qcom_ice_derive_sw_secret(struct qcom_ice *ice, const u8 wkey[],
> }
> EXPORT_SYMBOL_GPL(qcom_ice_derive_sw_secret);
>
> +/**
> + * qcom_ice_generate_key() - Generate a wrapped key for inline encryption
> + * @lt_key: longterm wrapped key that is generated, which is
> + * BLK_CRYPTO_MAX_HW_WRAPPED_KEY_SIZE in size.
> + *
> + * Make a scm call into trustzone to generate a wrapped key for storage
> + * encryption using hwkm.
> + *
> + * Return: 0 on success; err on failure.
> + */
> +int qcom_ice_generate_key(struct qcom_ice *ice,
> + u8 lt_key[BLK_CRYPTO_MAX_HW_WRAPPED_KEY_SIZE])
> +{
> + return qcom_scm_generate_ice_key(lt_key,
> + QCOM_ICE_HWKM_WRAPPED_KEY_SIZE(ice->hwkm_version));
> +}
> +EXPORT_SYMBOL_GPL(qcom_ice_generate_key);
> +
> +/**
> + * qcom_ice_prepare_key() - Prepare a longterm wrapped key for inline encryption
> + * @lt_key: longterm wrapped key that is generated or imported.
> + * @lt_key_size: size of the longterm wrapped_key
> + * @eph_key: wrapped key returned which has been wrapped with a per-boot ephemeral key,
> + * size of which is BLK_CRYPTO_MAX_HW_WRAPPED_KEY_SIZE in size.
> + *
> + * Make a scm call into trustzone to prepare a wrapped key for storage
> + * encryption by rewrapping the longterm wrapped key with a per boot ephemeral
> + * key using hwkm.
> + *
> + * Return: 0 on success; err on failure.
avoid Return value documentation as it is not adding any specific
information.
> + */
> +int qcom_ice_prepare_key(struct qcom_ice *ice, const u8 *lt_key, size_t lt_key_size,
> + u8 eph_key[BLK_CRYPTO_MAX_HW_WRAPPED_KEY_SIZE])
> +{
> + return qcom_scm_prepare_ice_key(lt_key, lt_key_size, eph_key,
> + QCOM_ICE_HWKM_WRAPPED_KEY_SIZE(ice->hwkm_version));
> +}
> +EXPORT_SYMBOL_GPL(qcom_ice_prepare_key);
> +
> +/**
> + * qcom_ice_import_key() - Import a raw key for inline encryption
> + * @imp_key: raw key that has to be imported
> + * @imp_key_size: size of the imported key
> + * @lt_key: longterm wrapped key that is imported, which is
> + * BLK_CRYPTO_MAX_HW_WRAPPED_KEY_SIZE in size.
> + *
> + * Make a scm call into trustzone to import a raw key for storage encryption
> + * and generate a longterm wrapped key using hwkm.
> + *
> + * Return: 0 on success; err on failure.
> + */
> +int qcom_ice_import_key(struct qcom_ice *ice, const u8 *imp_key, size_t imp_key_size,
> + u8 lt_key[BLK_CRYPTO_MAX_HW_WRAPPED_KEY_SIZE])
> +{
> + return qcom_scm_import_ice_key(imp_key, imp_key_size, lt_key,
> + QCOM_ICE_HWKM_WRAPPED_KEY_SIZE(ice->hwkm_version));
> +}
> +EXPORT_SYMBOL_GPL(qcom_ice_import_key);
> +
> static struct qcom_ice *qcom_ice_create(struct device *dev,
> void __iomem *base)
> {
> diff --git a/include/soc/qcom/ice.h b/include/soc/qcom/ice.h
> index dabe0d3a1fd0..dcf277d196ff 100644
> --- a/include/soc/qcom/ice.h
> +++ b/include/soc/qcom/ice.h
> @@ -39,5 +39,13 @@ bool qcom_ice_hwkm_supported(struct qcom_ice *ice);
> int qcom_ice_derive_sw_secret(struct qcom_ice *ice, const u8 wkey[],
> unsigned int wkey_size,
> u8 sw_secret[BLK_CRYPTO_SW_SECRET_SIZE]);
> +int qcom_ice_generate_key(struct qcom_ice *ice,
> + u8 lt_key[BLK_CRYPTO_MAX_HW_WRAPPED_KEY_SIZE]);
> +int qcom_ice_prepare_key(struct qcom_ice *ice,
> + const u8 *lt_key, size_t lt_key_size,
> + u8 eph_key[BLK_CRYPTO_MAX_HW_WRAPPED_KEY_SIZE]);
> +int qcom_ice_import_key(struct qcom_ice *ice,
> + const u8 *imp_key, size_t imp_key_size,
> + u8 lt_key[BLK_CRYPTO_MAX_HW_WRAPPED_KEY_SIZE]);
> struct qcom_ice *of_qcom_ice_get(struct device *dev);
> #endif /* __QCOM_ICE_H__ */
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v3 10/12] ufs: host: support for generate, import and prepare key
2023-11-22 5:38 [PATCH v3 00/12] Hardware wrapped key support for qcom ice and ufs Gaurav Kashyap
` (8 preceding siblings ...)
2023-11-22 5:38 ` [PATCH v3 09/12] soc: qcom: support for generate, import and prepare key Gaurav Kashyap
@ 2023-11-22 5:38 ` Gaurav Kashyap
2023-12-08 8:29 ` Om Prakash Singh
2023-11-22 5:38 ` [PATCH v3 11/12] arm64: dts: qcom: sm8650: add hwkm support to ufs ice Gaurav Kashyap
` (3 subsequent siblings)
13 siblings, 1 reply; 39+ messages in thread
From: Gaurav Kashyap @ 2023-11-22 5:38 UTC (permalink / raw)
To: linux-scsi, linux-arm-msm, ebiggers, neil.armstrong, srinivas.kandagatla
Cc: linux-mmc, linux-block, linux-fscrypt, omprsing, quic_psodagud,
abel.vesa, quic_spuppala, kernel, Gaurav Kashyap
Implements the vops for generate, prepare and import key
apis in ufs qcom and call the respective ICE module apis
for them.
Signed-off-by: Gaurav Kashyap <quic_gaurkash@quicinc.com>
---
drivers/ufs/host/ufs-qcom.c | 34 ++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)
diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
index 6b09e09b1a30..b4644d07d6f3 100644
--- a/drivers/ufs/host/ufs-qcom.c
+++ b/drivers/ufs/host/ufs-qcom.c
@@ -191,10 +191,41 @@ int ufs_qcom_ice_derive_sw_secret(struct ufs_hba *hba, const u8 wkey[],
return qcom_ice_derive_sw_secret(host->ice, wkey, wkey_size, sw_secret);
}
+int ufs_qcom_ice_generate_key(struct ufs_hba *hba,
+ u8 lt_key[BLK_CRYPTO_MAX_HW_WRAPPED_KEY_SIZE])
+{
+ struct ufs_qcom_host *host = ufshcd_get_variant(hba);
+
+ return qcom_ice_generate_key(host->ice, lt_key);
+}
+
+int ufs_qcom_ice_prepare_key(struct ufs_hba *hba,
+ const u8 *lt_key, size_t lt_key_size,
+ u8 eph_key[BLK_CRYPTO_MAX_HW_WRAPPED_KEY_SIZE])
+{
+ struct ufs_qcom_host *host = ufshcd_get_variant(hba);
+
+ return qcom_ice_prepare_key(host->ice, lt_key, lt_key_size,
+ eph_key);
+}
+
+int ufs_qcom_ice_import_key(struct ufs_hba *hba,
+ const u8 *imp_key, size_t imp_key_size,
+ u8 lt_key[BLK_CRYPTO_MAX_HW_WRAPPED_KEY_SIZE])
+{
+ struct ufs_qcom_host *host = ufshcd_get_variant(hba);
+
+ return qcom_ice_import_key(host->ice, imp_key, imp_key_size,
+ lt_key);
+}
+
#else
#define ufs_qcom_ice_program_key NULL
#define ufs_qcom_ice_derive_sw_secret NULL
+#define ufs_qcom_ice_generate_key NULL
+#define ufs_qcom_ice_prepare_key NULL
+#define ufs_qcom_ice_import_key NULL
static inline void ufs_qcom_ice_enable(struct ufs_qcom_host *host)
{
@@ -2017,6 +2048,9 @@ static const struct ufs_hba_variant_ops ufs_hba_qcom_vops = {
.config_scaling_param = ufs_qcom_config_scaling_param,
.program_key = ufs_qcom_ice_program_key,
.derive_sw_secret = ufs_qcom_ice_derive_sw_secret,
+ .generate_key = ufs_qcom_ice_generate_key,
+ .prepare_key = ufs_qcom_ice_prepare_key,
+ .import_key = ufs_qcom_ice_import_key,
.reinit_notify = ufs_qcom_reinit_notify,
.mcq_config_resource = ufs_qcom_mcq_config_resource,
.get_hba_mac = ufs_qcom_get_hba_mac,
--
2.25.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [PATCH v3 10/12] ufs: host: support for generate, import and prepare key
2023-11-22 5:38 ` [PATCH v3 10/12] ufs: host: " Gaurav Kashyap
@ 2023-12-08 8:29 ` Om Prakash Singh
0 siblings, 0 replies; 39+ messages in thread
From: Om Prakash Singh @ 2023-12-08 8:29 UTC (permalink / raw)
To: Gaurav Kashyap, linux-scsi, linux-arm-msm, ebiggers,
neil.armstrong, srinivas.kandagatla
Cc: linux-mmc, linux-block, linux-fscrypt, omprsing, quic_psodagud,
abel.vesa, quic_spuppala, kernel
On 11/22/2023 11:08 AM, Gaurav Kashyap wrote:
> Implements the vops for generate, prepare and import key apis in ufs
> qcom and call the respective ICE module apis for them. Signed-off-by:
> Gaurav Kashyap <quic_gaurkash@quicinc.com> ---
Reviewed-by: Om Prakash Singh <quic_omprsing@quicinc.com>
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v3 11/12] arm64: dts: qcom: sm8650: add hwkm support to ufs ice
2023-11-22 5:38 [PATCH v3 00/12] Hardware wrapped key support for qcom ice and ufs Gaurav Kashyap
` (9 preceding siblings ...)
2023-11-22 5:38 ` [PATCH v3 10/12] ufs: host: " Gaurav Kashyap
@ 2023-11-22 5:38 ` Gaurav Kashyap
2023-12-08 3:51 ` Bjorn Andersson
2023-12-08 8:46 ` Om Prakash Singh
2023-11-22 5:38 ` [PATCH v3 12/12] dt-bindings: crypto: ice: document the hwkm property Gaurav Kashyap
` (2 subsequent siblings)
13 siblings, 2 replies; 39+ messages in thread
From: Gaurav Kashyap @ 2023-11-22 5:38 UTC (permalink / raw)
To: linux-scsi, linux-arm-msm, ebiggers, neil.armstrong, srinivas.kandagatla
Cc: linux-mmc, linux-block, linux-fscrypt, omprsing, quic_psodagud,
abel.vesa, quic_spuppala, kernel, Gaurav Kashyap
The Inline Crypto Engine (ICE) for UFS supports the
Hardware Key Manager (hwkm) to securely manage storage
keys. Enable using this hardware on sm8650.
Signed-off-by: Gaurav Kashyap <quic_gaurkash@quicinc.com>
---
arch/arm64/boot/dts/qcom/sm8650.dtsi | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi
index bbebe15437aa..b61066210e09 100644
--- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
@@ -763,7 +763,8 @@ rng: rng@10c3000 {
ice: crypto@1d88000 {
compatible = "qcom,sm8650-inline-crypto-engine",
"qcom,inline-crypto-engine";
- reg = <0 0x01d88000 0 0x8000>;
+ reg = <0 0x01d88000 0 0x10000>;
+ qcom,ice-use-hwkm;
clocks = <&gcc GCC_UFS_PHY_ICE_CORE_CLK>;
};
--
2.25.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [PATCH v3 11/12] arm64: dts: qcom: sm8650: add hwkm support to ufs ice
2023-11-22 5:38 ` [PATCH v3 11/12] arm64: dts: qcom: sm8650: add hwkm support to ufs ice Gaurav Kashyap
@ 2023-12-08 3:51 ` Bjorn Andersson
2023-12-08 8:45 ` Om Prakash Singh
2023-12-08 8:46 ` Om Prakash Singh
1 sibling, 1 reply; 39+ messages in thread
From: Bjorn Andersson @ 2023-12-08 3:51 UTC (permalink / raw)
To: Gaurav Kashyap
Cc: linux-scsi, linux-arm-msm, ebiggers, neil.armstrong,
srinivas.kandagatla, linux-mmc, linux-block, linux-fscrypt,
omprsing, quic_psodagud, abel.vesa, quic_spuppala, kernel
On Tue, Nov 21, 2023 at 09:38:16PM -0800, Gaurav Kashyap wrote:
> The Inline Crypto Engine (ICE) for UFS supports the
> Hardware Key Manager (hwkm) to securely manage storage
> keys. Enable using this hardware on sm8650.
I'm unable to understand why the size of the reg changes based on this
motivation.
Regards,
Bjorn
>
> Signed-off-by: Gaurav Kashyap <quic_gaurkash@quicinc.com>
> ---
> arch/arm64/boot/dts/qcom/sm8650.dtsi | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi
> index bbebe15437aa..b61066210e09 100644
> --- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
> @@ -763,7 +763,8 @@ rng: rng@10c3000 {
> ice: crypto@1d88000 {
> compatible = "qcom,sm8650-inline-crypto-engine",
> "qcom,inline-crypto-engine";
> - reg = <0 0x01d88000 0 0x8000>;
> + reg = <0 0x01d88000 0 0x10000>;
> + qcom,ice-use-hwkm;
>
> clocks = <&gcc GCC_UFS_PHY_ICE_CORE_CLK>;
> };
> --
> 2.25.1
>
>
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [PATCH v3 11/12] arm64: dts: qcom: sm8650: add hwkm support to ufs ice
2023-12-08 3:51 ` Bjorn Andersson
@ 2023-12-08 8:45 ` Om Prakash Singh
0 siblings, 0 replies; 39+ messages in thread
From: Om Prakash Singh @ 2023-12-08 8:45 UTC (permalink / raw)
To: Bjorn Andersson, Gaurav Kashyap
Cc: linux-scsi, linux-arm-msm, ebiggers, neil.armstrong,
srinivas.kandagatla, linux-mmc, linux-block, linux-fscrypt,
omprsing, quic_psodagud, abel.vesa, quic_spuppala, kernel
On 12/8/2023 9:21 AM, Bjorn Andersson wrote:
> On Tue, Nov 21, 2023 at 09:38:16PM -0800, Gaurav Kashyap wrote:
>> The Inline Crypto Engine (ICE) for UFS supports the
>> Hardware Key Manager (hwkm) to securely manage storage
>> keys. Enable using this hardware on sm8650.
>
> I'm unable to understand why the size of the reg changes based on this
> motivation.
>
> Regards,
> Bjorn
>
Actual size of register space size is 0x18000 (as per IPCAT). But all
registers space are not used.
HWKM register offset is start at 0x8000. Hence needed to expand reg map
size.
>>
>> Signed-off-by: Gaurav Kashyap <quic_gaurkash@quicinc.com>
>> ---
>> arch/arm64/boot/dts/qcom/sm8650.dtsi | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi
>> index bbebe15437aa..b61066210e09 100644
>> --- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
>> @@ -763,7 +763,8 @@ rng: rng@10c3000 {
>> ice: crypto@1d88000 {
>> compatible = "qcom,sm8650-inline-crypto-engine",
>> "qcom,inline-crypto-engine";
>> - reg = <0 0x01d88000 0 0x8000>;
>> + reg = <0 0x01d88000 0 0x10000>;
>> + qcom,ice-use-hwkm;
>>
>> clocks = <&gcc GCC_UFS_PHY_ICE_CORE_CLK>;
>> };
>> --
>> 2.25.1
>>
>>
>
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v3 11/12] arm64: dts: qcom: sm8650: add hwkm support to ufs ice
2023-11-22 5:38 ` [PATCH v3 11/12] arm64: dts: qcom: sm8650: add hwkm support to ufs ice Gaurav Kashyap
2023-12-08 3:51 ` Bjorn Andersson
@ 2023-12-08 8:46 ` Om Prakash Singh
1 sibling, 0 replies; 39+ messages in thread
From: Om Prakash Singh @ 2023-12-08 8:46 UTC (permalink / raw)
To: Gaurav Kashyap, linux-scsi, linux-arm-msm, ebiggers,
neil.armstrong, srinivas.kandagatla
Cc: linux-mmc, linux-block, linux-fscrypt, omprsing, quic_psodagud,
abel.vesa, quic_spuppala, kernel
On 11/22/2023 11:08 AM, Gaurav Kashyap wrote:
> The Inline Crypto Engine (ICE) for UFS supports the Hardware Key Manager
> (hwkm) to securely manage storage keys. Enable using this hardware on
> sm8650. Signed-off-by: Gaurav Kashyap <quic_gaurkash@quicinc.com> ---
Reviewed-by: Om Prakash Singh <quic_omprsing@quicinc.com>
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v3 12/12] dt-bindings: crypto: ice: document the hwkm property
2023-11-22 5:38 [PATCH v3 00/12] Hardware wrapped key support for qcom ice and ufs Gaurav Kashyap
` (10 preceding siblings ...)
2023-11-22 5:38 ` [PATCH v3 11/12] arm64: dts: qcom: sm8650: add hwkm support to ufs ice Gaurav Kashyap
@ 2023-11-22 5:38 ` Gaurav Kashyap
2023-11-22 9:53 ` Krzysztof Kozlowski
2023-12-08 4:16 ` Bjorn Andersson
2023-11-22 9:55 ` [PATCH v3 00/12] Hardware wrapped key support for qcom ice and ufs Krzysztof Kozlowski
2023-12-05 17:33 ` neil.armstrong
13 siblings, 2 replies; 39+ messages in thread
From: Gaurav Kashyap @ 2023-11-22 5:38 UTC (permalink / raw)
To: linux-scsi, linux-arm-msm, ebiggers, neil.armstrong, srinivas.kandagatla
Cc: linux-mmc, linux-block, linux-fscrypt, omprsing, quic_psodagud,
abel.vesa, quic_spuppala, kernel, Gaurav Kashyap
Add documentation for the ice-use-hwkm property in
qcom ice.
Signed-off-by: Gaurav Kashyap <quic_gaurkash@quicinc.com>
---
.../bindings/crypto/qcom,inline-crypto-engine.yaml | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/Documentation/devicetree/bindings/crypto/qcom,inline-crypto-engine.yaml b/Documentation/devicetree/bindings/crypto/qcom,inline-crypto-engine.yaml
index ca4f7d1cefaa..93e017dddc9d 100644
--- a/Documentation/devicetree/bindings/crypto/qcom,inline-crypto-engine.yaml
+++ b/Documentation/devicetree/bindings/crypto/qcom,inline-crypto-engine.yaml
@@ -24,6 +24,13 @@ properties:
clocks:
maxItems: 1
+ qcom,ice-use-hwkm:
+ type: boolean
+ description:
+ Use the supported Hardware Key Manager (HWKM) in Qualcomm
+ ICE to support wrapped keys. This dictates if wrapped keys
+ have to be used by ICE.
+
required:
- compatible
- reg
--
2.25.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [PATCH v3 12/12] dt-bindings: crypto: ice: document the hwkm property
2023-11-22 5:38 ` [PATCH v3 12/12] dt-bindings: crypto: ice: document the hwkm property Gaurav Kashyap
@ 2023-11-22 9:53 ` Krzysztof Kozlowski
2023-12-08 4:16 ` Bjorn Andersson
1 sibling, 0 replies; 39+ messages in thread
From: Krzysztof Kozlowski @ 2023-11-22 9:53 UTC (permalink / raw)
To: Gaurav Kashyap, linux-scsi, linux-arm-msm, ebiggers,
neil.armstrong, srinivas.kandagatla
Cc: linux-mmc, linux-block, linux-fscrypt, omprsing, quic_psodagud,
abel.vesa, quic_spuppala, kernel
On 22/11/2023 06:38, Gaurav Kashyap wrote:
> Add documentation for the ice-use-hwkm property in
> qcom ice.
>
> Signed-off-by: Gaurav Kashyap <quic_gaurkash@quicinc.com>
Another day, another same comments:
https://lore.kernel.org/all/6e435e84-fea9-4f74-8977-d589cbc31ded@kernel.org/
Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC. It might happen, that command when run on an older
kernel, gives you outdated entries. Therefore please be sure you base
your patches on recent Linux kernel.
You missed at least devicetree list (maybe more), so this won't be
tested by automated tooling. Performing review on untested code might be
a waste of time, thus I will skip this patch entirely till you follow
the process allowing the patch to be tested.
Please kindly resend and include all necessary To/Cc entries.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v3 12/12] dt-bindings: crypto: ice: document the hwkm property
2023-11-22 5:38 ` [PATCH v3 12/12] dt-bindings: crypto: ice: document the hwkm property Gaurav Kashyap
2023-11-22 9:53 ` Krzysztof Kozlowski
@ 2023-12-08 4:16 ` Bjorn Andersson
1 sibling, 0 replies; 39+ messages in thread
From: Bjorn Andersson @ 2023-12-08 4:16 UTC (permalink / raw)
To: Gaurav Kashyap
Cc: linux-scsi, linux-arm-msm, ebiggers, neil.armstrong,
srinivas.kandagatla, linux-mmc, linux-block, linux-fscrypt,
omprsing, quic_psodagud, abel.vesa, quic_spuppala, kernel
On Tue, Nov 21, 2023 at 09:38:17PM -0800, Gaurav Kashyap wrote:
> Add documentation for the ice-use-hwkm property in
> qcom ice.
Please describe the problem that you're solving by adding this property
to the binding (the description of hardware/firmware).
>
> Signed-off-by: Gaurav Kashyap <quic_gaurkash@quicinc.com>
> ---
> .../bindings/crypto/qcom,inline-crypto-engine.yaml | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/crypto/qcom,inline-crypto-engine.yaml b/Documentation/devicetree/bindings/crypto/qcom,inline-crypto-engine.yaml
> index ca4f7d1cefaa..93e017dddc9d 100644
> --- a/Documentation/devicetree/bindings/crypto/qcom,inline-crypto-engine.yaml
> +++ b/Documentation/devicetree/bindings/crypto/qcom,inline-crypto-engine.yaml
> @@ -24,6 +24,13 @@ properties:
> clocks:
> maxItems: 1
>
> + qcom,ice-use-hwkm:
> + type: boolean
> + description:
> + Use the supported Hardware Key Manager (HWKM) in Qualcomm
> + ICE to support wrapped keys. This dictates if wrapped keys
> + have to be used by ICE.
As described it sounds like software configuration, rather than a
description of the capabilities of the hardware/firmware.
In what cases would we, and would we not, specify this property?
Regards,
Bjorn
> +
> required:
> - compatible
> - reg
> --
> 2.25.1
>
>
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v3 00/12] Hardware wrapped key support for qcom ice and ufs
2023-11-22 5:38 [PATCH v3 00/12] Hardware wrapped key support for qcom ice and ufs Gaurav Kashyap
` (11 preceding siblings ...)
2023-11-22 5:38 ` [PATCH v3 12/12] dt-bindings: crypto: ice: document the hwkm property Gaurav Kashyap
@ 2023-11-22 9:55 ` Krzysztof Kozlowski
2023-12-05 17:33 ` neil.armstrong
13 siblings, 0 replies; 39+ messages in thread
From: Krzysztof Kozlowski @ 2023-11-22 9:55 UTC (permalink / raw)
To: Gaurav Kashyap, linux-scsi, linux-arm-msm, ebiggers,
neil.armstrong, srinivas.kandagatla
Cc: linux-mmc, linux-block, linux-fscrypt, omprsing, quic_psodagud,
abel.vesa, quic_spuppala, kernel
On 22/11/2023 06:38, Gaurav Kashyap wrote:
> ice, ufs, mmc: use blk_crypto_key for program_key
> qcom_scm: scm call for deriving a software secret
> soc: qcom: ice: add hwkm support in ice
> soc: qcom: ice: support for hardware wrapped keys
> ufs: core: support wrapped keys in ufs core
> ufs: host: wrapped keys support in ufs qcom
> qcom_scm: scm call for create, prepare and import keys
> ufs: core: add support for generate, import and prepare keys
> soc: qcom: support for generate, import and prepare key
> ufs: host: support for generate, import and prepare key
> arm64: dts: qcom: sm8650: add hwkm support to ufs ice
This is close to a spaghetti patchset. ICE, UFS, MMC, then followed up
by SoC patches, then UFS, then firmware, then UFS, then again SoC and we
are back at UFS.
Crazy dependencies... or you collected unrelated patches into one
patchset making the merge strategy tricky.
> dt-bindings: crypto: ice: document the hwkm property
Bindings come before users.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [PATCH v3 00/12] Hardware wrapped key support for qcom ice and ufs
2023-11-22 5:38 [PATCH v3 00/12] Hardware wrapped key support for qcom ice and ufs Gaurav Kashyap
` (12 preceding siblings ...)
2023-11-22 9:55 ` [PATCH v3 00/12] Hardware wrapped key support for qcom ice and ufs Krzysztof Kozlowski
@ 2023-12-05 17:33 ` neil.armstrong
13 siblings, 0 replies; 39+ messages in thread
From: neil.armstrong @ 2023-12-05 17:33 UTC (permalink / raw)
To: Gaurav Kashyap, linux-scsi, linux-arm-msm, ebiggers, srinivas.kandagatla
Cc: linux-mmc, linux-block, linux-fscrypt, omprsing, quic_psodagud,
abel.vesa, quic_spuppala, kernel
Hi Gaurav,
On 22/11/2023 06:38, Gaurav Kashyap wrote:
> These are the third iteration of patches that add support to Qualcomm ICE (Inline Crypto Engine) for hardware wrapped keys using Qualcomm Hardware Key Manager (HWKM)
>
> They patches do the following:
> - Address comments from v2 (Found here: https://lore.kernel.org/all/20230719170423.220033-1-quic_gaurkash@quicinc.com/)
> - Rebased and tested on top of Eric's latest patchset: https://lore.kernel.org/all/20231104211259.17448-1-ebiggers@kernel.org/
> - Rebased and tested on top of SM8650 patches from Linaro: https://lore.kernel.org/all/?q=sm8650
>
> Information about patches copied over from v2:
>
> "
> Explanation and use of hardware-wrapped-keys can be found here:
> Documentation/block/inline-encryption.rst
>
> This patch is organized as follows:
>
> Patch 1 - Prepares ICE and storage layers (UFS and EMMC) to pass around wrapped keys.
> Patch 2 - Adds a new SCM api to support deriving software secret when wrapped keys are used
> Patch 3-4 - Adds support for wrapped keys in the ICE driver. This includes adding HWKM support
> Patch 5-6 - Adds support for wrapped keys in UFS
> Patch 7-10 - Supports generate, prepare and import functionality in ICE and UFS
>
> NOTE: MMC will have similar changes to UFS and will be uploaded in a different patchset
> Patch 3, 4, 8, 10 will have MMC equivalents.
> "
>
> Testing:
> Test platform: SM8650 MTP
>
> The changes were tested by mounting initramfs and running the fscryptctl
> tool (Ref: https://github.com/ebiggers/fscryptctl/tree/wip-wrapped-keys) to
> generate and prepare keys, as well as to set policies on folders, which
> consequently invokes disk encryption flows through UFS.
>
> Tested both standard and wrapped keys (Removing qcom,ice-use-hwkm from dtsi will support using standard keys)
>
> Steps to test:
>
> The following configs were enabled:
> CONFIG_BLK_INLINE_ENCRYPTION=y
> CONFIG_QCOM_INLINE_CRYPTO_ENGINE=m
> CONFIG_FS_ENCRYPTION_INLINE_CRYPT=y
> CONFIG_SCSI_UFS_CRYPTO=y
>
> Flash boot image, boot to shell and run the following commands
>
> Creating and preparing keys
> - mkfs.ext4 -F -O encrypt,stable_inodes /dev/disk/by-partlabel/userdata
> - mount /dev/disk/by-partlabel/userdata -o inlinecrypt /mnt
> - ./fscryptctl generate_hw_wrapped_key /dev/disk/by-partlabel/userdata > /mnt/key.longterm
> Note: import_hw_wrapped_key currently has a big which just got fixed, so it will be functional in the next SM8650 release
> (It might already be available by the time the boards are available to public)
> - ./fscryptctl prepare_hw_wrapped_key /dev/disk/by-partlabel/userdata < /mnt/key.longterm > /tmp/key.ephemeral
> - ./fscryptctl add_key --hw-wrapped-key < /tmp/key.ephemeral /mnt
>
> Create a folder and associate created keys with the folder
> - rm -rf /mnt/dir
> - mkdir /mnt/dir
> - ./fscryptctl set_policy --hw-wrapped-key --iv-ino-lblk-64 "$keyid" /mnt/dir
> - dmesg > /mnt/dir/test.txt
> - sync
>
> - Reboot
> - mount /dev/disk/by-partlabel/userdata -o inlinecrypt /mnt
> - ls /mnt/dir (You should see an encrypted file)
> - ./fscryptctl prepare_hw_wrapped_key /dev/disk/by-partlabel/userdata < /mnt/key.longterm > /tmp/key.ephemeral
> - ./fscryptctl add_key --hw-wrapped-key < /tmp/key.ephemeral /mnt
> - cat /mnt/dir/test.txt
I successfully tested with those instructions on the SM8650 QRD,
Tested-by: Neil Armstrong <neil.armstrong@linaro.org> # on SM8650-QRD
however I got some build errors on sdhci-msm:
drivers/mmc/host/sdhci-msm.c:1862:19: error: ‘blk_crypto_key_type’ defined as wrong kind of tag
1862 | const struct blk_crypto_key_type *bkey,
| ^~~~~~~~~~~~~~~~~~~
drivers/mmc/host/sdhci-msm.c:1862:19: warning: ‘struct blk_crypto_key_type’ declared inside parameter list will not be visible outside of thi
s definition or declaration
drivers/mmc/host/sdhci-msm.c: In function ‘sdhci_msm_program_key’:
drivers/mmc/host/sdhci-msm.c:1882:24: error: passing argument 4 of ‘qcom_ice_program_key’ from incompatible pointer type [-Werror=incompatibl
e-pointer-types]
1882 | ice_key_size, bkey,
| ^~~~
| |
| const struct blk_crypto_key_type *
In file included from drivers/mmc/host/sdhci-msm.c:21:
include/soc/qcom/ice.h:35:34: note: expected ‘const struct blk_crypto_key *’ but argument is of type ‘const struct blk_crypto_key_type *’
35 | const struct blk_crypto_key *bkey,
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
drivers/mmc/host/sdhci-msm.c: At top level:
drivers/mmc/host/sdhci-msm.c:1993:17: error: initialization of ‘int (*)(struct cqhci_host *, const struct blk_crypto_key *, const union cqhci
_crypto_cfg_entry *, int)’ from incompatible pointer type ‘int (*)(struct cqhci_host *, const struct blk_crypto_key_type *, const union cqhci_crypto_cfg_entry *, int)’ [-Werro
r=incompatible-pointer-types]
1993 | .program_key = sdhci_msm_program_key,
| ^~~~~~~~~~~~~~~~~~~~~
drivers/mmc/host/sdhci-msm.c:1993:17: note: (near initialization for ‘sdhci_msm_cqhci_ops.program_key’)
Thanks,
Neil
>
> Gaurav Kashyap (12):
> ice, ufs, mmc: use blk_crypto_key for program_key
> qcom_scm: scm call for deriving a software secret
> soc: qcom: ice: add hwkm support in ice
> soc: qcom: ice: support for hardware wrapped keys
> ufs: core: support wrapped keys in ufs core
> ufs: host: wrapped keys support in ufs qcom
> qcom_scm: scm call for create, prepare and import keys
> ufs: core: add support for generate, import and prepare keys
> soc: qcom: support for generate, import and prepare key
> ufs: host: support for generate, import and prepare key
> arm64: dts: qcom: sm8650: add hwkm support to ufs ice
> dt-bindings: crypto: ice: document the hwkm property
>
> .../crypto/qcom,inline-crypto-engine.yaml | 7 +
> arch/arm64/boot/dts/qcom/sm8650.dtsi | 3 +-
> drivers/firmware/qcom/qcom_scm.c | 276 +++++++++++++++
> drivers/firmware/qcom/qcom_scm.h | 4 +
> drivers/mmc/host/cqhci-crypto.c | 7 +-
> drivers/mmc/host/cqhci.h | 2 +
> drivers/mmc/host/sdhci-msm.c | 6 +-
> drivers/soc/qcom/ice.c | 321 +++++++++++++++++-
> drivers/ufs/core/ufshcd-crypto.c | 87 ++++-
> drivers/ufs/host/ufs-qcom.c | 61 +++-
> include/linux/firmware/qcom/qcom_scm.h | 7 +
> include/soc/qcom/ice.h | 18 +-
> include/ufs/ufshcd.h | 22 ++
> 13 files changed, 784 insertions(+), 37 deletions(-)
>
^ permalink raw reply [flat|nested] 39+ messages in thread