[PATCH v2 0/4] DT related fixes for RISC-V UEFI

[PATCH v2 0/4] 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 provide one of the option to update reserved-memory node for Linux.
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 indepedently 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 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 (4):
riscv: Add boot hartid to Device tree
cmd: bootefi: Parse reserved-memory node from DT
riscv: Provide a mechanism for riscv boards to parse reserved memory
riscv: Setup reserved-memory node for FU540

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                | 59 +++++++++++++++++++++++++++
board/sifive/fu540/fu540.c            | 15 +++++++
cmd/bootefi.c                         | 42 +++++++++++++++----
configs/sifive_fu540_defconfig        |  1 +
8 files changed, 112 insertions(+), 9 deletions(-)

--
2.25.1

[PATCH v2 1/4] 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 v2 2/4] 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 | 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");
+				continue;
+			}
+			efi_reserve_memory(addr, size);
+			subnode = fdt_next_subnode(fdt, subnode);
+		}
 	}
 }
 
-- 
2.25.1

[PATCH v2 3/4] riscv: Provide a mechanism for riscv boards to parse 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.

In U-boot, any board may use the DT in following ways.
1. OF_SEPARTE: It ignores the DT from previous stage and uses the DT
from U-Boot sources.
2. OF_PRIOR_STATE: It reuses the DT from previous stage.
For case 1: U-Boot needs to parse the reserved-memory node from the
DT passed from the previous stage and update the DT in use.

This patch provides a framework to do that from any RISC-V boards.

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                | 37 +++++++++++++++++++++++++++
 5 files changed, 41 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..3a4d0bf14c86 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,42 @@ __weak void board_quiesce_devices(void)
 {
 }
 
+int riscv_board_reserved_mem_fixup(void *fdt)
+{
+	uint32_t phandle;
+	struct fdt_memory pmp_mem;
+	int err;
+	void *src_fdt_addr;
+	int offset, node;
+	phys_addr_t addr, size;
+
+	src_fdt_addr = map_sysmem(gd->arch.firmware_fdt_addr, 0);
+	offset = fdt_path_offset(src_fdt_addr, "/reserved-memory");
+	if (offset < 0) {
+		printf("No reserved memory region found in FDT\n");
+		return offset;
+	}
+
+	fdt_for_each_subnode(node, src_fdt_addr, offset) {
+		const char *name = fdt_get_name(src_fdt_addr, node, NULL);
+
+		addr = fdtdec_get_addr_size(src_fdt_addr, node, "reg", &size);
+		if (addr == FDT_ADDR_T_NONE) {
+			debug("failed to read address/size for %s\n", name);
+			continue;
+		}
+		pmp_mem.start = addr;
+		pmp_mem.end = addr + size;
+		err = fdtdec_add_reserved_memory(fdt, name, &pmp_mem, &phandle);
+		if (err < 0) {
+			printf("failed to add reserved memory: %d\n", err);
+			return err;
+		}
+	}
+
+	return 0;
+}
+
 int arch_fixup_fdt(void *blob)
 {
 	u32 size;
-- 
2.25.1

[PATCH v2 4/4] 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 v2 0/4] DT related fixes for RISC-V UEFI

[Atish Patra]

On Fri, Mar 13, 2020 at 5:11 PM Atish Patra <atish.patra@wdc.com> wrote:
>
> 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 provide one of the option to update reserved-memory node for Linux.
> 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 indepedently 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 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 (4):
> riscv: Add boot hartid to Device tree
> cmd: bootefi: Parse reserved-memory node from DT
> riscv: Provide a mechanism for riscv boards to parse reserved memory
> riscv: Setup reserved-memory node for FU540
>
> 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                | 59 +++++++++++++++++++++++++++
> board/sifive/fu540/fu540.c            | 15 +++++++
> cmd/bootefi.c                         | 42 +++++++++++++++----
> configs/sifive_fu540_defconfig        |  1 +
> 8 files changed, 112 insertions(+), 9 deletions(-)
>
> --
> 2.25.1
>

