⚠️ Constrain/optimize MTVAL and MCOUNTEREN CSRs

CHANGELOG.md

@@ -32,7 +32,8 @@ mimpid = 0x01040312 -> Version 01.04.03.12 -> v1.4.3.12
 
 | Date (*dd.mm.yyyy*) | Version | Comment |
 |:-------------------:|:-------:|:--------|
-| 12.08.2023 | 1.8.7.7 | remove `Zicond` ISA extension; minor rtl code cleanups and optimizations; [#670](https://github.com/stnolting/neorv32/pull/670) |
+| 13.08.2023 | 1.8.7.8 | :warning: constrain/optimize `mtval` and `mcounteren` CSRs; [#671](https://github.com/stnolting/neorv32/pull/671) |
+| 12.08.2023 | 1.8.7.7 | remove _unratified_ `Zicond` ISA extension; minor rtl code cleanups and optimizations; [#670](https://github.com/stnolting/neorv32/pull/670) |
 | 05.08.2023 | 1.8.7.6 | :bug: fix bug: HPM width configurations below 32 bit fail; [#665](https://github.com/stnolting/neorv32/pull/665) |
 | 04.08.2023 | 1.8.7.5 | :warning: major code edits/cleanups and file renaming; [#664](https://github.com/stnolting/neorv32/pull/664) |
 | 29.07.2023 | 1.8.7.4 | RTL cleanup and optimizations (less synthesis warnings, less resource requirements); [#660](https://github.com/stnolting/neorv32/pull/660) |

docs/datasheet/cpu.adoc

@@ -30,15 +30,15 @@ test framework is available in a separate repository at GitHub: https://github.c
 .Unsupported ISA Extensions
 [TIP]
 Executing instructions or accessing CSRs from yet unsupported ISA extensions will raise an illegal
-instruction exception (-> <<_full_virtualization>>).
+instruction exception (see section  <<_full_virtualization>>).
 
 
 **Incompatibility Issues and Limitations**
 
 .`time[h]` CSRs (Wall Clock Time)
 [IMPORTANT]
-The NEORV32 does not implement the user-mode `time[h]` registers. Any access to these registers will trap.
-It is recommended that the trap handler software provides a means of accessing the platform-defined <<_machine_system_timer_mtime>>.
+The NEORV32 does not implement the `time[h]` registers. Any access to these registers will trap. It is
+recommended that the trap handler software provides a means of accessing the platform-defined <<_machine_system_timer_mtime>>.
 
 .No Hardware Support of Misaligned Memory Accesses
 [IMPORTANT]
@@ -807,8 +807,8 @@ show the values written to the according CSRs when a trap is triggered:
 
 * **I-PC** - address of interrupted instruction (instruction has _not_ been executed yet)
 * **PC** - address of instruction that caused the trap (instruction has been executed)
-* **ADR** - bad memory access address that caused the trap
-* **CMD** - the instruction word that caused the trap (zero-extended if compressed instruction)
+* **ADR** - bad data memory access address that caused the trap
+* **INS** - the (decompressed) instruction word that caused the trap
 * **0** - zero
 
 .NEORV32 Trap Listing
@@ -817,16 +817,16 @@ show the values written to the according CSRs when a trap is triggered:
 |=======================
 | Prio. | `mcause`     | ID [C]                   | Cause                                | `mepc`   | `mtval`
 6+^| **Exceptions** (_synchronous_ to instruction execution)
-| 1     | `0x00000000` | `TRAP_CODE_I_MISALIGNED` | instruction address misaligned       | **I-PC** | **0**
-| 2     | `0x00000001` | `TRAP_CODE_I_ACCESS`     | instruction access bus fault         | **I-PC** | **0**
-| 3     | `0x00000002` | `TRAP_CODE_I_ILLEGAL`    | illegal instruction                  | **PC**   | **CMD**
-| 4     | `0x0000000B` | `TRAP_CODE_MENV_CALL`    | environment call from M-mode         | **PC**   | **0**
+| 1     | `0x00000000` | `TRAP_CODE_I_MISALIGNED` | instruction fetch address misaligned | **I-PC** | **0**
+| 2     | `0x00000001` | `TRAP_CODE_I_ACCESS`     | instruction fetch bus access fault   | **I-PC** | **0**
+| 3     | `0x00000002` | `TRAP_CODE_I_ILLEGAL`    | illegal instruction                  | **PC**   | **INS**
+| 4     | `0x0000000b` | `TRAP_CODE_MENV_CALL`    | environment call from M-mode         | **PC**   | **0**
 | 5     | `0x00000008` | `TRAP_CODE_UENV_CALL`    | environment call from U-mode         | **PC**   | **0**
-| 6     | `0x00000003` | `TRAP_CODE_BREAKPOINT`   | software breakpoint / trigger firing | **PC**   | **PC**
+| 6     | `0x00000003` | `TRAP_CODE_BREAKPOINT`   | software breakpoint / trigger firing | **PC**   | **0**
 | 7     | `0x00000006` | `TRAP_CODE_S_MISALIGNED` | store address misaligned             | **PC**   | **ADR**
 | 8     | `0x00000004` | `TRAP_CODE_L_MISALIGNED` | load address misaligned              | **PC**   | **ADR**
-| 9     | `0x00000007` | `TRAP_CODE_S_ACCESS`     | store access bus fault               | **PC**   | **ADR**
-| 10    | `0x00000005` | `TRAP_CODE_L_ACCESS`     | load access bus fault                | **PC**   | **ADR**
+| 9     | `0x00000007` | `TRAP_CODE_S_ACCESS`     | store bus access fault               | **PC**   | **ADR**
+| 10    | `0x00000005` | `TRAP_CODE_L_ACCESS`     | load bus access fault                | **PC**   | **ADR**
 6+^| **Interrupts** (_asynchronous_ to instruction execution)
 | 11    | `0x80000010` | `TRAP_CODE_FIRQ_0`       | fast interrupt request channel 0     | **I-PC** | **0**
 | 12    | `0x80000011` | `TRAP_CODE_FIRQ_1`       | fast interrupt request channel 1     | **I-PC** | **0**

docs/datasheet/cpu_csr.adoc

@@ -228,8 +228,8 @@ Any illegal read access to a CSR will return zero in the operation's destination
 [options="header",grid="rows"]
 |=======================
 | Bit   | Name [C] | R/W | Function
-| 3     | `CSR_MSTATUS_MIE`  | r/w | **MIE**: Machine global interrupt enable flag
-| 7     | `CSR_MSTATUS_MPIE` | r/w | **MPIE**: Previous machine global interrupt enable flag state
+| 3     | `CSR_MSTATUS_MIE`  | r/w | **MIE**: Machine-mode interrupt enable flag
+| 7     | `CSR_MSTATUS_MPIE` | r/w | **MPIE**: Previous machine-mode interrupt enable flag state
 | 12:11 | `CSR_MSTATUS_MPP_H` : `CSR_MSTATUS_MPP_L` | r/w | **MPP**: Previous machine privilege mode, 11 = machine (M) mode, 00 = user (U) mode
 | 17    | `CSR_MSTATUS_MPRV` | r/w | **MPRV**: Effective privilege mode for load/stores in machine mode; use `MPP`'s as effective privilege mode when set; hardwired to zero if user-mode not implemented
 | 21    | `CSR_MSTATUS_TW`   | r/w | **TW**: Trap on execution of `wfi` instruction in user mode when set; hardwired to zero if user-mode not implemented
@@ -351,13 +351,18 @@ This CSR is also available if U mode is disabled, but the register is hardwired
 [cols="^1,^1,<8"]
 [options="header",grid="rows"]
 |=======================
-| Bit  | R/W | Function
-| 0    | r/w | **CY**: User-mode code is allowed to read <<_cycleh>> CSRs when set
-| 1    | r/- | **TM**: Hardwired to zero as `time[h]` CSRs are not implemented
-| 2    | r/w | **IR**: User-mode code is allowed to read <<_instreth>> CSRs when set
-| 15:3 | r/w | **HPM**: user-mode code is allowed to read <<_hpmcounterh>> CSRs when set
+| Bit  | R/W     | Function
+| 0    | r/w (!) | **CY**: User-mode is allowed to read <<_cycleh>> CSRs when set
+| 1    | r/w (!) | **TM**: User-mode is allowed to read `time[h]` CSRs when set
+| 2    | r/w (!) | **IR**: User-mode is allowed to read <<_instreth>> CSRs when set
+| 15:3 | r/w (!) | **HPM**: user-mode is allowed to read <<_hpmcounterh>> CSRs when set
 |=======================
 
+[IMPORTANT]
+Physically, the NEORV32's `mcounteren` CSR is implemented as a **single 1-bit register**. Setting _any_ bit of
+the CSR will result in all bits being set. Hence, user-mode access can only be granted for _all_ counter CSRs
+or denied for _all_ counter CSRs. 
+
 
 {empty} +
 [discrete]

rtl/core/neorv32_cpu_control.vhd

@@ -248,8 +248,8 @@ architecture neorv32_cpu_control_rtl of neorv32_cpu_control is
     wdata         : std_ulogic_vector(XLEN-1 downto 0); -- csr write data
     rdata         : std_ulogic_vector(XLEN-1 downto 0); -- csr read data
     --
-    mstatus_mie   : std_ulogic; -- global IRQ enable
-    mstatus_mpie  : std_ulogic; -- previous global IRQ enable
+    mstatus_mie   : std_ulogic; -- machine-mode IRQ enable
+    mstatus_mpie  : std_ulogic; -- previous machine-mode IRQ enable
     mstatus_mpp   : std_ulogic; -- machine previous privilege mode
     mstatus_mprv  : std_ulogic; -- effective privilege level for machine-mode load/stores
     mstatus_tw    : std_ulogic; -- do not allow user mode to execute WFI instruction when set
@@ -268,7 +268,7 @@ architecture neorv32_cpu_control_rtl of neorv32_cpu_control is
     mtvec         : std_ulogic_vector(XLEN-1 downto 0); -- machine trap-handler base address
     mtval         : std_ulogic_vector(XLEN-1 downto 0); -- machine bad address or instruction
     mscratch      : std_ulogic_vector(XLEN-1 downto 0); -- machine scratch register
-    mcounteren    : std_ulogic_vector(15 downto 0); -- machine counter access enable
+    mcounteren    : std_ulogic; -- machine counter access enable (from user-mode)
     mcountinhibit : std_ulogic_vector(15 downto 0); -- inhibit counter auto-increment
     --
     dcsr_ebreakm  : std_ulogic; -- behavior of ebreak instruction in m-mode
@@ -1229,10 +1229,9 @@ begin
     if ((csr.addr = csr_dcsr_c) or (csr.addr = csr_dpc_c) or (csr.addr = csr_dscratch0_c)) and -- debug-mode-only CSR?
        (CPU_EXTENSION_RISCV_Sdext = true) and (debug_ctrl.running = '0') then -- debug-mode implemented and not running?
       csr_priv_valid <= '0'; -- invalid access
-    elsif (csr.addr(11 downto 8) = csr_cycle_c(11 downto 8)) and -- user counter access
+    elsif (csr.addr(11 downto 8) = csr_cycle_c(11 downto 8)) and -- user-mode counter access
           ((CPU_EXTENSION_RISCV_Zicntr = true) or (CPU_EXTENSION_RISCV_Zihpm = true)) and -- any counters available?
-          (CPU_EXTENSION_RISCV_U = true) and (csr.privilege_eff = '0') and -- user mode enabled and active
-          (csr.mcounteren(to_integer(unsigned(csr.addr(3 downto 0)))) = '0') then -- access not allowed?
+          (CPU_EXTENSION_RISCV_U = true) and (csr.privilege_eff = '0') and (csr.mcounteren = '0') then -- user mode enabled, active and access not allowed?
       csr_priv_valid <= '0'; -- invalid access
     elsif (csr.addr(9 downto 8) /= "00") and (csr.privilege_eff = '0') then -- invalid privilege level
       csr_priv_valid <= '0'; -- invalid access
@@ -1281,15 +1280,15 @@ begin
           when others => illegal_cmd <= '1';
         end case;
 
-      when opcode_amo_c => -- check funct7 and funct3
+      when opcode_amo_c => -- check funct7 and funct3 via decode helper
       -- ------------------------------------------------------------
         if (CPU_EXTENSION_RISCV_A = true) and ((decode_aux.is_a_lr = '1') or (decode_aux.is_a_sc = '1')) then -- LR.W/SC.W
           illegal_cmd <= '0';
         else
           illegal_cmd <= '1';
         end if;
 
-      when opcode_alu_c => -- check operation specifier
+      when opcode_alu_c => -- check operation identifier
       -- ------------------------------------------------------------
         if ((((execute_engine.ir(instr_funct3_msb_c downto instr_funct3_lsb_c) = funct3_subadd_c) or (execute_engine.ir(instr_funct3_msb_c downto instr_funct3_lsb_c) = funct3_sr_c)) and
              (execute_engine.ir(instr_funct7_msb_c-2 downto instr_funct7_lsb_c) = "00000") and (execute_engine.ir(instr_funct7_msb_c) = '0')) or
@@ -1308,7 +1307,7 @@ begin
           illegal_cmd <= '1';
         end if;
 
-      when opcode_alui_c => -- check operation specifier
+      when opcode_alui_c => -- check operation identifier
       -- ------------------------------------------------------------
         if ((execute_engine.ir(instr_funct3_msb_c downto instr_funct3_lsb_c) = funct3_subadd_c) or
             (execute_engine.ir(instr_funct3_msb_c downto instr_funct3_lsb_c) = funct3_slt_c) or
@@ -1355,7 +1354,7 @@ begin
           illegal_cmd <= '0';
         end if;
 
-      when opcode_fop_c => -- all encodings valid if FPU enabled
+      when opcode_fop_c => -- all Zfinx encodings valid if FPU enabled
       -- ------------------------------------------------------------
         illegal_cmd <= (not bool_to_ulogic_f(CPU_EXTENSION_RISCV_Zfinx)) or (not decode_aux.is_f_op);
 
@@ -1629,7 +1628,7 @@ begin
       csr.mepc          <= (others => '0');
       csr.mcause        <= (others => '0');
       csr.mtval         <= (others => '0');
-      csr.mcounteren    <= (others => '0');
+      csr.mcounteren    <= '0';
       csr.mcountinhibit <= (others => '0');
       csr.mip_firq_nclr <= (others => '0');
       csr.dcsr_ebreakm  <= '0';
@@ -1679,12 +1678,12 @@ begin
 
           when csr_mcounteren_c => -- machine counter access enable
             if (CPU_EXTENSION_RISCV_U = true) then
-              if (CPU_EXTENSION_RISCV_Zicntr = true) then
-                csr.mcounteren(0) <= csr.wdata(0);
-                csr.mcounteren(2) <= csr.wdata(2);
-              end if;
-              if (CPU_EXTENSION_RISCV_Zihpm = true) then
-                csr.mcounteren(15 downto 3) <= csr.wdata(15 downto 3);
+              if (CPU_EXTENSION_RISCV_Zicntr = true) and (CPU_EXTENSION_RISCV_Zihpm = true) then
+                csr.mcounteren <= or_reduce_f(csr.wdata(15 downto 0)); -- hpms, instret, time, cycle
+              elsif (CPU_EXTENSION_RISCV_Zicntr = true) then
+                csr.mcounteren <= or_reduce_f(csr.wdata(02 downto 0)); -- instret, time, cycle
+              elsif (CPU_EXTENSION_RISCV_Zihpm = true) then
+                csr.mcounteren <= or_reduce_f(csr.wdata(15 downto 3)); -- hpms
               end if;
             end if;
 
@@ -1779,8 +1778,6 @@ begin
                 csr.mtval <= mar_i; -- faulting data access address
               when trap_iil_c => -- illegal instruction
                 csr.mtval <= execute_engine.ir; -- faulting instruction word
-              when trap_brk_c => -- breakpoint instruction
-                csr.mtval <= trap_ctrl.epc(XLEN-1 downto 1) & '0'; -- address of breakpoint instruction [NOTE] redundant - might be removed again
               when others => -- everything else including all interrupts
                 csr.mtval <= (others => '0');
             end case;
@@ -1807,7 +1804,7 @@ begin
         -- --------------------------------------------------------------------
         elsif (trap_ctrl.env_exit = '1') then
 
-          -- return from debug mode --
+          -- return from debug mode trap --
           if (CPU_EXTENSION_RISCV_Sdext = true) and (debug_ctrl.running = '1') then
             if (CPU_EXTENSION_RISCV_U = true) then
               csr.privilege <= csr.dcsr_prv;
@@ -1816,7 +1813,7 @@ begin
               end if;
             end if;
 
-          -- return from "normal trap" --
+          -- return from normal trap --
           else
             if (CPU_EXTENSION_RISCV_U = true) then
               csr.privilege   <= csr.mstatus_mpp; -- restore previous privilege mode
@@ -1825,7 +1822,7 @@ begin
                 csr.mstatus_mprv <= '0'; -- clear if return priv. mode is less than M
               end if;
             end if;
-            csr.mstatus_mie  <= csr.mstatus_mpie; -- restore global IRQ enable flag
+            csr.mstatus_mie  <= csr.mstatus_mpie; -- restore machine-mode IRQ enable flag
             csr.mstatus_mpie <= '1';
           end if;
 
@@ -1839,18 +1836,15 @@ begin
       -- ********************************************************************************
 
       -- hardwired bits --
-      csr.mcounteren(1)    <= '0'; -- time[h] not implemented
       csr.mcountinhibit(1) <= '0'; -- time[h] not implemented
 
       -- no base counters --
       if (CPU_EXTENSION_RISCV_Zicntr = false) then
-        csr.mcounteren(2 downto 0)    <= (others => '0');
         csr.mcountinhibit(2 downto 0) <= (others => '0');
       end if;
 
      -- no hardware performance monitors --
       if (CPU_EXTENSION_RISCV_Zihpm = false) then
-        csr.mcounteren(15 downto 3)    <= (others => '0');
         csr.mcountinhibit(15 downto 3) <= (others => '0');
       end if;
 
@@ -1862,7 +1856,7 @@ begin
         csr.mstatus_tw    <= '0';
         csr.dcsr_ebreaku  <= '0';
         csr.dcsr_prv      <= '0';
-        csr.mcounteren    <= (others => '0');
+        csr.mcounteren    <= '0';
       end if;
 
       -- no debug mode --
@@ -1931,12 +1925,11 @@ begin
 
       when csr_mcounteren_c => -- machine counter enable register
         if (CPU_EXTENSION_RISCV_U = true) then
-          csr_rdata(0) <= csr.mcounteren(0); -- cycle[h]
-          csr_rdata(2) <= csr.mcounteren(2); -- instret[h]
-          if (CPU_EXTENSION_RISCV_Zihpm = true) and (hpm_num_c > 0) then
-            for i in 3 to (hpm_num_c+3)-1 loop
-              csr_rdata(i) <= csr.mcounteren(i); -- hpmcounter*[h]
-            end loop;
+          if (CPU_EXTENSION_RISCV_Zicntr = true) then
+            csr_rdata(02 downto 0) <= (others => csr.mcounteren); -- instret[h], time[h], cycle[h]
+          end if;
+          if (CPU_EXTENSION_RISCV_Zihpm = true) then
+            csr_rdata(15 downto 3) <= (others => csr.mcounteren); -- hpmcounter*[h]
           end if;
         end if;
 

rtl/core/neorv32_package.vhd

@@ -56,7 +56,7 @@ package neorv32_package is
 
   -- Architecture Constants -----------------------------------------------------------------
   -- -------------------------------------------------------------------------------------------
-  constant hw_version_c : std_ulogic_vector(31 downto 0) := x"01080707"; -- hardware version
+  constant hw_version_c : std_ulogic_vector(31 downto 0) := x"01080708"; -- hardware version
   constant archid_c     : natural := 19; -- official RISC-V architecture ID
   constant XLEN         : natural := 32; -- native data path width, do not change!
 

sw/example/processor_check/main.c

@@ -160,7 +160,7 @@ int main() {
 
   // prepare (performance) counters
   neorv32_cpu_csr_write(CSR_MCOUNTINHIBIT, 0); // enable counter auto increment (ALL counters)
-  neorv32_cpu_csr_write(CSR_MCOUNTEREN, 7); // allow access from user-mode code to base counters only
+  neorv32_cpu_csr_write(CSR_MCOUNTEREN, -1); // allow counter access from user-mode code
 
   // set CMP of machine system timer MTIME to max to prevent an IRQ
   neorv32_mtime_set_timecmp(-1);
@@ -338,25 +338,20 @@ int main() {
   if (neorv32_cpu_csr_read(CSR_MISA) & (1 << CSR_MISA_U)) {
     cnt_test++;
 
-    // no access to instret CSR
-    neorv32_cpu_csr_clr(CSR_MCOUNTEREN, 0b111);
-    neorv32_cpu_csr_set(CSR_MCOUNTEREN, 0b001);
-
     // stop base counters
     neorv32_cpu_csr_set(CSR_MCOUNTINHIBIT, 0b101);
 
+    // no access to counter CSRs
+    neorv32_cpu_csr_write(CSR_MCOUNTEREN, 0);
+
     // read counters from user mode
     neorv32_cpu_goto_user_mode();
     {
-      asm volatile ("rdcycle   %[rd]" : [rd] "=r" (tmp_a) : );
       asm volatile ("addi      %[rd], zero, 123 \n" // this value must not change
-                    "rdinstret %[rd]" : [rd] "=r" (tmp_b) : ); // has to fail
+                    "rdinstret %[rd]" : [rd] "=r" (tmp_a) : ); // has to trap
     }
 
-    // make sure counter have NOT incremented and there was no exception during access
-    if ((tmp_a == neorv32_cpu_csr_read(CSR_MCYCLE)) &&
-        (tmp_b == 123) &&
-        ((neorv32_cpu_csr_read(CSR_MCOUNTEREN) & 7) == 0b001) &&
+    if ((tmp_a == 123) && ((neorv32_cpu_csr_read(CSR_MCOUNTEREN) & 7) == 0b000) &&
         (neorv32_cpu_csr_read(CSR_MCAUSE) == TRAP_CODE_I_ILLEGAL)) {
       test_ok();
     }