On Thu, Jan 21, 2021 at 12:23 AM Eric Biggers <ebiggers@xxxxxxxxxx> wrote:
>
> From: Eric Biggers <ebiggers@xxxxxxxxxx>
>
> Add a resource-managed variant of blk_ksm_init() so that drivers don't
> have to worry about calling blk_ksm_destroy().
>
> Note that the implementation uses a custom devres action to call
> blk_ksm_destroy() rather than switching the two allocations to be
> directly devres-managed, e.g. with devm_kmalloc(). This is because we
> need to keep zeroing the memory containing the keyslots when it is
> freed, and also because we want to continue using kvmalloc() (and there
> is no devm_kvmalloc()).
>
> Signed-off-by: Eric Biggers <ebiggers@xxxxxxxxxx>
> ---
> Documentation/block/inline-encryption.rst | 12 +++++-----
> block/keyslot-manager.c | 29 +++++++++++++++++++++++
> include/linux/keyslot-manager.h | 3 +++
> 3 files changed, 38 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/block/inline-encryption.rst b/Documentation/block/inline-encryption.rst
> index e75151e467d39..7f9b40d6b416b 100644
> --- a/Documentation/block/inline-encryption.rst
> +++ b/Documentation/block/inline-encryption.rst
> @@ -182,8 +182,9 @@ API presented to device drivers
>
> A :c:type:``struct blk_keyslot_manager`` should be set up by device drivers in
> the ``request_queue`` of the device. The device driver needs to call
> -``blk_ksm_init`` on the ``blk_keyslot_manager``, which specifying the number of
> -keyslots supported by the hardware.
> +``blk_ksm_init`` (or its resource-managed variant ``devm_blk_ksm_init``) on the
> +``blk_keyslot_manager``, while specifying the number of keyslots supported by
> +the hardware.
>
> 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
> @@ -202,10 +203,9 @@ needs each and every of its keyslots to be reprogrammed with the key it
> "should have" at the point in time when the function is called. This is useful
> e.g. if a device loses all its keys on runtime power down/up.
>
> -``blk_ksm_destroy`` should be called to free up all resources used by a keyslot
> -manager upon ``blk_ksm_init``, once the ``blk_keyslot_manager`` is no longer
> -needed.
> -
> +If the driver used ``blk_ksm_init`` instead of ``devm_blk_ksm_init``, then
> +``blk_ksm_destroy`` should be called to free up all resources used by a
> +``blk_keyslot_manager`` once it is no longer needed.
>
> Layered Devices
> ===============
> diff --git a/block/keyslot-manager.c b/block/keyslot-manager.c
> index 86f8195d8039e..324bf4244f5fb 100644
> --- a/block/keyslot-manager.c
> +++ b/block/keyslot-manager.c
> @@ -29,6 +29,7 @@
> #define pr_fmt(fmt) "blk-crypto: " fmt
>
> #include <linux/keyslot-manager.h>
> +#include <linux/device.h>
> #include <linux/atomic.h>
> #include <linux/mutex.h>
> #include <linux/pm_runtime.h>
> @@ -127,6 +128,34 @@ int blk_ksm_init(struct blk_keyslot_manager *ksm, unsigned int num_slots)
> }
> EXPORT_SYMBOL_GPL(blk_ksm_init);
>
> +static void blk_ksm_destroy_callback(void *ksm)
> +{
> + blk_ksm_destroy(ksm);
> +}
> +
> +/**
> + * devm_blk_ksm_init() - Resource-managed blk_ksm_init()
> + * @dev: The device which owns the blk_keyslot_manager.
> + * @ksm: The blk_keyslot_manager to initialize.
> + * @num_slots: The number of key slots to manage.
> + *
> + * Like blk_ksm_init(), but causes blk_ksm_destroy() to be called automatically
> + * on driver detach.
> + *
> + * Return: 0 on success, or else a negative error code.
> + */
> +int devm_blk_ksm_init(struct device *dev, struct blk_keyslot_manager *ksm,
> + unsigned int num_slots)
> +{
> + int err = blk_ksm_init(ksm, num_slots);
> +
> + if (err)
> + return err;
> +
> + return devm_add_action_or_reset(dev, blk_ksm_destroy_callback, ksm);
> +}
> +EXPORT_SYMBOL_GPL(devm_blk_ksm_init);
> +
> static inline struct hlist_head *
> blk_ksm_hash_bucket_for_key(struct blk_keyslot_manager *ksm,
> const struct blk_crypto_key *key)
> diff --git a/include/linux/keyslot-manager.h b/include/linux/keyslot-manager.h
> index 18f3f5346843f..443ad817c6c57 100644
> --- a/include/linux/keyslot-manager.h
> +++ b/include/linux/keyslot-manager.h
> @@ -85,6 +85,9 @@ struct blk_keyslot_manager {
>
> int blk_ksm_init(struct blk_keyslot_manager *ksm, unsigned int num_slots);
>
> +int devm_blk_ksm_init(struct device *dev, struct blk_keyslot_manager *ksm,
> + unsigned int num_slots);
> +
> blk_status_t blk_ksm_get_slot_for_key(struct blk_keyslot_manager *ksm,
> const struct blk_crypto_key *key,
> struct blk_ksm_keyslot **slot_ptr);
> --
> 2.30.0
Looks good to me. Please feel free to add
Reviewed-by: Satya Tangirala <satyat@xxxxxxxxxx>
