On Mon, Jul 06, 2020 at 07:10:16PM +0000, Satya Tangirala wrote:
> +/**
> + * ufshcd_init_crypto - Initialize crypto hardware
> + * @hba: Per adapter instance
> + */
> +void ufshcd_init_crypto(struct ufs_hba *hba)
> +{
> + int slot = 0;
> +
> + if (!(hba->caps & UFSHCD_CAP_CRYPTO))
> + return;
> +
> + /* Clear all keyslots - the number of keyslots is (CFGC + 1) */
> + for (slot = 0; slot < hba->crypto_capabilities.config_count + 1; slot++)
> + ufshcd_clear_keyslot(hba, slot);
> +}
This should just use 'int slot;'. No need to initialize to 0.
> diff --git a/drivers/scsi/ufs/ufshcd-crypto.h b/drivers/scsi/ufs/ufshcd-crypto.h
> index 22677619de59..2512aef03f76 100644
> --- a/drivers/scsi/ufs/ufshcd-crypto.h
> +++ b/drivers/scsi/ufs/ufshcd-crypto.h
> @@ -10,9 +10,36 @@
> #include "ufshcd.h"
> #include "ufshci.h"
>
> +static inline void ufshcd_prepare_lrbp_crypto(struct ufs_hba *hba,
> + struct request *rq,
> + struct ufshcd_lrb *lrbp)
> +{
> + if (!rq || !rq->crypt_keyslot) {
> + lrbp->crypto_key_slot = -1;
> + return;
> + }
> +
> + lrbp->crypto_key_slot = blk_ksm_get_slot_idx(rq->crypt_keyslot);
> + lrbp->data_unit_num = rq->crypt_ctx->bc_dun[0];
> +}
This function doesn't use the 'hba' argument, so it should be removed.
> +
> +static inline void
> +ufshcd_prepare_req_desc_hdr_crypto(struct ufshcd_lrb *lrbp, u32 *dword_0,
> + u32 *dword_1, u32 *dword_3)
> +{
> + if (lrbp->crypto_key_slot >= 0) {
> + *dword_0 |= UTP_REQ_DESC_CRYPTO_ENABLE_CMD;
> + *dword_0 |= lrbp->crypto_key_slot;
> + *dword_1 = lower_32_bits(lrbp->data_unit_num);
> + *dword_3 = upper_32_bits(lrbp->data_unit_num);
> + }
> +}
> +
> bool ufshcd_crypto_enable(struct ufs_hba *hba);
>
> -int ufshcd_hba_init_crypto(struct ufs_hba *hba);
> +void ufshcd_init_crypto(struct ufs_hba *hba);
> +
> +int ufshcd_hba_init_crypto_capabilities(struct ufs_hba *hba);
The .c file defines ufshcd_hba_init_crypto_capabilities() before
ufshcd_init_crypto(). It would be nice to put the declarations in the same
order.
> @@ -2554,6 +2577,7 @@ static int ufshcd_compose_dev_cmd(struct ufs_hba *hba,
> lrbp->task_tag = tag;
> lrbp->lun = 0; /* device management cmd is not specific to any LUN */
> lrbp->intr_cmd = true; /* No interrupt aggregation */
> + ufshcd_prepare_lrbp_crypto(hba, NULL, lrbp);
> hba->dev_cmd.type = cmd_type;
>
> return ufshcd_comp_devman_upiu(hba, lrbp);
> @@ -4650,6 +4674,8 @@ static int ufshcd_slave_configure(struct scsi_device *sdev)
> if (ufshcd_is_rpm_autosuspend_allowed(hba))
> sdev->rpm_autosuspend = 1;
>
> + ufshcd_crypto_setup_rq_keyslot_manager(hba, q);
> +
> return 0;
> }
>
> @@ -6115,6 +6141,9 @@ static int ufshcd_issue_devman_upiu_cmd(struct ufs_hba *hba,
> lrbp->task_tag = tag;
> lrbp->lun = 0;
> lrbp->intr_cmd = true;
> +#ifdef CONFIG_SCSI_UFS_CRYPTO
> + lrbp->crypto_key_slot = -1; /* No crypto operations */
> +#endif
This should use ufshcd_prepare_lrbp_crypto(hba, NULL, lrbp),
like ufshcd_compose_dev_cmd() does.
- Eric
