Hello Xiang Wang,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/31179
to review the following change.
Change subject: riscv: Simplify payload handling ......................................................................
riscv: Simplify payload handling
1. Simplify payload code and convert it to C 2. Save the FDT pointer to HLS (hart-local storage). 3. Don't use mscratch to pass FDT pointer as it is used for exception handling.
Change-Id: I32bf2a99e07a65358a7f19b899259f0816eb45e8 Signed-off-by: Xiang Wang wxjstz@126.com Signed-off-by: Philipp Hug philipp@hug.cx --- M src/arch/riscv/Makefile.inc M src/arch/riscv/boot.c M src/arch/riscv/bootblock.S M src/arch/riscv/include/arch/boot.h M src/arch/riscv/include/arch/stages.h M src/arch/riscv/include/mcall.h M src/arch/riscv/mcall.c D src/arch/riscv/payload.S A src/arch/riscv/payload.c M src/arch/riscv/stages.c 10 files changed, 87 insertions(+), 57 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/79/31179/1
diff --git a/src/arch/riscv/Makefile.inc b/src/arch/riscv/Makefile.inc index e4c8468..9d91f0c 100644 --- a/src/arch/riscv/Makefile.inc +++ b/src/arch/riscv/Makefile.inc @@ -137,7 +137,7 @@ ramstage-y += smp.c ramstage-y += boot.c ramstage-y += tables.c -ramstage-y += payload.S +ramstage-y += payload.c ramstage-$(ARCH_RISCV_PMP) += pmp.c ramstage-y += \ $(top)/src/lib/memchr.c \ diff --git a/src/arch/riscv/boot.c b/src/arch/riscv/boot.c index 3d8d5d6..dab4e09 100644 --- a/src/arch/riscv/boot.c +++ b/src/arch/riscv/boot.c @@ -19,6 +19,7 @@ #include <arch/encoding.h> #include <console/console.h> #include <arch/smp/smp.h> +#include <mcall.h>
/* * A pointer to the Flattened Device Tree passed to coreboot by the boot ROM. @@ -26,27 +27,27 @@ * * This pointer is only used in ramstage! */ -const void *rom_fdt;
static void do_arch_prog_run(struct prog *prog) { - void (*doit)(void *) = prog_entry(prog); - void riscvpayload(const void *fdt, void *payload); + void (*doit)(int hart_id, void *fdt); + int hart_id; + void *fdt = prog_entry_arg(prog);
if (ENV_RAMSTAGE && prog_type(prog) == PROG_PAYLOAD) { /* - * FIXME: This is wrong and will crash. Linux can't (in early - * boot) access memory that's before its own loading address. - * We need to copy the FDT to a place where Linux can access it. + * If prog_entry_arg is not set (e.g. by fit_payload), use fdt from HLS instead. */ - const void *fdt = rom_fdt; - - printk(BIOS_SPEW, "FDT is at %p\n", fdt); - printk(BIOS_SPEW, "OK, let's go\n"); - riscvpayload(fdt, doit); + if (fdt == NULL) + fdt = HLS()->fdt; + run_payload(prog, fdt, RISCV_PAYLOAD_MODE_S); + return; }
- doit(prog_entry_arg(prog)); + doit = prog_entry(prog); + hart_id = HLS()->hart_id; + + doit(hart_id, fdt); }
void arch_prog_run(struct prog *prog) diff --git a/src/arch/riscv/bootblock.S b/src/arch/riscv/bootblock.S index 7f84215..d4b8be7 100644 --- a/src/arch/riscv/bootblock.S +++ b/src/arch/riscv/bootblock.S @@ -50,6 +50,7 @@
# initialize hart-local storage csrr a0, mhartid + csrrw a1, mscratch, zero call hls_init
li a0, CONFIG_RISCV_WORKING_HARTID diff --git a/src/arch/riscv/include/arch/boot.h b/src/arch/riscv/include/arch/boot.h index 24c1bed..34a507e 100644 --- a/src/arch/riscv/include/arch/boot.h +++ b/src/arch/riscv/include/arch/boot.h @@ -16,6 +16,12 @@ #ifndef ARCH_RISCV_INCLUDE_ARCH_BOOT_H #define ARCH_RISCV_INCLUDE_ARCH_BOOT_H
-extern const void *rom_fdt; +#include <program_loading.h> + +#define RISCV_PAYLOAD_MODE_U 0 +#define RISCV_PAYLOAD_MODE_S 1 +#define RISCV_PAYLOAD_MODE_M 3 + +void run_payload(struct prog *prog, void *fdt, int payload_mode);
#endif diff --git a/src/arch/riscv/include/arch/stages.h b/src/arch/riscv/include/arch/stages.h index 90bd60b..138298f 100644 --- a/src/arch/riscv/include/arch/stages.h +++ b/src/arch/riscv/include/arch/stages.h @@ -18,6 +18,7 @@
#include <main_decl.h>
-void stage_entry(void) __attribute__((section(".text.stage_entry"))); +void stage_entry(int hart_id, void *fdt) + __attribute__((section(".text.stage_entry")));
#endif diff --git a/src/arch/riscv/include/mcall.h b/src/arch/riscv/include/mcall.h index 29df736..cd1ed6d 100644 --- a/src/arch/riscv/include/mcall.h +++ b/src/arch/riscv/include/mcall.h @@ -52,9 +52,14 @@ int ipi_pending; uint64_t *timecmp; uint64_t *time; + void *fdt; struct blocker entry; } hls_t;
+_Static_assert( + sizeof(hls_t) == HLS_SIZE, + "HLS_SIZE must equal to sizeof(hls_t)"); + #define MACHINE_STACK_TOP() ({ \ /* coverity[uninit_use] : FALSE */ \ register uintptr_t sp asm ("sp"); \ @@ -66,7 +71,8 @@
#define MACHINE_STACK_SIZE RISCV_PGSIZE
-void hls_init(uint32_t hart_id); // need to call this before launching linux +// need to call this before launching linux +void hls_init(uint32_t hart_id, void *fdt);
/* This function is used to initialize HLS()->time/HLS()->timecmp */ void mtime_init(void); diff --git a/src/arch/riscv/mcall.c b/src/arch/riscv/mcall.c index 47cdd88..eaef644 100644 --- a/src/arch/riscv/mcall.c +++ b/src/arch/riscv/mcall.c @@ -34,10 +34,11 @@
int mcalldebug; // set this interactively for copious debug.
-void hls_init(uint32_t hart_id) +void hls_init(uint32_t hart_id, void *fdt) { printk(BIOS_SPEW, "hart %d: HLS is %p\n", hart_id, HLS()); memset(HLS(), 0, sizeof(*HLS())); + HLS()->fdt = fdt; HLS()->hart_id = hart_id;
mtime_init(); diff --git a/src/arch/riscv/payload.S b/src/arch/riscv/payload.S deleted file mode 100644 index 1b8cb96..0000000 --- a/src/arch/riscv/payload.S +++ /dev/null @@ -1,35 +0,0 @@ -/* - * This file is part of the coreboot project. - * - * Copyright (C) 2016 Google Inc - * Copyright (C) 2018 Jonathan Neuschäfer - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; version 2 of the License. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - */ - -// "return" to a payload. a0: FDT, a1: entry point - .global riscvpayload -riscvpayload: - /* Load the entry point */ - mv t0, a1 - csrw mepc, t0 - csrr t0, mstatus - - /* Set mstatus.MPP (the previous privilege mode) to supervisor mode */ - li t1, ~(3<<11) - and t0, t0, t1 - li t2, (1<<11) - or t0, t0, t2 - csrw mstatus, t0 - - /* Pass the right arguments and jump! */ - mv a1, a0 - csrr a0, mhartid - mret diff --git a/src/arch/riscv/payload.c b/src/arch/riscv/payload.c new file mode 100644 index 0000000..af2542f --- /dev/null +++ b/src/arch/riscv/payload.c @@ -0,0 +1,48 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (C) 2016 Google Inc + * Copyright (C) 2018 HardenedLinux + * Copyright (C) 2018 Jonathan Neuschäfer + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <stdint.h> +#include <arch/boot.h> +#include <arch/encoding.h> +#include <console/console.h> + +void run_payload(struct prog *prog, void *fdt, int payload_mode) +{ + void (*doit)(int hart_id, void *fdt) = prog_entry(prog); + int hart_id = read_csr(mhartid); + uintptr_t status = read_csr(mstatus); + status &= ~MSTATUS_MPIE; + status &= ~MSTATUS_MPP; + switch (payload_mode) { + case RISCV_PAYLOAD_MODE_U: + break; + case RISCV_PAYLOAD_MODE_S: + status |= MSTATUS_SPP; + break; + case RISCV_PAYLOAD_MODE_M: + doit(hart_id, fdt); + return; + default: + die("wrong privilege level for payload"); + break; + } + write_csr(mstatus, status); + write_csr(mepc, doit); + asm volatile("mv a0, %0"::"r"(hart_id):"a0"); + asm volatile("mv a1, %0"::"r"(fdt):"a1"); + asm volatile("mret"); +} diff --git a/src/arch/riscv/stages.c b/src/arch/riscv/stages.c index 5e7fa4f..07b898f 100644 --- a/src/arch/riscv/stages.c +++ b/src/arch/riscv/stages.c @@ -28,18 +28,19 @@ #include <arch/encoding.h> #include <arch/stages.h> #include <arch/smp/smp.h> +#include <rules.h> +#include <mcall.h>
-void stage_entry(void) +void stage_entry(int hart_id, void *fdt) { - smp_pause(CONFIG_RISCV_WORKING_HARTID); - /* * Save the FDT pointer before entering ramstage, because mscratch * might be overwritten in the trap handler, and there is code in * ramstage that generates misaligned access faults. */ - if (ENV_RAMSTAGE) - rom_fdt = (const void *)read_csr(mscratch); + HLS()->hart_id = hart_id; + HLS()->fdt = fdt; + smp_pause(CONFIG_RISCV_WORKING_HARTID);
main(); }