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

Christoph Hellwig
2019-10-31T18:26:31Z

> /* Transfer request descriptor header fields */
> + if (lrbp->crypto_enable) {

Maybe we want a little inline function so that we can use IS_ENABLED
to make sure the compiler eliminates the dead code if crypt config
option is not set.

a) don't have to define the crypto_enable if the config options are
not set

> + dword_0 |= UTP_REQ_DESC_CRYPTO_ENABLE_CMD;
> + dword_0 |= lrbp->crypto_key_slot;
> + req_desc->header.dword_1 =
> + cpu_to_le32((u32)lrbp->data_unit_num);
> + req_desc->header.dword_3 =
> + cpu_to_le32((u32)(lrbp->data_unit_num >> 32));

This should use ther upper_32_bits / lower_32_bits helpers.

> +static inline int ufshcd_prepare_lrbp_crypto(struct ufs_hba *hba,
> + struct scsi_cmnd *cmd,
> + struct ufshcd_lrb *lrbp)
> +{
> + int key_slot;
> +
> + if (!cmd->request->bio ||
> + !bio_crypt_should_process(cmd->request->bio, cmd->request->q)) {
> + lrbp->crypto_enable = false;
> + return 0;
> + }
> +
> + if (WARN_ON(!ufshcd_is_crypto_enabled(hba))) {
> + /*
> + * Upper layer asked us to do inline encryption
> + * but that isn't enabled, so we fail this request.
> + */
> + return -EINVAL;
> + }
> + key_slot = bio_crypt_get_keyslot(cmd->request->bio);
> + if (!ufshcd_keyslot_valid(hba, key_slot))
> + return -EINVAL;
> +
> + lrbp->crypto_enable = true;
> + lrbp->crypto_key_slot = key_slot;
> + lrbp->data_unit_num = bio_crypt_data_unit_num(cmd->request->bio);
> +
> + return 0;

I think this should go into ufshcd-crypto.c so that it can be stubbed
out for non-crypto builds. That also means we can remove various
stubs for the block layer helpers and just dereference the fields
directly, helping with code readability.