| Message ID | 20180618152315.34233-12-agraf@suse.de |
|---|---|
| State | New |
| Headers | show |
| Series | sandbox: efi_loader support | expand |
Hi Alex, On 18 June 2018 at 09:23, Alexander Graf <agraf@suse.de> wrote: > Varargs differ between sysv and ms abi. On x86_64 we have to follow the ms > abi though, so we also need to make sure we use x86_64 varargs helpers. > > This patch introduces generic efi vararg helpers that adhere to the > respective EFI ABI. That way we can deal with them properly from efi > loader code and properly interpret variable arguments. > > This fixes the InstallMultipleProtocolInterfaces tests in the efi selftests > on x86_64 for me. > > Signed-off-by: Alexander Graf <agraf@suse.de> > --- > include/efi.h | 8 ++++++++ > lib/efi_loader/efi_boottime.c | 36 ++++++++++++++++++------------------ > 2 files changed, 26 insertions(+), 18 deletions(-) I thought this was a bug in gcc. Should we include this workaround only for older gcc versions? Regards, Simon
On 06/21/2018 04:02 AM, Simon Glass wrote: > Hi Alex, > > On 18 June 2018 at 09:23, Alexander Graf <agraf@suse.de> wrote: >> Varargs differ between sysv and ms abi. On x86_64 we have to follow the ms >> abi though, so we also need to make sure we use x86_64 varargs helpers. >> >> This patch introduces generic efi vararg helpers that adhere to the >> respective EFI ABI. That way we can deal with them properly from efi >> loader code and properly interpret variable arguments. >> >> This fixes the InstallMultipleProtocolInterfaces tests in the efi selftests >> on x86_64 for me. >> >> Signed-off-by: Alexander Graf <agraf@suse.de> >> --- >> include/efi.h | 8 ++++++++ >> lib/efi_loader/efi_boottime.c | 36 ++++++++++++++++++------------------ >> 2 files changed, 26 insertions(+), 18 deletions(-) > I thought this was a bug in gcc. Should we include this workaround > only for older gcc versions? The bug is something different - it's about using unannotated helper functions on an ms_abi declared vararg list. This patch really just implements expected behavior regardless of that bug. Alex
> Varargs differ between sysv and ms abi. On x86_64 we have to follow the ms > abi though, so we also need to make sure we use x86_64 varargs helpers. > > This patch introduces generic efi vararg helpers that adhere to the > respective EFI ABI. That way we can deal with them properly from efi > loader code and properly interpret variable arguments. > > This fixes the InstallMultipleProtocolInterfaces tests in the efi selftests > on x86_64 for me. > > Signed-off-by: Alexander Graf <agraf@suse.de> Thanks, applied to efi-next Alex
Hi Alex, On Thu, Jun 21, 2018 at 11:13 PM, Alexander Graf <agraf@suse.de> wrote: >> Varargs differ between sysv and ms abi. On x86_64 we have to follow the ms >> abi though, so we also need to make sure we use x86_64 varargs helpers. >> >> This patch introduces generic efi vararg helpers that adhere to the >> respective EFI ABI. That way we can deal with them properly from efi >> loader code and properly interpret variable arguments. >> >> This fixes the InstallMultipleProtocolInterfaces tests in the efi selftests >> on x86_64 for me. >> >> Signed-off-by: Alexander Graf <agraf@suse.de> > > Thanks, applied to efi-next > I applied this patch on my x86 tree and tested there, but qemu-x86_64_defconfig still fails when 'bootefi selftest'. Anything I am missing? Regards, Bin
On 06/23/2018 10:37 AM, Bin Meng wrote: > Hi Alex, > > On Thu, Jun 21, 2018 at 11:13 PM, Alexander Graf <agraf@suse.de> wrote: >>> Varargs differ between sysv and ms abi. On x86_64 we have to follow the ms >>> abi though, so we also need to make sure we use x86_64 varargs helpers. >>> >>> This patch introduces generic efi vararg helpers that adhere to the >>> respective EFI ABI. That way we can deal with them properly from efi >>> loader code and properly interpret variable arguments. >>> >>> This fixes the InstallMultipleProtocolInterfaces tests in the efi selftests >>> on x86_64 for me. >>> >>> Signed-off-by: Alexander Graf <agraf@suse.de> >> Thanks, applied to efi-next >> > I applied this patch on my x86 tree and tested there, but > qemu-x86_64_defconfig still fails when 'bootefi selftest'. Anything I > am missing? Where does it fail? There is basically this pitfall and setjmp/longjmp that can easily go wrong. Alex
Hi Alex, On Tue, Jun 26, 2018 at 12:47 AM, Alexander Graf <agraf@suse.de> wrote: > On 06/23/2018 10:37 AM, Bin Meng wrote: >> >> Hi Alex, >> >> On Thu, Jun 21, 2018 at 11:13 PM, Alexander Graf <agraf@suse.de> wrote: >>>> >>>> Varargs differ between sysv and ms abi. On x86_64 we have to follow the >>>> ms >>>> abi though, so we also need to make sure we use x86_64 varargs helpers. >>>> >>>> This patch introduces generic efi vararg helpers that adhere to the >>>> respective EFI ABI. That way we can deal with them properly from efi >>>> loader code and properly interpret variable arguments. >>>> >>>> This fixes the InstallMultipleProtocolInterfaces tests in the efi >>>> selftests >>>> on x86_64 for me. >>>> >>>> Signed-off-by: Alexander Graf <agraf@suse.de> >>> >>> Thanks, applied to efi-next >>> >> I applied this patch on my x86 tree and tested there, but >> qemu-x86_64_defconfig still fails when 'bootefi selftest'. Anything I >> am missing? > > > Where does it fail? There is basically this pitfall and setjmp/longjmp that > can easily go wrong. It just reboots without printing any error message. You can reproduce this in the latest u-boot/master. Regards, Bin
On 06/26/2018 03:51 AM, Bin Meng wrote: > Hi Alex, > > On Tue, Jun 26, 2018 at 12:47 AM, Alexander Graf <agraf@suse.de> wrote: >> On 06/23/2018 10:37 AM, Bin Meng wrote: >>> Hi Alex, >>> >>> On Thu, Jun 21, 2018 at 11:13 PM, Alexander Graf <agraf@suse.de> wrote: >>>>> Varargs differ between sysv and ms abi. On x86_64 we have to follow the >>>>> ms >>>>> abi though, so we also need to make sure we use x86_64 varargs helpers. >>>>> >>>>> This patch introduces generic efi vararg helpers that adhere to the >>>>> respective EFI ABI. That way we can deal with them properly from efi >>>>> loader code and properly interpret variable arguments. >>>>> >>>>> This fixes the InstallMultipleProtocolInterfaces tests in the efi >>>>> selftests >>>>> on x86_64 for me. >>>>> >>>>> Signed-off-by: Alexander Graf <agraf@suse.de> >>>> Thanks, applied to efi-next >>>> >>> I applied this patch on my x86 tree and tested there, but >>> qemu-x86_64_defconfig still fails when 'bootefi selftest'. Anything I >>> am missing? >> >> Where does it fail? There is basically this pitfall and setjmp/longjmp that >> can easily go wrong. > It just reboots without printing any error message. You can reproduce > this in the latest u-boot/master. How do you run this? I tried the qemu x86_64 target, but it complains about a missing serial driver. I guess I need to pass some DT in? Alex $ qemu-system-x86_64 -nographic -pflash u-boot.rom -m 1G WARNING: Image format was not specified for 'u-boot.rom' and probing guessed raw. Automatically detecting the format is dangerous for raw images, write operations on block 0 will be restricted. Specify the 'raw' format explicitly to remove the restrictions. U-Boot SPL 2018.07-rc2-00066-gebea1f8367 (Jun 26 2018 - 13:13:34 +0200) CPU: x86_64, vendor AMD, device 663h Trying to boot from SPI Jumping to 64-bit U-Boot: Note many features are missing No serial driver found QEMU: Terminated
Hi Alex, On Tue, Jun 26, 2018 at 7:18 PM, Alexander Graf <agraf@suse.de> wrote: > On 06/26/2018 03:51 AM, Bin Meng wrote: >> >> Hi Alex, >> >> On Tue, Jun 26, 2018 at 12:47 AM, Alexander Graf <agraf@suse.de> wrote: >>> >>> On 06/23/2018 10:37 AM, Bin Meng wrote: >>>> >>>> Hi Alex, >>>> >>>> On Thu, Jun 21, 2018 at 11:13 PM, Alexander Graf <agraf@suse.de> wrote: >>>>>> >>>>>> Varargs differ between sysv and ms abi. On x86_64 we have to follow >>>>>> the >>>>>> ms >>>>>> abi though, so we also need to make sure we use x86_64 varargs >>>>>> helpers. >>>>>> >>>>>> This patch introduces generic efi vararg helpers that adhere to the >>>>>> respective EFI ABI. That way we can deal with them properly from efi >>>>>> loader code and properly interpret variable arguments. >>>>>> >>>>>> This fixes the InstallMultipleProtocolInterfaces tests in the efi >>>>>> selftests >>>>>> on x86_64 for me. >>>>>> >>>>>> Signed-off-by: Alexander Graf <agraf@suse.de> >>>>> >>>>> Thanks, applied to efi-next >>>>> >>>> I applied this patch on my x86 tree and tested there, but >>>> qemu-x86_64_defconfig still fails when 'bootefi selftest'. Anything I >>>> am missing? >>> >>> >>> Where does it fail? There is basically this pitfall and setjmp/longjmp >>> that >>> can easily go wrong. >> >> It just reboots without printing any error message. You can reproduce >> this in the latest u-boot/master. > > > How do you run this? I tried the qemu x86_64 target, but it complains about > a missing serial driver. I guess I need to pass some DT in? > That's weird. I never saw such boot logs before. There is no need to pass DT in. > Alex > > $ qemu-system-x86_64 -nographic -pflash u-boot.rom -m 1G I was using '-bios' instead of '-pflash', but I just tested '-pflash' and it worked too. I am using QEMU 2.5.0 (the one shipped on Ubuntu 16.04) > WARNING: Image format was not specified for 'u-boot.rom' and probing guessed > raw. > Automatically detecting the format is dangerous for raw images, > write operations on block 0 will be restricted. > Specify the 'raw' format explicitly to remove the restrictions. > > U-Boot SPL 2018.07-rc2-00066-gebea1f8367 (Jun 26 2018 - 13:13:34 +0200) > CPU: x86_64, vendor AMD, device 663h > Trying to boot from SPI > Jumping to 64-bit U-Boot: Note many features are missing > No serial driver found > QEMU: Terminated > Regards, Bin
diff --git a/include/efi.h b/include/efi.h index 826d484977..7be7798d8d 100644 --- a/include/efi.h +++ b/include/efi.h @@ -22,8 +22,16 @@ /* EFI on x86_64 uses the Microsoft ABI which is not the default for GCC */ #ifdef __x86_64__ #define EFIAPI __attribute__((ms_abi)) +#define efi_va_list __builtin_ms_va_list +#define efi_va_start __builtin_ms_va_start +#define efi_va_arg __builtin_va_arg +#define efi_va_end __builtin_ms_va_end #else #define EFIAPI asmlinkage +#define efi_va_list va_list +#define efi_va_start va_start +#define efi_va_arg va_arg +#define efi_va_end va_end #endif /* __x86_64__ */ struct efi_device_path; diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 50d311548e..404743fe01 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -2273,7 +2273,7 @@ static efi_status_t EFIAPI efi_install_multiple_protocol_interfaces( { EFI_ENTRY("%p", handle); - va_list argptr; + efi_va_list argptr; const efi_guid_t *protocol; void *protocol_interface; efi_status_t r = EFI_SUCCESS; @@ -2282,12 +2282,12 @@ static efi_status_t EFIAPI efi_install_multiple_protocol_interfaces( if (!handle) return EFI_EXIT(EFI_INVALID_PARAMETER); - va_start(argptr, handle); + efi_va_start(argptr, handle); for (;;) { - protocol = va_arg(argptr, efi_guid_t*); + protocol = efi_va_arg(argptr, efi_guid_t*); if (!protocol) break; - protocol_interface = va_arg(argptr, void*); + protocol_interface = efi_va_arg(argptr, void*); r = EFI_CALL(efi_install_protocol_interface( handle, protocol, EFI_NATIVE_INTERFACE, @@ -2296,19 +2296,19 @@ static efi_status_t EFIAPI efi_install_multiple_protocol_interfaces( break; i++; } - va_end(argptr); + efi_va_end(argptr); if (r == EFI_SUCCESS) return EFI_EXIT(r); /* If an error occurred undo all changes. */ - va_start(argptr, handle); + efi_va_start(argptr, handle); for (; i; --i) { - protocol = va_arg(argptr, efi_guid_t*); - protocol_interface = va_arg(argptr, void*); + protocol = efi_va_arg(argptr, efi_guid_t*); + protocol_interface = efi_va_arg(argptr, void*); EFI_CALL(efi_uninstall_protocol_interface(handle, protocol, protocol_interface)); } - va_end(argptr); + efi_va_end(argptr); return EFI_EXIT(r); } @@ -2332,7 +2332,7 @@ static efi_status_t EFIAPI efi_uninstall_multiple_protocol_interfaces( { EFI_ENTRY("%p", handle); - va_list argptr; + efi_va_list argptr; const efi_guid_t *protocol; void *protocol_interface; efi_status_t r = EFI_SUCCESS; @@ -2341,12 +2341,12 @@ static efi_status_t EFIAPI efi_uninstall_multiple_protocol_interfaces( if (!handle) return EFI_EXIT(EFI_INVALID_PARAMETER); - va_start(argptr, handle); + efi_va_start(argptr, handle); for (;;) { - protocol = va_arg(argptr, efi_guid_t*); + protocol = efi_va_arg(argptr, efi_guid_t*); if (!protocol) break; - protocol_interface = va_arg(argptr, void*); + protocol_interface = efi_va_arg(argptr, void*); r = EFI_CALL(efi_uninstall_protocol_interface( handle, protocol, protocol_interface)); @@ -2354,20 +2354,20 @@ static efi_status_t EFIAPI efi_uninstall_multiple_protocol_interfaces( break; i++; } - va_end(argptr); + efi_va_end(argptr); if (r == EFI_SUCCESS) return EFI_EXIT(r); /* If an error occurred undo all changes. */ - va_start(argptr, handle); + efi_va_start(argptr, handle); for (; i; --i) { - protocol = va_arg(argptr, efi_guid_t*); - protocol_interface = va_arg(argptr, void*); + protocol = efi_va_arg(argptr, efi_guid_t*); + protocol_interface = efi_va_arg(argptr, void*); EFI_CALL(efi_install_protocol_interface(&handle, protocol, EFI_NATIVE_INTERFACE, protocol_interface)); } - va_end(argptr); + efi_va_end(argptr); return EFI_EXIT(r); }
Varargs differ between sysv and ms abi. On x86_64 we have to follow the ms abi though, so we also need to make sure we use x86_64 varargs helpers. This patch introduces generic efi vararg helpers that adhere to the respective EFI ABI. That way we can deal with them properly from efi loader code and properly interpret variable arguments. This fixes the InstallMultipleProtocolInterfaces tests in the efi selftests on x86_64 for me. Signed-off-by: Alexander Graf <agraf@suse.de> --- include/efi.h | 8 ++++++++ lib/efi_loader/efi_boottime.c | 36 ++++++++++++++++++------------------ 2 files changed, 26 insertions(+), 18 deletions(-)