On 01.10.16 23:32, Stefan Brüns wrote: > Implementation essentially unchanged, but use EFI_PAGE_MASK/SHIFT > instead of numeric constants.
Sorry for being nitpicky about the commit message again. Imagine you're a distribution maintainer for U-Boot and you need to read the commit messages of all commits in the tree to figure out whether you need to cherry-pick a patch or not. Or you git bisect down to this commit and need to figure out what the original intention of this patch was. How about something like --- We currently handle efi_allocate_pool() inside of our boot time service file. In the following patch, pool allocation will receive additional internal semantics that we should preserve inside efi_memory.c instead. As foundation for those changes, split the function into an externally facing helper efi_allocate_pool_ext() for use by payloads and an internal function efi_allocate_pool() in efi_memory.c that handles the actual allocation. While at it, change the magic 0xfff / 12 constants to the more obvious EFI_PAGE_MASK/SHIFT defines. --- Alex > > Signed-off-by: Stefan Brüns <stefan.bru...@rwth-aachen.de> > --- > include/efi_loader.h | 3 +++ > lib/efi_loader/efi_boottime.c | 11 +++++------ > lib/efi_loader/efi_memory.c | 11 +++++++++++ > 3 files changed, 19 insertions(+), 6 deletions(-) > > diff --git a/include/efi_loader.h b/include/efi_loader.h > index 9738835..40e7beb 100644 > --- a/include/efi_loader.h > +++ b/include/efi_loader.h > @@ -119,6 +119,9 @@ efi_status_t efi_allocate_pages(int type, int > memory_type, unsigned long pages, > uint64_t *memory); > /* EFI memory free function. Not implemented today */ > efi_status_t efi_free_pages(uint64_t memory, unsigned long pages); > +/* EFI memory allocator for small allocations, called by EFI payloads */ The function that gets called by EFI payloads is actually the _ext one, as that's the one that swizzles the gd pointer as well. Just remove the last bit of the sentence. > +efi_status_t efi_allocate_pool(int pool_type, unsigned long size, > + 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 784891b..eb74cb0 100644 > --- a/lib/efi_loader/efi_boottime.c > +++ b/lib/efi_loader/efi_boottime.c > @@ -130,15 +130,14 @@ efi_status_t EFIAPI efi_get_memory_map_ext(unsigned > long *memory_map_size, > return EFI_EXIT(r); > } > > -static efi_status_t EFIAPI efi_allocate_pool(int pool_type, unsigned long > size, > - void **buffer) > +static efi_status_t EFIAPI efi_allocate_pool_ext(int pool_type, > + unsigned long size, > + void **buffer) > { > efi_status_t r; > - efi_physical_addr_t t; > > EFI_ENTRY("%d, %ld, %p", pool_type, size, buffer); > - r = efi_allocate_pages(0, pool_type, (size + 0xfff) >> 12, &t); > - *buffer = (void *)(uintptr_t)t; > + r = efi_allocate_pool(pool_type, size, buffer); > return EFI_EXIT(r); > } > > @@ -736,7 +735,7 @@ static const struct efi_boot_services efi_boot_services = > { > .allocate_pages = efi_allocate_pages_ext, > .free_pages = efi_free_pages_ext, > .get_memory_map = efi_get_memory_map_ext, > - .allocate_pool = efi_allocate_pool, > + .allocate_pool = efi_allocate_pool_ext, > .free_pool = efi_free_pool, > .create_event = efi_create_event, > .set_timer = efi_set_timer, > diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c > index 5d71fdf..045558d 100644 > --- a/lib/efi_loader/efi_memory.c > +++ b/lib/efi_loader/efi_memory.c > @@ -327,6 +327,17 @@ efi_status_t efi_free_pages(uint64_t memory, unsigned > long pages) > return EFI_SUCCESS; > } > > +efi_status_t efi_allocate_pool(int pool_type, unsigned long size, > + void **buffer) > +{ > + efi_status_t r; > + efi_physical_addr_t t; > + u64 num_pages = (size + EFI_PAGE_MASK) >> EFI_PAGE_SHIFT; > + > + r = efi_allocate_pages(0, pool_type, num_pages, &t); This is missing the *buffer update, no? > + return EFI_EXIT(r); Double EFI_EXIT is fatal. Please just return r here. Alex > +} > + > efi_status_t efi_get_memory_map(unsigned long *memory_map_size, > struct efi_mem_desc *memory_map, > unsigned long *map_key, > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot