[PATCH v5 0/6] RISC-V DT related fixes for reserved memory & UEFI

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 master OpenSBI branch.
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 v4->v5: 1. Added comments for new functions.
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 | 150 ++++++++++++++++++++++++++ configs/sifive_fu540_defconfig | 1 + lib/fdtdec.c | 3 +- 9 files changed, 159 insertions(+), 6 deletions(-) create mode 100644 arch/riscv/lib/fdt_fixup.c
-- 2.25.1

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 | 102 ++++++++++++++++++++++++++ 6 files changed, 108 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..1fce41490973 --- /dev/null +++ b/arch/riscv/lib/fdt_fixup.c @@ -0,0 +1,102 @@ +// 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; + +/** + * riscv_fdt_copy_resv_mem_node() - Copy reserve memory node entry + * @src: Pointer to the source device tree from which reserved memory node + * needs to be copied. + * @dst: Pointer to the destination device tree to which reserved memory node + * needs to be copied. + * + * Return: 0 on success or if source doesn't have reserved memory node. + * Error if copy process failed. + */ +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; +} + +/** + * riscv_board_reserved_mem_fixup() - Fix up reserved memory node for a board + * @fdt: Pointer to the device tree in which reserved memory node needs to be + * added. + * + * In RISC-V, any board compiled with OF_SEPARATE needs to copy the reserved + * memory node from the device tree provided by the firmware to the device tree + * used by U-Boot. This is a common function that individual board fixup + * functions can invoke. + * + * Return: 0 on success or error otherwise. + */ +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; +}

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

Keep all the fdt fixups together for better code management.
Signed-off-by: Atish Patra atish.patra@wdc.com Reviewed-by: Bin Meng bmeng.cn@gmail.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 af12e484db9b..20e0759f135b 100644 --- a/arch/riscv/lib/fdt_fixup.c +++ b/arch/riscv/lib/fdt_fixup.c @@ -115,3 +115,36 @@ int board_fix_fdt(void *fdt) return 0; } #endif + +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; +}

On Mon, 6 Apr 2020 at 22:45, Atish Patra atish.patra@wdc.com wrote:
This series adds few DT related fixes required for Linux EFI stub to work on RISC-V.
I'm not sure how this is supposed to work, since DT reserved memory regions are not used by EFI. If you want to reserve memory on a UEFI system, you have to reserve it in the UEFI memory map from firmware. The DT reserved-memory node is taken into account too late, the /memreserve/ entries are ignored entirely.
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 master OpenSBI branch.
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 v4->v5:
- Added comments for new functions.
Changes v3->v4:
- Dropped generic efi fix patch as it is already merged.
- Moved all the fdt fixups to a common file.
- Addressed few nit comments.
Changes from v2->v3:
- Update the DT meant for OS if it is different from the one used by U-Boot
- Use different FDT api to obtain "reg" address & size to honor the cell count.
Changes from v1->v2:
- Fix the issue if chosen node is not present.
Changes from previous version:
- Renamed the DT node property to "boot-hartid" from "efi-boot-hartid".
- 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 | 150 ++++++++++++++++++++++++++ configs/sifive_fu540_defconfig | 1 + lib/fdtdec.c | 3 +- 9 files changed, 159 insertions(+), 6 deletions(-) create mode 100644 arch/riscv/lib/fdt_fixup.c
-- 2.25.1

On 4/6/20 11:01 PM, Ard Biesheuvel wrote:
On Mon, 6 Apr 2020 at 22:45, Atish Patra atish.patra@wdc.com wrote:
This series adds few DT related fixes required for Linux EFI stub to work on RISC-V.
I'm not sure how this is supposed to work, since DT reserved memory regions are not used by EFI. If you want to reserve memory on a UEFI system, you have to reserve it in the UEFI memory map from firmware. The DT reserved-memory node is taken into account too late, the /memreserve/ entries are ignored entirely.
Hello Ard,
thanks for reviewing.
What do you mean by "The DT reserved-memory node is taken into account too late"?
Cf. commit 7be64b885a36 ("cmd: bootefi: Parse reserved-memory node from DT")
Best regards
Heinrich
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 master OpenSBI branch.
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 v4->v5:
- Added comments for new functions.
Changes v3->v4:
- Dropped generic efi fix patch as it is already merged.
- Moved all the fdt fixups to a common file.
- Addressed few nit comments.
Changes from v2->v3:
- Update the DT meant for OS if it is different from the one used by U-Boot
- Use different FDT api to obtain "reg" address & size to honor the cell count.
Changes from v1->v2:
- Fix the issue if chosen node is not present.
Changes from previous version:
- Renamed the DT node property to "boot-hartid" from "efi-boot-hartid".
- 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 | 150 ++++++++++++++++++++++++++ configs/sifive_fu540_defconfig | 1 + lib/fdtdec.c | 3 +- 9 files changed, 159 insertions(+), 6 deletions(-) create mode 100644 arch/riscv/lib/fdt_fixup.c
-- 2.25.1

On Tue, 7 Apr 2020 at 08:46, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 4/6/20 11:01 PM, Ard Biesheuvel wrote:
On Mon, 6 Apr 2020 at 22:45, Atish Patra atish.patra@wdc.com wrote:
This series adds few DT related fixes required for Linux EFI stub to work on RISC-V.
I'm not sure how this is supposed to work, since DT reserved memory regions are not used by EFI. If you want to reserve memory on a UEFI system, you have to reserve it in the UEFI memory map from firmware. The DT reserved-memory node is taken into account too late, the /memreserve/ entries are ignored entirely.
Hello Ard,
thanks for reviewing.
What do you mean by "The DT reserved-memory node is taken into account too late"?
Cf. commit 7be64b885a36 ("cmd: bootefi: Parse reserved-memory node from DT")
What I mean is that the EFI stub in Linux uses memory allocation functions, expecting the firmware to ensure that those allocations do not conflict with memory descriptions and reservations in DT. So if the firmware wants to express this redundantly, by passing reservations in both reserved-memory and in the EFI memory map, that is probably fine.
Also, I must sheepishly admit that I only realize now that this patch set is against u-boot not Linux :-)
So if fixed reserved-memory regions are only being used to seed the reserved regions in the EFI memory map, you can safely ignore me. Apologies for the noise.

On Mon, Apr 6, 2020 at 11:51 PM Ard Biesheuvel ard.biesheuvel@linaro.org wrote:
On Tue, 7 Apr 2020 at 08:46, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 4/6/20 11:01 PM, Ard Biesheuvel wrote:
On Mon, 6 Apr 2020 at 22:45, Atish Patra atish.patra@wdc.com wrote:
This series adds few DT related fixes required for Linux EFI stub to work on RISC-V.
I'm not sure how this is supposed to work, since DT reserved memory regions are not used by EFI. If you want to reserve memory on a UEFI system, you have to reserve it in the UEFI memory map from firmware. The DT reserved-memory node is taken into account too late, the /memreserve/ entries are ignored entirely.
Hello Ard,
thanks for reviewing.
What do you mean by "The DT reserved-memory node is taken into account too late"?
Cf. commit 7be64b885a36 ("cmd: bootefi: Parse reserved-memory node from DT")
What I mean is that the EFI stub in Linux uses memory allocation functions, expecting the firmware to ensure that those allocations do not conflict with memory descriptions and reservations in DT. So if the firmware wants to express this redundantly, by passing reservations in both reserved-memory and in the EFI memory map, that is probably fine.
Also, I must sheepishly admit that I only realize now that this patch set is against u-boot not Linux :-)
:)
So if fixed reserved-memory regions are only being used to seed the reserved regions in the EFI memory map, you can safely ignore me.
Yeah. That's the purpose. Having reserved memory nodes in the final DT used by linux also ensures that proper Linux adds a reserved memory block or removes it from memblock entries depending on "no-map" property.
Apologies for the noise.

On Tue, Apr 7, 2020 at 10:35 AM Atish Patra atishp@atishpatra.org wrote:
On Mon, Apr 6, 2020 at 11:51 PM Ard Biesheuvel ard.biesheuvel@linaro.org wrote:
On Tue, 7 Apr 2020 at 08:46, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 4/6/20 11:01 PM, Ard Biesheuvel wrote:
On Mon, 6 Apr 2020 at 22:45, Atish Patra atish.patra@wdc.com wrote:
This series adds few DT related fixes required for Linux EFI stub to work on RISC-V.
I'm not sure how this is supposed to work, since DT reserved memory regions are not used by EFI. If you want to reserve memory on a UEFI system, you have to reserve it in the UEFI memory map from firmware. The DT reserved-memory node is taken into account too late, the /memreserve/ entries are ignored entirely.
Hello Ard,
thanks for reviewing.
What do you mean by "The DT reserved-memory node is taken into account too late"?
Cf. commit 7be64b885a36 ("cmd: bootefi: Parse reserved-memory node from DT")
What I mean is that the EFI stub in Linux uses memory allocation functions, expecting the firmware to ensure that those allocations do not conflict with memory descriptions and reservations in DT. So if the firmware wants to express this redundantly, by passing reservations in both reserved-memory and in the EFI memory map, that is probably fine.
Also, I must sheepishly admit that I only realize now that this patch set is against u-boot not Linux :-)
:)
So if fixed reserved-memory regions are only being used to seed the reserved regions in the EFI memory map, you can safely ignore me.
Yeah. That's the purpose. Having reserved memory nodes in the final DT used by linux also ensures that proper Linux adds a reserved memory block or removes it from memblock entries depending on "no-map" property.
Apologies for the noise.
-- Regards, Atish
Any other comments on the series ? It would be great if this series could be merged before v2020.07 release.

Hi Atish,
On Tue, Apr 14, 2020 at 6:02 AM Atish Patra atishp@atishpatra.org wrote:
On Tue, Apr 7, 2020 at 10:35 AM Atish Patra atishp@atishpatra.org wrote:
On Mon, Apr 6, 2020 at 11:51 PM Ard Biesheuvel ard.biesheuvel@linaro.org wrote:
On Tue, 7 Apr 2020 at 08:46, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 4/6/20 11:01 PM, Ard Biesheuvel wrote:
On Mon, 6 Apr 2020 at 22:45, Atish Patra atish.patra@wdc.com wrote:
This series adds few DT related fixes required for Linux EFI stub to work on RISC-V.
I'm not sure how this is supposed to work, since DT reserved memory regions are not used by EFI. If you want to reserve memory on a UEFI system, you have to reserve it in the UEFI memory map from firmware. The DT reserved-memory node is taken into account too late, the /memreserve/ entries are ignored entirely.
Hello Ard,
thanks for reviewing.
What do you mean by "The DT reserved-memory node is taken into account too late"?
Cf. commit 7be64b885a36 ("cmd: bootefi: Parse reserved-memory node from DT")
What I mean is that the EFI stub in Linux uses memory allocation functions, expecting the firmware to ensure that those allocations do not conflict with memory descriptions and reservations in DT. So if the firmware wants to express this redundantly, by passing reservations in both reserved-memory and in the EFI memory map, that is probably fine.
Also, I must sheepishly admit that I only realize now that this patch set is against u-boot not Linux :-)
:)
So if fixed reserved-memory regions are only being used to seed the reserved regions in the EFI memory map, you can safely ignore me.
Yeah. That's the purpose. Having reserved memory nodes in the final DT used by linux also ensures that proper Linux adds a reserved memory block or removes it from memblock entries depending on "no-map" property.
Apologies for the noise.
-- Regards, Atish
Any other comments on the series ? It would be great if this series could be merged before v2020.07 release.
I hope so if no one objects the proposed solution here in U-Boot vs. the PMP SBI extension. I need have another look at the latest version of patches though.
Regards, Bin

On Mon, Apr 13, 2020 at 3:42 PM Bin Meng bmeng.cn@gmail.com wrote:
Hi Atish,
On Tue, Apr 14, 2020 at 6:02 AM Atish Patra atishp@atishpatra.org wrote:
On Tue, Apr 7, 2020 at 10:35 AM Atish Patra atishp@atishpatra.org wrote:
On Mon, Apr 6, 2020 at 11:51 PM Ard Biesheuvel ard.biesheuvel@linaro.org wrote:
On Tue, 7 Apr 2020 at 08:46, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 4/6/20 11:01 PM, Ard Biesheuvel wrote:
On Mon, 6 Apr 2020 at 22:45, Atish Patra atish.patra@wdc.com wrote: > > This series adds few DT related fixes required for Linux EFI stub to work > on RISC-V. >
I'm not sure how this is supposed to work, since DT reserved memory regions are not used by EFI. If you want to reserve memory on a UEFI system, you have to reserve it in the UEFI memory map from firmware. The DT reserved-memory node is taken into account too late, the /memreserve/ entries are ignored entirely.
Hello Ard,
thanks for reviewing.
What do you mean by "The DT reserved-memory node is taken into account too late"?
Cf. commit 7be64b885a36 ("cmd: bootefi: Parse reserved-memory node from DT")
What I mean is that the EFI stub in Linux uses memory allocation functions, expecting the firmware to ensure that those allocations do not conflict with memory descriptions and reservations in DT. So if the firmware wants to express this redundantly, by passing reservations in both reserved-memory and in the EFI memory map, that is probably fine.
Also, I must sheepishly admit that I only realize now that this patch set is against u-boot not Linux :-)
:)
So if fixed reserved-memory regions are only being used to seed the reserved regions in the EFI memory map, you can safely ignore me.
Yeah. That's the purpose. Having reserved memory nodes in the final DT used by linux also ensures that proper Linux adds a reserved memory block or removes it from memblock entries depending on "no-map" property.
Apologies for the noise.
-- Regards, Atish
Any other comments on the series ? It would be great if this series could be merged before v2020.07 release.
I hope so if no one objects the proposed solution here in U-Boot vs. the PMP SBI extension. I need have another look at the latest version of patches though.
Thanks. As far as I know, there is no opposition to the current approach adopted in U-Boot. I am hoping EFI stub series can be merged before 5.8. If this series can go in v2020.07, RISC-V will have all required support to boot via EFI from Linux kernel v5.8 and U-Boot v2020.07.
Regards, Bin
participants (5)
-
Ard Biesheuvel
-
Atish Patra
-
Atish Patra
-
Bin Meng
-
Heinrich Schuchardt