[U-Boot] [PATCH 00/23] efi_loader implement missing functions

[Alexander Graf]

On 08/29/2017 12:52 PM, Heinrich Schuchardt wrote:
> On 08/27/2017 10:10 PM, Simon Glass wrote:
>> Hi Heinrich,
>>
>> On 26 August 2017 at 16:51, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>>> This patch sequence contains all patches needed to load
>>> iPXE and use it for downloading and executing images
>>> via https or http or to mount iSCSI volumes.
>>>
>>> Network speed on an Odroid C2 reached 30 MB/s which should be
>>> enough for most use cases.
>>>
>>> I have tested the following iPXE commands successfully
>>> * dhcp
>>> * route
>>> * ntp
>>> * sanhook iSCSI-target
>>> * chain http-target
>>> * kernel http-target
>>> * boot (after calling kernel)
>>> * exit
>>> * reboot
>>>
>>> The only adjustment in iPXE was adding file src/config/local/nap.h with
>>>    #undef NAP_EFIX86
>>>    #undef NAP_EFIARM
>>>    #define NAP_NULL
>>> and src/config/local/myscript.ipxe with
>>>    #!ipxe
>>>    shell
>>> before building iPXE with
>>>    make bin-arm64-efi/snp.efi EMBED=config/local/myscript.ipxe
>>>
>>> The next task will be to put iXPE binaries on a server
>>> and to create Travis CI test cases.
>> Some general comments on the series as a whole:
>>
>> 1. It really needs to have tests. This is a lot of new code in U-Boot,
>> and relying on travis CI test cases (which takes forever to run) is
>> not a good option. We have a 'make tests' target which you should hook
>> into, via the pytests framework. This runs in a minute or so. There is
>> quite a bit of documentation in test/py for this. It should be easy
>> enough to build up the data structures in sandbox and then test that
>> each function does what is expected.
>>
>> 2. Exported functions should be commented to describe their purpose,
>> arguments and return value. Non-trivial static functions should be
>> commented too.
>>
>> Regards,
>> Simon
>>
> Hello Alex,

I think you meant to say Hello Simon?

>
> testing the EFI API is only possible from an EFI program.
>
> I suggest to build an EFI app selftest.efi like the helloworld.efi have.
> I would add command
> bootefi selftest.efi
> to run the tests and provide the python wrapper code to add it to the
> test suite.

I think that's a great idea, yes.

>
> There are some very simple patches which you may decide to cherry pick,
> notably
>
> [PATCH 02/23] efi_loader: notify when ExitBootServices is invoked
> [PATCH 03/23] efi_loader: support 16 protocols per efi_object
> [PATCH 17/23] efi_loader: efi_net: hwaddr_size = 6
> [PATCH 18/23] efi_net: return EFI_UNSUPPORTED where appropriate

How important are those? I would ideally like to see the unit test thing 
first, then add new functionality :)


Alex