[PATCH v2 0/2] Fix boot failure due to misaligned DMA buffer

[PATCH v2 0/2] Fix boot failure due to misaligned DMA buffer

[Nam Cao]

Hi,

We observed the following sporadic boot failure while booting from MMC
device:

=> boot
CACHE: Misaligned operation at range [9efa25f8, 9efa27f8]
CACHE: Misaligned operation at range [9efa25f8, 9efa27f8]
CACHE: Misaligned operation at range [9efa25f8, 9efa27f8]
CACHE: Misaligned operation at range [9efa25f8, 9efa27f8]
** Booting bootflow 'mmc@2194000.bootdev.part_1' with extlinux
Ignoring unknown command: �D���D��
Boot failed (err=-14)

The reason is because while allocating buffer to read a file from MMC,
alignment of 1 byte is used. Thus, the buffer doesn't work for performing
DMA, and garbage data is read.

While looking at this issue, I also noticed that if no alignment specified
(align=0) then fs_read_alloc() is documented to use the default. But the
default is no alignment. Therefore, other users of fs_read_alloc() which
specify align=0 may be broken as well.

The first patch changes extlinux_read_bootflow() to use proper buffer
alignment for DMA.

The second patch changes the default alignment of fs_read_alloc() to be
DMA-suitable, to fix other potential bugs.

Nam Cao (2):
  boot: extlinux: Fix unaligned buffer for reading data from file system
  fs: Use ARCH_DMA_MINALIGN as default alignment for fs_read_alloc()

 boot/bootmeth_extlinux.c | 3 ++-
 fs/fs.c                  | 4 ++++
 include/fs.h             | 2 +-
 3 files changed, 7 insertions(+), 2 deletions(-)

-- 
2.39.5

[PATCH v2 1/2] boot: extlinux: Fix unaligned buffer for reading data from file system

[Nam Cao]

extlinux_read_bootflow() allocates a buffer to read from file system
without any alignment.

But for some block devices which transfer data via DMA, ARCH_DMA_MINALIGN
alignment is required. For example, due to misaligned buffer, the below
boot failure is observed.

=> boot
CACHE: Misaligned operation at range [9efa25f8, 9efa27f8]
CACHE: Misaligned operation at range [9efa25f8, 9efa27f8]
CACHE: Misaligned operation at range [9efa25f8, 9efa27f8]
CACHE: Misaligned operation at range [9efa25f8, 9efa27f8]
** Booting bootflow 'mmc@2194000.bootdev.part_1' with extlinux
Ignoring unknown command: �D���D��
Boot failed (err=-14)

Change the buffer alignment to ARCH_DMA_MINALIGN.

Fixes: 31aefaf89a5b ("bootstd: Add an implementation of distro boot")
Signed-off-by: Nam Cao <namcao@linutronix.de>
Tested-by: Javier Fernandez Pastrana <javier.pastrana@linutronix.de>
---
v2:
  - use ARCH_DMA_MINALIGN to be consistent with bootmeth_script [Simon]

 boot/bootmeth_extlinux.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/boot/bootmeth_extlinux.c b/boot/bootmeth_extlinux.c
index ae0ad1d53e3..ac046705033 100644
--- a/boot/bootmeth_extlinux.c
+++ b/boot/bootmeth_extlinux.c
@@ -8,6 +8,7 @@
 
 #define LOG_CATEGORY UCLASS_BOOTSTD
 
+#include <asm/cache.h>
 #include <common.h>
 #include <bootdev.h>
 #include <bootflow.h>
@@ -127,7 +128,7 @@ static int extlinux_read_bootflow(struct udevice *dev, struct bootflow *bflow)
 		return log_msg_ret("try", ret);
 	size = bflow->size;
 
-	ret = bootmeth_alloc_file(bflow, 0x10000, 1);
+	ret = bootmeth_alloc_file(bflow, 0x10000, ARCH_DMA_MINALIGN);
 	if (ret)
 		return log_msg_ret("read", ret);
 
-- 
2.39.5

Re: [PATCH v2 1/2] boot: extlinux: Fix unaligned buffer for reading data from file system

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1047 bytes --]

On Thu, Nov 07, 2024 at 04:01:05PM +0100, Nam Cao wrote:

> extlinux_read_bootflow() allocates a buffer to read from file system
> without any alignment.
> 
> But for some block devices which transfer data via DMA, ARCH_DMA_MINALIGN
> alignment is required. For example, due to misaligned buffer, the below
> boot failure is observed.
> 
> => boot
> CACHE: Misaligned operation at range [9efa25f8, 9efa27f8]
> CACHE: Misaligned operation at range [9efa25f8, 9efa27f8]
> CACHE: Misaligned operation at range [9efa25f8, 9efa27f8]
> CACHE: Misaligned operation at range [9efa25f8, 9efa27f8]
> ** Booting bootflow 'mmc@2194000.bootdev.part_1' with extlinux
> Ignoring unknown command: �D���D��
> Boot failed (err=-14)
> 
> Change the buffer alignment to ARCH_DMA_MINALIGN.
> 
> Fixes: 31aefaf89a5b ("bootstd: Add an implementation of distro boot")
> Signed-off-by: Nam Cao <namcao@linutronix.de>
> Tested-by: Javier Fernandez Pastrana <javier.pastrana@linutronix.de>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

[PATCH v2 2/2] fs: Use ARCH_DMA_MINALIGN as default alignment for fs_read_alloc()

