Syslinux - Nov 2015 - [PATCH] efi: Call ExitBootServices at least twice

On Tue, Aug 25, 2015 at 11:54 PM, celelibi--- via Syslinux
<syslinux at zytor.com> wrote:
> From: Sylvain Gault <sylvain.gault at gmail.com>
>
> Some firmware implementations may need ExitBootServices to be called
> twice. The second time with an updated memory map key.
>
> Signed-off-by: Sylvain Gault <sylvain.gault at gmail.com>
> ---
>  efi/main.c | 75
+++++++++++++++++++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 64 insertions(+), 11 deletions(-)
>
> diff --git a/efi/main.c b/efi/main.c
> index 6dbc259..a39ab43 100644
> --- a/efi/main.c
> +++ b/efi/main.c
> @@ -1001,19 +1001,79 @@ static int handle_ramdisks(struct linux_header
*hdr,
>         return 0;
>  }
>
> +/*
> + * Like get_memory_map but try to use the given buffer first, reallocate
it if
> + * it's too small and always set the allocated size.
> + */
> +static EFI_MEMORY_DESCRIPTOR *
> +get_memory_map_realloc(EFI_MEMORY_DESCRIPTOR *map, UINTN *allocsize,
> +                      UINTN *nr_entries, UINTN *key, UINTN *desc_sz,
> +                      UINT32 *desc_ver)
> +{
> +       EFI_STATUS status;
> +       UINTN size, allocsizeadd;
> +
> +       allocsizeadd = sizeof(*map) * 2;
> +
> +       do {
> +               size = *allocsize;
> +               status = uefi_call_wrapper(BS->GetMemoryMap, 5,
&size, map, key,
> +                                          desc_sz, desc_ver);
Why not check for NULL and call get_memory_map() first?
> +               if (status == EFI_BUFFER_TOO_SMALL) {
> +                       if (map)
> +                               FreePool(map);
> +                       allocsizeadd *= 2;
> +                       *allocsize = size + allocsizeadd;
> +                       map = AllocatePool(*allocsize);
> +               }
Why guess at the exact size needed?  GetMemoryMap is required to tell
us the exact resize needed in the return of "size", even in UEFI v2.0.
Was there a system that didn't follow this?
> +       } while (status == EFI_BUFFER_TOO_SMALL);
> +
> +       if (status == EFI_SUCCESS) {
> +               *nr_entries = size / *desc_sz;
> +               return map;
> +       }
> +
> +       if (map)
> +               FreePool(map);
> +       return NULL;
> +}
> +
>  static int exit_boot(struct boot_params *bp)
>  {
>         struct e820_entry *e820buf, *e;
>         EFI_MEMORY_DESCRIPTOR *map;
>         EFI_STATUS status;
>         uint32_t e820_type;
> -       UINTN i, nr_entries, key, desc_sz;
> +       UINTN i, nr_entries, key, desc_sz, allocsize;
>         UINT32 desc_ver;
> +       int retry;
>
> -       /* Build efi memory map */
> -       map = get_memory_map(&nr_entries, &key, &desc_sz,
&desc_ver);
> -       if (!map)
> +       /*
> +        * Build efi memory map and call ExitBootServices as soon after as
> +        * possible. Some implementations need two calls to
ExitBootServices.
> +        */
> +
> +       map = NULL;
> +       allocsize = 0;
> +       retry = 0;
> +       do {
> +               map = get_memory_map_realloc(map, &allocsize,
&nr_entries, &key,
> +                                            &desc_sz, &desc_ver);
> +               if (!map)
> +                       return -1;
> +
> +               status = uefi_call_wrapper(BS->ExitBootServices, 2,
> +                                          image_handle, key);
> +               retry++;
> +       } while (status != EFI_SUCCESS && retry < 3);
> +
> +       if (status != EFI_SUCCESS) {
> +               printf("Failed to exit boot services:
0x%016lx\n", status);
> +               FreePool(map);
>                 return -1;
> +       }
> +
Call ExitBootServices() prior to any map parsing.  This makes sense as
long as no other functions get added after this that might want to
allocate more RAM.  This function looks good from here out.
>         bp->efi.memmap = (uint32_t)(unsigned long)map;
>         bp->efi.memmap_size = nr_entries * desc_sz;
> @@ -1088,13 +1148,6 @@ static int exit_boot(struct boot_params *bp)
>
>         bp->e820_entries = e - e820buf;
>
> -       status = uefi_call_wrapper(BS->ExitBootServices, 2,
image_handle, key);
> -       if (status != EFI_SUCCESS) {
> -               printf("Failed to exit boot services:
0x%016lx\n", status);
> -               FreePool(map);
> -               return -1;
> -       }
> -
>         return 0;
>  }
-- 
-Gene

2015-11-02 12:34 UTC+01:00, Gene Cumm <gene.cumm at
gmail.com>:
> On Tue, Aug 25, 2015 at 11:54 PM, celelibi--- via Syslinux
> <syslinux at zytor.com> wrote:
>> From: Sylvain Gault <sylvain.gault at gmail.com>
>>
>> Some firmware implementations may need ExitBootServices to be called
>> twice. The second time with an updated memory map key.
>>
>> Signed-off-by: Sylvain Gault <sylvain.gault at gmail.com>
>> ---
>>  efi/main.c | 75
>> +++++++++++++++++++++++++++++++++++++++++++++++++++++---------
>>  1 file changed, 64 insertions(+), 11 deletions(-)
>>
>> diff --git a/efi/main.c b/efi/main.c
>> index 6dbc259..a39ab43 100644
>> --- a/efi/main.c
>> +++ b/efi/main.c
>> @@ -1001,19 +1001,79 @@ static int handle_ramdisks(struct linux_header
>> *hdr,
>>         return 0;
>>  }
>>
>> +/*
>> + * Like get_memory_map but try to use the given buffer first,
reallocate
>> it if
>> + * it's too small and always set the allocated size.
>> + */
>> +static EFI_MEMORY_DESCRIPTOR *
>> +get_memory_map_realloc(EFI_MEMORY_DESCRIPTOR *map, UINTN *allocsize,
>> +                      UINTN *nr_entries, UINTN *key, UINTN *desc_sz,
>> +                      UINT32 *desc_ver)
>> +{
>> +       EFI_STATUS status;
>> +       UINTN size, allocsizeadd;
>> +
>> +       allocsizeadd = sizeof(*map) * 2;
>> +
>> +       do {
>> +               size = *allocsize;
>> +               status = uefi_call_wrapper(BS->GetMemoryMap, 5,
&size,
>> map, key,
>> +                                          desc_sz, desc_ver);
>
> Why not check for NULL and call get_memory_map() first?
I cannot call get_memory_map at all because I wouldn't control how
much memory is allocated, and this is a crucial point to comply as
much as possible with the UEFI specification. (See below.)

(BTW, I hope we agree that get_memory_map is the function provided by
gnu-efi and GetMemoryMap is the one provided by the firmware.)
>
>> +               if (status == EFI_BUFFER_TOO_SMALL) {
>> +                       if (map)
>> +                               FreePool(map);
>> +                       allocsizeadd *= 2;
>> +                       *allocsize = size + allocsizeadd;
>> +                       map = AllocatePool(*allocsize);
>> +               }
>
> Why guess at the exact size needed?  GetMemoryMap is required to tell
> us the exact resize needed in the return of "size", even in UEFI
v2.0.
> Was there a system that didn't follow this?
There are two different things happening here.

First, allocating some memory *does* increase the size of the memory
map. Thus, allocating the exact amount of memory GetMemoryMap said it
needs and calling it again with that buffer is likely to result in an
error EFI_BUFFER_TOO_SMALL.

Second, ExitBootServices does - again - increase the size of the
memory map on some hardware (although it is forbidden). So, if after
the first call to ExitBootServices we call GetMemoryMap with a buffer
that was exactly fitted for the map *before* calling ExitBootServices,
it is - again - likely to result in an EFI_BUFFER_TOO_SMALL. And this
time, things are really bad since we're not supposed to perform a
memory allocation after we called ExitBootServices.

I specifically choosed that on the first call to AllocatePool, the
variable allocsizeadd is 4 times sizeof(EFI_MEMORY_DESCRIPTOR), so
that on my (buggy) hardware the second call to get_memory_map_realloc,
the memory map can fit into the buffer on the first try. Thus the
non-standard behavior of the firmware is worked around with only
standard code.

If some UEFI implementations require a buffer yet bigger than that for
the second round of GetMemoryMap, well... on a pure standard way, we
lost the game. After calling ExitBootServices the first time we're not
supposed to perform a memory allocation and we can't go back either.
So, why not try a memory allocation anyway? It works on some
implementations (including mine) despite being forbidden by the
specification, but we're in this situation because the firmware was
already not complying with the UEFI spec.


All this is a bit complex. But working around bugs is never trivial.
Something similar is implemented in at least GRUB and Linux.

GRUB fiddle around to find the optimal size for the memory map and in
the end add 3 pages of memory for safety.
http://git.savannah.gnu.org/cgit/grub.git/tree/grub-core/loader/i386/linux.c?id=206676601eb853fc319df14cd3398fbdfde665ac#n104

GRUB then retry in an infinite loop to call GetMemoryMap +
ExitBootServices. On each iteration it allocates a copy of the memory
map and free it in case of failure. However, GRUB seems to have its
own memory management, which allocate a quarter of the available
memory on startup. This allows those allocation on each iteration to
not disturb the memory map while trying to call ExitBootServices.

http://git.savannah.gnu.org/cgit/grub.git/tree/grub-core/kern/efi/mm.c?id=206676601eb853fc319df14cd3398fbdfde665ac#n148
http://git.savannah.gnu.org/cgit/grub.git/tree/grub-core/kern/efi/mm.c?id=206676601eb853fc319df14cd3398fbdfde665ac#n494


Linux does something similar. It calls get_memory_map +
exit_boot_services a maximum of two times. However, it allows itself
to always call allocate_pool (in efi_get_memory_map) even after
exit_boot_services failed once. Which does not comply with the UEFI
specification.

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/x86/boot/compressed/eboot.c?id=e86328c489d7ecdca99410a06a3f448caf7857bf#n1299
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/firmware/efi/libstub/efi-stub-helper.c?id=e86328c489d7ecdca99410a06a3f448caf7857bf#n66


>
>> +       } while (status == EFI_BUFFER_TOO_SMALL);
>> +
>> +       if (status == EFI_SUCCESS) {
>> +               *nr_entries = size / *desc_sz;
>> +               return map;
>> +       }
>> +
>> +       if (map)
>> +               FreePool(map);
>> +       return NULL;
>> +}
>> +
>>  static int exit_boot(struct boot_params *bp)
>>  {
>>         struct e820_entry *e820buf, *e;
>>         EFI_MEMORY_DESCRIPTOR *map;
>>         EFI_STATUS status;
>>         uint32_t e820_type;
>> -       UINTN i, nr_entries, key, desc_sz;
>> +       UINTN i, nr_entries, key, desc_sz, allocsize;
>>         UINT32 desc_ver;
>> +       int retry;
>>
>> -       /* Build efi memory map */
>> -       map = get_memory_map(&nr_entries, &key, &desc_sz,
&desc_ver);
>> -       if (!map)
>> +       /*
>> +        * Build efi memory map and call ExitBootServices as soon after
>> as
>> +        * possible. Some implementations need two calls to
>> ExitBootServices.
>> +        */
>> +
>> +       map = NULL;
>> +       allocsize = 0;
>> +       retry = 0;
>> +       do {
>> +               map = get_memory_map_realloc(map, &allocsize,
&nr_entries,
>> &key,
>> +                                            &desc_sz,
&desc_ver);
>> +               if (!map)
>> +                       return -1;
>> +
>> +               status = uefi_call_wrapper(BS->ExitBootServices, 2,
>> +                                          image_handle, key);
>> +               retry++;
>> +       } while (status != EFI_SUCCESS && retry < 3);
>> +
>> +       if (status != EFI_SUCCESS) {
>> +               printf("Failed to exit boot services:
0x%016lx\n",
>> status);
>> +               FreePool(map);
>>                 return -1;
>> +       }
>> +
>
> Call ExitBootServices() prior to any map parsing.  This makes sense as
> long as no other functions get added after this that might want to
> allocate more RAM.  This function looks good from here out.
>
>>         bp->efi.memmap = (uint32_t)(unsigned long)map;
>>         bp->efi.memmap_size = nr_entries * desc_sz;
>> @@ -1088,13 +1148,6 @@ static int exit_boot(struct boot_params *bp)
>>
>>         bp->e820_entries = e - e820buf;
>>
>> -       status = uefi_call_wrapper(BS->ExitBootServices, 2,
image_handle,
>> key);
>> -       if (status != EFI_SUCCESS) {
>> -               printf("Failed to exit boot services:
0x%016lx\n",
>> status);
>> -               FreePool(map);
>> -               return -1;
>> -       }
>> -
>>         return 0;
>>  }
>
> --
> -Gene
>

I hope it's a bit clearer now. Sorry for the lengthy text. ^^


Celelibi

2015-11-02 12:34 UTC+01:00, Gene Cumm <gene.cumm at
gmail.com>:
> On Tue, Aug 25, 2015 at 11:54 PM, celelibi--- via Syslinux
> <syslinux at zytor.com> wrote:
>> From: Sylvain Gault <sylvain.gault at gmail.com>
>>
>> Some firmware implementations may need ExitBootServices to be called
>> twice. The second time with an updated memory map key.
>>
>> Signed-off-by: Sylvain Gault <sylvain.gault at gmail.com>
>> ---
>>  efi/main.c | 75
>> +++++++++++++++++++++++++++++++++++++++++++++++++++++---------
>>  1 file changed, 64 insertions(+), 11 deletions(-)
>>
>> diff --git a/efi/main.c b/efi/main.c
>> index 6dbc259..a39ab43 100644
>> --- a/efi/main.c
>> +++ b/efi/main.c
>> @@ -1001,19 +1001,79 @@ static int handle_ramdisks(struct linux_header
>> *hdr,
>>         return 0;
>>  }
>>
>> +/*
>> + * Like get_memory_map but try to use the given buffer first,
reallocate
>> it if
>> + * it's too small and always set the allocated size.
>> + */
>> +static EFI_MEMORY_DESCRIPTOR *
>> +get_memory_map_realloc(EFI_MEMORY_DESCRIPTOR *map, UINTN *allocsize,
>> +                      UINTN *nr_entries, UINTN *key, UINTN *desc_sz,
>> +                      UINT32 *desc_ver)
>> +{
>> +       EFI_STATUS status;
>> +       UINTN size, allocsizeadd;
>> +
>> +       allocsizeadd = sizeof(*map) * 2;
>> +
>> +       do {
>> +               size = *allocsize;
>> +               status = uefi_call_wrapper(BS->GetMemoryMap, 5,
&size,
>> map, key,
>> +                                          desc_sz, desc_ver);
>
> Why not check for NULL and call get_memory_map() first?
>
>> +               if (status == EFI_BUFFER_TOO_SMALL) {
>> +                       if (map)
>> +                               FreePool(map);
>> +                       allocsizeadd *= 2;
>> +                       *allocsize = size + allocsizeadd;
>> +                       map = AllocatePool(*allocsize);
>> +               }
>
> Why guess at the exact size needed?  GetMemoryMap is required to tell
> us the exact resize needed in the return of "size", even in UEFI
v2.0.
> Was there a system that didn't follow this?
>
>> +       } while (status == EFI_BUFFER_TOO_SMALL);
>> +
>> +       if (status == EFI_SUCCESS) {
>> +               *nr_entries = size / *desc_sz;
>> +               return map;
>> +       }
>> +
>> +       if (map)
>> +               FreePool(map);
>> +       return NULL;
>> +}
>> +
>>  static int exit_boot(struct boot_params *bp)
>>  {
>>         struct e820_entry *e820buf, *e;
>>         EFI_MEMORY_DESCRIPTOR *map;
>>         EFI_STATUS status;
>>         uint32_t e820_type;
>> -       UINTN i, nr_entries, key, desc_sz;
>> +       UINTN i, nr_entries, key, desc_sz, allocsize;
>>         UINT32 desc_ver;
>> +       int retry;
>>
>> -       /* Build efi memory map */
>> -       map = get_memory_map(&nr_entries, &key, &desc_sz,
&desc_ver);
>> -       if (!map)
>> +       /*
>> +        * Build efi memory map and call ExitBootServices as soon after
>> as
>> +        * possible. Some implementations need two calls to
>> ExitBootServices.
>> +        */
>> +
>> +       map = NULL;
>> +       allocsize = 0;
>> +       retry = 0;
>> +       do {
>> +               map = get_memory_map_realloc(map, &allocsize,
&nr_entries,
>> &key,
>> +                                            &desc_sz,
&desc_ver);
>> +               if (!map)
>> +                       return -1;
>> +
>> +               status = uefi_call_wrapper(BS->ExitBootServices, 2,
>> +                                          image_handle, key);
>> +               retry++;
>> +       } while (status != EFI_SUCCESS && retry < 3);
>> +
>> +       if (status != EFI_SUCCESS) {
>> +               printf("Failed to exit boot services:
0x%016lx\n",
>> status);
>> +               FreePool(map);
>>                 return -1;
>> +       }
>> +
>
> Call ExitBootServices() prior to any map parsing.  This makes sense as
> long as no other functions get added after this that might want to
> allocate more RAM.  This function looks good from here out.
I forgot to reply to this.
The justification for moving ExitBootServices earlier is simple:
*Nothing* should happen between GetMemoryMap and ExitBootServices. If
something happened to the memory map, ExitBootServices would fail. So
moving those calls as close together as possible (as recommended by
the UEFI spec) is the most logical thing to do.
And allocating some memory while parsing the memory map may not be the
smartest thing to do as it would invalidate the memory map being
parsed.
>
>>         bp->efi.memmap = (uint32_t)(unsigned long)map;
>>         bp->efi.memmap_size = nr_entries * desc_sz;
>> @@ -1088,13 +1148,6 @@ static int exit_boot(struct boot_params *bp)
>>
>>         bp->e820_entries = e - e820buf;
>>
>> -       status = uefi_call_wrapper(BS->ExitBootServices, 2,
image_handle,
>> key);
>> -       if (status != EFI_SUCCESS) {
>> -               printf("Failed to exit boot services:
0x%016lx\n",
>> status);
>> -               FreePool(map);
>> -               return -1;
>> -       }
>> -
>>         return 0;
>>  }
>
> --
> -Gene
>

Celelibi

On Mon, Nov 2, 2015 at 10:23 PM, Celelibi <celelibi at gmail.com>
wrote:
> 2015-11-02 12:34 UTC+01:00, Gene Cumm <gene.cumm at gmail.com>:
>> On Tue, Aug 25, 2015 at 11:54 PM, celelibi--- via Syslinux
>> <syslinux at zytor.com> wrote:
>>> From: Sylvain Gault <sylvain.gault at gmail.com>
>>>
>>> Some firmware implementations may need ExitBootServices to be
called
>>> twice. The second time with an updated memory map key.
>>>
>>> Signed-off-by: Sylvain Gault <sylvain.gault at gmail.com>
>>> ---
>>>  efi/main.c | 75
>>> +++++++++++++++++++++++++++++++++++++++++++++++++++++---------
>>>  1 file changed, 64 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/efi/main.c b/efi/main.c
>>> index 6dbc259..a39ab43 100644
>>> --- a/efi/main.c
>>> +++ b/efi/main.c
>>> @@ -1001,19 +1001,79 @@ static int handle_ramdisks(struct
linux_header
>>> *hdr,
>>>         return 0;
>>>  }
>>>
>>> +/*
>>> + * Like get_memory_map but try to use the given buffer first,
reallocate
>>> it if
>>> + * it's too small and always set the allocated size.
>>> + */
>>> +static EFI_MEMORY_DESCRIPTOR *
>>> +get_memory_map_realloc(EFI_MEMORY_DESCRIPTOR *map, UINTN
*allocsize,
>>> +                      UINTN *nr_entries, UINTN *key, UINTN
*desc_sz,
>>> +                      UINT32 *desc_ver)
>>> +{
>>> +       EFI_STATUS status;
>>> +       UINTN size, allocsizeadd;
>>> +
>>> +       allocsizeadd = sizeof(*map) * 2;
>>> +
>>> +       do {
>>> +               size = *allocsize;
>>> +               status = uefi_call_wrapper(BS->GetMemoryMap, 5,
&size,
>>> map, key,
>>> +                                          desc_sz, desc_ver);
>>
>> Why not check for NULL and call get_memory_map() first?
>
> I cannot call get_memory_map at all because I wouldn't control how
> much memory is allocated, and this is a crucial point to comply as
> much as possible with the UEFI specification. (See below.)
>
> (BTW, I hope we agree that get_memory_map is the function provided by
> gnu-efi and GetMemoryMap is the one provided by the firmware.)
No, it's the function you effectively replaced.
>>> +               if (status == EFI_BUFFER_TOO_SMALL) {
>>> +                       if (map)
>>> +                               FreePool(map);
>>> +                       allocsizeadd *= 2;
>>> +                       *allocsize = size + allocsizeadd;
>>> +                       map = AllocatePool(*allocsize);
>>> +               }
>>
>> Why guess at the exact size needed?  GetMemoryMap is required to tell
>> us the exact resize needed in the return of "size", even in
UEFI v2.0.
>> Was there a system that didn't follow this?
>
> There are two different things happening here.
>
> First, allocating some memory *does* increase the size of the memory
> map. Thus, allocating the exact amount of memory GetMemoryMap said it
> needs and calling it again with that buffer is likely to result in an
> error EFI_BUFFER_TOO_SMALL.
>
> Second, ExitBootServices does - again - increase the size of the
> memory map on some hardware (although it is forbidden). So, if after
> the first call to ExitBootServices we call GetMemoryMap with a buffer
> that was exactly fitted for the map *before* calling ExitBootServices,
> it is - again - likely to result in an EFI_BUFFER_TOO_SMALL. And this
> time, things are really bad since we're not supposed to perform a
> memory allocation after we called ExitBootServices.
>
> I specifically choosed that on the first call to AllocatePool, the
> variable allocsizeadd is 4 times sizeof(EFI_MEMORY_DESCRIPTOR), so
> that on my (buggy) hardware the second call to get_memory_map_realloc,
> the memory map can fit into the buffer on the first try. Thus the
> non-standard behavior of the firmware is worked around with only
> standard code.
I see.  In this way you get "size" (the size you actually need upon
return from GetMemoryMap) plus 4 more descriptors, hopefully enough to
handle the FreePool()/AllocatePool() that's to follow.  It's quite
probable that LibMemoryMap (gnu-efi) implements this double call too.
> If some UEFI implementations require a buffer yet bigger than that for
> the second round of GetMemoryMap, well... on a pure standard way, we
> lost the game. After calling ExitBootServices the first time we're not
> supposed to perform a memory allocation and we can't go back either.
After calling ExitBootServices _successfully_.
> So, why not try a memory allocation anyway? It works on some
> implementations (including mine) despite being forbidden by the
> specification, but we're in this situation because the firmware was
> already not complying with the UEFI spec.
If ExitBootServices is unsuccessful, UEFI firmware memory allocation
should still be valid.  I don't believe it's forbidden at all since
ExitBootServices() was NOT successful.
> All this is a bit complex. But working around bugs is never trivial.
> Something similar is implemented in at least GRUB and Linux.
Results vary but can often be non-trivial.
> GRUB fiddle around to find the optimal size for the memory map and in
> the end add 3 pages of memory for safety.
>
http://git.savannah.gnu.org/cgit/grub.git/tree/grub-core/loader/i386/linux.c?id=206676601eb853fc319df14cd3398fbdfde665ac#n104
>
> GRUB then retry in an infinite loop to call GetMemoryMap +
> ExitBootServices. On each iteration it allocates a copy of the memory
> map and free it in case of failure. However, GRUB seems to have its
> own memory management, which allocate a quarter of the available
> memory on startup. This allows those allocation on each iteration to
> not disturb the memory map while trying to call ExitBootServices.
>
>
http://git.savannah.gnu.org/cgit/grub.git/tree/grub-core/kern/efi/mm.c?id=206676601eb853fc319df14cd3398fbdfde665ac#n148
>
http://git.savannah.gnu.org/cgit/grub.git/tree/grub-core/kern/efi/mm.c?id=206676601eb853fc319df14cd3398fbdfde665ac#n494
>
>
> Linux does something similar. It calls get_memory_map +
> exit_boot_services a maximum of two times. However, it allows itself
> to always call allocate_pool (in efi_get_memory_map) even after
> exit_boot_services failed once. Which does not comply with the UEFI
> specification.
>
>
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/x86/boot/compressed/eboot.c?id=e86328c489d7ecdca99410a06a3f448caf7857bf#n1299
>
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/firmware/efi/libstub/efi-stub-helper.c?id=e86328c489d7ecdca99410a06a3f448caf7857bf#n66
>
>
>
>>
>>> +       } while (status == EFI_BUFFER_TOO_SMALL);
>>> +
>>> +       if (status == EFI_SUCCESS) {
>>> +               *nr_entries = size / *desc_sz;
>>> +               return map;
>>> +       }
>>> +
>>> +       if (map)
>>> +               FreePool(map);
>>> +       return NULL;
>>> +}
>>> +
>>>  static int exit_boot(struct boot_params *bp)
>>>  {
>>>         struct e820_entry *e820buf, *e;
>>>         EFI_MEMORY_DESCRIPTOR *map;
>>>         EFI_STATUS status;
>>>         uint32_t e820_type;
>>> -       UINTN i, nr_entries, key, desc_sz;
>>> +       UINTN i, nr_entries, key, desc_sz, allocsize;
>>>         UINT32 desc_ver;
>>> +       int retry;
>>>
>>> -       /* Build efi memory map */
>>> -       map = get_memory_map(&nr_entries, &key,
&desc_sz, &desc_ver);
>>> -       if (!map)
>>> +       /*
>>> +        * Build efi memory map and call ExitBootServices as soon
after
>>> as
>>> +        * possible. Some implementations need two calls to
>>> ExitBootServices.
>>> +        */
>>> +
>>> +       map = NULL;
>>> +       allocsize = 0;
>>> +       retry = 0;
>>> +       do {
>>> +               map = get_memory_map_realloc(map, &allocsize,
&nr_entries,
>>> &key,
>>> +                                            &desc_sz,
&desc_ver);
>>> +               if (!map)
>>> +                       return -1;
>>> +
>>> +               status = uefi_call_wrapper(BS->ExitBootServices,
2,
>>> +                                          image_handle, key);
>>> +               retry++;
>>> +       } while (status != EFI_SUCCESS && retry < 3);
>>> +
>>> +       if (status != EFI_SUCCESS) {
>>> +               printf("Failed to exit boot services:
0x%016lx\n",
>>> status);
>>> +               FreePool(map);
>>>                 return -1;
>>> +       }
>>> +
>>
>> Call ExitBootServices() prior to any map parsing.  This makes sense as
>> long as no other functions get added after this that might want to
>> allocate more RAM.  This function looks good from here out.
It's also noted in the ExitBootServices() function to call
GetMemoryMap() then immediately call ExitBootServices().
>>>         bp->efi.memmap = (uint32_t)(unsigned long)map;
>>>         bp->efi.memmap_size = nr_entries * desc_sz;
>>> @@ -1088,13 +1148,6 @@ static int exit_boot(struct boot_params *bp)
>>>
>>>         bp->e820_entries = e - e820buf;
>>>
>>> -       status = uefi_call_wrapper(BS->ExitBootServices, 2,
image_handle,
>>> key);
>>> -       if (status != EFI_SUCCESS) {
>>> -               printf("Failed to exit boot services:
0x%016lx\n",
>>> status);
>>> -               FreePool(map);
>>> -               return -1;
>>> -       }
>>> -
>>>         return 0;
>>>  }
> I hope it's a bit clearer now. Sorry for the lengthy text. ^^
>
>
> Celelibi
I'd say it makes more sense.

-- 
-Gene