On 23 April 2016 at 07:51, Mark Kettenis <mark.kette...@xs4all.nl> wrote:
>> Date: Sat, 23 Apr 2016 13:23:49 +0200 (CEST)
>> From: Mark Kettenis <mark.kette...@xs4all.nl>
>>
>> > Date: Sat, 23 Apr 2016 12:49:25 +0200 (CEST)
>> > From: Mark Kettenis <mark.kette...@xs4all.nl>
>> >
>> > > Could it be this problem is related to
>> > > sys/arch/amd64/stand/efi/include/i386/efibind.h using sys/stdint.h
>> > > which looks like it will make uint64_t unsigned long on for BOOTIA32.EFI
>> > > not unsigned long long?
>> >
>> > We consistently define uint64_t as unsigned long long (unlike some
>> > other OSes) so this shouldn't be the case.
>> >
>> > I didn't look in detail at the report yet, but I can confirm that I
>> > had a similar problem on the Asus x205ta. I worked around it by
>> > nuking the GPT and using a traditional MBR partition table instead.
>> > Never got around investigating the issue any further.
>>
>> Seems that this is a structure memberalignment issue:
>>
>> For BOOTX64 we have:
>>
>> (gdb) print &((EFI_BLOCK_IO_MEDIA *)0)->LastBlock
>> $1 = (EFI_LBA *) 0x18
>>
>> wheras for BOTIA32 we get:
>>
>> (gdb) print &((EFI_BLOCK_IO_MEDIA *)0)->LastBlock
>> $1 = (EFI_LBA *) 0x14
>>
>> Looking at the struct definition:
>>
>> typedef struct {
>> UINT32 MediaId;
>> BOOLEAN RemovableMedia;
>> BOOLEAN MediaPresent;
>>
>> BOOLEAN LogicalPartition;
>> BOOLEAN ReadOnly;
>> BOOLEAN WriteCaching;
>>
>> UINT32 BlockSize;
>> UINT32 IoAlign;
>>
>> EFI_LBA LastBlock;
>> } EFI_BLOCK_IO_MEDIA;
>>
>> We see that the 64-bit code has padding between IoAlign and LastBlock
>> to make sure that LastBlock is properly aligned. That padding isn't
>> there in the 32-bit code since the System V ABI doesn't require 64-bit
>> alignment here. But I guess the 32-bit Windows ABI does.
>
> And here is a possible fix. Got the inspiration from the SYSLINUX
> bootloader. This forces alignment of the EFI-specific 64-bit types.
> It leaves the normal int64_t/uint64_t alone, but we shouldn't use
> those directly to define EFI data structures.
>
> John, can you confirm that this fixes the issue for you?
>
> Jonathan, Ken, provided that it does, ok?
>
>
> Index: efi/include/i386/efibind.h
> ===================================================================
> RCS file: /home/cvs/src/sys/arch/amd64/stand/efi/include/i386/efibind.h,v
> retrieving revision 1.2
> diff -u -p -r1.2 efibind.h
> --- efi/include/i386/efibind.h 11 Dec 2015 20:17:10 -0000 1.2
> +++ efi/include/i386/efibind.h 23 Apr 2016 11:38:26 -0000
> @@ -88,8 +88,8 @@ Revision History
> #ifndef ACPI_THREAD_ID /* ACPI's definitions are fine, use those */
> #define ACPI_USE_SYSTEM_INTTYPES 1 /* Tell ACPI we've defined types */
>
> -typedef uint64_t UINT64;
> -typedef int64_t INT64;
> +typedef uint64_t UINT64 __attribute__((__aligned__(8)));
> +typedef int64_t INT64 __attribute__((__aligned__(8)));
>
> #ifndef _BASETSD_H_
> typedef uint32_t UINT32;
>
Your analysis is much more useful than mine was. :-). I was wondering
how many other places might get bit by the mis-alignment, but this
approach would seem to get them all in one fell swoop.
If this works it is ok with me. It would be good to get yasuoka@ ok too.
.... Ken
