Skip to content

Commit

Permalink
elfloader: fix UEFI integration bug: descriptor
Browse files Browse the repository at this point in the history
size mismatch.

The UEFI specification 2.10 says in section 7 for
EFI_BOOT_SERVICES.GetMemoryMap():

"The GetMemoryMap() function also returns the size and revision
number of the EFI_MEMORY_DESCRIPTOR. The DescriptorSize represents
the size in bytes of an EFI_MEMORY_DESCRIPTOR array element returned
in MemoryMap. The size is returned to allow for future expansion of
the EFI_MEMORY_DESCRIPTOR in response to hardware innovation.

The structure of the EFI_MEMORY_DESCRIPTOR may be extended in the
future but it will remain backwards compatible with the current
definition. Thus OS software must use the DescriptorSize to find
the start of each EFI_MEMORY_DESCRIPTOR in the MemoryMap array."

This mismatch is the case on (our) Orin UEFI. The compiled size of
a memory descriptor is 40 Bytes, but the Orin UEFI implementation
uses 48 Bytes per descriptor.

Thus, due to the requirement to use a larger size than the
returned total size (due to the fact that the buffer allocation
itself may lead to one more entry in the memory map), we must
increase by the size (in terms of number of descriptors), but
use the number of bytes that UEFI uses for one memory map entry,
not what we think it might be.

Some other people already stumbled over this:
https://forum.osdev.org/viewtopic.php?f=1&t=32953

Based on the comment in the existing code, the author seems to
not have understood how the size of the memory map can be
determined. Just read the spec!
So we better update that misleading comment.

Signed-off-by: Matthias Rosenfelder <[email protected]>
  • Loading branch information
mro-github-12345 authored and Andy Bui committed Feb 25, 2024
1 parent 80b8902 commit 0e0a99d
Showing 1 changed file with 8 additions and 3 deletions.
11 changes: 8 additions & 3 deletions elfloader-tool/src/binaries/efi/efi_init.c
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ static unsigned long exit_boot_services(void)

/*
* As the number of existing memory segments are unknown,
* we need to resort to a trial and error to guess that.
* We start from 32 and increase it by one until get a valid value.
* we need to start somewhere. The API then tells us how much space we need
* if it is not enough.
*/
map_size = sizeof(*memory_map) * 32;

Expand All @@ -81,7 +81,12 @@ static unsigned long exit_boot_services(void)
memory_map = NULL;

if (status == EFI_BUFFER_TOO_SMALL) {
map_size += sizeof(*memory_map);
/* Note: "map_size" is an IN/OUT-parameter and has been updated to the
* required size. We still add one more entry ("desc_size" is in bytes)
* due to the hint from the spec ("since allocation of the new buffer
* may potentially increase memory map size.").
*/
map_size += desc_size;
} else {
/* some other error; bail out! */
return status;
Expand Down

0 comments on commit 0e0a99d

Please sign in to comment.