[YazanHussnain-10x]

• I have searched for similar pull requests
• I am a human engaging in an interpersonal interaction. During this interaction, my words are my own and are not generated. If relevant, I provide links to my sources.

Why

The Sdtrig trigger module does not follow the RISC-V Debug Specification in 13 cases. 7 cases have an open issue. The other 6 cases have no issue.
This PR corrects the 13 cases, with one commit for each case. Each correction applies to RV32 and RV64.

On master, 3 more cases are in the load unit. The master_candidate branch already corrects them, so they are not in this PR.

Related issues

Closes #3618, closes #3619, closes #3620, closes #3622, closes #3624, closes #3646, closes #3647.

Changes

#3618: a write of 0 to tdata1 does not disable the trigger

Issue: on RV64, the write of 0 has no effect. A trigger of type 15 reads back as type 0.

RISC-V Debug Specification, section 5.7.2 (Trigger Data 1, tdata1):

Writing 0 to this register must result in a trigger that is disabled. If this trigger supports multiple types, then the hardware should disable it by changing type to 15.

Fix: a write of type 0, 15 or an unsupported type sets type 15, and type 15 reads back as 15.

#3622: tdata1 can hold dmode=0 with action=1

Issue: a write from M-mode clears dmode, but keeps action=1.

RISC-V Debug Specification, section 5.2 (Actions), action 1:

This action is only legal when the trigger's dmode is 1. Since tdata1 is WARL, hardware must prevent it from containing dmode=0 and action=1.

Fix: the action is 1 only when the new dmode is 1. All other values give action 0.

#3624: dcsr.cause is 3 for a trigger with action=1

Issue: a trigger and a halt request use the same internal cause.

RISC-V Debug Specification, section 4.9.1 (Debug Control and Status, dcsr), field cause:

2 (trigger): A Trigger Module trigger fired with action=1.

Fix: a new internal cause DEBUG_TRIGGER gives dcsr.cause = 2.

#3620: etrigger fires on an interrupt, and with no trap

Issue: the match does not use ex_i.valid or the interrupt bit of the cause.

RISC-V Debug Specification, section 5.7.15 (Exception Trigger, etrigger):

This trigger may fire on up to XLEN of the Exception Codes defined in mcause (described in the Privileged Spec, with Interrupt=0).

Fix: the etrigger matches only on an exception trap, with one match flag for each trigger.

#3646: itrigger fires after the handler returns

Issue: the itrigger waits for an mret, and the breakpoint writes the PC to tval.

RISC-V Debug Specification, section 5.7.14 (Interrupt Trigger, itrigger):

When the trigger matches, it fires after the trap occurs, just before the first instruction of the trap handler is executed. If action=0, the standard CSRs are updated for taking the breakpoint trap, and zero is written to the relevant tval CSR.

Fix: the itrigger uses the etrigger path and fires before the first handler instruction, with tval = 0.

#3647: icount reports a wrong mepc

Issue: the trap is raised when no valid instruction is at commit, and count=0 also fires.

RISC-V Debug Specification, section 5.7.13 (Instruction Count, icount):

When pending is set, the trigger fires just before any further instructions are executed in a mode where the trigger is enabled.

Fix: pending is set only when count goes to 0, and the trap is raised only on a valid instruction.

#3619: icount counts in modes where it is not enabled

Issue: count decreases for all retired instructions, and stops after a trap until mret.

RISC-V Debug Specification, section 5.7.13 (Instruction Count, icount):

This trigger matches when: An instruction retires after having been fetched in a privilege mode where the trigger is enabled. [...] A trap is taken from a privilege mode where the trigger is enabled.

Fix: count decreases only for the instructions and traps of the enabled modes.

etrigger and itrigger do not fire again when hit is set

Issue: after the first fire, hit=1 blocks the trigger until software clears hit.

RISC-V Debug Specification, section 5.7.15 (Exception Trigger, etrigger), field hit:

If this bit is implemented, the hardware sets it when this trigger matches. The trigger's user can set or clear it at any time.

Fix: hit does not block the fire of an etrigger or an itrigger.

mcontrol6 load and store triggers do not see lr and the AMOs correctly

Issue: all AMOs, lr and sc are stores for the trigger module. A load trigger does not match an AMO or lr, and a store trigger matches lr.

RISC-V Debug Specification, section 5.5.1 (A Extension):

