On 09/29/2016 09:28 AM, Bin Meng wrote:
Hi Alex,

On Thu, Sep 29, 2016 at 3:13 PM, Alexander Graf <ag...@suse.de> wrote:
On 09/29/2016 07:37 AM, Bin Meng wrote:
Hi Alex,

On Thu, Sep 29, 2016 at 1:08 PM, Alexander Graf <ag...@suse.de> wrote:

Am 29.09.2016 um 05:37 schrieb Bin Meng <bmeng...@gmail.com>:

Hi Simon,

On Wed, Sep 28, 2016 at 10:43 PM, Simon Glass <s...@chromium.org> wrote:

Hi Bin,


On 27 September 2016 at 19:23, Bin Meng <bmeng...@gmail.com> wrote:

Hi Simon,


On Wed, Sep 28, 2016 at 1:55 AM, Simon Glass <s...@chromium.org> wrote:

Hi Bin,


On 26 September 2016 at 20:44, Bin Meng <bmeng...@gmail.com> wrote:

Hi Simon,


On Tue, Sep 27, 2016 at 8:35 AM, Simon Glass <s...@chromium.org> wrote:

Hi Bin,


On 26 September 2016 at 02:50, Bin Meng <bmeng...@gmail.com> wrote:

Hi Simon,


On Mon, Sep 26, 2016 at 5:27 AM, Simon Glass <s...@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 <s...@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.

This shows no value to me. In the end, providing EFI loader in the
U-Boot is to load some EFI apps. But this can be done from the
original UEFI BIOS without the need to have the middle-stage U-Boot
payload.

You could say the same about running U-Boot on EFI. You can as well just load your payload from the original uEFI firmware :).

What layering problem do we want to fix here? Are you saying
testing EFI loader in the U-Boot is not enough, so that we should
support such configuration for additional testing?

I'm saying that I don't see anything that speaks against it working, so why should we disable it? At least for prototyping it's a convenient option to have and in general divergence is a bad thing.


Alex

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to