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

[PATCH 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 the default to align the buffer suitable for DMA.

The second patch changes extlinux_read_bootflow() to use the default
alignment.

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

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

-- 
2.39.5

[PATCH 1/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>
---
 fs/fs.c | 4 ++++
 1 file changed, 4 insertions(+)

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);
-- 
2.39.5

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

[Simon Glass]

Hi Nam,

On Wed, 6 Nov 2024 at 09:34, Nam Cao <namcao@linutronix.de> 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>
> ---
>  fs/fs.c | 4 ++++
>  1 file changed, 4 insertions(+)
>

Please update the function comment in fs.h to specify the default value

Regards,
Simon

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

[Nam Cao]

Hi Simon,

On Thu, Nov 07, 2024 at 06:44:32AM -0700, Simon Glass wrote:
> Hi Nam,
> 
> On Wed, 6 Nov 2024 at 09:34, Nam Cao <namcao@linutronix.de> 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>
> > ---
> >  fs/fs.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> 
> Please update the function comment in fs.h to specify the default value

Sure.

Best regards,
Nam

[PATCH 2/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 alignment to default (which is 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>
---
 boot/bootmeth_extlinux.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/boot/bootmeth_extlinux.c b/boot/bootmeth_extlinux.c
index ae0ad1d53e3..5424d179bf8 100644
--- a/boot/bootmeth_extlinux.c
+++ b/boot/bootmeth_extlinux.c
@@ -127,7 +127,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, 0);
 	if (ret)
 		return log_msg_ret("read", ret);
 
-- 
2.39.5

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

[Simon Glass]

Hi Nam,

On Wed, 6 Nov 2024 at 09:34, Nam Cao <namcao@linutronix.de> 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 alignment to default (which is 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>
> ---
>  boot/bootmeth_extlinux.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/boot/bootmeth_extlinux.c b/boot/bootmeth_extlinux.c
> index ae0ad1d53e3..5424d179bf8 100644
> --- a/boot/bootmeth_extlinux.c
> +++ b/boot/bootmeth_extlinux.c
> @@ -127,7 +127,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, 0);

Can you please use ARCH_DMA_MINALIGN here? Either that or update
bootmeth_script to do the same  as here.

>         if (ret)
>                 return log_msg_ret("read", ret);
>
> --
> 2.39.5
>

Regards,
Simon

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

[Nam Cao]

Hi Simon,

On Thu, Nov 07, 2024 at 06:44:30AM -0700, Simon Glass wrote:
> On Wed, 6 Nov 2024 at 09:34, Nam Cao <namcao@linutronix.de> 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 alignment to default (which is 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>
> > ---
> >  boot/bootmeth_extlinux.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/boot/bootmeth_extlinux.c b/boot/bootmeth_extlinux.c
> > index ae0ad1d53e3..5424d179bf8 100644
> > --- a/boot/bootmeth_extlinux.c
> > +++ b/boot/bootmeth_extlinux.c
> > @@ -127,7 +127,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, 0);
> 
> Can you please use ARCH_DMA_MINALIGN here? Either that or update
> bootmeth_script to do the same  as here.

No preference from me. Let's just use ARCH_DMA_MINALIGN here.

Thanks for the comments,
Nam