lr instructions are loads.

Each AMO instruction is a load for the read portion of the operation.

Each AMO instruction is a store for the write portion of the operation.

Fix: the trigger module gets the LSU operation. lr is a load, sc is a store, an AMO is both, and the store unit takes the action.

The textra context match is not used

Issue: build_config_pkg sets SdtrigSupportTextra to 0. When it is 1, sselect=0 blocks all matches and scontext is compared in S-mode only.

RISC-V Debug Specification, section 5.7.17 (Trigger Extra, textra32), field sselect:

0 (ignore): Ignore svalue.

1 (scontext): This trigger will only match or fire if the low bits of scontext equal svalue.

Fix: build_config_pkg keeps the option, sselect=0 ignores svalue, the match applies in all modes, and sselect is 0 or 1.

A trigger with action=0 fires in its own trap handler

Issue: there is no reentrancy rule. An action=0 trigger fires in an M-mode trap handler and overwrites mepc and mcause.

RISC-V Debug Specification, section 5.4 (Native Triggers):

The hardware prevents triggers with action=0 from matching or firing while in M-mode and while MIE in mstatus is 0. If medeleg [3]=1 then it prevents triggers with action=0 from matching or firing while in S-mode and while SIE in sstatus is 0.

Fix: the trigger module gets mstatus.MIE, mstatus.SIE and medeleg[3], and blocks the action=0 triggers as the spec says.

A write to tinfo causes an illegal instruction exception

Issue: tinfo has no write case in csr_regfile. A write causes an illegal instruction exception.

RISC-V Privileged Specification, section 2.1 (CSR Address Mapping Conventions):

A read/write register might also contain some bits that are read-only, in which case writes to the read-only bits are ignored.

Fix: a write to tinfo is ignored when Sdtrig is enabled.

tinfo shows trigger types that the configuration does not have

Issue: tinfo.info is a constant. It shows types 3, 4, 5 and 6 also when the configuration disables them.

RISC-V Debug Specification, section 5.7.5 (Trigger Info, tinfo), field info:

One bit for each possible type enumerated in tdata1. Bit N corresponds to type N. If the bit is set, then that type is supported by the currently selected trigger.

Fix: tinfo.info gets the type bits from SdtrigIcount, SdtrigItrigger, SdtrigEtrigger and SdtrigMcontrol6.

Fixed on master_candidate

These 3 cases are on master. The new load unit on master_candidate corrects them.

A load address trigger stops the hart

Issue: the load unit cancels the load one cycle after the match. The load has then left lsu_ctrl_i, so the exception goes to a wrong transaction id and the hart waits for ever.

Fixed on master_candidate by 4146ee8db (Stop a load matching a load address trigger before it reaches memory).

Failing ACT4 tests on master without it: FAIL_LA_PLACEHOLDER

A load data trigger reports HARDWARE_ERROR

Issue: the load unit sets the cause to HARDWARE_ERROR for each load response, also when the response has no error. This cause replaces the cause of a load data trigger.

Fixed on master_candidate by e0f1ef8d5 (port: Turn a cache error on a load into an exception). The cause is HARDWARE_ERROR only when the load response has an error.

Failing ACT4 tests on master without it: FAIL_LDCAUSE_PLACEHOLDER

A load data trigger stops the hart

Issue: the trigger module compares the load data after the LSU pipeline registers, without load_valid. The load unit marks the exception one cycle before, so the trigger does not fire, and the old data keeps the load unit stalled.

Fixed on master_candidate by af5972146 (Compare the load data trigger after the load output register).

Failing ACT4 tests on master without it: FAIL_LD_PLACEHOLDER

Open case

A load data trigger does not see the data of lr

Issue: lr gets its data at commit from the AMO response. The trigger module compares only the load write back, so a load data trigger never matches lr. This case is also on master_candidate, where sdtrig_load_data_o is load_result_o.

RISC-V Debug Specification, section 5.5.1 (A Extension):

lr instructions are loads.

This PR does not correct it. The correction needs the load data trigger of af5972146 and the deferred break of 56cfe006d (Take a deferred load data break on the next valid instruction). It is easier to make it on top of master_candidate: compare the lr data at commit, and take the trigger action on the next valid instruction.

Failing ACT4 tests: SdtrigSm_lrsc_data, SdtrigS_lrsc_data, SdtrigU_lrsc_data.