[Nam Cao]

The comment above fs_read_alloc() explains:

    @align: Alignment to use for memory allocation (0 for default)

However, in the actual implementation, there is no alignment when @align is
zero.

This current default is probably fine for most cases. But for some block
devices which transfer data via DMA, ARCH_DMA_MINALIGN is needed.

Change the default alignment to ARCH_DMA_MINALIGN.

Fixes: de7b5a8a1ac0 ("fs: Create functions to load and allocate a file")
Signed-off-by: Nam Cao <namcao@linutronix.de>
Tested-by: Javier Fernandez Pastrana <javier.pastrana@linutronix.de>
---
v2:
  - mention the default value in function comment [Simon]

 fs/fs.c      | 4 ++++
 include/fs.h | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/fs/fs.c b/fs/fs.c
index acf465bdd80..bfa51c89d40 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -23,6 +23,7 @@
 #include <semihostingfs.h>
 #include <ubifs_uboot.h>
 #include <btrfs.h>
+#include <asm/cache.h>
 #include <asm/global_data.h>
 #include <asm/io.h>
 #include <div64.h>
@@ -1001,6 +1002,9 @@ int fs_read_alloc(const char *fname, ulong size, uint align, void **bufp)
 	char *buf;
 	int ret;
 
+	if (!align)
+		align = ARCH_DMA_MINALIGN;
+
 	buf = memalign(align, size + 1);
 	if (!buf)
 		return log_msg_ret("buf", -ENOMEM);
diff --git a/include/fs.h b/include/fs.h
index ef540e7c23d..2f879968d0c 100644
--- a/include/fs.h
+++ b/include/fs.h
@@ -309,7 +309,7 @@ int do_fs_types(struct cmd_tbl *cmdtp, int flag, int argc, char * const argv[]);
  *
  * @fname: Filename to read
  * @size: Size of file to read (must be correct!)
- * @align: Alignment to use for memory allocation (0 for default)
+ * @align: Alignment to use for memory allocation (0 for default: ARCH_DMA_MINALIGN)
  * @bufp: On success, returns the allocated buffer with the nul-terminated file
  *	in it
  * Return: 0 if OK, -ENOMEM if out of memory, -EIO if read failed
-- 
2.39.5

Re: [PATCH v2 2/2] fs: Use ARCH_DMA_MINALIGN as default alignment for fs_read_alloc()

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 730 bytes --]

On Thu, Nov 07, 2024 at 04:01:06PM +0100, Nam Cao wrote:

> The comment above fs_read_alloc() explains:
> 
>     @align: Alignment to use for memory allocation (0 for default)
> 
> However, in the actual implementation, there is no alignment when @align is
> zero.
> 
> This current default is probably fine for most cases. But for some block
> devices which transfer data via DMA, ARCH_DMA_MINALIGN is needed.
> 
> Change the default alignment to ARCH_DMA_MINALIGN.
> 
> Fixes: de7b5a8a1ac0 ("fs: Create functions to load and allocate a file")
> Signed-off-by: Nam Cao <namcao@linutronix.de>
> Tested-by: Javier Fernandez Pastrana <javier.pastrana@linutronix.de>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v2 0/2] Fix boot failure due to misaligned DMA buffer

[Tony Dinh]

Hi Nam,


On Thu, Nov 7, 2024 at 1:53 PM Nam Cao <namcao@linutronix.de> wrote:
>
> Hi,
>
> We observed the following sporadic boot failure while booting from MMC
> device:
>
> => boot
> CACHE: Misaligned operation at range [9efa25f8, 9efa27f8]
> CACHE: Misaligned operation at range [9efa25f8, 9efa27f8]
> CACHE: Misaligned operation at range [9efa25f8, 9efa27f8]
> CACHE: Misaligned operation at range [9efa25f8, 9efa27f8]
> ** Booting bootflow 'mmc@2194000.bootdev.part_1' with extlinux
> Ignoring unknown command: �D���D��
> Boot failed (err=-14)
>
> The reason is because while allocating buffer to read a file from MMC,
> alignment of 1 byte is used. Thus, the buffer doesn't work for performing
> DMA, and garbage data is read.
>
> While looking at this issue, I also noticed that if no alignment specified
> (align=0) then fs_read_alloc() is documented to use the default. But the
> default is no alignment. Therefore, other users of fs_read_alloc() which
> specify align=0 may be broken as well.
>
> The first patch changes extlinux_read_bootflow() to use proper buffer
> alignment for DMA.
>
> The second patch changes the default alignment of fs_read_alloc() to be
> DMA-suitable, to fix other potential bugs.
>
> Nam Cao (2):
>   boot: extlinux: Fix unaligned buffer for reading data from file system
>   fs: Use ARCH_DMA_MINALIGN as default alignment for fs_read_alloc()
>
>  boot/bootmeth_extlinux.c | 3 ++-
>  fs/fs.c                  | 4 ++++
>  include/fs.h             | 2 +-
>  3 files changed, 7 insertions(+), 2 deletions(-)
>
> --
> 2.39.5
>

FYI, this "CACHE: Misaligned operation" problem was also fixed in
bootstd script method by this previous patch:
https://lists.denx.de/pipermail/u-boot/2023-September/530927.html

For the series.
Reviewed-by: Tony Dinh <mibodhi@gmail.com>

All the best,
Tony