| Message ID | 20210121082155.111333-2-ebiggers@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Resource-managed blk_ksm_init() | expand |
On Thu, Jan 21, 2021 at 12:23 AM Eric Biggers <ebiggers@kernel.org> wrote: > > From: Eric Biggers <ebiggers@google.com> > > 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@google.com> > --- > 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@google.com>
On Mon, Jan 25, 2021 at 12:14:00PM -0800, Satya Tangirala wrote: > On Thu, Jan 21, 2021 at 12:23 AM Eric Biggers <ebiggers@kernel.org> wrote: > > > > From: Eric Biggers <ebiggers@google.com> > > > > 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@google.com> [..] > > 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); > > -- > > Looks good to me. Please feel free to add > Reviewed-by: Satya Tangirala <satyat@google.com> Thanks Satya. Jens, any objection to this patch going in through the MMC tree? - Eric
On 1/25/21 1:25 PM, Eric Biggers wrote: > On Mon, Jan 25, 2021 at 12:14:00PM -0800, Satya Tangirala wrote: >> On Thu, Jan 21, 2021 at 12:23 AM Eric Biggers <ebiggers@kernel.org> wrote: >>> >>> From: Eric Biggers <ebiggers@google.com> >>> >>> 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@google.com> > [..] >>> 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); >>> -- >> >> Looks good to me. Please feel free to add >> Reviewed-by: Satya Tangirala <satyat@google.com> > > Thanks Satya. Jens, any objection to this patch going in through the MMC tree? No objections from me, doesn't look like we have any real worries of conflicts.
On Mon, 25 Jan 2021 at 22:16, Jens Axboe <axboe@kernel.dk> wrote: > > On 1/25/21 1:25 PM, Eric Biggers wrote: > > On Mon, Jan 25, 2021 at 12:14:00PM -0800, Satya Tangirala wrote: > >> On Thu, Jan 21, 2021 at 12:23 AM Eric Biggers <ebiggers@kernel.org> wrote: > >>> > >>> From: Eric Biggers <ebiggers@google.com> > >>> > >>> 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@google.com> > > [..] > >>> 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); > >>> -- > >> > >> Looks good to me. Please feel free to add > >> Reviewed-by: Satya Tangirala <satyat@google.com> > > > > Thanks Satya. Jens, any objection to this patch going in through the MMC tree? > > No objections from me, doesn't look like we have any real worries of > conflicts. Applied to my mmc for the next branch, by adding Jens' ack. Thanks everybody! Kind regards Uffe
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);