🐛 [rtl] cycle & instret bug fix, wishbone.we bug fix; minor rtl updates

CHANGELOG.md

@@ -32,6 +32,7 @@ mimpid = 0x01040312 => 01.04.03.12 => Version 01.04.03.12 => v1.4.3.12
 
 | Date (*dd.mm.yyyy*) | Version | Comment |
 |:-------------------:|:-------:|:--------|
+| 15.07.2022 | 1.7.4.1 | :bug: fix permanent stall of `[m]cycle[h]` and `[m]instret[h]` counter if _HPM_NUM_CNTS_ = 0; :bug: fixed bug in Wishbone `we` signal when _ASYNC_TX_ mode enabled; hardwire `dcsr.mprven` to 1; [#367](https://github.com/stnolting/neorv32/pull/367) |
 | 14.07.2022 | [**:rocket:1.7.4**](https://github.com/stnolting/neorv32/releases/tag/v1.7.4) | **New release** |
 | 14.07.2022 | 1.7.3.11 | reset all "core" CSRs to all-zero; [#366](https://github.com/stnolting/neorv32/pull/366) |
 | 13.07.2022 | 1.7.3.10 | :bug: reworked/fixed **physical memory protection**; :sparkles: added `mstatus.MPRV` flag; [#365](https://github.com/stnolting/neorv32/pull/365) |

docs/datasheet/on_chip_debugger.adoc

@@ -578,7 +578,7 @@ is raised.
 [frame="topbot",grid="none"]
 |======
 | 0x7b0 | **Debug control and status register** | `dcsr`
-3+| Reset value: `0x40000403`
+3+| Reset value: `0x40000413`
 3+| The `dcsr` CSR is compatible to the RISC-V debug spec. It is used to configure debug mode and provides additional status information.
 The following bits are implemented. The reaming bits are read-only and always read as zero.
 |======
@@ -599,7 +599,7 @@ The following bits are implemented. The reaming bits are read-only and always re
 | 9     | `stoptime`    | r/- | `0` - timers increment as usual
 | 8:6   | `cause`       | r/- | cause identifier - why debug mode was entered (see below)
 | 5     | -             | r/- | `0` - _reserved_
-| 4     | `mprven`      | r/- | `0` - `mstatus.mprv` is ignored when in debug mode
+| 4     | `mprven`      | r/- | `1` - <<_mstatus>>`.mprv` is also evaluated when in debug mode
 | 3     | `nmip`        | r/- | `0` - non-maskable interrupt is pending
 | 2     | `step`        | r/w | enable single-stepping when set
 | 1:0   | `prv`         | r/w | CPU privilege level before/after debug mode
@@ -608,7 +608,7 @@ The following bits are implemented. The reaming bits are read-only and always re
 Cause codes in `dcsr.cause` (highest priority first):
 
 * `010` - trigger by hardware <<_trigger_module>>
-* `001` - executed EBREAK instruction
+* `001` - executed `EBREAK` instruction
 * `011` - external halt request (from DM)
 * `100` - return from single-stepping
 

rtl/core/neorv32_cfs.vhd

@@ -56,6 +56,7 @@ entity neorv32_cfs is
     -- host access --
     clk_i       : in  std_ulogic; -- global clock line
     rstn_i      : in  std_ulogic; -- global reset line, low-active, use as async
+    priv_i      : in  std_ulogic; -- current CPU privilege mode
     addr_i      : in  std_ulogic_vector(31 downto 0); -- address
     rden_i      : in  std_ulogic; -- read enable
     wren_i      : in  std_ulogic; -- word write enable
@@ -187,12 +188,14 @@ begin
   --
   -- Following the interface protocol, each read or write access has to be acknowledged in the following cycle using the ack_o
   -- signal (or even later if the module needs additional time). If no ACK is generated at all, the bus access will time out
-  -- and cause a bus access fault exception.
+  -- and cause a bus access fault exception. The current CPU privilege level is available via the 'priv_i' signal (0 = user mode,
+  -- 1 = machine mode), which can be used to constrain access to certain registers or features to privileged software only.
   --
   -- This module also provides an optional ERROR signal to indicate a faulty access operation (for example when accessing an
   -- unused, read-only or "locked" CFS register address). This signal may only be set when the module is actually accessed
   -- and is set INSTEAD of the ACK signal. Setting the ERR signal will raise a bus access exception with a "Device Error" qualifier
-  -- that can be handled by the application software.
+  -- that can be handled by the application software. Note that the current privilege level should not be exposed to software to
+  -- maintain full virtualization. Hence, CFS-based "privilege escalation" should trigger a bus access exception (e.g. by setting 'err_o').
 
   err_o <= '0'; -- Tie to zero if not explicitly used.
 

rtl/core/neorv32_cpu_control.vhd

@@ -669,16 +669,16 @@ begin
       execute_engine.is_ci       <= def_rst_val_c;
       execute_engine.is_ici      <= def_rst_val_c;
       execute_engine.i_reg_last  <= (others => def_rst_val_c);
+      execute_engine.pc_last     <= (others => def_rst_val_c);
       execute_engine.next_pc     <= (others => def_rst_val_c);
+      execute_engine.branched    <= def_rst_val_c;
       -- registers that DO require a specific RESET state --
       ctrl                       <= (others => '0');
       execute_engine.pc          <= CPU_BOOT_ADDR(data_width_c-1 downto 2) & "00"; -- 32-bit aligned!
-      execute_engine.pc_last     <= CPU_BOOT_ADDR(data_width_c-1 downto 2) & "00";
       execute_engine.state       <= BRANCHED; -- reset is a branch from "somewhere"
       execute_engine.state_prev  <= BRANCHED; -- actual reset value is not relevant
       execute_engine.state_prev2 <= BRANCHED; -- actual reset value is not relevant
       execute_engine.sleep       <= '0';
-      execute_engine.branched    <= '1'; -- reset is a branch from "somewhere"
     elsif rising_edge(clk_i) then
       -- PC update --
       if (execute_engine.pc_we = '1') then
@@ -1302,7 +1302,7 @@ begin
 
       -- trigger module CSRs --
       when csr_tselect_c | csr_tdata1_c | csr_tdata2_c | csr_tdata3_c | csr_tinfo_c | csr_tcontrol_c | csr_mcontext_c | csr_scontext_c =>
-        -- access in debug-mode or M-mode (M-mode: writes to tdata* are ignored as DMODE is hardwired to 1)
+        -- access in debug-mode or M-mode only (M-mode: writes to tdata* are ignored as DMODE is hardwired to 1)
         csr_acc_valid <= (debug_ctrl.running or csr.privilege_eff) and bool_to_ulogic_f(CPU_EXTENSION_RISCV_DEBUG);
 
       -- undefined / not implemented --
@@ -1705,7 +1705,7 @@ begin
       trap_ctrl.cause_nxt <= trap_msi_c;
 
     -- interrupt: 1.7 machine timer interrupt (MTI) --
-    else--if (trap_ctrl.irq_buf(irq_mtime_irq_c) = '1') then -- last condition, so NO IF required
+    else--if (trap_ctrl.irq_buf(irq_mtime_irq_c) = '1') then -- last condition, so NO "IF" required
       trap_ctrl.cause_nxt <= trap_mti_c;
 
     end if;
@@ -2502,7 +2502,6 @@ begin
     csr.mhpmcounter_nxt(i) <= std_ulogic_vector(unsigned('0' & csr.mhpmcounter(i)) + 1);
   end generate;
 
-
   -- hpm counter read --
   hpm_connect: process(csr)
   begin
@@ -2544,36 +2543,28 @@ begin
     end if;
   end process hpmcnt_ctrl;
 
-  hpm_triggers:
-  if (HPM_NUM_CNTS /= 0) generate
-    -- counter event trigger - RISC-V-specific --
-    cnt_event(hpmcnt_event_cy_c)      <= '1' when (execute_engine.sleep = '0') else '0'; -- active cycle
-    cnt_event(hpmcnt_event_never_c)   <= '0'; -- "never" (position would be TIME)
-    cnt_event(hpmcnt_event_ir_c)      <= '1' when (execute_engine.state = EXECUTE) else '0'; -- any executed instruction
-
-    -- counter event trigger - custom / NEORV32-specific --
-    cnt_event(hpmcnt_event_cir_c)     <= '1' when (execute_engine.state = EXECUTE)   and (execute_engine.is_ci      = '1')       else '0'; -- executed compressed instruction
-    cnt_event(hpmcnt_event_wait_if_c) <= '1' when (fetch_engine.state   = S_PENDING) and (fetch_engine.state_prev   = S_PENDING) else '0'; -- instruction fetch memory wait cycle
-    cnt_event(hpmcnt_event_wait_ii_c) <= '1' when (execute_engine.state = DISPATCH)  and (execute_engine.state_prev = DISPATCH)  else '0'; -- instruction issue wait cycle
-    cnt_event(hpmcnt_event_wait_mc_c) <= '1' when (execute_engine.state = ALU_WAIT)                                              else '0'; -- multi-cycle alu-operation wait cycle
+  -- STD counters event trigger (RISC-V-specific) --
+  cnt_event(hpmcnt_event_cy_c)      <= '1' when (execute_engine.sleep = '0') else '0'; -- active cycle
+  cnt_event(hpmcnt_event_never_c)   <= '0'; -- "never" (position would be TIME)
+  cnt_event(hpmcnt_event_ir_c)      <= '1' when (execute_engine.state = EXECUTE) else '0'; -- any executed instruction
 
-    cnt_event(hpmcnt_event_load_c)    <= '1' when (ctrl(ctrl_bus_rd_c) = '1') else '0'; -- load operation
-    cnt_event(hpmcnt_event_store_c)   <= '1' when (ctrl(ctrl_bus_wr_c) = '1') else '0'; -- store operation
-    cnt_event(hpmcnt_event_wait_ls_c) <= '1' when (execute_engine.state = MEM_WAIT) and (execute_engine.state_prev2 = MEM_WAIT) else '0'; -- load/store memory wait cycle
+  -- HPM counters event trigger (NEORV32-specific) --
+  cnt_event(hpmcnt_event_cir_c)     <= '1' when (execute_engine.state = EXECUTE)   and (execute_engine.is_ci      = '1')       else '0'; -- executed compressed instruction
+  cnt_event(hpmcnt_event_wait_if_c) <= '1' when (fetch_engine.state   = S_PENDING) and (fetch_engine.state_prev   = S_PENDING) else '0'; -- instruction fetch memory wait cycle
+  cnt_event(hpmcnt_event_wait_ii_c) <= '1' when (execute_engine.state = DISPATCH)  and (execute_engine.state_prev = DISPATCH)  else '0'; -- instruction issue wait cycle
+  cnt_event(hpmcnt_event_wait_mc_c) <= '1' when (execute_engine.state = ALU_WAIT)                                              else '0'; -- multi-cycle alu-operation wait cycle
 
-    cnt_event(hpmcnt_event_jump_c)    <= '1' when (execute_engine.state = BRANCH)   and (execute_engine.i_reg(instr_opcode_lsb_c+2) = '1') else '0'; -- jump (unconditional)
-    cnt_event(hpmcnt_event_branch_c)  <= '1' when (execute_engine.state = BRANCH)   and (execute_engine.i_reg(instr_opcode_lsb_c+2) = '0') else '0'; -- branch (conditional, taken or not taken)
-    cnt_event(hpmcnt_event_tbranch_c) <= '1' when (execute_engine.state = BRANCHED) and (execute_engine.state_prev = BRANCH) and
-                                                                                        (execute_engine.i_reg(instr_opcode_lsb_c+2) = '0') else '0'; -- taken branch (conditional)
+  cnt_event(hpmcnt_event_load_c)    <= '1' when (ctrl(ctrl_bus_rd_c) = '1') else '0'; -- load operation
+  cnt_event(hpmcnt_event_store_c)   <= '1' when (ctrl(ctrl_bus_wr_c) = '1') else '0'; -- store operation
+  cnt_event(hpmcnt_event_wait_ls_c) <= '1' when (execute_engine.state = MEM_WAIT) and (execute_engine.state_prev2 = MEM_WAIT) else '0'; -- load/store memory wait cycle
 
-    cnt_event(hpmcnt_event_trap_c)    <= '1' when (trap_ctrl.env_start_ack = '1')                                    else '0'; -- entered trap
-    cnt_event(hpmcnt_event_illegal_c) <= '1' when (trap_ctrl.env_start_ack = '1') and (trap_ctrl.cause = trap_iil_c) else '0'; -- illegal operation
-  end generate; --/hpm_triggers
+  cnt_event(hpmcnt_event_jump_c)    <= '1' when (execute_engine.state = BRANCH)   and (execute_engine.i_reg(instr_opcode_lsb_c+2) = '1') else '0'; -- jump (unconditional)
+  cnt_event(hpmcnt_event_branch_c)  <= '1' when (execute_engine.state = BRANCH)   and (execute_engine.i_reg(instr_opcode_lsb_c+2) = '0') else '0'; -- branch (conditional, taken or not taken)
+  cnt_event(hpmcnt_event_tbranch_c) <= '1' when (execute_engine.state = BRANCHED) and (execute_engine.state_prev = BRANCH) and
+                                                                                      (execute_engine.i_reg(instr_opcode_lsb_c+2) = '0') else '0'; -- taken branch (conditional)
 
-  hpm_triggers_none:
-  if (HPM_NUM_CNTS = 0) generate
-    cnt_event <= (others => '0');
-  end generate; --/hpm_triggers_none
+  cnt_event(hpmcnt_event_trap_c)    <= '1' when (trap_ctrl.env_start_ack = '1')                                    else '0'; -- entered trap
+  cnt_event(hpmcnt_event_illegal_c) <= '1' when (trap_ctrl.env_start_ack = '1') and (trap_ctrl.cause = trap_iil_c) else '0'; -- illegal operation
 
 
 -- ****************************************************************************************************************************
@@ -2652,7 +2643,7 @@ begin
   csr.dcsr_rd(09)           <= '0'; -- stoptime: timers increment as usual
   csr.dcsr_rd(08 downto 06) <= csr.dcsr_cause; -- debug mode entry cause
   csr.dcsr_rd(05)           <= '0'; -- reserved
-  csr.dcsr_rd(04)           <= '0'; -- mprven: mstatus.mprv is ignored in debug mode
+  csr.dcsr_rd(04)           <= '1'; -- mprven: mstatus.mprv is also evaluated in debug mode
   csr.dcsr_rd(03)           <= '0'; -- nmip: no pending non-maskable interrupt
   csr.dcsr_rd(02)           <= csr.dcsr_step; -- step: single-step mode
   csr.dcsr_rd(01 downto 00) <= (others => csr.dcsr_prv); -- prv: privilege mode when debug mode was entered

rtl/core/neorv32_debug_dtm.vhd

@@ -251,9 +251,9 @@ begin
       dmi_ctrl.dmihardreset <= '1';
       dmi_ctrl.dmireset     <= '1';
       dmi_ctrl.err          <= '0';
-      dmi_ctrl.rdata        <= (others => '-');
-      dmi_ctrl.wdata        <= (others => '-');
-      dmi_ctrl.addr         <= (others => '-');
+      dmi_ctrl.rdata        <= (others => '0');
+      dmi_ctrl.wdata        <= (others => '0');
+      dmi_ctrl.addr         <= (others => '0');
     elsif rising_edge(clk_i) then
 
       -- DMI status and control --

rtl/core/neorv32_fifo.vhd

@@ -75,7 +75,7 @@ architecture neorv32_fifo_rtl of neorv32_fifo is
     r_pnt : std_ulogic_vector(index_size_f(FIFO_DEPTH) downto 0); -- read pointer
     level : std_ulogic_vector(index_size_f(FIFO_DEPTH) downto 0); -- fill count
     data  : fifo_data_t; -- fifo memory
-    datas : std_ulogic_vector(FIFO_WIDTH-1 downto 0);
+    buf   : std_ulogic_vector(FIFO_WIDTH-1 downto 0); -- if single-entry FIFO
     match : std_ulogic;
     empty : std_ulogic;
     full  : std_ulogic;
@@ -150,23 +150,23 @@ begin
 
   -- FIFO Memory ----------------------------------------------------------------------------
   -- -------------------------------------------------------------------------------------------
-  fifo_memory_write: process(clk_i)
+  fifo_write: process(clk_i)
   begin
     if rising_edge(clk_i) then
       if (fifo.we = '1') then
         if (FIFO_DEPTH = 1) then
-          fifo.datas <= wdata_i;
+          fifo.buf <= wdata_i;
         else
           fifo.data(to_integer(unsigned(fifo.w_pnt(fifo.w_pnt'left-1 downto 0)))) <= wdata_i;
         end if;
       end if;
     end if;
-  end process fifo_memory_write;
+  end process fifo_write;
 
   -- "asynchronous" read --
   fifo_read_async:
   if (FIFO_RSYNC = false) generate
-    rdata_o <= fifo.datas when (FIFO_DEPTH = 1) else fifo.data(to_integer(unsigned(fifo.r_pnt(fifo.r_pnt'left-1 downto 0))));
+    rdata_o <= fifo.buf when (FIFO_DEPTH = 1) else fifo.data(to_integer(unsigned(fifo.r_pnt(fifo.r_pnt'left-1 downto 0))));
   end generate;
 
   -- synchronous read --
@@ -176,7 +176,7 @@ begin
     begin
       if rising_edge(clk_i) then
         if (FIFO_DEPTH = 1) then
-          rdata_o <= fifo.datas;
+          rdata_o <= fifo.buf;
         else
           rdata_o <= fifo.data(to_integer(unsigned(fifo.r_pnt(fifo.r_pnt'left-1 downto 0))));
         end if;

rtl/core/neorv32_package.vhd

@@ -63,7 +63,7 @@ package neorv32_package is
   -- Architecture Constants (do not modify!) ------------------------------------------------
   -- -------------------------------------------------------------------------------------------
   constant data_width_c : natural := 32; -- native data path width - do not change!
-  constant hw_version_c : std_ulogic_vector(31 downto 0) := x"01070400"; -- NEORV32 version - no touchy!
+  constant hw_version_c : std_ulogic_vector(31 downto 0) := x"01070401"; -- NEORV32 version - no touchy!
   constant archid_c     : natural := 19; -- official RISC-V architecture ID - hands off!
 
   -- Check if we're inside the Matrix -------------------------------------------------------
@@ -1887,6 +1887,7 @@ package neorv32_package is
       -- host access --
       clk_i       : in  std_ulogic; -- global clock line
       rstn_i      : in  std_ulogic; -- global reset line, low-active, use as async
+      priv_i      : in  std_ulogic; -- current CPU privilege mode
       addr_i      : in  std_ulogic_vector(31 downto 0); -- address
       rden_i      : in  std_ulogic; -- read enable
       wren_i      : in  std_ulogic; -- word write enable

rtl/core/neorv32_top.vhd

@@ -995,6 +995,7 @@ begin
       -- host access --
       clk_i       => clk_i,                    -- global clock line
       rstn_i      => rstn_int,                 -- global reset line, low-active, use as async
+      priv_i      => p_bus.priv,               -- current CPU privilege mode
       addr_i      => p_bus.addr,               -- address
       rden_i      => io_rden,                  -- read enable
       wren_i      => io_wren,                  -- word write enable

rtl/core/neorv32_wishbone.vhd

@@ -148,7 +148,7 @@ begin
   -- -------------------------------------------------------------------------------------------
   assert false report
   "NEORV32 PROCESSOR CONFIG NOTE: Ext. Bus Interface - " &
-  cond_sel_string_f(PIPE_MODE, "PIPELINED", "CASSIC/STANDARD") & "Wishbone protocol, " &
+  cond_sel_string_f(PIPE_MODE, "PIPELINED", "CLASSIC/STANDARD") & " Wishbone protocol, " &
   cond_sel_string_f(boolean(BUS_TIMEOUT /= 0), "auto-timeout (" & integer'image(BUS_TIMEOUT) & " cycles), ", "NO auto-timeout, ") &
   cond_sel_string_f(BIG_ENDIAN, "BIG", "LITTLE") & "-endian byte order, " &
   cond_sel_string_f(ASYNC_RX, "ASYNC ", "buffered ") & "RX path, " &
@@ -263,7 +263,7 @@ begin
 
   wb_adr_o <= addr_i when (ASYNC_TX = true) else ctrl.adr;
   wb_dat_o <= data_i when (ASYNC_TX = true) else ctrl.wdat;
-  wb_we_o  <= (wren_i or ctrl.we)  when (ASYNC_TX = true) else ctrl.we;
+  wb_we_o  <= (wren_i or (ctrl.we and ctrl.state)) when (ASYNC_TX = true) else ctrl.we;
   wb_sel_o <= end_byteen when (ASYNC_TX = true) else ctrl.sel;
   wb_stb_o <= stb_int when (PIPE_MODE = true) else cyc_int;
   wb_cyc_o <= cyc_int;

sim/neorv32_tb.vhd

@@ -559,7 +559,7 @@ begin
 
         -- bus output register --
         wb_mem_a.err <= '0';
-        if (ext_mem_a.ack(ext_mem_a_latency_c-1) = '1') and (wb_mem_a.cyc = '1') and (wb_mem_a.ack = '0') then
+        if (ext_mem_a.ack(ext_mem_a_latency_c-1) = '1') and (wb_mem_a.cyc = '1') then
           wb_mem_a.rdata <= ext_mem_a.rdata(ext_mem_a_latency_c-1);
           wb_mem_a.ack   <= '1';
         else
@@ -608,7 +608,7 @@ begin
 
       -- bus output register --
       wb_mem_b.err <= '0';
-      if (ext_mem_b.ack(ext_mem_b_latency_c-1) = '1') and (wb_mem_b.cyc = '1') and (wb_mem_b.ack = '0') then
+      if (ext_mem_b.ack(ext_mem_b_latency_c-1) = '1') and (wb_mem_b.cyc = '1') then
         wb_mem_b.rdata <= ext_mem_b.rdata(ext_mem_b_latency_c-1);
         wb_mem_b.ack   <= '1';
       else
@@ -647,7 +647,7 @@ begin
       end if;
 
       -- bus output register --
-      if (ext_mem_c.ack(ext_mem_c_latency_c-1) = '1') and (wb_mem_c.cyc = '1') and (wb_mem_c.ack = '0') then
+      if (ext_mem_c.ack(ext_mem_c_latency_c-1) = '1') and (wb_mem_c.cyc = '1') then
         wb_mem_c.rdata <= ext_mem_c.rdata(ext_mem_c_latency_c-1);
         wb_mem_c.ack   <= '1';
         wb_mem_c.err   <= '0';

sim/simple/neorv32_tb.simple.vhd

@@ -426,7 +426,7 @@ begin
 
         -- bus output register --
         wb_mem_a.err <= '0';
-        if (ext_mem_a.ack(ext_mem_a_latency_c-1) = '1') and (wb_mem_a.cyc = '1') and (wb_mem_a.ack = '0') then
+        if (ext_mem_a.ack(ext_mem_a_latency_c-1) = '1') and (wb_mem_a.cyc = '1') then
           wb_mem_a.rdata <= ext_mem_a.rdata(ext_mem_a_latency_c-1);
           wb_mem_a.ack   <= '1';
         else
@@ -475,7 +475,7 @@ begin
 
       -- bus output register --
       wb_mem_b.err <= '0';
-      if (ext_mem_b.ack(ext_mem_b_latency_c-1) = '1') and (wb_mem_b.cyc = '1') and (wb_mem_b.ack = '0') then
+      if (ext_mem_b.ack(ext_mem_b_latency_c-1) = '1') and (wb_mem_b.cyc = '1') then
         wb_mem_b.rdata <= ext_mem_b.rdata(ext_mem_b_latency_c-1);
         wb_mem_b.ack   <= '1';
       else
@@ -514,7 +514,7 @@ begin
       end if;
 
       -- bus output register --
-      if (ext_mem_c.ack(ext_mem_c_latency_c-1) = '1') and (wb_mem_c.cyc = '1') and (wb_mem_c.ack = '0') then
+      if (ext_mem_c.ack(ext_mem_c_latency_c-1) = '1') and (wb_mem_c.cyc = '1') then
         wb_mem_c.rdata <= ext_mem_c.rdata(ext_mem_c_latency_c-1);
         wb_mem_c.ack   <= '1';
         wb_mem_c.err   <= '0';