[U-Boot] disk: fix unaligned access in efi partitions

start_sect is not aligned to a 4 byte boundary thus causing exceptions
on ARM platforms. Access this field via the get_unaligned macro.

Signed-off-by: Marc Dietrich <marvin24@gmx.de>
---
 disk/part_efi.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Hi Marc,

On Fri, 29 Mar 2013 12:28:58 +0100, Marc Dietrich <marvin24@gmx.de>
wrote:

> start_sect is not aligned to a 4 byte boundary thus causing exceptions
> on ARM platforms. Access this field via the get_unaligned macro.
> 
> Signed-off-by: Marc Dietrich <marvin24@gmx.de>
> ---
>  disk/part_efi.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/disk/part_efi.c b/disk/part_efi.c
> index b3fd0e9..6678a4c 100644
> --- a/disk/part_efi.c
> +++ b/disk/part_efi.c
> @@ -30,6 +30,7 @@
>   *
>   * This limits the maximum size of addressable storage to < 2 Terra Bytes
>   */
> +#include <asm/unaligned.h>
>  #include <common.h>
>  #include <command.h>
>  #include <ide.h>
> @@ -505,7 +506,7 @@ err:
>  static int pmbr_part_valid(struct partition *part)
>  {
>  	if (part->sys_ind == EFI_PMBR_OSTYPE_EFI_GPT &&
> -		le32_to_cpu(part->start_sect) == 1UL) {
> +		le32_to_cpu(get_unaligned(&part->start_sect)) == 1UL) {
>  		return 1;
>  	}

Suits me :) but did you not say the same issue was also affecting
nr_sects?

Also, beside the asm/unaligned.h file I gave as an example, other header
files in include/linux/unaligned/ exist that provide alignment-related
macros, notably with _le and _be versions. Maybe you can find one that
combines the effects of both le32_to_cpu() and get_unaligned()?

Amicalement,

Albert,

On Friday 29 March 2013 13:32:26 Albert ARIBAUD wrote:
> On Fri, 29 Mar 2013 12:28:58 +0100, Marc Dietrich <marvin24@gmx.de> wrote:
> > start_sect is not aligned to a 4 byte boundary thus causing exceptions
> > on ARM platforms. Access this field via the get_unaligned macro.
> > 
> > Signed-off-by: Marc Dietrich <marvin24@gmx.de>
> > ---
> > 
> >  disk/part_efi.c |    3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/disk/part_efi.c b/disk/part_efi.c
> > index b3fd0e9..6678a4c 100644
> > --- a/disk/part_efi.c
> > +++ b/disk/part_efi.c
> > @@ -30,6 +30,7 @@
> > 
> >   *
> >   * This limits the maximum size of addressable storage to < 2 Terra Bytes
> >   */
> > 
> > +#include <asm/unaligned.h>
> > 
> >  #include <common.h>
> >  #include <command.h>
> >  #include <ide.h>
> > 
> > @@ -505,7 +506,7 @@ err:
> >  static int pmbr_part_valid(struct partition *part)
> >  {
> >  
> >  	if (part->sys_ind == EFI_PMBR_OSTYPE_EFI_GPT &&
> > 
> > -		le32_to_cpu(part->start_sect) == 1UL) {
> > +		le32_to_cpu(get_unaligned(&part->start_sect)) == 1UL) {
> > 
> >  		return 1;
> >  	
> >  	}
> 
> Suits me :) but did you not say the same issue was also affecting
> nr_sects?

I checked again and this field is never read - only written to but to an 
aligned buffer. So there shouldn't be a problem.

> Also, beside the asm/unaligned.h file I gave as an example, other header
> files in include/linux/unaligned/ exist that provide alignment-related
> macros, notably with _le and _be versions. Maybe you can find one that
> combines the effects of both le32_to_cpu() and get_unaligned()?

I looks like get_unaligned does the job for us already, so we can leave the 
le32_to_cpu out. But I cannot test it here because of missing big endian 
machine with efi and my two brain halfs are already swapped until confusion 
(start_sect is defined as _le32). 

Albert, I like to get a fix for this into 2013.04 if possible. Maybe someone 
with more endian experience can look at it quickly.

Marc

On 03/29/2013 09:13 AM, Marc Dietrich wrote:
> On Friday 29 March 2013 13:32:26 Albert ARIBAUD wrote:
>> On Fri, 29 Mar 2013 12:28:58 +0100, Marc Dietrich <marvin24@gmx.de> wrote:
>>> start_sect is not aligned to a 4 byte boundary thus causing exceptions
>>> on ARM platforms. Access this field via the get_unaligned macro.

>>> -		le32_to_cpu(part->start_sect) == 1UL) {
>>> +		le32_to_cpu(get_unaligned(&part->start_sect)) == 1UL) {

>> Also, beside the asm/unaligned.h file I gave as an example, other header
>> files in include/linux/unaligned/ exist that provide alignment-related
>> macros, notably with _le and _be versions. Maybe you can find one that
>> combines the effects of both le32_to_cpu() and get_unaligned()?
> 
> I looks like get_unaligned does the job for us already, so we can leave the 
> le32_to_cpu out. But I cannot test it here because of missing big endian 
> machine with efi and my two brain halfs are already swapped until confusion 
> (start_sect is defined as _le32). 
> 
> Albert, I like to get a fix for this into 2013.04 if possible. Maybe someone 
> with more endian experience can look at it quickly.

Looking at include/linux/unaligned/generic.h, I think you can just use:

__get_unaligned_le(&part->start_sect) == 1UL

I don't believe the __le32 marking of the start_sect field will have any
effect, since the implementation of __get_unaligned_le accesses the
variable byte-by-byte.