Re: [PATCH v2 2/2] crypto: engine - Permit to enqueue skcipher request
On Wed, Aug 09, 2017 at 11:40:12AM +0200, Corentin Labbe wrote: > > I really didnt see how to do that since cra_type is const. > Anyway, I think it cannot be possible since we could have two different > engine with two different prepare_request(). > > I will really appreciate any advice on what you want exactly. The issue is that we have these handlers such as prepare_request that gives us a level of indirection into the driver. However, on top of those we're adding another level of indirection based on the type of request. I'd like to see these two combined into just a single function pointer. My idea of using cra_type is indeed broken as these functions ultimately live in the driver and not crypto API. So how about something like this: struct crypto_engine_op { int (*prepare_request)(struct crypto_engine *engine, struct crypto_async_request *req); int (*unprepare_request)(struct crypto_engine *engine, struct crypto_async_request *req); int (*do_one_request)(struct crypto_engine *engine, struct crypto_async_request *req); }; struct crypto_engine_reqctx { struct crypto_engine_op *op; }; struct crypto_engine { struct crypto_engine_op aead; struct crypto_engine_op hash; struct crypto_engine_op cipher; ... }; Then in the request_to_engine functions you would store the right op pointer inside the request context area. Obviously the users of crypto_engine would need to allocate space for the struct crypto_engine_reqctx in their reqctx structure. The only wart with this scheme is that the drivers end up seeing struct crypto_async_request and will need to convert that to the respective request types but I couldn't really find a better way. Cheers, -- Email: Herbert XuHome Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH v2 2/2] crypto: engine - Permit to enqueue skcipher request
On Fri, Jul 28, 2017 at 05:01:19PM +0200, Corentin Labbe wrote: > On Fri, Jul 28, 2017 at 09:52:57PM +0800, Herbert Xu wrote: > > On Fri, Jul 14, 2017 at 01:15:36PM +0200, Corentin Labbe wrote: > > > On Fri, Jun 23, 2017 at 02:48:37PM +0800, Herbert Xu wrote: > > > > On Mon, Jun 19, 2017 at 09:55:24AM +0200, Corentin Labbe wrote: > > > > > > > > > > Since there are two different user of "crypto engine + ablkcipher", > > > > > it will be not easy to convert them in one serie. (I could do it, but > > > > > I simply could not test it for OMAP (lack of hw)) > > > > > And any new user which want to use crypto engine+skcipher (like me > > > > > with the sun8i-ce driver) are simply stuck. > > > > > > > > You're right. We'll need to do this in a backwards-compatible way. In > > > > fact > > > > we already do something similar in skcipher.c itself. Simply look at > > > > the > > > > cra_type field and if it matches blkcipher/ablkcipher/givcipher then > > > > it's > > > > legacy ablkcipher, otherwise it's skcipher. > > > > > > > > Also the way crypto_engine looks at the request type in the data-path is > > > > suboptimal. This should really be built into the cra_type object. For > > > > example, we can have cra_type->engine->prepare_request which would just > > > > do the right thing. > > > > > > > > > > Not sure to have well understand what you want. > > > You want that I switch on cra_type instead of crypto_tfm_alg_type() ? > > > > No I mean that we should have an engine hooks object registered > > under cra_type so that you simply call > > > > cra_type->engine->prepare_request() > > > > regardless of what type you're using. > > > > I am sorry, I didnt see how to do that. Hello I really didnt see how to do that since cra_type is const. Anyway, I think it cannot be possible since we could have two different engine with two different prepare_request(). I will really appreciate any advice on what you want exactly. Regards
Re: [PATCH v2 2/2] crypto: engine - Permit to enqueue skcipher request
On Fri, Jul 28, 2017 at 09:52:57PM +0800, Herbert Xu wrote: > On Fri, Jul 14, 2017 at 01:15:36PM +0200, Corentin Labbe wrote: > > On Fri, Jun 23, 2017 at 02:48:37PM +0800, Herbert Xu wrote: > > > On Mon, Jun 19, 2017 at 09:55:24AM +0200, Corentin Labbe wrote: > > > > > > > > Since there are two different user of "crypto engine + ablkcipher", it > > > > will be not easy to convert them in one serie. (I could do it, but I > > > > simply could not test it for OMAP (lack of hw)) > > > > And any new user which want to use crypto engine+skcipher (like me with > > > > the sun8i-ce driver) are simply stuck. > > > > > > You're right. We'll need to do this in a backwards-compatible way. In > > > fact > > > we already do something similar in skcipher.c itself. Simply look at the > > > cra_type field and if it matches blkcipher/ablkcipher/givcipher then it's > > > legacy ablkcipher, otherwise it's skcipher. > > > > > > Also the way crypto_engine looks at the request type in the data-path is > > > suboptimal. This should really be built into the cra_type object. For > > > example, we can have cra_type->engine->prepare_request which would just > > > do the right thing. > > > > > > > Not sure to have well understand what you want. > > You want that I switch on cra_type instead of crypto_tfm_alg_type() ? > > No I mean that we should have an engine hooks object registered > under cra_type so that you simply call > > cra_type->engine->prepare_request() > > regardless of what type you're using. > I am sorry, I didnt see how to do that.
Re: [PATCH v2 2/2] crypto: engine - Permit to enqueue skcipher request
On Fri, Jul 14, 2017 at 01:15:36PM +0200, Corentin Labbe wrote: > On Fri, Jun 23, 2017 at 02:48:37PM +0800, Herbert Xu wrote: > > On Mon, Jun 19, 2017 at 09:55:24AM +0200, Corentin Labbe wrote: > > > > > > Since there are two different user of "crypto engine + ablkcipher", it > > > will be not easy to convert them in one serie. (I could do it, but I > > > simply could not test it for OMAP (lack of hw)) > > > And any new user which want to use crypto engine+skcipher (like me with > > > the sun8i-ce driver) are simply stuck. > > > > You're right. We'll need to do this in a backwards-compatible way. In fact > > we already do something similar in skcipher.c itself. Simply look at the > > cra_type field and if it matches blkcipher/ablkcipher/givcipher then it's > > legacy ablkcipher, otherwise it's skcipher. > > > > Also the way crypto_engine looks at the request type in the data-path is > > suboptimal. This should really be built into the cra_type object. For > > example, we can have cra_type->engine->prepare_request which would just > > do the right thing. > > > > Not sure to have well understand what you want. > You want that I switch on cra_type instead of crypto_tfm_alg_type() ? No I mean that we should have an engine hooks object registered under cra_type so that you simply call cra_type->engine->prepare_request() regardless of what type you're using. Cheers, -- Email: Herbert XuHome Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH v2 2/2] crypto: engine - Permit to enqueue skcipher request
On Fri, Jun 23, 2017 at 02:48:37PM +0800, Herbert Xu wrote: > On Mon, Jun 19, 2017 at 09:55:24AM +0200, Corentin Labbe wrote: > > > > Since there are two different user of "crypto engine + ablkcipher", it will > > be not easy to convert them in one serie. (I could do it, but I simply > > could not test it for OMAP (lack of hw)) > > And any new user which want to use crypto engine+skcipher (like me with the > > sun8i-ce driver) are simply stuck. > > You're right. We'll need to do this in a backwards-compatible way. In fact > we already do something similar in skcipher.c itself. Simply look at the > cra_type field and if it matches blkcipher/ablkcipher/givcipher then it's > legacy ablkcipher, otherwise it's skcipher. > > Also the way crypto_engine looks at the request type in the data-path is > suboptimal. This should really be built into the cra_type object. For > example, we can have cra_type->engine->prepare_request which would just > do the right thing. > Not sure to have well understand what you want. You want that I switch on cra_type instead of crypto_tfm_alg_type() ?
Re: [PATCH v2 2/2] crypto: engine - Permit to enqueue skcipher request
On Mon, Jun 19, 2017 at 09:55:24AM +0200, Corentin Labbe wrote: > > Since there are two different user of "crypto engine + ablkcipher", it will > be not easy to convert them in one serie. (I could do it, but I simply could > not test it for OMAP (lack of hw)) > And any new user which want to use crypto engine+skcipher (like me with the > sun8i-ce driver) are simply stuck. You're right. We'll need to do this in a backwards-compatible way. In fact we already do something similar in skcipher.c itself. Simply look at the cra_type field and if it matches blkcipher/ablkcipher/givcipher then it's legacy ablkcipher, otherwise it's skcipher. Also the way crypto_engine looks at the request type in the data-path is suboptimal. This should really be built into the cra_type object. For example, we can have cra_type->engine->prepare_request which would just do the right thing. Thanks, -- Email: Herbert XuHome Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH v2 2/2] crypto: engine - Permit to enqueue skcipher request
On Mon, Jun 19, 2017 at 01:27:08PM +0800, Herbert Xu wrote: > On Tue, Jun 06, 2017 at 03:44:17PM +0200, Corentin Labbe wrote: > > The crypto engine could actually only enqueue hash and ablkcipher request. > > This patch permit it to enqueue skcipher requets by adding all necessary > > functions. > > The only problem is that ablkcipher and skcipher id are the same, so > > only one cipher type is usable on the same crypto engine. > > > > Signed-off-by: Corentin Labbe> > I think this should be done as part of the skcipher conversion rather > than as a standalone patch. > Since there are two different user of "crypto engine + ablkcipher", it will be not easy to convert them in one serie. (I could do it, but I simply could not test it for OMAP (lack of hw)) And any new user which want to use crypto engine+skcipher (like me with the sun8i-ce driver) are simply stuck. Regards
Re: [PATCH v2 2/2] crypto: engine - Permit to enqueue skcipher request
On Tue, Jun 06, 2017 at 03:44:17PM +0200, Corentin Labbe wrote: > The crypto engine could actually only enqueue hash and ablkcipher request. > This patch permit it to enqueue skcipher requets by adding all necessary > functions. > The only problem is that ablkcipher and skcipher id are the same, so > only one cipher type is usable on the same crypto engine. > > Signed-off-by: Corentin LabbeI think this should be done as part of the skcipher conversion rather than as a standalone patch. Thanks, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH v2 2/2] crypto: engine - Permit to enqueue skcipher request
On Tue, Jun 06, 2017 at 04:07:25PM +0200, Stephan Müller wrote: > Am Dienstag, 6. Juni 2017, 15:44:17 CEST schrieb Corentin Labbe: > > Hi Corentin, > > > The crypto engine could actually only enqueue hash and ablkcipher request. > > This patch permit it to enqueue skcipher requets by adding all necessary > > functions. > > The only problem is that ablkcipher and skcipher id are the same, so > > only one cipher type is usable on the same crypto engine. > > Why do you think this is needed? I thought that skcipher is the successor to > ablkcipher AND blkcipher. I.e. ablkcipher and blkcipher should be phased out. > Thus, I would infer that any ablkcipher support should or could be dropped. > There are two user of it, crypto/omap and crypto/virtio. And for crypto/omap, I have no way of testing any convertion to skcipher (no hardware). Regards Corentin Labbe
Re: [PATCH v2 2/2] crypto: engine - Permit to enqueue skcipher request
Am Dienstag, 6. Juni 2017, 15:44:17 CEST schrieb Corentin Labbe: Hi Corentin, > The crypto engine could actually only enqueue hash and ablkcipher request. > This patch permit it to enqueue skcipher requets by adding all necessary > functions. > The only problem is that ablkcipher and skcipher id are the same, so > only one cipher type is usable on the same crypto engine. Why do you think this is needed? I thought that skcipher is the successor to ablkcipher AND blkcipher. I.e. ablkcipher and blkcipher should be phased out. Thus, I would infer that any ablkcipher support should or could be dropped. Ciao Stephan
[PATCH v2 2/2] crypto: engine - Permit to enqueue skcipher request
The crypto engine could actually only enqueue hash and ablkcipher request. This patch permit it to enqueue skcipher requets by adding all necessary functions. The only problem is that ablkcipher and skcipher id are the same, so only one cipher type is usable on the same crypto engine. Signed-off-by: Corentin Labbe--- crypto/crypto_engine.c | 138 include/crypto/engine.h | 14 + 2 files changed, 141 insertions(+), 11 deletions(-) diff --git a/crypto/crypto_engine.c b/crypto/crypto_engine.c index 61e7c4e02fd2..fc0f58b6299c 100644 --- a/crypto/crypto_engine.c +++ b/crypto/crypto_engine.c @@ -36,6 +36,7 @@ static void crypto_pump_requests(struct crypto_engine *engine, struct crypto_async_request *async_req, *backlog; struct ahash_request *hreq; struct ablkcipher_request *breq; + struct skcipher_request *skreq; unsigned long flags; bool was_busy = false; int ret, rtype; @@ -123,17 +124,34 @@ static void crypto_pump_requests(struct crypto_engine *engine, } return; case CRYPTO_ALG_TYPE_ABLKCIPHER: - breq = ablkcipher_request_cast(engine->cur_req); - if (engine->prepare_cipher_request) { - ret = engine->prepare_cipher_request(engine, breq); - if (ret) { - dev_err(engine->dev, "failed to prepare request: %d\n", - ret); - goto req_err; + /* since CRYPTO_ALG_TYPE_ABLKCIPHER == CRYPTO_ALG_TYPE_SKCIPHER +* differenciate both with the presence of cipher_one_request +*/ + if (engine->cipher_one_request) { + breq = ablkcipher_request_cast(engine->cur_req); + if (engine->prepare_cipher_request) { + ret = engine->prepare_cipher_request(engine, breq); + if (ret) { + dev_err(engine->dev, "failed to prepare request: %d\n", + ret); + goto req_err; + } + engine->cur_req_prepared = true; } - engine->cur_req_prepared = true; + ret = engine->cipher_one_request(engine, breq); + } else { + skreq = skcipher_request_cast(engine->cur_req); + if (engine->prepare_skcipher_request) { + ret = engine->prepare_skcipher_request(engine, skreq); + if (ret) { + dev_err(engine->dev, "failed to prepare request: %d\n", + ret); + goto req_err; + } + engine->cur_req_prepared = true; + } + ret = engine->skcipher_one_request(engine, skreq); } - ret = engine->cipher_one_request(engine, breq); if (ret) { dev_err(engine->dev, "failed to cipher one request from queue\n"); goto req_err; @@ -151,8 +169,13 @@ static void crypto_pump_requests(struct crypto_engine *engine, crypto_finalize_hash_request(engine, hreq, ret); break; case CRYPTO_ALG_TYPE_ABLKCIPHER: - breq = ablkcipher_request_cast(engine->cur_req); - crypto_finalize_cipher_request(engine, breq, ret); + if (engine->cipher_one_request) { + breq = ablkcipher_request_cast(engine->cur_req); + crypto_finalize_cipher_request(engine, breq, ret); + } else { + skreq = skcipher_request_cast(engine->cur_req); + crypto_finalize_skcipher_request(engine, skreq, ret); + } break; } return; @@ -213,6 +236,49 @@ int crypto_transfer_cipher_request_to_engine(struct crypto_engine *engine, EXPORT_SYMBOL_GPL(crypto_transfer_cipher_request_to_engine); /** + * crypto_transfer_skcipher_request - transfer the new request into the + * enginequeue + * @engine: the hardware engine + * @req: the request need to be listed into the engine queue + */ +int crypto_transfer_skcipher_request(struct crypto_engine *engine, +struct skcipher_request *req, +bool need_pump) +{ + unsigned long flags; + int ret; + + spin_lock_irqsave(>queue_lock, flags); + + if (!engine->running) { + spin_unlock_irqrestore(>queue_lock, flags); + return