On 22.06.2022 03:23, Demi Marie Obenour wrote:
> @@ -1051,6 +1110,62 @@ 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;
> + unsigned int i;
> + void *memory_map = NULL;
> +
> + for (;;) {
Nit: Style:
for ( ; ; )
{
> + status = efi_bs->GetMemoryMap(&info_size, memory_map, &map_key,
> + &efi_mdesc_size, &mdesc_ver);
Unless you have a reason to (which I don't see), please don't alter
global variables here.
> + if ( status == EFI_SUCCESS && memory_map != NULL )
> + break;
> + if ( status == EFI_BUFFER_TOO_SMALL || memory_map == NULL ) {
Nit: Brace placement.
> + info_size *= 2;
Doubling the buffer size seems excessive to me. If you really want it
like that, I think this deserves saying a word in the description. As
said before, the same increment as in efi_exit_boot() would look best
to me, for consistency.
> + if ( memory_map != NULL )
> + efi_bs->FreePool(memory_map);
> + status = efi_bs->AllocatePool(EfiLoaderData, info_size,
> &memory_map);
> + if ( status == EFI_SUCCESS )
> + continue;
> + }
> + return;
Perhaps emit a message?
> + }
> +
> + /* 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);
> +
> + if ( !esrt_size )
> + continue;
> + if ( ((EFI_MEMORY_DESCRIPTOR *)(efi_memmap + i))->Type ==
> + EfiRuntimeServicesData )
> + return; /* ESRT already safe from reuse */
"break" here so that ...
> + status = efi_bs->AllocatePool(EfiRuntimeServicesData, esrt_size,
> + &new_esrt);
> + if ( status == EFI_SUCCESS && new_esrt )
> + {
> + memcpy(new_esrt, (void *)esrt, esrt_size);
> + status = efi_bs->InstallConfigurationTable(&esrt_guid, new_esrt);
> + if ( status != EFI_SUCCESS )
> + {
> + PrintErr(L"Cannot install new ESRT\r\n");
> + efi_bs->FreePool(new_esrt);
> + }
> + }
> + else
> + PrintErr(L"Cannot allocate memory for ESRT\r\n");
> + break;
> + }
... you can free the memory map here.
> @@ -1067,6 +1182,13 @@ static void __init efi_exit_boot(EFI_HANDLE
> ImageHandle, EFI_SYSTEM_TABLE *Syste
> if ( !efi_memmap )
> blexit(L"Unable to allocate memory for EFI memory map");
>
> + status = SystemTable->BootServices->GetMemoryMap(&efi_memmap_size,
> + efi_memmap, &map_key,
> + &efi_mdesc_size,
> + &mdesc_ver);
> + if ( EFI_ERROR(status) )
> + PrintErrMesg(L"Cannot obtain memory map", status);
> +
> for ( retry = false; ; retry = true )
> {
> efi_memmap_size = info_size;
What is this change about? If it really is needed for some reason, I
further don't see why you don't use efi_bs (it cannot be used inside
the loop, but is fine to use ahead of it).
Jan
