[PATCH v2 01/28] efi: Define fields in struct efi_mem_desc

[Heinrich Schuchardt]

On 28.11.24 16:47, Simon Glass wrote:
> There is quite a bit of confusion in the EFI code as to whether a field
> contains an address or a pointer. As a first step towards resolving
> this, document the memory-descriptor struct, indicating that it holds
> pointers, not addresses.
>
> Dro the same for efi_add_memory_map() as it is widely used, as well as
> efi_add_memory_map_pg() which is only used by lmb
>
> Signed-off-by: Simon Glass <sjg at chromium.org>
> ---
>
> Changes in v2:
> - Fix missing @
> - Note that this holds pointers, not addresses
>
>   include/efi.h               | 15 +++++++++++++++
>   include/efi_loader.h        |  6 ++++--
>   lib/efi_loader/efi_memory.c |  4 +++-
>   3 files changed, 22 insertions(+), 3 deletions(-)
>
> diff --git a/include/efi.h b/include/efi.h
> index c559fda3004..3fce281233c 100644
> --- a/include/efi.h
> +++ b/include/efi.h
> @@ -273,6 +273,21 @@ enum efi_memory_type {
>   #define EFI_PAGE_SIZE		(1ULL << EFI_PAGE_SHIFT)
>   #define EFI_PAGE_MASK		(EFI_PAGE_SIZE - 1)
>
> +/**
> + * struct efi_mem_desc - defines an EFI memory record

Please, add here that this structure implements the
EFI_MEMORY_DESCRIPTOR type of the UEFI specification.

> + *
> + * Note that this struct is for use outside U-Boot, so the two 'start' fields
> + * are pointers, not addresses. Use map_to_sysmem() to convert to an address, so
> + * that sandbox operates correctly.
> + *
> + * @type (enum efi_memory_type): EFI memory-type
> + * @reserved: unused
> + * @physical_start: Start address of region in physical memory
> + * @virtual_start: Start address of region in physical memory

%s/virtual memory/

where both addresses will always be the same before
SetVirtualMemoryMap() as the UEFI specification requires identity mapping.

> + * @num_pages: Number of EFI pages this record covers (each is EFI_PAGE_SIZE
> + *	bytes)
> + * @attribute: Memory attributes (see EFI_MEMORY_...)
> + */
>   struct efi_mem_desc {
>   	u32 type;
>   	u32 reserved;
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index 39809eac1bc..ee0cdd36500 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -788,8 +788,10 @@ efi_status_t efi_add_memory_map(u64 start, u64 size, int memory_type);
>   /**
>    * efi_add_memory_map_pg() - add pages to the memory map
>    *
> - * @start:			start address, must be a multiple of
> - *				EFI_PAGE_SIZE
> + * @start: start address, must be a multiple of EFI_PAGE_SIZE. Note that this
> + * is actually a pointer provided as an integer. To pass in an address, pass

%s/an address/a sandbox virtual address/

> + * in (ulong)map_to_sysmem(addr)
> + *
>    * @pages:			number of pages to add
>    * @memory_type:		type of memory added
>    * @overlap_conventional:	region may only overlap free(conventional)
> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
> index e493934c713..f1154f73e05 100644
> --- a/lib/efi_loader/efi_memory.c
> +++ b/lib/efi_loader/efi_memory.c
> @@ -384,7 +384,9 @@ efi_status_t efi_add_memory_map_pg(u64 start, u64 pages,
>   /**
>    * efi_add_memory_map() - add memory area to the memory map
>    *
> - * @start:		start address of the memory area
> + * @start:		start address of the memory area. Note that this is
> + *			actually a pointer provided as an integer. To pass in
> + *			an address, pass in (ulong)map_to_sysmem(addr)

%s/an address/a sandbox virtual address/

Best regards

Heinrich

>    * @size:		length in bytes of the memory area
>    * @memory_type:	type of memory added
>    *