All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/6] DT related fixes for RISC-V UEFI
@ 2020-03-24  4:16 Atish Patra
  2020-03-24  4:16 ` [PATCH v4 1/6] riscv: Add boot hartid to Device tree Atish Patra
                   ` (5 more replies)
  0 siblings, 6 replies; 13+ messages in thread
From: Atish Patra @ 2020-03-24  4:16 UTC (permalink / raw)
  To: u-boot

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 fdtdec related to reserved memory node.

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 v3->v4:
1. Dropped generic efi fix patch as it is already merged.
2. Moved all the fdt fixups to a common file.
3. Addressed few nit comments.

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 (6):
riscv: Add boot hartid to Device tree
fdtdec: Fix boundary check
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
riscv: Move all fdt fixups together

arch/riscv/cpu/start.S                |   1 +
arch/riscv/include/asm/global_data.h  |   1 +
arch/riscv/include/asm/u-boot-riscv.h |   2 +
arch/riscv/lib/Makefile               |   1 +
arch/riscv/lib/asm-offsets.c          |   1 +
arch/riscv/lib/bootm.c                |   5 -
arch/riscv/lib/fdt_fixup.c            | 128 ++++++++++++++++++++++++++
configs/sifive_fu540_defconfig        |   1 +
lib/fdtdec.c                          |   3 +-
9 files changed, 137 insertions(+), 6 deletions(-)
create mode 100644 arch/riscv/lib/fdt_fixup.c

--
2.25.1

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v4 1/6] riscv: Add boot hartid to Device tree
  2020-03-24  4:16 [PATCH v4 0/6] DT related fixes for RISC-V UEFI Atish Patra
@ 2020-03-24  4:16 ` Atish Patra
  2020-03-24  4:16 ` [PATCH v4 2/6] fdtdec: Fix boundary check Atish Patra
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Atish Patra @ 2020-03-24  4:16 UTC (permalink / raw)
  To: u-boot

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..87cadad5016d 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)
 {
+#ifdef CONFIG_EFI_LOADER
+	int err;
+	u32 size;
+	int chosen_offset;
+
+	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 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 err;
+		}
+	}
+	/* Overwrite the boot-hartid as U-Boot is the last stage BL */
+	fdt_setprop_u32(blob, chosen_offset, "boot-hartid", gd->arch.boot_hart);
+#endif
 	return 0;
 }
 
-- 
2.25.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v4 2/6] fdtdec: Fix boundary check
  2020-03-24  4:16 [PATCH v4 0/6] DT related fixes for RISC-V UEFI Atish Patra
  2020-03-24  4:16 ` [PATCH v4 1/6] riscv: Add boot hartid to Device tree Atish Patra
@ 2020-03-24  4:16 ` Atish Patra
  2020-03-24  4:16 ` [PATCH v4 3/6] riscv: Provide a mechanism to fix DT for reserved memory Atish Patra
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Atish Patra @ 2020-03-24  4:16 UTC (permalink / raw)
  To: u-boot

In U-Boot, the reserved memory end address is considered as a inclusive
address. This notion is followed while adding a reserved memory node to
the DT.

For example:
end_address = start_address + size - 1

Follow the same notion and fix the end address computation while checking
for existing nodes.

Signed-off-by: Atish Patra <atish.patra@wdc.com>
---
 lib/fdtdec.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/fdtdec.c b/lib/fdtdec.c
index eb11fc898e30..07ba9f5c97e9 100644
--- a/lib/fdtdec.c
+++ b/lib/fdtdec.c
@@ -1311,7 +1311,8 @@ int fdtdec_add_reserved_memory(void *blob, const char *basename,
 			continue;
 		}
 
-		if (addr == carveout->start && (addr + size) == carveout->end) {
+		if (addr == carveout->start && (addr + size - 1) ==
+						carveout->end) {
 			if (phandlep)
 				*phandlep = fdt_get_phandle(blob, node);
 			return 0;
-- 
2.25.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v4 3/6] riscv: Provide a mechanism to fix DT for reserved memory
  2020-03-24  4:16 [PATCH v4 0/6] DT related fixes for RISC-V UEFI Atish Patra
  2020-03-24  4:16 ` [PATCH v4 1/6] riscv: Add boot hartid to Device tree Atish Patra
  2020-03-24  4:16 ` [PATCH v4 2/6] fdtdec: Fix boundary check Atish Patra
@ 2020-03-24  4:16 ` Atish Patra
  2020-03-24  6:13   ` Heinrich Schuchardt
  2020-03-24  4:16 ` [PATCH v4 4/6] riscv: Setup reserved-memory node for FU540 Atish Patra
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Atish Patra @ 2020-03-24  4:16 UTC (permalink / raw)
  To: u-boot

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 |  2 +
 arch/riscv/lib/Makefile               |  1 +
 arch/riscv/lib/asm-offsets.c          |  1 +
 arch/riscv/lib/fdt_fixup.c            | 80 +++++++++++++++++++++++++++
 6 files changed, 86 insertions(+)
 create mode 100644 arch/riscv/lib/fdt_fixup.c

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..543a1688db8f 100644
--- a/arch/riscv/include/asm/u-boot-riscv.h
+++ b/arch/riscv/include/asm/u-boot-riscv.h
@@ -17,5 +17,7 @@ int cleanup_before_linux(void);
 /* board/.../... */
 int board_init(void);
 void board_quiesce_devices(void);
