Trigger types 0x5 and 0xF #706

silabs-oysteink · 2022-11-29T08:22:32Z

Implemented trigger type 0x5 (exception trigger) and 0xF (disabled trigger).

WARL handling of both tdata1 and tdata2 to ensure no invalid combination of tdata1 and tdata2 can occure.
Exception trigger uses same logic as single step debug entry to allow for exception side effects to happen before debug entry is done.

Signed-off-by: Oystein Knauserud [email protected]

…igger). - WARL handling of both tdata1 and tdata2 to ensure no invalid combination of tdata1 and tdata2 can occure. - Exception trigger uses same logic as single step debug entry to allow for exception side effects to happen before debug entry is done. Signed-off-by: Oystein Knauserud <[email protected]>

Signed-off-by: Oystein Knauserud <[email protected]>

Silabs-ArjanB · 2022-11-29T09:10:44Z

rtl/cv32e40x_core.sv

@@ -811,6 +813,7 @@ module cv32e40x_core import cv32e40x_pkg::*;
    // Debug
    .trigger_match_if_o         ( trigger_match_if       ),
    .trigger_match_ex_o         ( trigger_match_ex       ),
+    .etrigger_wb_o              ( etrigger_wb       ),


Align brackets

Silabs-ArjanB · 2022-11-29T09:21:44Z

rtl/cv32e40x_debug_triggers.sv

@@ -243,11 +297,30 @@ import cv32e40x_pkg::*;
                                           (tdata1_rdata[idx][MCONTROL6_U] && (priv_lvl_ex_i == PRIV_LVL_U));

        // Enable LSU address matching
-        assign lsu_addr_match_en[idx] = lsu_valid_ex_i && ((tdata1_rdata[idx][MCONTROL6_LOAD] && !lsu_we_ex_i) || (tdata1_rdata[idx][MCONTROL6_STORE] && lsu_we_ex_i));
+        assign lsu_addr_match_en[idx] = lsu_valid_ex_i && (tdata1_rdata[idx][TDATA1_TTYPE_HIGH:TDATA1_TTYPE_LOW] == TTYPE_MCONTROL6) &&


Not consistent with IF trigger match handling. Make sure that the (tdata1_rdata[idx][TDATA1_TTYPE_HIGH:TDATA1_TTYPE_LOW] == TTYPE_MCONTROL6) classification is done on the same type of signal, so for example on trigger_match_* signals.

Moved classification to align with IF and WB

Silabs-ArjanB · 2022-11-29T09:36:14Z

rtl/include/cv32e40x_pkg.sv

