[PATCH] efi_loader: get_memory_map: return parameters whenever possible

[Heinrich Schuchardt]

On 3/11/20 7:18 AM, AKASHI Takahiro wrote:
> Currently, if GetMemoryMap API returns EFI_BUFFER_TOO_SMALL, it doesn't
> set valid values to other parameters, descriptor_size and
> descriptor_version, except memory_map_size.
> Some efi applications, however, may use those value; in particular,
> xen uses descriptor_size to calculate a size of buffer to be allocated.
>
> While UEFI specification is ambiguous in this point, it would be better
> to address this issue proactively to maximize the compatibility with
> existing efi applications.
>
> With this patch, for example, xen.efi (and hence linux kernel) can be
> started via bootefi without modification.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>

Your change matches the logic in EDK2.

There is still a difference. This is a snippet from
MdeModulePkg/Core/Dxe/Mem/Page.c:

   Size = sizeof (EFI_MEMORY_DESCRIPTOR);

   //
   // Make sure Size != sizeof(EFI_MEMORY_DESCRIPTOR). This will
   // prevent people from having pointer math bugs in their code.
   // now you have to use *DescriptorSize to make things work.
   //
   Size += sizeof(UINT64) - (Size % sizeof (UINT64));

   if (DescriptorSize != NULL) {
     *DescriptorSize = Size;
   }

If Size is not a multiple of 8 it rounds up to a multiple of 8. But if
Size is a multiple of 8 is 8 is added. The size of EFI_MEMORY_DESCRIPTOR
is a multiple of 8. - I have no clue why EDK2 is doing this and would
not advice to copy this.

Reviewed-by: Heinrich Schuchardt <xypron.glpk at gmx.de>

> ---
>   lib/efi_loader/efi_memory.c | 12 ++++++------
>   1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
> index 89adf2031024..97d90f069a63 100644
> --- a/lib/efi_loader/efi_memory.c
> +++ b/lib/efi_loader/efi_memory.c
> @@ -627,18 +627,18 @@ efi_status_t efi_get_memory_map(efi_uintn_t *memory_map_size,
>
>   	*memory_map_size = map_size;
>
> -	if (provided_map_size < map_size)
> -		return EFI_BUFFER_TOO_SMALL;
> -
> -	if (!memory_map)
> -		return EFI_INVALID_PARAMETER;
> -
>   	if (descriptor_size)
>   		*descriptor_size = sizeof(struct efi_mem_desc);
>
>   	if (descriptor_version)
>   		*descriptor_version = EFI_MEMORY_DESCRIPTOR_VERSION;
>
> +	if (provided_map_size < map_size)
> +		return EFI_BUFFER_TOO_SMALL;
> +
> +	if (!memory_map)
> +		return EFI_INVALID_PARAMETER;
> +
>   	/* Copy list into array */
>   	/* Return the list in ascending order */
>   	memory_map = &memory_map[map_entries - 1];
>