| Message ID | 20241008181435.1753814-13-sughosh.ganu@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Make EFI memory allocations synchronous with LMB | expand |
Hi Sughosh, On Tue, 8 Oct 2024 at 21:15, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > The EFI_CONVENTIONAL_MEMORY type, which is the usable RAM memory is > now being managed by the LMB module. Remove the addition of this > memory type to the EFI memory map. This memory now gets added to the > EFI memory map as part of the LMB memory map update event handler. > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > Reviewed-by: Simon Glass <sjg@chromium.org> > --- > Changes since V1: None > > include/efi_loader.h | 13 ++++--- > lib/efi_loader/efi_memory.c | 75 +++---------------------------------- > 2 files changed, 12 insertions(+), 76 deletions(-) > > diff --git a/include/efi_loader.h b/include/efi_loader.h > index 081cb65d3f7..0d5555c7597 100644 > --- a/include/efi_loader.h > +++ b/include/efi_loader.h > @@ -798,10 +798,6 @@ efi_status_t efi_add_memory_map_pg(u64 start, u64 pages, > int memory_type, > bool overlap_only_ram); > > -/* Adds a conventional range into the EFI memory map */ > -efi_status_t efi_add_conventional_memory_map(u64 ram_start, u64 ram_end, > - u64 ram_top); > - > /* Called by board init to initialize the EFI drivers */ > efi_status_t efi_driver_init(void); > /* Called when a block device is added */ > @@ -1186,9 +1182,14 @@ efi_status_t efi_console_get_u16_string > efi_status_t efi_disk_get_device_name(const efi_handle_t handle, char *buf, int size); > > /** > - * efi_add_known_memory() - add memory banks to EFI memory map > + * efi_add_known_memory() - add memory types to the EFI memory map > + * > + * This function is to be used to adding different memory types other /adding/add > + * than EFI_CONVENTIONAL_MEMORY to the EFI memory map. The conventional > + * memory is handled by the LMB module, and gets added to the memory no comma after module > + * map through the LMB module. > * > - * This weak function may be overridden for specific architectures. > + * This function may be overridden for specific architectures. Since you are rewriting this I think for "architecture specific purposes" is better > */ > void efi_add_known_memory(void); > > diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c > index 974ecc51113..0f25094dbf8 100644 > --- a/lib/efi_loader/efi_memory.c > +++ b/lib/efi_loader/efi_memory.c > @@ -780,82 +780,17 @@ efi_status_t efi_get_memory_map_alloc(efi_uintn_t *map_size, > } > > /** > - * efi_add_conventional_memory_map() - add a RAM memory area to the map > + * efi_add_known_memory() - add memory types to the EFI memory map > * > - * @ram_start: start address of a RAM memory area > - * @ram_end: end address of a RAM memory area > - * @ram_top: max address to be used as conventional memory > - * Return: status code > - */ > -efi_status_t efi_add_conventional_memory_map(u64 ram_start, u64 ram_end, > - u64 ram_top) > -{ > - u64 pages; > - > - /* Remove partial pages */ > - ram_end &= ~EFI_PAGE_MASK; > - ram_start = (ram_start + EFI_PAGE_MASK) & ~EFI_PAGE_MASK; > - > - if (ram_end <= ram_start) { > - /* Invalid mapping */ > - return EFI_INVALID_PARAMETER; > - } > - > - pages = (ram_end - ram_start) >> EFI_PAGE_SHIFT; > - > - efi_add_memory_map_pg(ram_start, pages, > - EFI_CONVENTIONAL_MEMORY, false); > - > - /* > - * Boards may indicate to the U-Boot memory core that they > - * can not support memory above ram_top. Let's honor this > - * in the efi_loader subsystem too by declaring any memory > - * above ram_top as "already occupied by firmware". > - */ > - if (ram_top < ram_start) { > - /* ram_top is before this region, reserve all */ > - efi_add_memory_map_pg(ram_start, pages, > - EFI_BOOT_SERVICES_DATA, true); > - } else if (ram_top < ram_end) { > - /* ram_top is inside this region, reserve parts */ > - pages = (ram_end - ram_top) >> EFI_PAGE_SHIFT; > - > - efi_add_memory_map_pg(ram_top, pages, > - EFI_BOOT_SERVICES_DATA, true); > - } > - > - return EFI_SUCCESS; > -} > - > -/** > - * efi_add_known_memory() - add memory banks to map > + * This function is to be used to adding different memory types other > + * than EFI_CONVENTIONAL_MEMORY to the EFI memory map. The conventional > + * memory is handled by the LMB module, and gets added to the memory > + * map through the LMB module. > * > * This function may be overridden for specific architectures. > */ > __weak void efi_add_known_memory(void) We can also get rid of this in the future. Any idea if we have platforms using it after the last 2 you removed? > { > - u64 ram_top = gd->ram_top & ~EFI_PAGE_MASK; > - int i; > - > - /* > - * ram_top is just outside mapped memory. So use an offset of one for > - * mapping the sandbox address. > - */ > - ram_top = (uintptr_t)map_sysmem(ram_top - 1, 0) + 1; > - > - /* Fix for 32bit targets with ram_top at 4G */ > - if (!ram_top) > - ram_top = 0x100000000ULL; > - > - /* Add RAM */ > - for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) { > - u64 ram_end, ram_start; > - > - ram_start = (uintptr_t)map_sysmem(gd->bd->bi_dram[i].start, 0); > - ram_end = ram_start + gd->bd->bi_dram[i].size; > - > - efi_add_conventional_memory_map(ram_start, ram_end, ram_top); > - } > } > > /** > -- > 2.34.1 > With the spelling nits fixed Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
On Fri, 11 Oct 2024 at 16:33, Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote: > > Hi Sughosh, > > On Tue, 8 Oct 2024 at 21:15, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > > > The EFI_CONVENTIONAL_MEMORY type, which is the usable RAM memory is > > now being managed by the LMB module. Remove the addition of this > > memory type to the EFI memory map. This memory now gets added to the > > EFI memory map as part of the LMB memory map update event handler. > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > > Reviewed-by: Simon Glass <sjg@chromium.org> > > --- > > Changes since V1: None > > > > include/efi_loader.h | 13 ++++--- > > lib/efi_loader/efi_memory.c | 75 +++---------------------------------- > > 2 files changed, 12 insertions(+), 76 deletions(-) > > > > diff --git a/include/efi_loader.h b/include/efi_loader.h > > index 081cb65d3f7..0d5555c7597 100644 > > --- a/include/efi_loader.h > > +++ b/include/efi_loader.h > > @@ -798,10 +798,6 @@ efi_status_t efi_add_memory_map_pg(u64 start, u64 pages, > > int memory_type, > > bool overlap_only_ram); > > > > -/* Adds a conventional range into the EFI memory map */ > > -efi_status_t efi_add_conventional_memory_map(u64 ram_start, u64 ram_end, > > - u64 ram_top); > > - > > /* Called by board init to initialize the EFI drivers */ > > efi_status_t efi_driver_init(void); > > /* Called when a block device is added */ > > @@ -1186,9 +1182,14 @@ efi_status_t efi_console_get_u16_string > > efi_status_t efi_disk_get_device_name(const efi_handle_t handle, char *buf, int size); > > > > /** > > - * efi_add_known_memory() - add memory banks to EFI memory map > > + * efi_add_known_memory() - add memory types to the EFI memory map > > + * > > + * This function is to be used to adding different memory types other > > /adding/add Okay > > > + * than EFI_CONVENTIONAL_MEMORY to the EFI memory map. The conventional > > + * memory is handled by the LMB module, and gets added to the memory > > no comma after module Okay > > > + * map through the LMB module. > > * > > - * This weak function may be overridden for specific architectures. > > + * This function may be overridden for specific architectures. > > Since you are rewriting this I think for "architecture specific > purposes" is better Okay > > > */ > > void efi_add_known_memory(void); > > > > diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c > > index 974ecc51113..0f25094dbf8 100644 > > --- a/lib/efi_loader/efi_memory.c > > +++ b/lib/efi_loader/efi_memory.c > > @@ -780,82 +780,17 @@ efi_status_t efi_get_memory_map_alloc(efi_uintn_t *map_size, > > } > > > > /** > > - * efi_add_conventional_memory_map() - add a RAM memory area to the map > > + * efi_add_known_memory() - add memory types to the EFI memory map > > * > > - * @ram_start: start address of a RAM memory area > > - * @ram_end: end address of a RAM memory area > > - * @ram_top: max address to be used as conventional memory > > - * Return: status code > > - */ > > -efi_status_t efi_add_conventional_memory_map(u64 ram_start, u64 ram_end, > > - u64 ram_top) > > -{ > > - u64 pages; > > - > > - /* Remove partial pages */ > > - ram_end &= ~EFI_PAGE_MASK; > > - ram_start = (ram_start + EFI_PAGE_MASK) & ~EFI_PAGE_MASK; > > - > > - if (ram_end <= ram_start) { > > - /* Invalid mapping */ > > - return EFI_INVALID_PARAMETER; > > - } > > - > > - pages = (ram_end - ram_start) >> EFI_PAGE_SHIFT; > > - > > - efi_add_memory_map_pg(ram_start, pages, > > - EFI_CONVENTIONAL_MEMORY, false); > > - > > - /* > > - * Boards may indicate to the U-Boot memory core that they > > - * can not support memory above ram_top. Let's honor this > > - * in the efi_loader subsystem too by declaring any memory > > - * above ram_top as "already occupied by firmware". > > - */ > > - if (ram_top < ram_start) { > > - /* ram_top is before this region, reserve all */ > > - efi_add_memory_map_pg(ram_start, pages, > > - EFI_BOOT_SERVICES_DATA, true); > > - } else if (ram_top < ram_end) { > > - /* ram_top is inside this region, reserve parts */ > > - pages = (ram_end - ram_top) >> EFI_PAGE_SHIFT; > > - > > - efi_add_memory_map_pg(ram_top, pages, > > - EFI_BOOT_SERVICES_DATA, true); > > - } > > - > > - return EFI_SUCCESS; > > -} > > - > > -/** > > - * efi_add_known_memory() - add memory banks to map > > + * This function is to be used to adding different memory types other > > + * than EFI_CONVENTIONAL_MEMORY to the EFI memory map. The conventional > > + * memory is handled by the LMB module, and gets added to the memory > > + * map through the LMB module. > > * > > * This function may be overridden for specific architectures. > > */ > > __weak void efi_add_known_memory(void) > > We can also get rid of this in the future. Any idea if we have > platforms using it after the last 2 you removed? Yes, this is being used by some x86 platform, which is adding other memory types to the EFI memory map. -sughosh > > > > { > > - u64 ram_top = gd->ram_top & ~EFI_PAGE_MASK; > > - int i; > > - > > - /* > > - * ram_top is just outside mapped memory. So use an offset of one for > > - * mapping the sandbox address. > > - */ > > - ram_top = (uintptr_t)map_sysmem(ram_top - 1, 0) + 1; > > - > > - /* Fix for 32bit targets with ram_top at 4G */ > > - if (!ram_top) > > - ram_top = 0x100000000ULL; > > - > > - /* Add RAM */ > > - for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) { > > - u64 ram_end, ram_start; > > - > > - ram_start = (uintptr_t)map_sysmem(gd->bd->bi_dram[i].start, 0); > > - ram_end = ram_start + gd->bd->bi_dram[i].size; > > - > > - efi_add_conventional_memory_map(ram_start, ram_end, ram_top); > > - } > > } > > > > /** > > -- > > 2.34.1 > > > > With the spelling nits fixed > Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
diff --git a/include/efi_loader.h b/include/efi_loader.h index 081cb65d3f7..0d5555c7597 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -798,10 +798,6 @@ efi_status_t efi_add_memory_map_pg(u64 start, u64 pages, int memory_type, bool overlap_only_ram); -/* Adds a conventional range into the EFI memory map */ -efi_status_t efi_add_conventional_memory_map(u64 ram_start, u64 ram_end, - u64 ram_top); - /* Called by board init to initialize the EFI drivers */ efi_status_t efi_driver_init(void); /* Called when a block device is added */ @@ -1186,9 +1182,14 @@ efi_status_t efi_console_get_u16_string efi_status_t efi_disk_get_device_name(const efi_handle_t handle, char *buf, int size); /** - * efi_add_known_memory() - add memory banks to EFI memory map + * efi_add_known_memory() - add memory types to the EFI memory map + * + * This function is to be used to adding different memory types other + * than EFI_CONVENTIONAL_MEMORY to the EFI memory map. The conventional + * memory is handled by the LMB module, and gets added to the memory + * map through the LMB module. * - * This weak function may be overridden for specific architectures. + * This function may be overridden for specific architectures. */ void efi_add_known_memory(void); diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c index 974ecc51113..0f25094dbf8 100644 --- a/lib/efi_loader/efi_memory.c +++ b/lib/efi_loader/efi_memory.c @@ -780,82 +780,17 @@ efi_status_t efi_get_memory_map_alloc(efi_uintn_t *map_size, } /** - * efi_add_conventional_memory_map() - add a RAM memory area to the map + * efi_add_known_memory() - add memory types to the EFI memory map * - * @ram_start: start address of a RAM memory area - * @ram_end: end address of a RAM memory area - * @ram_top: max address to be used as conventional memory - * Return: status code - */ -efi_status_t efi_add_conventional_memory_map(u64 ram_start, u64 ram_end, - u64 ram_top) -{ - u64 pages; - - /* Remove partial pages */ - ram_end &= ~EFI_PAGE_MASK; - ram_start = (ram_start + EFI_PAGE_MASK) & ~EFI_PAGE_MASK; - - if (ram_end <= ram_start) { - /* Invalid mapping */ - return EFI_INVALID_PARAMETER; - } - - pages = (ram_end - ram_start) >> EFI_PAGE_SHIFT; - - efi_add_memory_map_pg(ram_start, pages, - EFI_CONVENTIONAL_MEMORY, false); - - /* - * Boards may indicate to the U-Boot memory core that they - * can not support memory above ram_top. Let's honor this - * in the efi_loader subsystem too by declaring any memory - * above ram_top as "already occupied by firmware". - */ - if (ram_top < ram_start) { - /* ram_top is before this region, reserve all */ - efi_add_memory_map_pg(ram_start, pages, - EFI_BOOT_SERVICES_DATA, true); - } else if (ram_top < ram_end) { - /* ram_top is inside this region, reserve parts */ - pages = (ram_end - ram_top) >> EFI_PAGE_SHIFT; - - efi_add_memory_map_pg(ram_top, pages, - EFI_BOOT_SERVICES_DATA, true); - } - - return EFI_SUCCESS; -} - -/** - * efi_add_known_memory() - add memory banks to map + * This function is to be used to adding different memory types other + * than EFI_CONVENTIONAL_MEMORY to the EFI memory map. The conventional + * memory is handled by the LMB module, and gets added to the memory + * map through the LMB module. * * This function may be overridden for specific architectures. */ __weak void efi_add_known_memory(void) { - u64 ram_top = gd->ram_top & ~EFI_PAGE_MASK; - int i; - - /* - * ram_top is just outside mapped memory. So use an offset of one for - * mapping the sandbox address. - */ - ram_top = (uintptr_t)map_sysmem(ram_top - 1, 0) + 1; - - /* Fix for 32bit targets with ram_top at 4G */ - if (!ram_top) - ram_top = 0x100000000ULL; - - /* Add RAM */ - for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) { - u64 ram_end, ram_start; - - ram_start = (uintptr_t)map_sysmem(gd->bd->bi_dram[i].start, 0); - ram_end = ram_start + gd->bd->bi_dram[i].size; - - efi_add_conventional_memory_map(ram_start, ram_end, ram_top); - } } /**