Thanks Sughosh

+CC Anton who tested that

On Fri, 25 Oct 2024 at 14:14, Sughosh Ganu sughosh.ganu@linaro.org wrote:

From: Masahisa Kojima kojima.masahisa@socionext.com

One of the problems OS installers face, when running in EFI, is that the mounted ISO after calling ExitBootServices goes away. For some distros this is a problem since they rely on finding some core packages before continuing the installation. Distros have works around this -- e.g Fedora has a special kernel command line parameter called inst.stage2 [0].

ACPI has NFIT and NVDIMM support to provide ramdisks to the OS, but we don't have anything in place for DTs. Linux and device trees have support for persistent memory devices. So add a function that can inject a pmem node in a DT, so we can use it when launhing OS installers with EFI.

[0] https://docs.redhat.com/en/documentation/red_hat_enterprise_linux/7/html/ins...

Signed-off-by: Masahisa Kojima kojima.masahisa@socionext.com Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org

---

boot/fdt_support.c | 41 +++++++++++++++++++++++++++++++++++++++-- include/fdt_support.h | 13 +++++++++++++ 2 files changed, 52 insertions(+), 2 deletions(-)

diff --git a/boot/fdt_support.c b/boot/fdt_support.c index 2392027d40..61f725389b 100644 --- a/boot/fdt_support.c +++ b/boot/fdt_support.c @@ -18,6 +18,7 @@ #include <dm/ofnode.h> #include <linux/ctype.h> #include <linux/types.h> +#include <linux/sizes.h> #include <asm/global_data.h> #include <asm/unaligned.h> #include <linux/libfdt.h> @@ -463,7 +464,6 @@ void do_fixup_by_compat_u32(void *fdt, const char *compat, do_fixup_by_compat(fdt, compat, prop, &tmp, 4, create); }

-#ifdef CONFIG_ARCH_FIXUP_FDT_MEMORY /*

• fdt_pack_reg - pack address and size array into the "reg"-suitable stream

*/ @@ -491,7 +491,7 @@ static int fdt_pack_reg(const void *fdt, void *buf, u64 *address, u64 *size,

    return p - (char *)buf;

}

+#ifdef CONFIG_ARCH_FIXUP_FDT_MEMORY #if CONFIG_NR_DRAM_BANKS > 4 #define MEMORY_BANKS_MAX CONFIG_NR_DRAM_BANKS #else @@ -2221,3 +2221,40 @@ int fdt_valid(struct fdt_header **blobp) } return 1; }

+int fdt_fixup_pmem_region(void *blob, ulong addr, u32 size) +{

•   u64 pmem_start[2] = { 0 };
  

•   u64 pmem_size[2] = { 0 };
  

•   char pmem_node[32] = {0};
  

•   int nodeoffset, len;
  

•   int err;
  

•   u8 tmp[4 * 16]; /* Up to 64-bit address + 64-bit size */
  

•   if (!IS_ALIGNED(addr, SZ_2M) || !IS_ALIGNED(addr + size, SZ_2M)) {
  

•           printf("Start and end address needs at 2MB alignment\n");
  

•           return -1;
  

•   }
  

•   snprintf(pmem_node, sizeof(pmem_node), "pmem@%lx", addr);
  

•   nodeoffset = fdt_find_or_add_subnode(blob, 0, pmem_node);
  

•   if (nodeoffset < 0)
  

•           return nodeoffset;
  

•   err = fdt_setprop_string(blob, nodeoffset, "compatible", "pmem-region");
  

•   if (err)
  

•           return err;
  

•   err = fdt_setprop_empty(blob, nodeoffset, "volatile");
  

•   if (err)
  

•           return err;
  

•   pmem_start[0] = addr;
  

•   pmem_size[0] = size;
  

•   len = fdt_pack_reg(blob, tmp, pmem_start, pmem_size, 1);
  

•   err = fdt_setprop(blob, nodeoffset, "reg", tmp, len);
  

•   if (err < 0) {
  

•           printf("WARNING: could not set pmem %s %s.\n", "reg",
  

•                  fdt_strerror(err));
  

•           return err;
  

•   }
  

•   return 0;
  

+} diff --git a/include/fdt_support.h b/include/fdt_support.h index 741e2360c2..63ece0de32 100644 --- a/include/fdt_support.h +++ b/include/fdt_support.h @@ -473,4 +473,17 @@ void fdt_fixup_pstore(void *blob); */ int fdt_kaslrseed(void *blob, bool overwrite);

+/**

• • fdt_fixup_pmem_region() - add a pmem node on the device tree
• • This functions injects a pmem node to the device tree. Usually
• • used with EFI installers to preserve installer images
• • @blob: device tree provided by caller
• • @addr: start address of the pmem node
• • @size: size of the memory of the pmem node
• • Return: 0 on success or < 0 on failure
• */

+int fdt_fixup_pmem_region(void *blob, ulong addr, u32 size);

#endif /* ifndef __FDT_SUPPORT_H */

2.34.1

+CC Anton

On Fri, 25 Oct 2024 at 14:14, Sughosh Ganu sughosh.ganu@linaro.org wrote:

From: Ilias Apalodimas ilias.apalodimas@linaro.org

With upcoming changes supporting pmem nodes, we need to ommit the pmem area rfom the EFI memory map. Add a function to do that

Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org

---

include/efi_loader.h | 11 +++++--- lib/efi_loader/efi_memory.c | 51 +++++++++++++++++++++++++++---------- lib/lmb.c | 4 +-- 3 files changed, 47 insertions(+), 19 deletions(-)