+int riscv_board_reserved_mem_fixup(void *fdt);
+int riscv_fdt_copy_resv_mem_node(const void *src_fdt, void *dest_fdt);
 
 #endif	/* _U_BOOT_RISCV_H_ */
diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile
index adadbf4bcbef..d132b59ce32c 100644
--- a/arch/riscv/lib/Makefile
+++ b/arch/riscv/lib/Makefile
@@ -24,6 +24,7 @@ obj-y	+= reset.o
 obj-y   += setjmp.o
 obj-$(CONFIG_SMP) += smp.o
 obj-$(CONFIG_SPL_BUILD)	+= spl.o
+obj-y   += fdt_fixup.o
 
 # For building EFI apps
 CFLAGS_$(EFI_CRT0) := $(CFLAGS_EFI)
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/fdt_fixup.c b/arch/riscv/lib/fdt_fixup.c
new file mode 100644
index 000000000000..f3d1ec5c5d02
--- /dev/null
+++ b/arch/riscv/lib/fdt_fixup.c
@@ -0,0 +1,80 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2020 Western Digital Corporation or its affiliates
+ *
+ */
+
+#include <common.h>
+#include <fdt_support.h>
+#include <mapmem.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+int riscv_fdt_copy_resv_mem_node(const void *src, void *dst)
+{
+	u32 phandle;
+	struct fdt_memory pmp_mem;
+	fdt_addr_t addr;
+	fdt_size_t size;
+	int offset, node, err, rmem_offset;
+	bool nomap = true;
+	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,
+							  false);
+		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 - 1;
+		err = fdtdec_add_reserved_memory(dst, basename, &pmp_mem,
+						 &phandle);
+		if (err < 0) {
+			printf("failed to add reserved memory: %d\n", err);
+			return err;
+		}
+		if (!fdt_getprop(src, node, "no-map", NULL))
+			nomap = false;
+		if (nomap) {
+			rmem_offset = fdt_node_offset_by_phandle(dst, phandle);
+			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;
+}
-- 
2.25.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v4 4/6] riscv: Setup reserved-memory node for FU540
  2020-03-24  4:16 [PATCH v4 0/6] DT related fixes for RISC-V UEFI Atish Patra
                   ` (2 preceding siblings ...)
  2020-03-24  4:16 ` [PATCH v4 3/6] riscv: Provide a mechanism to fix DT for reserved memory Atish Patra
@ 2020-03-24  4:16 ` Atish Patra
  2020-03-24  4:16 ` [PATCH v4 5/6] riscv: Copy the reserved-memory nodes to final DT Atish Patra
  2020-03-24  4:16 ` [PATCH v4 6/6] riscv: Move all fdt fixups together Atish Patra
  5 siblings, 0 replies; 13+ messages in thread
From: Atish Patra @ 2020-03-24  4:16 UTC (permalink / raw)
  To: u-boot

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>
---
 arch/riscv/lib/fdt_fixup.c     | 15 +++++++++++++++
 configs/sifive_fu540_defconfig |  1 +
 2 files changed, 16 insertions(+)

diff --git a/arch/riscv/lib/fdt_fixup.c b/arch/riscv/lib/fdt_fixup.c
index f3d1ec5c5d02..e41f06f5b7f1 100644
--- a/arch/riscv/lib/fdt_fixup.c
+++ b/arch/riscv/lib/fdt_fixup.c
@@ -10,6 +10,21 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
+#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 riscv_fdt_copy_resv_mem_node(const void *src, void *dst)
 {
 	u32 phandle;
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

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v4 5/6] riscv: Copy the reserved-memory nodes to final DT
  2020-03-24  4:16 [PATCH v4 0/6] DT related fixes for RISC-V UEFI Atish Patra
                   ` (3 preceding siblings ...)
  2020-03-24  4:16 ` [PATCH v4 4/6] riscv: Setup reserved-memory node for FU540 Atish Patra
@ 2020-03-24  4:16 ` Atish Patra
  2020-03-24  6:23   ` Heinrich Schuchardt
  2020-03-24  4:16 ` [PATCH v4 6/6] riscv: Move all fdt fixups together Atish Patra
  5 siblings, 1 reply; 13+ messages in thread
