diff mbox

[U-Boot,v2,5/6] efi_loader: Allow to compile helloworld.efi w/o bundling it

Message ID 1479403867-54792-6-git-send-email-agraf@suse.de
State Superseded
Headers show

Commit Message

Alexander Graf Nov. 17, 2016, 5:31 p.m. UTC
Today we can compile a self-contained hello world efi test binary that
allows us to quickly verify whether the EFI loader framwork works.

We can use that binary outside of the self-contained test case though,
by providing it to a to-be-tested system via tftp.

This patch separates compilation of the helloworld.efi file from
including it in the u-boot binary for "bootefi hello". It also modifies
the efi_loader test case to enable travis to pick up the compiled file.
Because we're now no longer bloating the resulting u-boot binary, we
can enable compilation always, giving us good travis test coverage.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 arch/arm/lib/Makefile                    |  2 +-
 arch/x86/config.mk                       |  2 +-
 arch/x86/lib/Makefile                    |  2 +-
 cmd/Kconfig                              | 15 ++++++++++++++-
 configs/qemu-x86_efi_payload64_defconfig |  1 +
 lib/efi_loader/Makefile                  |  3 +++
 test/py/tests/test_efi_loader.py         |  2 +-
 7 files changed, 22 insertions(+), 5 deletions(-)

Comments

Simon Glass Nov. 19, 2016, 1:48 p.m. UTC | #1
Hi Alex,

On 17 November 2016 at 10:31, Alexander Graf <agraf@suse.de> wrote:
> Today we can compile a self-contained hello world efi test binary that
> allows us to quickly verify whether the EFI loader framwork works.
>
> We can use that binary outside of the self-contained test case though,
> by providing it to a to-be-tested system via tftp.
>
> This patch separates compilation of the helloworld.efi file from
> including it in the u-boot binary for "bootefi hello". It also modifies
> the efi_loader test case to enable travis to pick up the compiled file.
> Because we're now no longer bloating the resulting u-boot binary, we
> can enable compilation always, giving us good travis test coverage.
>
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  arch/arm/lib/Makefile                    |  2 +-
>  arch/x86/config.mk                       |  2 +-
>  arch/x86/lib/Makefile                    |  2 +-
>  cmd/Kconfig                              | 15 ++++++++++++++-
>  configs/qemu-x86_efi_payload64_defconfig |  1 +
>  lib/efi_loader/Makefile                  |  3 +++
>  test/py/tests/test_efi_loader.py         |  2 +-
>  7 files changed, 22 insertions(+), 5 deletions(-)

Ick.

Can you not achieve the same effect just by copying the file somewhere?

Regards,
Simon
Alexander Graf Nov. 19, 2016, 2:13 p.m. UTC | #2
On 19/11/2016 14:48, Simon Glass wrote:
> Hi Alex,
>
> On 17 November 2016 at 10:31, Alexander Graf <agraf@suse.de> wrote:
>> Today we can compile a self-contained hello world efi test binary that
>> allows us to quickly verify whether the EFI loader framwork works.
>>
>> We can use that binary outside of the self-contained test case though,
>> by providing it to a to-be-tested system via tftp.
>>
>> This patch separates compilation of the helloworld.efi file from
>> including it in the u-boot binary for "bootefi hello". It also modifies
>> the efi_loader test case to enable travis to pick up the compiled file.
>> Because we're now no longer bloating the resulting u-boot binary, we
>> can enable compilation always, giving us good travis test coverage.
>>
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>> ---
>>  arch/arm/lib/Makefile                    |  2 +-
>>  arch/x86/config.mk                       |  2 +-
>>  arch/x86/lib/Makefile                    |  2 +-
>>  cmd/Kconfig                              | 15 ++++++++++++++-
>>  configs/qemu-x86_efi_payload64_defconfig |  1 +
>>  lib/efi_loader/Makefile                  |  3 +++
>>  test/py/tests/test_efi_loader.py         |  2 +-
>>  7 files changed, 22 insertions(+), 5 deletions(-)
>
> Ick.
>
> Can you not achieve the same effect just by copying the file somewhere?

