[PATCH v3 0/5] DT related fixes for RISC-V UEFI

[PATCH v3 0/5] DT related fixes for RISC-V UEFI

[Atish Patra]

This series adds few DT related fixes required for Linux EFI stub to work
on RISC-V.

Patch 1 adds the boot hartid property under /chosen node. The related
discussion can be found here.

https://patchwork.ozlabs.org/patch/1233664/
https://lists.denx.de/pipermail/u-boot/2020-March/402085.html

Patch 2 fixes a generic issue in bootefi.

Patch 3,4,5 provide one of the option to update reserved-memory node for next
stage.
It depends on Bin's following series in OpenSBI
http://lists.infradead.org/pipermail/opensbi/2020-March/001316.html

The other options are SBI extension and trap/emulate on PMP csr access.
The detaild discussion can be found here.
https://github.com/riscv/riscv-sbi-doc/pull/37

Patch 1 & 2 can be applied independently from 3 and 4. I want to keep all
the patches together to provide a holistic view of changes required for
RISC-V UEFI.

Changes from v2->v3:
1. Update the DT meant for OS if it is different from the one used by U-Boot
2. Use different FDT api to obtain "reg" address & size to honor the cell count.

Changes from v1->v2:
1. Fix the issue if chosen node is not present.

Changes from previous version:
1. Renamed the DT node property to "boot-hartid" from "efi-boot-hartid".
2. Changed the property type to u32 instead of u64 for RV32 compatibility.

Atish Patra (5):
riscv: Add boot hartid to Device tree
cmd: bootefi: Parse reserved-memory node from DT
riscv: Provide a mechanism to fix DT for reserved memory
riscv: Setup reserved-memory node for FU540
riscv: Copy the reserved-memory nodes to final DT

arch/riscv/cpu/start.S                |  1 +
arch/riscv/include/asm/global_data.h  |  1 +
arch/riscv/include/asm/u-boot-riscv.h |  1 +
arch/riscv/lib/asm-offsets.c          |  1 +
arch/riscv/lib/bootm.c                | 95 +++++++++++++++++++++++++++
board/sifive/fu540/fu540.c            | 15 +++++
cmd/bootefi.c                         | 44 ++++++++++---
configs/sifive_fu540_defconfig        |  1 +
8 files changed, 150 insertions(+), 9 deletions(-)

--
2.25.1

[PATCH v3 1/5] riscv: Add boot hartid to Device tree

[Atish Patra]

Linux booting protocol mandates that register "a0" contains the hartid.
However, U-boot can not pass the hartid via a0 during via standard UEFI
protocol. DT nodes are commonly used to pass such information to the OS.

Add a DT node under chosen node to indicate the boot hartid. EFI stub
in Linux kernel will parse this node and pass it to the real kernel
in "a0" before jumping to it.

Signed-off-by: Atish Patra <atish.patra@wdc.com>
Reviewed-by: Rick Chen <rick@andestech.com>
---
 arch/riscv/lib/bootm.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/arch/riscv/lib/bootm.c b/arch/riscv/lib/bootm.c
index fad16901c5f2..f927694ae32f 100644
--- a/arch/riscv/lib/bootm.c
+++ b/arch/riscv/lib/bootm.c
@@ -28,6 +28,28 @@ __weak void board_quiesce_devices(void)
 
 int arch_fixup_fdt(void *blob)
 {
+	u32 size;
+	int chosen_offset, err;
+
+	size = fdt_totalsize(blob);
+	err  = fdt_open_into(blob, blob, size + 32);
+	if (err < 0) {
+		printf("Device Tree can't be expanded to accommodate new node");
+		return -1;
+	}
+	chosen_offset = fdt_path_offset(blob, "/chosen");
+	if (chosen_offset < 0) {
+		err = fdt_add_subnode(blob, 0, "chosen");
+		if (err < 0) {
+			printf("chosen node can not be added\n");
+			return -1;
+		}
+	}
+
+	/* Overwrite the boot-hartid as U-Boot is the last state BL */
+	fdt_setprop_u32(blob, chosen_offset, "boot-hartid",
+			   gd->arch.boot_hart);
+
 	return 0;
 }
 
-- 
2.25.1

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

[Atish Patra]

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@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;
+		}
+		efi_reserve_memory(addr, size);
+		subnode = fdt_next_subnode(fdt, subnode);
 	}
 }
 
-- 
2.25.1

[PATCH v3 3/5] riscv: Provide a mechanism to fix DT for reserved memory

[Atish Patra]

In RISC-V, M-mode software can reserve physical memory regions
by setting appropriate physical memory protection (PMP) csr. As the
PMP csr are accessible only in M-mode, S-mode U-Boot can not read
this configuration directly. However, M-mode software can pass this
information via reserved-memory node in device tree so that S-mode
software can access this information.

This patch provides a framework to copy to the reserved-memory node
from one DT to another. This will be used to update the DT used by
U-Boot and the DT passed to the next stage OS.

Signed-off-by: Atish Patra <atish.patra@wdc.com>
---
 arch/riscv/cpu/start.S                |  1 +
 arch/riscv/include/asm/global_data.h  |  1 +
 arch/riscv/include/asm/u-boot-riscv.h |  1 +
 arch/riscv/lib/asm-offsets.c          |  1 +
 arch/riscv/lib/bootm.c                | 68 +++++++++++++++++++++++++++
 5 files changed, 72 insertions(+)

diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S
index 6b3ff99c3882..0282685c2906 100644
--- a/arch/riscv/cpu/start.S
+++ b/arch/riscv/cpu/start.S
@@ -121,6 +121,7 @@ call_board_init_f_0:
 
 	jal	board_init_f_init_reserve
 
+	SREG	s1, GD_FIRMWARE_FDT_ADDR(gp)
 	/* save the boot hart id to global_data */
 	SREG	tp, GD_BOOT_HART(gp)
 
