On Thu, Apr 30, 2020 at 11:59:48AM +0000, Satya Tangirala wrote:
> The blk-crypto framework adds support for inline encryption. There
> are numerous changes throughout the storage stack. This patch documents
> the main design choices in the block layer, the API presented to users
> of the block layer (like fscrypt or layered devices) and the API presented
> to drivers for adding support for inline encryption.
>
> Signed-off-by: Satya Tangirala <satyat@xxxxxxxxxx>
Looks good, you can add:
Reviewed-by: Eric Biggers <ebiggers@xxxxxxxxxx>
But a few comments for when you resend:
> +When a bio is added to a request, the request takes over ownership of the
> +``bi_crypt_context`` of the bio - in particular, the request keeps the
> +``bi_crypt_context`` of the first bio in its bio-list, and frees the rest
> +(blk-mq needs to be careful to maintain this invariant during bio and request
> +merges).
Is this part up-to-date? There was discussion about not freeing the bios' crypt
contexts.
> +``blk_crypto_evict_key`` should be called by upper layers when they want
> +to ensure that a key is removed from memory and from any keyslots in inline
> +encryption hardware that the key might have been programmed into (or the
> +blk-crypto-fallback).
This should be reworded to emphasize that blk_crypto_evict_key()
*must* be called (as now the keyslot manager has a pointer to the key).
> +API presented to device drivers
> +===============================
> +
> +A :c:type:``struct keyslot_manager`` should be set up by device drivers in the
"keyslot_manager" => "blk_keyslot_manager". Likewise below.
> +``request_queue`` of the device. The device driver needs to call
> +``blk_ksm_init`` on the ``keyslot_manager``, which specfying the number of
> +keyslots supported by the hardware.
"which specfying" => "while specifying"
> +The device driver also needs to tell the KSM how to actually manipulate the
> +IE hardware in the device to do things like programming the crypto key into
> +the IE hardware into a particular keyslot. All this is achieved through the
> +:c:type:`struct keyslot_mgmt_ll_ops` field in the KSM that the device driver
> +must fill up after initing the ``keyslot_manager``.
"keyslot_mgmt_ll_ops" => "blk_ksm_ll_ops"
- Eric
