diff mbox series

[RFC,v2,46/48] lmb: mark the EFI runtime memory regions as reserved

Message ID 20240704073544.670249-47-sughosh.ganu@linaro.org
State New
Headers show
Series Make U-Boot memory reservations coherent | expand

Commit Message

Sughosh Ganu July 4, 2024, 7:35 a.m. UTC
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(-)

Comments

Simon Glass July 13, 2024, 3:16 p.m. UTC | #1
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
Sughosh Ganu July 15, 2024, 9:41 a.m. UTC | #2
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
Simon Glass July 15, 2024, 11:39 a.m. UTC | #3
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
Sughosh Ganu July 16, 2024, 6:31 a.m. UTC | #4
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 mbox series

Patch

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)