From: Atish Patra @ 2020-03-24  4:16 UTC (permalink / raw)
  To: u-boot

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>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
---
 arch/riscv/lib/bootm.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/riscv/lib/bootm.c b/arch/riscv/lib/bootm.c
index 87cadad5016d..8ff8db6bf533 100644
--- a/arch/riscv/lib/bootm.c
+++ b/arch/riscv/lib/bootm.c
@@ -28,8 +28,8 @@ __weak void board_quiesce_devices(void)
 
 int arch_fixup_fdt(void *blob)
 {
-#ifdef CONFIG_EFI_LOADER
 	int err;
+#ifdef CONFIG_EFI_LOADER
 	u32 size;
 	int chosen_offset;
 
@@ -50,6 +50,12 @@ int arch_fixup_fdt(void *blob)
 	/* Overwrite the boot-hartid as U-Boot is the last stage BL */
 	fdt_setprop_u32(blob, chosen_offset, "boot-hartid", gd->arch.boot_hart);
 #endif
+
+	/* 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

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v4 6/6] riscv: Move all fdt fixups together
  2020-03-24  4:16 [PATCH v4 0/6] DT related fixes for RISC-V UEFI Atish Patra
                   ` (4 preceding siblings ...)
  2020-03-24  4:16 ` [PATCH v4 5/6] riscv: Copy the reserved-memory nodes to final DT Atish Patra
@ 2020-03-24  4:16 ` Atish Patra
  5 siblings, 0 replies; 13+ messages in thread
From: Atish Patra @ 2020-03-24  4:16 UTC (permalink / raw)
  To: u-boot

Keep all the fdt fixups together for better code management.

Signed-off-by: Atish Patra <atish.patra@wdc.com>
---
 arch/riscv/lib/bootm.c     | 33 ---------------------------------
 arch/riscv/lib/fdt_fixup.c | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 33 insertions(+), 33 deletions(-)

diff --git a/arch/riscv/lib/bootm.c b/arch/riscv/lib/bootm.c
index 8ff8db6bf533..0d06095da11a 100644
--- a/arch/riscv/lib/bootm.c
+++ b/arch/riscv/lib/bootm.c
@@ -26,39 +26,6 @@ __weak void board_quiesce_devices(void)
 {
 }
 
-int arch_fixup_fdt(void *blob)
-{
-	int err;
-#ifdef CONFIG_EFI_LOADER
-	u32 size;
-	int chosen_offset;
-
-	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 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 err;
-		}
-	}
-	/* Overwrite the boot-hartid as U-Boot is the last stage BL */
-	fdt_setprop_u32(blob, chosen_offset, "boot-hartid", gd->arch.boot_hart);
-#endif
-
-	/* 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;
-}
-
 /**
  * announce_and_cleanup() - Print message and prepare for kernel boot
  *
diff --git a/arch/riscv/lib/fdt_fixup.c b/arch/riscv/lib/fdt_fixup.c
index e41f06f5b7f1..df340d1e9fda 100644
--- a/arch/riscv/lib/fdt_fixup.c
+++ b/arch/riscv/lib/fdt_fixup.c
@@ -93,3 +93,36 @@ int riscv_board_reserved_mem_fixup(void *fdt)
 
 	return 0;
 }
+
+int arch_fixup_fdt(void *blob)
+{
+	int err;
+#ifdef CONFIG_EFI_LOADER
+	u32 size;
+	int chosen_offset;
+
+	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 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 err;
+		}
+	}
+	/* Overwrite the boot-hartid as U-Boot is the last stage BL */
+	fdt_setprop_u32(blob, chosen_offset, "boot-hartid", gd->arch.boot_hart);
+#endif
+
+	/* 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

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v4 3/6] riscv: Provide a mechanism to fix DT for reserved memory
  2020-03-24  4:16 ` [PATCH v4 3/6] riscv: Provide a mechanism to fix DT for reserved memory Atish Patra
@ 2020-03-24  6:13   ` Heinrich Schuchardt
  2020-03-24  7:15     ` Atish Patra
  0 siblings, 1 reply; 13+ messages in thread
From: Heinrich Schuchardt @ 2020-03-24  6:13 UTC (permalink / raw)
  To: u-boot

On 3/24/20 5:16 AM, Atish Patra 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 |  2 +
>   arch/riscv/lib/Makefile               |  1 +
>   arch/riscv/lib/asm-offsets.c          |  1 +
>   arch/riscv/lib/fdt_fixup.c            | 80 +++++++++++++++++++++++++++
>   6 files changed, 86 insertions(+)
>   create mode 100644 arch/riscv/lib/fdt_fixup.c
>
> 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..543a1688db8f 100644
> --- a/arch/riscv/include/asm/u-boot-riscv.h
> +++ b/arch/riscv/include/asm/u-boot-riscv.h
> @@ -17,5 +17,7 @@ int cleanup_before_linux(void);
>   /* board/.../... */
>   int board_init(void);
>   void board_quiesce_devices(void);
> +int riscv_board_reserved_mem_fixup(void *fdt);
> +int riscv_fdt_copy_resv_mem_node(const void *src_fdt, void *dest_fdt);
>
>   #endif	/* _U_BOOT_RISCV_H_ */
> diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile
> index adadbf4bcbef..d132b59ce32c 100644
> --- a/arch/riscv/lib/Makefile
> +++ b/arch/riscv/lib/Makefile
> @@ -24,6 +24,7 @@ obj-y	+= reset.o
>   obj-y   += setjmp.o
>   obj-$(CONFIG_SMP) += smp.o
>   obj-$(CONFIG_SPL_BUILD)	+= spl.o
> +obj-y   += fdt_fixup.o
>
>   # For building EFI apps
>   CFLAGS_$(EFI_CRT0) := $(CFLAGS_EFI)
> 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/fdt_fixup.c b/arch/riscv/lib/fdt_fixup.c
> new file mode 100644
> index 000000000000..f3d1ec5c5d02
> --- /dev/null
> +++ b/arch/riscv/lib/fdt_fixup.c
> @@ -0,0 +1,80 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2020 Western Digital Corporation or its affiliates
> + *
> + */
> +
> +#include <common.h>
> +#include <fdt_support.h>
> +#include <mapmem.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +

Please, provide Sphinx style comments for new functions. Cf.
https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html.

> +int riscv_fdt_copy_resv_mem_node(const void *src, void *dst)
> +{
> +	u32 phandle;
> +	struct fdt_memory pmp_mem;
> +	fdt_addr_t addr;
> +	fdt_size_t size;
> +	int offset, node, err, rmem_offset;
> +	bool nomap = true;
> +	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,
> +							  false);
> +		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 - 1;
> +		err = fdtdec_add_reserved_memory(dst, basename, &pmp_mem,
> +						 &phandle);
> +		if (err < 0) {
> +			printf("failed to add reserved memory: %d\n", err);
> +			return err;
> +		}
> +		if (!fdt_getprop(src, node, "no-map", NULL))
> +			nomap = false;
> +		if (nomap) {
> +			rmem_offset = fdt_node_offset_by_phandle(dst, phandle);
> +			fdt_setprop_empty(dst, rmem_offset, "no-map");
> +		}
> +	}
> +
> +	return 0;
> +}
> +

Same here.

Best regards

Heinrich

> +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;
> +}
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v4 5/6] riscv: Copy the reserved-memory nodes to final DT
  2020-03-24  4:16 ` [PATCH v4 5/6] riscv: Copy the reserved-memory nodes to final DT Atish Patra
@ 2020-03-24  6:23   ` Heinrich Schuchardt
  2020-03-24  7:15     ` Atish Patra
  0 siblings, 1 reply; 13+ messages in thread
From: Heinrich Schuchardt @ 2020-03-24  6:23 UTC (permalink / raw)
  To: u-boot

On 3/24/20 5:16 AM, Atish Patra 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.

The bootefi command works in the following sequence:

* copy the passed fdt of if none is passed the internal fdt
* call image_setup_libfdt() for the copy
* create memory reservations in the UEFI memory map

Cf.
fef907b2e440 ("efi_loader: create reservations after ft_board_setup")
7be64b885a36 ("cmd: bootefi: Parse reserved-memory node from DT")

image_setup_libfdt() executes among others:

* fdt_chosen()
* arch_fixup_fdt()

Please, explain why you need to copy memory reservations, if
arch_fixup_fdt() is executed inside the bootefi command:

What would be the source of memory reservation that you would otherwise
miss in the final device tree? Do you expect that the Linux device tree
would lack reservation that are generated on the fly before U-Boot?

Best regards

Heinrich

>
> Signed-off-by: Atish Patra <atish.patra@wdc.com>
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> ---
>   arch/riscv/lib/bootm.c | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/arch/riscv/lib/bootm.c b/arch/riscv/lib/bootm.c
> index 87cadad5016d..8ff8db6bf533 100644
> --- a/arch/riscv/lib/bootm.c
> +++ b/arch/riscv/lib/bootm.c
> @@ -28,8 +28,8 @@ __weak void board_quiesce_devices(void)
>
>   int arch_fixup_fdt(void *blob)
>   {
> -#ifdef CONFIG_EFI_LOADER
>   	int err;
> +#ifdef CONFIG_EFI_LOADER
>   	u32 size;
>   	int chosen_offset;
>
> @@ -50,6 +50,12 @@ int arch_fixup_fdt(void *blob)
>   	/* Overwrite the boot-hartid as U-Boot is the last stage BL */
>   	fdt_setprop_u32(blob, chosen_offset, "boot-hartid", gd->arch.boot_hart);
>   #endif
> +
> +	/* 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;
>   }
>
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v4 5/6] riscv: Copy the reserved-memory nodes to final DT
  2020-03-24  6:23   ` Heinrich Schuchardt
