diff mbox series

[v3,2/5] cmd: bootefi: Parse reserved-memory node from DT

Message ID 20200317211946.28062-3-atish.patra@wdc.com
State New
Headers show
Series DT related fixes for RISC-V UEFI | expand

Commit Message

Atish Patra March 17, 2020, 9:19 p.m. UTC 
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.patra at wdc.com>
---
 cmd/bootefi.c | 44 +++++++++++++++++++++++++++++++++++---------
 1 file changed, 35 insertions(+), 9 deletions(-)

Comments

Heinrich Schuchardt March 17, 2020, 10:01 p.m. UTC | #1 
On 3/17/20 10:19 PM, Atish Patra 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.
>
> 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 | 44 +++++++++++++++++++++++++++++++++++---------
>   1 file changed, 35 insertions(+), 9 deletions(-)
>
> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> index 24fc42ae898e..291cb2d69ff6 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,26 @@ 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)
> +		return;
> +	subnode = fdt_first_subnode(fdt, nodeoffset);
> +	while (subnode >= 0) {
> +		/* check if this subnode has a reg property */
> +		addr = fdtdec_get_addr_size_auto_noparent(fdt, subnode,
> +							  "reg", 0,
> +							  (fdt_size_t *)&size,
> +							  true);
> +		if (addr == FDT_ADDR_T_NONE) {
> +			debug("failed to read address/size\n");
> +			continue;

As you do not update subnode you never leave the loop, cf.
https://lists.denx.de/pipermail/u-boot/2020-March/402891.html

Best regards

Heinrich

> +		}
> +		efi_reserve_memory(addr, size);
> +		subnode = fdt_next_subnode(fdt, subnode);
>   	}
>   }
>
>
Heinrich Schuchardt March 18, 2020, 6:24 p.m. UTC | #2 
On 3/17/20 11:01 PM, Heinrich Schuchardt wrote:
> On 3/17/20 10:19 PM, Atish Patra 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.
>>
>> 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 | 44 +++++++++++++++++++++++++++++++++++---------
>> ? 1 file changed, 35 insertions(+), 9 deletions(-)
>>
>> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
>> index 24fc42ae898e..291cb2d69ff6 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,26 @@ 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)
>> +??????? return;
>> +??? subnode = fdt_first_subnode(fdt, nodeoffset);
>> +??? while (subnode >= 0) {
>> +??????? /* check if this subnode has a reg property */
>> +??????? addr = fdtdec_get_addr_size_auto_noparent(fdt, subnode,
>> +????????????????????????????? "reg", 0,
>> +????????????????????????????? (fdt_size_t *)&size,
>> +????????????????????????????? true);
>> +??????? if (addr == FDT_ADDR_T_NONE) {
>> +??????????? debug("failed to read address/size\n");
>> +??????????? continue;
> 
> As you do not update subnode you never leave the loop, cf.
> https://lists.denx.de/pipermail/u-boot/2020-March/402891.html
> 
> Best regards
> 
> Heinrich

Corrected and merged into origin/master.

Best regards

Heinrich

> 
>> +??????? }
>> +??????? efi_reserve_memory(addr, size);
>> +??????? subnode = fdt_next_subnode(fdt, subnode);
>> ????? }
>> ? }
>>
>>
>
Atish Patra March 18, 2020, 6:44 p.m. UTC | #3 
On Tue, Mar 17, 2020 at 3:02 PM Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>
> On 3/17/20 10:19 PM, Atish Patra 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.
> >
> > 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 | 44 +++++++++++++++++++++++++++++++++++---------
> >   1 file changed, 35 insertions(+), 9 deletions(-)
> >
> > diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> > index 24fc42ae898e..291cb2d69ff6 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,26 @@ 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)
> > +             return;
> > +     subnode = fdt_first_subnode(fdt, nodeoffset);
> > +     while (subnode >= 0) {
> > +             /* check if this subnode has a reg property */
> > +             addr = fdtdec_get_addr_size_auto_noparent(fdt, subnode,
> > +                                                       "reg", 0,
> > +                                                       (fdt_size_t *)&size,
> > +                                                       true);
> > +             if (addr == FDT_ADDR_T_NONE) {
> > +                     debug("failed to read address/size\n");
> > +                     continue;
>
> As you do not update subnode you never leave the loop, cf.
> https://lists.denx.de/pipermail/u-boot/2020-March/402891.html
>


Thanks for the fix. I have verified your sandbox patches as well with
bootefi hello
and efidebug memmap as well.

> Best regards
>
> Heinrich
>
> > +             }
> > +             efi_reserve_memory(addr, size);
> > +             subnode = fdt_next_subnode(fdt, subnode);
> >       }
> >   }
> >
> >
>
diff mbox series

Patch

diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index 24fc42ae898e..291cb2d69ff6 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,26 @@  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)
+		return;
+	subnode = fdt_first_subnode(fdt, nodeoffset);
+	while (subnode >= 0) {
+		/* check if this subnode has a reg property */
+		addr = fdtdec_get_addr_size_auto_noparent(fdt, subnode,
+							  "reg", 0,
+							  (fdt_size_t *)&size,
+							  true);
+		if (addr == FDT_ADDR_T_NONE) {
+			debug("failed to read address/size\n");
+			continue;
+		}
+		efi_reserve_memory(addr, size);
+		subnode = fdt_next_subnode(fdt, subnode);
 	}
 }