On Wed, Nov 03, 2021 at 04:18:40PM -0700, Gaurav Kashyap wrote: > Add support for wrapped keys in ufs and common ICE library. > Qualcomm's ICE solution uses a hardware block called Hardware > Key Manager (HWKM) to handle wrapped keys. > > This patch adds the following changes to support this. > 1. Link to HWKM library for initialization. > 2. Most of the key management is done from Trustzone via scm calls. > Added calls to this from the ICE library. > 3. Added support for this framework in UFS. > 4. Added support for deriving SW secret as it cannot be done in > linux kernel for wrapped keys. > > Signed-off-by: Gaurav Kashyap <quic_gaurkash@xxxxxxxxxxx> > --- > drivers/scsi/ufs/ufs-qcom-ice.c | 34 +++++++++- > drivers/scsi/ufs/ufs-qcom.c | 1 + > drivers/scsi/ufs/ufs-qcom.h | 5 ++ > drivers/scsi/ufs/ufshcd-crypto.c | 47 ++++++++++--- > drivers/scsi/ufs/ufshcd.h | 5 ++ > drivers/soc/qcom/qti-ice-common.c | 108 ++++++++++++++++++++++++++---- > include/linux/qti-ice-common.h | 7 +- > 7 files changed, 180 insertions(+), 27 deletions(-) > > diff --git a/drivers/scsi/ufs/ufs-qcom-ice.c b/drivers/scsi/ufs/ufs-qcom-ice.c > index 6608a9015eab..79d642190997 100644 > --- a/drivers/scsi/ufs/ufs-qcom-ice.c > +++ b/drivers/scsi/ufs/ufs-qcom-ice.c > @@ -45,6 +45,21 @@ int ufs_qcom_ice_init(struct ufs_qcom_host *host) > } > mmio.ice_mmio = host->ice_mmio; > > +#if IS_ENABLED(CONFIG_QTI_HW_WRAPPED_KEYS) > + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ice_hwkm"); > + if (!res) { > + dev_warn(dev, "ICE HWKM registers not found\n"); > + goto disable; > + } > + > + host->ice_hwkm_mmio = devm_ioremap_resource(dev, res); > + if (IS_ERR(host->ice_hwkm_mmio)) { > + err = PTR_ERR(host->ice_hwkm_mmio); > + dev_err(dev, "Failed to map ICE registers; err=%d\n", err); > + return err; > + } > + mmio.ice_hwkm_mmio = host->ice_hwkm_mmio; > +#endif The driver shouldn't completely disable ICE support just because HW wrapped keys aren't supported by the hardware or by the device tree file. Instead, it should declare support for standard keys only. I.e. CONFIG_QTI_HW_WRAPPED_KEYS shouldn't force the use of HW wrapped keys, it should just add support for them. > diff --git a/drivers/scsi/ufs/ufshcd-crypto.c b/drivers/scsi/ufs/ufshcd-crypto.c > index 0ed82741f981..965a8cc6c183 100644 > --- a/drivers/scsi/ufs/ufshcd-crypto.c > +++ b/drivers/scsi/ufs/ufshcd-crypto.c ufshcd-crypto.c is part of ufshcd-core, not ufs_qcom. It should be changed in a separate patch. > @@ -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 *key, > 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, false); > > if (hba->vops && hba->vops->program_key) { > - err = hba->vops->program_key(hba, cfg, slot); > + err = hba->vops->program_key(hba, key, cfg, slot); > goto out; > } vops->program_key shouldn't take in both a key and a cfg. It should be just one or the other. 'cfg' doesn't appear to work for HW wrapped keys, and it seems the existing user doesn't really need a 'cfg' in the first place, so it would have to be just 'key'. > +#if IS_ENABLED(CONFIG_QTI_HW_WRAPPED_KEYS) As noted above, ufshcd-crypto isn't specific to ufs_qcom. It therefore must not contain references to CONFIG_QTI_HW_WRAPPED_KEYS, as that kconfig option is specific to Qualcomm platforms. > +static int ufshcd_crypto_derive_sw_secret(struct blk_crypto_profile *profile, > + const u8 *wrapped_key, > + unsigned int wrapped_key_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_secret) > + return hba->vops->derive_secret(wrapped_key, > + wrapped_key_size, sw_secret); > + > + return 0; > +} The fallback case should return -EOPNOTSUPP, which indicates that the operation is not supported, rather than 0 which indicates that it succeeded. > @@ -190,7 +213,11 @@ 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; > +#if IS_ENABLED(CONFIG_QTI_HW_WRAPPED_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; > +#endif > hba->crypto_profile.dev = hba->dev; My comments from above apply to this too. Checking a Qualcomm-specific kconfig option isn't appropriate here. Also the supported key types shouldn't be static from the kconfig; they should be determined by the actual hardware capabilities. Note that in the Android kernels, for the division of work between ufshcd-core and host drivers, we ended up going with a solution where the UFS host drivers can just override the whole blk_crypto_profile (previously called blk_keyslot_manager). You may have to do the same, although it would be preferable to find a way to share more code. Also, at runtime, does any of the Qualcomm hardware support multiple key types, and if so can they be used at the same time? - Eric
![]() |