Message ID | 6ff0548717babcbc52d225394549106e659cc303.1364556523.git.marvin24@gmx.de |
---|---|
State | Superseded |
Headers | show |
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.
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; }
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(-)