diff --git a/arch/riscv/include/asm/global_data.h b/arch/riscv/include/asm/global_data.h
index b74bd7e738bb..51ac8d1c98e2 100644
--- a/arch/riscv/include/asm/global_data.h
+++ b/arch/riscv/include/asm/global_data.h
@@ -15,6 +15,7 @@
 /* Architecture-specific global data */
 struct arch_global_data {
 	long boot_hart;		/* boot hart id */
+	phys_addr_t firmware_fdt_addr;
 #ifdef CONFIG_SIFIVE_CLINT
 	void __iomem *clint;	/* clint base address */
 #endif
diff --git a/arch/riscv/include/asm/u-boot-riscv.h b/arch/riscv/include/asm/u-boot-riscv.h
index 49febd588102..b7bea0ba184d 100644
--- a/arch/riscv/include/asm/u-boot-riscv.h
+++ b/arch/riscv/include/asm/u-boot-riscv.h
@@ -17,5 +17,6 @@ int cleanup_before_linux(void);
 /* board/.../... */
 int board_init(void);
 void board_quiesce_devices(void);
+int riscv_board_reserved_mem_fixup(void *fdt);
 
 #endif	/* _U_BOOT_RISCV_H_ */
diff --git a/arch/riscv/lib/asm-offsets.c b/arch/riscv/lib/asm-offsets.c
index 4fa4fd371473..7301c1b98e23 100644
--- a/arch/riscv/lib/asm-offsets.c
+++ b/arch/riscv/lib/asm-offsets.c
@@ -14,6 +14,7 @@
 int main(void)
 {
 	DEFINE(GD_BOOT_HART, offsetof(gd_t, arch.boot_hart));
+	DEFINE(GD_FIRMWARE_FDT_ADDR, offsetof(gd_t, arch.firmware_fdt_addr));
 #ifndef CONFIG_XIP
 	DEFINE(GD_AVAILABLE_HARTS, offsetof(gd_t, arch.available_harts));
 #endif
diff --git a/arch/riscv/lib/bootm.c b/arch/riscv/lib/bootm.c
index f927694ae32f..5e907e96701c 100644
--- a/arch/riscv/lib/bootm.c
+++ b/arch/riscv/lib/bootm.c
@@ -19,6 +19,7 @@
 #include <dm/device.h>
 #include <dm/root.h>
 #include <u-boot/zlib.h>
+#include <mapmem.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -26,6 +27,73 @@ __weak void board_quiesce_devices(void)
 {
 }
 
+int riscv_fdt_copy_resv_mem_node(const void *src, void *dst)
+{
+	uint32_t phandle;
+	struct fdt_memory pmp_mem;
+	fdt_addr_t addr;
+	fdt_size_t size;
+	int offset, node, err, rmem_offset;
+	bool nomap = false;
+	char basename[32] = {0};
+	int bname_len;
+	int max_len = sizeof(basename);
+	const char *name;
+	char *temp;
+
+	offset = fdt_path_offset(src, "/reserved-memory");
+	if (offset < 0) {
+		printf("No reserved memory region found in source FDT\n");
+		return 0;
+	}
+
+	fdt_for_each_subnode(node, src, offset) {
+		name = fdt_get_name(src, node, NULL);
+
+		addr = fdtdec_get_addr_size_auto_noparent(src, node,
+							  "reg", 0, &size,
+							  true);
+		if (addr == FDT_ADDR_T_NONE) {
+			debug("failed to read address/size for %s\n", name);
+			continue;
+		}
+		strncpy(basename, name, max_len);
+		temp = strchr(basename, '@');
+		if (temp) {
+			bname_len = strnlen(basename, max_len) - strnlen(temp,
+								       max_len);
+			*(basename+bname_len) = '\0';
+		}
+		pmp_mem.start = addr;
+		pmp_mem.end = addr + size;
+		err = fdtdec_add_reserved_memory(dst, basename, &pmp_mem,
+						 &phandle);
+		if (err < 0) {
+			printf("failed to add reserved memory: %d\n", err);
+			return err;
+		}
+		rmem_offset = fdt_node_offset_by_phandle(dst, phandle);
+		nomap = fdt_get_property(src, node, "no-map", NULL);
+		if (nomap)
+			fdt_setprop_empty(dst, rmem_offset, "no-map");
+	}
+
+	return 0;
+}
+
+int riscv_board_reserved_mem_fixup(void *fdt)
+{
+	int err;
+	void *src_fdt_addr;
+
+	src_fdt_addr = map_sysmem(gd->arch.firmware_fdt_addr, 0);
+	err = riscv_fdt_copy_resv_mem_node(src_fdt_addr, fdt);
+	if (err < 0)
+		return err;
+
+	return 0;
+}
+
 int arch_fixup_fdt(void *blob)
 {
 	u32 size;
-- 
2.25.1

[PATCH v3 4/5] riscv: Setup reserved-memory node for FU540

[Atish Patra]

FU540 uses OF_SEPARATE instead of OF_PRIOR.

Enable OF_BOARD_FIXUP to update the DT with reserved-memory node.

Signed-off-by: Atish Patra <atish.patra@wdc.com>
---
 board/sifive/fu540/fu540.c     | 15 +++++++++++++++
 configs/sifive_fu540_defconfig |  1 +
 2 files changed, 16 insertions(+)

diff --git a/board/sifive/fu540/fu540.c b/board/sifive/fu540/fu540.c
index 47a20902517c..82b3a9c8e729 100644
--- a/board/sifive/fu540/fu540.c
+++ b/board/sifive/fu540/fu540.c
@@ -141,6 +141,21 @@ int misc_init_r(void)
 
 #endif
 
+#ifdef CONFIG_OF_BOARD_FIXUP
+int board_fix_fdt(void *fdt)
+{
+	int err;
+
+	err = riscv_board_reserved_mem_fixup(fdt);
+	if (err < 0) {
+		printf("failed to fixup DT for reserved memory: %d\n", err);
+		return err;
+	}
+
+	return 0;
+}
+#endif
+
 int board_init(void)
 {
 	/* For now nothing to do here. */
diff --git a/configs/sifive_fu540_defconfig b/configs/sifive_fu540_defconfig
index 6d61e6c960ee..8fb3794cd578 100644
--- a/configs/sifive_fu540_defconfig
+++ b/configs/sifive_fu540_defconfig
@@ -12,3 +12,4 @@ CONFIG_DISPLAY_BOARDINFO=y
 CONFIG_DEFAULT_DEVICE_TREE="hifive-unleashed-a00"
 CONFIG_SYS_RELOC_GD_ENV_ADDR=y
 CONFIG_DM_MTD=y
+CONFIG_OF_BOARD_FIXUP=y
-- 
2.25.1

[PATCH v3 5/5] riscv: Copy the reserved-memory nodes to final DT

[Atish Patra]

The DT used by U-Boot may be different from the DT being passed to
the OS if the DT is loaded from external media such as network or
mmc. In that case, the reserved-memory node needs to be copied to
the DT passed to the OS.

Signed-off-by: Atish Patra <atish.patra@wdc.com>
---
 arch/riscv/lib/bootm.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/riscv/lib/bootm.c b/arch/riscv/lib/bootm.c
index 5e907e96701c..1ca8370849a0 100644
--- a/arch/riscv/lib/bootm.c
+++ b/arch/riscv/lib/bootm.c
@@ -118,6 +118,11 @@ int arch_fixup_fdt(void *blob)
 	fdt_setprop_u32(blob, chosen_offset, "boot-hartid",
 			   gd->arch.boot_hart);
 
+	/* Copy the reserved-memory node to the DT used by OS */
+	err = riscv_fdt_copy_resv_mem_node(gd->fdt_blob, blob);
+	if (err < 0)
+		return err;
+
 	return 0;
 }
 
-- 
2.25.1

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

[Heinrich Schuchardt]

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@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);
>   	}
>   }
>
>

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

[Heinrich Schuchardt]

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@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);
>> ????? }
>> ? }
>>
>>
> 

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