Sure, we could. But the file is only defined inside the env of the 
particular test case. So if you want to test against non-travis, you can 
copy it wherever you like.

This way the travis description simplifies a lot, because we can just 
expose the build directory as tftp root.


Alex
Simon Glass Nov. 19, 2016, 8:02 p.m. UTC | #3
Hi Alex,

On 19 November 2016 at 07:13, Alexander Graf <agraf@suse.de> wrote:
>
>
> On 19/11/2016 14:48, Simon Glass wrote:
>>
>> Hi Alex,
>>
>> On 17 November 2016 at 10:31, Alexander Graf <agraf@suse.de> wrote:
>>>
>>> Today we can compile a self-contained hello world efi test binary that
>>> allows us to quickly verify whether the EFI loader framwork works.
>>>
>>> We can use that binary outside of the self-contained test case though,
>>> by providing it to a to-be-tested system via tftp.
>>>
>>> This patch separates compilation of the helloworld.efi file from
>>> including it in the u-boot binary for "bootefi hello". It also modifies
>>> the efi_loader test case to enable travis to pick up the compiled file.
>>> Because we're now no longer bloating the resulting u-boot binary, we
>>> can enable compilation always, giving us good travis test coverage.
>>>
>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>> ---
>>>  arch/arm/lib/Makefile                    |  2 +-
>>>  arch/x86/config.mk                       |  2 +-
>>>  arch/x86/lib/Makefile                    |  2 +-
>>>  cmd/Kconfig                              | 15 ++++++++++++++-
>>>  configs/qemu-x86_efi_payload64_defconfig |  1 +
>>>  lib/efi_loader/Makefile                  |  3 +++
>>>  test/py/tests/test_efi_loader.py         |  2 +-
>>>  7 files changed, 22 insertions(+), 5 deletions(-)
>>
>>
>> Ick.
>>
>> Can you not achieve the same effect just by copying the file somewhere?
>
>
> Sure, we could. But the file is only defined inside the env of the
> particular test case. So if you want to test against non-travis, you can
> copy it wherever you like.
>
> This way the travis description simplifies a lot, because we can just expose
> the build directory as tftp root.

Or use .PRECIOUS on the existing file? You could copy it into the root
directory of the build, perhaps? It just seems like a lot of extra
stuff for a file that is already built.

Regards,
Simon
Alexander Graf Nov. 19, 2016, 9:47 p.m. UTC | #4
> Am 19.11.2016 um 21:02 schrieb Simon Glass <sjg@chromium.org>:
> 
> Hi Alex,
> 
>> On 19 November 2016 at 07:13, Alexander Graf <agraf@suse.de> wrote:
>> 
>> 
>>> On 19/11/2016 14:48, Simon Glass wrote:
>>> 
>>> Hi Alex,
>>> 
>>>> On 17 November 2016 at 10:31, Alexander Graf <agraf@suse.de> wrote:
>>>> 
>>>> Today we can compile a self-contained hello world efi test binary that
>>>> allows us to quickly verify whether the EFI loader framwork works.
>>>> 
>>>> We can use that binary outside of the self-contained test case though,
>>>> by providing it to a to-be-tested system via tftp.
>>>> 
>>>> This patch separates compilation of the helloworld.efi file from
>>>> including it in the u-boot binary for "bootefi hello". It also modifies
>>>> the efi_loader test case to enable travis to pick up the compiled file.
>>>> Because we're now no longer bloating the resulting u-boot binary, we
>>>> can enable compilation always, giving us good travis test coverage.
>>>> 
>>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>>> ---
>>>> arch/arm/lib/Makefile                    |  2 +-
>>>> arch/x86/config.mk                       |  2 +-
>>>> arch/x86/lib/Makefile                    |  2 +-
>>>> cmd/Kconfig                              | 15 ++++++++++++++-
>>>> configs/qemu-x86_efi_payload64_defconfig |  1 +
>>>> lib/efi_loader/Makefile                  |  3 +++
>>>> test/py/tests/test_efi_loader.py         |  2 +-
>>>> 7 files changed, 22 insertions(+), 5 deletions(-)
>>> 
>>> 
>>> Ick.
>>> 
>>> Can you not achieve the same effect just by copying the file somewhere?
>> 
>> 
>> Sure, we could. But the file is only defined inside the env of the
>> particular test case. So if you want to test against non-travis, you can
>> copy it wherever you like.
>> 
>> This way the travis description simplifies a lot, because we can just expose
>> the build directory as tftp root.
> 
> Or use .PRECIOUS on the existing file? You could copy it into the root
> directory of the build, perhaps? It just seems like a lot of extra
> stuff for a file that is already built.

