[PATCH 0/5] efi_loader: user EfiBootServicesData for device path

[PATCH 0/5] efi_loader: user EfiBootServicesData for device path

[Heinrich Schuchardt]

When creating device paths or converting the device paths to text we must
memory of type EfiBootServiceData.

Change function definitions to use enums instead of int to avoid future
misuse of constants.

Heinrich Schuchardt (5):
  efi_loader: use an enum for the memory allocation types
  efi_loader rename enum efi_mem_type to efi_memory_type
  efi_loader: use correct type for AllocatePages, AllocatePool
  efi_loader: use EfiBootServicesData for device path
  efi_loader: use EfiBootServicesData for DP to text

 arch/x86/include/asm/hob.h               |  2 +-
 include/efi.h                            | 36 +++++++++++++++++++-----
 include/efi_api.h                        |  2 +-
 include/efi_loader.h                     |  9 +++---
 lib/efi_loader/efi_device_path.c         |  2 +-
 lib/efi_loader/efi_device_path_to_text.c |  2 +-
 lib/efi_loader/efi_memory.c              |  5 ++--
 7 files changed, 41 insertions(+), 17 deletions(-)

-- 
2.30.2

[PATCH 1/5] efi_loader: use an enum for the memory allocation types

[Heinrich Schuchardt]

For type checking we need an enum.

Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
 include/efi.h | 34 ++++++++++++++++++++++++++++------
 1 file changed, 28 insertions(+), 6 deletions(-)

diff --git a/include/efi.h b/include/efi.h
index 6417a9b8c5..4eb573a280 100644
--- a/include/efi.h
+++ b/include/efi.h
@@ -125,6 +125,34 @@ struct efi_table_hdr {
 	u32 reserved;
 };
 
