mbox series

[0/5] crypto: caam - avoid allocating memory at crypto request runtime

Message ID 20201203013524.30495-1-iuliana.prodan@oss.nxp.com
Headers show
Series crypto: caam - avoid allocating memory at crypto request runtime | expand

Message

Iuliana Prodan (OSS) Dec. 3, 2020, 1:35 a.m. UTC
From: Iuliana Prodan <iuliana.prodan@nxp.com>

This series removes CRYPTO_ALG_ALLOCATES_MEMORY flag and
allocates the memory needed by the driver, to fulfil a
request, within the crypto request object.
The extra size needed for base extended descriptor, hw
descriptor commands and link tables is added to the reqsize
field that indicates how much memory could be needed per request.

CRYPTO_ALG_ALLOCATES_MEMORY flag is limited only to
dm-crypt use-cases, which seems to be 4 entries maximum.
Therefore in reqsize we allocate memory for maximum 4 entries
for src and 4 for dst, aligned.
If the driver needs more than the 4 entries maximum, the memory
is dynamically allocated, at runtime.

Iuliana Prodan (5):
  crypto: caam/jr - avoid allocating memory at crypto request runtime
    for skcipher
  crypto: caam/jr - avoid allocating memory at crypto request runtime
    for aead
  crypto: caam/jr - avoid allocating memory at crypto request runtime
    fost hash
  crypto: caam/qi - avoid allocating memory at crypto request runtime
  crypto: caam/qi2 - avoid allocating memory at crypto request runtime

 drivers/crypto/caam/caamalg.c     | 141 +++++++---
 drivers/crypto/caam/caamalg_qi.c  | 134 ++++++----
 drivers/crypto/caam/caamalg_qi2.c | 415 ++++++++++++++++++++----------
 drivers/crypto/caam/caamalg_qi2.h |   6 +
 drivers/crypto/caam/caamhash.c    |  77 ++++--
 5 files changed, 538 insertions(+), 235 deletions(-)

Comments

Ard Biesheuvel Dec. 3, 2020, 8:34 a.m. UTC | #1
On Thu, 3 Dec 2020 at 02:35, Iuliana Prodan (OSS)
<iuliana.prodan@oss.nxp.com> wrote:
>
> From: Iuliana Prodan <iuliana.prodan@nxp.com>
>
> This series removes CRYPTO_ALG_ALLOCATES_MEMORY flag and
> allocates the memory needed by the driver, to fulfil a
> request, within the crypto request object.
> The extra size needed for base extended descriptor, hw
> descriptor commands and link tables is added to the reqsize
> field that indicates how much memory could be needed per request.
>
> CRYPTO_ALG_ALLOCATES_MEMORY flag is limited only to
> dm-crypt use-cases, which seems to be 4 entries maximum.
> Therefore in reqsize we allocate memory for maximum 4 entries
> for src and 4 for dst, aligned.
> If the driver needs more than the 4 entries maximum, the memory
> is dynamically allocated, at runtime.
>

I'm confused. So the driver does allocate memory in some cases, right?
So why is it justified to remove CRYPTO_ALG_ALLOCATES_MEMORY?

> Iuliana Prodan (5):
>   crypto: caam/jr - avoid allocating memory at crypto request runtime
>     for skcipher
>   crypto: caam/jr - avoid allocating memory at crypto request runtime
>     for aead
>   crypto: caam/jr - avoid allocating memory at crypto request runtime
>     fost hash
>   crypto: caam/qi - avoid allocating memory at crypto request runtime
>   crypto: caam/qi2 - avoid allocating memory at crypto request runtime
>
>  drivers/crypto/caam/caamalg.c     | 141 +++++++---
>  drivers/crypto/caam/caamalg_qi.c  | 134 ++++++----
>  drivers/crypto/caam/caamalg_qi2.c | 415 ++++++++++++++++++++----------
>  drivers/crypto/caam/caamalg_qi2.h |   6 +
>  drivers/crypto/caam/caamhash.c    |  77 ++++--
>  5 files changed, 538 insertions(+), 235 deletions(-)
>
> --
> 2.17.1
>
Horia Geanta Dec. 10, 2020, 8:28 a.m. UTC | #2
On 12/3/2020 3:35 AM, Iuliana Prodan (OSS) wrote:
> From: Iuliana Prodan <iuliana.prodan@nxp.com>

> 

> This series removes CRYPTO_ALG_ALLOCATES_MEMORY flag and

> allocates the memory needed by the driver, to fulfil a

> request, within the crypto request object.

> The extra size needed for base extended descriptor, hw

> descriptor commands and link tables is added to the reqsize

> field that indicates how much memory could be needed per request.

> 

> CRYPTO_ALG_ALLOCATES_MEMORY flag is limited only to

> dm-crypt use-cases, which seems to be 4 entries maximum.

> Therefore in reqsize we allocate memory for maximum 4 entries

> for src and 4 for dst, aligned.

> If the driver needs more than the 4 entries maximum, the memory

> is dynamically allocated, at runtime.

> 

Moving the memory allocations from caam driver into the generic crypto API
has the side effect of dropping the GFP_DMA allocation flag.

For cases when caam device is limited to 32-bit address space and
there's no IOMMU, this could lead to DMA API using bounce buffering.

We need to measure the performance impact of this change before deciding
what we should do next.

Thanks,
Horia
Herbert Xu Dec. 11, 2020, 10:09 a.m. UTC | #3
On Thu, Dec 10, 2020 at 10:28:35AM +0200, Horia Geantă wrote:
>

> Moving the memory allocations from caam driver into the generic crypto API

> has the side effect of dropping the GFP_DMA allocation flag.

> 

> For cases when caam device is limited to 32-bit address space and

> there's no IOMMU, this could lead to DMA API using bounce buffering.

> 

> We need to measure the performance impact of this change before deciding

> what we should do next.


This only applies to the control data, right? The actual data
being operated on surely is the most important factor.

In any case, did you respond to Ard's concern about potentially
exhausting DMA memory?

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt