| Message ID | 20220406142715.2270256-8-ardb@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | crypto: avoid DMA padding for request structures | expand |
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 --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(
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(-)