On 06/11/2018 06:35 PM, Bin Meng wrote: > Hi Heinrich, > > On Mon, Jun 11, 2018 at 11:31 PM, Heinrich Schuchardt > <xypron.g...@gmx.de> wrote: >> On 06/11/2018 01:36 AM, Bin Meng wrote: >>> Hi Heinrich, >>> >>> On Mon, Jun 11, 2018 at 2:17 AM, Heinrich Schuchardt <xypron.g...@gmx.de> >>> wrote: >>>> On 06/10/2018 04:30 PM, Bin Meng wrote: >>>>> Hi Heinrich, >>>>> >>>>> On Sun, Jun 10, 2018 at 10:02 PM, Heinrich Schuchardt >>>>> <xypron.g...@gmx.de> wrote: >>>>>> On 06/10/2018 03:25 PM, Bin Meng wrote: >>>>>>> Since commit bb0bb91cf0aa ("efi_stub: Use efi_uintn_t"), EFI x86 >>>>>>> 64-bit payload does not work anymore. The call to GetMemoryMap() >>>>>>> in efi_stub.c fails with return code EFI_INVALID_PARAMETER. Since >>>>>>> the payload itself is still 32-bit U-Boot >>>>>> >>>>>> Above you say 64-bit payload and now you say 32-bit? >>>>>> >>>>>> Why don't you compile U-Boot as 64-bit? How do you want to load a 64bit >>>>>> Linux EFI stub from an 32-bit EFI implementation in U-Boot? >>>>>> >>>>> >>>>> U-Boot itself as the EFI pyaload is 32-bit. The EFI stub is 64-bit as >>>>> it has to be loaded from the 64-bit EFI BIOS. Note in case you >>>>> misunderstand: the generated u-boot-payload.efi is 64-bit stub codes >>>>> (for 64-bit EFI BIOS) or 32-bit stub codes (for 32-bit EFI BIOS) plus >>>>> 32-bit U-Boot payload. The payload is always 32-bit as of today as >>>>> U-Boot on x86 is mainly on 32-bit. 64-bit support, as you see from >>>>> README.x86, is far from mature yet. >>>>> >>>>>>> , efi_uintn_t gets wrongly >>>>>>> interpreted as int, but it should actually be long in a 64-bit EFI >>>>>>> environment. >>>>>>> >>>>>>> Fixes: bb0bb91cf0aa ("efi_stub: Use efi_uintn_t") >>>>>>> Signed-off-by: Bin Meng <bmeng...@gmail.com> >>>>>>> --- >>>>>>> >>>>>>> include/efi_api.h | 4 ++++ >>>>>>> 1 file changed, 4 insertions(+) >>>>>>> >>>>>>> diff --git a/include/efi_api.h b/include/efi_api.h >>>>>>> index 64c27e4..d1158de 100644 >>>>>>> --- a/include/efi_api.h >>>>>>> +++ b/include/efi_api.h >>>>>>> @@ -28,7 +28,11 @@ enum efi_timer_delay { >>>>>>> EFI_TIMER_RELATIVE = 2 >>>>>>> }; >>>>>>> >>>>>>> +#if defined(CONFIG_EFI_STUB_64BIT) && defined(EFI_STUB) >>>>>>> +#define efi_uintn_t unsigned long >>>>>>> +#else >>>>>>> #define efi_uintn_t size_t >>>>>> >>>>>> NAK >>>>>> >>>>>> This change will create a lot of build warnings if EFI_STUB and >>>>>> EFI_LOADER are both configured. >>>>>> >>>>> >>>>> I don't see any build warnings when building efi-x86_payload32 or >>>>> efi-x86_payload64. I see both EFI_STUB and EFI_LOADER are enabled with >>>>> these two targets. AFAIK, only x86 supports EFI_STUB currently. I >>>>> don't know where you see a lot of build warnings. >>>> >>>> Currently you cannot build with EFI_LOADER=Y on 32 bit with a 64bit >>>> stub. See lib/efi_loader/Kconfig. The problem is with the build scripts >>>> for the stub using the same CONFIG variables as those used for other >>>> binaries. >>>> >>>> To emulate what would happen with your change once we can build with >>>> EFI_LOADER=y and 64bit stub I made the following change: >>>> >>>> --- a/include/efi_api.h >>>> +++ b/include/efi_api.h >>>> @@ -28,7 +28,8 @@ enum efi_timer_delay { >>>> EFI_TIMER_RELATIVE = 2 >>>> }; >>>> >>>> -#define efi_uintn_t size_t >>>> +#define efi_uintn_t unsigned long >>>> typedef uint16_t *efi_string_t; >>>> >>> >>> I am not sure why do you unconditionally change efi_uintn_t? My patch has >>> >>> #if defined(CONFIG_EFI_STUB_64BIT) && defined(EFI_STUB) >> >> Currently it is not possible to enable CONFIG_EFI_STUB_64BIT and >> EFI_LOADER on 32bit U-Boot. Once we have removed this restriction in the >> build system your patch together with EFI_LOADER=y and >> CONFIG_EFI_STUB_64BIT=y will provoke all those errors. This is why I >> NAKed you patch. >>
My mistake was to confuse EFI_STUB and CONFIG_EFI_STUB. EFI_STUB is only defined in lib/efi/Makefile when compiling lib/efi/efi.c and lib/efi/efi_stub.c These two files are compiled with -m64 when CONFIG_EFI_STUB_64BIT=y. The problem should be fixed in arch/x86/include/asm/posix_types.h That way anybody using size_t in lib/efi/ can be sure that size_t has the same size as a pointer, cf: lib/efi/efi_stub.c:92: void *memcpy(void *dest, const void *src, size_t size) lib/efi/efi_stub.c:104: void *memset(void *inptr, int ch, size_t size) > > I did the following to try to reproduce what you saw: > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > index d38780b..29e67b6 100644 > --- a/lib/efi_loader/Kconfig > +++ b/lib/efi_loader/Kconfig > @@ -3,10 +3,6 @@ config EFI_LOADER > depends on (ARM || X86) && OF_LIBFDT > # We do not support bootefi booting ARMv7 in non-secure mode > depends on !ARMV7_NONSEC > - # We need EFI_STUB_64BIT to be set on x86_64 with EFI_STUB > - depends on !EFI_STUB || !X86_64 || EFI_STUB_64BIT > - # We need EFI_STUB_32BIT to be set on x86_32 with EFI_STUB > - depends on !EFI_STUB || !X86 || X86_64 || EFI_STUB_32BIT > default y > select LIB_UUID > select HAVE_BLOCK_DEVICE > > Then do a > $ make efi-x86_payload32_defconfig > $ make > > It succeeded without any error > > But doing: > $ make efi-x86_payload64_defconfig > $ make > > Failed with a different error (no compiler warning were seen) > > toolchains/gcc-7.3.0-nolibc/i386-linux/bin/i386-linux-ld.bfd: i386 > architecture of input file `lib/efi_loader/helloworld.o' is > incompatible with i386:x86-64 output > scripts/Makefile.lib:407: recipe for target > 'lib/efi_loader/helloworld_efi.so' failed This is due to using variable EFI_LDS, EFI_CRT0, and EFI_RELOC both when building the stub (64bit) and helloworld (32bit) (see arch/x86/config.mk). To resolve the issue we would have to use different values for the stub and the other EFI payloads (helloworld and efi_selftest_miniapp_*). Best regards Heinrich > > Switching to x86_64-linux-gcc, exposed the same error: > > toolchains/gcc-7.3.0-nolibc/x86_64-linux/bin/x86_64-linux-ld.bfd: i386 > architecture of input file `lib/efi_loader/helloworld.o' is > incompatible with i386:x86-64 output > > Regards, > Bin > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot