[U-Boot] [PATCH 2/5] x86: efi_loader: Build EFI memory map per E820 table

[Bin Meng]

Hi Heinrich,

On Sun, Jun 24, 2018 at 5:41 PM, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> On 06/23/2018 12:03 PM, Bin Meng wrote:
>> On x86 traditional E820 table is used to pass the memory information
>> to kernel. With EFI loader we can build the EFI memory map from it.
>>
>> Signed-off-by: Bin Meng <bmeng.cn at gmail.com>
>> ---
>>
>>  arch/x86/lib/e820.c | 39 +++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 39 insertions(+)
>>
>> diff --git a/arch/x86/lib/e820.c b/arch/x86/lib/e820.c
>> index 9a9ec99..8b34f67 100644
>> --- a/arch/x86/lib/e820.c
>> +++ b/arch/x86/lib/e820.c
>> @@ -4,6 +4,7 @@
>>   */
>>
>>  #include <common.h>
>> +#include <efi_loader.h>
>>  #include <asm/e820.h>
>>
>>  DECLARE_GLOBAL_DATA_PTR;
>> @@ -34,3 +35,41 @@ __weak unsigned int install_e820_map(unsigned int max_entries,
>>
>>       return 4;
>>  }
>> +
>> +#if defined(CONFIG_EFI_LOADER) && !defined(CONFIG_SPL_BUILD)
>> +void efi_add_known_memory(void)
>> +{
>> +     struct e820_entry e820[E820MAX];
>> +     unsigned int i, num;
>> +     u64 start, pages;
>> +     int type;
>> +
>> +     num = install_e820_map(ARRAY_SIZE(e820), e820);
>
> Thanks for the patch. Not handling the E820 table is a known deficiency
> of our EFI implementation.
>
> I am a bit worried about error handling here.
>
> What guarantees that lib_sysinfo.n_memranges <= E820MAX?
>

I think you referred to coreboot here? The guarantee comes form the
caller who calls install_e820_map(), which in the coreboot case is 32.
So 32 is the maximum number of E820 table entry it can write.

> install_e820_map() prints a message if max_entries is too small but it
> does not return an error code.
>
> So only if num < E820MAX we can be sure that the call returned all
> information of interest.
>
> My idea is that the interface of efi_add_known_memory() should be
> changed to allow returning an error code if it fails. And
> efi_memory_init() should return an eventual error to board_init_r().
>

Regards,
Bin