ron minnich has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/81367?usp=email )
Change subject: arch/riscv: linuxcheck framework which runs from ramstage ......................................................................
arch/riscv: linuxcheck framework which runs from ramstage
Errors in Linux booting are notoriously hard to debug.
This change adds a linuxcheck framework that allows testing of things Linux (and other kernels) needs, without having to debug Linux.
If LINUXCHECK is set during menuconfig, the linuxcheck function will be called, in S mode, from the ramstage. That function then runs code, in S mode, that tests coreboot M mode code such as SBI.
This is extremely handy: because linuxcheck is built into the ramstage, it allows calling any ramstage function, e.g. printk, if required; at the same time, the function is running in S mode, so it is very easy to test SBI calls, instruction emulation, and so on. Note that, if CONFIG_LINUXCHECK is set, the PMPs should enable RWX permissions (instead of R permissions) on the ramstage. For now, until we get the rest of the debugging done, we unconditionally set the permissions.
This is a very powerful approach that could also form the core of an SBI fuzzing suite; unlike the currently proposed one, which is based on the KVM fuzzer, this code would not require a Linux kernel to test. It is very light weight.
This is also better than the older linuxcheck payload, since it does not need libpayload or seperate compilation. It reduces the testing of SBI and other trap code to its simplest form, and allows to focus on testing SBI, not fighting build systems and large complex kernels.
Change-Id: I7d19147b9df57c63ec7301da243fd5541e9952a7 Signed-off-by: Ronald G Minnich rminnich@gmail.com --- M src/arch/riscv/Kconfig M src/arch/riscv/Makefile.mk M src/arch/riscv/include/arch/cpu.h A src/arch/riscv/linuxcheck.c M src/arch/riscv/payload.c M src/arch/riscv/trap_handler.c 6 files changed, 144 insertions(+), 16 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/81367/1
diff --git a/src/arch/riscv/Kconfig b/src/arch/riscv/Kconfig index 971dda3..1d10f88 100644 --- a/src/arch/riscv/Kconfig +++ b/src/arch/riscv/Kconfig @@ -78,6 +78,15 @@ that is the mode usually used for the payload. If the hart does not support Supervisor mode OpenSBI will again look for a hart that does support it.
+config LINUXCHECK + bool "run a linux check function in S mode from the ramstage" + depends on !RISCV_OPENSBI + default n + help + Build a linuxcheck test into the ramstage. When this is set, ramstage + will run the function in S mode. This removes many variables save + a test of how well the SBI is working. + config ARCH_RISCV_U # U (user) mode is for programs. bool diff --git a/src/arch/riscv/Makefile.mk b/src/arch/riscv/Makefile.mk index 1131bd5..056a460 100644 --- a/src/arch/riscv/Makefile.mk +++ b/src/arch/riscv/Makefile.mk @@ -125,6 +125,7 @@ ramstage-y += tables.c ramstage-y += payload.c ramstage-y += fit_payload.c +ramstage-$(CONFIG_LINUXCHECK) += linuxcheck.c
$(eval $(call create_class_compiler,rmodules,riscv))
diff --git a/src/arch/riscv/include/arch/cpu.h b/src/arch/riscv/include/arch/cpu.h index d51d8a9..d866d85 100644 --- a/src/arch/riscv/include/arch/cpu.h +++ b/src/arch/riscv/include/arch/cpu.h @@ -32,4 +32,6 @@ return (1 << mxl) * 16; }
+void linuxcheck(void); + #endif /* __ARCH_CPU_H__ */ diff --git a/src/arch/riscv/linuxcheck.c b/src/arch/riscv/linuxcheck.c new file mode 100644 index 0000000..3682229 --- /dev/null +++ b/src/arch/riscv/linuxcheck.c @@ -0,0 +1,72 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <cbmem.h> +#include <delay.h> +#include <program_loading.h> +#include <stdint.h> +#include <arch/boot.h> +#include <arch/cpu.h> +#include <arch/encoding.h> +#include <arch/exception.h> +#include <arch/pmp.h> +#include <arch/smp/atomic.h> +#include <console/console.h> +#include <mcall.h> +#include <sbi.h> +#include <vm.h> + +/* ecall code from Linux. */ + +// SPDX-License-Identifier: GPL-2.0-only +/* + * SBI initialilization and all extension implementation. + * + * Copyright (c) 2020 Western Digital Corporation or its affiliates. + */ + +/* get rid of struct returns. */ +static int sbi_ecall(int ext, int fid, unsigned long arg0, + unsigned long arg1, unsigned long arg2, + unsigned long arg3, unsigned long arg4, + unsigned long arg5, unsigned long *ret) +{ + register uintptr_t a0 asm ("a0") = (uintptr_t)(arg0); + register uintptr_t a1 asm ("a1") = (uintptr_t)(arg1); + register uintptr_t a2 asm ("a2") = (uintptr_t)(arg2); + register uintptr_t a3 asm ("a3") = (uintptr_t)(arg3); + register uintptr_t a4 asm ("a4") = (uintptr_t)(arg4); + register uintptr_t a5 asm ("a5") = (uintptr_t)(arg5); + register uintptr_t a6 asm ("a6") = (uintptr_t)(fid); + register uintptr_t a7 asm ("a7") = (uintptr_t)(ext); + asm volatile ("ecall" + : "+r" (a0), "+r" (a1) + : "r" (a2), "r" (a3), "r" (a4), "r" (a5), "r" (a6), "r" (a7) + : "memory"); + *ret = a1; + return a0; +} + +static void sbi_console_putchar(int ch) +{ + unsigned long ret2; + sbi_ecall(SBI_CONSOLE_PUTCHAR, 0, ch, 0, 0, 0, 0, 0, &ret2); +} + +/* This static is here to easy access from debuggers. */ +static uintptr_t t; + +void linuxcheck(void) +{ + extern uintptr_t testing_linuxcheck; + + testing_linuxcheck = 1; + for(int iter = 0; iter < 128; iter++) { + + + sbi_console_putchar('h'); + t = rdtime(); + printk(BIOS_EMERG, "gettime returns %lx %lx\n", testing_linuxcheck, t); + testing_linuxcheck = 1; + } + testing_linuxcheck = 0; +} diff --git a/src/arch/riscv/payload.c b/src/arch/riscv/payload.c index fd6fd3d..794203f 100644 --- a/src/arch/riscv/payload.c +++ b/src/arch/riscv/payload.c @@ -5,6 +5,7 @@ #include <program_loading.h> #include <stdint.h> #include <arch/boot.h> +#include <arch/cpu.h> #include <arch/encoding.h> #include <arch/pmp.h> #include <arch/smp/atomic.h> @@ -69,7 +70,7 @@ */ printk(BIOS_DEBUG, "setup %p %p PMP_R\n", _text, _estack); setup_pmp((u64)(uintptr_t) _text, - (u64)(uintptr_t) _estack - (u64)(uintptr_t) _text, PMP_R); + (u64)(uintptr_t) _estack - (u64)(uintptr_t) _text, 7 | PMP_R);
/* * All pmp operations should be finished when close_pmp is called. @@ -105,7 +106,25 @@ break; } printk(BIOS_DEBUG, "%s(%p, %p) mstatus: 0x%lx\n", __func__, doit, fdt, status); + printk(BIOS_EMERG, "%s: medeleg is %lx\n", __func__, read_csr(medeleg)); write_csr(mstatus, status); + + /* + * If LINUXCHECK is enabled, run it. Note: + * it may not return. If you have a JTAG debugger + * or are in QEMU, you can force it to break out, however. + */ + if (CONFIG(LINUXCHECK)) { + write_csr(mepc, linuxcheck); + asm volatile( + "mv a0, %0\n\t" + "mv a1, %1\n\t" + "mret" ::"r"(hart_id), + "r"(fdt) + : "a0", "a1"); + } + + /* Run the payload. */ write_csr(mepc, doit); printk(BIOS_DEBUG, "GO!\n"); asm volatile( diff --git a/src/arch/riscv/trap_handler.c b/src/arch/riscv/trap_handler.c index 8bcb0968..98de008 100644 --- a/src/arch/riscv/trap_handler.c +++ b/src/arch/riscv/trap_handler.c @@ -6,6 +6,7 @@ #include <arch/encoding.h> #include <arch/exception.h> #include <console/console.h> +#include <console/uart.h> #include <vm.h> #include <mcall.h> #include <sbi.h> @@ -112,6 +113,14 @@ void (*trap_handler)(struct trapframe *tf) = default_trap_handler;
/* + * testing_linuxcheck is a general purpose debug variable + * with no set use. Change it as needed. It is a useful way + * to probe bits of M mode code when triggered by linuxcheck. + * It let us do a lot of debugging, quickly, for sbi. + */ +uintptr_t testing_linuxcheck = 0; + +/* * ill is the bare minimum, very unsophisticated illegal instruction handler. * It is deliberately dumb because, in the long term, we don't intend * to use it very much. People have a way of latching on to functions like @@ -125,25 +134,41 @@ * Also, we COULD make this very general, but compilers seem to use * only a small subset of possible variations, so that may not be * worth it. + * Seriously, avoid the temptation to plug some kind of generalized "framework" + * or something in here. Every tick you take is one you don't want. + * For reference, the soft TLB load code on SGI systems was 13 instruction, + * I was once told. Anyone who wanted to add one more instruction, "because + * elegance", was not viewed kindly. */ static void ill(struct trapframe *tf) { u32 insn = (u32) (tf->badvaddr); int insn_size = 4;
- switch (insn) { - case 0xc0102773: // csrrs x14, time, x0 - tf->gpr[24] = *HLS()->time; - break; - case 0xc0102573: // csrrs x10, time, x0 - tf->gpr[20] = *HLS()->time; - break; - default: // proceed anyway. - printk(BIOS_EMERG, "Unhandled instruction: %x\n", insn); - print_trap_information(tf); - break; - } + /* always advance mepc, infinite loop is awful. */ write_csr(mepc, read_csr(mepc) + insn_size); + + /* + * this is a set of switches. put the most + * likely ones at the start, least likely at the + * end. For now, rdtime is at the end, as on + * newer architectures, it is never called. + */ + /* csrrs x##, time, x0 */ + if ((insn & ~0xf80) == (0xc0102773 & ~0xf80)) { + int reg = (insn >> 7) & 0x1f; + /* + * In this case, linuxcheck lets us quickly check the returned time. + * This let us debug our register decoding. + */ + if (testing_linuxcheck) + testing_linuxcheck = *HLS()->time; + tf->gpr[reg] = *HLS()->time; + return; + } + + printk(BIOS_EMERG, "Unhandled instruction: %x\n", insn); + print_trap_information(tf); }
void default_trap_handler(struct trapframe *tf) @@ -154,9 +179,6 @@ }
switch (tf->cause) { - case CAUSE_ILLEGAL_INSTRUCTION: - ill(tf); - return; case CAUSE_MISALIGNED_FETCH: case CAUSE_FETCH_ACCESS: case CAUSE_BREAKPOINT: @@ -175,6 +197,9 @@ print_trap_information(tf); handle_misaligned(tf); return; + case CAUSE_ILLEGAL_INSTRUCTION: + ill(tf); + return; default: printk(BIOS_EMERG, "================================\n"); printk(BIOS_EMERG, "coreboot: can not handle a trap:\n");
-
ron minnich (Code Review)