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