[U-Boot] [PATCH] disk: add -mno-unaligned-access to CFLAGS

[U-Boot] [PATCH] disk: add -mno-unaligned-access to CFLAGS

[Marc Dietrich]

Many on-disk structures used in the directory are accessed in a
non aligned manner. gcc => 4.7 (and gcc-4.6 from Linaro) switched
to -munaligned-access on default causing exceptions on ARM. The
easiest way to fix this is to force no-unaligned-access in this
(non speed critical) directory.

Signed-off-by: Marc Dietrich <marvin24@gmx.de>
---
 disk/Makefile |    1 +
 1 file changed, 1 insertion(+)

diff --git a/disk/Makefile b/disk/Makefile
index 5affe34..01134a3 100644
--- a/disk/Makefile
+++ b/disk/Makefile
@@ -24,6 +24,7 @@
 include $(TOPDIR)/config.mk
 
 #CFLAGS += -DET_DEBUG -DDEBUG
+CFLAGS += -mno-unaligned-access
 
 LIB	= $(obj)libdisk.o
 
-- 
1.7.9.5

[U-Boot] [PATCH] disk: add -mno-unaligned-access to CFLAGS

[Albert ARIBAUD]

Hi Marc,

On Thu, 28 Mar 2013 19:57:31 +0100, Marc Dietrich <marvin24@gmx.de>
wrote:

> Many on-disk structures used in the directory are accessed in a
> non aligned manner. gcc => 4.7 (and gcc-4.6 from Linaro) switched
> to -munaligned-access on default causing exceptions on ARM. The
> easiest way to fix this is to force no-unaligned-access in this
> (non speed critical) directory.
> 
> Signed-off-by: Marc Dietrich <marvin24@gmx.de>
> ---
>  disk/Makefile |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/disk/Makefile b/disk/Makefile
> index 5affe34..01134a3 100644
> --- a/disk/Makefile
> +++ b/disk/Makefile
> @@ -24,6 +24,7 @@
>  include $(TOPDIR)/config.mk
>  
>  #CFLAGS += -DET_DEBUG -DDEBUG
> +CFLAGS += -mno-unaligned-access
>  
>  LIB	= $(obj)libdisk.o

Which fields, which structures, which files are affected by the
unalignment issue?

Amicalement,
-- 
Albert.

[U-Boot] [PATCH] disk: add -mno-unaligned-access to CFLAGS

[Marc Dietrich]

Hi Albert,

On Thursday 28 March 2013 21:42:13 Albert ARIBAUD wrote:
> On Thu, 28 Mar 2013 19:57:31 +0100, Marc Dietrich <marvin24@gmx.de>
> wrote:
> > Many on-disk structures used in the directory are accessed in a
> > non aligned manner. gcc => 4.7 (and gcc-4.6 from Linaro) switched
> > to -munaligned-access on default causing exceptions on ARM. The
> > easiest way to fix this is to force no-unaligned-access in this
> > (non speed critical) directory.
> > 
> > Signed-off-by: Marc Dietrich <marvin24@gmx.de>
> > ---
> > 
> >  disk/Makefile |    1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/disk/Makefile b/disk/Makefile
> > index 5affe34..01134a3 100644
> > --- a/disk/Makefile
> > +++ b/disk/Makefile
> > @@ -24,6 +24,7 @@
> > 
> >  include $(TOPDIR)/config.mk
> >  
> >  #CFLAGS += -DET_DEBUG -DDEBUG
> > 
> > +CFLAGS += -mno-unaligned-access
> > 
> >  LIB	= $(obj)libdisk.o
> 
> Which fields, which structures, which files are affected by the
> unalignment issue?

in my test case, it is the start sector of a partition (check 
include/part_efi.h). disk/part_efi.c reads the legacy mbr (to an aligned 
buffer) which has a partition structure on offset 440+4+2 (<- not aligned to 4 
byte boundary) and inside this a 32 bit field start_sect (aligned to 4 byte 
boundary). Reading this field (and also the next, nr_sects) will cause an 
exception. Same is for part_dos, but there we still use le32_to_int which 
reads byte by byte. I didn't checked others.