@ 2020-03-24  7:15     ` Atish Patra
  2020-03-24 22:11       ` Heinrich Schuchardt
  0 siblings, 1 reply; 13+ messages in thread
From: Atish Patra @ 2020-03-24  7:15 UTC (permalink / raw)
  To: u-boot

On Mon, Mar 23, 2020 at 11:23 PM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 3/24/20 5:16 AM, Atish Patra 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.
>
> The bootefi command works in the following sequence:
>
> * copy the passed fdt of if none is passed the internal fdt
> * call image_setup_libfdt() for the copy
> * create memory reservations in the UEFI memory map
>
> Cf.
> fef907b2e440 ("efi_loader: create reservations after ft_board_setup")
> 7be64b885a36 ("cmd: bootefi: Parse reserved-memory node from DT")
>
> image_setup_libfdt() executes among others:
>
> * fdt_chosen()
> * arch_fixup_fdt()
>
> Please, explain why you need to copy memory reservations, if
> arch_fixup_fdt() is executed inside the bootefi command:
>

As per my understanding, there can be two different DTs in U-Boot.
1. Internal device tree or DT passed from previous stage (gd->fdt_blob). This
    device tree is being used to boot OS as well.
2. User can load the DT from MMC/network to any valid address and use
that address in bootefi/booti command line to boot OS.

efi_install_fdt copies DT from the given user address (via cmdline) and copies
to the efi memory.

There is no need to 2nd copy for case 1 as the DT already contains the
reserved-memory node. In case 2, the DT loaded from external media may not
have the reserved memory regions set by the firmware. That's why we need to
copy the reserved-memory nodes. Now, arch_fixup_fdt() is the last place to edit
the DT passed to OS. The copy operation has to be done in arch_fixup_fdt.
For case 1, fdtdec_add_reserved_memory will simply return without
modifying the DT.

