Re: [PATCH v5 1/6] riscv: Add boot hartid to Device tree
On 4/6/20 10:44 PM, Atish Patra 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..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; }
I have tested this on qemu-riscv64_defconfig by comparing the device tree before and after running helloworld.efi:
=> fdt addr $fdtcontroladdr => fdt print /chosen chosen { bootargs = ""; stdout-path = "/uart@10000000"; }; ... => dhcp $kernel_addr_r helloworld.efi => bootefi $kernel_addr_r ... => fdt addr 0x87F00000 => fdt print /chosen chosen { riscv,kernel-end = <0x00000000 0x00000000>; riscv,kernel-start = <0x00000000 0x00000000>; boot-hartid = <0x00000000>; bootargs = ""; stdout-path = "/uart@10000000"; };
The entry for boot-hardid seems to be ok.
But the riscv,kernel-end and riscv,kernel-start values are just some dummy values introduced in:
board/emulation/qemu-riscv/qemu-riscv.c:84 commit 897206c5cc5c ("riscv: qemu: clear kernel-start/-end in device tree as workaround for BBL)"
@Lukas: Why are these values set to zero and not deleted (using fdt_delprop()) from the device tree? I cannot see that we need them when booting via UEFI.
Tested-by: Heinrich Schuchardt xypron.glpk@gmx.de
Hi Heinrich,
On Tue, Apr 14, 2020 at 7:05 AM Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 4/6/20 10:44 PM, Atish Patra 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..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; }
I have tested this on qemu-riscv64_defconfig by comparing the device tree before and after running helloworld.efi:
=> fdt addr $fdtcontroladdr => fdt print /chosen chosen { bootargs = ""; stdout-path = "/uart@10000000"; }; ... => dhcp $kernel_addr_r helloworld.efi => bootefi $kernel_addr_r ... => fdt addr 0x87F00000 => fdt print /chosen chosen { riscv,kernel-end = <0x00000000 0x00000000>; riscv,kernel-start = <0x00000000 0x00000000>; boot-hartid = <0x00000000>; bootargs = ""; stdout-path = "/uart@10000000"; };
The entry for boot-hardid seems to be ok.
But the riscv,kernel-end and riscv,kernel-start values are just some dummy values introduced in:
board/emulation/qemu-riscv/qemu-riscv.c:84 commit 897206c5cc5c ("riscv: qemu: clear kernel-start/-end in device tree as workaround for BBL)"
@Lukas: Why are these values set to zero and not deleted (using fdt_delprop()) from the device tree? I cannot see that we need them when booting via UEFI.
This should be removed as BBL is legacy and we only support working with OpenSBI.
Tested-by: Heinrich Schuchardt xypron.glpk@gmx.de
Regards, Bin
Hi Heinrich, hi Bin,
On Tue, 2020-04-14 at 07:08 +0800, Bin Meng wrote:
Hi Heinrich,
On Tue, Apr 14, 2020 at 7:05 AM Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 4/6/20 10:44 PM, Atish Patra 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..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; }
I have tested this on qemu-riscv64_defconfig by comparing the device tree before and after running helloworld.efi:
=> fdt addr $fdtcontroladdr => fdt print /chosen chosen { bootargs = ""; stdout-path = "/uart@10000000"; }; ... => dhcp $kernel_addr_r helloworld.efi => bootefi $kernel_addr_r ... => fdt addr 0x87F00000 => fdt print /chosen chosen { riscv,kernel-end = <0x00000000 0x00000000>; riscv,kernel-start = <0x00000000 0x00000000>; boot-hartid = <0x00000000>; bootargs = ""; stdout-path = "/uart@10000000"; };
The entry for boot-hardid seems to be ok.
But the riscv,kernel-end and riscv,kernel-start values are just some dummy values introduced in:
board/emulation/qemu-riscv/qemu-riscv.c:84 commit 897206c5cc5c ("riscv: qemu: clear kernel-start/-end in device tree as workaround for BBL)"
@Lukas: Why are these values set to zero and not deleted (using fdt_delprop()) from the device tree? I cannot see that we need them when booting via UEFI.
That was a workaround for BBL, which required them to be set if I remember correctly.
This should be removed as BBL is legacy and we only support working with OpenSBI.
Absolutely correct, this is not required anymore since we are using OpenSBI now. I will send a patch later today reverting the workaround.
Regards, Lukas
Tested-by: Heinrich Schuchardt xypron.glpk@gmx.de
Regards, Bin
participants (3)
-
Auer, Lukas -
Bin Meng -
Heinrich Schuchardt