| Message ID | 20240704073544.670249-47-sughosh.ganu@linaro.org |
|---|---|
| State | RFC |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | Make U-Boot memory reservations coherent | expand |
Hi Sughosh, On Thu, 4 Jul 2024 at 08:38, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > Mark the EFI runtime memory region as reserved memory during board > init so that it does not get allocated by the LMB module on subsequent > memory requests. > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > --- > Changes since V1: New patch > > lib/lmb.c | 41 ++++++++++++++++++++--------------------- > 1 file changed, 20 insertions(+), 21 deletions(-) I see again that this is getting circular. Can you look at what is actually allocated by EFI on init (i.e. before anything is booted)? > > diff --git a/lib/lmb.c b/lib/lmb.c > index 387ec2ac65..6018f1de31 100644 > --- a/lib/lmb.c > +++ b/lib/lmb.c > @@ -19,6 +19,7 @@ > #include <asm/global_data.h> > #include <asm/sections.h> > #include <linux/kernel.h> > +#include <linux/sizes.h> > > DECLARE_GLOBAL_DATA_PTR; > > @@ -212,33 +213,31 @@ void arch_lmb_reserve_generic(ulong sp, ulong end, ulong align) > /** > * efi_lmb_reserve() - add reservations for EFI memory > * > - * Add reservations for all EFI memory areas that are not > - * EFI_CONVENTIONAL_MEMORY. > + * Add reservations for EFI runtime services memory > * > - * Return: 0 on success, 1 on failure > + * Return: None > */ > -static __maybe_unused int efi_lmb_reserve(void) > +static __maybe_unused void efi_lmb_reserve(void) > { > - struct efi_mem_desc *memmap = NULL, *map; > - efi_uintn_t i, map_size = 0; > - efi_status_t ret; > + phys_addr_t runtime_start, runtime_end; > + unsigned long runtime_mask = EFI_PAGE_MASK; > > - ret = efi_get_memory_map_alloc(&map_size, &memmap); > - if (ret != EFI_SUCCESS) > - return 1; > +#if defined(__aarch64__) > + /* > + * Runtime Services must be 64KiB aligned according to the > + * "AArch64 Platforms" section in the UEFI spec (2.7+). > + */ > > - for (i = 0, map = memmap; i < map_size / sizeof(*map); ++map, ++i) { > - if (map->type != EFI_CONVENTIONAL_MEMORY) { > - lmb_reserve_flags(map_to_sysmem((void *)(uintptr_t) > - map->physical_start), > - map->num_pages * EFI_PAGE_SIZE, > - map->type == EFI_RESERVED_MEMORY_TYPE > - ? LMB_NOMAP : LMB_NONE); > - } > - } > - efi_free_pool(memmap); > + runtime_mask = SZ_64K - 1; > +#endif > > - return 0; > + /* Reserve the EFI runtime services memory */ > + runtime_start = (uintptr_t)__efi_runtime_start & ~runtime_mask; > + runtime_end = (uintptr_t)__efi_runtime_stop; > + runtime_end = (runtime_end + runtime_mask) & ~runtime_mask; > + > + lmb_reserve_flags(runtime_start, runtime_end - runtime_start, > + LMB_NOOVERWRITE | LMB_NONOTIFY); > } > > static void lmb_reserve_common(void *fdt_blob) > -- > 2.34.1 > Regards, Simon
hi Simon, On Sat, 13 Jul 2024 at 20:46, Simon Glass <sjg@chromium.org> wrote: > > Hi Sughosh, > > On Thu, 4 Jul 2024 at 08:38, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > > > Mark the EFI runtime memory region as reserved memory during board > > init so that it does not get allocated by the LMB module on subsequent > > memory requests. > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > > --- > > Changes since V1: New patch > > > > lib/lmb.c | 41 ++++++++++++++++++++--------------------- > > 1 file changed, 20 insertions(+), 21 deletions(-) > > I see again that this is getting circular. Can you look at what is > actually allocated by EFI on init (i.e. before anything is booted)? This code is simply ensuring that the EFI runtime regions are marked as reserved by the LMB module to prevent that region from being allocated. Do you have any other way by which this can be communicated to the LMB module ? -sughosh > > > > > diff --git a/lib/lmb.c b/lib/lmb.c > > index 387ec2ac65..6018f1de31 100644 > > --- a/lib/lmb.c > > +++ b/lib/lmb.c > > @@ -19,6 +19,7 @@ > > #include <asm/global_data.h> > > #include <asm/sections.h> > > #include <linux/kernel.h> > > +#include <linux/sizes.h> > > > > DECLARE_GLOBAL_DATA_PTR; > > > > @@ -212,33 +213,31 @@ void arch_lmb_reserve_generic(ulong sp, ulong end, ulong align) > > /** > > * efi_lmb_reserve() - add reservations for EFI memory > > * > > - * Add reservations for all EFI memory areas that are not > > - * EFI_CONVENTIONAL_MEMORY. > > + * Add reservations for EFI runtime services memory > > * > > - * Return: 0 on success, 1 on failure > > + * Return: None > > */ > > -static __maybe_unused int efi_lmb_reserve(void) > > +static __maybe_unused void efi_lmb_reserve(void) > > { > > - struct efi_mem_desc *memmap = NULL, *map; > > - efi_uintn_t i, map_size = 0; > > - efi_status_t ret; > > + phys_addr_t runtime_start, runtime_end; > > + unsigned long runtime_mask = EFI_PAGE_MASK; > > > > - ret = efi_get_memory_map_alloc(&map_size, &memmap); > > - if (ret != EFI_SUCCESS) > > - return 1; > > +#if defined(__aarch64__) > > + /* > > + * Runtime Services must be 64KiB aligned according to the > > + * "AArch64 Platforms" section in the UEFI spec (2.7+). > > + */ > > > > - for (i = 0, map = memmap; i < map_size / sizeof(*map); ++map, ++i) { > > - if (map->type != EFI_CONVENTIONAL_MEMORY) { > > - lmb_reserve_flags(map_to_sysmem((void *)(uintptr_t) > > - map->physical_start), > > - map->num_pages * EFI_PAGE_SIZE, > > - map->type == EFI_RESERVED_MEMORY_TYPE > > - ? LMB_NOMAP : LMB_NONE); > > - } > > - } > > - efi_free_pool(memmap); > > + runtime_mask = SZ_64K - 1; > > +#endif > > > > - return 0; > > + /* Reserve the EFI runtime services memory */ > > + runtime_start = (uintptr_t)__efi_runtime_start & ~runtime_mask; > > + runtime_end = (uintptr_t)__efi_runtime_stop; > > + runtime_end = (runtime_end + runtime_mask) & ~runtime_mask; > > + > > + lmb_reserve_flags(runtime_start, runtime_end - runtime_start, > > + LMB_NOOVERWRITE | LMB_NONOTIFY); > > } > > > > static void lmb_reserve_common(void *fdt_blob) > > -- > > 2.34.1 > > > > Regards, > Simon
Hi Sughosh, On Mon, 15 Jul 2024 at 10:42, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > hi Simon, > > On Sat, 13 Jul 2024 at 20:46, Simon Glass <sjg@chromium.org> wrote: > > > > Hi Sughosh, > > > > On Thu, 4 Jul 2024 at 08:38, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > > > > > Mark the EFI runtime memory region as reserved memory during board > > > init so that it does not get allocated by the LMB module on subsequent > > > memory requests. > > > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > > > --- > > > Changes since V1: New patch > > > > > > lib/lmb.c | 41 ++++++++++++++++++++--------------------- > > > 1 file changed, 20 insertions(+), 21 deletions(-) > > > > I see again that this is getting circular. Can you look at what is > > actually allocated by EFI on init (i.e. before anything is booted)? > > This code is simply ensuring that the EFI runtime regions are marked > as reserved by the LMB module to prevent that region from being > allocated. Do you have any other way by which this can be communicated > to the LMB module ? But this region is within the U-Boot code area, so it should be covered by the normal exclusion of that area? Regards, Simon
hi Simon, On Mon, 15 Jul 2024 at 17:09, Simon Glass <sjg@chromium.org> wrote: > > Hi Sughosh, > > On Mon, 15 Jul 2024 at 10:42, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > > > hi Simon, > > > > On Sat, 13 Jul 2024 at 20:46, Simon Glass <sjg@chromium.org> wrote: > > > > > > Hi Sughosh, > > > > > > On Thu, 4 Jul 2024 at 08:38, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > > > > > > > Mark the EFI runtime memory region as reserved memory during board > > > > init so that it does not get allocated by the LMB module on subsequent > > > > memory requests. > > > > > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > > > > --- > > > > Changes since V1: New patch > > > > > > > > lib/lmb.c | 41 ++++++++++++++++++++--------------------- > > > > 1 file changed, 20 insertions(+), 21 deletions(-) > > > > > > I see again that this is getting circular. Can you look at what is > > > actually allocated by EFI on init (i.e. before anything is booted)? > > > > This code is simply ensuring that the EFI runtime regions are marked > > as reserved by the LMB module to prevent that region from being > > allocated. Do you have any other way by which this can be communicated > > to the LMB module ? > > But this region is within the U-Boot code area, so it should be > covered by the normal exclusion of that area? Yes that is the case for all arch's today. Let me check if I can simply drop this function then. -sughosh > > Regards, > Simon
diff --git a/lib/lmb.c b/lib/lmb.c index 387ec2ac65..6018f1de31 100644 --- a/lib/lmb.c +++ b/lib/lmb.c @@ -19,6 +19,7 @@ #include <asm/global_data.h> #include <asm/sections.h> #include <linux/kernel.h> +#include <linux/sizes.h> DECLARE_GLOBAL_DATA_PTR; @@ -212,33 +213,31 @@ void arch_lmb_reserve_generic(ulong sp, ulong end, ulong align) /** * efi_lmb_reserve() - add reservations for EFI memory * - * Add reservations for all EFI memory areas that are not - * EFI_CONVENTIONAL_MEMORY. + * Add reservations for EFI runtime services memory * - * Return: 0 on success, 1 on failure + * Return: None */ -static __maybe_unused int efi_lmb_reserve(void) +static __maybe_unused void efi_lmb_reserve(void) { - struct efi_mem_desc *memmap = NULL, *map; - efi_uintn_t i, map_size = 0; - efi_status_t ret; + phys_addr_t runtime_start, runtime_end; + unsigned long runtime_mask = EFI_PAGE_MASK; - ret = efi_get_memory_map_alloc(&map_size, &memmap); - if (ret != EFI_SUCCESS) - return 1; +#if defined(__aarch64__) + /* + * Runtime Services must be 64KiB aligned according to the + * "AArch64 Platforms" section in the UEFI spec (2.7+). + */ - for (i = 0, map = memmap; i < map_size / sizeof(*map); ++map, ++i) { - if (map->type != EFI_CONVENTIONAL_MEMORY) { - lmb_reserve_flags(map_to_sysmem((void *)(uintptr_t) - map->physical_start), - map->num_pages * EFI_PAGE_SIZE, - map->type == EFI_RESERVED_MEMORY_TYPE - ? LMB_NOMAP : LMB_NONE); - } - } - efi_free_pool(memmap); + runtime_mask = SZ_64K - 1; +#endif - return 0; + /* Reserve the EFI runtime services memory */ + runtime_start = (uintptr_t)__efi_runtime_start & ~runtime_mask; + runtime_end = (uintptr_t)__efi_runtime_stop; + runtime_end = (runtime_end + runtime_mask) & ~runtime_mask; + + lmb_reserve_flags(runtime_start, runtime_end - runtime_start, + LMB_NOOVERWRITE | LMB_NONOTIFY); } static void lmb_reserve_common(void *fdt_blob)
Mark the EFI runtime memory region as reserved memory during board init so that it does not get allocated by the LMB module on subsequent memory requests. Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> --- Changes since V1: New patch lib/lmb.c | 41 ++++++++++++++++++++--------------------- 1 file changed, 20 insertions(+), 21 deletions(-)