On Tue, Apr 12, 2022 at 06:18:46PM +0800, Herbert Xu wrote: > On Tue, Apr 12, 2022 at 11:02:54AM +0100, Catalin Marinas wrote: > > This series does not penalise any architecture. It doesn't even make > > arm64 any worse than it currently is. > > Right, the patch as it stands doesn't change anything. However, > it is also broken as it stands. As I said before, CRYPTO_MINALIGN > is not something that is guaranteed by the Crypto API, it is simply > a statement of whatever kmalloc returns. I agree that CRYPTO_MINALIGN is not guaranteed by the Crypto API. What I'm debating is the intended use for CRYPTO_MINALIGN in some (most?) of the drivers. It's not just about kmalloc() but also a build-time offset of buffers within structures to guarantee DMA safety. This can't be fixed by cra_alignmask. We could leave CRYPTO_MINALIGN as ARCH_KMALLOC_MINALIGN and that matches it being just a statement of the kmalloc() minimum alignment. But since it is also overloaded with the DMA in-structure offset alignment, we'd need a new CRYPTO_DMA_MINALIGN (and _ATTR) to annotate those structures. I have a suspicion there'll be fewer of the original CRYPTO_MINALIGN uses left, hence my approach to making this bigger from the start. There's also Ard's series introducing CRYPTO_REQ_MINALIGN while leaving CRYPT_MINALIGN for DMA-safe offsets (IIUC): https://lore.kernel.org/r/20220406142715.2270256-1-ardb@xxxxxxxxxx > So if kmalloc is no longer returning CRYPTO_MINALIGN-aligned > memory, then those drivers that need this alignment for DMA > will break anyway. No. As per one of my previous emails, kmalloc() will preserve the DMA alignment for an SoC even if smaller than CRYPTO_MINALIGN (or a new CRYPTO_DMA_MINALIGN). Since kmalloc() returns DMA-safe pointers and CRYPTO_MINALIGN (or a new CRYPTO_DMA_MINALIGN) is DMA-safe, so would an offset from a pointer returned by kmalloc(). > If you want the Crypto API to guarantee alignment over and above > that returned by kmalloc, the correct way is to use cra_alignmask. For kmalloc(), this would work, but for the current CRYPTO_MINALIGN_ATTR uses it won't. Thanks. -- Catalin