I want to make sure that by default we never compile the hello world efi example into the u-boot binary, but still have the file build tested and available for travis.

Alex
Simon Glass Nov. 19, 2016, 11:56 p.m. UTC | #5
Hi Alex,

On 19 November 2016 at 14:47, Alexander Graf <agraf@suse.de> wrote:
>
>
>> Am 19.11.2016 um 21:02 schrieb Simon Glass <sjg@chromium.org>:
>>
>> Hi Alex,
>>
>>> On 19 November 2016 at 07:13, Alexander Graf <agraf@suse.de> wrote:
>>>
>>>
>>>> On 19/11/2016 14:48, Simon Glass wrote:
>>>>
>>>> Hi Alex,
>>>>
>>>>> On 17 November 2016 at 10:31, Alexander Graf <agraf@suse.de> wrote:
>>>>>
>>>>> Today we can compile a self-contained hello world efi test binary that
>>>>> allows us to quickly verify whether the EFI loader framwork works.
>>>>>
>>>>> We can use that binary outside of the self-contained test case though,
>>>>> by providing it to a to-be-tested system via tftp.
>>>>>
>>>>> This patch separates compilation of the helloworld.efi file from
>>>>> including it in the u-boot binary for "bootefi hello". It also modifies
>>>>> the efi_loader test case to enable travis to pick up the compiled file.
>>>>> Because we're now no longer bloating the resulting u-boot binary, we
>>>>> can enable compilation always, giving us good travis test coverage.
>>>>>
>>>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>>>> ---
>>>>> arch/arm/lib/Makefile                    |  2 +-
>>>>> arch/x86/config.mk                       |  2 +-
>>>>> arch/x86/lib/Makefile                    |  2 +-
>>>>> cmd/Kconfig                              | 15 ++++++++++++++-
>>>>> configs/qemu-x86_efi_payload64_defconfig |  1 +
>>>>> lib/efi_loader/Makefile                  |  3 +++
>>>>> test/py/tests/test_efi_loader.py         |  2 +-
>>>>> 7 files changed, 22 insertions(+), 5 deletions(-)
>>>>
>>>>
>>>> Ick.
>>>>
>>>> Can you not achieve the same effect just by copying the file somewhere?
>>>
>>>
>>> Sure, we could. But the file is only defined inside the env of the
>>> particular test case. So if you want to test against non-travis, you can
>>> copy it wherever you like.
>>>
>>> This way the travis description simplifies a lot, because we can just expose
>>> the build directory as tftp root.
>>
>> Or use .PRECIOUS on the existing file? You could copy it into the root
>> directory of the build, perhaps? It just seems like a lot of extra
>> stuff for a file that is already built.
>
> I want to make sure that by default we never compile the hello world efi example into the u-boot binary, but still have the file build tested and available for travis.

So how about just having two cases:

1. Compile hello world and produce it as an output
2. As 1 but also build it into the U-Boot binary

The first one could be controlled by EFI_LOADER, the second with the
existing option for the 'bootefi hello' command.

