mbox series

[RFC,00/10] crypto: engine: permit to batch requests

Message ID 20200114135936.32422-1-clabbe.montjoie@gmail.com (mailing list archive)
Headers show
Series crypto: engine: permit to batch requests | expand

Message

Corentin Labbe Jan. 14, 2020, 1:59 p.m. UTC
Hello

The sun8i-ce hardware can work on multiple requests in one batch.
For this it use a task descriptor, and chain them.
For the moment, the driver does not use this mechanism and do requests
one at a time and issue an irq for each.

Using the chaning will permit to issue less interrupts, and increase
thoughput.

But the crypto/engine can enqueue lots of requests but can ran them only
one by one.

This serie introduce a way to batch requests in crypto/engine by
- setting a batch limit (1 by default)
- refactor the prepare/unprepare code to permit to have x requests
  prepared/unprepared at the same time.

For testing the serie, the selftest are not enough, since it issue
request one at a time.
I have used LUKS for testing it.

Please give me what you think about this serie, specially maintainers
which have hardware with the same kind of capability.

Regards

Corentin Labbe (10):
  crypto: sun8i-ce: move iv data to request context
  crypto: sun8i-ce: increase task list size
  crypto: sun8i-ce: split into prepare/run/unprepare
  crypto: sun8i-ce: introduce the slot number
  crypto: engine: transform cur_req in an array
  crypto: engine: introduce ct
  crypto: sun8i-ce: handle slot > 0
  crypto: engine: add slot parameter
  crypto: engine: permit to batch requests
  crypto: sun8i-ce: use the new batch mechanism

 crypto/crypto_engine.c                        |  76 +++++++----
 .../allwinner/sun8i-ce/sun8i-ce-cipher.c      | 121 +++++++++++++-----
 .../crypto/allwinner/sun8i-ce/sun8i-ce-core.c |  17 ++-
 drivers/crypto/allwinner/sun8i-ce/sun8i-ce.h  |  17 ++-
 drivers/crypto/omap-aes-gcm.c                 |   2 +-
 drivers/crypto/omap-aes.c                     |   4 +-
 drivers/crypto/omap-des.c                     |   4 +-
 drivers/crypto/stm32/stm32-cryp.c             |   8 +-
 drivers/crypto/stm32/stm32-hash.c             |   4 +-
 include/crypto/engine.h                       |  27 +++-
 10 files changed, 201 insertions(+), 79 deletions(-)

Comments

Iuliana Prodan Jan. 16, 2020, 11:33 a.m. UTC | #1
On 1/14/2020 3:59 PM, Corentin Labbe wrote:
> Hello
> 
> The sun8i-ce hardware can work on multiple requests in one batch.
> For this it use a task descriptor, and chain them.
> For the moment, the driver does not use this mechanism and do requests
> one at a time and issue an irq for each.
> 
> Using the chaning will permit to issue less interrupts, and increase
> thoughput.
> 
> But the crypto/engine can enqueue lots of requests but can ran them only
> one by one.
> 
> This serie introduce a way to batch requests in crypto/engine by
> - setting a batch limit (1 by default)
> - refactor the prepare/unprepare code to permit to have x requests
>    prepared/unprepared at the same time.
> 
> For testing the serie, the selftest are not enough, since it issue
> request one at a time.
> I have used LUKS for testing it.
> 
> Please give me what you think about this serie, specially maintainers
> which have hardware with the same kind of capability.
> 
Hi,

I'm working on CAAM, on adding support for crypto-engine.
These modifications are not working on CAAM.
They seem to be specific to requests that are linked. CAAM can work on 
multiple request, at the same time, but they are processed independently.
So, I believe the parallelization is a good idea, but the requests still 
need to be independent.
I'll follow up with comments on each patch.

Also, IMO you should send the patches for crypto-engine improvements in 
a separate series from the one for allwinner driver.

Thanks,
Iulia

> Regards
> 
> Corentin Labbe (10):
>    crypto: sun8i-ce: move iv data to request context
>    crypto: sun8i-ce: increase task list size
>    crypto: sun8i-ce: split into prepare/run/unprepare
>    crypto: sun8i-ce: introduce the slot number
>    crypto: engine: transform cur_req in an array
>    crypto: engine: introduce ct
>    crypto: sun8i-ce: handle slot > 0
>    crypto: engine: add slot parameter
>    crypto: engine: permit to batch requests
>    crypto: sun8i-ce: use the new batch mechanism
> 
>   crypto/crypto_engine.c                        |  76 +++++++----
>   .../allwinner/sun8i-ce/sun8i-ce-cipher.c      | 121 +++++++++++++-----
>   .../crypto/allwinner/sun8i-ce/sun8i-ce-core.c |  17 ++-
>   drivers/crypto/allwinner/sun8i-ce/sun8i-ce.h  |  17 ++-
>   drivers/crypto/omap-aes-gcm.c                 |   2 +-
>   drivers/crypto/omap-aes.c                     |   4 +-
>   drivers/crypto/omap-des.c                     |   4 +-
>   drivers/crypto/stm32/stm32-cryp.c             |   8 +-
>   drivers/crypto/stm32/stm32-hash.c             |   4 +-
>   include/crypto/engine.h                       |  27 +++-
>   10 files changed, 201 insertions(+), 79 deletions(-)
>
Corentin Labbe Jan. 16, 2020, 1:16 p.m. UTC | #2
On Thu, Jan 16, 2020 at 11:33:24AM +0000, Iuliana Prodan wrote:
> On 1/14/2020 3:59 PM, Corentin Labbe wrote:
> > Hello
> > 
> > The sun8i-ce hardware can work on multiple requests in one batch.
> > For this it use a task descriptor, and chain them.
> > For the moment, the driver does not use this mechanism and do requests
> > one at a time and issue an irq for each.
> > 
> > Using the chaning will permit to issue less interrupts, and increase
> > thoughput.
> > 
> > But the crypto/engine can enqueue lots of requests but can ran them only
> > one by one.
> > 
> > This serie introduce a way to batch requests in crypto/engine by
> > - setting a batch limit (1 by default)
> > - refactor the prepare/unprepare code to permit to have x requests
> >    prepared/unprepared at the same time.
> > 
> > For testing the serie, the selftest are not enough, since it issue
> > request one at a time.
> > I have used LUKS for testing it.
> > 
> > Please give me what you think about this serie, specially maintainers
> > which have hardware with the same kind of capability.
> > 
> Hi,
> 
> I'm working on CAAM, on adding support for crypto-engine.
> These modifications are not working on CAAM.
> They seem to be specific to requests that are linked. CAAM can work on 
> multiple request, at the same time, but they are processed independently.
> So, I believe the parallelization is a good idea, but the requests still 
> need to be independent.
> I'll follow up with comments on each patch.

Hello

Thanks for the review.
Yes my serie is for doing "linked" request.
For the CAAM, if you can do multiple request independently, why not having x crypto engine ? (like sun8i-ce/sun8i-ss/amlogic)

> 
> Also, IMO you should send the patches for crypto-engine improvements in 
> a separate series from the one for allwinner driver.

For this RFC serie, I tried to do real atomic patch, for let people see the whole process.

Regards