[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