[U-Boot] [PATCH v2 4/8] x86: Tidy up selection of building the EFI stub

[Alexander Graf]

On 09/29/2016 07:37 AM, Bin Meng wrote:
> Hi Alex,
>
> On Thu, Sep 29, 2016 at 1:08 PM, Alexander Graf <agraf at suse.de> wrote:
>>
>> Am 29.09.2016 um 05:37 schrieb Bin Meng <bmeng.cn at gmail.com>:
>>
>> Hi Simon,
>>
>> On Wed, Sep 28, 2016 at 10:43 PM, Simon Glass <sjg at chromium.org> wrote:
>>
>> Hi Bin,
>>
>>
>> On 27 September 2016 at 19:23, Bin Meng <bmeng.cn at gmail.com> wrote:
>>
>> Hi Simon,
>>
>>
>> On Wed, Sep 28, 2016 at 1:55 AM, Simon Glass <sjg at chromium.org> wrote:
>>
>> Hi Bin,
>>
>>
>> On 26 September 2016 at 20:44, Bin Meng <bmeng.cn at gmail.com> wrote:
>>
>> Hi Simon,
>>
>>
>> On Tue, Sep 27, 2016 at 8:35 AM, Simon Glass <sjg at chromium.org> wrote:
>>
>> Hi Bin,
>>
>>
>> On 26 September 2016 at 02:50, Bin Meng <bmeng.cn at gmail.com> wrote:
>>
>> Hi Simon,
>>
>>
>> On Mon, Sep 26, 2016 at 5:27 AM, Simon Glass <sjg at chromium.org> wrote:
>>
>> At present we use a CONFIG option in efi.h to determine whether we are
>>
>> building the EFI stub or not. This means that the same header cannot be
>>
>> used for EFI_LOADER support. The CONFIG option will be enabled for the
>>
>> whole build, even when not building the stub.
>>
>>
>> Use a different define instead, set up just for the files that make up the
>>
>> stub.
>>
>>
>> Signed-off-by: Simon Glass <sjg at chromium.org>
>>
>> ---
>>
>>
>> Changes in v2:
>>
>> - Add new patch to tidy up selection of building the EFI stub
>>
>>
>> include/efi.h    | 7 +++++--
>>
>> lib/efi/Makefile | 4 ++--
>>
>> 2 files changed, 7 insertions(+), 4 deletions(-)
>>
>>
>> diff --git a/include/efi.h b/include/efi.h
>>
>> index d07187c..3d58780 100644
>>
>> --- a/include/efi.h
>>
>> +++ b/include/efi.h
>>
>> @@ -30,8 +30,11 @@ struct efi_device_path;
>>
>>
>> #define EFI_BITS_PER_LONG      BITS_PER_LONG
>>
>>
>> -/* With 64-bit EFI stub, EFI_BITS_PER_LONG has to be 64 */
>>
>> -#ifdef CONFIG_EFI_STUB_64BIT
>>
>> +/*
>>
>> + * With 64-bit EFI stub, EFI_BITS_PER_LONG has to be 64. EFI_STUB is set
>>
>> + * in lib/efi/Makefile, when building the stub.
>>
>> + */
>>
>> +#if defined(CONFIG_EFI_STUB_64BIT) && defined(EFI_STUB)
>>
>>
>> I don't understand why this is needed?
>>
>>
>> If building the 64-bit EFI stub, we need to use 64-bit ints for the
>>
>> stub, but 32-bits for the rest of U-Boot. So this header gets used
>>
>> both ways.
>>
>>
>>
>> For 32-bit EFI stub or U-Boot itself, EFI_BITS_PER_LONG is defined as
>>
>> BITS_PER_LONG which is 32.
>>
>>
>> Yes that's right. But in the case of the stub, it can be different
>>
>> from U-Boot itself. This case takes care of that.
>>
>>
>>
>> Sorry but I still don't get it. What's broken without this change?
>>
>>
>> When building U-Boot with CONFIG_EFI_STUB_64BIT enabled, at present
>>
>> EFI_BITS_PER_LONG will be 64.
>>
>>
>> This is fine for building the stub.
>>
>>
>>
>> Yes
>>
>> But for building U-Boot, we still want it to be 32.
>>
>>
>>
>> Yes
>>
>> At present the code overrides EFI_BITS_PER_LONG, setting it to 64 if
>>
>> CONFIG_EFI_STUB_64BIT is enabled.
>>
>>
>> This means that EFI_LOADER support does not build properly, since it
>>
>> uses 64 instead of 32.
>>
>>
>>
>> So you want CONFIG_EFI_STUB_64BIT and CONFIG_EFI_LOADER both be
>> defined? I don't think it is a valid configuration.
>>
>>
>> Why not?
>>
> So the board has a 64-bit UEFI BIOS, and with CONFIG_EFI_STUB_64BIT we
> build U-Boot as a 64-bit UEFI payload and let the UEFI BIOS boot the
> payload (U-Boot), then with CONFIG_EFI_LOADER we are trying to provide
> the UEFI runtime environment within the U-Boot. What value are we
> looking for? This is asking for troubles.

Why is this asking for trouble? The inner uefi payload has no idea that 
the outer uefi firmware exists. It only ever talks to u-boot. I would 
argue the other way around: If we can't make it work, we have a layering 
problem.


Alex