> What would be the source of memory reservation that you would otherwise
> miss in the final device tree? Do you expect that the Linux device tree
> would lack reservation that are generated on the fly before U-Boot?
>
> Best regards
>
> Heinrich
>
> >
> > Signed-off-by: Atish Patra <atish.patra@wdc.com>
> > Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> > ---
> >   arch/riscv/lib/bootm.c | 8 +++++++-
> >   1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/riscv/lib/bootm.c b/arch/riscv/lib/bootm.c
> > index 87cadad5016d..8ff8db6bf533 100644
> > --- a/arch/riscv/lib/bootm.c
> > +++ b/arch/riscv/lib/bootm.c
> > @@ -28,8 +28,8 @@ __weak void board_quiesce_devices(void)
> >
> >   int arch_fixup_fdt(void *blob)
> >   {
> > -#ifdef CONFIG_EFI_LOADER
> >       int err;
> > +#ifdef CONFIG_EFI_LOADER
> >       u32 size;
> >       int chosen_offset;
> >
> > @@ -50,6 +50,12 @@ int arch_fixup_fdt(void *blob)
> >       /* Overwrite the boot-hartid as U-Boot is the last stage BL */
> >       fdt_setprop_u32(blob, chosen_offset, "boot-hartid", gd->arch.boot_hart);
> >   #endif
> > +
> > +     /* 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;
> >   }
> >
> >
>


-- 
Regards,
Atish

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v4 3/6] riscv: Provide a mechanism to fix DT for reserved memory
  2020-03-24  6:13   ` Heinrich Schuchardt
@ 2020-03-24  7:15     ` Atish Patra
  0 siblings, 0 replies; 13+ messages in thread
From: Atish Patra @ 2020-03-24  7:15 UTC (permalink / raw)
  To: u-boot

On Mon, Mar 23, 2020 at 11:14 PM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 3/24/20 5:16 AM, Atish Patra 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 |  2 +
> >   arch/riscv/lib/Makefile               |  1 +
> >   arch/riscv/lib/asm-offsets.c          |  1 +
> >   arch/riscv/lib/fdt_fixup.c            | 80 +++++++++++++++++++++++++++
> >   6 files changed, 86 insertions(+)
> >   create mode 100644 arch/riscv/lib/fdt_fixup.c
> >
> > 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..543a1688db8f 100644
> > --- a/arch/riscv/include/asm/u-boot-riscv.h
> > +++ b/arch/riscv/include/asm/u-boot-riscv.h
> > @@ -17,5 +17,7 @@ int cleanup_before_linux(void);
> >   /* board/.../... */
> >   int board_init(void);
> >   void board_quiesce_devices(void);
> > +int riscv_board_reserved_mem_fixup(void *fdt);
> > +int riscv_fdt_copy_resv_mem_node(const void *src_fdt, void *dest_fdt);
> >
> >   #endif      /* _U_BOOT_RISCV_H_ */
> > diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile
> > index adadbf4bcbef..d132b59ce32c 100644
> > --- a/arch/riscv/lib/Makefile
> > +++ b/arch/riscv/lib/Makefile
> > @@ -24,6 +24,7 @@ obj-y       += reset.o
> >   obj-y   += setjmp.o
> >   obj-$(CONFIG_SMP) += smp.o
> >   obj-$(CONFIG_SPL_BUILD)     += spl.o
> > +obj-y   += fdt_fixup.o
> >
> >   # For building EFI apps
> >   CFLAGS_$(EFI_CRT0) := $(CFLAGS_EFI)
> > 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/fdt_fixup.c b/arch/riscv/lib/fdt_fixup.c
> > new file mode 100644
> > index 000000000000..f3d1ec5c5d02
> > --- /dev/null
> > +++ b/arch/riscv/lib/fdt_fixup.c
> > @@ -0,0 +1,80 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright (c) 2020 Western Digital Corporation or its affiliates
> > + *
> > + */
> > +
> > +#include <common.h>
> > +#include <fdt_support.h>
> > +#include <mapmem.h>
> > +
> > +DECLARE_GLOBAL_DATA_PTR;
> > +
>
> Please, provide Sphinx style comments for new functions. Cf.
> https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html.
>

Sure. I will do that.

> > +int riscv_fdt_copy_resv_mem_node(const void *src, void *dst)
> > +{
> > +     u32 phandle;
> > +     struct fdt_memory pmp_mem;
> > +     fdt_addr_t addr;
> > +     fdt_size_t size;
> > +     int offset, node, err, rmem_offset;
> > +     bool nomap = true;
> > +     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,
> > +                                                       false);
> > +             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 - 1;
> > +             err = fdtdec_add_reserved_memory(dst, basename, &pmp_mem,
> > +                                              &phandle);
> > +             if (err < 0) {
> > +                     printf("failed to add reserved memory: %d\n", err);
> > +                     return err;
> > +             }
> > +             if (!fdt_getprop(src, node, "no-map", NULL))
> > +                     nomap = false;
> > +             if (nomap) {
> > +                     rmem_offset = fdt_node_offset_by_phandle(dst, phandle);
> > +                     fdt_setprop_empty(dst, rmem_offset, "no-map");
> > +             }
> > +     }
> > +
> > +     return 0;
> > +}
> > +
>
> Same here.
>
> Best regards
>
> Heinrich
>
> > +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;
> > +}
> >
>


