On Thu, 7 May 2020 at 15:37, Lenny Szubowicz <lszubowi@xxxxxxxxxx> wrote: > > In allocate_e820(), call the EFI get_memory_map() service directly > instead of indirectly via efi_get_memory_map(). This avoids allocation > of a buffer and return of the full EFI memory map, which is not needed > here and would otherwise need to be freed. > > Routine allocate_e820() only needs to know how many EFI memory > descriptors there are in the map to allocate an adequately sized > e820ext buffer, if it's needed. Note that since efi_get_memory_map() > returns a memory map buffer sized with extra headroom, allocate_e820() > now needs to explicitly factor that into the e820ext size calculation. > > Signed-off-by: Lenny Szubowicz <lszubowi@xxxxxxxxxx> > Suggested-by: Ard Biesheuvel <ardb@xxxxxxxxxx> > -- > v2: > - Instead of freeing the EFI memory map buffer allocated by > efi_get_memory_map(), avoid the allocation in the first place. > > - Changed the title of the patch because the v1 title no longer > applies. v1 ref: > https://lore.kernel.org/lkml/20200505190016.4350-1-lszubowi@xxxxxxxxxx/ > -- > --- > drivers/firmware/efi/libstub/efistub.h | 2 ++ > drivers/firmware/efi/libstub/mem.c | 5 +++++ > drivers/firmware/efi/libstub/x86-stub.c | 22 ++++++++-------------- > 3 files changed, 15 insertions(+), 14 deletions(-) > > diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h > index 67d26949fd26..191468b771b6 100644 > --- a/drivers/firmware/efi/libstub/efistub.h > +++ b/drivers/firmware/efi/libstub/efistub.h > @@ -616,6 +616,8 @@ void efi_printk(char *str); > > void efi_free(unsigned long size, unsigned long addr); > > +unsigned int efi_get_mmap_nr_slack_slots(void); > + > char *efi_convert_cmdline(efi_loaded_image_t *image, int *cmd_line_len, > unsigned long max_addr); > > diff --git a/drivers/firmware/efi/libstub/mem.c b/drivers/firmware/efi/libstub/mem.c > index 869a79c8946f..fc8b4fb08505 100644 > --- a/drivers/firmware/efi/libstub/mem.c > +++ b/drivers/firmware/efi/libstub/mem.c > @@ -7,6 +7,11 @@ > > #define EFI_MMAP_NR_SLACK_SLOTS 8 > > +unsigned int efi_get_mmap_nr_slack_slots(void) > +{ > + return EFI_MMAP_NR_SLACK_SLOTS; > +} > + Can we just move this #define to efistub.h and use it directly? > static inline bool mmap_has_headroom(unsigned long buff_size, > unsigned long map_size, > unsigned long desc_size) > diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c > index 05ccb229fb45..9a5ee0678434 100644 > --- a/drivers/firmware/efi/libstub/x86-stub.c > +++ b/drivers/firmware/efi/libstub/x86-stub.c > @@ -606,24 +606,18 @@ static efi_status_t allocate_e820(struct boot_params *params, > struct setup_data **e820ext, > u32 *e820ext_size) > { > - unsigned long map_size, desc_size, buff_size; > - struct efi_boot_memmap boot_map; > - efi_memory_desc_t *map; > + unsigned long map_size, desc_size; > efi_status_t status; > __u32 nr_desc; > > - boot_map.map = ↦ > - boot_map.map_size = &map_size; > - boot_map.desc_size = &desc_size; > - boot_map.desc_ver = NULL; > - boot_map.key_ptr = NULL; > - boot_map.buff_size = &buff_size; > + /* Only need the size of the mem map and size of each mem descriptor */ > + map_size = 0; > + status = efi_bs_call(get_memory_map, &map_size, NULL, NULL, > + &desc_size, NULL); > + if (status != EFI_BUFFER_TOO_SMALL) > + return (status != EFI_SUCCESS) ? status : EFI_UNSUPPORTED; > > - status = efi_get_memory_map(&boot_map); > - if (status != EFI_SUCCESS) > - return status; > - > - nr_desc = buff_size / desc_size; > + nr_desc = map_size / desc_size + efi_get_mmap_nr_slack_slots(); > > if (nr_desc > ARRAY_SIZE(params->e820_table)) { > u32 nr_e820ext = nr_desc - ARRAY_SIZE(params->e820_table); > -- > 2.18.4 >
