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

[Bin Meng]

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.

Regards,
Bin