public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Hector Palacios <hector.palacios@digi.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH V2] disk:efi: avoid unaligned access on efi partition
Date: Wed, 19 Feb 2014 15:56:39 +0100	[thread overview]
Message-ID: <5304C627.2080503@digi.com> (raw)
In-Reply-To: <1390913212-11217-1-git-send-email-p.wilczek@samsung.com>

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.

[1] http://en.wikipedia.org/wiki/Master_boot_record#Sector_layout

Best regards,
--
Hector Palacios

  parent reply	other threads:[~2014-02-19 14:56 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-11 13:31 [U-Boot] [PATCH] disk:efi: avoid unaligned access on efi partition Piotr Wilczek
2013-10-11 16:00 ` Albert ARIBAUD
2013-10-11 23:28 ` Måns Rullgård
2013-10-14  8:30   ` Piotr Wilczek
2013-10-14 10:50     ` Måns Rullgård
2013-10-14 11:46       ` Albert ARIBAUD
2013-10-14 12:19         ` Måns Rullgård
2013-10-14 13:00           ` Albert ARIBAUD
2013-10-14 13:05             ` Måns Rullgård
2013-10-14 13:48               ` Albert ARIBAUD
2013-10-14 14:09                 ` Måns Rullgård
2013-10-14 15:18                   ` Albert ARIBAUD
2013-10-14 15:59                     ` Måns Rullgård
2013-10-14 17:26                       ` Albert ARIBAUD
2013-10-15 15:23                         ` Måns Rullgård
2013-10-15 16:21                           ` Albert ARIBAUD
2013-10-15 16:29                             ` Måns Rullgård
2013-10-15 17:07                               ` Albert ARIBAUD
2013-10-14 13:49               ` Piotr Wilczek
2013-10-14 14:22                 ` Albert ARIBAUD
2014-01-28 12:46 ` [U-Boot] [PATCH V2] " Piotr Wilczek
2014-01-29 21:48   ` Tom Rini
2014-02-19 14:56   ` Hector Palacios [this message]
2014-02-19 15:03     ` Tom Rini
2014-02-19 15:23       ` Tom Rini
2014-02-24 15:56         ` Lukasz Majewski
2014-02-24 16:23           ` Tom Rini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5304C627.2080503@digi.com \
    --to=hector.palacios@digi.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox