Skip to content

Commit

Permalink
🐛 fix bug in instruction-misaligned exception handling (#734)
Browse files Browse the repository at this point in the history
  • Loading branch information
@stnolting
stnolting committed Nov 23, 2023
2 parents c85bec3 + aeb7ba3 commit 8dfd2d9
Show file tree
Hide file tree
Showing 10 changed files with 63 additions and 50 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ mimpid = 0x01040312 -> Version 01.04.03.12 -> v1.4.3.12

| Date (*dd.mm.yyyy*) | Version | Comment |
|:-------------------:|:-------:|:--------|
| 21.11.2023 | 1.9.1.4 | :bug: fix bug in handling of "misaligned instruction exception"; [#734](https://github.com/stnolting/neorv32/pull/734) |
| 20.11.2023 | 1.9.1.3 | :bug: fix wiring of FPU exception flags; [#733](https://github.com/stnolting/neorv32/pull/733) |
| 18.11.2023 | 1.9.1.2 | add XIP clock divider to fine-tune SPI frequency; [#731](https://github.com/stnolting/neorv32/pull/731) |
| 18.11.2023 | 1.9.1.1 | (re-)add SPI high-speed mode, :bug: fix bug in SPI shift register - introduced in v1.9.0.9; [#730](https://github.com/stnolting/neorv32/pull/730) |
Expand Down
2 changes: 1 addition & 1 deletion do.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ def task_BuildAndInstallSoftwareFrameworkTests():
"make -C sw/bootloader clean_all info bootloader",
# Compile and install test application, redirect UART0 TX to text.io simulation output via <UARTx_SIM_MODE> user flags
"echo 'Compiling and installing CPU/Processor test application'",
"make -C sw/example/processor_check clean_all USER_FLAGS+=-DUART0_SIM_MODE USER_FLAGS+=-DUART1_SIM_MODE USER_FLAGS+=-flto EFFORT=-Os MARCH=rv32imac_zicsr_zifencei info all",
"make -C sw/example/processor_check clean_all USER_FLAGS+=-DUART0_SIM_MODE USER_FLAGS+=-DUART1_SIM_MODE USER_FLAGS+=-flto EFFORT=-Os MARCH=rv32ima_zicsr_zifencei info all",
],
"doc": "Build all sw/example/*; install bootloader and processor check",
}
Expand Down
8 changes: 4 additions & 4 deletions docs/datasheet/cpu.adoc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -363,8 +363,8 @@ The generic type "suv(x:y)" defines a `std_ulogic_vector(x downto y)`.
|=======================
| Name | Type | Description
| `CPU_BOOT_ADDR` | suv(31:0) | CPU reset address. See section <<_address_space>>.
| `CPU_DEBUG_PARK_ADDR` | suv(31:0) | "Park loop" entry address for the <<_on_chip_debugger_ocd>>.
| `CPU_DEBUG_EXC_ADDR` | suv(31:0) | "Exception" entry address for the <<_on_chip_debugger_ocd>>.
| `CPU_DEBUG_PARK_ADDR` | suv(31:0) | "Park loop" entry address for the <<_on_chip_debugger_ocd>>, has to be 4-byte aligned.
| `CPU_DEBUG_EXC_ADDR` | suv(31:0) | "Exception" entry address for the <<_on_chip_debugger_ocd>>, has to be 4-byte aligned.
| `CPU_EXTENSION_RISCV_Sdext` | boolean | Implement RISC-V-compatible "debug" CPU operation mode required for the <<_on_chip_debugger_ocd>>.
| `CPU_EXTENSION_RISCV_Sdtrig` | boolean | Implement RISC-V-compatible trigger module. See section <<_on_chip_debugger_ocd>>.
|=======================
Expand Down Expand Up @@ -864,8 +864,8 @@ written to the according CSRs when a trap is triggered:
|=======================
| Prio. | `mcause` | RTE Trap ID | Cause | `mepc` | `mtval` | `mtinst`
7+^| **Exceptions** (_synchronous_ to instruction execution)
| 1 | `0x00000000` | `TRAP_CODE_I_MISALIGNED` | instruction fetch address misaligned | I-PC | 0 | INS
| 2 | `0x00000001` | `TRAP_CODE_I_ACCESS` | instruction fetch bus access fault | I-PC | 0 | INS
| 1 | `0x00000000` | `TRAP_CODE_I_MISALIGNED` | instruction address misaligned | PC | 0 | INS
| 2 | `0x00000001` | `TRAP_CODE_I_ACCESS` | instruction bus access fault | I-PC | 0 | INS
| 3 | `0x00000002` | `TRAP_CODE_I_ILLEGAL` | illegal instruction | PC | 0 | INS
| 4 | `0x0000000b` | `TRAP_CODE_MENV_CALL` | environment call from M-mode | PC | 0 | INS
| 5 | `0x00000008` | `TRAP_CODE_UENV_CALL` | environment call from U-mode | PC | 0 | INS
Expand Down
4 changes: 4 additions & 0 deletions docs/datasheet/cpu_csr.adoc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -373,8 +373,12 @@ or entirely denied allowing access to **none** counter CSRs.
| ISA | `Zicsr`
| Description | The `mepc` CSR provides the instruction address where execution has stopped/failed when
an instruction is triggered / an exception is raised. See section <<_traps_exceptions_and_interrupts>> for a list of all legal values.
The `mret` instruction will return to the address stored in `mepc` by automatically moving `mepc` to the program counter.
|=======================

[NOTE]
`mepc[0]` is hardwired to zero. If IALIGN = 32 (i.e. <<_c_isa_extension>> is disabled) then `mepc[1]` is also hardwired to zero.


{empty} +
[discrete]
Expand Down
5 changes: 4 additions & 1 deletion docs/datasheet/on_chip_debugger.adoc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -631,9 +631,12 @@ Cause codes in `dcsr.cause` (highest priority first):
| Reset value | `0x00000000`
| ISA | `Zicsr` & `Sdext`
| Description | The register is used to store the current program counter when debug mode is entered. The `dret` instruction will
return to `dpc` by automatically moving `dpc` to the program counter.
return to the address stored in `dpc` by automatically moving `dpc` to the program counter.
|=======================

[NOTE]
`dpc[0]` is hardwired to zero. If IALIGN = 32 (i.e. <<_c_isa_extension>> is disabled) then `dpc[1]` is also hardwired to zero.


:sectnums!:
===== **`dscratch0`**
Expand Down
67 changes: 35 additions & 32 deletions rtl/core/neorv32_cpu_control.vhd
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ entity neorv32_cpu_control is
HART_ID : std_ulogic_vector(31 downto 0); -- hardware thread ID
VENDOR_ID : std_ulogic_vector(31 downto 0); -- vendor's JEDEC ID
CPU_BOOT_ADDR : std_ulogic_vector(31 downto 0); -- cpu boot address
CPU_DEBUG_PARK_ADDR : std_ulogic_vector(31 downto 0); -- cpu debug mode parking loop entry address
CPU_DEBUG_EXC_ADDR : std_ulogic_vector(31 downto 0); -- cpu debug mode exception entry address
CPU_DEBUG_PARK_ADDR : std_ulogic_vector(31 downto 0); -- cpu debug mode parking loop entry address, 4-byte aligned
CPU_DEBUG_EXC_ADDR : std_ulogic_vector(31 downto 0); -- cpu debug mode exception entry address, 4-byte aligned
-- RISC-V CPU Extensions --
CPU_EXTENSION_RISCV_A : boolean; -- implement atomic memory operations extension?
CPU_EXTENSION_RISCV_B : boolean; -- implement bit-manipulation extension?
Expand Down Expand Up @@ -140,12 +140,11 @@ architecture neorv32_cpu_control_rtl of neorv32_cpu_control is
pc : std_ulogic_vector(XLEN-1 downto 2); -- word-aligned
reset : std_ulogic;
resp : std_ulogic; -- bus response
a_err : std_ulogic; -- alignment error
end record;
signal fetch_engine : fetch_engine_t;

-- instruction prefetch buffer (FIFO) interface --
type ipb_data_t is array (0 to 1) of std_ulogic_vector((2+16)-1 downto 0); -- status (bus_error, align_error) & 16-bit instruction
type ipb_data_t is array (0 to 1) of std_ulogic_vector(16 downto 0); -- bus_error & 16-bit instruction
type ipb_t is record
wdata : ipb_data_t;
we : std_ulogic_vector(1 downto 0); -- trigger write
Expand All @@ -163,7 +162,7 @@ architecture neorv32_cpu_control_rtl of neorv32_cpu_control is
align_clr : std_ulogic;
ci_i16 : std_ulogic_vector(15 downto 0);
ci_i32 : std_ulogic_vector(31 downto 0);
data : std_ulogic_vector((3+32)-1 downto 0); -- 3-bit status + 32-bit instruction
data : std_ulogic_vector((2+32)-1 downto 0); -- 2-bit status + 32-bit instruction
valid : std_ulogic_vector(1 downto 0); -- data word is valid
ack : std_ulogic;
end record;
Expand Down Expand Up @@ -416,16 +415,13 @@ begin
-- instruction fetch (read) request if IPB not full --
bus_req_o.stb <= '1' when (fetch_engine.state = IF_REQUEST) and (ipb.free = "11") else '0';

-- unaligned access error (no alignment exceptions possible when using C-extension) --
fetch_engine.a_err <= '1' when (fetch_engine.unaligned = '1') and (CPU_EXTENSION_RISCV_C = false) else '0';

-- instruction bus response --
-- [NOTE] PMP and alignment errors will keep pending until the triggered bus access request retires
fetch_engine.resp <= '1' when (bus_rsp_i.ack = '1') or (bus_rsp_i.err = '1') else '0';

-- IPB instruction data and status --
ipb.wdata(0) <= (bus_rsp_i.err or i_pmp_fault_i) & fetch_engine.a_err & bus_rsp_i.data(15 downto 00);
ipb.wdata(1) <= (bus_rsp_i.err or i_pmp_fault_i) & fetch_engine.a_err & bus_rsp_i.data(31 downto 16);
ipb.wdata(0) <= (bus_rsp_i.err or i_pmp_fault_i) & bus_rsp_i.data(15 downto 00);
ipb.wdata(1) <= (bus_rsp_i.err or i_pmp_fault_i) & bus_rsp_i.data(31 downto 16);

-- IPB write enable --
ipb.we(0) <= '1' when (fetch_engine.state = IF_PENDING) and (fetch_engine.resp = '1') and
Expand Down Expand Up @@ -526,20 +522,20 @@ begin
if (ipb.rdata(0)(1 downto 0) /= "11") then -- compressed
issue_engine.align_set <= ipb.avail(0); -- start of next instruction word is NOT 32-bit-aligned
issue_engine.valid(0) <= ipb.avail(0);
issue_engine.data <= ipb.rdata(0)(17 downto 16) & '1' & issue_engine.ci_i32;
issue_engine.data <= ipb.rdata(0)(16) & '1' & issue_engine.ci_i32;
else -- aligned uncompressed; use IPB(0) status flags only
issue_engine.valid <= (others => (ipb.avail(0) and ipb.avail(1)));
issue_engine.data <= ipb.rdata(0)(17 downto 16) & '0' & ipb.rdata(1)(15 downto 0) & ipb.rdata(0)(15 downto 0);
issue_engine.data <= ipb.rdata(0)(16) & '0' & ipb.rdata(1)(15 downto 0) & ipb.rdata(0)(15 downto 0);
end if;
-- start with HIGH half-word --
else
if (ipb.rdata(1)(1 downto 0) /= "11") then -- compressed
issue_engine.align_clr <= ipb.avail(1); -- start of next instruction word is 32-bit-aligned again
issue_engine.valid(1) <= ipb.avail(1);
issue_engine.data <= ipb.rdata(1)(17 downto 16) & '1' & issue_engine.ci_i32;
issue_engine.data <= ipb.rdata(1)(16) & '1' & issue_engine.ci_i32;
else -- unaligned uncompressed; use IPB(0) status flags only
issue_engine.valid <= (others => (ipb.avail(0) and ipb.avail(1)));
issue_engine.data <= ipb.rdata(0)(17 downto 16) & '0' & ipb.rdata(0)(15 downto 0) & ipb.rdata(1)(15 downto 0);
issue_engine.data <= ipb.rdata(0)(16) & '0' & ipb.rdata(0)(15 downto 0) & ipb.rdata(1)(15 downto 0);
end if;
end if;
end process issue_engine_fsm_comb;
Expand All @@ -549,7 +545,7 @@ begin
issue_engine_disabled: -- use IPB(0) status flags only
if (CPU_EXTENSION_RISCV_C = false) generate
issue_engine.valid <= (others => ipb.avail(0));
issue_engine.data <= ipb.rdata(0)(17 downto 16) & '0' & (ipb.rdata(1)(15 downto 0) & ipb.rdata(0)(15 downto 0));
issue_engine.data <= ipb.rdata(0)(16) & '0' & (ipb.rdata(1)(15 downto 0) & ipb.rdata(0)(15 downto 0));
end generate; -- /issue_engine_disabled

-- update IPB FIFOs --
Expand Down Expand Up @@ -645,8 +641,10 @@ begin
-- program counter (PC) --
if (execute_engine.pc_we = '1') then
if (execute_engine.state = BRANCH) then -- jump/taken-branch
execute_engine.pc <= alu_add_i(XLEN-1 downto 1) & '0';
else -- new/next instruction address
if (alu_add_i(1) = '0') or (CPU_EXTENSION_RISCV_C = true) then -- update only if not misaligned
execute_engine.pc <= alu_add_i(XLEN-1 downto 1) & '0';
end if;
else -- new/next instruction address (address will always be properly aligned)
execute_engine.pc <= execute_engine.next_pc(XLEN-1 downto 1) & '0';
end if;
end if;
Expand All @@ -655,9 +653,9 @@ begin
case execute_engine.state is
when TRAP_ENTER => -- starting trap environment
if (trap_ctrl.cause(5) = '1') and (CPU_EXTENSION_RISCV_Sdext = true) then -- debug mode (re-)entry
execute_engine.next_pc <= CPU_DEBUG_PARK_ADDR; -- debug mode enter; start at "parking loop" <normal_entry>
execute_engine.next_pc <= CPU_DEBUG_PARK_ADDR(XLEN-1 downto 2) & "00"; -- debug mode enter; start at "parking loop" <normal_entry>
elsif (debug_ctrl.running = '1') and (CPU_EXTENSION_RISCV_Sdext = true) then -- any other trap INSIDE debug mode
execute_engine.next_pc <= CPU_DEBUG_EXC_ADDR; -- debug mode enter: start at "parking loop" <exception_entry>
execute_engine.next_pc <= CPU_DEBUG_EXC_ADDR(XLEN-1 downto 2) & "00"; -- debug mode enter: start at "parking loop" <exception_entry>
else -- normal start of trap
if (csr.mtvec(1 downto 0) = "01") and (trap_ctrl.cause(6) = '1') then -- vectored mode + interrupt
execute_engine.next_pc <= csr.mtvec(XLEN-1 downto 7) & trap_ctrl.cause(4 downto 0) & "00"; -- pc = mtvec + 4 * mcause
Expand All @@ -681,6 +679,10 @@ begin
end if;
end process execute_engine_fsm_sync;

-- check if branch destination is misaligned --
trap_ctrl.instr_ma <= '1' when (execute_engine.state = BRANCH) and (execute_engine.pc_we = '1') and
(alu_add_i(1) = '1') and (CPU_EXTENSION_RISCV_C = false) else '0';

-- PC increment for next LINEAR instruction (+2 for compressed instr., +4 otherwise) --
execute_engine.next_pc_inc(XLEN-1 downto 4) <= (others => '0');
execute_engine.next_pc_inc(3 downto 0) <= x"4" when ((execute_engine.is_ci = '0') or (CPU_EXTENSION_RISCV_C = false)) else x"2";
Expand Down Expand Up @@ -804,7 +806,6 @@ begin
trap_ctrl.env_enter <= '0';
trap_ctrl.env_exit <= '0';
trap_ctrl.instr_be <= '0';
trap_ctrl.instr_ma <= '0';
trap_ctrl.env_call <= '0';
trap_ctrl.break_point <= '0';
--
Expand Down Expand Up @@ -857,8 +858,7 @@ begin
execute_engine.state_nxt <= TRAP_ENTER;
else -- normal execution
issue_engine.ack <= '1';
trap_ctrl.instr_be <= issue_engine.data(34); -- bus access fault during instruction fetch
trap_ctrl.instr_ma <= issue_engine.data(33) and (not bool_to_ulogic_f(CPU_EXTENSION_RISCV_C)); -- misaligned instruction fetch (if C disabled)
trap_ctrl.instr_be <= issue_engine.data(33); -- bus access fault during instruction fetch
execute_engine.is_ci_nxt <= issue_engine.data(32); -- this is a de-compressed instruction
execute_engine.ir_nxt <= issue_engine.data(31 downto 0); -- instruction word
execute_engine.pc_we <= '1'; -- pc <= next_pc
Expand Down Expand Up @@ -993,7 +993,7 @@ begin
when BRANCH => -- update PC on taken branches and jumps
-- ------------------------------------------------------------
ctrl_nxt.rf_mux <= rf_mux_npc_c; -- return address = next PC
ctrl_nxt.rf_wb_en <= execute_engine.ir(instr_opcode_lsb_c+2); -- save return address if link operation
ctrl_nxt.rf_wb_en <= execute_engine.ir(instr_opcode_lsb_c+2); -- save return address if link operation (will not happen if misaligned)
if (trap_ctrl.exc_buf(exc_illegal_c) = '0') then -- update only if not illegal instruction
execute_engine.pc_we <= '1'; -- update PC with branch DST; will be overridden in DISPATCH if branch not taken
end if;
Expand Down Expand Up @@ -1025,13 +1025,12 @@ begin
when MEM_WAIT => -- wait for bus transaction to finish
-- ------------------------------------------------------------
ctrl_nxt.rf_mux <= rf_mux_mem_c; -- RF input = memory read data
if (trap_ctrl.exc_buf(exc_laccess_c) = '1') or (trap_ctrl.exc_buf(exc_saccess_c) = '1') or -- bus access error
(trap_ctrl.exc_buf(exc_lalign_c) = '1') or (trap_ctrl.exc_buf(exc_salign_c) = '1') then -- alignment error
execute_engine.state_nxt <= DISPATCH; -- abort
elsif (lsu_wait_i = '0') then -- bus system has completed the transaction
if (lsu_wait_i = '0') or -- bus system has completed the transaction
(trap_ctrl.exc_buf(exc_salign_c) = '1') or (trap_ctrl.exc_buf(exc_saccess_c) = '1') or -- store exception
(trap_ctrl.exc_buf(exc_lalign_c) = '1') or (trap_ctrl.exc_buf(exc_laccess_c) = '1') then -- load exception
if ((CPU_EXTENSION_RISCV_A = true) and (decode_aux.opcode(2) = opcode_amo_c(2))) or -- atomic operation
(execute_engine.ir(instr_opcode_msb_c-1) = '0') then -- normal load
ctrl_nxt.rf_wb_en <= '1'; -- allow write-back to register file
ctrl_nxt.rf_wb_en <= '1'; -- allow write-back to register file (won't happen in case of exception)
end if;
execute_engine.state_nxt <= DISPATCH;
end if;
Expand Down Expand Up @@ -1071,7 +1070,7 @@ begin
-- -------------------------------------------------------------------------------------------

-- register file --
ctrl_o.rf_wb_en <= ctrl.rf_wb_en and (not trap_ctrl.exc_buf(exc_illegal_c)); -- no write if illegal instruction
ctrl_o.rf_wb_en <= ctrl.rf_wb_en and (not or_reduce_f(trap_ctrl.exc_buf(exc_laccess_c downto exc_iaccess_c))); -- no write if (non-debug) exception
ctrl_o.rf_rs1 <= execute_engine.ir(instr_rs1_msb_c downto instr_rs1_lsb_c);
ctrl_o.rf_rs2 <= execute_engine.ir(instr_rs2_msb_c downto instr_rs2_lsb_c);
ctrl_o.rf_rs3 <= execute_engine.ir(instr_rs3_msb_c downto instr_rs3_lsb_c);
Expand Down Expand Up @@ -1479,8 +1478,6 @@ begin
elsif (trap_ctrl.exc_buf(exc_lalign_c) = '1') then trap_ctrl.cause <= trap_lma_c; -- load address misaligned
elsif (trap_ctrl.exc_buf(exc_saccess_c) = '1') then trap_ctrl.cause <= trap_saf_c; -- store access fault
elsif (trap_ctrl.exc_buf(exc_laccess_c) = '1') then trap_ctrl.cause <= trap_laf_c; -- load access fault


-- standard RISC-V debug mode exceptions and interrupts --
elsif (trap_ctrl.irq_buf(irq_db_halt_c) = '1') then trap_ctrl.cause <= trap_db_halt_c; -- external halt request (async)
elsif (trap_ctrl.exc_buf(exc_db_hw_c) = '1') then trap_ctrl.cause <= trap_db_trig_c; -- hardware trigger (sync)
Expand Down Expand Up @@ -1682,7 +1679,10 @@ begin
csr.mscratch <= csr.wdata;

when csr_mepc_c => -- machine exception program counter
csr.mepc <= csr.wdata;
csr.mepc <= csr.wdata(XLEN-1 downto 1) & '0';
if (CPU_EXTENSION_RISCV_C = false) then -- RISC-V priv. spec.: MEPC[1] is masked when IALIGN = 32
csr.mepc(1) <= '0';
end if;

when csr_mcause_c => -- machine trap cause
csr.mcause <= csr.wdata(31) & csr.wdata(4 downto 0); -- type (exception/interrupt) & identifier
Expand Down Expand Up @@ -1728,6 +1728,9 @@ begin
when csr_dpc_c => -- debug mode program counter
if (CPU_EXTENSION_RISCV_Sdext = true) then
csr.dpc <= csr.wdata(XLEN-1 downto 1) & '0';
if (CPU_EXTENSION_RISCV_C = false) then -- RISC-V priv. spec.: DPC[1] is masked when IALIGN = 32
csr.dpc(1) <= '0';
end if;
end if;

when csr_dscratch0_c => -- debug mode scratch register 0
Expand Down
2 changes: 1 addition & 1 deletion rtl/core/neorv32_package.vhd
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ package neorv32_package is

-- Architecture Constants -----------------------------------------------------------------
-- -------------------------------------------------------------------------------------------
constant hw_version_c : std_ulogic_vector(31 downto 0) := x"01090103"; -- hardware version
constant hw_version_c : std_ulogic_vector(31 downto 0) := x"01090104"; -- hardware version
constant archid_c : natural := 19; -- official RISC-V architecture ID
constant XLEN : natural := 32; -- native data path width, do not change!

Expand Down
2 changes: 1 addition & 1 deletion sim/simple/neorv32_tb.simple.vhd
View file
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ begin
-- RISC-V CPU Extensions --
CPU_EXTENSION_RISCV_A => true, -- implement atomic memory operations extension?
CPU_EXTENSION_RISCV_B => true, -- implement bit-manipulation extension?
CPU_EXTENSION_RISCV_C => true, -- implement compressed extension?
CPU_EXTENSION_RISCV_C => false, -- implement compressed extension?
CPU_EXTENSION_RISCV_E => false, -- implement embedded RF extension?
CPU_EXTENSION_RISCV_M => true, -- implement mul/div extension?
CPU_EXTENSION_RISCV_U => true, -- implement user mode extension?
Expand Down
Loading

0 comments on commit 8dfd2d9

Please sign in to comment.