+/* Allocation types for calls to boottime->allocate_pages*/
+/**
+ * enum efi_allocate_type - address restriction for memory allocation
+ */
+enum efi_allocate_type {
+	/**
+	 * @EFI_ALLOCATE_ANY_PAGES:
+	 * Allocate any block of sufficient size. Ignore memory address.
+	 */
+	EFI_ALLOCATE_ANY_PAGES,
+	/**
+	 * @EFI_ALLOCATE_MAX_ADDRESS:
+	 * Allocate a memory block with an uppermost address less or equal
+	 * to the indicated address.
+	 */
+	EFI_ALLOCATE_MAX_ADDRESS,
+	/**
+	 * @EFI_ALLOCATE_ADDRESS:
+	 * Allocate a memory block starting at the indicated address.
+	 */
+	EFI_ALLOCATE_ADDRESS,
+	/**
+	 * @EFI_MAX_ALLOCATE_TYPE:
+	 * Value use for range checking.
+	 */
+	EFI_MAX_ALLOCATE_TYPE,
+};
+
 /* Enumeration of memory types introduced in UEFI */
 enum efi_mem_type {
 	EFI_RESERVED_MEMORY_TYPE,
@@ -224,12 +252,6 @@ struct efi_mem_desc {
 
 #define EFI_MEMORY_DESCRIPTOR_VERSION 1
 
-/* Allocation types for calls to boottime->allocate_pages*/
-#define EFI_ALLOCATE_ANY_PAGES		0
-#define EFI_ALLOCATE_MAX_ADDRESS	1
-#define EFI_ALLOCATE_ADDRESS		2
-#define EFI_MAX_ALLOCATE_TYPE		3
-
 /* Types and defines for Time Services */
 #define EFI_TIME_ADJUST_DAYLIGHT 0x1
 #define EFI_TIME_IN_DAYLIGHT     0x2
-- 
2.30.2

[PATCH 2/5] efi_loader rename enum efi_mem_type to efi_memory_type

[Heinrich Schuchardt]

Use the same name as in the UEFI specification to avoid confusion.

Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
 arch/x86/include/asm/hob.h | 2 +-
 include/efi.h              | 2 +-
 include/efi_api.h          | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/hob.h b/arch/x86/include/asm/hob.h
index 56e11dbb28..2f5b6e24c2 100644
--- a/arch/x86/include/asm/hob.h
+++ b/arch/x86/include/asm/hob.h
@@ -91,7 +91,7 @@ struct hob_mem_alloc {
 	 * Type EFI_MEMORY_TYPE is defined in AllocatePages() in the UEFI 2.0
 	 * specification.
 	 */
-	enum efi_mem_type	mem_type;
+	enum efi_memory_type	mem_type;
 	/* padding */
 	u8			reserved[4];
 };
diff --git a/include/efi.h b/include/efi.h
index 4eb573a280..18c13e0370 100644
--- a/include/efi.h
+++ b/include/efi.h
@@ -154,7 +154,7 @@ enum efi_allocate_type {
 };
 
 /* Enumeration of memory types introduced in UEFI */
-enum efi_mem_type {
+enum efi_memory_type {
 	EFI_RESERVED_MEMORY_TYPE,
 	/*
 	 * The code portions of a loaded application.
diff --git a/include/efi_api.h b/include/efi_api.h
index 38ac47f164..c8f959bb72 100644
--- a/include/efi_api.h
+++ b/include/efi_api.h
@@ -249,7 +249,7 @@ struct efi_memory_range {
 struct efi_memory_range_capsule {
 	struct efi_capsule_header *header;
 	/* EFI_MEMORY_TYPE: 0x80000000-0xFFFFFFFF */
-	enum efi_mem_type os_requested_memory_type;
+	enum efi_memory_type os_requested_memory_type;
 	u64 number_of_memory_ranges;
 	struct efi_memory_range memory_ranges[];
 } __packed;
-- 
2.30.2

[PATCH 3/5] efi_loader: use correct type for AllocatePages, AllocatePool

[Heinrich Schuchardt]

Use enum efi_memory_type and enum_allocate_type in the definitions of the
efi_allocate_pages(), efi_allocate_pool().

In the external UEFI API leave the type as int as the UEFI specification
explicitly requires that enums use a 32bit type.

Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
 include/efi_loader.h        | 9 +++++----
 lib/efi_loader/efi_memory.c | 5 +++--
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/include/efi_loader.h b/include/efi_loader.h
index 32cb8d0f1e..c440962fe5 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -676,13 +676,14 @@ struct efi_device_path *efi_get_dp_from_boot(const efi_guid_t guid);
 /* Generic EFI memory allocator, call this to get memory */
 void *efi_alloc(uint64_t len, int memory_type);
 /* More specific EFI memory allocator, called by EFI payloads */
-efi_status_t efi_allocate_pages(int type, int memory_type, efi_uintn_t pages,
-				uint64_t *memory);
+efi_status_t efi_allocate_pages(enum efi_allocate_type type,
+				enum efi_memory_type memory_type,
+				efi_uintn_t pages, uint64_t *memory);
 /* EFI memory free function. */
 efi_status_t efi_free_pages(uint64_t memory, efi_uintn_t pages);
 /* EFI memory allocator for small allocations */
-efi_status_t efi_allocate_pool(int pool_type, efi_uintn_t size,
-			       void **buffer);
+efi_status_t efi_allocate_pool(enum efi_memory_type pool_type,
+			       efi_uintn_t size, void **buffer);
 /* EFI pool memory free function. */
 efi_status_t efi_free_pool(void *buffer);
 /* Returns the EFI memory map */
diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
index be2f655dff..f4acbee4f9 100644
--- a/lib/efi_loader/efi_memory.c
+++ b/lib/efi_loader/efi_memory.c
@@ -454,7 +454,8 @@ static uint64_t efi_find_free_memory(uint64_t len, uint64_t max_addr)
  * @memory		allocated memory
  * @return		status code
  */
-efi_status_t efi_allocate_pages(int type, int memory_type,
+efi_status_t efi_allocate_pages(enum efi_allocate_type type,
+				enum efi_memory_type memory_type,
 				efi_uintn_t pages, uint64_t *memory)
 {
 	u64 len = pages << EFI_PAGE_SHIFT;
@@ -556,7 +557,7 @@ efi_status_t efi_free_pages(uint64_t memory, efi_uintn_t pages)
  * @buffer:	allocated memory
  * Return:	status code
  */
-efi_status_t efi_allocate_pool(int pool_type, efi_uintn_t size, void **buffer)
+efi_status_t efi_allocate_pool(enum efi_memory_type pool_type, efi_uintn_t size, void **buffer)
 {
 	efi_status_t r;
 	u64 addr;
-- 
2.30.2

[PATCH 4/5] efi_loader: use EfiBootServicesData for device path

[Heinrich Schuchardt]

dp_alloc() was using a constant from the wrong enum resulting in creating
device paths in EfiReservedMemory.

Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
 lib/efi_loader/efi_device_path.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c
index 9c3ac712fe..cbdb466da4 100644
--- a/lib/efi_loader/efi_device_path.c
+++ b/lib/efi_loader/efi_device_path.c
@@ -68,7 +68,7 @@ static void *dp_alloc(size_t sz)
 {
 	void *buf;
 
-	if (efi_allocate_pool(EFI_ALLOCATE_ANY_PAGES, sz, &buf) !=
+	if (efi_allocate_pool(EFI_BOOT_SERVICES_DATA, sz, &buf) !=
 	    EFI_SUCCESS) {
 		debug("EFI: ERROR: out of memory in %s\n", __func__);
 		return NULL;
-- 
2.30.2

[PATCH 5/5] efi_loader: use EfiBootServicesData for DP to text

[Heinrich Schuchardt]

Memory allocated in the implementation of the
EFI_DEVICE_PATH_TO_TEXT_PROTOCOL must be of type EfiBootServicesData.

Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
 lib/efi_loader/efi_device_path_to_text.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/efi_loader/efi_device_path_to_text.c b/lib/efi_loader/efi_device_path_to_text.c
index d46327a1c9..57fa9d97f7 100644
--- a/lib/efi_loader/efi_device_path_to_text.c
+++ b/lib/efi_loader/efi_device_path_to_text.c
@@ -34,7 +34,7 @@ static u16 *efi_str_to_u16(char *str)
 	efi_status_t ret;
 
 	len = sizeof(u16) * (utf8_utf16_strlen(str) + 1);
-	ret = efi_allocate_pool(EFI_ALLOCATE_ANY_PAGES, len, (void **)&out);
+	ret = efi_allocate_pool(EFI_BOOT_SERVICES_DATA, len, (void **)&out);
 	if (ret != EFI_SUCCESS)
 		return NULL;
 	dst = out;
-- 
2.30.2

Re: [PATCH 3/5] efi_loader: use correct type for AllocatePages, AllocatePool

[AKASHI Takahiro]

Heinrich,

On Tue, Aug 17, 2021 at 06:02:23PM +0200, Heinrich Schuchardt wrote:
> Use enum efi_memory_type and enum_allocate_type in the definitions of the
> efi_allocate_pages(), efi_allocate_pool().
> 
> In the external UEFI API leave the type as int as the UEFI specification
> explicitly requires that enums use a 32bit type.
> 
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
>  include/efi_loader.h        | 9 +++++----
>  lib/efi_loader/efi_memory.c | 5 +++--
>  2 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index 32cb8d0f1e..c440962fe5 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -676,13 +676,14 @@ struct efi_device_path *efi_get_dp_from_boot(const efi_guid_t guid);
>  /* Generic EFI memory allocator, call this to get memory */
>  void *efi_alloc(uint64_t len, int memory_type);
>  /* More specific EFI memory allocator, called by EFI payloads */
> -efi_status_t efi_allocate_pages(int type, int memory_type, efi_uintn_t pages,
> -				uint64_t *memory);
> +efi_status_t efi_allocate_pages(enum efi_allocate_type type,
> +				enum efi_memory_type memory_type,
> +				efi_uintn_t pages, uint64_t *memory);
>  /* EFI memory free function. */
>  efi_status_t efi_free_pages(uint64_t memory, efi_uintn_t pages);
>  /* EFI memory allocator for small allocations */
> -efi_status_t efi_allocate_pool(int pool_type, efi_uintn_t size,
> -			       void **buffer);
> +efi_status_t efi_allocate_pool(enum efi_memory_type pool_type,
> +			       efi_uintn_t size, void **buffer);
>  /* EFI pool memory free function. */
>  efi_status_t efi_free_pool(void *buffer);
>  /* Returns the EFI memory map */
> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
> index be2f655dff..f4acbee4f9 100644
> --- a/lib/efi_loader/efi_memory.c
> +++ b/lib/efi_loader/efi_memory.c
> @@ -454,7 +454,8 @@ static uint64_t efi_find_free_memory(uint64_t len, uint64_t max_addr)
>   * @memory		allocated memory
>   * @return		status code
>   */
> -efi_status_t efi_allocate_pages(int type, int memory_type,
> +efi_status_t efi_allocate_pages(enum efi_allocate_type type,
> +				enum efi_memory_type memory_type,
>  				efi_uintn_t pages, uint64_t *memory)
>  {
>  	u64 len = pages << EFI_PAGE_SHIFT;
> @@ -556,7 +557,7 @@ efi_status_t efi_free_pages(uint64_t memory, efi_uintn_t pages)
>   * @buffer:	allocated memory
>   * Return:	status code
>   */
> -efi_status_t efi_allocate_pool(int pool_type, efi_uintn_t size, void **buffer)
> +efi_status_t efi_allocate_pool(enum efi_memory_type pool_type, efi_uintn_t size, void **buffer)

Given the purpose of this patch series, I think that the second argument
of this function should be renamed from "pool_type" to "memory_type"
which is also used in efi_allocate_pages() to avoid any confusion.
(and the description for @pool_type as well)

Otherwise, it looks good.

-Takahiro Akashi


>  {
>  	efi_status_t r;
>  	u64 addr;
> -- 
> 2.30.2
> 

Re: [PATCH 3/5] efi_loader: use correct type for AllocatePages, AllocatePool

[Heinrich Schuchardt]

Am 18. August 2021 03:45:28 MESZ schrieb AKASHI Takahiro <takahiro.akashi@linaro.org>:
>Heinrich,
>
>On Tue, Aug 17, 2021 at 06:02:23PM +0200, Heinrich Schuchardt wrote:
>> Use enum efi_memory_type and enum_allocate_type in the definitions of the
>> efi_allocate_pages(), efi_allocate_pool().
>> 
>> In the external UEFI API leave the type as int as the UEFI specification
>> explicitly requires that enums use a 32bit type.
>> 
>> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
>> ---
>>  include/efi_loader.h        | 9 +++++----
>>  lib/efi_loader/efi_memory.c | 5 +++--
>>  2 files changed, 8 insertions(+), 6 deletions(-)
>> 
>> diff --git a/include/efi_loader.h b/include/efi_loader.h
>> index 32cb8d0f1e..c440962fe5 100644
>> --- a/include/efi_loader.h
>> +++ b/include/efi_loader.h
>> @@ -676,13 +676,14 @@ struct efi_device_path *efi_get_dp_from_boot(const efi_guid_t guid);
>>  /* Generic EFI memory allocator, call this to get memory */
>>  void *efi_alloc(uint64_t len, int memory_type);
>>  /* More specific EFI memory allocator, called by EFI payloads */
>> -efi_status_t efi_allocate_pages(int type, int memory_type, efi_uintn_t pages,
>> -				uint64_t *memory);
>> +efi_status_t efi_allocate_pages(enum efi_allocate_type type,
>> +				enum efi_memory_type memory_type,
>> +				efi_uintn_t pages, uint64_t *memory);
>>  /* EFI memory free function. */
>>  efi_status_t efi_free_pages(uint64_t memory, efi_uintn_t pages);
>>  /* EFI memory allocator for small allocations */
>> -efi_status_t efi_allocate_pool(int pool_type, efi_uintn_t size,
>> -			       void **buffer);
>> +efi_status_t efi_allocate_pool(enum efi_memory_type pool_type,
>> +			       efi_uintn_t size, void **buffer);
>>  /* EFI pool memory free function. */
>>  efi_status_t efi_free_pool(void *buffer);
>>  /* Returns the EFI memory map */
>> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
>> index be2f655dff..f4acbee4f9 100644
>> --- a/lib/efi_loader/efi_memory.c
>> +++ b/lib/efi_loader/efi_memory.c
>> @@ -454,7 +454,8 @@ static uint64_t efi_find_free_memory(uint64_t len, uint64_t max_addr)
>>   * @memory		allocated memory
>>   * @return		status code
>>   */
>> -efi_status_t efi_allocate_pages(int type, int memory_type,
>> +efi_status_t efi_allocate_pages(enum efi_allocate_type type,
>> +				enum efi_memory_type memory_type,
>>  				efi_uintn_t pages, uint64_t *memory)
>>  {
>>  	u64 len = pages << EFI_PAGE_SHIFT;
>> @@ -556,7 +557,7 @@ efi_status_t efi_free_pages(uint64_t memory, efi_uintn_t pages)
>>   * @buffer:	allocated memory
>>   * Return:	status code
>>   */
>> -efi_status_t efi_allocate_pool(int pool_type, efi_uintn_t size, void **buffer)
>> +efi_status_t efi_allocate_pool(enum efi_memory_type pool_type, efi_uintn_t size, void **buffer)
>
>Given the purpose of this patch series, I think that the second argument
>of this function should be renamed from "pool_type" to "memory_type"
>which is also used in efi_allocate_pages() to avoid any confusion.
>(and the description for @pool_type as well)

pool_type is the name used by the UEFI specification.

Best regards

Heinrich

>
>Otherwise, it looks good.
>
>-Takahiro Akashi
>
>
>>  {
>>  	efi_status_t r;
>>  	u64 addr;
>> -- 
>> 2.30.2
>> 

Re: [PATCH 3/5] efi_loader: use correct type for AllocatePages, AllocatePool

[AKASHI Takahiro]

On Wed, Aug 18, 2021 at 06:50:40AM +0200, Heinrich Schuchardt wrote:
> Am 18. August 2021 03:45:28 MESZ schrieb AKASHI Takahiro <takahiro.akashi@linaro.org>:
> >Heinrich,
> >
> >On Tue, Aug 17, 2021 at 06:02:23PM +0200, Heinrich Schuchardt wrote:
> >> Use enum efi_memory_type and enum_allocate_type in the definitions of the
> >> efi_allocate_pages(), efi_allocate_pool().
> >> 
> >> In the external UEFI API leave the type as int as the UEFI specification
> >> explicitly requires that enums use a 32bit type.
> >> 
> >> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> >> ---
> >>  include/efi_loader.h        | 9 +++++----
> >>  lib/efi_loader/efi_memory.c | 5 +++--
> >>  2 files changed, 8 insertions(+), 6 deletions(-)
> >> 
> >> diff --git a/include/efi_loader.h b/include/efi_loader.h
> >> index 32cb8d0f1e..c440962fe5 100644
> >> --- a/include/efi_loader.h
> >> +++ b/include/efi_loader.h
> >> @@ -676,13 +676,14 @@ struct efi_device_path *efi_get_dp_from_boot(const efi_guid_t guid);
> >>  /* Generic EFI memory allocator, call this to get memory */
> >>  void *efi_alloc(uint64_t len, int memory_type);
> >>  /* More specific EFI memory allocator, called by EFI payloads */
> >> -efi_status_t efi_allocate_pages(int type, int memory_type, efi_uintn_t pages,
> >> -				uint64_t *memory);
> >> +efi_status_t efi_allocate_pages(enum efi_allocate_type type,
> >> +				enum efi_memory_type memory_type,
> >> +				efi_uintn_t pages, uint64_t *memory);
> >>  /* EFI memory free function. */
> >>  efi_status_t efi_free_pages(uint64_t memory, efi_uintn_t pages);
> >>  /* EFI memory allocator for small allocations */
> >> -efi_status_t efi_allocate_pool(int pool_type, efi_uintn_t size,
> >> -			       void **buffer);
> >> +efi_status_t efi_allocate_pool(enum efi_memory_type pool_type,
> >> +			       efi_uintn_t size, void **buffer);
> >>  /* EFI pool memory free function. */
> >>  efi_status_t efi_free_pool(void *buffer);
> >>  /* Returns the EFI memory map */
> >> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
> >> index be2f655dff..f4acbee4f9 100644
> >> --- a/lib/efi_loader/efi_memory.c
> >> +++ b/lib/efi_loader/efi_memory.c
> >> @@ -454,7 +454,8 @@ static uint64_t efi_find_free_memory(uint64_t len, uint64_t max_addr)
> >>   * @memory		allocated memory
> >>   * @return		status code
> >>   */
> >> -efi_status_t efi_allocate_pages(int type, int memory_type,
> >> +efi_status_t efi_allocate_pages(enum efi_allocate_type type,
> >> +				enum efi_memory_type memory_type,
> >>  				efi_uintn_t pages, uint64_t *memory)
> >>  {
> >>  	u64 len = pages << EFI_PAGE_SHIFT;
> >> @@ -556,7 +557,7 @@ efi_status_t efi_free_pages(uint64_t memory, efi_uintn_t pages)
> >>   * @buffer:	allocated memory
> >>   * Return:	status code
> >>   */
> >> -efi_status_t efi_allocate_pool(int pool_type, efi_uintn_t size, void **buffer)
> >> +efi_status_t efi_allocate_pool(enum efi_memory_type pool_type, efi_uintn_t size, void **buffer)
> >
> >Given the purpose of this patch series, I think that the second argument
> >of this function should be renamed from "pool_type" to "memory_type"
> >which is also used in efi_allocate_pages() to avoid any confusion.
> >(and the description for @pool_type as well)
> 
> pool_type is the name used by the UEFI specification.

So what?

(for efi_allocate_pages())
!/*
! * Allocate memory pages.
! *
! * @type                type of allocation to be performed
! * @memory_type         usage type of the allocated memory

!/**
! * efi_allocate_pool - allocate memory from pool
! *
! * @pool_type:  type of the pool from which memory is to be allocated

You give the same type of arguments different names and explanation.
I think that could be confusing.
It is worth noting that efi_allocate_pool() directly passes
the "pool_type" variable to efi_allocate_pages().

-Takahiro Akashi

> Best regards
> 
> Heinrich
> 
> >
> >Otherwise, it looks good.
> >
> >-Takahiro Akashi
> >
> >
> >>  {
> >>  	efi_status_t r;
> >>  	u64 addr;
> >> -- 
> >> 2.30.2
> >> 
> 

Re: [PATCH 3/5] efi_loader: use correct type for AllocatePages, AllocatePool

[AKASHI Takahiro]

Heinrich,

On Wed, Aug 18, 2021 at 02:23:40PM +0900, AKASHI Takahiro wrote:
> On Wed, Aug 18, 2021 at 06:50:40AM +0200, Heinrich Schuchardt wrote:
> > Am 18. August 2021 03:45:28 MESZ schrieb AKASHI Takahiro <takahiro.akashi@linaro.org>:
> > >Heinrich,
> > >
> > >On Tue, Aug 17, 2021 at 06:02:23PM +0200, Heinrich Schuchardt wrote:
> > >> Use enum efi_memory_type and enum_allocate_type in the definitions of the
> > >> efi_allocate_pages(), efi_allocate_pool().
> > >> 
> > >> In the external UEFI API leave the type as int as the UEFI specification
> > >> explicitly requires that enums use a 32bit type.
> > >> 
> > >> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> > >> ---
> > >>  include/efi_loader.h        | 9 +++++----
> > >>  lib/efi_loader/efi_memory.c | 5 +++--
> > >>  2 files changed, 8 insertions(+), 6 deletions(-)
> > >> 
> > >> diff --git a/include/efi_loader.h b/include/efi_loader.h
> > >> index 32cb8d0f1e..c440962fe5 100644
> > >> --- a/include/efi_loader.h
> > >> +++ b/include/efi_loader.h
> > >> @@ -676,13 +676,14 @@ struct efi_device_path *efi_get_dp_from_boot(const efi_guid_t guid);
> > >>  /* Generic EFI memory allocator, call this to get memory */
> > >>  void *efi_alloc(uint64_t len, int memory_type);
> > >>  /* More specific EFI memory allocator, called by EFI payloads */
> > >> -efi_status_t efi_allocate_pages(int type, int memory_type, efi_uintn_t pages,
> > >> -				uint64_t *memory);
> > >> +efi_status_t efi_allocate_pages(enum efi_allocate_type type,
> > >> +				enum efi_memory_type memory_type,
> > >> +				efi_uintn_t pages, uint64_t *memory);
> > >>  /* EFI memory free function. */
> > >>  efi_status_t efi_free_pages(uint64_t memory, efi_uintn_t pages);
> > >>  /* EFI memory allocator for small allocations */
> > >> -efi_status_t efi_allocate_pool(int pool_type, efi_uintn_t size,
> > >> -			       void **buffer);
> > >> +efi_status_t efi_allocate_pool(enum efi_memory_type pool_type,
> > >> +			       efi_uintn_t size, void **buffer);
> > >>  /* EFI pool memory free function. */
> > >>  efi_status_t efi_free_pool(void *buffer);
> > >>  /* Returns the EFI memory map */
> > >> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
> > >> index be2f655dff..f4acbee4f9 100644
> > >> --- a/lib/efi_loader/efi_memory.c
> > >> +++ b/lib/efi_loader/efi_memory.c
> > >> @@ -454,7 +454,8 @@ static uint64_t efi_find_free_memory(uint64_t len, uint64_t max_addr)
> > >>   * @memory		allocated memory
> > >>   * @return		status code
> > >>   */
> > >> -efi_status_t efi_allocate_pages(int type, int memory_type,
> > >> +efi_status_t efi_allocate_pages(enum efi_allocate_type type,
> > >> +				enum efi_memory_type memory_type,
> > >>  				efi_uintn_t pages, uint64_t *memory)
> > >>  {
> > >>  	u64 len = pages << EFI_PAGE_SHIFT;
> > >> @@ -556,7 +557,7 @@ efi_status_t efi_free_pages(uint64_t memory, efi_uintn_t pages)
> > >>   * @buffer:	allocated memory
> > >>   * Return:	status code
> > >>   */
> > >> -efi_status_t efi_allocate_pool(int pool_type, efi_uintn_t size, void **buffer)
> > >> +efi_status_t efi_allocate_pool(enum efi_memory_type pool_type, efi_uintn_t size, void **buffer)
> > >
> > >Given the purpose of this patch series, I think that the second argument
> > >of this function should be renamed from "pool_type" to "memory_type"
> > >which is also used in efi_allocate_pages() to avoid any confusion.
> > >(and the description for @pool_type as well)
> > 
> > pool_type is the name used by the UEFI specification.
> 
> So what?
> 
> (for efi_allocate_pages())
> !/*
> ! * Allocate memory pages.
> ! *
> ! * @type                type of allocation to be performed
> ! * @memory_type         usage type of the allocated memory
> 
> !/**
> ! * efi_allocate_pool - allocate memory from pool
> ! *
> ! * @pool_type:  type of the pool from which memory is to be allocated
> 
> You give the same type of arguments different names and explanation.
> I think that could be confusing.
> It is worth noting that efi_allocate_pool() directly passes
> the "pool_type" variable to efi_allocate_pages().

Did you ignore my comment?

-Takahiro Akashi


> -Takahiro Akashi
> 
> > Best regards
> > 
> > Heinrich
> > 
> > >
> > >Otherwise, it looks good.
> > >
> > >-Takahiro Akashi
> > >
> > >
> > >>  {
> > >>  	efi_status_t r;
> > >>  	u64 addr;
> > >> -- 
> > >> 2.30.2
> > >> 
> >