[PATCH] uboot-lantiq: fix out of bounds cache invalidate

[Daniel Schwierzeck]

Am Dienstag, dem 02.11.2021 um 23:35 +0100 schrieb Mathias Kresin:
> If u-boot is compiled with gcc 10+ the relevant part of the memory is
> used as following:
> 
> variable BootFile is at address: 0x83ffe4ec
> variable data is at address:     0x83ffdee2
> variable data has len:           0x600
> 
> Within invalidate_dcache_range() the cache covering the data variable
> should be invalidated (0x83ffdee2 till 0x83ffdef2).
> 
> Due to dcache line size of 32 and some alignment correction, the
> actual
> memory range for which the cache is invalidated is from 0x83ffdee0
> till
> 0x83ffe500, which includes the location of the BootFile variable as
> well.
> 
> Afterwards the BootFile variable is 0. The issue can be observed by
> using tftpboot, which always uses the default/generated filename
> instead
> of the specified one.
> 
> To fix the issue, align the memory addresses properly before passing
> them to invalidate_dcache_range(). The misalignment can be observed
> with
> older gcc versions as well but the BootFile variable is placed more
> than
> 0x1000 bytes after the data variable and doesn't get null'ed.
> 
> Fixes: FS#4113
> 
> Signed-off-by: Mathias Kresin <dev at kresin.me>
> ---
>  ...q-fix-out-of-bounds-cache-invalidate.patch | 48
> +++++++++++++++++++
>  1 file changed, 48 insertions(+)
>  create mode 100644 package/boot/uboot-lantiq/patches/0031-dma-
> lantiq-fix-out-of-bounds-cache-invalidate.patch
> 
> diff --git a/package/boot/uboot-lantiq/patches/0031-dma-lantiq-fix-
> out-of-bounds-cache-invalidate.patch b/package/boot/uboot-
> lantiq/patches/0031-dma-lantiq-fix-out-of-bounds-cache-
> invalidate.patch
> new file mode 100644
> index 0000000000..5a2bf39441
> --- /dev/null
> +++ b/package/boot/uboot-lantiq/patches/0031-dma-lantiq-fix-out-of-
> bounds-cache-invalidate.patch
> @@ -0,0 +1,48 @@
> +From f10ffb65865d5bfa5679293b13e027e0474df1fe Mon Sep 17 00:00:00
> 2001
> +From: Mathias Kresin <dev at kresin.me>
> +Date: Tue, 2 Nov 2021 21:24:29 +0100
> +Subject: [PATCH] dma: lantiq: fix out of bounds cache invalidate
> +
> +If u-boot is compiled with gcc 10+ the relevant part of the memory
> is
> +used as following:
> +
> +variable BootFile is at address: 0x83ffe4ec
> +variable data is at address:     0x83ffdee2
> +variable data has len:           0x600
> +
> +Within invalidate_dcache_range() the cache covering the data
> variable
> +should be invalidated (0x83ffdee2 till 0x83ffdef2).
> +
> +Due to dcache line size of 32 and some alignment correction, the
> actual
> +memory range for which the cache is invalidated is from 0x83ffdee0
> till
> +0x83ffe500, which includes the location of the BootFile variable as
> +well.
> +
> +Afterwards the BootFile variable is 0. The issue can be observed by
> +using tftpboot, which always uses the default/generated filename
> instead
> +of the specified one.
> +
> +To fix the issue, align the memory addresses properly before passing
> +them to invalidate_dcache_range(). The misalignment can be observed
> with
> +older gcc versions as well but the BootFile variable is placed more
> than
> +0x1000 bytes after the data variable and doesn't get null'ed.
> +
> +Signed-off-by: Mathias Kresin <dev at kresin.me>
> +---
> + drivers/dma/lantiq_dma.c | 4 ++--
> + 1 file changed, 2 insertions(+), 2 deletions(-)
> +
> +--- a/drivers/dma/lantiq_dma.c
> ++++ b/drivers/dma/lantiq_dma.c
> +@@ -122,9 +122,9 @@ static inline void ltq_dma_dcache_wb_inv
> + 
> + static inline void ltq_dma_dcache_inv(const void *ptr, size_t size)
> + {
> +-	unsigned long addr = (unsigned long) ptr;
> ++	unsigned long addr = (unsigned long) ptr & ~(ARCH_DMA_MINALIGN
> - 1);
> + 
> +-	invalidate_dcache_range(addr, addr + size);
> ++	invalidate_dcache_range(addr, ALIGN(addr + size,
> ARCH_DMA_MINALIGN));
> + }

actually invalidate_dcache_range() already aligns start and end address
to CONFIG_SYS_CACHELINE_SIZE which is the same as ARCH_DMA_MINALIGN.

In mainline we fixed the cache functions to always issue a sync() after
the cache ops to wait for all HW transactions to be complete. The
Lantiq DMA driver only does this in ltq_dma_dcache_wb_inv() but not
in ltq_dma_dcache_inv(). Maybe adding a ltq_dma_sync() there would also
fix your problem?

> + 
> + void ltq_dma_init(void)
-- 
- Daniel