Regards,
Simon
Alexander Graf Nov. 20, 2016, 12:13 a.m. UTC | #6
> Am 20.11.2016 um 00:56 schrieb Simon Glass <sjg@chromium.org>:
> 
> Hi Alex,
> 
>> On 19 November 2016 at 14:47, Alexander Graf <agraf@suse.de> wrote:
>> 
>> 
>>> Am 19.11.2016 um 21:02 schrieb Simon Glass <sjg@chromium.org>:
>>> 
>>> Hi Alex,
>>> 
>>>> On 19 November 2016 at 07:13, Alexander Graf <agraf@suse.de> wrote:
>>>> 
>>>> 
>>>>> On 19/11/2016 14:48, Simon Glass wrote:
>>>>> 
>>>>> Hi Alex,
>>>>> 
>>>>>> On 17 November 2016 at 10:31, Alexander Graf <agraf@suse.de> wrote:
>>>>>> 
>>>>>> Today we can compile a self-contained hello world efi test binary that
>>>>>> allows us to quickly verify whether the EFI loader framwork works.
>>>>>> 
>>>>>> We can use that binary outside of the self-contained test case though,
>>>>>> by providing it to a to-be-tested system via tftp.
>>>>>> 
>>>>>> This patch separates compilation of the helloworld.efi file from
>>>>>> including it in the u-boot binary for "bootefi hello". It also modifies
>>>>>> the efi_loader test case to enable travis to pick up the compiled file.
>>>>>> Because we're now no longer bloating the resulting u-boot binary, we
>>>>>> can enable compilation always, giving us good travis test coverage.
>>>>>> 
>>>>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>>>>> ---
>>>>>> arch/arm/lib/Makefile                    |  2 +-
>>>>>> arch/x86/config.mk                       |  2 +-
>>>>>> arch/x86/lib/Makefile                    |  2 +-
>>>>>> cmd/Kconfig                              | 15 ++++++++++++++-
>>>>>> configs/qemu-x86_efi_payload64_defconfig |  1 +
>>>>>> lib/efi_loader/Makefile                  |  3 +++
>>>>>> test/py/tests/test_efi_loader.py         |  2 +-
>>>>>> 7 files changed, 22 insertions(+), 5 deletions(-)
>>>>> 
>>>>> 
>>>>> Ick.
>>>>> 
>>>>> Can you not achieve the same effect just by copying the file somewhere?
>>>> 
>>>> 
>>>> Sure, we could. But the file is only defined inside the env of the
>>>> particular test case. So if you want to test against non-travis, you can
>>>> copy it wherever you like.
>>>> 
>>>> This way the travis description simplifies a lot, because we can just expose
>>>> the build directory as tftp root.
>>> 
>>> Or use .PRECIOUS on the existing file? You could copy it into the root
>>> directory of the build, perhaps? It just seems like a lot of extra
>>> stuff for a file that is already built.
>> 
>> I want to make sure that by default we never compile the hello world efi example into the u-boot binary, but still have the file build tested and available for travis.
> 
> So how about just having two cases:
> 
> 1. Compile hello world and produce it as an output
> 2. As 1 but also build it into the U-Boot binary

Yes, that's precisely what this patch does :). I'm glad we agree.

> 
> The first one could be controlled by EFI_LOADER,

Unfortunately the hello world binary doesn't build on stm32 while there is no reason to disable EFI_LOADER on that platform, so I want to keep the options separately.

Also, if someone comes in and enables a new architecture, I would like to make the bar as low as I can for that. For that reason too, I would prefer to keep it as a separate config option.

> the second with the
> existing option for the 'bootefi hello' command.

Yes, that too is what the patch does :).

Alex
Simon Glass Nov. 22, 2016, 5:50 p.m. UTC | #7
Hi Alex,