-- 
Regards,
Atish

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v4 5/6] riscv: Copy the reserved-memory nodes to final DT
  2020-03-24  7:15     ` Atish Patra
@ 2020-03-24 22:11       ` Heinrich Schuchardt
  2020-03-25  3:38         ` Atish Patra
  0 siblings, 1 reply; 13+ messages in thread
From: Heinrich Schuchardt @ 2020-03-24 22:11 UTC (permalink / raw)
  To: u-boot

On 3/24/20 8:15 AM, Atish Patra wrote:
> On Mon, Mar 23, 2020 at 11:23 PM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>
>> On 3/24/20 5:16 AM, Atish Patra 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.
>>
>> The bootefi command works in the following sequence:
>>
>> * copy the passed fdt of if none is passed the internal fdt
>> * call image_setup_libfdt() for the copy
>> * create memory reservations in the UEFI memory map
>>
>> Cf.
>> fef907b2e440 ("efi_loader: create reservations after ft_board_setup")
>> 7be64b885a36 ("cmd: bootefi: Parse reserved-memory node from DT")
>>
>> image_setup_libfdt() executes among others:
>>
>> * fdt_chosen()
>> * arch_fixup_fdt()
>>
>> Please, explain why you need to copy memory reservations, if
>> arch_fixup_fdt() is executed inside the bootefi command:
>>
>
> As per my understanding, there can be two different DTs in U-Boot.
> 1. Internal device tree or DT passed from previous stage (gd->fdt_blob). This
>      device tree is being used to boot OS as well.
> 2. User can load the DT from MMC/network to any valid address and use
> that address in bootefi/booti command line to boot OS.
>
> efi_install_fdt copies DT from the given user address (via cmdline) and copies
> to the efi memory.
>
> There is no need to 2nd copy for case 1 as the DT already contains the
> reserved-memory node. In case 2, the DT loaded from external media may not
> have the reserved memory regions set by the firmware. That's why we need to
> copy the reserved-memory nodes. Now, arch_fixup_fdt() is the last place to edit
> the DT passed to OS. The copy operation has to be done in arch_fixup_fdt.
> For case 1, fdtdec_add_reserved_memory will simply return without
> modifying the DT.

There are three cases of device trees:

* U-Boot's internal device trees
* device trees provided by a earlier stage firmware to U-Boot
   (e.g. on QEMU)
* device trees supplied by the user

We cannot use U-Boot's internal device tree directly because a program
called via bootefi may manipulate the device tree and return afterwards.

As arch_fixup_fdt is called by all boot commands there is no need to
insert reserved memory nodes at an earlier stage.

The copy operation is only needed for bootefi as the other boot commands
cannot return. So it does not make to me to move the device tree copying
to arch_fixup_fdt().

Best regards

Heinrich

>
>> What would be the source of memory reservation that you would otherwise
>> miss in the final device tree? Do you expect that the Linux device tree
>> would lack reservation that are generated on the fly before U-Boot?
>>
>> Best regards
>>
>> Heinrich
>>
>>>
>>> Signed-off-by: Atish Patra <atish.patra@wdc.com>
>>> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
>>> ---
>>>    arch/riscv/lib/bootm.c | 8 +++++++-
>>>    1 file changed, 7 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/riscv/lib/bootm.c b/arch/riscv/lib/bootm.c
>>> index 87cadad5016d..8ff8db6bf533 100644
>>> --- a/arch/riscv/lib/bootm.c
>>> +++ b/arch/riscv/lib/bootm.c
>>> @@ -28,8 +28,8 @@ __weak void board_quiesce_devices(void)
>>>
>>>    int arch_fixup_fdt(void *blob)
>>>    {
>>> -#ifdef CONFIG_EFI_LOADER
>>>        int err;
>>> +#ifdef CONFIG_EFI_LOADER
>>>        u32 size;
>>>        int chosen_offset;
>>>
>>> @@ -50,6 +50,12 @@ int arch_fixup_fdt(void *blob)
>>>        /* Overwrite the boot-hartid as U-Boot is the last stage BL */
>>>        fdt_setprop_u32(blob, chosen_offset, "boot-hartid", gd->arch.boot_hart);
>>>    #endif
>>> +
>>> +     /* 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;
>>>    }
>>>
>>>
>>
>
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v4 5/6] riscv: Copy the reserved-memory nodes to final DT
  2020-03-24 22:11       ` Heinrich Schuchardt
