[PATCH v2 2/4] cmd: bootefi: Parse reserved-memory node from DT

Atish Patra atishp at atishpatra.org
Wed Mar 18 10:10:31 CET 2020


On Sat, Mar 14, 2020 at 12:14 AM Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>
> On 3/14/20 1:55 AM, Atish Patra wrote:
> > On Fri, Mar 13, 2020 at 5:12 PM Atish Patra <atish.patra at wdc.com> wrote:
> >>
> >> 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.
>
> Hello Atish,
>
> thanks for reporting the problem.
>
> I would appreciate a test to verify that the code really works. We
> should add both types of reserved-memory device tree node to the sandbox
> device-tree and parse the output of 'efidebug memmap' and compare it to
> the output of 'fdt print'. Could you, please, provide a first patch for
> sandbox.dts and sandbox64.dts. The next step then would be to build a
> Python test.
>

Sorry I didn't see your reply before sending out a v3. I missed this
email earlier because of a wrong email filter.
I will add the tests and fix the endless loop problem in v4 as you suggested.

> This patch series could be split into two. One for EFI and one for RISC-V.
>
> (see comment below)
>
> >>
> >> 1. <U-Boot source>/doc/device-tree-bindings/reserved-memory/reserved-memory.txt]
> >>
> >> Signed-off-by: Atish Patra <atish.patra at wdc.com>
> >> ---
> >>   cmd/bootefi.c | 42 +++++++++++++++++++++++++++++++++---------
> >>   1 file changed, 33 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> >> index 24fc42ae898e..43b36fbacfcd 100644
> >> --- a/cmd/bootefi.c
> >> +++ b/cmd/bootefi.c
> >> @@ -149,6 +149,20 @@ done:
> >>          return ret;
> >>   }
> >>
> >> +static void efi_reserve_memory(uint64_t addr, uint64_t size)
> >> +{
> >> +       uint64_t 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",
> >> +                     (unsigned long long)addr, (unsigned long long)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;
> >> +       uint64_t addr, size;
> >> +       int nodeoffset, subnode;
> >>
> >>          nr_rsv = fdt_num_mem_rsv(fdt);
> >>
> >> @@ -169,15 +184,24 @@ 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);
> >> +                       if (addr == FDT_ADDR_T_NONE) {
> >> +                               debug("failed to read address/size\n");
>
> Linux
> Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt has:
>
> "Each child of the reserved-memory node specifies one or more regions of
> reserved memory. Each child node may either use a 'reg' property to
> specify a specific range of reserved memory, or a 'size' property with
> optional constraints to request a dynamically allocated block of memory.
> ... If both reg and size are present, then the reg property takes
> precedence and size is ignored."
>
> So finding no reg property should not be considered a bug. I see no need
> for a debug statement here.
>
> For the sandbox test we should have a node with 'size' and at least two
> nodes with 'reg'.
>
> Otherwise:
>
> Reviewed-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
>
> >> +                               continue;
> >> +                       }
> >> +                       efi_reserve_memory(addr, size);
> >> +                       subnode = fdt_next_subnode(fdt, subnode);
> >> +               }
> >>          }
> >>   }
> >>
> >> --
> >> 2.25.1
> >>
> >
> > Fixed palmer's email address. Sorry for the spam.
> >
>


-- 
Regards,
Atish


More information about the U-Boot mailing list