[U-Boot] [PATCH] disk:efi: avoid unaligned access on efi partition

Måns Rullgård
2013-10-14T10:50:42Z

Piotr Wilczek <p.wilczek@samsung.com> writes:

>> -----Original Message-----
>> From: M?ns Rullg?rd [mailto:mans at mansr.com]
>> Sent: Saturday, October 12, 2013 1:29 AM
>> To: Piotr Wilczek
>> Cc: u-boot at lists.denx.de; Tom Rini; Kyungmin Park
>> Subject: Re: [PATCH] disk:efi: avoid unaligned access on efi partition
>>
>> Piotr Wilczek <p.wilczek@samsung.com> 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 <p.wilczek@samsung.com>
>> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>> > CC: Tom Rini <trini@ti.com>
>> > ---
>> > 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.

--
M?ns Rullg?rd
mans at mansr.com