Fixed palmer's email address. Sorry for the spam.
-- 
Regards,
Atish

[PATCH v2 1/4] riscv: Add boot hartid to Device tree

[Atish Patra]

On Fri, Mar 13, 2020 at 5:12 PM 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
> 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
>

Fixed palmer's email address. Sorry for the spam.
-- 
Regards,
Atish

[PATCH v2 3/4] riscv: Provide a mechanism for riscv boards to parse reserved memory

[Atish Patra]

On Fri, Mar 13, 2020 at 5:12 PM 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.
>
> In U-boot, any board may use the DT in following ways.
> 1. OF_SEPARTE: It ignores the DT from previous stage and uses the DT
> from U-Boot sources.
> 2. OF_PRIOR_STATE: It reuses the DT from previous stage.
> For case 1: U-Boot needs to parse the reserved-memory node from the
> DT passed from the previous stage and update the DT in use.
>
> This patch provides a framework to do that from any RISC-V boards.
>
> 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                | 37 +++++++++++++++++++++++++++
>  5 files changed, 41 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..3a4d0bf14c86 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,42 @@ __weak void board_quiesce_devices(void)
>  {
>  }
>
> +int riscv_board_reserved_mem_fixup(void *fdt)
> +{
> +       uint32_t phandle;
> +       struct fdt_memory pmp_mem;
> +       int err;
> +       void *src_fdt_addr;
> +       int offset, node;
> +       phys_addr_t addr, size;
> +
> +       src_fdt_addr = map_sysmem(gd->arch.firmware_fdt_addr, 0);
> +       offset = fdt_path_offset(src_fdt_addr, "/reserved-memory");
> +       if (offset < 0) {
> +               printf("No reserved memory region found in FDT\n");
> +               return offset;
> +       }
> +
> +       fdt_for_each_subnode(node, src_fdt_addr, offset) {
> +               const char *name = fdt_get_name(src_fdt_addr, node, NULL);
> +
> +               addr = fdtdec_get_addr_size(src_fdt_addr, node, "reg", &size);
> +               if (addr == FDT_ADDR_T_NONE) {
> +                       debug("failed to read address/size for %s\n", name);
> +                       continue;
> +               }
> +               pmp_mem.start = addr;
> +               pmp_mem.end = addr + size;
> +               err = fdtdec_add_reserved_memory(fdt, name, &pmp_mem, &phandle);
> +               if (err < 0) {
> +                       printf("failed to add reserved memory: %d\n", err);
> +                       return err;
> +               }
> +       }
> +
> +       return 0;
> +}
> +
>  int arch_fixup_fdt(void *blob)
>  {
>         u32 size;
> --
> 2.25.1
>

Fixed palmer's email address. Sorry for the spam.

-- 
Regards,
Atish

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

[Atish Patra]

On Fri, Mar 13, 2020 at 5:12 PM 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)
> +{
> +       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
>

Fixed palmer's email address. Sorry for the spam.

-- 
Regards,
Atish

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

[Atish Patra]

On Fri, Mar 13, 2020 at 5:12 PM Atish Patra <atish.patra@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.
>
> 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 | 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");
> +                               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

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

[Heinrich Schuchardt]

On 3/14/20 1:55 AM, Atish Patra wrote:
> On Fri, Mar 13, 2020 at 5:12 PM Atish Patra <atish.patra@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.

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@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@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.
>

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

[Heinrich Schuchardt]

On 3/14/20 8:14 AM, Heinrich Schuchardt wrote:
> On 3/14/20 1:55 AM, Atish Patra wrote:
>> On Fri, Mar 13, 2020 at 5:12 PM Atish Patra <atish.patra@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.
> 
> 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@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'.
> 
>>> +?????????????????????????????? continue;

You do not advance subnode here, creating an endless loop. So this 
should be:

         /*
          * 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);
}

Best regards

Heinrich

>>> +?????????????????????? }
>>> +?????????????????????? efi_reserve_memory(addr, size);
>>> +?????????????????????? subnode = fdt_next_subnode(fdt, subnode);
>>> +?????????????? }
>>> ???????? }
>>> ? }
>>>
>>> -- 
>>> 2.25.1
>>>
>>
>> Fixed palmer's email address. Sorry for the spam.
>>
> 

[PATCH v2 3/4] riscv: Provide a mechanism for riscv boards to parse reserved memory

[Bin Meng]

Hi Atish,

On Sat, Mar 14, 2020 at 8:54 AM Atish Patra <atishp@atishpatra.org> wrote:
>
> On Fri, Mar 13, 2020 at 5:12 PM 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.
> >
> > In U-boot, any board may use the DT in following ways.

nits: U-Boot

> > 1. OF_SEPARTE: It ignores the DT from previous stage and uses the DT

typo: OF_SEPARATE

> > from U-Boot sources.
> > 2. OF_PRIOR_STATE: It reuses the DT from previous stage.

typo: OF_PRIOR_STAGE

> > For case 1: U-Boot needs to parse the reserved-memory node from the
> > DT passed from the previous stage and update the DT in use.
> >
> > This patch provides a framework to do that from any RISC-V boards.
> >
> > 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                | 37 +++++++++++++++++++++++++++
> >  5 files changed, 41 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..3a4d0bf14c86 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,42 @@ __weak void board_quiesce_devices(void)
> >  {
> >  }
> >
> > +int riscv_board_reserved_mem_fixup(void *fdt)
> > +{
> > +       uint32_t phandle;
> > +       struct fdt_memory pmp_mem;
> > +       int err;
> > +       void *src_fdt_addr;
> > +       int offset, node;
> > +       phys_addr_t addr, size;
> > +
> > +       src_fdt_addr = map_sysmem(gd->arch.firmware_fdt_addr, 0);
> > +       offset = fdt_path_offset(src_fdt_addr, "/reserved-memory");
> > +       if (offset < 0) {
> > +               printf("No reserved memory region found in FDT\n");
> > +               return offset;

We should return 0 to allow cases that do not have /reserved-memory
node. Otherwise we are mandating a newer OpenSBI version to work with
us.

> > +       }
> > +
> > +       fdt_for_each_subnode(node, src_fdt_addr, offset) {
> > +               const char *name = fdt_get_name(src_fdt_addr, node, NULL);
> > +
> > +               addr = fdtdec_get_addr_size(src_fdt_addr, node, "reg", &size);
> > +               if (addr == FDT_ADDR_T_NONE) {
> > +                       debug("failed to read address/size for %s\n", name);
> > +                       continue;
> > +               }
> > +               pmp_mem.start = addr;
> > +               pmp_mem.end = addr + size;
> > +               err = fdtdec_add_reserved_memory(fdt, name, &pmp_mem, &phandle);
> > +               if (err < 0) {
> > +                       printf("failed to add reserved memory: %d\n", err);
> > +                       return err;
> > +               }
> > +       }
> > +
> > +       return 0;
> > +}
> > +
> >  int arch_fixup_fdt(void *blob)
> >  {
> >         u32 size;
> > --

Regards,
Bin

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

[Bin Meng]

Hi Atish,

On Sat, Mar 14, 2020 at 8:11 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 is used to fix-up the DT that U-Boot uses, not the DT that
operating system uses.

As I mentioned in
https://github.com/riscv/riscv-sbi-doc/pull/37#issuecomment-596184723,
we need consider U-Boot SPL usage.

> +{
> +       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
> --

Regards,
Bin

[PATCH v2 3/4] riscv: Provide a mechanism for riscv boards to parse reserved memory

[Atish Patra]

On Sat, Mar 14, 2020 at 3:00 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Atish,
>
> On Sat, Mar 14, 2020 at 8:54 AM Atish Patra <atishp@atishpatra.org> wrote:
> >
> > On Fri, Mar 13, 2020 at 5:12 PM 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.
> > >
> > > In U-boot, any board may use the DT in following ways.
>
> nits: U-Boot
>
> > > 1. OF_SEPARTE: It ignores the DT from previous stage and uses the DT
>
> typo: OF_SEPARATE
>
> > > from U-Boot sources.
> > > 2. OF_PRIOR_STATE: It reuses the DT from previous stage.
>
> typo: OF_PRIOR_STAGE
>
> > > For case 1: U-Boot needs to parse the reserved-memory node from the
> > > DT passed from the previous stage and update the DT in use.
> > >
> > > This patch provides a framework to do that from any RISC-V boards.
> > >
> > > 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                | 37 +++++++++++++++++++++++++++
> > >  5 files changed, 41 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..3a4d0bf14c86 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,42 @@ __weak void board_quiesce_devices(void)
> > >  {
> > >  }
> > >
> > > +int riscv_board_reserved_mem_fixup(void *fdt)
> > > +{
> > > +       uint32_t phandle;
> > > +       struct fdt_memory pmp_mem;
> > > +       int err;
> > > +       void *src_fdt_addr;
> > > +       int offset, node;
> > > +       phys_addr_t addr, size;
> > > +
> > > +       src_fdt_addr = map_sysmem(gd->arch.firmware_fdt_addr, 0);
> > > +       offset = fdt_path_offset(src_fdt_addr, "/reserved-memory");
> > > +       if (offset < 0) {
> > > +               printf("No reserved memory region found in FDT\n");
> > > +               return offset;
>
> We should return 0 to allow cases that do not have /reserved-memory
> node. Otherwise we are mandating a newer OpenSBI version to work with
> us.
>

Correct. I will update the patch along with the nit fixes you suggested above.

> > > +       }
> > > +
> > > +       fdt_for_each_subnode(node, src_fdt_addr, offset) {
> > > +               const char *name = fdt_get_name(src_fdt_addr, node, NULL);
> > > +
> > > +               addr = fdtdec_get_addr_size(src_fdt_addr, node, "reg", &size);
> > > +               if (addr == FDT_ADDR_T_NONE) {
> > > +                       debug("failed to read address/size for %s\n", name);
> > > +                       continue;
> > > +               }
> > > +               pmp_mem.start = addr;
> > > +               pmp_mem.end = addr + size;
> > > +               err = fdtdec_add_reserved_memory(fdt, name, &pmp_mem, &phandle);
> > > +               if (err < 0) {
> > > +                       printf("failed to add reserved memory: %d\n", err);
> > > +                       return err;
> > > +               }
> > > +       }
> > > +
> > > +       return 0;
> > > +}
> > > +
> > >  int arch_fixup_fdt(void *blob)
> > >  {
> > >         u32 size;
> > > --
>
> Regards,
> Bin



-- 
Regards,
Atish

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

[Atish Patra]

On Sat, Mar 14, 2020 at 3:18 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Atish,
>
> On Sat, Mar 14, 2020 at 8:11 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 is used to fix-up the DT that U-Boot uses, not the DT that
> operating system uses.
>

Ahh yes. User can load DT from sdcard or network for OS usage.
We should also update the arch_fixup_fdt as well so that OS sees
the reserved memory nodes.

> As I mentioned in
> https://github.com/riscv/riscv-sbi-doc/pull/37#issuecomment-596184723,
> we need consider U-Boot SPL usage.
>

If U-Boot SPL is built without a DT, OpenSBI should be built with a DT.
Here are the constraints to use reserved-memory:

For any DT based platform:
1. If the stage prior to OpenSBI i.e. FSBL, U-Boot SPL is built
without a DT, OpenSBI
must be built with a DT.

2. If U-boot proper is built with OF_SEPARATE, OF_BOARD_FIXUP should be enabled
as well so that U-Boot can copy the reserved-memory nodes from
previous stage DT and update
the current DT in use.

> > +{
> > +       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
> > --
>
> Regards,
> Bin



-- 
Regards,
Atish

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

[Atish Patra]

On Sat, Mar 14, 2020 at 12:14 AM Heinrich Schuchardt <xypron.glpk@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@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@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@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