[U-Boot] [PATCH 05/18] efi: Fix efi_uintn_t for 64-bit EFI payload

[Bin Meng]

Hi Heinrich,

On Sun, Jun 10, 2018 at 10:02 PM, Heinrich Schuchardt
<xypron.glpk at 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.cn at 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.

> Could you, please, explain under which compiler settings size_t and
> unsigned long have a different number of bits?
>

As mentioned above, the EFI stub codes (efi_stub.c) are built for
64-bit, which expects 'efi_uintn_t' to be 64-bit. However, it is
defined as size_t, which in U-Boot 32, will be 'unsigned int' (see
arch/x86/include/asm/posix_types.h) - the bug here!

> Obviously we have the EFI API exposed by U-Boot which has the bitness of
> U-Boot.
>
> If you want to consume an EFI API of another bitness I suggest that you
> create separate interface definitions.
>

Regards,
Bin