Hi Måns, On Mon, 14 Oct 2013 11:50:42 +0100, Måns Rullgård <[email protected]> wrote:
> Piotr Wilczek <[email protected]> writes: > > >> -----Original Message----- > >> From: Måns Rullgård [mailto:[email protected]] > >> Sent: Saturday, October 12, 2013 1:29 AM > >> To: Piotr Wilczek > >> Cc: [email protected]; Tom Rini; Kyungmin Park > >> Subject: Re: [PATCH] disk:efi: avoid unaligned access on efi partition > >> > >> Piotr Wilczek <[email protected]> writes: > >> > >> > In this patch static variable and memcpy instead of an assignment are > >> > used to avoid unaligned access exception on some ARM platforms. > >> > > >> > Signed-off-by: Piotr Wilczek <[email protected]> > >> > Signed-off-by: Kyungmin Park <[email protected]> > >> > CC: Tom Rini <[email protected]> > >> > --- > >> > disk/part_efi.c | 6 ++++-- > >> > 1 file changed, 4 insertions(+), 2 deletions(-) > >> > > >> > diff --git a/disk/part_efi.c b/disk/part_efi.c index b7524d6..303b8af > >> > 100644 > >> > --- a/disk/part_efi.c > >> > +++ b/disk/part_efi.c > >> > @@ -224,7 +224,8 @@ static int set_protective_mbr(block_dev_desc_t > >> *dev_desc) > >> > p_mbr->signature = MSDOS_MBR_SIGNATURE; > >> > p_mbr->partition_record[0].sys_ind = EFI_PMBR_OSTYPE_EFI_GPT; > >> > p_mbr->partition_record[0].start_sect = 1; > >> > - p_mbr->partition_record[0].nr_sects = (u32) dev_desc->lba; > >> > + memcpy(&p_mbr->partition_record[0].nr_sects, &dev_desc->lba, > >> > + sizeof(dev_desc->lba)); > >> > >> Why is this assignment problematic? Note that the compiler may > >> optimise the memcpy() call into a plain assignment including any > >> alignment assumptions it was making in the original code. > >> > >> The correct fix is either to ensure that pointers are properly aligned > >> or that things are annotated as potentially unaligned, whichever is > >> more appropriate. > >> > > Problem is that the legacy_mbr structure consists 'le16 unknown' field > > before 'partition_record' filed and the structure is packed. As a result the > > address of 'partition_record' filed is halfword aligned. The compiler uses > > str/ldr instructions (address must be 4-byte aligned) to copy u32 'lba' data > > thus causing unaligned access exception. > > If the struct has __attribute__((packed)), gcc should do the right thing > already. Note that on ARMv6 and later ldr/str support unaligned > addresses unless this is explicitly disabled in the system control > register. If you do this, you _MUST_ compile with -mno-unaligned-access. > Otherwise you will get problems. Please do not advise using native unaligned accesses on code that is not strictly used by ARMv6+ architectures: the present code, for instance, might be run on pre-ARMv6 or non-ARM platforms, and thus, should never assume ability to perform unaligned accesses natively. Amicalement, -- Albert. _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