diff --git a/include/efi_loader.h b/include/efi_loader.h index 291eca5c07..d450e304c6 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -786,7 +786,7 @@ efi_status_t efi_get_memory_map(efi_uintn_t *memory_map_size, efi_status_t efi_add_memory_map(u64 start, u64 size, int memory_type);

/**

• • efi_add_memory_map_pg() - add pages to the memory map

• • efi_update_memory_map() - update the memory map by adding/removing pages
  • @start: start address, must be a multiple of

  •                         EFI_PAGE_SIZE
    

@@ -794,11 +794,14 @@ efi_status_t efi_add_memory_map(u64 start, u64 size, int memory_type);

• @memory_type: type of memory added
• @overlap_conventional: region may only overlap free(conventional)

•                         memory
  

• • @remove: remove memory map
  • Return: status code
  */

-efi_status_t efi_add_memory_map_pg(u64 start, u64 pages,

•                              int memory_type,
  

•                              bool overlap_conventional);
  

+efi_status_t efi_update_memory_map(u64 start, u64 pages, int memory_type,

•                              bool overlap_conventional, bool remove);
  

+/* Remove memory from the EFI memory map */ +efi_status_t efi_remove_memory_map(u64 start, u64 size, int memory_type);

/* Called by board init to initialize the EFI drivers */ efi_status_t efi_driver_init(void); diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c index 3d742fa191..cb93bfa55f 100644 --- a/lib/efi_loader/efi_memory.c +++ b/lib/efi_loader/efi_memory.c @@ -258,7 +258,7 @@ static s64 efi_mem_carve_out(struct efi_mem_list *map, }

/**

• • efi_add_memory_map_pg() - add pages to the memory map

• • efi_update_memory_map() - update the memory map by adding/removing pages
  • @start: start address, must be a multiple of

  •                         EFI_PAGE_SIZE
    

@@ -266,11 +266,11 @@ static s64 efi_mem_carve_out(struct efi_mem_list *map,

• @memory_type: type of memory added
• @overlap_conventional: region may only overlap free(conventional)

•                         memory
  

• • @remove: remove memory map
  • Return: status code
  */

-efi_status_t efi_add_memory_map_pg(u64 start, u64 pages,

•                              int memory_type,
  

•                              bool overlap_conventional)
  

+efi_status_t efi_update_memory_map(u64 start, u64 pages, int memory_type,

•                              bool overlap_conventional, bool remove)
  

{ struct efi_mem_list *lmem; struct efi_mem_list *newlist; @@ -278,9 +278,9 @@ efi_status_t efi_add_memory_map_pg(u64 start, u64 pages, uint64_t carved_pages = 0; struct efi_event *evt;

•   EFI_PRINT("%s: 0x%llx 0x%llx %d %s\n", __func__,
  

•   EFI_PRINT("%s: 0x%llx 0x%llx %d %s %s\n", __func__,
              start, pages, memory_type, overlap_conventional ?
  

•             "yes" : "no");
  

•             "yes" : "no", remove ? "remove" : "add");
  
    if (memory_type >= EFI_MAX_MEMORY_TYPE)
            return EFI_INVALID_PARAMETER;
  

@@ -363,7 +363,10 @@ efi_status_t efi_add_memory_map_pg(u64 start, u64 pages, }

    /* Add our new map */

•    list_add_tail(&newlist->link, &efi_mem);
  

•   if (!remove)
  

•           list_add_tail(&newlist->link, &efi_mem);
  

•   else
  

•           free(newlist);
  
    /* And make sure memory is listed in descending order */
    efi_mem_sort();
  

@@ -400,7 +403,29 @@ efi_status_t efi_add_memory_map(u64 start, u64 size, int memory_type) pages = efi_size_in_pages(size + (start & EFI_PAGE_MASK)); start &= ~EFI_PAGE_MASK;

•   return efi_add_memory_map_pg(start, pages, memory_type, false);
  

•   return efi_update_memory_map(start, pages, memory_type, false, false);
  

+}

+/**

• • efi_remove_memory_map() - remove memory area to the memory map
• • @start: start address of the memory area
• • @size: length in bytes of the memory area
• • @memory_type: type of memory removed
• • Return: status code
• • This function automatically aligns the start and size of the memory area
• • to EFI_PAGE_SIZE.
• */

+efi_status_t efi_remove_memory_map(u64 start, u64 size, int memory_type) +{

•   u64 pages;
  

•   pages = efi_size_in_pages(size + (start & EFI_PAGE_MASK));
  

•   start &= ~EFI_PAGE_MASK;
  

•   return efi_update_memory_map(start, pages, memory_type, false, true);
  

}

/** @@ -500,7 +525,7 @@ efi_status_t efi_allocate_pages(enum efi_allocate_type type,

    addr = (u64)(uintptr_t)map_sysmem(addr, 0);
    /* Reserve that map in our memory maps */

•   ret = efi_add_memory_map_pg(addr, pages, memory_type, true);
  

•   ret = efi_update_memory_map(addr, pages, memory_type, true, false);
    if (ret != EFI_SUCCESS)
            /* Map would overlap, bail out */
            return  EFI_OUT_OF_RESOURCES;
  

@@ -542,8 +567,8 @@ efi_status_t efi_free_pages(uint64_t memory, efi_uintn_t pages) if (status) return EFI_NOT_FOUND;

•   ret = efi_add_memory_map_pg(memory, pages, EFI_CONVENTIONAL_MEMORY,
  

•                               false);
  

•   ret = efi_update_memory_map(memory, pages, EFI_CONVENTIONAL_MEMORY,
  

•                               false, false);
    if (ret != EFI_SUCCESS)
            return EFI_NOT_FOUND;
  
  

@@ -828,8 +853,8 @@ static void add_u_boot_and_runtime(void) runtime_end = (uintptr_t)__efi_runtime_stop; runtime_end = (runtime_end + runtime_mask) & ~runtime_mask; runtime_pages = (runtime_end - runtime_start) >> EFI_PAGE_SHIFT;

•   efi_add_memory_map_pg(runtime_start, runtime_pages,
  

•                         EFI_RUNTIME_SERVICES_CODE, false);
  

•   efi_update_memory_map(runtime_start, runtime_pages,
  

•                         EFI_RUNTIME_SERVICES_CODE, false, false);
  

}

int efi_memory_init(void) diff --git a/lib/lmb.c b/lib/lmb.c index 7e90f17876..05f3cd093d 100644 --- a/lib/lmb.c +++ b/lib/lmb.c @@ -55,11 +55,11 @@ static int __maybe_unused lmb_map_update_notify(phys_addr_t addr, pages = efi_size_in_pages(size + (efi_addr & EFI_PAGE_MASK)); efi_addr &= ~EFI_PAGE_MASK;

•   status = efi_add_memory_map_pg(efi_addr, pages,
  

•   status = efi_update_memory_map(efi_addr, pages,
                                   op == MAP_OP_RESERVE ?
                                   EFI_BOOT_SERVICES_DATA :
                                   EFI_CONVENTIONAL_MEMORY,
  

•                                  false);
  

•                                  false, false);
    if (status != EFI_SUCCESS) {
            log_err("%s: LMB Map notify failure %lu\n", __func__,
                    status & ~EFI_ERROR_MASK);
  

-- 2.34.1

Hi Heinrich

On Mon, 11 Nov 2024 at 09:52, Heinrich Schuchardt xypron.glpk@gmx.de wrote:

On 10/25/24 13:14, Sughosh Ganu wrote:

...

From: Ilias Apalodimas ilias.apalodimas@linaro.org

With upcoming changes supporting pmem nodes, we need to ommit the

Nits: %s/ommit/remove/

...

pmem area rfom the EFI memory map. Add a function to do that

%s/rfom/from/ %s/that/that./

...

Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org

The patch is not applicable. Please, rebase on origin/master.

Will do thanks!

In the medium term we should get rid of the double book keeping. The memory map should be generated on the fly from LMB data.

Yes, I plan to have an RFC so we can see how that looks like

Thanks /Ilias

Best regards

Heinrich

...

---

include/efi_loader.h | 11 +++++--- lib/efi_loader/efi_memory.c | 51 +++++++++++++++++++++++++++---------- lib/lmb.c | 4 +-- 3 files changed, 47 insertions(+), 19 deletions(-)

diff --git a/include/efi_loader.h b/include/efi_loader.h index 291eca5c07..d450e304c6 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -786,7 +786,7 @@ efi_status_t efi_get_memory_map(efi_uintn_t *memory_map_size, efi_status_t efi_add_memory_map(u64 start, u64 size, int memory_type);

/**

• • efi_add_memory_map_pg() - add pages to the memory map

• • efi_update_memory_map() - update the memory map by adding/removing pages
  • @start: start address, must be a multiple of

  •                      EFI_PAGE_SIZE
    

@@ -794,11 +794,14 @@ efi_status_t efi_add_memory_map(u64 start, u64 size, int memory_type);

• @memory_type: type of memory added
• @overlap_conventional: region may only overlap free(conventional)

•                      memory
  

• • @remove: remove memory map
  • Return: status code
  */

-efi_status_t efi_add_memory_map_pg(u64 start, u64 pages,

•                            int memory_type,
  

•                            bool overlap_conventional);
  

+efi_status_t efi_update_memory_map(u64 start, u64 pages, int memory_type,

•                            bool overlap_conventional, bool remove);
  

+/* Remove memory from the EFI memory map */ +efi_status_t efi_remove_memory_map(u64 start, u64 size, int memory_type);

/* Called by board init to initialize the EFI drivers */ efi_status_t efi_driver_init(void); diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c index 3d742fa191..cb93bfa55f 100644 --- a/lib/efi_loader/efi_memory.c +++ b/lib/efi_loader/efi_memory.c @@ -258,7 +258,7 @@ static s64 efi_mem_carve_out(struct efi_mem_list *map, }

/**

• • efi_add_memory_map_pg() - add pages to the memory map

• • efi_update_memory_map() - update the memory map by adding/removing pages
  • @start: start address, must be a multiple of

  •                      EFI_PAGE_SIZE
    

@@ -266,11 +266,11 @@ static s64 efi_mem_carve_out(struct efi_mem_list *map,

• @memory_type: type of memory added
• @overlap_conventional: region may only overlap free(conventional)

•                      memory
  

• • @remove: remove memory map
  • Return: status code
  */

-efi_status_t efi_add_memory_map_pg(u64 start, u64 pages,

•                            int memory_type,
  

•                            bool overlap_conventional)
  

+efi_status_t efi_update_memory_map(u64 start, u64 pages, int memory_type,

•                            bool overlap_conventional, bool remove)
  

  { struct efi_mem_list *lmem; struct efi_mem_list *newlist;

@@ -278,9 +278,9 @@ efi_status_t efi_add_memory_map_pg(u64 start, u64 pages, uint64_t carved_pages = 0; struct efi_event *evt;

• EFI_PRINT("%s: 0x%llx 0x%llx %d %s\n", __func__,
  

• EFI_PRINT("%s: 0x%llx 0x%llx %d %s %s\n", __func__,
            start, pages, memory_type, overlap_conventional ?
  

•           "yes" : "no");
  

•           "yes" : "no", remove ? "remove" : "add");
  
  if (memory_type >= EFI_MAX_MEMORY_TYPE)
          return EFI_INVALID_PARAMETER;
  

@@ -363,7 +363,10 @@ efi_status_t efi_add_memory_map_pg(u64 start, u64 pages, }

  /* Add our new map */

•    list_add_tail(&newlist->link, &efi_mem);
  

• if (!remove)
  

•         list_add_tail(&newlist->link, &efi_mem);
  

• else
  

•         free(newlist);
  
  /* And make sure memory is listed in descending order */
  efi_mem_sort();
  

@@ -400,7 +403,29 @@ efi_status_t efi_add_memory_map(u64 start, u64 size, int memory_type) pages = efi_size_in_pages(size + (start & EFI_PAGE_MASK)); start &= ~EFI_PAGE_MASK;

• return efi_add_memory_map_pg(start, pages, memory_type, false);
  

• return efi_update_memory_map(start, pages, memory_type, false, false);
  

+}

+/**

• • efi_remove_memory_map() - remove memory area to the memory map
• • @start: start address of the memory area
• • @size: length in bytes of the memory area
• • @memory_type: type of memory removed
• • Return: status code
• • This function automatically aligns the start and size of the memory area
• • to EFI_PAGE_SIZE.
• */

+efi_status_t efi_remove_memory_map(u64 start, u64 size, int memory_type) +{

• u64 pages;
  

• pages = efi_size_in_pages(size + (start & EFI_PAGE_MASK));
  

• start &= ~EFI_PAGE_MASK;
  

• return efi_update_memory_map(start, pages, memory_type, false, true);
  

  }

  /**

@@ -500,7 +525,7 @@ efi_status_t efi_allocate_pages(enum efi_allocate_type type,

  addr = (u64)(uintptr_t)map_sysmem(addr, 0);
  /* Reserve that map in our memory maps */

• ret = efi_add_memory_map_pg(addr, pages, memory_type, true);
  

• ret = efi_update_memory_map(addr, pages, memory_type, true, false);
  if (ret != EFI_SUCCESS)
          /* Map would overlap, bail out */
          return  EFI_OUT_OF_RESOURCES;
  

@@ -542,8 +567,8 @@ efi_status_t efi_free_pages(uint64_t memory, efi_uintn_t pages) if (status) return EFI_NOT_FOUND;

• ret = efi_add_memory_map_pg(memory, pages, EFI_CONVENTIONAL_MEMORY,
  

•                             false);
  

• ret = efi_update_memory_map(memory, pages, EFI_CONVENTIONAL_MEMORY,
  

•                             false, false);
  if (ret != EFI_SUCCESS)
          return EFI_NOT_FOUND;
  
  

@@ -828,8 +853,8 @@ static void add_u_boot_and_runtime(void) runtime_end = (uintptr_t)__efi_runtime_stop; runtime_end = (runtime_end + runtime_mask) & ~runtime_mask; runtime_pages = (runtime_end - runtime_start) >> EFI_PAGE_SHIFT;

• efi_add_memory_map_pg(runtime_start, runtime_pages,
  

•                       EFI_RUNTIME_SERVICES_CODE, false);
  

• efi_update_memory_map(runtime_start, runtime_pages,
  

•                       EFI_RUNTIME_SERVICES_CODE, false, false);
  

  }

  int efi_memory_init(void)

diff --git a/lib/lmb.c b/lib/lmb.c index 7e90f17876..05f3cd093d 100644 --- a/lib/lmb.c +++ b/lib/lmb.c @@ -55,11 +55,11 @@ static int __maybe_unused lmb_map_update_notify(phys_addr_t addr, pages = efi_size_in_pages(size + (efi_addr & EFI_PAGE_MASK)); efi_addr &= ~EFI_PAGE_MASK;

• status = efi_add_memory_map_pg(efi_addr, pages,
  

• status = efi_update_memory_map(efi_addr, pages,
                                 op == MAP_OP_RESERVE ?
                                 EFI_BOOT_SERVICES_DATA :
                                 EFI_CONVENTIONAL_MEMORY,
  

•                                false);
  

•                                false, false);
  if (status != EFI_SUCCESS) {
          log_err("%s: LMB Map notify failure %lu\n", __func__,
                  status & ~EFI_ERROR_MASK);
  

+CC Anton

On Fri, 25 Oct 2024 at 14:14, Sughosh Ganu sughosh.ganu@linaro.org wrote:

From: Ilias Apalodimas ilias.apalodimas@linaro.org

One of the problems OS installers face, when running in EFI, is that the mounted ISO after calling ExitBootServices goes away. For some distros this is a problem since they rely on finding some core packages before continuing the installation. Distros have works around this -- e.g Fedora has a special kernel command line parameter called inst.stage2 [0].

ACPI has NFIT and NVDIMM support to provide ramdisks to the OS, but we don't have anything in place for DTs. Linux and device trees have support for persistent memory devices.

It's worth noting that for linux to instantiate the /dev/pmemX device, the memory described in the pmem node has to be omitted from the EFI memory map we hand over to the OS if ZONE_DEVICES and SPARSEMEM is enabled. With those enabled the pmem driver ends up calling devm_memremap_pages() instead of devm_memremap(). The latter works whether the memory is omitted or marked as reserved, but mapping pages only works if the memory is omitted.

On top of that, depending on how the kernel is configured, that memory area must be page aligned or 2MB aligned. PowerPC is an exception here and requires 16MB alignment, but since we don't have EFI support for it, limit the alignment to 2MB.

Ensure that the ISO image is 2MB aligned and remove the region occupied by the image from the EFI memory map.

Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org

---

lib/efi_loader/efi_bootmgr.c | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c index a3aa2b8d1b..16f75555f6 100644 --- a/lib/efi_loader/efi_bootmgr.c +++ b/lib/efi_loader/efi_bootmgr.c @@ -18,6 +18,8 @@ #include <efi_loader.h> #include <efi_variable.h> #include <asm/unaligned.h> +#include <linux/kernel.h> +#include <linux/sizes.h>

static const struct efi_boot_services *bs; static const struct efi_runtime_services *rs; @@ -358,13 +360,16 @@ static efi_status_t prepare_loaded_image(u16 *label, ulong addr, ulong size, }

    /*

•    * TODO: expose the ramdisk to OS.
  

•    * Need to pass the ramdisk information by the architecture-specific
  

•    * methods such as 'pmem' device-tree node.
  

•    * Linux supports 'pmem' which allows OS installers to find, reclaim
  

•    * the mounted images and continue the installation since the contents
  

•    * of the pmem region are treated as local media.
  

•    *
  

•    * The memory regions used for it needs to be carved out of the EFI
  

•    * memory map.
     */
  

•   ret = efi_add_memory_map(addr, size, EFI_RESERVED_MEMORY_TYPE);
  

•   ret = efi_remove_memory_map(addr, size, EFI_CONVENTIONAL_MEMORY);
    if (ret != EFI_SUCCESS) {
  

•           log_err("Memory reservation failed\n");
  

•           log_err("Failed to reserve memory\n");
            goto err;
    }
  
  

@@ -486,6 +491,13 @@ static efi_status_t try_load_from_uri_path(struct efi_device_path_uri *uridp, ret = EFI_INVALID_PARAMETER; goto err; }

•   /*
  

•    * Depending on the kernel configuration, pmem memory area must be page
  

•    * aligned or 2MB aligned. PowerPC is an exception here and requires
  

•    * 16MB alignment, but since we don't have EFI support for it, limit
  

•    * the alignment to 2MB.
  

•    */
  

•   image_size = ALIGN(image_size, SZ_2M);
  
    /*
     * If the file extension is ".iso" or ".img", mount it and try to load
  

-- 2.34.1

On Mon, 11 Nov 2024 at 10:05, Heinrich Schuchardt xypron.glpk@gmx.de wrote:

On 10/25/24 13:14, Sughosh Ganu wrote:

...

From: Ilias Apalodimas ilias.apalodimas@linaro.org

One of the problems OS installers face, when running in EFI, is that the mounted ISO after calling ExitBootServices goes away. For some distros this is a problem since they rely on finding some core packages before continuing the installation. Distros have works around this -- e.g Fedora has a special kernel command line parameter called inst.stage2 [0].

ACPI has NFIT and NVDIMM support to provide ramdisks to the OS, but we don't have anything in place for DTs. Linux and device trees have support for persistent memory devices.

It's worth noting that for linux to instantiate the /dev/pmemX device, the memory described in the pmem node has to be omitted from the EFI memory map we hand over to the OS if ZONE_DEVICES and SPARSEMEM is enabled. With those enabled the pmem driver ends up calling devm_memremap_pages() instead of devm_memremap(). The latter works whether the memory is omitted or marked as reserved, but mapping pages only works if the memory is omitted.

On top of that, depending on how the kernel is configured, that memory area must be page aligned or 2MB aligned. PowerPC is an exception here and requires 16MB alignment, but since we don't have EFI support for it, limit the alignment to 2MB.

Ensure that the ISO image is 2MB aligned and remove the region occupied by the image from the EFI memory map.

Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org

---

lib/efi_loader/efi_bootmgr.c | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c index a3aa2b8d1b..16f75555f6 100644 --- a/lib/efi_loader/efi_bootmgr.c +++ b/lib/efi_loader/efi_bootmgr.c @@ -18,6 +18,8 @@ #include <efi_loader.h> #include <efi_variable.h> #include <asm/unaligned.h> +#include <linux/kernel.h> +#include <linux/sizes.h>

static const struct efi_boot_services *bs; static const struct efi_runtime_services *rs; @@ -358,13 +360,16 @@ static efi_status_t prepare_loaded_image(u16 *label, ulong addr, ulong size, }

  /*

•  * TODO: expose the ramdisk to OS.
  

•  * Need to pass the ramdisk information by the architecture-specific
  

•  * methods such as 'pmem' device-tree node.
  

•  * Linux supports 'pmem' which allows OS installers to find, reclaim
  

•  * the mounted images and continue the installation since the contents
  

•  * of the pmem region are treated as local media.
  

•  *
  

•  * The memory regions used for it needs to be carved out of the EFI
  

•  * memory map.
   */
  

• ret = efi_add_memory_map(addr, size, EFI_RESERVED_MEMORY_TYPE);
  

• ret = efi_remove_memory_map(addr, size, EFI_CONVENTIONAL_MEMORY);
  if (ret != EFI_SUCCESS) {
  

•         log_err("Memory reservation failed\n");
  

•         log_err("Failed to reserve memory\n");
          goto err;
  }
  
  

@@ -486,6 +491,13 @@ static efi_status_t try_load_from_uri_path(struct efi_device_path_uri *uridp, ret = EFI_INVALID_PARAMETER; goto err; }

• /*
  

•  * Depending on the kernel configuration, pmem memory area must be page
  

•  * aligned or 2MB aligned. PowerPC is an exception here and requires
  

•  * 16MB alignment, but since we don't have EFI support for it, limit
  

•  * the alignment to 2MB.
  

•  */
  

• image_size = ALIGN(image_size, SZ_2M);
  

The code regarding .iso and .img handling seems to be misplaced. Why should we treat a file loaded from a block device differently to a file loaded from the network?

If the installer is located in a block device, the kernel will find the image when it scans those devices during boot. It's only when we have it on a memory area that we need to preserve it

Thanks /Ilias

Best regards

Heinrich

...

  /*
   * If the file extension is ".iso" or ".img", mount it and try to load

On 10/25/24 13:14, Sughosh Ganu wrote:

The EFI HTTP boot puts the iso installer image at some location in memory which needs to be reserved in the devicetree as persistent memory (pmem). Add helper functions which add this pmem node when the EFI_DT_FIXUP protocol's fixup callback is invoked.

Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org

boot/image-fdt.c | 9 +++++++++ include/efi_loader.h | 17 +++++++++++++++++ lib/efi_loader/efi_bootmgr.c | 21 +++++++++++++++++++++ lib/efi_loader/efi_helper.c | 12 ++++++++++++ 4 files changed, 59 insertions(+)

diff --git a/boot/image-fdt.c b/boot/image-fdt.c index 8eda521693..b39e81ad30 100644 --- a/boot/image-fdt.c +++ b/boot/image-fdt.c @@ -11,6 +11,7 @@ #include <command.h> #include <fdt_support.h> #include <fdtdec.h> +#include <efi_loader.h> #include <env.h> #include <errno.h> #include <image.h> @@ -648,6 +649,14 @@ int image_setup_libfdt(struct bootm_headers *images, void *blob, bool lmb) if (!ft_verify_fdt(blob)) goto err;

• if (CONFIG_IS_ENABLED(EFI_HTTP_BOOT)) {

• fdt_ret = fdt_efi_pmem_setup(blob);
  

Having EFI_HTTP_BOOT is not the only way to load an image to memory. You could have downloaded it via TFTP or taken it from a block device. So this config check looks wrong. Why don't you check for the presence of memory block devices here?

You need to check if the board is using ACPI, if yes, provide the ACPI table.

Best regards

Heinrich

• if (fdt_ret) {
  

• 	printf("ERROR: HTTP boot pmem fixup failed\n");
  

• 	goto err;
  

• }
  

• }
• /* after here we are using a livetree */ if (!of_live_active() && CONFIG_IS_ENABLED(EVENT)) { struct event_ft_fixup fixup;

diff --git a/include/efi_loader.h b/include/efi_loader.h index d450e304c6..031de18746 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -748,6 +748,15 @@ bool efi_varname_is_load_option(u16 *var_name16, int *index); efi_status_t efi_next_variable_name(efi_uintn_t *size, u16 **buf, efi_guid_t *guid);

+/**

• • fdt_efi_pmem_setup() - Setup the pmem node in the devicetree
• • @fdt: Pointer to the devicetree
• • Return: 0 on success, negative on failure
• */

+int fdt_efi_pmem_setup(void *fdt);

• /**
  • efi_size_in_pages() - convert size in bytes to size in pages

@@ -964,6 +973,14 @@ efi_status_t efi_set_load_options(efi_handle_t handle, void *load_options); efi_status_t efi_bootmgr_load(efi_handle_t *handle, void **load_options);

+/**

• • efi_bootmgr_pmem_setup() - Put a pmem node for UEFI HTTP installers
• • @fdt: Pointer to the DT blob
• • Return: status code
• */

+efi_status_t efi_bootmgr_pmem_setup(void *fdt);

• /**
  • struct efi_image_regions - A list of memory regions

diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c index 16f75555f6..1d9246be61 100644 --- a/lib/efi_loader/efi_bootmgr.c +++ b/lib/efi_loader/efi_bootmgr.c @@ -41,6 +41,8 @@ struct uridp_context { efi_handle_t mem_handle; };

+static struct uridp_context *uctx;

• const efi_guid_t efi_guid_bootmenu_auto_generated = EFICONFIG_AUTO_GENERATED_ENTRY_GUID;

@@ -423,6 +425,7 @@ efi_status_t efi_bootmgr_release_uridp(struct uridp_context *ctx)

efi_free_pool(ctx->loaded_dp); free(ctx);

• uctx = NULL;

  return ret == EFI_SUCCESS ? ret2 : ret; }

@@ -443,6 +446,23 @@ static void EFIAPI efi_bootmgr_http_return(struct efi_event *event, EFI_EXIT(ret); }

+/**

• • efi_bootmgr_pmem_setup() - Put a pmem node for UEFI HTTP installers
• • @fdt: Pointer to the DT blob
• • Return: status code
• */

+efi_status_t efi_bootmgr_pmem_setup(void *fdt) +{

• if (!uctx) {

• log_warning("No EFI HTTP boot context found\n");
  

• return EFI_SUCCESS;
  

• }
• return !fdt_fixup_pmem_region(fdt, uctx->image_addr, uctx->image_size) ?

• EFI_SUCCESS : EFI_INVALID_PARAMETER;
  

+}

• /**
  • try_load_from_uri_path() - Handle the URI device path

@@ -472,6 +492,7 @@ static efi_status_t try_load_from_uri_path(struct efi_device_path_uri *uridp, if (!ctx) return EFI_OUT_OF_RESOURCES;

• uctx = ctx; s = env_get("loadaddr"); if (!s) { log_err("Error: loadaddr is not set\n");

diff --git a/lib/efi_loader/efi_helper.c b/lib/efi_loader/efi_helper.c index 00167bd2a1..33cd8b9a50 100644 --- a/lib/efi_loader/efi_helper.c +++ b/lib/efi_loader/efi_helper.c @@ -242,6 +242,18 @@ int efi_unlink_dev(efi_handle_t handle) return 0; }

+/**

• • fdt_efi_pmem_setup() - Setup the pmem node in the devicetree
• • @fdt: Pointer to the devicetree
• • Return: 0 on success, negative on failure
• */

+int fdt_efi_pmem_setup(void *fdt) +{

• return efi_bootmgr_pmem_setup(fdt) == EFI_SUCCESS ? 0 : -1;

+}

• static int u16_tohex(u16 c) { if (c >= '0' && c <= '9')

On 10/25/24 13:14, Sughosh Ganu wrote:

The EFI HTTP boot puts the iso installer image at some location in memory which needs to be reserved in the devicetree as persistent memory (pmem). Add helper functions which add this pmem node when the EFI_DT_FIXUP protocol's fixup callback is invoked.

Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org

boot/image-fdt.c | 9 +++++++++ include/efi_loader.h | 17 +++++++++++++++++ lib/efi_loader/efi_bootmgr.c | 21 +++++++++++++++++++++ lib/efi_loader/efi_helper.c | 12 ++++++++++++ 4 files changed, 59 insertions(+)

diff --git a/boot/image-fdt.c b/boot/image-fdt.c index 8eda521693..b39e81ad30 100644 --- a/boot/image-fdt.c +++ b/boot/image-fdt.c @@ -11,6 +11,7 @@ #include <command.h> #include <fdt_support.h> #include <fdtdec.h> +#include <efi_loader.h> #include <env.h> #include <errno.h> #include <image.h> @@ -648,6 +649,14 @@ int image_setup_libfdt(struct bootm_headers *images, void *blob, bool lmb) if (!ft_verify_fdt(blob)) goto err;

• if (CONFIG_IS_ENABLED(EFI_HTTP_BOOT)) {

• fdt_ret = fdt_efi_pmem_setup(blob);
  

I can see no reason why pmem setup should depend on HTTP boot.

It should be possible to pass a memory block device to the kernel no matter how it was created.

Best regards

Heinrich

• if (fdt_ret) {
  

• 	printf("ERROR: HTTP boot pmem fixup failed\n");
  

• 	goto err;
  

• }
  

• }
• /* after here we are using a livetree */ if (!of_live_active() && CONFIG_IS_ENABLED(EVENT)) { struct event_ft_fixup fixup;

diff --git a/include/efi_loader.h b/include/efi_loader.h index d450e304c6..031de18746 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -748,6 +748,15 @@ bool efi_varname_is_load_option(u16 *var_name16, int *index); efi_status_t efi_next_variable_name(efi_uintn_t *size, u16 **buf, efi_guid_t *guid);

+/**

• • fdt_efi_pmem_setup() - Setup the pmem node in the devicetree
• • @fdt: Pointer to the devicetree
• • Return: 0 on success, negative on failure
• */

+int fdt_efi_pmem_setup(void *fdt);

• /**
  • efi_size_in_pages() - convert size in bytes to size in pages

@@ -964,6 +973,14 @@ efi_status_t efi_set_load_options(efi_handle_t handle, void *load_options); efi_status_t efi_bootmgr_load(efi_handle_t *handle, void **load_options);

+/**

• • efi_bootmgr_pmem_setup() - Put a pmem node for UEFI HTTP installers
• • @fdt: Pointer to the DT blob
• • Return: status code
• */

+efi_status_t efi_bootmgr_pmem_setup(void *fdt);

• /**
  • struct efi_image_regions - A list of memory regions

diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c index 16f75555f6..1d9246be61 100644 --- a/lib/efi_loader/efi_bootmgr.c +++ b/lib/efi_loader/efi_bootmgr.c @@ -41,6 +41,8 @@ struct uridp_context { efi_handle_t mem_handle; };

+static struct uridp_context *uctx;

• const efi_guid_t efi_guid_bootmenu_auto_generated = EFICONFIG_AUTO_GENERATED_ENTRY_GUID;

@@ -423,6 +425,7 @@ efi_status_t efi_bootmgr_release_uridp(struct uridp_context *ctx)

efi_free_pool(ctx->loaded_dp); free(ctx);

• uctx = NULL;

  return ret == EFI_SUCCESS ? ret2 : ret; }

@@ -443,6 +446,23 @@ static void EFIAPI efi_bootmgr_http_return(struct efi_event *event, EFI_EXIT(ret); }

+/**

• • efi_bootmgr_pmem_setup() - Put a pmem node for UEFI HTTP installers
• • @fdt: Pointer to the DT blob
• • Return: status code
• */

+efi_status_t efi_bootmgr_pmem_setup(void *fdt) +{

• if (!uctx) {

• log_warning("No EFI HTTP boot context found\n");
  

• return EFI_SUCCESS;
  

• }
• return !fdt_fixup_pmem_region(fdt, uctx->image_addr, uctx->image_size) ?

• EFI_SUCCESS : EFI_INVALID_PARAMETER;
  

+}

• /**
  • try_load_from_uri_path() - Handle the URI device path

@@ -472,6 +492,7 @@ static efi_status_t try_load_from_uri_path(struct efi_device_path_uri *uridp, if (!ctx) return EFI_OUT_OF_RESOURCES;

• uctx = ctx; s = env_get("loadaddr"); if (!s) { log_err("Error: loadaddr is not set\n");

diff --git a/lib/efi_loader/efi_helper.c b/lib/efi_loader/efi_helper.c index 00167bd2a1..33cd8b9a50 100644 --- a/lib/efi_loader/efi_helper.c +++ b/lib/efi_loader/efi_helper.c @@ -242,6 +242,18 @@ int efi_unlink_dev(efi_handle_t handle) return 0; }

+/**

• • fdt_efi_pmem_setup() - Setup the pmem node in the devicetree
• • @fdt: Pointer to the devicetree
• • Return: 0 on success, negative on failure
• */

+int fdt_efi_pmem_setup(void *fdt) +{

• return efi_bootmgr_pmem_setup(fdt) == EFI_SUCCESS ? 0 : -1;

+}

• static int u16_tohex(u16 c) { if (c >= '0' && c <= '9')

On 11/11/24 11:45, Ilias Apalodimas wrote:

Hi Heinrich

On Mon, 11 Nov 2024 at 10:08, Heinrich Schuchardt xypron.glpk@gmx.de wrote:

...

On 10/25/24 13:14, Sughosh Ganuefi_bootmgr_pmem_setup wrote:

...

The EFI HTTP boot puts the iso installer image at some location in memory which needs to be reserved in the devicetree as persistent memory (pmem). Add helper functions which add this pmem node when the EFI_DT_FIXUP protocol's fixup callback is invoked.

Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org

boot/image-fdt.c | 9 +++++++++ include/efi_loader.h | 17 +++++++++++++++++ lib/efi_loader/efi_bootmgr.c | 21 +++++++++++++++++++++ lib/efi_loader/efi_helper.c | 12 ++++++++++++ 4 files changed, 59 insertions(+)

diff --git a/boot/image-fdt.c b/boot/image-fdt.c index 8eda521693..b39e81ad30 100644 --- a/boot/image-fdt.c +++ b/boot/image-fdt.c @@ -11,6 +11,7 @@ #include <command.h> #include <fdt_support.h> #include <fdtdec.h> +#include <efi_loader.h> #include <env.h> #include <errno.h> #include <image.h> @@ -648,6 +649,14 @@ int image_setup_libfdt(struct bootm_headers *images, void *blob, bool lmb) if (!ft_verify_fdt(blob)) goto err;

• if (CONFIG_IS_ENABLED(EFI_HTTP_BOOT)) {
  

•         fdt_ret = fdt_efi_pmem_setup(blob);
  

I can see no reason why pmem setup should depend on HTTP boot.

The reason is that we *know* we want to preserve image on HTTP installers. but we should only add it if that image is booted, not unconditionally if EFI_HTTP is enabled

...

It should be possible to pass a memory block device to the kernel no matter how it was created.

Yes, we can. But how do we know we need to setup a pmem node? In this specific case, we know http installers need the image. If we can find similar rules (or perhaps a command line option), we can preserve it for all images

Simon is working on patches to track loaded images. I guess there we would need to add the detection of image types including bare kernels, EFI binaries, and ISOs.

Thanks /Ilias

...

Best regards

Heinrich

...

•         if (fdt_ret) {
  

•                 printf("ERROR: HTTP boot pmem fixup failed\n");
  

Please, use log_err() and remove "ERROR: ".

...

...

•                 goto err;
  

•         }
  

• }
  

•  /* after here we are using a livetree */
   if (!of_live_active() && CONFIG_IS_ENABLED(EVENT)) {
           struct event_ft_fixup fixup;
  

diff --git a/include/efi_loader.h b/include/efi_loader.h index d450e304c6..031de18746 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -748,6 +748,15 @@ bool efi_varname_is_load_option(u16 *var_name16, int *index); efi_status_t efi_next_variable_name(efi_uintn_t *size, u16 **buf, efi_guid_t *guid);

+/**

• • fdt_efi_pmem_setup() - Setup the pmem node in the devicetree
• • @fdt: Pointer to the devicetree
• • Return: 0 on success, negative on failure
• */

+int fdt_efi_pmem_setup(void *fdt);

• /**
  • efi_size_in_pages() - convert size in bytes to size in pages

@@ -964,6 +973,14 @@ efi_status_t efi_set_load_options(efi_handle_t handle, void *load_options); efi_status_t efi_bootmgr_load(efi_handle_t *handle, void **load_options);

+/**

• • efi_bootmgr_pmem_setup() - Put a pmem node for UEFI HTTP installers
• • @fdt: Pointer to the DT blob
• • Return: status code
• */

+efi_status_t efi_bootmgr_pmem_setup(void *fdt);

• /**
  • struct efi_image_regions - A list of memory regions

diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c index 16f75555f6..1d9246be61 100644 --- a/lib/efi_loader/efi_bootmgr.c +++ b/lib/efi_loader/efi_bootmgr.c @@ -41,6 +41,8 @@ struct uridp_context { efi_handle_t mem_handle; };

+static struct uridp_context *uctx;

• const efi_guid_t efi_guid_bootmenu_auto_generated = EFICONFIG_AUTO_GENERATED_ENTRY_GUID;

@@ -423,6 +425,7 @@ efi_status_t efi_bootmgr_release_uridp(struct uridp_context *ctx)

   efi_free_pool(ctx->loaded_dp);
   free(ctx);

• uctx = NULL;
  
   return ret == EFI_SUCCESS ? ret2 : ret;
  

  }

@@ -443,6 +446,23 @@ static void EFIAPI efi_bootmgr_http_return(struct efi_event *event, EFI_EXIT(ret); }

+/**

• • efi_bootmgr_pmem_setup() - Put a pmem node for UEFI HTTP installers
• • @fdt: Pointer to the DT blob
• • Return: status code
• */

+efi_status_t efi_bootmgr_pmem_setup(void *fdt) +{

• if (!uctx) {
  

•         log_warning("No EFI HTTP boot context found\n");
  

Are we writing a warning here when loading grubriscv64.efi from disk and executing it?

Best regards

Heinrich

...

...

•         return EFI_SUCCESS;
  

• }
  

• return !fdt_fixup_pmem_region(fdt, uctx->image_addr, uctx->image_size) ?
  

•         EFI_SUCCESS : EFI_INVALID_PARAMETER;
  

+}

• /**
  • try_load_from_uri_path() - Handle the URI device path

@@ -472,6 +492,7 @@ static efi_status_t try_load_from_uri_path(struct efi_device_path_uri *uridp, if (!ctx) return EFI_OUT_OF_RESOURCES;

• uctx = ctx;
   s = env_get("loadaddr");
   if (!s) {
           log_err("Error: loadaddr is not set\n");
  

diff --git a/lib/efi_loader/efi_helper.c b/lib/efi_loader/efi_helper.c index 00167bd2a1..33cd8b9a50 100644 --- a/lib/efi_loader/efi_helper.c +++ b/lib/efi_loader/efi_helper.c @@ -242,6 +242,18 @@ int efi_unlink_dev(efi_handle_t handle) return 0; }

+/**

• • fdt_efi_pmem_setup() - Setup the pmem node in the devicetree
• • @fdt: Pointer to the devicetree
• • Return: 0 on success, negative on failure
• */

+int fdt_efi_pmem_setup(void *fdt) +{

• return efi_bootmgr_pmem_setup(fdt) == EFI_SUCCESS ? 0 : -1;
  

+}

• static int u16_tohex(u16 c) { if (c >= '0' && c <= '9')