From 88fea7acfaaa2c19e074e6178244f5907eb44425 Mon Sep 17 00:00:00 2001 From: Luke Wren Date: Sun, 12 Dec 2021 18:28:23 +0000 Subject: [PATCH] Fix lockup on misaligned AMO. Add tests for misaligned/faulting AMOs. --- hdl/hazard3_core.v | 19 ++- test/sim/sw_testcases/Makefile | 1 + test/sim/sw_testcases/amo_fault.c | 95 +++++++++++ test/sim/sw_testcases/amo_misalign.c | 167 ++++++++++++++++++++ test/sim/sw_testcases/include/amo_outline.h | 71 +++++++++ 5 files changed, 346 insertions(+), 7 deletions(-) create mode 100644 test/sim/sw_testcases/amo_fault.c create mode 100644 test/sim/sw_testcases/amo_misalign.c create mode 100644 test/sim/sw_testcases/include/amo_outline.h diff --git a/hdl/hazard3_core.v b/hdl/hazard3_core.v index 32b2d8d..44cda28 100644 --- a/hdl/hazard3_core.v +++ b/hdl/hazard3_core.v @@ -409,7 +409,7 @@ hazard3_alu #( always @ (posedge clk or negedge rst_n) begin if (!rst_n) begin x_amo_phase <= 3'h0; - end else if (|EXTENSION_A && (bus_aph_ready_d || bus_dph_ready_d || m_trap_enter_vld)) begin + end else if (|EXTENSION_A && (bus_aph_ready_d || bus_dph_ready_d || m_trap_enter_vld || x_unaligned_addr)) begin if (!d_memop_is_amo) begin x_amo_phase <= 3'h0; end else if (x_stall_on_raw) begin @@ -419,8 +419,12 @@ always @ (posedge clk or negedge rst_n) begin assert(x_amo_phase == 3'h0); `endif x_amo_phase <= 3'h0; - end else if (m_trap_enter_vld) begin + end else if (m_trap_enter_vld && (x_amo_phase != 3'h4 || m_trap_enter_rdy)) begin + // If AMO caused the exception (amo_phase is 4) wait for rdy. + // Otherwise bail out to 0 to squash the in-progress AMO. x_amo_phase <= 3'h0; + end else if (x_unaligned_addr) begin + x_amo_phase <= 3'h4; end else if (x_amo_phase == 3'h1 && !bus_dph_exokay_d) begin // Load reserve fail indicates the memory region does not support // exclusives, so we will never succeed at store. Exception. @@ -620,11 +624,12 @@ end wire [W_ADDR-1:0] m_exception_return_addr; wire [W_EXCEPT-1:0] x_except = - x_csr_illegal_access ? EXCEPT_INSTR_ILLEGAL : - |EXTENSION_A && x_unaligned_addr && d_memop_is_amo ? EXCEPT_STORE_ALIGN : - |EXTENSION_A && x_amo_phase == 3'h4 ? EXCEPT_STORE_FAULT : - x_unaligned_addr && x_memop_write ? EXCEPT_STORE_ALIGN : - x_unaligned_addr && !x_memop_write ? EXCEPT_LOAD_ALIGN : d_except; + x_csr_illegal_access ? EXCEPT_INSTR_ILLEGAL : + |EXTENSION_A && x_unaligned_addr && d_memop_is_amo ? EXCEPT_STORE_ALIGN : + |EXTENSION_A && x_amo_phase == 3'h4 && x_unaligned_addr? EXCEPT_STORE_ALIGN : + |EXTENSION_A && x_amo_phase == 3'h4 ? EXCEPT_STORE_FAULT : + x_unaligned_addr && x_memop_write ? EXCEPT_STORE_ALIGN : + x_unaligned_addr && !x_memop_write ? EXCEPT_LOAD_ALIGN : d_except; // If an instruction causes an exceptional condition we do not consider it to have retired. wire x_except_counts_as_retire = x_except == EXCEPT_EBREAK || x_except == EXCEPT_MRET || x_except == EXCEPT_ECALL; diff --git a/test/sim/sw_testcases/Makefile b/test/sim/sw_testcases/Makefile index 1b793fd..d98da03 100644 --- a/test/sim/sw_testcases/Makefile +++ b/test/sim/sw_testcases/Makefile @@ -2,6 +2,7 @@ APP := hellow SRCS := ../common/init.S $(APP).c CCFLAGS := -march=rv32imac -Os MAX_CYCLES := 1000000 +INCDIR := include ../common include ../common/src_only_app.mk diff --git a/test/sim/sw_testcases/amo_fault.c b/test/sim/sw_testcases/amo_fault.c new file mode 100644 index 0000000..e680dcb --- /dev/null +++ b/test/sim/sw_testcases/amo_fault.c @@ -0,0 +1,95 @@ +#include "tb_cxxrtl_io.h" +#include "hazard3_csr.h" +#include "amo_outline.h" + +// Check AMOs which encounter bus faults generate exceptions with correct mepc +// and mcause + +int main() { + uint32_t *bad_addr = (uint32_t*)0xc0000000; + uint32_t tmp = 0; + + tb_puts("amoswap.w\n"); + tmp = amoswap(123, bad_addr); + tb_assert(tmp == 123, "bad"); + + tb_puts("amoadd.w\n"); + tmp = amoadd(123, bad_addr); + tb_assert(tmp == 123, "bad"); + + tb_puts("amoxor.w\n"); + tmp = amoxor(123, bad_addr); + tb_assert(tmp == 123, "bad"); + + tb_puts("amoand.w\n"); + tmp = amoand(123, bad_addr); + tb_assert(tmp == 123, "bad"); + + tb_puts("amoor.w\n"); + tmp = amoor(123, bad_addr); + tb_assert(tmp == 123, "bad"); + + tb_puts("amomin.w\n"); + tmp = amomin(123, bad_addr); + tb_assert(tmp == 123, "bad"); + + tb_puts("amomax.w\n"); + tmp = amomax(123, bad_addr); + tb_assert(tmp == 123, "bad"); + + tb_puts("amominu.w\n"); + tmp = amominu(123, bad_addr); + tb_assert(tmp == 123, "bad"); + + tb_puts("amomaxu.w\n"); + tmp = amomaxu(123, bad_addr); + tb_assert(tmp == 123, "bad"); + + return 0; +} + +void __attribute__((interrupt)) handle_exception() { + tb_printf("-> exception, mcause = %u\n", read_csr(mcause)); + write_csr(mcause, 0); + uint32_t mepc = read_csr(mepc); + tb_printf("instr: %04x%04x\n", *(uint16_t*)(mepc + 2), *(uint16_t*)mepc); + + if ((*(uint16_t*)mepc & 0x3) == 0x3) { + write_csr(mepc, mepc + 4); + } + else { + write_csr(mepc, mepc + 2); + } +} + +/*EXPECTED-OUTPUT*************************************************************** + +amoswap.w +-> exception, mcause = 7 +instr: 08a5a52f +amoadd.w +-> exception, mcause = 7 +instr: 00a5a52f +amoxor.w +-> exception, mcause = 7 +instr: 20a5a52f +amoand.w +-> exception, mcause = 7 +instr: 60a5a52f +amoor.w +-> exception, mcause = 7 +instr: 40a5a52f +amomin.w +-> exception, mcause = 7 +instr: 80a5a52f +amomax.w +-> exception, mcause = 7 +instr: a0a5a52f +amominu.w +-> exception, mcause = 7 +instr: c0a5a52f +amomaxu.w +-> exception, mcause = 7 +instr: e0a5a52f + +*******************************************************************************/ diff --git a/test/sim/sw_testcases/amo_misalign.c b/test/sim/sw_testcases/amo_misalign.c new file mode 100644 index 0000000..8be7e10 --- /dev/null +++ b/test/sim/sw_testcases/amo_misalign.c @@ -0,0 +1,167 @@ +#include "tb_cxxrtl_io.h" +#include "hazard3_csr.h" +#include "amo_outline.h" + +// Check AMOs which are not word-aligned generate exceptions with correct mepc +// and mcause + +int main() { + volatile uint32_t target_word = -1u; + uint32_t tmp = 0; + + tb_puts("amoswap.w\n"); + tmp = amoswap(123, (uint32_t*)((uintptr_t)&target_word + 1)); + tb_assert(tmp == 123, "bad"); + tmp = amoswap(123, (uint32_t*)((uintptr_t)&target_word + 2)); + tb_assert(tmp == 123, "bad"); + tmp = amoswap(123, (uint32_t*)((uintptr_t)&target_word + 3)); + tb_assert(tmp == 123, "bad"); + + tb_puts("amoadd.w\n"); + tmp = amoadd(123, (uint32_t*)((uintptr_t)&target_word + 1)); + tb_assert(tmp == 123, "bad"); + tmp = amoadd(123, (uint32_t*)((uintptr_t)&target_word + 2)); + tb_assert(tmp == 123, "bad"); + tmp = amoadd(123, (uint32_t*)((uintptr_t)&target_word + 3)); + tb_assert(tmp == 123, "bad"); + + tb_puts("amoxor.w\n"); + tmp = amoxor(123, (uint32_t*)((uintptr_t)&target_word + 1)); + tb_assert(tmp == 123, "bad"); + tmp = amoxor(123, (uint32_t*)((uintptr_t)&target_word + 2)); + tb_assert(tmp == 123, "bad"); + tmp = amoxor(123, (uint32_t*)((uintptr_t)&target_word + 3)); + tb_assert(tmp == 123, "bad"); + + tb_puts("amoand.w\n"); + tmp = amoand(123, (uint32_t*)((uintptr_t)&target_word + 1)); + tb_assert(tmp == 123, "bad"); + tmp = amoand(123, (uint32_t*)((uintptr_t)&target_word + 2)); + tb_assert(tmp == 123, "bad"); + tmp = amoand(123, (uint32_t*)((uintptr_t)&target_word + 3)); + tb_assert(tmp == 123, "bad"); + + tb_puts("amoor.w\n"); + tmp = amoor(123, (uint32_t*)((uintptr_t)&target_word + 1)); + tb_assert(tmp == 123, "bad"); + tmp = amoor(123, (uint32_t*)((uintptr_t)&target_word + 2)); + tb_assert(tmp == 123, "bad"); + tmp = amoor(123, (uint32_t*)((uintptr_t)&target_word + 3)); + tb_assert(tmp == 123, "bad"); + + tb_puts("amomin.w\n"); + tmp = amomin(123, (uint32_t*)((uintptr_t)&target_word + 1)); + tb_assert(tmp == 123, "bad"); + tmp = amomin(123, (uint32_t*)((uintptr_t)&target_word + 2)); + tb_assert(tmp == 123, "bad"); + tmp = amomin(123, (uint32_t*)((uintptr_t)&target_word + 3)); + tb_assert(tmp == 123, "bad"); + + tb_puts("amomax.w\n"); + tmp = amomax(123, (uint32_t*)((uintptr_t)&target_word + 1)); + tb_assert(tmp == 123, "bad"); + tmp = amomax(123, (uint32_t*)((uintptr_t)&target_word + 2)); + tb_assert(tmp == 123, "bad"); + tmp = amomax(123, (uint32_t*)((uintptr_t)&target_word + 3)); + tb_assert(tmp == 123, "bad"); + + tb_puts("amominu.w\n"); + tmp = amominu(123, (uint32_t*)((uintptr_t)&target_word + 1)); + tb_assert(tmp == 123, "bad"); + tmp = amominu(123, (uint32_t*)((uintptr_t)&target_word + 2)); + tb_assert(tmp == 123, "bad"); + tmp = amominu(123, (uint32_t*)((uintptr_t)&target_word + 3)); + tb_assert(tmp == 123, "bad"); + + tb_puts("amomaxu.w\n"); + tmp = amomaxu(123, (uint32_t*)((uintptr_t)&target_word + 1)); + tb_assert(tmp == 123, "bad"); + tmp = amomaxu(123, (uint32_t*)((uintptr_t)&target_word + 2)); + tb_assert(tmp == 123, "bad"); + tmp = amomaxu(123, (uint32_t*)((uintptr_t)&target_word + 3)); + tb_assert(tmp == 123, "bad"); + + return 0; +} + +void __attribute__((interrupt)) handle_exception() { + tb_printf("-> exception, mcause = %u\n", read_csr(mcause)); + write_csr(mcause, 0); + uint32_t mepc = read_csr(mepc); + tb_printf("instr: %04x%04x\n", *(uint16_t*)(mepc + 2), *(uint16_t*)mepc); + + if ((*(uint16_t*)mepc & 0x3) == 0x3) { + write_csr(mepc, mepc + 4); + } + else { + write_csr(mepc, mepc + 2); + } +} + +/*EXPECTED-OUTPUT*************************************************************** + +amoswap.w +-> exception, mcause = 6 +instr: 08a5a52f +-> exception, mcause = 6 +instr: 08a5a52f +-> exception, mcause = 6 +instr: 08a5a52f +amoadd.w +-> exception, mcause = 6 +instr: 00a5a52f +-> exception, mcause = 6 +instr: 00a5a52f +-> exception, mcause = 6 +instr: 00a5a52f +amoxor.w +-> exception, mcause = 6 +instr: 20a5a52f +-> exception, mcause = 6 +instr: 20a5a52f +-> exception, mcause = 6 +instr: 20a5a52f +amoand.w +-> exception, mcause = 6 +instr: 60a5a52f +-> exception, mcause = 6 +instr: 60a5a52f +-> exception, mcause = 6 +instr: 60a5a52f +amoor.w +-> exception, mcause = 6 +instr: 40a5a52f +-> exception, mcause = 6 +instr: 40a5a52f +-> exception, mcause = 6 +instr: 40a5a52f +amomin.w +-> exception, mcause = 6 +instr: 80a5a52f +-> exception, mcause = 6 +instr: 80a5a52f +-> exception, mcause = 6 +instr: 80a5a52f +amomax.w +-> exception, mcause = 6 +instr: a0a5a52f +-> exception, mcause = 6 +instr: a0a5a52f +-> exception, mcause = 6 +instr: a0a5a52f +amominu.w +-> exception, mcause = 6 +instr: c0a5a52f +-> exception, mcause = 6 +instr: c0a5a52f +-> exception, mcause = 6 +instr: c0a5a52f +amomaxu.w +-> exception, mcause = 6 +instr: e0a5a52f +-> exception, mcause = 6 +instr: e0a5a52f +-> exception, mcause = 6 +instr: e0a5a52f + +*******************************************************************************/ diff --git a/test/sim/sw_testcases/include/amo_outline.h b/test/sim/sw_testcases/include/amo_outline.h new file mode 100644 index 0000000..68ab554 --- /dev/null +++ b/test/sim/sw_testcases/include/amo_outline.h @@ -0,0 +1,71 @@ +#ifndef _AMO_OUTLINE_H +#define _AMO_OUTLINE_H + +// "what if -moutline-atomics but manually" (abusing calling convention to get +// stable register allocation, since we may log these instructions to +// confirm mepc + +static uint32_t __attribute__((naked, noinline)) amoswap(uint32_t val, uint32_t *addr) { + asm volatile ( + "amoswap.w a0, a0, (a1)\n" + "ret\n" + ); +} + +static uint32_t __attribute__((naked, noinline)) amoadd(uint32_t val, uint32_t *addr) { + asm volatile ( + "amoadd.w a0, a0, (a1)\n" + "ret\n" + ); +} + +static uint32_t __attribute__((naked, noinline)) amoxor(uint32_t val, uint32_t *addr) { + asm volatile ( + "amoxor.w a0, a0, (a1)\n" + "ret\n" + ); +} + +static uint32_t __attribute__((naked, noinline)) amoand(uint32_t val, uint32_t *addr) { + asm volatile ( + "amoand.w a0, a0, (a1)\n" + "ret\n" + ); +} + +static uint32_t __attribute__((naked, noinline)) amoor(uint32_t val, uint32_t *addr) { + asm volatile ( + "amoor.w a0, a0, (a1)\n" + "ret\n" + ); +} + +static uint32_t __attribute__((naked, noinline)) amomin(uint32_t val, uint32_t *addr) { + asm volatile ( + "amomin.w a0, a0, (a1)\n" + "ret\n" + ); +} + +static uint32_t __attribute__((naked, noinline)) amomax(uint32_t val, uint32_t *addr) { + asm volatile ( + "amomax.w a0, a0, (a1)\n" + "ret\n" + ); +} + +static uint32_t __attribute__((naked, noinline)) amominu(uint32_t val, uint32_t *addr) { + asm volatile ( + "amominu.w a0, a0, (a1)\n" + "ret\n" + ); +} + +static uint32_t __attribute__((naked, noinline)) amomaxu(uint32_t val, uint32_t *addr) { + asm volatile ( + "amomaxu.w a0, a0, (a1)\n" + "ret\n" + ); +} + +#endif