On 19 November 2016 at 17:13, Alexander Graf <agraf@suse.de> wrote:
>
>
>> Am 20.11.2016 um 00:56 schrieb Simon Glass <sjg@chromium.org>:
>>
>> Hi Alex,
>>
>>> On 19 November 2016 at 14:47, Alexander Graf <agraf@suse.de> wrote:
>>>
>>>
>>>> Am 19.11.2016 um 21:02 schrieb Simon Glass <sjg@chromium.org>:
>>>>
>>>> Hi Alex,
>>>>
>>>>> On 19 November 2016 at 07:13, Alexander Graf <agraf@suse.de> wrote:
>>>>>
>>>>>
>>>>>> On 19/11/2016 14:48, Simon Glass wrote:
>>>>>>
>>>>>> Hi Alex,
>>>>>>
>>>>>>> On 17 November 2016 at 10:31, Alexander Graf <agraf@suse.de> wrote:
>>>>>>>
>>>>>>> Today we can compile a self-contained hello world efi test binary that
>>>>>>> allows us to quickly verify whether the EFI loader framwork works.
>>>>>>>
>>>>>>> We can use that binary outside of the self-contained test case though,
>>>>>>> by providing it to a to-be-tested system via tftp.
>>>>>>>
>>>>>>> This patch separates compilation of the helloworld.efi file from
>>>>>>> including it in the u-boot binary for "bootefi hello". It also modifies
>>>>>>> the efi_loader test case to enable travis to pick up the compiled file.
>>>>>>> Because we're now no longer bloating the resulting u-boot binary, we
>>>>>>> can enable compilation always, giving us good travis test coverage.
>>>>>>>
>>>>>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>>>>>> ---
>>>>>>> arch/arm/lib/Makefile                    |  2 +-
>>>>>>> arch/x86/config.mk                       |  2 +-
>>>>>>> arch/x86/lib/Makefile                    |  2 +-
>>>>>>> cmd/Kconfig                              | 15 ++++++++++++++-
>>>>>>> configs/qemu-x86_efi_payload64_defconfig |  1 +
>>>>>>> lib/efi_loader/Makefile                  |  3 +++
>>>>>>> test/py/tests/test_efi_loader.py         |  2 +-
>>>>>>> 7 files changed, 22 insertions(+), 5 deletions(-)
>>>>>>
>>>>>>
>>>>>> Ick.
>>>>>>
>>>>>> Can you not achieve the same effect just by copying the file somewhere?
>>>>>
>>>>>
>>>>> Sure, we could. But the file is only defined inside the env of the
>>>>> particular test case. So if you want to test against non-travis, you can
>>>>> copy it wherever you like.
>>>>>
>>>>> This way the travis description simplifies a lot, because we can just expose
>>>>> the build directory as tftp root.
>>>>
>>>> Or use .PRECIOUS on the existing file? You could copy it into the root
>>>> directory of the build, perhaps? It just seems like a lot of extra
>>>> stuff for a file that is already built.
>>>
>>> I want to make sure that by default we never compile the hello world efi example into the u-boot binary, but still have the file build tested and available for travis.
>>
>> So how about just having two cases:
>>
>> 1. Compile hello world and produce it as an output
>> 2. As 1 but also build it into the U-Boot binary
>
> Yes, that's precisely what this patch does :). I'm glad we agree.

Except that I don't think we need the extra CONFIG.

>
>>
>> The first one could be controlled by EFI_LOADER,
>
> Unfortunately the hello world binary doesn't build on stm32 while there is no reason to disable EFI_LOADER on that platform, so I want to keep the options separately.

Well if no one can compile for stm32 then it is unlikely to work
anyway. Does anyone actually use Thumb 1 with EFI?

>
> Also, if someone comes in and enables a new architecture, I would like to make the bar as low as I can for that. For that reason too, I would prefer to keep it as a separate config option.

I think you might have it backwards. As someone who just enabled a new
architecture (x86) I can tell you that the best approach by far was to
use the embedded hello world test. In fact that is why I wrote it. It
provides a fast and easy to test to allow things to be brought up.
Using something like grub is so much more painful.

>
>> the second with the
>> existing option for the 'bootefi hello' command.
>
> Yes, that too is what the patch does :).

So I think we should disable the one stm32 board until someone
actually gets it working, at least with the hello world test.

Regards,
Simon
Tom Rini Nov. 22, 2016, 6:27 p.m. UTC | #8
On Tue, Nov 22, 2016 at 10:50:50AM -0700, Simon Glass wrote:
> Hi Alex,
> 
> On 19 November 2016 at 17:13, Alexander Graf <agraf@suse.de> wrote:
[snip]
> > Unfortunately the hello world binary doesn't build on stm32 while there is no reason to disable EFI_LOADER on that platform, so I want to keep the options separately.
> 
> Well if no one can compile for stm32 then it is unlikely to work
> anyway. Does anyone actually use Thumb 1 with EFI?

