diff mbox series

[7/8] crypto: ahash - avoid DMA alignment for request structures unless needed

Message ID 20220406142715.2270256-8-ardb@kernel.org (mailing list archive)
State Changes Requested
Delegated to: Herbert Xu
Headers show
Series crypto: avoid DMA padding for request structures | expand

Commit Message

Ard Biesheuvel April 6, 2022, 2:27 p.m. UTC
AHASH request structures are currently aligned to minimal DMA alignment
for the arcitecture, which defaults to 128 bytes on arm64. This is
excessive, and rarely needed, i.e., only when doing non-coherent inbound
DMA on the contents of the request context buffer. So let's relax this
requirement, and only use this alignment if the
CRYPTO_ALG_NEED_DMA_ALIGNMENT flag is set by the implementation.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 include/crypto/hash.h          |  5 +++--
 include/crypto/internal/hash.h | 10 +++++++++-
 2 files changed, 12 insertions(+), 3 deletions(-)

Comments

kernel test robot April 6, 2022, 8:05 p.m. UTC | #1
Hi Ard,

I love your patch! Perhaps something to improve:

[auto build test WARNING on herbert-cryptodev-2.6/master]
[also build test WARNING on herbert-crypto-2.6/master sunxi/sunxi/for-next v5.18-rc1 next-20220406]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Ard-Biesheuvel/crypto-avoid-DMA-padding-for-request-structures/20220407-010855
base:   https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git master
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20220407/202204070345.cL3xZHZm-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/072f0a01b9f98cd545d8179b1591b9f37be5cb2a
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Ard-Biesheuvel/crypto-avoid-DMA-padding-for-request-structures/20220407-010855
        git checkout 072f0a01b9f98cd545d8179b1591b9f37be5cb2a
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=m68k SHELL=/bin/bash drivers/crypto/inside-secure/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from include/vdso/const.h:5,
                    from include/linux/const.h:4,
                    from include/linux/list.h:9,
                    from include/linux/preempt.h:11,
                    from arch/m68k/include/asm/irqflags.h:6,
                    from include/linux/irqflags.h:16,
                    from arch/m68k/include/asm/atomic.h:6,
                    from include/linux/atomic.h:7,
                    from include/linux/crypto.h:15,
                    from include/crypto/aes.h:10,
                    from drivers/crypto/inside-secure/safexcel_hash.c:8:
   In function 'ahash_request_ctx',
       inlined from 'safexcel_ahash_exit_inv' at drivers/crypto/inside-secure/safexcel_hash.c:627:36:
>> include/crypto/hash.h:421:26: warning: '*(struct ahash_request *)(&__req_desc[0]).base.tfm' is used uninitialized [-Wuninitialized]
     421 |                          crypto_tfm_alg_req_alignmask(req->base.tfm) + 1);
         |                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/uapi/linux/const.h:32:50: note: in definition of macro '__ALIGN_KERNEL_MASK'
      32 | #define __ALIGN_KERNEL_MASK(x, mask)    (((x) + (mask)) & ~(mask))
         |                                                  ^~~~
   include/linux/align.h:8:33: note: in expansion of macro '__ALIGN_KERNEL'
       8 | #define ALIGN(x, a)             __ALIGN_KERNEL((x), (a))
         |                                 ^~~~~~~~~~~~~~
   include/linux/align.h:11:45: note: in expansion of macro 'ALIGN'
      11 | #define PTR_ALIGN(p, a)         ((typeof(p))ALIGN((unsigned long)(p), (a)))
         |                                             ^~~~~
   include/crypto/hash.h:420:16: note: in expansion of macro 'PTR_ALIGN'
     420 |         return PTR_ALIGN(&req->__ctx,
         |                ^~~~~~~~~
   In file included from drivers/crypto/inside-secure/safexcel_hash.c:21:
   drivers/crypto/inside-secure/safexcel_hash.c: In function 'safexcel_ahash_exit_inv':
   drivers/crypto/inside-secure/safexcel.h:69:14: note: '__req_desc' declared here
      69 |         char __##name##_desc[size] CRYPTO_MINALIGN_ATTR; \
         |              ^~
   drivers/crypto/inside-secure/safexcel_hash.c:626:9: note: in expansion of macro 'EIP197_REQUEST_ON_STACK'
     626 |         EIP197_REQUEST_ON_STACK(req, ahash, EIP197_AHASH_REQ_SIZE);
         |         ^~~~~~~~~~~~~~~~~~~~~~~


vim +421 include/crypto/hash.h

   417	
   418	static inline void *ahash_request_ctx(struct ahash_request *req)
   419	{
   420		return PTR_ALIGN(&req->__ctx,
 > 421				 crypto_tfm_alg_req_alignmask(req->base.tfm) + 1);
   422	}
   423
diff mbox series

Patch

diff --git a/include/crypto/hash.h b/include/crypto/hash.h
index f140e4643949..cd16c37c38af 100644
--- a/include/crypto/hash.h
+++ b/include/crypto/hash.h
@@ -56,7 +56,7 @@  struct ahash_request {
 	/* This field may only be used by the ahash API code. */
 	void *priv;
 
-	void *__ctx[] CRYPTO_MINALIGN_ATTR;
+	void *__ctx[] CRYPTO_REQ_MINALIGN_ATTR;
 };
 
 /**
@@ -417,7 +417,8 @@  static inline unsigned int crypto_ahash_reqsize(struct crypto_ahash *tfm)
 
 static inline void *ahash_request_ctx(struct ahash_request *req)
 {
-	return req->__ctx;
+	return PTR_ALIGN(&req->__ctx,
+			 crypto_tfm_alg_req_alignmask(req->base.tfm) + 1);
 }
 
 /**
diff --git a/include/crypto/internal/hash.h b/include/crypto/internal/hash.h
index 25806141db59..222c2df009c6 100644
--- a/include/crypto/internal/hash.h
+++ b/include/crypto/internal/hash.h
@@ -143,7 +143,15 @@  static inline struct ahash_alg *__crypto_ahash_alg(struct crypto_alg *alg)
 static inline void crypto_ahash_set_reqsize(struct crypto_ahash *tfm,
 					    unsigned int reqsize)
 {
-	tfm->reqsize = reqsize;
+	unsigned int align = crypto_tfm_alg_req_alignmask(crypto_ahash_tfm(tfm)) + 1;
+
+	/*
+	 * The request structure itself is only aligned to CRYPTO_REQ_MINALIGN,
+	 * so we need to add some headroom, allowing us to return a suitably
+	 * aligned context buffer pointer. We also need to round up the size so
+	 * we don't end up sharing a cacheline at the end of the buffer.
+	 */
+	tfm->reqsize = ALIGN(reqsize, align) + align - CRYPTO_REQ_MINALIGN;
 }
 
 static inline struct crypto_instance *ahash_crypto_instance(