aboutsummaryrefslogtreecommitdiff
path: root/src/arch
diff options
context:
space:
mode:
authorRonald G Minnich <rminnich@gmail.com>2024-03-21 14:04:58 -0700
committerron minnich <rminnich@gmail.com>2024-03-27 14:45:06 +0000
commit3ee97e47a6b814df58584031c8412ae1960360b9 (patch)
tree986621842bcdcc4ed66511569367f0b9a5c5da9e /src/arch
parent559ca8b5fbf2f484925f3e37c8acc4149532e69b (diff)
arch/riscv: remove misaligned load/store/fetch handling
Testing on the unmatched shows the code no longer works completely correctly; Linux has taken over the handling of misalignment anyway, because handling it in firmware, with the growing complexity of the ISA and the awkward way in which it has to be handled, is more trouble than its worth. Plus, we don't WANT misalignment handled, magically, in firmware: the cost of getting it wrong is high (as I've spent a month learning); the performance is terrible (350x slowdown; and most toolchains now know to avoid unaligned load/store on RISC-V anyway. But, mostly, if alignment problems exist, *we need to know*, and if they're handled invisibly in firmware, we don't. The problem with invisible handling was shown a while back in the Go toolchain: runtime had a small error, such that many misaligned load/store were happening, and it was not discovered for some time. Had a trap been directed to kernel or user on misalignment, the problem would have been known immediately, not after many months. (The error, btw, was masking the address with 3, not 7, to detect misalignment; an easy mistake!). But, the coreboot code does not work any more any way, and it's not worth fixing. Remove it. Tested by booting Linux to runlevel 1; before, it would hang on an alignment fault, as the alignment code was failing (somewhere). This takes the coreboot SBI code much closer to revival. Change-Id: I84a8d433ed2f50745686a8c109d101e8718f2a46 Signed-off-by: Ronald G Minnich <rminnich@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/81416 Reviewed-by: Maximilian Brune <maximilian.brune@9elements.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Diffstat (limited to 'src/arch')
-rw-r--r--src/arch/riscv/Makefile.mk1
-rw-r--r--src/arch/riscv/include/arch/exception.h1
-rw-r--r--src/arch/riscv/misaligned.c260
-rw-r--r--src/arch/riscv/trap_handler.c3
-rw-r--r--src/arch/riscv/virtual_memory.c4
5 files changed, 4 insertions, 265 deletions
diff --git a/src/arch/riscv/Makefile.mk b/src/arch/riscv/Makefile.mk
index 1131bd5e3d..d5defea0e7 100644
--- a/src/arch/riscv/Makefile.mk
+++ b/src/arch/riscv/Makefile.mk
@@ -52,7 +52,6 @@ COMPILER_RT_ramstage = $(shell $(GCC_ramstage) $(simple_riscv_flags) -print-li
all-y += trap_util.S
all-y += trap_handler.c
all-y += fp_asm.S
-all-y += misaligned.c
all-y += sbi.c
all-y += mcall.c
all-y += virtual_memory.c
diff --git a/src/arch/riscv/include/arch/exception.h b/src/arch/riscv/include/arch/exception.h
index e807a3f3c8..2eb575e608 100644
--- a/src/arch/riscv/include/arch/exception.h
+++ b/src/arch/riscv/include/arch/exception.h
@@ -28,6 +28,5 @@ static inline void exception_init(void)
void redirect_trap(void);
void default_trap_handler(struct trapframe *tf);
void handle_supervisor_call(struct trapframe *tf);
-void handle_misaligned(struct trapframe *tf);
#endif
diff --git a/src/arch/riscv/misaligned.c b/src/arch/riscv/misaligned.c
deleted file mode 100644
index 151ed9962a..0000000000
--- a/src/arch/riscv/misaligned.c
+++ /dev/null
@@ -1,260 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-
-#include <vm.h>
-#include <arch/exception.h>
-#include <commonlib/helpers.h>
-#include <types.h>
-
-/* these functions are defined in src/arch/riscv/fp_asm.S */
-#if defined(__riscv_flen)
-#if __riscv_flen >= 32
-extern void read_f32(int regnum, uint32_t *v);
-extern void write_f32(int regnum, uint32_t *v);
-#endif // __riscv_flen >= 32
-#if __riscv_flen >= 64
-extern void read_f64(int regnum, uint64_t *v);
-extern void write_f64(int regnum, uint64_t *v);
-#endif // __riscv_flen >= 64
-#endif // defined(__riscv_flen)
-
-/* This union makes it easy to read multibyte types by byte operations. */
-union endian_buf {
- uint8_t b[8];
- uint16_t h[4];
- uint32_t w[2];
- uint64_t d[1];
- uintptr_t v;
-};
-
-/* This struct hold info of load/store instruction */
-struct memory_instruction_info {
- /* opcode/mask used to identify instruction,
- * (instruction_val) & mask == opcode */
- uint32_t opcode;
- uint32_t mask;
- /* reg_shift/reg_mask/reg_addition used to get register number
- * ((instruction_val >> reg_shift) & reg_mask) + reg_addition */
- unsigned int reg_shift;
- unsigned int reg_mask;
- unsigned int reg_addition;
- unsigned int is_fp : 1; /* mark as a float operation */
- unsigned int is_load : 1; /* mark as a load operation */
- unsigned int width : 8; /* Record the memory width of the operation */
- unsigned int sign_extend : 1; /* mark need to be sign extended */
-};
-
-static struct memory_instruction_info insn_info[] = {
-#if __riscv_xlen == 128
- { 0x00002000, 0x0000e003, 2, 7, 8, 0, 1, 16, 1}, // C.LQ
-#else
- { 0x00002000, 0x0000e003, 2, 7, 8, 1, 1, 8, 0}, // C.FLD
-#endif
- { 0x00004000, 0x0000e003, 2, 7, 8, 0, 1, 4, 1}, // C.LW
-#if __riscv_xlen == 32
- { 0x00006000, 0x0000e003, 2, 7, 8, 1, 1, 4, 0}, // C.FLW
-#else
- { 0x00006000, 0x0000e003, 2, 7, 8, 0, 1, 8, 1}, // C.LD
-#endif
-
-#if __riscv_xlen == 128
- { 0x0000a000, 0x0000e003, 2, 7, 8, 0, 0, 16, 0}, // C.SQ
-#else
- { 0x0000a000, 0x0000e003, 2, 7, 8, 1, 0, 8, 0}, // C.FSD
-#endif
- { 0x0000c000, 0x0000e003, 2, 7, 8, 0, 0, 4, 0}, // C.SW
-#if __riscv_xlen == 32
- { 0x0000e000, 0x0000e003, 2, 7, 8, 1, 0, 4, 0}, // C.FSW
-#else
- { 0x0000e000, 0x0000e003, 2, 7, 8, 0, 0, 8, 0}, // C.SD
-#endif
-
-#if __riscv_xlen == 128
- { 0x00002002, 0x0000e003, 7, 15, 0, 0, 1, 16, 1}, // C.LQSP
-#else
- { 0x00002002, 0x0000e003, 7, 15, 0, 1, 1, 8, 0}, // C.FLDSP
-#endif
- { 0x00004002, 0x0000e003, 7, 15, 0, 0, 1, 4, 1}, // C.LWSP
-#if __riscv_xlen == 32
- { 0x00006002, 0x0000e003, 7, 15, 0, 1, 1, 4, 0}, // C.FLWSP
-#else
- { 0x00006002, 0x0000e003, 7, 15, 0, 0, 1, 8, 1}, // C.LDSP
-#endif
-
-#if __riscv_xlen == 128
- { 0x0000a002, 0x0000e003, 2, 15, 0, 0, 0, 16, 0}, // C.SQSP
-#else
- { 0x0000a002, 0x0000e003, 2, 15, 0, 1, 0, 8, 0}, // C.FSDSP
-#endif
- { 0x0000c002, 0x0000e003, 2, 15, 0, 0, 0, 4, 0}, // C.SWSP
-#if __riscv_xlen == 32
- { 0x0000e002, 0x0000e003, 2, 15, 0, 1, 0, 4, 0}, // C.FSWSP
-#else
- { 0x0000e002, 0x0000e003, 2, 15, 0, 0, 0, 8, 0}, // C.SDSP
-#endif
-
- { 0x00000003, 0x0000707f, 7, 15, 0, 0, 1, 1, 1}, // LB
- { 0x00001003, 0x0000707f, 7, 15, 0, 0, 1, 2, 1}, // LH
- { 0x00002003, 0x0000707f, 7, 15, 0, 0, 1, 4, 1}, // LW
-#if __riscv_xlen > 32
- { 0x00003003, 0x0000707f, 7, 15, 0, 0, 1, 8, 1}, // LD
-#endif
- { 0x00004003, 0x0000707f, 7, 15, 0, 0, 1, 1, 0}, // LBU
- { 0x00005003, 0x0000707f, 7, 15, 0, 0, 1, 2, 0}, // LHU
- { 0x00006003, 0x0000707f, 7, 15, 0, 0, 1, 4, 0}, // LWU
-
- { 0x00000023, 0x0000707f, 20, 15, 0, 0, 0, 1, 0}, // SB
- { 0x00001023, 0x0000707f, 20, 15, 0, 0, 0, 2, 0}, // SH
- { 0x00002023, 0x0000707f, 20, 15, 0, 0, 0, 4, 0}, // SW
-#if __riscv_xlen > 32
- { 0x00003023, 0x0000707f, 20, 15, 0, 0, 0, 8, 0}, // SD
-#endif
-
-#if defined(__riscv_flen)
-#if __riscv_flen >= 32
- { 0x00002007, 0x0000707f, 7, 15, 0, 1, 1, 4, 0}, // FLW
- { 0x00003007, 0x0000707f, 7, 15, 0, 1, 1, 8, 0}, // FLD
-#endif // __riscv_flen >= 32
-
-#if __riscv_flen >= 64
- { 0x00002027, 0x0000707f, 20, 15, 0, 1, 0, 4, 0}, // FSW
- { 0x00003027, 0x0000707f, 20, 15, 0, 1, 0, 8, 0}, // FSD
-#endif // __riscv_flen >= 64
-#endif // defined(__riscv_flen)
-};
-
-static struct memory_instruction_info *match_instruction(uintptr_t insn)
-{
- int i;
- for (i = 0; i < ARRAY_SIZE(insn_info); i++)
- if ((insn_info[i].mask & insn) == insn_info[i].opcode)
- return &(insn_info[i]);
- return NULL;
-}
-
-static enum cb_err fetch_16bit_instruction(uintptr_t vaddr, uintptr_t *insn, int *size)
-{
- uint16_t ins = mprv_read_mxr_u16((uint16_t *)vaddr);
- if (EXTRACT_FIELD(ins, 0x3) != 3) {
- *insn = ins;
- *size = 2;
- return CB_SUCCESS;
- }
- return CB_ERR;
-}
-
-static enum cb_err fetch_32bit_instruction(uintptr_t vaddr, uintptr_t *insn, int *size)
-{
- uint32_t l = (uint32_t)mprv_read_mxr_u16((uint16_t *)vaddr + 0);
- uint32_t h = (uint32_t)mprv_read_mxr_u16((uint16_t *)vaddr + 1);
- uint32_t ins = (h << 16) | l;
- if ((EXTRACT_FIELD(ins, 0x3) == 3) &&
- (EXTRACT_FIELD(ins, 0x1c) != 0x7)) {
- *insn = ins;
- *size = 4;
- return CB_SUCCESS;
- }
- return CB_ERR;
-}
-
-void handle_misaligned(struct trapframe *tf)
-{
- uintptr_t insn = 0;
- union endian_buf buff;
- int insn_size = 0;
-
- /* try to fetch 16/32 bits instruction */
- if (fetch_16bit_instruction(tf->epc, &insn, &insn_size) < 0) {
- if (fetch_32bit_instruction(tf->epc, &insn, &insn_size) < 0) {
- redirect_trap();
- return;
- }
- }
-
- /* matching instruction */
- struct memory_instruction_info *match = match_instruction(insn);
-
- if (!match) {
- redirect_trap();
- return;
- }
-
- int regnum;
- regnum = ((insn >> match->reg_shift) & match->reg_mask);
- regnum = regnum + match->reg_addition;
- buff.v = 0;
- if (match->is_load) {
- /* load operation */
-
- /* reading from memory by bytes prevents misaligned
- * memory access */
- for (int i = 0; i < match->width; i++) {
- uint8_t *addr = (uint8_t *)(tf->badvaddr + i);
- buff.b[i] = mprv_read_u8(addr);
- }
-
- /* sign extend for signed integer loading */
- if (match->sign_extend)
- if (buff.v >> (8 * match->width - 1))
- buff.v |= -1 << (8 * match->width);
-
- /* write to register */
- if (match->is_fp) {
- int done = 0;
-#if defined(__riscv_flen)
-#if __riscv_flen >= 32
- /* single-precision floating-point */
- if (match->width == 4) {
- write_f32(regnum, buff.w);
- done = 1;
- }
-#endif // __riscv_flen >= 32
-#if __riscv_flen >= 64
- /* double-precision floating-point */
- if (match->width == 8) {
- write_f64(regnum, buff.d);
- done = 1;
- }
-#endif // __riscv_flen >= 64
-#endif // defined(__riscv_flen)
- if (!done)
- redirect_trap();
- } else {
- tf->gpr[regnum] = buff.v;
- }
- } else {
- /* store operation */
-
- /* reading from register */
- if (match->is_fp) {
- int done = 0;
-#if defined(__riscv_flen)
-#if __riscv_flen >= 32
- if (match->width == 4) {
- read_f32(regnum, buff.w);
- done = 1;
- }
-#endif // __riscv_flen >= 32
-#if __riscv_flen >= 64
- if (match->width == 8) {
- read_f64(regnum, buff.d);
- done = 1;
- }
-#endif // __riscv_flen >= 64
-#endif // defined(__riscv_flen)
- if (!done)
- redirect_trap();
- } else {
- buff.v = tf->gpr[regnum];
- }
-
- /* writing to memory by bytes prevents misaligned
- * memory access */
- for (int i = 0; i < match->width; i++) {
- uint8_t *addr = (uint8_t *)(tf->badvaddr + i);
- mprv_write_u8(addr, buff.b[i]);
- }
- }
-
- /* return to where we came from */
- write_csr(mepc, read_csr(mepc) + insn_size);
-}
diff --git a/src/arch/riscv/trap_handler.c b/src/arch/riscv/trap_handler.c
index f3968bb0a6..2f22ce29e5 100644
--- a/src/arch/riscv/trap_handler.c
+++ b/src/arch/riscv/trap_handler.c
@@ -119,7 +119,6 @@ void default_trap_handler(struct trapframe *tf)
}
switch (tf->cause) {
- case CAUSE_MISALIGNED_FETCH:
case CAUSE_FETCH_ACCESS:
case CAUSE_ILLEGAL_INSTRUCTION:
case CAUSE_BREAKPOINT:
@@ -133,10 +132,10 @@ void default_trap_handler(struct trapframe *tf)
case CAUSE_SUPERVISOR_ECALL:
handle_sbi(tf);
return;
+ case CAUSE_MISALIGNED_FETCH:
case CAUSE_MISALIGNED_LOAD:
case CAUSE_MISALIGNED_STORE:
print_trap_information(tf);
- handle_misaligned(tf);
return;
default:
printk(BIOS_EMERG, "================================\n");
diff --git a/src/arch/riscv/virtual_memory.c b/src/arch/riscv/virtual_memory.c
index 43e3d70304..ed2127c152 100644
--- a/src/arch/riscv/virtual_memory.c
+++ b/src/arch/riscv/virtual_memory.c
@@ -13,8 +13,10 @@
* to M-mode). In practice, this variable has been a lifesaver. It is
* still not quite determined which delegation might by unallowed by
* the spec so for now we enumerate and set them all. */
-static int delegate = 0
+static u32 delegate = 0
| (1 << CAUSE_MISALIGNED_FETCH)
+ | (1 << CAUSE_MISALIGNED_STORE)
+ | (1 << CAUSE_MISALIGNED_LOAD)
| (1 << CAUSE_FETCH_ACCESS)
| (1 << CAUSE_ILLEGAL_INSTRUCTION)
| (1 << CAUSE_BREAKPOINT)