On just this point, maybe the right answer is to say we turn off EFI for
STM32 as no, that sounds a bit overkill.
Alexander Graf Nov. 22, 2016, 6:48 p.m. UTC | #9
> Am 22.11.2016 um 19:27 schrieb Tom Rini <trini@konsulko.com>:
> 
>> On Tue, Nov 22, 2016 at 10:50:50AM -0700, Simon Glass wrote:
>> Hi Alex,
>> 
>> On 19 November 2016 at 17:13, Alexander Graf <agraf@suse.de> wrote:
> [snip]
>>> Unfortunately the hello world binary doesn't build on stm32 while there is no reason to disable EFI_LOADER on that platform, so I want to keep the options separately.
>> 
>> Well if no one can compile for stm32 then it is unlikely to work
>> anyway. Does anyone actually use Thumb 1 with EFI?
> 
> On just this point, maybe the right answer is to say we turn off EFI for
> STM32 as no, that sounds a bit overkill.

EFI support should work just fine with Thumb 1. the hello world crt0 is what's breaking.

Alex

> 
> -- 
> Tom
Alexander Graf Nov. 22, 2016, 8:49 p.m. UTC | #10
On 22/11/2016 18:50, Simon Glass wrote:
> Hi Alex,
>
> On 19 November 2016 at 17:13, Alexander Graf <agraf@suse.de> wrote:
>>
>>
>>> Am 20.11.2016 um 00:56 schrieb Simon Glass <sjg@chromium.org>:
>>>
>>> Hi Alex,
>>>
>>>> On 19 November 2016 at 14:47, Alexander Graf <agraf@suse.de> wrote:
>>>>
>>>>
>>>>> Am 19.11.2016 um 21:02 schrieb Simon Glass <sjg@chromium.org>:
>>>>>
>>>>> Hi Alex,
>>>>>
>>>>>> On 19 November 2016 at 07:13, Alexander Graf <agraf@suse.de> wrote:
>>>>>>
>>>>>>
>>>>>>> On 19/11/2016 14:48, Simon Glass wrote:
>>>>>>>
>>>>>>> Hi Alex,
>>>>>>>
>>>>>>>> On 17 November 2016 at 10:31, Alexander Graf <agraf@suse.de> wrote:
>>>>>>>>
>>>>>>>> Today we can compile a self-contained hello world efi test binary that
>>>>>>>> allows us to quickly verify whether the EFI loader framwork works.
>>>>>>>>
>>>>>>>> We can use that binary outside of the self-contained test case though,
>>>>>>>> by providing it to a to-be-tested system via tftp.
>>>>>>>>
>>>>>>>> This patch separates compilation of the helloworld.efi file from
>>>>>>>> including it in the u-boot binary for "bootefi hello". It also modifies
>>>>>>>> the efi_loader test case to enable travis to pick up the compiled file.
>>>>>>>> Because we're now no longer bloating the resulting u-boot binary, we
>>>>>>>> can enable compilation always, giving us good travis test coverage.
>>>>>>>>
>>>>>>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>>>>>>> ---
>>>>>>>> arch/arm/lib/Makefile                    |  2 +-
>>>>>>>> arch/x86/config.mk                       |  2 +-
>>>>>>>> arch/x86/lib/Makefile                    |  2 +-
>>>>>>>> cmd/Kconfig                              | 15 ++++++++++++++-
>>>>>>>> configs/qemu-x86_efi_payload64_defconfig |  1 +
>>>>>>>> lib/efi_loader/Makefile                  |  3 +++
>>>>>>>> test/py/tests/test_efi_loader.py         |  2 +-
>>>>>>>> 7 files changed, 22 insertions(+), 5 deletions(-)
>>>>>>>
>>>>>>>
>>>>>>> Ick.
>>>>>>>
>>>>>>> Can you not achieve the same effect just by copying the file somewhere?
>>>>>>
>>>>>>
>>>>>> Sure, we could. But the file is only defined inside the env of the
>>>>>> particular test case. So if you want to test against non-travis, you can
>>>>>> copy it wherever you like.
>>>>>>
>>>>>> This way the travis description simplifies a lot, because we can just expose
>>>>>> the build directory as tftp root.
>>>>>
>>>>> Or use .PRECIOUS on the existing file? You could copy it into the root
>>>>> directory of the build, perhaps? It just seems like a lot of extra
>>>>> stuff for a file that is already built.
>>>>
>>>> I want to make sure that by default we never compile the hello world efi example into the u-boot binary, but still have the file build tested and available for travis.
>>>
>>> So how about just having two cases:
>>>
>>> 1. Compile hello world and produce it as an output
>>> 2. As 1 but also build it into the U-Boot binary
>>
>> Yes, that's precisely what this patch does :). I'm glad we agree.
>
> Except that I don't think we need the extra CONFIG.

