diff mbox

[U-Boot,v4,4/7] efi_loader: Track size of pool allocations to allow freeing

Message ID a9c2dfd819b44ebeb89bf46cec7dcd68@rwthex-w2-b.rwth-ad.de
State Accepted
Commit 42417bc84d1d56f739d43d562773d4821cf1bf47
Delegated to: Alexander Graf
Headers show

Commit Message

Stefan Brüns Oct. 9, 2016, 8:17 p.m. UTC
We need a functional free_pool implementation, as otherwise each
allocate_pool causes growth of the memory descriptor table.

Different to free_pages, free_pool does not provide the size for the
to be freed allocation, thus we have to track the size ourselves.

As the only EFI requirement for pool allocation is an alignment of
8 bytes, we can keep allocating a range using the page allocator,
reserve the first 8 bytes for our bookkeeping and hand out the
remainder to the caller. This saves us from having to use any
independent data structures for tracking.

To simplify the conversion between pool allocations and the corresponding
page allocation, we create an auxiliary struct efi_pool_allocation.

Given the allocation size free_pool size can handoff freeing the page
range, which was indirectly allocated by a call to allocate_pool,
to free_pages.

Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>
---
 include/efi_loader.h          |  2 ++
 lib/efi_loader/efi_boottime.c |  6 +++---
 lib/efi_loader/efi_memory.c   | 42 +++++++++++++++++++++++++++++++++++++++---
 3 files changed, 44 insertions(+), 6 deletions(-)

Comments

Alexander Graf Oct. 10, 2016, 11:39 a.m. UTC | #1
On 10/09/2016 10:17 PM, Stefan Brüns wrote:
> We need a functional free_pool implementation, as otherwise each
> allocate_pool causes growth of the memory descriptor table.
>
> Different to free_pages, free_pool does not provide the size for the
> to be freed allocation, thus we have to track the size ourselves.
>
> As the only EFI requirement for pool allocation is an alignment of
> 8 bytes, we can keep allocating a range using the page allocator,
> reserve the first 8 bytes for our bookkeeping and hand out the
> remainder to the caller. This saves us from having to use any
> independent data structures for tracking.
>
> To simplify the conversion between pool allocations and the corresponding
> page allocation, we create an auxiliary struct efi_pool_allocation.
>
> Given the allocation size free_pool size can handoff freeing the page
> range, which was indirectly allocated by a call to allocate_pool,
> to free_pages.
>
> Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>

Reviewed-by: Alexander Graf <agraf@suse.de>


Alex
Alexander Graf Oct. 13, 2016, 2:34 p.m. UTC | #2
> We need a functional free_pool implementation, as otherwise each
> allocate_pool causes growth of the memory descriptor table.
> 
> Different to free_pages, free_pool does not provide the size for the
> to be freed allocation, thus we have to track the size ourselves.
> 
> As the only EFI requirement for pool allocation is an alignment of
> 8 bytes, we can keep allocating a range using the page allocator,
> reserve the first 8 bytes for our bookkeeping and hand out the
> remainder to the caller. This saves us from having to use any
> independent data structures for tracking.
> 
> To simplify the conversion between pool allocations and the corresponding
> page allocation, we create an auxiliary struct efi_pool_allocation.
> 
> Given the allocation size free_pool size can handoff freeing the page
> range, which was indirectly allocated by a call to allocate_pool,
> to free_pages.
> 
> Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>
> Reviewed-by: Alexander Graf <agraf@suse.de>

Thanks, applied to
diff mbox

Patch

diff --git a/include/efi_loader.h b/include/efi_loader.h
index f0473ab..3dad24e 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -122,6 +122,8 @@  efi_status_t efi_free_pages(uint64_t memory, unsigned long pages);
 /* EFI memory allocator for small allocations */
 efi_status_t efi_allocate_pool(int pool_type, unsigned long size,
 			       void **buffer);
+/* EFI pool memory free function. */
+efi_status_t efi_free_pool(void *buffer);
 /* Returns the EFI memory map */
 efi_status_t efi_get_memory_map(unsigned long *memory_map_size,
 				struct efi_mem_desc *memory_map,
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index eb74cb0..8274d8e 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -141,12 +141,12 @@  static efi_status_t EFIAPI efi_allocate_pool_ext(int pool_type,
 	return EFI_EXIT(r);
 }
 
-static efi_status_t EFIAPI efi_free_pool(void *buffer)
+static efi_status_t EFIAPI efi_free_pool_ext(void *buffer)
 {
 	efi_status_t r;
 
 	EFI_ENTRY("%p", buffer);
-	r = efi_free_pages((ulong)buffer, 0);
+	r = efi_free_pool(buffer);
 	return EFI_EXIT(r);
 }
 
@@ -736,7 +736,7 @@  static const struct efi_boot_services efi_boot_services = {
 	.free_pages = efi_free_pages_ext,
 	.get_memory_map = efi_get_memory_map_ext,
 	.allocate_pool = efi_allocate_pool_ext,
-	.free_pool = efi_free_pool,
+	.free_pool = efi_free_pool_ext,
 	.create_event = efi_create_event,
 	.set_timer = efi_set_timer,
 	.wait_for_event = efi_wait_for_event,
diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
index be642f1..de28db6 100644
--- a/lib/efi_loader/efi_memory.c
+++ b/lib/efi_loader/efi_memory.c
@@ -34,6 +34,19 @@  void *efi_bounce_buffer;
 #endif
 
 /*
+ * U-Boot services each EFI AllocatePool request as a separate
+ * (multiple) page allocation.  We have to track the number of pages
+ * to be able to free the correct amount later.
+ * EFI requires 8 byte alignment for pool allocations, so we can
+ * prepend each allocation with an 64 bit header tracking the
+ * allocation size, and hand out the remainder to the caller.
+ */
+struct efi_pool_allocation {
+	u64 num_pages;
+	char data[];
+};
+
+/*
  * Sorts the memory list from highest address to lowest address
  *
  * When allocating memory we should always start from the highest
@@ -332,11 +345,34 @@  efi_status_t efi_allocate_pool(int pool_type, unsigned long size,
 {
 	efi_status_t r;
 	efi_physical_addr_t t;
-	u64 num_pages = (size + EFI_PAGE_MASK) >> EFI_PAGE_SHIFT;
+	u64 num_pages = (size + sizeof(u64) + EFI_PAGE_MASK) >> EFI_PAGE_SHIFT;
+
+	if (size == 0) {
+		*buffer = NULL;
+		return EFI_SUCCESS;
+	}
 
 	r = efi_allocate_pages(0, pool_type, num_pages, &t);
-	if (r == EFI_SUCCESS)
-		*buffer = (void *)(uintptr_t)t;
+
+	if (r == EFI_SUCCESS) {
+		struct efi_pool_allocation *alloc = (void *)(uintptr_t)t;
+		alloc->num_pages = num_pages;
+		*buffer = alloc->data;
+	}
+
+	return r;
+}
+
+efi_status_t efi_free_pool(void *buffer)
+{
+	efi_status_t r;
+	struct efi_pool_allocation *alloc;
+
+	alloc = container_of(buffer, struct efi_pool_allocation, data);
+	/* Sanity check, was the supplied address returned by allocate_pool */
+	assert(((uintptr_t)alloc & EFI_PAGE_MASK) == 0);
+
+	r = efi_free_pages((uintptr_t)alloc, alloc->num_pages);
 
 	return r;
 }