@@ -669,6 +669,14 @@ parameter logic [31:0] TDATA1_RST_VAL = {
  parameter MCONTROL6_STORE      = 1;
  parameter MCONTROL6_LOAD       = 0;

+  parameter ETRIGGER_M = 9;
+  parameter ETRIGGER_U = 7;


Silabs-ArjanB · 2022-11-29T09:43:38Z

rtl/cv32e40x_debug_triggers.sv

+          // tdata2 contains a legal value for etrigger.
+
+          // Detect if any cleared bits in ETRIGGER_TDATA2_MASK are set in tdata2
+          if (0) begin //|(tdata2_rdata_o & (~ETRIGGER_TDATA2_MASK))) begin


Will fixing this also fix the case we discussed (i.e. keeping type DISABLED when trying to switch to a type that is incomptabile with values in tdata1 and tdata2?

Oh, this should definitely not have propagated here, this came from some debugging and slipped through.

Silabs-ArjanB · 2022-11-29T09:48:01Z

rtl/cv32e40x_debug_triggers.sv

+        tdata1_n  = tdata1_rdata_o;
+        tdata2_n  = tdata2_rdata_o;
+
+        if (csr_wdata_i[TDATA1_TTYPE_HIGH:TDATA1_TTYPE_LOW] == TTYPE_MCONTROL6) begin


Shouldn''t such code only be done when there is an actual write to tdata1? Same question with respect to code related to tdata_2. No need maybe to change the code, but do add a remark that the _n data is not used without the worresponding _we signals

True, added qualifiers for tdata1_we_i and tdata2_we_i

Silabs-ArjanB · 2022-11-29T09:52:25Z

rtl/cv32e40x_controller_fsm.sv

+  // Only set the etrigger_in_wb flag when wb_valid is true (WB is not halted or killed).
+  // If a higher priority event than taking an exception (NMI, external debug or interrupts) are present, wb_valid_i will be
+  // suppressed by either halt_wb followed by kill_wb (debug), or kill_wb (NMI/interrupt).
+  assign etrigger_in_wb = etrigger_wb_i && wb_valid_i;


Shouldn't this expression also contain ex_wb_pipe_i.instr_valid like all other *_in_wb signals?

ex_wb_pipe_i.instr_valid is already implied by wb_valid_i as that signal is only set when (ex_wb_pipe_i.instr_valid && !halt_wb && !kill_wb).

Silabs-ArjanB · 2022-11-29T10:00:01Z

rtl/cv32e40x_controller_fsm.sv

@@ -988,6 +999,7 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*;
          ctrl_fsm_o.kill_ex = 1'b1;
          // Ebreak that causes debug entry should not be killed, otherwise RVFI will skip it
          // Trigger match should also be signalled as not killed (all write enables are suppressed in ID), otherwise RVFI/ISS will not attempt to execute and detect trigger
+          // Exception trigger match should have nothing in WB, excepted instruction finished the previous cycle and set mepc and mcause due to the exception.


As always when claiming that something 'should be' the case, please check it with an assertion

This is checked by the a_etrigger_dpc_write in cs_registers_sva. It check for !wb_valid the cycle after an etrigger has been taken. I added a check for !ex_wb_pipe.instr_valid just to make it more clear.

Silabs-ArjanB · 2022-11-29T10:00:50Z

rtl/cv32e40x_controller_fsm.sv

          // and to ensure only one instruction can retire during single step
-        if (pending_single_step) begin
+        if (pending_single_step || etrigger_in_wb) begin


Add an assertion that etrigger_in_wb implies exception_in_wb

Added assertion

Signed-off-by: Oystein Knauserud <[email protected]>

silabs-oysteink added 2 commits November 29, 2022 09:12

Removed wrong comment and fixed WB priv_lvl_en for etrigger.

8eb8bfa

Signed-off-by: Oystein Knauserud <[email protected]>

silabs-oysteink requested a review from Silabs-ArjanB November 29, 2022 08:22

silabs-oysteink added the Component:RTL For issues in the RTL (e.g. for files in the rtl directory) label Nov 29, 2022

Silabs-ArjanB requested changes Nov 29, 2022

View reviewed changes

Updates after review.

908d24e

Signed-off-by: Oystein Knauserud <[email protected]>

silabs-oysteink requested a review from Silabs-ArjanB November 29, 2022 10:21

Silabs-ArjanB approved these changes Nov 29, 2022

View reviewed changes

Silabs-ArjanB merged commit e652677 into openhwgroup:master Nov 29, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Trigger types 0x5 and 0xF #706

Trigger types 0x5 and 0xF #706

silabs-oysteink commented Nov 29, 2022

Silabs-ArjanB Nov 29, 2022

silabs-oysteink Nov 29, 2022

Silabs-ArjanB Nov 29, 2022

silabs-oysteink Nov 29, 2022

Silabs-ArjanB Nov 29, 2022

silabs-oysteink Nov 29, 2022

Silabs-ArjanB Nov 29, 2022

Silabs-ArjanB Nov 29, 2022

silabs-oysteink Nov 29, 2022

Silabs-ArjanB Nov 29, 2022 •

edited

Loading

silabs-oysteink Nov 29, 2022

Silabs-ArjanB Nov 29, 2022

silabs-oysteink Nov 29, 2022

Silabs-ArjanB Nov 29, 2022

silabs-oysteink Nov 29, 2022

Silabs-ArjanB Nov 29, 2022

silabs-oysteink Nov 29, 2022

Trigger types 0x5 and 0xF #706

Trigger types 0x5 and 0xF #706

Conversation

silabs-oysteink commented Nov 29, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Silabs-ArjanB Nov 29, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Silabs-ArjanB Nov 29, 2022 •

edited

Loading