Marc

[U-Boot] [PATCH] disk: add -mno-unaligned-access to CFLAGS

[Albert ARIBAUD]

Hi Marc,

On Thu, 28 Mar 2013 23:35 +0100, Marc Dietrich <marvin24@gmx.de> wrote:

> Hi Albert,
> 
> On Thursday 28 March 2013 21:42:13 Albert ARIBAUD wrote:
> > On Thu, 28 Mar 2013 19:57:31 +0100, Marc Dietrich <marvin24@gmx.de>
> > wrote:
> > > Many on-disk structures used in the directory are accessed in a
> > > non aligned manner. gcc => 4.7 (and gcc-4.6 from Linaro) switched
> > > to -munaligned-access on default causing exceptions on ARM. The
> > > easiest way to fix this is to force no-unaligned-access in this
> > > (non speed critical) directory.
> > > 
> > > Signed-off-by: Marc Dietrich <marvin24@gmx.de>
> > > ---
> > > 
> > >  disk/Makefile |    1 +
> > >  1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/disk/Makefile b/disk/Makefile
> > > index 5affe34..01134a3 100644
> > > --- a/disk/Makefile
> > > +++ b/disk/Makefile
> > > @@ -24,6 +24,7 @@
> > > 
> > >  include $(TOPDIR)/config.mk
> > >  
> > >  #CFLAGS += -DET_DEBUG -DDEBUG
> > > 
> > > +CFLAGS += -mno-unaligned-access
> > > 
> > >  LIB	= $(obj)libdisk.o
> > 
> > Which fields, which structures, which files are affected by the
> > unalignment issue?
> 
> in my test case, it is the start sector of a partition (check 
> include/part_efi.h). disk/part_efi.c reads the legacy mbr (to an aligned 
> buffer) which has a partition structure on offset 440+4+2 (<- not aligned to 4 
> byte boundary) and inside this a 32 bit field start_sect (aligned to 4 byte 
> boundary). Reading this field (and also the next, nr_sects) will cause an 
> exception. Same is for part_dos, but there we still use le32_to_int which 
> reads byte by byte. I didn't checked others.

Thanks for clarifying.

Considering this is about a very small number of reads, I strongly
prefer that these reads be done through the get_unaligned(&field) macro
defined in e.g. arch/arm/include/asm/unaligned.h, even at the slightly
added cost of decomposing the reads into 8-bit accesses.

Doing so solves the issue at hand while still allowing the rest of the
code to detect cases where unalignment stems from error conditions, e.g.
bad pointers or erroneous changes in structures, etc.

> Marc

Amicalement,
-- 
Albert.

[U-Boot] [PATCH] disk: add -mno-unaligned-access to CFLAGS

[Marc Dietrich]

Albert,

On Friday 29 March 2013 08:00:54 Albert ARIBAUD wrote:
> On Thu, 28 Mar 2013 23:35 +0100, Marc Dietrich <marvin24@gmx.de> wrote:
> > [ ... ]
> > On Thursday 28 March 2013 21:42:13 Albert ARIBAUD wrote:
> > > Which fields, which structures, which files are affected by the
> > > unalignment issue?
> > 
> > in my test case, it is the start sector of a partition (check
> > include/part_efi.h). disk/part_efi.c reads the legacy mbr (to an aligned
> > buffer) which has a partition structure on offset 440+4+2 (<- not aligned
> > to 4 byte boundary) and inside this a 32 bit field start_sect (aligned to
> > 4 byte boundary). Reading this field (and also the next, nr_sects) will
> > cause an exception. Same is for part_dos, but there we still use
> > le32_to_int which reads byte by byte. I didn't checked others.
> 
> Thanks for clarifying.
> 
> Considering this is about a very small number of reads, I strongly
> prefer that these reads be done through the get_unaligned(&field) macro
> defined in e.g. arch/arm/include/asm/unaligned.h, even at the slightly
> added cost of decomposing the reads into 8-bit accesses.

ok, this is what I was actually looking for :-) I've sent a new patch.

Marc