On 3/17/20 9:06 AM, Patrick DELAUNAY wrote:
Hi,

From: U-Boot <u-boot-boun...@lists.denx.de> On Behalf Of Heinrich Schuchardt
Sent: dimanche 15 mars 2020 13:03

From: Atish Patra <atish.pa...@wdc.com>

Currently, bootefi only parses memory reservation block to setup EFI reserved
memory mappings. However, it doesn't parse the reserved-memory[1] device tree
node that also can contain the reserved memory regions.

Add capability to parse reserved-memory node and update the EFI memory
mappings accordingly.

1. <U-Boot source>/doc/device-tree-bindings/reserved-memory/reserved-
memory.txt]

Signed-off-by: Atish Patra <atish.pa...@wdc.com>

Fix an endless loop.

The /reserved-memory node may have children without reg property. Remove a
superfluous debug statement.

Signed-off-by: Heinrich Schuchardt <xypron.g...@gmx.de>
---
  cmd/bootefi.c | 43 ++++++++++++++++++++++++++++++++++---------
  1 file changed, 34 insertions(+), 9 deletions(-)

diff --git a/cmd/bootefi.c b/cmd/bootefi.c index 24fc42ae89..b5b9d88273 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -149,6 +149,20 @@ done:
        return ret;
  }

+static void efi_reserve_memory(u64 addr, u64 size) {
+       u64 pages;
+
+       /* Convert from sandbox address space. */
+       addr = (uintptr_t)map_sysmem(addr, 0);
+       pages = efi_size_in_pages(size + (addr & EFI_PAGE_MASK));
+       addr &= ~EFI_PAGE_MASK;
+       if (efi_add_memory_map(addr, pages, EFI_RESERVED_MEMORY_TYPE,
+                              false) != EFI_SUCCESS)
+               printf("Reserved memory mapping failed addr %llx size %llx\n",
+                      addr, size);
+}
+
  /**
   * efi_carve_out_dt_rsv() - Carve out DT reserved memory ranges
   *
@@ -161,7 +175,8 @@ done:
  static void efi_carve_out_dt_rsv(void *fdt)  {
        int nr_rsv, i;
-       uint64_t addr, size, pages;
+       u64 addr, size;
+       int nodeoffset, subnode;

        nr_rsv = fdt_num_mem_rsv(fdt);

@@ -169,15 +184,25 @@ static void efi_carve_out_dt_rsv(void *fdt)
        for (i = 0; i < nr_rsv; i++) {
                if (fdt_get_mem_rsv(fdt, i, &addr, &size) != 0)
                        continue;
+               efi_reserve_memory(addr, size);
+       }

-               /* Convert from sandbox address space. */
-               addr = (uintptr_t)map_sysmem(addr, 0);
-
-               pages = efi_size_in_pages(size + (addr & EFI_PAGE_MASK));
-               addr &= ~EFI_PAGE_MASK;
-               if (efi_add_memory_map(addr, pages,
EFI_RESERVED_MEMORY_TYPE,
-                                      false) != EFI_SUCCESS)
-                       printf("FDT memrsv map %d: Failed to add to map\n", i);
+       /* process reserved-memory */
+       nodeoffset = fdt_subnode_offset(fdt, 0, "reserved-memory");
+       if (nodeoffset >= 0) {
+               subnode = fdt_first_subnode(fdt, nodeoffset);
+               while (subnode >= 0) {
+                       /* check if this subnode has a reg property */
+                       addr = fdtdec_get_addr_size(fdt, subnode, "reg",
+                                                   (fdt_size_t *)&size);
+                       /*
+                        * The /reserved-memory node may have children with
+                        * a size instead of a reg property.
+                        */
+                       if (addr != FDT_ADDR_T_NONE)
+                               efi_reserve_memory(addr, size);
+                       subnode = fdt_next_subnode(fdt, subnode);
+               }
        }
  }

This part seens the same that the existing function
common/image-fdt.c::boot_fdt_add_mem_rsv_regions()

For information, a status check was added by commit
"image: fdt: check "status" of "/reserved-memory" subnodes"

See SHA1 28b417ce859490d6b06e71dbf4e842841e64d34d

Perhaps need to be added also here.

It makes sense to keep these coding stretches in sync.

@Thirupathaiah
* I did not find any example in the Linux code where a status for
  a subnode of /reserved-memory was specified. Where did you
  observe reserved memory not having status="okay"?
* Does Linux check the status?
* Could the /reserved-memory node itself have a status other than
  "okay"?

How should we treat reserved memory marked as 'reusable'? Should UEFI
make these regions as reserved?

I tried to understand when boot_fdt_add_mem_rsv_regions() is invoked. I
was astonished that the dhcp command calls it when loading a file. At
that time the device-tree used for booting is not known:

=> dhcp helloworld_arm64.efi
e1000: 52:54:00:12:34:56

Warning: e1000#0 using MAC address from ROM
BOOTP broadcast 1
BOOTP broadcast 2
BOOTP broadcast 3
DHCP client bound to address 10.0.2.15 (1010 ms)
Using e1000#0 device
TFTP from server 10.0.2.2; our IP address is 10.0.2.15
Filename 'helloworld_arm64.efi'.
common/image-fdt.c(110) boot_fdt_add_mem_rsv_regions:
Load address: 0x40200000
Loading: #
         2.2 MiB/s
done
Bytes transferred = 4528 (11b0 hex)

Best regards

Heinrich

Reply via email to