If that's the only disagreement we have, then let's have the extra 
CONFIG. Having more usually shouldn't hurt.

>>> The first one could be controlled by EFI_LOADER,
>>
>> Unfortunately the hello world binary doesn't build on stm32 while there is no reason to disable EFI_LOADER on that platform, so I want to keep the options separately.
>
> Well if no one can compile for stm32 then it is unlikely to work
> anyway. Does anyone actually use Thumb 1 with EFI?

I've verified thumb 1 back in the day when I did the setjmp/longjmp 
implementation, yeah.

>> Also, if someone comes in and enables a new architecture, I would like to make the bar as low as I can for that. For that reason too, I would prefer to keep it as a separate config option.
>
> I think you might have it backwards. As someone who just enabled a new
> architecture (x86) I can tell you that the best approach by far was to
> use the embedded hello world test. In fact that is why I wrote it. It
> provides a fast and easy to test to allow things to be brought up.
> Using something like grub is so much more painful.

I think you might have it backwards :). Different people have different 
approaches to problems. If someone wants to port the hello world example 
first, I'm more than happy to have them do it. But if their flow is 
different, I'm not going to be the one standing in their way.

My goal is to have EFI support enabled and working well for as many 
devices as we can. The path to get there doesn't matter that much to me.

>
>>
>>> the second with the
>>> existing option for the 'bootefi hello' command.
>>
>> Yes, that too is what the patch does :).
>
> So I think we should disable the one stm32 board until someone
> actually gets it working, at least with the hello world test.

Can you prove that it doesn't work? So far the only thing that breaks is 
your new hello world test code.


Alex
diff mbox

Patch

diff --git a/arch/arm/lib/Makefile b/arch/arm/lib/Makefile
index a812306..0051f76 100644
--- a/arch/arm/lib/Makefile
+++ b/arch/arm/lib/Makefile
@@ -100,5 +100,5 @@  CFLAGS_REMOVE_$(EFI_CRT0) := $(CFLAGS_NON_EFI)
 CFLAGS_$(EFI_RELOC) := $(CFLAGS_EFI)
 CFLAGS_REMOVE_$(EFI_RELOC) := $(CFLAGS_NON_EFI)
 
-extra-$(CONFIG_CMD_BOOTEFI_HELLO) += $(EFI_CRT0) $(EFI_RELOC)
+extra-$(CONFIG_CMD_BOOTEFI_HELLO_COMPILE) += $(EFI_CRT0) $(EFI_RELOC)
 extra-$(CONFIG_EFI) += $(EFI_CRT0) $(EFI_RELOC)
diff --git a/arch/x86/config.mk b/arch/x86/config.mk
index 03c71f7..1697dca 100644
--- a/arch/x86/config.mk
+++ b/arch/x86/config.mk
@@ -66,7 +66,7 @@  LDFLAGS_FINAL += --gc-sections -pie
 
 endif
 
