diff mbox series

[RFC,2/7] crypto: api - Add crypto_tfm_ctx_dma

Message ID E1noNhu-00BzV4-4N@fornost.hmeau.com
State New
Headers show
Series crypto: Add helpers for allocating with DMA alignment | expand

Commit Message

Herbert Xu May 10, 2022, 11:07 a.m. UTC
This patch adds the helper crypto_tfm_ctx_dma.  It's similar to
crypto_tfm_ctx_aligned but the alignment is hard-coded to the
macro ARCH_DMA_MINALIGN.

This patch also moves crypto_tfm_ctx into algapi.h.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---

 include/crypto/algapi.h |   28 ++++++++++++++++++++++++++--
 include/linux/crypto.h  |    5 -----
 2 files changed, 26 insertions(+), 7 deletions(-)

Comments

Catalin Marinas May 10, 2022, 5:10 p.m. UTC | #1
Hi Herbert,

Thanks for putting this together.

On Tue, May 10, 2022 at 07:07:10PM +0800, Herbert Xu wrote:
> diff --git a/include/crypto/algapi.h b/include/crypto/algapi.h
> index f50c5d1725da5..cdf12e51c53a0 100644
> --- a/include/crypto/algapi.h
> +++ b/include/crypto/algapi.h
> @@ -189,10 +189,34 @@ static inline void crypto_xor_cpy(u8 *dst, const u8 *src1, const u8 *src2,
>  	}
>  }
>  
> +static inline void *crypto_tfm_ctx(struct crypto_tfm *tfm)
> +{
> +	return tfm->__crt_ctx;
> +}
> +
> +static inline void *crypto_tfm_ctx_align(struct crypto_tfm *tfm,
> +					 unsigned int align)
> +{
> +	if (align <= crypto_tfm_ctx_alignment())
> +		align = 1;
> +
> +	return PTR_ALIGN(crypto_tfm_ctx(tfm), align);
> +}
> +
>  static inline void *crypto_tfm_ctx_aligned(struct crypto_tfm *tfm)
>  {
> -	return PTR_ALIGN(crypto_tfm_ctx(tfm),
> -			 crypto_tfm_alg_alignmask(tfm) + 1);
> +	return crypto_tfm_ctx_align(tfm, crypto_tfm_alg_alignmask(tfm) + 1);
> +}
> +
> +static inline void *crypto_tfm_ctx_dma(struct crypto_tfm *tfm)
> +{
> +	unsigned int align = 1;
> +
> +#ifdef ARCH_DMA_MINALIGN
> +	align = ARCH_DMA_MINALIGN;
> +#endif
> +
> +	return crypto_tfm_ctx_align(tfm, align);
>  }

Is there a case where a driver needs the minimum alignment between
ARCH_DMA_MINALIGN and crypto_tfm_alg_alignmask()+1? Maybe for platforms
where ARCH_DMA_MINALIGN is 8 (fully coherent) but the device's bus
master alignment requirements are higher.

My plan is to have ARCH_DMA_MINALIGN always defined but potentially
higher than ARCH_KMALLOC_MINALIGN on specific architectures. I think
crypto_tfm_ctx_dma() should use ARCH_KMALLOC_MINALIGN (and no #ifdefs)
until I get my patches sorted and I'll replace it with ARCH_DMA_MINALIGN
once it's defined globally (still no #ifdefs). Currently in mainline
it's ARCH_KMALLOC_MINALIGN that gives the static DMA alignment.

With the explicit crypto_tfm_ctx_dma(), can CRYPTO_MINALIGN_ATTR be
dropped entirely? This may be beneficial in reducing the structure size
when no DMA is required.
Herbert Xu May 12, 2022, 3:57 a.m. UTC | #2
On Tue, May 10, 2022 at 06:10:34PM +0100, Catalin Marinas wrote:
>
> Is there a case where a driver needs the minimum alignment between
> ARCH_DMA_MINALIGN and crypto_tfm_alg_alignmask()+1? Maybe for platforms
> where ARCH_DMA_MINALIGN is 8 (fully coherent) but the device's bus
> master alignment requirements are higher.

Yes, for example on x86 aesni requires 16-byte alignment.

> My plan is to have ARCH_DMA_MINALIGN always defined but potentially
> higher than ARCH_KMALLOC_MINALIGN on specific architectures. I think
> crypto_tfm_ctx_dma() should use ARCH_KMALLOC_MINALIGN (and no #ifdefs)
> until I get my patches sorted and I'll replace it with ARCH_DMA_MINALIGN
> once it's defined globally (still no #ifdefs). Currently in mainline
> it's ARCH_KMALLOC_MINALIGN that gives the static DMA alignment.
> 
> With the explicit crypto_tfm_ctx_dma(), can CRYPTO_MINALIGN_ATTR be
> dropped entirely? This may be beneficial in reducing the structure size
> when no DMA is required.

We always need CRYPTO_MINALIGN to reflect what alignment kmalloc
guarantees.  It is used to minimise the amount of extra padding
for users such aesni.

This shouldn't have any impact on your plans though as once the
drivers in question switch over to the DMA helpers you can safely
lower ARCH_KMALLOC_MINALIGN.

Cheers,
diff mbox series

Patch

diff --git a/include/crypto/algapi.h b/include/crypto/algapi.h
index f50c5d1725da5..cdf12e51c53a0 100644
--- a/include/crypto/algapi.h
+++ b/include/crypto/algapi.h
@@ -189,10 +189,34 @@  static inline void crypto_xor_cpy(u8 *dst, const u8 *src1, const u8 *src2,
 	}
 }
 
+static inline void *crypto_tfm_ctx(struct crypto_tfm *tfm)
+{
+	return tfm->__crt_ctx;
+}
+
+static inline void *crypto_tfm_ctx_align(struct crypto_tfm *tfm,
+					 unsigned int align)
+{
+	if (align <= crypto_tfm_ctx_alignment())
+		align = 1;
+
+	return PTR_ALIGN(crypto_tfm_ctx(tfm), align);
+}
+
 static inline void *crypto_tfm_ctx_aligned(struct crypto_tfm *tfm)
 {
-	return PTR_ALIGN(crypto_tfm_ctx(tfm),
-			 crypto_tfm_alg_alignmask(tfm) + 1);
+	return crypto_tfm_ctx_align(tfm, crypto_tfm_alg_alignmask(tfm) + 1);
+}
+
+static inline void *crypto_tfm_ctx_dma(struct crypto_tfm *tfm)
+{
+	unsigned int align = 1;
+
+#ifdef ARCH_DMA_MINALIGN
+	align = ARCH_DMA_MINALIGN;
+#endif
+
+	return crypto_tfm_ctx_align(tfm, align);
 }
 
 static inline struct crypto_instance *crypto_tfm_alg_instance(
diff --git a/include/linux/crypto.h b/include/linux/crypto.h
index 2324ab6f1846b..5d1e961f810ec 100644
--- a/include/linux/crypto.h
+++ b/include/linux/crypto.h
@@ -714,11 +714,6 @@  static inline void crypto_tfm_clear_flags(struct crypto_tfm *tfm, u32 flags)
 	tfm->crt_flags &= ~flags;
 }
 
-static inline void *crypto_tfm_ctx(struct crypto_tfm *tfm)
-{
-	return tfm->__crt_ctx;
-}
-
 static inline unsigned int crypto_tfm_ctx_alignment(void)
 {
 	struct crypto_tfm *tfm;