@ 2020-03-25  3:38         ` Atish Patra
  0 siblings, 0 replies; 13+ messages in thread
From: Atish Patra @ 2020-03-25  3:38 UTC (permalink / raw)
  To: u-boot

On Tue, Mar 24, 2020 at 3:11 PM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 3/24/20 8:15 AM, Atish Patra wrote:
> > On Mon, Mar 23, 2020 at 11:23 PM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >>
> >> On 3/24/20 5:16 AM, Atish Patra 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.
> >>
> >> The bootefi command works in the following sequence:
> >>
> >> * copy the passed fdt of if none is passed the internal fdt
> >> * call image_setup_libfdt() for the copy
> >> * create memory reservations in the UEFI memory map
> >>
> >> Cf.
> >> fef907b2e440 ("efi_loader: create reservations after ft_board_setup")
> >> 7be64b885a36 ("cmd: bootefi: Parse reserved-memory node from DT")
> >>
> >> image_setup_libfdt() executes among others:
> >>
> >> * fdt_chosen()
> >> * arch_fixup_fdt()
> >>
> >> Please, explain why you need to copy memory reservations, if
> >> arch_fixup_fdt() is executed inside the bootefi command:
> >>
> >
> > As per my understanding, there can be two different DTs in U-Boot.
> > 1. Internal device tree or DT passed from previous stage (gd->fdt_blob). This
> >      device tree is being used to boot OS as well.
> > 2. User can load the DT from MMC/network to any valid address and use
> > that address in bootefi/booti command line to boot OS.
> >
> > efi_install_fdt copies DT from the given user address (via cmdline) and copies
> > to the efi memory.
> >
> > There is no need to 2nd copy for case 1 as the DT already contains the
> > reserved-memory node. In case 2, the DT loaded from external media may not
> > have the reserved memory regions set by the firmware. That's why we need to
> > copy the reserved-memory nodes. Now, arch_fixup_fdt() is the last place to edit
> > the DT passed to OS. The copy operation has to be done in arch_fixup_fdt.
> > For case 1, fdtdec_add_reserved_memory will simply return without
> > modifying the DT.
>
> There are three cases of device trees:
>
> * U-Boot's internal device trees
> * device trees provided by a earlier stage firmware to U-Boot
>    (e.g. on QEMU)
> * device trees supplied by the user
>
Yes. I combined 1st & 2nd case into one.

> We cannot use U-Boot's internal device tree directly because a program
> called via bootefi may manipulate the device tree and return afterwards.
>
> As arch_fixup_fdt is called by all boot commands there is no need to
> insert reserved memory nodes at an earlier stage.
>
> The copy operation is only needed for bootefi as the other boot commands
> cannot return. So it does not make to me to move the device tree copying
> to arch_fixup_fdt().
>

This series is aimed at providing reserved memory support all boot
commands in RISC-V.
All other boot commands booti/bootm also load the device tree supplied
by the user.
Hence, we need to copy the reserved memory nodes so that proper kernel
sees those DT
nodes.

The code path for booti/bootm is
image_setup_linux->image_setup_libfdt->arch_fixup_fdt

This is executed while executing booti/bootm and fixes the DT that
passed to the OS.

> Best regards
>
> Heinrich
>
> >
> >> What would be the source of memory reservation that you would otherwise
> >> miss in the final device tree? Do you expect that the Linux device tree
> >> would lack reservation that are generated on the fly before U-Boot?
> >>
> >> Best regards
> >>
> >> Heinrich
> >>
> >>>
> >>> Signed-off-by: Atish Patra <atish.patra@wdc.com>
> >>> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> >>> ---
> >>>    arch/riscv/lib/bootm.c | 8 +++++++-
> >>>    1 file changed, 7 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/arch/riscv/lib/bootm.c b/arch/riscv/lib/bootm.c
> >>> index 87cadad5016d..8ff8db6bf533 100644
> >>> --- a/arch/riscv/lib/bootm.c
> >>> +++ b/arch/riscv/lib/bootm.c
> >>> @@ -28,8 +28,8 @@ __weak void board_quiesce_devices(void)
> >>>
> >>>    int arch_fixup_fdt(void *blob)
> >>>    {
> >>> -#ifdef CONFIG_EFI_LOADER
> >>>        int err;
> >>> +#ifdef CONFIG_EFI_LOADER
> >>>        u32 size;
> >>>        int chosen_offset;
> >>>
> >>> @@ -50,6 +50,12 @@ int arch_fixup_fdt(void *blob)
> >>>        /* Overwrite the boot-hartid as U-Boot is the last stage BL */
> >>>        fdt_setprop_u32(blob, chosen_offset, "boot-hartid", gd->arch.boot_hart);
> >>>    #endif
> >>> +
> >>> +     /* 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;
> >>>    }
> >>>
> >>>
> >>
> >
> >
>


-- 
Regards,
Atish

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2020-03-25  3:38 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-24  4:16 [PATCH v4 0/6] DT related fixes for RISC-V UEFI Atish Patra
2020-03-24  4:16 ` [PATCH v4 1/6] riscv: Add boot hartid to Device tree Atish Patra
2020-03-24  4:16 ` [PATCH v4 2/6] fdtdec: Fix boundary check Atish Patra
2020-03-24  4:16 ` [PATCH v4 3/6] riscv: Provide a mechanism to fix DT for reserved memory Atish Patra
2020-03-24  6:13   ` Heinrich Schuchardt
2020-03-24  7:15     ` Atish Patra
2020-03-24  4:16 ` [PATCH v4 4/6] riscv: Setup reserved-memory node for FU540 Atish Patra
2020-03-24  4:16 ` [PATCH v4 5/6] riscv: Copy the reserved-memory nodes to final DT Atish Patra
2020-03-24  6:23   ` Heinrich Schuchardt
2020-03-24  7:15     ` Atish Patra
2020-03-24 22:11       ` Heinrich Schuchardt
2020-03-25  3:38         ` Atish Patra
2020-03-24  4:16 ` [PATCH v4 6/6] riscv: Move all fdt fixups together Atish Patra

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.