Re: [PATCH v8] Preserve the EFI System Resource Table for dom0

[Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>]

On 06/07/2022 11:32, Luca Fancellu wrote:
>> On 24 Jun 2022, at 19:17, Demi Marie Obenour <demi@xxxxxxxxxxxxxxxxxxxxxx> 
>> wrote:
>>
>> diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
>> index a25e1d29f1..f6f34aa816 100644
>> --- a/xen/common/efi/boot.c
>> +++ b/xen/common/efi/boot.c
>> @@ -567,6 +587,41 @@ static int __init efi_check_dt_boot(const 
>> EFI_LOADED_IMAGE *loaded_image)
>> }
>> #endif
>>
>> +static UINTN __initdata esrt = EFI_INVALID_TABLE_ADDR;
>> +
>> +static size_t __init get_esrt_size(const EFI_MEMORY_DESCRIPTOR *desc)
>> +{
>> +    size_t available_len, len;
>> +    const UINTN physical_start = desc->PhysicalStart;
> Hi,
>
> From my tests on an arm64 machine so far I can tell that desc is NULL here,
> for this reason we have the problem.
>
> I’ll have a further look on the cause of this, but if you are faster than me 
> you are
> welcome to give your opinion on that.

Given this observation, there's clearly ...

> @@ -1051,6 +1110,70 @@ static void __init 
> efi_set_gop_mode(EFI_GRAPHICS_OUTPUT_PROTOCOL *gop, UINTN gop
> #define INVALID_VIRTUAL_ADDRESS (0xBAAADUL << \
>                                  (EFI_PAGE_SHIFT + BITS_PER_LONG - 32))
>
> +static void __init efi_relocate_esrt(EFI_SYSTEM_TABLE *SystemTable)
> +{
> +    EFI_STATUS status;
> +    UINTN info_size = 0, map_key, mdesc_size;
> +    void *memory_map = NULL;
> +    UINT32 ver;
> +    unsigned int i;
> +
> +    for ( ; ; ) {
> +        status = efi_bs->GetMemoryMap(&info_size, memory_map, &map_key,
> +                                      &mdesc_size, &ver);
> +        if ( status == EFI_SUCCESS && memory_map != NULL )
> +            break;
> +        if ( status == EFI_BUFFER_TOO_SMALL || memory_map == NULL )
> +        {
> +            info_size += 8 * efi_mdesc_size;
> +            if ( memory_map != NULL )
> +                efi_bs->FreePool(memory_map);
> +            memory_map = NULL;
> +            status = efi_bs->AllocatePool(EfiLoaderData, info_size, 
> &memory_map);
> +            if ( status == EFI_SUCCESS )
> +                continue;
> +            PrintErr(L"Cannot allocate memory to relocate ESRT\r\n");
> +        }
> +        else
> +            PrintErr(L"Cannot obtain memory map to relocate ESRT\r\n");
> +        return;
> +    }
> +
> +    /* Try to obtain the ESRT.  Errors are not fatal. */
> +    for ( i = 0; i < info_size; i += efi_mdesc_size )
> +    {
> +        /*
> +         * ESRT needs to be moved to memory of type EfiRuntimeServicesData
> +         * so that the memory it is in will not be used for other purposes.
> +         */
> +        void *new_esrt = NULL;
> +        size_t esrt_size = get_esrt_size(efi_memmap + i);

... a NULL pointer here.  And the only way that could happen is if
efi_memmap is NULL.

Which perhaps isn't surprising because presumably ARM gets memory
information from the DT, not EFI?

~Andrew