Mcontrol6 #702
Mcontrol6 #702
Conversation
- Instruction address checks depend on M/U bit and actual privilege level
- tdata1.match {0,2,3} selects comparison {==, >= <}
- LSU address match, also with privilege checks and tdata1.match checks
-- LSU will not create an externally visible OBI request on trigger match
-- Matching is enabled for PUSH/POP mid sequence
Signed-off-by: Oystein Knauserud <[email protected]>
Signed-off-by: Oystein Knauserud <[email protected]>
silabs-oysteink
added
the
Component:RTL
…e data does not match any legal vale. Signed-off-by: Oystein Knauserud <[email protected]>
|
Pushed one update where the WARL resolve of tdata1.match was not correct. |
| // When a CLIC pointer is in the pipeline stages EX or WB, we must block debug. | ||
| // - Debug would otherwise kill the pointer and use the address of the pointer for dpc. A following dret would then return to the mtvt table, losing program progress. | ||
| assign debug_allowed = lsu_interruptible_i && !fencei_ongoing && !xif_in_wb && !clic_ptr_in_pipeline && sequence_interruptible; | ||
| assign debug_allowed = lsu_interruptible_i && !fencei_ongoing && !xif_in_wb && !clic_ptr_in_pipeline && (sequence_interruptible || trigger_match_in_wb); |
There was a problem hiding this comment.
Is this change in itself SEC clean when ZC_EXT = 0?
There was a problem hiding this comment.
SEC is still running after four days
rtl/cv32e40x_core.sv
Outdated
| @@ -634,11 +644,19 @@ module cv32e40x_core import cv32e40x_pkg::*; | |||
| .busy_o ( lsu_busy ), | |||
| .interruptible_o ( lsu_interruptible ), | |||
|
|
|||
| // Trigger match | |||
| .trigger_match_i ( trigger_match_ex ), | |||
There was a problem hiding this comment.
Stick to naming conventions:
.trigger_match_0_i ( trigger_match_ex ),
rtl/cv32e40x_debug_triggers.sv
Outdated
| @@ -108,7 +132,7 @@ import cv32e40x_pkg::*; | |||
| 4'b0000, // zero, size (match any size) 19:16 | |||
| 4'b0001, // action, WARL(1), enter debug 15:12 | |||
| 1'b0, // zero, chain 11 | |||
| 4'b0000, // match, WARL(0,2,3) 10:7 todo: resolve WARL | |||
| mcontrol6_match_resolve(tdata1_rdata_o[MCONTROL6_MATCH_H:MCONTROL6_MATCH_L], csr_wdata_i[MCONTROL6_MATCH_H:7]), // match, WARL(0,2,3) 10:7 | |||
There was a problem hiding this comment.
Stick to _HIGH, _LOW naming convention as used for other bitfields
rtl/cv32e40x_debug_triggers.sv
Outdated
| (tdata1_q[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_q[idx][MCONTROL6_LOAD] && !lsu_we_ex_i) || (tdata1_q[idx][MCONTROL6_STORE] && lsu_we_ex_i)); |
There was a problem hiding this comment.
Add todo to revisit this code when supporting Atomics
rtl/cv32e40x_debug_triggers.sv
Outdated
| assign lsu_addr_match_en[idx] = lsu_valid_ex_i && ((tdata1_q[idx][MCONTROL6_LOAD] && !lsu_we_ex_i) || (tdata1_q[idx][MCONTROL6_STORE] && lsu_we_ex_i)); | ||
|
|
||
| // LSU address matching | ||
| assign lsu_addr_match[idx] = (tdata1_q[idx][MCONTROL6_MATCH_H:MCONTROL6_MATCH_L] == 4'h2) ? (lsu_addr_ex_i >= tdata2_q[idx]) : |
There was a problem hiding this comment.
I think that the _llow value should be used for match == 2 instead.
Also, order match values 0, 2, 3 just like for the I-side comparisons
rtl/cv32e40x_debug_triggers.sv
Outdated
| // Load/Store address match (EX) | ||
| /////////////////////////////////////// | ||
|
|
||
| // As for instruction address match, the load/store address match happens before the instruction is executed. |
There was a problem hiding this comment.
Is this complexity caused by how misaligned loads/stores are handled? Do you generate all compare values for a misaligned load/store during its first operation? If so, why, and would it help to treat each opeartion separately?
There was a problem hiding this comment.
The adder to calculate boundaries came (partly) from checking all values, even for misaligned load/stores. I rewrote the code to check per-transaction, so two checks will be performed for misaligned. I also replaced the adder by logic to generate boundaries base on the byte enables instead.
Naming and typo updates Signed-off-by: Oystein Knauserud <[email protected]>
…ss and size. Removes 32-bit adder and reduces area by approximately 30% per trigger. Signed-off-by: Oystein Knauserud <[email protected]>
| always_comb begin | ||
| for (int b=0; b<4; b++) begin : gen_byte_checks | ||
| if (lsu_be_ex_i[b]) begin | ||
| lsu_addr_high_lsb = 2'(b); |
There was a problem hiding this comment.
Add default assignment for lsu_addr_high_lsb (with XIF none of the lsu_be_ex_i bits might be high)
|
|
||
| // Iterate through triggers and set tdata1/tdata2 rdata for the currently selected trigger | ||
| for (int i=0; i<DBG_NUM_TRIGGERS; i++) begin | ||
| if(tselect_q == i) begin | ||
| tdata1_rdata_o = tdata1_q[i]; | ||
| tdata2_rdata_o = tdata2_q[i]; | ||
| tdata1_rdata_o = tdata1_rdata[i]; |
There was a problem hiding this comment.
tselect_q -> tselect_rdata
rtl/cv32e40x_debug_triggers.sv
Outdated
| // Check address matches for (==), (>=) and (<) | ||
| // For ==, check that we match the 32-bit aligned word and that any of the accessed bytes matches tdata2[1:0] | ||
| // For >=, check that the highest accessed address is greater than or equal to tdata2 | ||
| // For <, check that the highest accessed address is less than tdata2 |
There was a problem hiding this comment.
Comment and implementation not according to debug spec
…ddressed byte within a transaction for use in the less-than comparison. Removed some usage of _q values and replaces with _rdata values. Signed-off-by: Oystein Knauserud <[email protected]> : rtl/cv32e40x_core.sv
Added the following features for trigger type 6 (mcontrol6):
-- LSU will not create an externally visible OBI request on trigger match
-- Matching is enabled for PUSH/POP mid sequence
Signed-off-by: Oystein Knauserud [email protected]