[PATCH 0/2] Fix boot failure due to misaligned DMA buffer
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(-)
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);
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
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
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);
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
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
participants (2)
-
Nam Cao -
Simon Glass