Re: [PATCH v4 01/15] ice, ufs, mmc: use blk_crypto_key for program_key

Ulf Hansson
2024-02-13T12:49:39Z

On Sun, 28 Jan 2024 at 00:26, Gaurav Kashyap <quic_gaurkash@quicinc.com> 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>
> Tested-by: Neil Armstrong <neil.armstrong@linaro.org>

I assume this is better funneled via some other tree than the MMC, so:

Acked-by: Ulf Hansson <ulf.hansson@linaro.org> # For MMC

Kind regards
Uffe

> ---
> 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..529ea9f4fa07 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 *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 39eef470f8fa..acf352594362 100644
> --- a/drivers/ufs/host/ufs-qcom.c
> +++ b/drivers/ufs/host/ufs-qcom.c
> @@ -144,6 +144,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)
> {
> @@ -151,6 +152,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];
> @@ -158,11 +160,11 @@ static int ufs_qcom_ice_program_key(struct ufs_hba *hba,
> cap.key_size != UFS_CRYPTO_KEY_SIZE_256)
> return -EOPNOTSUPP;
>
> + 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 cb2afcebbdf5..582d5a747e84 100644
> --- a/include/ufs/ufshcd.h
> +++ b/include/ufs/ufshcd.h
> @@ -363,6 +363,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.43.0
>