-ifneq ($(CONFIG_EFI_STUB)$(CONFIG_CMD_BOOTEFI_HELLO),)
+ifneq ($(CONFIG_EFI_STUB)$(CONFIG_CMD_BOOTEFI_HELLO_COMPILE),)
 
 ifneq ($(CONFIG_EFI_STUB_64BIT),)
 EFI_LDS := elf_x86_64_efi.lds
diff --git a/arch/x86/lib/Makefile b/arch/x86/lib/Makefile
index ff402dc..723288f 100644
--- a/arch/x86/lib/Makefile
+++ b/arch/x86/lib/Makefile
@@ -64,6 +64,6 @@  extra-$(CONFIG_EFI_STUB_64BIT) += crt0_x86_64_efi.o reloc_x86_64_efi.o
 
 endif
 
-ifneq ($(CONFIG_EFI_STUB)$(CONFIG_CMD_BOOTEFI_HELLO),)
+ifneq ($(CONFIG_EFI_STUB)$(CONFIG_CMD_BOOTEFI_HELLO_COMPILE),)
 extra-y += $(EFI_CRT0) $(EFI_RELOC)
 endif
diff --git a/cmd/Kconfig b/cmd/Kconfig
index 2a2f23e..b16c603 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -181,9 +181,22 @@  config CMD_BOOTEFI
 	help
 	  Boot an EFI image from memory.
 
+config CMD_BOOTEFI_HELLO_COMPILE
+	bool "Compile a standard EFI hello world binary for testing"
+	depends on CMD_BOOTEFI && (ARM || X86)
+	default y
+	help
+	  This compiles a standard EFI hello world application with U-Boot so
+	  that it can be used with the test/py testing framework. This is useful
+	  for testing that EFI is working at a basic level, and for bringing
+	  up EFI support on a new architecture.
+
+	  No additional space will be required in the resulting U-Boot binary
+	  when this option is enabled.
+
 config CMD_BOOTEFI_HELLO
 	bool "Allow booting a standard EFI hello world for testing"
-	depends on CMD_BOOTEFI && (ARM || X86)
+	depends on CMD_BOOTEFI_HELLO_COMPILE
 	help
 	  This adds a standard EFI hello world application to U-Boot so that
 	  it can be used with the 'bootefi hello' command. This is useful
diff --git a/configs/qemu-x86_efi_payload64_defconfig b/configs/qemu-x86_efi_payload64_defconfig
index c081ead..eaf0e32 100644
--- a/configs/qemu-x86_efi_payload64_defconfig
+++ b/configs/qemu-x86_efi_payload64_defconfig
@@ -52,3 +52,4 @@  CONFIG_USE_PRIVATE_LIBGCC=y
 CONFIG_EFI=y
 CONFIG_EFI_STUB=y
 CONFIG_EFI_STUB_64BIT=y
+# CONFIG_CMD_BOOTEFI_HELLO_COMPILE is not set
diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
index f466408..fa8b91a 100644
--- a/lib/efi_loader/Makefile
+++ b/lib/efi_loader/Makefile
@@ -10,6 +10,9 @@ 
 CFLAGS_helloworld.o := $(CFLAGS_EFI)
 CFLAGS_REMOVE_helloworld.o := $(CFLAGS_NON_EFI)
 
+efiprogs-$(CONFIG_CMD_BOOTEFI_HELLO_COMPILE) += helloworld.efi
+always := $(efiprogs-y)
+
 obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o
 obj-y += efi_image_loader.o efi_boottime.o efi_runtime.o efi_console.o
 obj-y += efi_memory.o
diff --git a/test/py/tests/test_efi_loader.py b/test/py/tests/test_efi_loader.py
index 1c42653..7bf0170 100644
--- a/test/py/tests/test_efi_loader.py
+++ b/test/py/tests/test_efi_loader.py
@@ -104,7 +104,7 @@  def test_efi_setup_static(u_boot_console):
     global net_set_up
     net_set_up = True
 
-@pytest.mark.buildconfigspec('cmd_bootefi_hello')
+@pytest.mark.buildconfigspec('cmd_bootefi_hello_compile')
 def test_efi_helloworld_net(u_boot_console):
     """Run the helloworld.efi binary via TFTP.