On 3 July 2018 at 03:50, Sai Praneeth Prakhya
<sai.praneeth.prakhya@xxxxxxxxx> wrote:
> From: Sai Praneeth <sai.praneeth.prakhya@xxxxxxxxx>
>
> efi_memmap_alloc() allocates memory depending on whether mm_init() has
> already been invoked or not. Apart from memblock_alloc() memory and
> alloc_pages() memory, efi memory map could also have a third variant of
> memory allocation and that is memblock_reserved. This happens only for
> the memory map passed to kernel by firmware and thus can happen only
> once during boot process.
>
> In order to identify these three different types of allocations and thus
> to call the appropriate free() variant, introduce an enum named
> efi_memmap_type and also introduce a efi memmap API named
> efi_memmap_free() to free memory allocated by efi_memmap_alloc().
>
> Signed-off-by: Sai Praneeth Prakhya <sai.praneeth.prakhya@xxxxxxxxx>
> Suggested-by: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx>
> Cc: Lee Chun-Yi <jlee@xxxxxxxx>
> Cc: Dave Young <dyoung@xxxxxxxxxx>
> Cc: Borislav Petkov <bp@xxxxxxxxx>
> Cc: Laszlo Ersek <lersek@xxxxxxxxxx>
> Cc: Jan Kiszka <jan.kiszka@xxxxxxxxxxx>
> Cc: Dave Hansen <dave.hansen@xxxxxxxxx>
> Cc: Bhupesh Sharma <bhsharma@xxxxxxxxxx>
> Cc: Nicolai Stange <nicstange@xxxxxxxxx>
> Cc: Naresh Bhat <naresh.bhat@xxxxxxxxxx>
> Cc: Ricardo Neri <ricardo.neri@xxxxxxxxx>
> Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> Cc: Taku Izumi <izumi.taku@xxxxxxxxxxxxxx>
> Cc: Ravi Shankar <ravi.v.shankar@xxxxxxxxx>
> Cc: Matt Fleming <matt@xxxxxxxxxxxxxxxxxxx>
> Cc: Dan Williams <dan.j.williams@xxxxxxxxx>
> Cc: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx>
Do you really think you should cc all these people?
> ---
> drivers/firmware/efi/memmap.c | 28 ++++++++++++++++++++++++++++
> include/linux/efi.h | 8 ++++++++
> 2 files changed, 36 insertions(+)
>
> diff --git a/drivers/firmware/efi/memmap.c b/drivers/firmware/efi/memmap.c
> index 5fc70520e04c..0686e063c644 100644
> --- a/drivers/firmware/efi/memmap.c
> +++ b/drivers/firmware/efi/memmap.c
> @@ -12,6 +12,7 @@
> #include <asm/early_ioremap.h>
> #include <linux/memblock.h>
> #include <linux/slab.h>
> +#include <linux/bootmem.h>
>
> static phys_addr_t __init __efi_memmap_alloc_early(unsigned long size)
> {
> @@ -50,6 +51,33 @@ phys_addr_t __init efi_memmap_alloc(unsigned int num_entries)
> }
>
> /**
> + * efi_memmap_free - Free memory allocated by efi_memmap_alloc()
> + * @mem: Physical address allocated by efi_memmap_alloc().
> + * @num_entries: Number of entries in the allocated map.
> + * @alloc_type: What type of allocation did efi_memmap_alloc() perform?
> + *
> + * Use this function to free memory allocated by efi_memmap_alloc().
> + * efi_memmap_alloc() allocates memory depending on whether mm_init()
> + * has already been invoked or not. It uses either memblock or "normal"
> + * page allocation, similarly, we free it in two different ways. Also
> + * note that there is a third type of memory used by memmap which is
> + * memblock_reserved() and is passed by EFI stub to kernel.
> + */
> +void __init efi_memmap_free(phys_addr_t mem, unsigned int num_entries,
> + enum efi_memmap_type alloc_type)
> +{
> + unsigned long size = num_entries * efi.memmap.desc_size;
> + unsigned int order = get_order(size);
> +
> + if (alloc_type == BUDDY_ALLOCATOR)
> + __free_pages(pfn_to_page(PHYS_PFN(mem)), order);
> + else if (alloc_type == MEMBLOCK)
> + memblock_free(mem, size);
> + else
> + free_bootmem(mem, size);
> +}
> +
> +/**
> * __efi_memmap_init - Common code for mapping the EFI memory map
> * @data: EFI memory map data
> * @late: Use early or late mapping function?
> diff --git a/include/linux/efi.h b/include/linux/efi.h
> index 56add823f190..455875c01ed1 100644
> --- a/include/linux/efi.h
> +++ b/include/linux/efi.h
> @@ -765,6 +765,12 @@ struct efi_memory_map_data {
> unsigned long desc_size;
> };
>
> +enum efi_memmap_type {
> + EFI_STUB,
> + MEMBLOCK,
> + BUDDY_ALLOCATOR,
> +};
> +
> struct efi_memory_map {
> phys_addr_t phys_map;
> void *map;
> @@ -1016,6 +1022,8 @@ extern int __init efi_memmap_split_count(efi_memory_desc_t *md,
> struct range *range);
> extern void __init efi_memmap_insert(struct efi_memory_map *old_memmap,
> void *buf, struct efi_mem_range *mem);
> +extern void __init efi_memmap_free(phys_addr_t mem, unsigned int num_entries,
> + enum efi_memmap_type alloc_type);
>
> extern int efi_config_init(efi_config_table_type_t *arch_tables);
> #ifdef CONFIG_EFI_ESRT
> --
> 2.7.4
>
Sai,
Thanks a lot for respinning all of this.
I am now thinking we should perhaps hide the allocation type from the
callers rather than passing around implementation details of the
allocator, and have explicit alloc/free operations.
Could we instead have a single 'efi_realloc_memmap()' function that
takes care of all of this? We'd still need a free routine for error
handling, I suppose, but it would get rid of the need for obscure
enums or bools that the callers have to pass around.
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
