Re: [PATCH v3 11/13] crypto: Use ARCH_DMA_MINALIGN instead of ARCH_KMALLOC_MINALIGN

[Catalin Marinas <catalin.marinas@arm.com>]

On Mon, Nov 07, 2022 at 05:12:53PM +0800, Herbert Xu wrote:
> On Mon, Nov 07, 2022 at 09:05:04AM +0000, Catalin Marinas wrote:
> > Well, it does ensure that the __alignof__ and sizeof structures like
> > crypto_alg and aead_request is still 128 after this change. A kmalloc()
> > of a size multiple of 128 returns a 128-byte aligned object. So the aim
> > is just to keep the current binary layout/alignment to 128 on arm64. In
> > theory, no functional change.
> 
> Changing CRYPTO_MINALIGN to 128 does not cause structures that are
> smaller than 128 bytes to magically become larger than 128 bytes.

For structures, it does (not arrays though):

#define __aligned(x)	__attribute__((__aligned__(x)))

struct align_test1 {
	char c;
	char __aligned(128) data[];
};

struct align_test2 {
	char c;
} __aligned(128);

char aligned_array[4] __aligned(128);

With the above, we have:

sizeof(align_test1) == 128; __alignof__(align_test1) == 128;
sizeof(align_test2) == 128; __alignof__(align_test2) == 128;
sizeof(align_array) == 4;   __alignof__(align_array) == 128;

> If you're set on doing it this way then I can proceed with the
> original patch-set to change the drivers.  I've just been putting
> it off because it seems that you guys weren't quite decided on
> which way to go.

Yes, reviving your patchset would help and that can be done
independently of this series as long as the crypto code starts using
dma_get_cache_alignment() and drops CRYPTO_MINALIGN_ATTR entirely. If at
the point of creating the mask the code knows whether the device is
coherent, it can even avoid any additional alignment (though still
honouring the cra_alignmask that a device requires). So such reworking
would be beneficial irrespective of this series.

It seems that swiotlb bouncing is the preferred route and least
intrusive but let's see the feedback on the other parts of the series.

Thanks.

-- 
Catalin