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

[Tom Rini <trini@ti.com>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 02/19/2014 10:03 AM, Tom Rini wrote:
> On 02/19/2014 09:56 AM, Hector Palacios wrote:
>> On 01/28/2014 01:46 PM, Piotr Wilczek wrote:
>>> This patch fixes part_efi code 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>
>>> CC: Albert ARIBAUD <albert.u.boot@aribaud.net>
>>> ---
>>> Chnages for V2:
>>>   - used put_unaligned to copy value;
>>>   - use __aligned to align local array;
>>>
>>>   disk/part_efi.c |    7 ++++---
>>>   1 file changed, 4 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/disk/part_efi.c b/disk/part_efi.c
>>> index 9c33ae7..eb2cd57 100644
>>> --- a/disk/part_efi.c
>>> +++ b/disk/part_efi.c
>>> @@ -225,7 +225,7 @@ 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;
>>> +    put_unaligned(dev_desc->lba, &p_mbr->partition_record[0].nr_sects);
>>>
>>>       /* Write MBR sector to the MMC device */
>>>       if (dev_desc->block_write(dev_desc->dev, 0, 1, p_mbr) != 1) {
>>> @@ -361,6 +361,8 @@ int gpt_fill_pte(gpt_header *gpt_h, gpt_entry *gpt_e,
>>>   #ifdef CONFIG_PARTITION_UUIDS
>>>       char *str_uuid;
>>>   #endif
>>> +    static efi_guid_t basic_guid __aligned(0x04) =
>>> +        PARTITION_BASIC_DATA_GUID;
>>>
>>>       for (i = 0; i < parts; i++) {
>>>           /* partition starting lba */
>>> @@ -388,8 +390,7 @@ int gpt_fill_pte(gpt_header *gpt_h, gpt_entry *gpt_e,
>>>               gpt_e[i].ending_lba = cpu_to_le64(offset - 1);
>>>
>>>           /* partition type GUID */
>>> -        memcpy(gpt_e[i].partition_type_guid.b,
>>> -            &PARTITION_BASIC_DATA_GUID, 16);
>>> +        gpt_e[i].partition_type_guid = basic_guid;
>>>
>>>   #ifdef CONFIG_PARTITION_UUIDS
>>>           str_uuid = partitions[i].uuid;
>>>
> 
>> Sorry, I sent my tested-by on a different thread:
>> http://patchwork.ozlabs.org/patch/319649/
> 
>> Tested on i.MX6 (armv7) custom board and it is working fine without
>> the -mno-unaligned-access flag.
> 
>> Tested-by: Hector Palacios <hector.palacios@digi.com>
> 
>> Replying to Tom's question in that other thread, regarding the issue:
> 
>>> Can you give me some steps on how to hit this bug?  I believe it's a bug
>>> and I believe we need to fix it, I just want to investigate a few things
>>> while we've got a trigger case right now.  Thanks!
> 
>> GPT partition table needs to write a protective MBR to sector 0.
>> The MBR structure has four partition entries (each occupying 16 bytes)
>> at unaligned offsets +1BEh, +1CEh, +1DEh, +1EEh (see [1]). The structure
>> of each partition entry is defined at include/part_efi.h
> 
>> struct partition {
>>     u8 boot_ind;        /* 0x80 - active */
>>     u8 head;        /* starting head */
>>     u8 sector;        /* starting sector */
>>     u8 cyl;            /* starting cylinder */
>>     u8 sys_ind;        /* What partition type */
>>     u8 end_head;        /* end head */
>>     u8 end_sector;        /* end sector */
>>     u8 end_cyl;        /* end cylinder */
>>     __le32 start_sect;    /* starting sector counting from 0 */
>>     __le32 nr_sects;    /* nr of sectors in partition */
>> } __packed;
> 
>> showing eight 1-byte fields and two 4-byte fields. Since the offsets for
>> each partition entry are unaligned, the last two fields (which are
>> 32bit) are unaligned as well. But it's not an error, it's just the
>> specification of the MBR requires these fields to be at those exact
>> offsets. So the usage of unaligned macros for accessing those fields is
>> justified.
> 
> Right.  I would have sworn I used the GPT commands since we've dropped
> -mno-unaligned-access but I'll just go re-test locally now then, thanks.

Indeed I hadn't re-tested recently enough, thanks.

- -- 
Tom
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJTBMxeAAoJENk4IS6UOR1WmKAP/05tEc0Ix4vrbI+WEYv4s26v
Up3Uj1yMHBgHhCmdUops6C6eNjoSX2Z2TzfFx8jQ1YCTrGXO229CSi0U/oXkTWCB
lbdMxQ6IyKh1v+Z4MdEJBq43XxcGMVE8g75Z8Eb/7B74kHRvsLeLGIDMg4A3z1yB
b95HOCaifhSykBELsrqWXpMhJr4KMkn+GT9lNA5umLJdHT/1A2pEglbkZWAu4tAt
ybDpdpXI3Ai8eVg9NuMJXEAcYEGezlg55esFv57gJfLfBPM/e0WLF4MtZYyFJmC/
0SLe46OG19E6JzHMKrHngZbyVSC+Iwzh5Mw6vY40IyVxA6fpXZEZ119AyxLtP4m8
BiWjjRAO65KC6qzRJJYzXKoXSD8Ky+VJ3ATWzUhVSeLQRHeE2am8JsT8ENj5dngC
f93pzqTmOp9LPqOB81dlIbu+R8QoruX0mOQxygNy+7tpTTijLn+UUu23z165j42b
qsBzrAddgFZzUxqfyJLHIr/bvrVqBpEP6BGv2i+5eA6Q/dPHMvVLV3hTZjnxtK52
I6RVfK2R4uTX8mK+yN2HzBdqZhCJqDgxbWYUMuDIkVq7QeY5rtPBaFlOguLPqwx2
+i38rtGj5abZcqr9ASDcmrfIoe1mIAj2NPuJejNLzbwyipyqRVO0CT+GG/FwXLLG
LFWWTaNc/X4FMjGctcyr
=1et2
-----END PGP SIGNATURE-----