| Message ID | 20180305103945.3517-1-horia.geanta@nxp.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Herbert Xu |
| Headers | show |
On Mon, Mar 05, 2018 at 12:39:45PM +0200, Horia Geantă wrote: > Even though it doesn't make too much sense, it is perfectly legal to: > - call .init() and then (as many times) .update() > - subseqently _not_ call any of .final(), .finup() or .export() Actually it makes perfect sense, because there can be an arbitrary number of requests for a given tfm. There is no requirement that you must finalise the first request before submitting new ones. IOW there can be an arbitrary number of outstanding requests even without the user intentionally abandoning any hash request. So please modify your commit description. Thanks,
On 3/16/2018 5:16 PM, Herbert Xu wrote: > On Mon, Mar 05, 2018 at 12:39:45PM +0200, Horia Geantă wrote: >> Even though it doesn't make too much sense, it is perfectly legal to: >> - call .init() and then (as many times) .update() >> - subseqently _not_ call any of .final(), .finup() or .export() > > Actually it makes perfect sense, because there can be an arbitrary > number of requests for a given tfm. There is no requirement that > you must finalise the first request before submitting new ones. > > IOW there can be an arbitrary number of outstanding requests even > without the user intentionally abandoning any hash request. > The fact that there can be multiple requests in parallel (for a given tfm) is a different topic. Each request object has its state in its own state machine, independent from the other request objects. I assume this is clear enough. Why I wanted to underline is that "abandoning" a hash request is allowed (even though doing this is at least questionable), thus implementations must take special care not to leak resources in this case. If you think the commit message should be updated, then probably so should the documentation update. Thanks, Horia
On Mon, Mar 19, 2018 at 06:39:50AM +0000, Horia Geantă wrote: > > The fact that there can be multiple requests in parallel (for a given tfm) is a > different topic. > Each request object has its state in its own state machine, independent from the > other request objects. > I assume this is clear enough. My point is that all of the state associated with a request needs to be stored in the request object. If you're start storing things in the driver/hardware, then things will get ugly one way or another. Cheers,
On 3/19/2018 11:25 AM, Herbert Xu wrote: > On Mon, Mar 19, 2018 at 06:39:50AM +0000, Horia Geantă wrote: >> >> The fact that there can be multiple requests in parallel (for a given tfm) is a >> different topic. >> Each request object has its state in its own state machine, independent from the >> other request objects. >> I assume this is clear enough. > > My point is that all of the state associated with a request needs > to be stored in the request object. If you're start storing things > in the driver/hardware, then things will get ugly one way or another. > Agree, the request state should be stored in the request object; I am not debating that. Still there are limitations even when keeping state in the request object. For e.g. an implementation cannot DMA map a buffer for the entire lifetime of a request object, because this lifetime is unknown - user can "abandon" the object after a few .update() calls, or even after .init(). By "abandon" I mean not call _ever_ any of .final(), .finup() or .export() on the object. The only solution to avoid leaks in this case is to repeatedly DMA map & unmap the buffer. IOW, if one wants to load/save HW state in a buffer after an .update() and to instruct the crypto engine to do this operation, the following steps are involved: -gpp: DMA map the buffer, get its IOVA -gpp: program the crypto engine with IOVA, wait for crypto engine's signal -crypto engine: load HW state from buffer, perform the partial hash, save HW state in buffer, signal gpp -gpp: DMA unmap the buffer I'd say this is pretty inefficient, yet I don't see an alternative. Good or bad, the documentation should reflect this limitation - hence this patch. Thanks, Horia
On Mon, Mar 19, 2018 at 11:04:24AM +0000, Horia Geantă wrote: > > The only solution to avoid leaks in this case is to repeatedly DMA map & unmap > the buffer. > IOW, if one wants to load/save HW state in a buffer after an .update() and to > instruct the crypto engine to do this operation, the following steps are involved: > -gpp: DMA map the buffer, get its IOVA > -gpp: program the crypto engine with IOVA, wait for crypto engine's signal > -crypto engine: load HW state from buffer, perform the partial hash, save HW > state in buffer, signal gpp > -gpp: DMA unmap the buffer What buffer are you talking about here? Is it the hash state? If it's the hash state and assuming DMA mapping was slow enough on your platform, you could solve it by maintaining a fixed set of hash states that are rotated through the actual hash requests. Let's say you allocate n such hash states which are always mapped, then if there are less than n hash requests outstanding, you could directly use the mapped hash states in the requests. If there are more than n, then you simply copy in/copy out for each hash operation. The number n limits how many operations can be pending to be processed by the hardware. Cheers,
diff --git a/Documentation/crypto/devel-algos.rst b/Documentation/crypto/devel-algos.rst index 66f50d32dcec..0f4617019227 100644 --- a/Documentation/crypto/devel-algos.rst +++ b/Documentation/crypto/devel-algos.rst @@ -236,6 +236,14 @@ when used from another part of the kernel. | '---------------> HASH2 +Note that it is perfectly legal to: +- call .init() and then (as many times) .update() +- subseqently _not_ call any of .final(), .finup() or .export() + +In other words mind the resource allocation and clean-up, +since this basically means no resources can remain allocated +after a call to .init() or .update(). + Specifics Of Asynchronous HASH Transformation ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Even though it doesn't make too much sense, it is perfectly legal to: - call .init() and then (as many times) .update() - subseqently _not_ call any of .final(), .finup() or .export() Update documentation since this is an important issue to consider from resource management perspective. Link: https://lkml.kernel.org/r/20180222114741.GA27631@gondor.apana.org.au Signed-off-by: Horia Geantă <horia.geanta@nxp.com> --- Documentation/crypto/devel-algos.rst | 8 ++++++++ 1 file changed, 8 insertions(+)