Re: [PATCH v6 6/9] scsi: ufs: Add inline encryption support to UFS

Christoph Hellwig
2020-01-17T13:58:09Z

On Wed, Dec 18, 2019 at 06:51:33AM -0800, Satya Tangirala wrote:
> Wire up ufshcd.c with the UFS Crypto API, the block layer inline
> encryption additions and the keyslot manager.

I think this patch should be merged into the previous patch, as the
previous one isn't useful without wiring it up.

> +int ufshcd_prepare_lrbp_crypto(struct ufs_hba *hba,
> + struct scsi_cmnd *cmd,
> + struct ufshcd_lrb *lrbp)
> +{
> + struct bio_crypt_ctx *bc;
> +
> + if (!bio_crypt_should_process(cmd->request)) {
> + lrbp->crypto_enable = false;
> + return 0;
> + }

I think this check belongs into the caller so that there is no function
call overhead for the !inline crypto case. If you extend the
conditional
to

if (!IS_ENABLED(CONFIG_SCSI_UFS_CRYPTO) ||
!bio_crypt_should_process(cmd->request))

you also don't need the stub for ufshcd_prepare_lrbp_crypto.

> +EXPORT_SYMBOL_GPL(ufshcd_prepare_lrbp_crypto);

No need to export this function.

> + if (ufshcd_lrbp_crypto_enabled(lrbp)) {
> +#if IS_ENABLED(CONFIG_SCSI_UFS_CRYPTO)
> + dword_0 |= UTP_REQ_DESC_CRYPTO_ENABLE_CMD;
> + dword_0 |= lrbp->crypto_key_slot;
> + req_desc->header.dword_1 =
> + cpu_to_le32(lower_32_bits(lrbp->data_unit_num));
> + req_desc->header.dword_3 =
> + cpu_to_le32(upper_32_bits(lrbp->data_unit_num));
> +#endif /* CONFIG_SCSI_UFS_CRYPTO */

This can be a plain old ifdef without the IS_ENABLED obsfucation.

> +#if IS_ENABLED(CONFIG_SCSI_UFS_CRYPTO)
> + lrbp->crypto_enable = false; /* No crypto operations */
> +#endif

Same here.

> +#if IS_ENABLED(CONFIG_SCSI_UFS_CRYPTO)
> + bool crypto_enable;
> + u8 crypto_key_slot;
> + u64 data_unit_num;
> +#endif /* CONFIG_SCSI_UFS_CRYPTO */

.. and here.