[Atish Patra]

On Tue, Mar 17, 2020 at 3:02 PM Heinrich Schuchardt <xypron.glpk@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@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);
> >       }
> >   }
> >
> >
>


-- 
Regards,
Atish

[PATCH v3 1/5] riscv: Add boot hartid to Device tree

[Bin Meng]

On Wed, Mar 18, 2020 at 5:19 AM Atish Patra <atish.patra@wdc.com> wrote:
>
> Linux booting protocol mandates that register "a0" contains the hartid.
> However, U-boot can not pass the hartid via a0 during via standard UEFI

nits: U-Boot

remove "during" ?

> protocol. DT nodes are commonly used to pass such information to the OS.
>
> Add a DT node under chosen node to indicate the boot hartid. EFI stub
> in Linux kernel will parse this node and pass it to the real kernel
> in "a0" before jumping to it.
>
> Signed-off-by: Atish Patra <atish.patra@wdc.com>
> Reviewed-by: Rick Chen <rick@andestech.com>
> ---
>  arch/riscv/lib/bootm.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
>
> diff --git a/arch/riscv/lib/bootm.c b/arch/riscv/lib/bootm.c
> index fad16901c5f2..f927694ae32f 100644
> --- a/arch/riscv/lib/bootm.c
> +++ b/arch/riscv/lib/bootm.c
> @@ -28,6 +28,28 @@ __weak void board_quiesce_devices(void)
>
>  int arch_fixup_fdt(void *blob)
>  {
> +       u32 size;
> +       int chosen_offset, err;
> +
> +       size = fdt_totalsize(blob);
> +       err  = fdt_open_into(blob, blob, size + 32);
> +       if (err < 0) {
> +               printf("Device Tree can't be expanded to accommodate new node");
> +               return -1;

return err

> +       }
> +       chosen_offset = fdt_path_offset(blob, "/chosen");
> +       if (chosen_offset < 0) {
> +               err = fdt_add_subnode(blob, 0, "chosen");
> +               if (err < 0) {
> +                       printf("chosen node can not be added\n");
> +                       return -1;

return err

> +               }
> +       }
> +
> +       /* Overwrite the boot-hartid as U-Boot is the last state BL */

typo: stage

> +       fdt_setprop_u32(blob, chosen_offset, "boot-hartid",
> +                          gd->arch.boot_hart);
> +
>         return 0;
>  }
>
> --

Regards,
Bin

[PATCH v3 1/5] riscv: Add boot hartid to Device tree

[Bin Meng]

On Thu, Mar 19, 2020 at 11:10 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Wed, Mar 18, 2020 at 5:19 AM Atish Patra <atish.patra@wdc.com> wrote:
> >
> > Linux booting protocol mandates that register "a0" contains the hartid.
> > However, U-boot can not pass the hartid via a0 during via standard UEFI
>
> nits: U-Boot
>
> remove "during" ?
>
> > protocol. DT nodes are commonly used to pass such information to the OS.
> >
> > Add a DT node under chosen node to indicate the boot hartid. EFI stub
> > in Linux kernel will parse this node and pass it to the real kernel
> > in "a0" before jumping to it.
> >
> > Signed-off-by: Atish Patra <atish.patra@wdc.com>
> > Reviewed-by: Rick Chen <rick@andestech.com>
> > ---
> >  arch/riscv/lib/bootm.c | 22 ++++++++++++++++++++++
> >  1 file changed, 22 insertions(+)
> >
> > diff --git a/arch/riscv/lib/bootm.c b/arch/riscv/lib/bootm.c
> > index fad16901c5f2..f927694ae32f 100644
> > --- a/arch/riscv/lib/bootm.c
> > +++ b/arch/riscv/lib/bootm.c
> > @@ -28,6 +28,28 @@ __weak void board_quiesce_devices(void)
> >
> >  int arch_fixup_fdt(void *blob)
> >  {

One additional note, do have need to guard the "boot-hartid" fix-up with

#ifdef CONFIG_EFI_LOADER

#endif

?

> > +       u32 size;
> > +       int chosen_offset, err;
> > +
> > +       size = fdt_totalsize(blob);
> > +       err  = fdt_open_into(blob, blob, size + 32);
> > +       if (err < 0) {
> > +               printf("Device Tree can't be expanded to accommodate new node");
> > +               return -1;
>
> return err
>
> > +       }
> > +       chosen_offset = fdt_path_offset(blob, "/chosen");
> > +       if (chosen_offset < 0) {
> > +               err = fdt_add_subnode(blob, 0, "chosen");
> > +               if (err < 0) {
> > +                       printf("chosen node can not be added\n");
> > +                       return -1;
>
> return err
>
> > +               }
> > +       }
> > +
> > +       /* Overwrite the boot-hartid as U-Boot is the last state BL */
>
> typo: stage
>
> > +       fdt_setprop_u32(blob, chosen_offset, "boot-hartid",
> > +                          gd->arch.boot_hart);
> > +
> >         return 0;
> >  }
> >
> > --

Regards,
Bin

[PATCH v3 3/5] riscv: Provide a mechanism to fix DT for reserved memory

[Bin Meng]

