Hi Alex, On Thu, Sep 29, 2016 at 4:06 PM, Alexander Graf <[email protected]> wrote: > On 09/29/2016 09:28 AM, Bin Meng wrote: >> >> Hi Alex, >> >> On Thu, Sep 29, 2016 at 3:13 PM, Alexander Graf <[email protected]> wrote: >>> >>> On 09/29/2016 07:37 AM, Bin Meng wrote: >>>> >>>> Hi Alex, >>>> >>>> On Thu, Sep 29, 2016 at 1:08 PM, Alexander Graf <[email protected]> wrote: >>>>> >>>>> >>>>> Am 29.09.2016 um 05:37 schrieb Bin Meng <[email protected]>: >>>>> >>>>> Hi Simon, >>>>> >>>>> On Wed, Sep 28, 2016 at 10:43 PM, Simon Glass <[email protected]> wrote: >>>>> >>>>> Hi Bin, >>>>> >>>>> >>>>> On 27 September 2016 at 19:23, Bin Meng <[email protected]> wrote: >>>>> >>>>> Hi Simon, >>>>> >>>>> >>>>> On Wed, Sep 28, 2016 at 1:55 AM, Simon Glass <[email protected]> wrote: >>>>> >>>>> Hi Bin, >>>>> >>>>> >>>>> On 26 September 2016 at 20:44, Bin Meng <[email protected]> wrote: >>>>> >>>>> Hi Simon, >>>>> >>>>> >>>>> On Tue, Sep 27, 2016 at 8:35 AM, Simon Glass <[email protected]> wrote: >>>>> >>>>> Hi Bin, >>>>> >>>>> >>>>> On 26 September 2016 at 02:50, Bin Meng <[email protected]> wrote: >>>>> >>>>> Hi Simon, >>>>> >>>>> >>>>> On Mon, Sep 26, 2016 at 5:27 AM, Simon Glass <[email protected]> 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 <[email protected]> >>>>> >>>>> --- >>>>> >>>>> >>>>> 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 :).
Wait, they are different things. EFI loader in U-Boot assumes U-Boot is the bootloader and we want a EFI runtime environment to load EFI payload without the need to have a full UEFI BIOS. I see value in supporting EFI loader in U-Boot. > >> 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. But this one, value-wise, no, but yes, technically we can make such configuration work :) Regards, Bin _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
