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

[Bin Meng]

Hi Heinrich,

On Mon, Jun 11, 2018 at 2:17 AM, Heinrich Schuchardt <xypron.glpk at 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.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.
>
> 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)

to guard the change.

> And then I tried to build:
> make qemu-x86_defconfig
> make -j6
>

I know. But my patch does not produce any build warnings on
efi-x86_payload32_defconfig and efi-x86_payload64_defconfig, on
u-boot-x86/efi-working branch.

> It gives me a bunch of errors like below.
>

[snip]

Regards,
Bin