On Wed, Mar 18, 2020 at 5:19 AM Atish Patra <atish.patra@wdc.com> wrote:
>
> In RISC-V, M-mode software can reserve physical memory regions
> by setting appropriate physical memory protection (PMP) csr. As the
> PMP csr are accessible only in M-mode, S-mode U-Boot can not read
> this configuration directly. However, M-mode software can pass this
> information via reserved-memory node in device tree so that S-mode
> software can access this information.
>
> This patch provides a framework to copy to the reserved-memory node
> from one DT to another. This will be used to update the DT used by
> U-Boot and the DT passed to the next stage OS.
>
> Signed-off-by: Atish Patra <atish.patra@wdc.com>
> ---
>  arch/riscv/cpu/start.S                |  1 +
>  arch/riscv/include/asm/global_data.h  |  1 +
>  arch/riscv/include/asm/u-boot-riscv.h |  1 +
>  arch/riscv/lib/asm-offsets.c          |  1 +
>  arch/riscv/lib/bootm.c                | 68 +++++++++++++++++++++++++++
>  5 files changed, 72 insertions(+)
>
> diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S
> index 6b3ff99c3882..0282685c2906 100644
> --- a/arch/riscv/cpu/start.S
> +++ b/arch/riscv/cpu/start.S
> @@ -121,6 +121,7 @@ call_board_init_f_0:
>
>         jal     board_init_f_init_reserve
>
> +       SREG    s1, GD_FIRMWARE_FDT_ADDR(gp)
>         /* save the boot hart id to global_data */
>         SREG    tp, GD_BOOT_HART(gp)
>
> diff --git a/arch/riscv/include/asm/global_data.h b/arch/riscv/include/asm/global_data.h
> index b74bd7e738bb..51ac8d1c98e2 100644
> --- a/arch/riscv/include/asm/global_data.h
> +++ b/arch/riscv/include/asm/global_data.h
> @@ -15,6 +15,7 @@
>  /* Architecture-specific global data */
>  struct arch_global_data {
>         long boot_hart;         /* boot hart id */
> +       phys_addr_t firmware_fdt_addr;
>  #ifdef CONFIG_SIFIVE_CLINT
>         void __iomem *clint;    /* clint base address */
>  #endif
> diff --git a/arch/riscv/include/asm/u-boot-riscv.h b/arch/riscv/include/asm/u-boot-riscv.h
> index 49febd588102..b7bea0ba184d 100644
> --- a/arch/riscv/include/asm/u-boot-riscv.h
> +++ b/arch/riscv/include/asm/u-boot-riscv.h
> @@ -17,5 +17,6 @@ int cleanup_before_linux(void);
>  /* board/.../... */
>  int board_init(void);
>  void board_quiesce_devices(void);
> +int riscv_board_reserved_mem_fixup(void *fdt);
>
>  #endif /* _U_BOOT_RISCV_H_ */
> diff --git a/arch/riscv/lib/asm-offsets.c b/arch/riscv/lib/asm-offsets.c
> index 4fa4fd371473..7301c1b98e23 100644
> --- a/arch/riscv/lib/asm-offsets.c
> +++ b/arch/riscv/lib/asm-offsets.c
> @@ -14,6 +14,7 @@
>  int main(void)
>  {
>         DEFINE(GD_BOOT_HART, offsetof(gd_t, arch.boot_hart));
> +       DEFINE(GD_FIRMWARE_FDT_ADDR, offsetof(gd_t, arch.firmware_fdt_addr));
>  #ifndef CONFIG_XIP
>         DEFINE(GD_AVAILABLE_HARTS, offsetof(gd_t, arch.available_harts));
>  #endif
> diff --git a/arch/riscv/lib/bootm.c b/arch/riscv/lib/bootm.c
> index f927694ae32f..5e907e96701c 100644
> --- a/arch/riscv/lib/bootm.c
> +++ b/arch/riscv/lib/bootm.c
> @@ -19,6 +19,7 @@
>  #include <dm/device.h>
>  #include <dm/root.h>
>  #include <u-boot/zlib.h>
> +#include <mapmem.h>
>
>  DECLARE_GLOBAL_DATA_PTR;
>
> @@ -26,6 +27,73 @@ __weak void board_quiesce_devices(void)
>  {
>  }
>
> +int riscv_fdt_copy_resv_mem_node(const void *src, void *dst)

We probably need find a better place for this routine, see below
comments for riscv_board_reserved_mem_fixup().

We need make this routine public in u-boot-riscv.h as well.

> +{
> +       uint32_t phandle;
> +       struct fdt_memory pmp_mem;
> +       fdt_addr_t addr;
> +       fdt_size_t size;
> +       int offset, node, err, rmem_offset;
> +       bool nomap = false;
> +       char basename[32] = {0};
> +       int bname_len;
> +       int max_len = sizeof(basename);
> +       const char *name;
> +       char *temp;
> +
> +       offset = fdt_path_offset(src, "/reserved-memory");
> +       if (offset < 0) {
> +               printf("No reserved memory region found in source FDT\n");
> +               return 0;
> +       }
> +
> +       fdt_for_each_subnode(node, src, offset) {
> +               name = fdt_get_name(src, node, NULL);
> +
> +               addr = fdtdec_get_addr_size_auto_noparent(src, node,
> +                                                         "reg", 0, &size,
> +                                                         true);
> +               if (addr == FDT_ADDR_T_NONE) {
> +                       debug("failed to read address/size for %s\n", name);
> +                       continue;
> +               }
> +               strncpy(basename, name, max_len);
> +               temp = strchr(basename, '@');
> +               if (temp) {
> +                       bname_len = strnlen(basename, max_len) - strnlen(temp,
> +                                                                      max_len);
> +                       *(basename+bname_len) = '\0';

nits: need space around +

> +               }
> +               pmp_mem.start = addr;
> +               pmp_mem.end = addr + size;
> +               err = fdtdec_add_reserved_memory(dst, basename, &pmp_mem,
> +                                                &phandle);
> +               if (err < 0) {
> +                       printf("failed to add reserved memory: %d\n", err);
> +                       return err;
> +               }
> +               rmem_offset = fdt_node_offset_by_phandle(dst, phandle);

nits: this line can be put in the "if (nomap)" block below

> +               nomap = fdt_get_property(src, node, "no-map", NULL);
> +               if (nomap)
> +                       fdt_setprop_empty(dst, rmem_offset, "no-map");
> +       }
> +
> +       return 0;
> +}
> +
> +int riscv_board_reserved_mem_fixup(void *fdt)

This routine should not be put to boom.c as it's nothing related to
bootm. Maybe arch/riscv/lib/fdt.c ?



> +{
> +       int err;
> +       void *src_fdt_addr;
> +
> +       src_fdt_addr = map_sysmem(gd->arch.firmware_fdt_addr, 0);
> +       err = riscv_fdt_copy_resv_mem_node(src_fdt_addr, fdt);
> +       if (err < 0)
> +               return err;
> +
> +       return 0;
> +}
> +
>  int arch_fixup_fdt(void *blob)
>  {
>         u32 size;
> --
> 2.25.1
>

[PATCH v3 4/5] riscv: Setup reserved-memory node for FU540

[Bin Meng]

On Wed, Mar 18, 2020 at 5:19 AM Atish Patra <atish.patra@wdc.com> wrote:
>
> FU540 uses OF_SEPARATE instead of OF_PRIOR.
>
> Enable OF_BOARD_FIXUP to update the DT with reserved-memory node.
>
> Signed-off-by: Atish Patra <atish.patra@wdc.com>
> ---
>  board/sifive/fu540/fu540.c     | 15 +++++++++++++++
>  configs/sifive_fu540_defconfig |  1 +
>  2 files changed, 16 insertions(+)
>
> diff --git a/board/sifive/fu540/fu540.c b/board/sifive/fu540/fu540.c
> index 47a20902517c..82b3a9c8e729 100644
> --- a/board/sifive/fu540/fu540.c
> +++ b/board/sifive/fu540/fu540.c
> @@ -141,6 +141,21 @@ int misc_init_r(void)
>
>  #endif
>
> +#ifdef CONFIG_OF_BOARD_FIXUP
> +int board_fix_fdt(void *fdt)

This routine should be put in a more generic file, as this could
potentially apply to all RISC-V platforms that need OF_BOARD_FIXUP
(e.g.: U-Boot itself is built with OF_SEPARATE).

In case other platform wants to override this, we can define it as a __weak.

> +{
> +       int err;
> +
> +       err = riscv_board_reserved_mem_fixup(fdt);
> +       if (err < 0) {
> +               printf("failed to fixup DT for reserved memory: %d\n", err);
> +               return err;
> +       }
> +
> +       return 0;
> +}
> +#endif
> +
>  int board_init(void)
>  {
>         /* For now nothing to do here. */
> diff --git a/configs/sifive_fu540_defconfig b/configs/sifive_fu540_defconfig
> index 6d61e6c960ee..8fb3794cd578 100644
> --- a/configs/sifive_fu540_defconfig
> +++ b/configs/sifive_fu540_defconfig
> @@ -12,3 +12,4 @@ CONFIG_DISPLAY_BOARDINFO=y
>  CONFIG_DEFAULT_DEVICE_TREE="hifive-unleashed-a00"
>  CONFIG_SYS_RELOC_GD_ENV_ADDR=y
>  CONFIG_DM_MTD=y
> +CONFIG_OF_BOARD_FIXUP=y

This line should be inserted after CONFIG_DISPLAY_BOARDINFO=y

Please ensure defconfig file is updated like this:

$ make sifive_fu540_defconfig
$ make savedefconfig
$ cp defconfig configs/sifive_fu540_defconfig

Regards,
Bin

[PATCH v3 5/5] riscv: Copy the reserved-memory nodes to final DT

[Bin Meng]

On Wed, Mar 18, 2020 at 5:19 AM Atish Patra <atish.patra@wdc.com> wrote:
>
> The DT used by U-Boot may be different from the DT being passed to
> the OS if the DT is loaded from external media such as network or
> mmc. In that case, the reserved-memory node needs to be copied to
> the DT passed to the OS.
>
> Signed-off-by: Atish Patra <atish.patra@wdc.com>
> ---
>  arch/riscv/lib/bootm.c | 5 +++++
>  1 file changed, 5 insertions(+)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

[PATCH v3 1/5] riscv: Add boot hartid to Device tree

[Atish Patra]

On Wed, Mar 18, 2020 at 8:14 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Thu, Mar 19, 2020 at 11:10 AM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > On Wed, Mar 18, 2020 at 5:19 AM Atish Patra <atish.patra@wdc.com> wrote:
> > >
> > > Linux booting protocol mandates that register "a0" contains the hartid.
> > > However, U-boot can not pass the hartid via a0 during via standard UEFI
> >
> > nits: U-Boot
> >
> > remove "during" ?
> >
> > > protocol. DT nodes are commonly used to pass such information to the OS.
> > >
> > > Add a DT node under chosen node to indicate the boot hartid. EFI stub
> > > in Linux kernel will parse this node and pass it to the real kernel
> > > in "a0" before jumping to it.
> > >
> > > Signed-off-by: Atish Patra <atish.patra@wdc.com>
> > > Reviewed-by: Rick Chen <rick@andestech.com>
> > > ---
> > >  arch/riscv/lib/bootm.c | 22 ++++++++++++++++++++++
> > >  1 file changed, 22 insertions(+)
> > >
> > > diff --git a/arch/riscv/lib/bootm.c b/arch/riscv/lib/bootm.c
> > > index fad16901c5f2..f927694ae32f 100644
> > > --- a/arch/riscv/lib/bootm.c
> > > +++ b/arch/riscv/lib/bootm.c
> > > @@ -28,6 +28,28 @@ __weak void board_quiesce_devices(void)
> > >
> > >  int arch_fixup_fdt(void *blob)
> > >  {
>
> One additional note, do have need to guard the "boot-hartid" fix-up with
>
> #ifdef CONFIG_EFI_LOADER
>
> #endif
>

Good point. Thanks I will add it.

> ?
>
> > > +       u32 size;
> > > +       int chosen_offset, err;
> > > +
> > > +       size = fdt_totalsize(blob);
> > > +       err  = fdt_open_into(blob, blob, size + 32);
> > > +       if (err < 0) {
> > > +               printf("Device Tree can't be expanded to accommodate new node");
> > > +               return -1;
> >
> > return err
> >

Sure.

> > > +       }
> > > +       chosen_offset = fdt_path_offset(blob, "/chosen");
> > > +       if (chosen_offset < 0) {
> > > +               err = fdt_add_subnode(blob, 0, "chosen");
> > > +               if (err < 0) {
> > > +                       printf("chosen node can not be added\n");
> > > +                       return -1;
> >
> > return err
> >

Sure.

> > > +               }
> > > +       }
> > > +
> > > +       /* Overwrite the boot-hartid as U-Boot is the last state BL */
> >
> > typo: stage
> >
> > > +       fdt_setprop_u32(blob, chosen_offset, "boot-hartid",
> > > +                          gd->arch.boot_hart);
> > > +
> > >         return 0;
> > >  }
> > >
> > > --
>
> Regards,
> Bin

I will address all other typos as well.

-- 
Regards,
Atish

[PATCH v3 3/5] riscv: Provide a mechanism to fix DT for reserved memory

[Atish Patra]

On Thu, Mar 19, 2020 at 7:31 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Wed, Mar 18, 2020 at 5:19 AM Atish Patra <atish.patra@wdc.com> wrote:
> >
> > In RISC-V, M-mode software can reserve physical memory regions
> > by setting appropriate physical memory protection (PMP) csr. As the
> > PMP csr are accessible only in M-mode, S-mode U-Boot can not read
> > this configuration directly. However, M-mode software can pass this
> > information via reserved-memory node in device tree so that S-mode
> > software can access this information.
> >
> > This patch provides a framework to copy to the reserved-memory node
> > from one DT to another. This will be used to update the DT used by
> > U-Boot and the DT passed to the next stage OS.
> >
> > Signed-off-by: Atish Patra <atish.patra@wdc.com>
> > ---
> >  arch/riscv/cpu/start.S                |  1 +
> >  arch/riscv/include/asm/global_data.h  |  1 +
> >  arch/riscv/include/asm/u-boot-riscv.h |  1 +
> >  arch/riscv/lib/asm-offsets.c          |  1 +
> >  arch/riscv/lib/bootm.c                | 68 +++++++++++++++++++++++++++
> >  5 files changed, 72 insertions(+)
> >
> > diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S
> > index 6b3ff99c3882..0282685c2906 100644
> > --- a/arch/riscv/cpu/start.S
> > +++ b/arch/riscv/cpu/start.S
> > @@ -121,6 +121,7 @@ call_board_init_f_0:
> >
> >         jal     board_init_f_init_reserve
> >
> > +       SREG    s1, GD_FIRMWARE_FDT_ADDR(gp)
> >         /* save the boot hart id to global_data */
> >         SREG    tp, GD_BOOT_HART(gp)
> >
> > diff --git a/arch/riscv/include/asm/global_data.h b/arch/riscv/include/asm/global_data.h
> > index b74bd7e738bb..51ac8d1c98e2 100644
> > --- a/arch/riscv/include/asm/global_data.h
> > +++ b/arch/riscv/include/asm/global_data.h
> > @@ -15,6 +15,7 @@
> >  /* Architecture-specific global data */
> >  struct arch_global_data {
> >         long boot_hart;         /* boot hart id */
> > +       phys_addr_t firmware_fdt_addr;
> >  #ifdef CONFIG_SIFIVE_CLINT
> >         void __iomem *clint;    /* clint base address */
> >  #endif
> > diff --git a/arch/riscv/include/asm/u-boot-riscv.h b/arch/riscv/include/asm/u-boot-riscv.h
> > index 49febd588102..b7bea0ba184d 100644
> > --- a/arch/riscv/include/asm/u-boot-riscv.h
> > +++ b/arch/riscv/include/asm/u-boot-riscv.h
> > @@ -17,5 +17,6 @@ int cleanup_before_linux(void);
> >  /* board/.../... */
> >  int board_init(void);
> >  void board_quiesce_devices(void);
> > +int riscv_board_reserved_mem_fixup(void *fdt);
> >
> >  #endif /* _U_BOOT_RISCV_H_ */
> > diff --git a/arch/riscv/lib/asm-offsets.c b/arch/riscv/lib/asm-offsets.c
> > index 4fa4fd371473..7301c1b98e23 100644
> > --- a/arch/riscv/lib/asm-offsets.c
> > +++ b/arch/riscv/lib/asm-offsets.c
> > @@ -14,6 +14,7 @@
> >  int main(void)
> >  {
> >         DEFINE(GD_BOOT_HART, offsetof(gd_t, arch.boot_hart));
> > +       DEFINE(GD_FIRMWARE_FDT_ADDR, offsetof(gd_t, arch.firmware_fdt_addr));
> >  #ifndef CONFIG_XIP
> >         DEFINE(GD_AVAILABLE_HARTS, offsetof(gd_t, arch.available_harts));
> >  #endif
> > diff --git a/arch/riscv/lib/bootm.c b/arch/riscv/lib/bootm.c
> > index f927694ae32f..5e907e96701c 100644
> > --- a/arch/riscv/lib/bootm.c
> > +++ b/arch/riscv/lib/bootm.c
> > @@ -19,6 +19,7 @@
> >  #include <dm/device.h>
> >  #include <dm/root.h>
> >  #include <u-boot/zlib.h>
> > +#include <mapmem.h>
> >
> >  DECLARE_GLOBAL_DATA_PTR;
> >
> > @@ -26,6 +27,73 @@ __weak void board_quiesce_devices(void)
> >  {
> >  }
> >
> > +int riscv_fdt_copy_resv_mem_node(const void *src, void *dst)
>
> We probably need find a better place for this routine, see below
> comments for riscv_board_reserved_mem_fixup().
>
> We need make this routine public in u-boot-riscv.h as well.
>
> > +{
> > +       uint32_t phandle;
> > +       struct fdt_memory pmp_mem;
> > +       fdt_addr_t addr;
> > +       fdt_size_t size;
> > +       int offset, node, err, rmem_offset;
> > +       bool nomap = false;
> > +       char basename[32] = {0};
> > +       int bname_len;
> > +       int max_len = sizeof(basename);
> > +       const char *name;
> > +       char *temp;
> > +
> > +       offset = fdt_path_offset(src, "/reserved-memory");
> > +       if (offset < 0) {
> > +               printf("No reserved memory region found in source FDT\n");
> > +               return 0;
> > +       }
> > +
> > +       fdt_for_each_subnode(node, src, offset) {
> > +               name = fdt_get_name(src, node, NULL);
> > +
> > +               addr = fdtdec_get_addr_size_auto_noparent(src, node,
> > +                                                         "reg", 0, &size,
> > +                                                         true);
> > +               if (addr == FDT_ADDR_T_NONE) {
> > +                       debug("failed to read address/size for %s\n", name);
> > +                       continue;
> > +               }
> > +               strncpy(basename, name, max_len);
> > +               temp = strchr(basename, '@');
> > +               if (temp) {
> > +                       bname_len = strnlen(basename, max_len) - strnlen(temp,
> > +                                                                      max_len);
> > +                       *(basename+bname_len) = '\0';
>
> nits: need space around +
>
sure.

> > +               }
> > +               pmp_mem.start = addr;
> > +               pmp_mem.end = addr + size;
> > +               err = fdtdec_add_reserved_memory(dst, basename, &pmp_mem,
> > +                                                &phandle);
> > +               if (err < 0) {
> > +                       printf("failed to add reserved memory: %d\n", err);
> > +                       return err;
> > +               }
> > +               rmem_offset = fdt_node_offset_by_phandle(dst, phandle);
>
> nits: this line can be put in the "if (nomap)" block below
>

sure.

> > +               nomap = fdt_get_property(src, node, "no-map", NULL);
> > +               if (nomap)
> > +                       fdt_setprop_empty(dst, rmem_offset, "no-map");
> > +       }
> > +
> > +       return 0;
> > +}
> > +
> > +int riscv_board_reserved_mem_fixup(void *fdt)
>
> This routine should not be put to boom.c as it's nothing related to
> bootm. Maybe arch/riscv/lib/fdt.c ?
>

I wanted to keep all the fdt fixups together  as arch_fixup_fdt was
already defined in bootm.c
But I agree with your point that it is not related to bootm. I think
we can move all the fdt fixups
including arch_fixup_fdt to arch/riscv/lib/fdt_fixup.c. Thoughts ?

>
>
> > +{
> > +       int err;
> > +       void *src_fdt_addr;
> > +
> > +       src_fdt_addr = map_sysmem(gd->arch.firmware_fdt_addr, 0);
> > +       err = riscv_fdt_copy_resv_mem_node(src_fdt_addr, fdt);
> > +       if (err < 0)
> > +               return err;
> > +
> > +       return 0;
> > +}
> > +
> >  int arch_fixup_fdt(void *blob)
> >  {
> >         u32 size;
> > --
> > 2.25.1
> >



-- 
Regards,
Atish

[PATCH v3 4/5] riscv: Setup reserved-memory node for FU540

[Atish Patra]

On Thu, Mar 19, 2020 at 7:32 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Wed, Mar 18, 2020 at 5:19 AM Atish Patra <atish.patra@wdc.com> wrote:
> >
> > FU540 uses OF_SEPARATE instead of OF_PRIOR.
> >
> > Enable OF_BOARD_FIXUP to update the DT with reserved-memory node.
> >
> > Signed-off-by: Atish Patra <atish.patra@wdc.com>
> > ---
> >  board/sifive/fu540/fu540.c     | 15 +++++++++++++++
> >  configs/sifive_fu540_defconfig |  1 +
> >  2 files changed, 16 insertions(+)
> >
> > diff --git a/board/sifive/fu540/fu540.c b/board/sifive/fu540/fu540.c
> > index 47a20902517c..82b3a9c8e729 100644
> > --- a/board/sifive/fu540/fu540.c
> > +++ b/board/sifive/fu540/fu540.c
> > @@ -141,6 +141,21 @@ int misc_init_r(void)
> >
> >  #endif
> >
> > +#ifdef CONFIG_OF_BOARD_FIXUP
> > +int board_fix_fdt(void *fdt)
>
> This routine should be put in a more generic file, as this could
> potentially apply to all RISC-V platforms that need OF_BOARD_FIXUP
> (e.g.: U-Boot itself is built with OF_SEPARATE).
>
> In case other platform wants to override this, we can define it as a __weak.
>
I am not opposed to that idea but board specific functions should be
defined in board specific file.
If we can violate that rule, I am okay with the proposal.

We can define a __weak board_fix_fdt in arch/riscv/lib/fdt_fixup.c and
guard it under CONFIG_OF_BOARD_FIXUP.

> > +{
> > +       int err;
> > +
> > +       err = riscv_board_reserved_mem_fixup(fdt);
> > +       if (err < 0) {
> > +               printf("failed to fixup DT for reserved memory: %d\n", err);
> > +               return err;
> > +       }
> > +
> > +       return 0;
> > +}
> > +#endif
> > +
> >  int board_init(void)
> >  {
> >         /* For now nothing to do here. */
> > diff --git a/configs/sifive_fu540_defconfig b/configs/sifive_fu540_defconfig
> > index 6d61e6c960ee..8fb3794cd578 100644
> > --- a/configs/sifive_fu540_defconfig
> > +++ b/configs/sifive_fu540_defconfig
> > @@ -12,3 +12,4 @@ CONFIG_DISPLAY_BOARDINFO=y
> >  CONFIG_DEFAULT_DEVICE_TREE="hifive-unleashed-a00"
> >  CONFIG_SYS_RELOC_GD_ENV_ADDR=y
> >  CONFIG_DM_MTD=y
> > +CONFIG_OF_BOARD_FIXUP=y
>
> This line should be inserted after CONFIG_DISPLAY_BOARDINFO=y
>
> Please ensure defconfig file is updated like this:
>
> $ make sifive_fu540_defconfig
> $ make savedefconfig
> $ cp defconfig configs/sifive_fu540_defconfig
>

Sure. Will do that.

> Regards,
> Bin



-- 
Regards,
Atish

[PATCH v3 4/5] riscv: Setup reserved-memory node for FU540

[Bin Meng]

On Fri, Mar 20, 2020 at 4:57 AM Atish Patra <atishp@atishpatra.org> wrote:
>
> On Thu, Mar 19, 2020 at 7:32 AM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > On Wed, Mar 18, 2020 at 5:19 AM Atish Patra <atish.patra@wdc.com> wrote:
> > >
> > > FU540 uses OF_SEPARATE instead of OF_PRIOR.
> > >
> > > Enable OF_BOARD_FIXUP to update the DT with reserved-memory node.
> > >
> > > Signed-off-by: Atish Patra <atish.patra@wdc.com>
> > > ---
> > >  board/sifive/fu540/fu540.c     | 15 +++++++++++++++
> > >  configs/sifive_fu540_defconfig |  1 +
> > >  2 files changed, 16 insertions(+)
> > >
> > > diff --git a/board/sifive/fu540/fu540.c b/board/sifive/fu540/fu540.c
> > > index 47a20902517c..82b3a9c8e729 100644
> > > --- a/board/sifive/fu540/fu540.c
> > > +++ b/board/sifive/fu540/fu540.c
> > > @@ -141,6 +141,21 @@ int misc_init_r(void)
> > >
> > >  #endif
> > >
> > > +#ifdef CONFIG_OF_BOARD_FIXUP
> > > +int board_fix_fdt(void *fdt)
> >
> > This routine should be put in a more generic file, as this could
> > potentially apply to all RISC-V platforms that need OF_BOARD_FIXUP
> > (e.g.: U-Boot itself is built with OF_SEPARATE).
> >
> > In case other platform wants to override this, we can define it as a __weak.
> >
> I am not opposed to that idea but board specific functions should be
> defined in board specific file.
> If we can violate that rule, I am okay with the proposal.

Probably we need a new option for this kind of fix-up, not
CONFIG_OF_BOARD_FIXUP that suggests it should be put in a board codes,
as it is really that board-specific.

>
> We can define a __weak board_fix_fdt in arch/riscv/lib/fdt_fixup.c and
> guard it under CONFIG_OF_BOARD_FIXUP.

Yep for now this looks good.

>
> > > +{
> > > +       int err;
> > > +
> > > +       err = riscv_board_reserved_mem_fixup(fdt);
> > > +       if (err < 0) {
> > > +               printf("failed to fixup DT for reserved memory: %d\n", err);
> > > +               return err;
> > > +       }
> > > +
> > > +       return 0;
> > > +}
> > > +#endif
> > > +
> > >  int board_init(void)
> > >  {
> > >         /* For now nothing to do here. */
> > > diff --git a/configs/sifive_fu540_defconfig b/configs/sifive_fu540_defconfig
> > > index 6d61e6c960ee..8fb3794cd578 100644
> > > --- a/configs/sifive_fu540_defconfig
> > > +++ b/configs/sifive_fu540_defconfig
> > > @@ -12,3 +12,4 @@ CONFIG_DISPLAY_BOARDINFO=y
> > >  CONFIG_DEFAULT_DEVICE_TREE="hifive-unleashed-a00"
> > >  CONFIG_SYS_RELOC_GD_ENV_ADDR=y
> > >  CONFIG_DM_MTD=y
> > > +CONFIG_OF_BOARD_FIXUP=y
> >
> > This line should be inserted after CONFIG_DISPLAY_BOARDINFO=y
> >
> > Please ensure defconfig file is updated like this:
> >
> > $ make sifive_fu540_defconfig
> > $ make savedefconfig
> > $ cp defconfig configs/sifive_fu540_defconfig
> >
>
> Sure. Will do that.

Regards,
Bin

[PATCH v3 3/5] riscv: Provide a mechanism to fix DT for reserved memory

[Bin Meng]

On Fri, Mar 20, 2020 at 4:46 AM Atish Patra <atishp@atishpatra.org> wrote:
>
> On Thu, Mar 19, 2020 at 7:31 AM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > On Wed, Mar 18, 2020 at 5:19 AM Atish Patra <atish.patra@wdc.com> wrote:
> > >
> > > In RISC-V, M-mode software can reserve physical memory regions
> > > by setting appropriate physical memory protection (PMP) csr. As the
> > > PMP csr are accessible only in M-mode, S-mode U-Boot can not read
> > > this configuration directly. However, M-mode software can pass this
> > > information via reserved-memory node in device tree so that S-mode
> > > software can access this information.
> > >
> > > This patch provides a framework to copy to the reserved-memory node
> > > from one DT to another. This will be used to update the DT used by
> > > U-Boot and the DT passed to the next stage OS.
> > >
> > > Signed-off-by: Atish Patra <atish.patra@wdc.com>
> > > ---
> > >  arch/riscv/cpu/start.S                |  1 +
> > >  arch/riscv/include/asm/global_data.h  |  1 +
> > >  arch/riscv/include/asm/u-boot-riscv.h |  1 +
> > >  arch/riscv/lib/asm-offsets.c          |  1 +
> > >  arch/riscv/lib/bootm.c                | 68 +++++++++++++++++++++++++++
> > >  5 files changed, 72 insertions(+)
> > >
> > > diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S
> > > index 6b3ff99c3882..0282685c2906 100644
> > > --- a/arch/riscv/cpu/start.S
> > > +++ b/arch/riscv/cpu/start.S
> > > @@ -121,6 +121,7 @@ call_board_init_f_0:
> > >
> > >         jal     board_init_f_init_reserve
> > >
> > > +       SREG    s1, GD_FIRMWARE_FDT_ADDR(gp)
> > >         /* save the boot hart id to global_data */
> > >         SREG    tp, GD_BOOT_HART(gp)
> > >
> > > diff --git a/arch/riscv/include/asm/global_data.h b/arch/riscv/include/asm/global_data.h
> > > index b74bd7e738bb..51ac8d1c98e2 100644
> > > --- a/arch/riscv/include/asm/global_data.h
> > > +++ b/arch/riscv/include/asm/global_data.h
> > > @@ -15,6 +15,7 @@
> > >  /* Architecture-specific global data */
> > >  struct arch_global_data {
> > >         long boot_hart;         /* boot hart id */
> > > +       phys_addr_t firmware_fdt_addr;
> > >  #ifdef CONFIG_SIFIVE_CLINT
> > >         void __iomem *clint;    /* clint base address */
> > >  #endif
> > > diff --git a/arch/riscv/include/asm/u-boot-riscv.h b/arch/riscv/include/asm/u-boot-riscv.h
> > > index 49febd588102..b7bea0ba184d 100644
> > > --- a/arch/riscv/include/asm/u-boot-riscv.h
> > > +++ b/arch/riscv/include/asm/u-boot-riscv.h
> > > @@ -17,5 +17,6 @@ int cleanup_before_linux(void);
> > >  /* board/.../... */
> > >  int board_init(void);
> > >  void board_quiesce_devices(void);
> > > +int riscv_board_reserved_mem_fixup(void *fdt);
> > >
> > >  #endif /* _U_BOOT_RISCV_H_ */
> > > diff --git a/arch/riscv/lib/asm-offsets.c b/arch/riscv/lib/asm-offsets.c
> > > index 4fa4fd371473..7301c1b98e23 100644
> > > --- a/arch/riscv/lib/asm-offsets.c
> > > +++ b/arch/riscv/lib/asm-offsets.c
> > > @@ -14,6 +14,7 @@
> > >  int main(void)
> > >  {
> > >         DEFINE(GD_BOOT_HART, offsetof(gd_t, arch.boot_hart));
> > > +       DEFINE(GD_FIRMWARE_FDT_ADDR, offsetof(gd_t, arch.firmware_fdt_addr));
> > >  #ifndef CONFIG_XIP
> > >         DEFINE(GD_AVAILABLE_HARTS, offsetof(gd_t, arch.available_harts));
> > >  #endif
> > > diff --git a/arch/riscv/lib/bootm.c b/arch/riscv/lib/bootm.c
> > > index f927694ae32f..5e907e96701c 100644
> > > --- a/arch/riscv/lib/bootm.c
> > > +++ b/arch/riscv/lib/bootm.c
> > > @@ -19,6 +19,7 @@
> > >  #include <dm/device.h>
> > >  #include <dm/root.h>
> > >  #include <u-boot/zlib.h>
> > > +#include <mapmem.h>
> > >
> > >  DECLARE_GLOBAL_DATA_PTR;
> > >
> > > @@ -26,6 +27,73 @@ __weak void board_quiesce_devices(void)
> > >  {
> > >  }
> > >
> > > +int riscv_fdt_copy_resv_mem_node(const void *src, void *dst)
> >
> > We probably need find a better place for this routine, see below
> > comments for riscv_board_reserved_mem_fixup().
> >
> > We need make this routine public in u-boot-riscv.h as well.
> >
> > > +{
> > > +       uint32_t phandle;
> > > +       struct fdt_memory pmp_mem;
> > > +       fdt_addr_t addr;
> > > +       fdt_size_t size;
> > > +       int offset, node, err, rmem_offset;
> > > +       bool nomap = false;
> > > +       char basename[32] = {0};
> > > +       int bname_len;
> > > +       int max_len = sizeof(basename);
> > > +       const char *name;
> > > +       char *temp;
> > > +
> > > +       offset = fdt_path_offset(src, "/reserved-memory");
> > > +       if (offset < 0) {
> > > +               printf("No reserved memory region found in source FDT\n");
> > > +               return 0;
> > > +       }
> > > +
> > > +       fdt_for_each_subnode(node, src, offset) {
> > > +               name = fdt_get_name(src, node, NULL);
> > > +
> > > +               addr = fdtdec_get_addr_size_auto_noparent(src, node,
> > > +                                                         "reg", 0, &size,
> > > +                                                         true);
> > > +               if (addr == FDT_ADDR_T_NONE) {
> > > +                       debug("failed to read address/size for %s\n", name);
> > > +                       continue;
> > > +               }
> > > +               strncpy(basename, name, max_len);
> > > +               temp = strchr(basename, '@');
> > > +               if (temp) {
> > > +                       bname_len = strnlen(basename, max_len) - strnlen(temp,
> > > +                                                                      max_len);
> > > +                       *(basename+bname_len) = '\0';
> >
> > nits: need space around +
> >
> sure.
>
> > > +               }
> > > +               pmp_mem.start = addr;
> > > +               pmp_mem.end = addr + size;
> > > +               err = fdtdec_add_reserved_memory(dst, basename, &pmp_mem,
> > > +                                                &phandle);
> > > +               if (err < 0) {
> > > +                       printf("failed to add reserved memory: %d\n", err);
> > > +                       return err;
> > > +               }
> > > +               rmem_offset = fdt_node_offset_by_phandle(dst, phandle);
> >
> > nits: this line can be put in the "if (nomap)" block below
> >
>
> sure.
>
> > > +               nomap = fdt_get_property(src, node, "no-map", NULL);
> > > +               if (nomap)
> > > +                       fdt_setprop_empty(dst, rmem_offset, "no-map");
> > > +       }
> > > +
> > > +       return 0;
> > > +}
> > > +
> > > +int riscv_board_reserved_mem_fixup(void *fdt)
> >
> > This routine should not be put to boom.c as it's nothing related to
> > bootm. Maybe arch/riscv/lib/fdt.c ?
> >
>
> I wanted to keep all the fdt fixups together  as arch_fixup_fdt was
> already defined in bootm.c
> But I agree with your point that it is not related to bootm. I think
> we can move all the fdt fixups
> including arch_fixup_fdt to arch/riscv/lib/fdt_fixup.c. Thoughts ?
>

Sounds good to me.

Regards,
Bin