[U-Boot] [PATCH v3 7/8] x86: efi: Add a hello world test program

[Alexander Graf]

On 14/11/2016 21:58, Simon Glass wrote:
> Hi Alex,
>
> On 14 November 2016 at 13:46, Alexander Graf <agraf at suse.de> wrote:
>>
>>
>> On 14/11/2016 21:44, Simon Glass wrote:
>>>
>>> Hi Alex,
>>>
>>> On 11 November 2016 at 23:23, Alexander Graf <agraf at suse.de> wrote:
>>>>
>>>>
>>>>
>>>>> Am 11.11.2016 um 17:17 schrieb Simon Glass <sjg at chromium.org>:
>>>>>
>>>>> Hi Alex,
>>>>>
>>>>>> On 7 November 2016 at 09:32, Alexander Graf <agraf at suse.de> wrote:
>>>>>>
>>>>>>
>>>>>>> On 07/11/2016 10:46, Simon Glass wrote:
>>>>>>>
>>>>>>> Hi Alex,
>>>>>>>
>>>>>>>> On 19 October 2016 at 01:09, Alexander Graf <agraf at suse.de> wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>> On 18/10/2016 22:37, Simon Glass wrote:
>>>>>>>>>
>>>>>>>>> Hi Alex,
>>>>>>>>>
>>>>>>>>>> On 18 October 2016 at 01:14, Alexander Graf <agraf at suse.de> wrote:
>>>>>>>>>>
>>>>>>>>>>> On 10/18/2016 04:29 AM, Simon Glass wrote:
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> It is useful to have a basic sanity check for EFI loader support.
>>>>>>>>>>> Add
>>>>>>>>>>> a
>>>>>>>>>>> 'bootefi hello' command which loads HelloWord.efi and runs it
>>>>>>>>>>> under
>>>>>>>>>>> U-Boot.
>>>>>>>>>>>
>>>>>>>>>>> Signed-off-by: Simon Glass <sjg at chromium.org>
>>>>>>>>>>> ---
>>>>>>>>>>>
>>>>>>>>>>> Changes in v3:
>>>>>>>>>>> - Include a link to the program instead of adding it to the tree
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> So, uh, where is the link?
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> I put it in the README (see the arm patch).
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> I'm really not convinced this command buys us anything yet. I do
>>>>>>>>>> agree
>>>>>>>>>> that
>>>>>>>>>> we want automated testing - but can't we get that using QEMU and a
>>>>>>>>>> downloadable image file that we pass in as disk and have the distro
>>>>>>>>>> boot do
>>>>>>>>>> its magic?
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> That seems very heavyweight as a sanity check, although I agree it
>>>>>>>>> is
>>>>>>>>> useful.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> It's not really much more heavy weight. The "image file" could simply
>>>>>>>> contain your hello world binary. But with this we don't just verify
>>>>>>>> whether "bootefi" works, but also whether the default boot path works
>>>>>>>> ok.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> I don't think I understand what you mean by 'image file'. Is it
>>>>>>> something other than the .efi file? Do you mean a disk image?
>>>>>>
>>>>>>
>>>>>>
>>>>>> Yes. For reasonable test coverage, we should also verify that the
>>>>>> distro
>>>>>> defaults wrote a sane boot script that automatically searches for a
>>>>>> default
>>>>>> EFI binary in /efi/boot/bootx86.efi on the first partition of all
>>>>>> devices
>>>>>> and runs it.
>>>>>>
>>>>>> So if we just provide an SD card image or hard disk image to QEMU which
>>>>>> contains a hello world .efi binary as that default boot file, we don't
>>>>>> only
>>>>>> test whether the "bootefi" command works, but also whether the distro
>>>>>> boot
>>>>>> script works.
>>>>>
>>>>>
>>>>> That's right.
>>>>>
>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>>> Here I am just making sure that EFI programs can start, print output
>>>>>>>>> and exit. It is a test that we can easily run without a lot of
>>>>>>>>> overhead, much less than a full distro boot.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Again, I don't think it's much more overhead and I do believe it
>>>>>>>> gives
>>>>>>>> us much cleaner separation between responsibilities of code (tests go
>>>>>>>> where tests are).
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> You are talking about a functional test, something that tests things
>>>>>>> end to end. I prefer to at least start with a smaller test. Granted it
>>>>>>> takes a little more work but it means there are fewer things to hunt
>>>>>>> through when something goes wrong.
>>>>>>
>>>>>>
>>>>>>
>>>>>> Yes, I personally find unit tests terribly annoying and unproductive
>>>>>> and
>>>>>> functional tests very helpful :). And in this case, the effort to write
>>>>>> it
>>>>>> is about the same for both, just that the functional test actually
>>>>>> tells you
>>>>>> that things work or don't work at the end of the day.
>>>>>>
>>>>>> With a code base like U-Boot, a simple functional test like the above
>>>>>> plus
>>>>>> git bisect should get you to an offending patch very quickly.
>>>>>
>>>>>
>>>>> This is not a unit test - in fact the EFI stuff has no unit tests. I
>>>>> suppose if we are trying to find a name this is a small functional
>>>>> test since it exercises the general functionality.
>>>>>
>>>>> I am much keener on small tests than large ones for finding simple
>>>>> bugs. Of course you can generally bisect to find a bug, but the more
>>>>> layers of software you need to look for the harder this is.
>>>>>
>>>>> We could definitely use a pytest which checks an EFI boot into an
>>>>> image, but I don't think this obviates the need for a smaller targeted
>>>>> test like this one.
>>>>
>>>>
>>>> I think arguing over this is moot :). More tests is usually a good thing,
>>>> so whoever gets to write them gets to push them ;). As long as the licenses
>>>> are sound at least.
>>>
>>>
>>> OK good, well please can you review this at some point?
>>
>>
>> Review what exactly?
>
> I mean the patches. There should be ~14 in your queue.

Interesting. I see them on patchwork, but neither in my U-Boot folder, 
my inbox nor my spam folder. I wonder what happened there.


Alex