Match Trigger (type2) tdata1.timing field may be too restricted

[Dolu1990]

Hi,

Currently, if you use a match trigger type 2 on a load/store address, the riscv openocd port only accept implementation with tdata1.timing=0

It is done on purpose ? or maybe we could add the tdata1.timing bit in the match_triggers_tdata1_fields.tdata1_ignore_mask ?

riscv-openocd/src/target/riscv/riscv.c

Line 967 in ca7d882

.tdata1_ignore_mask = CSR_MCONTROL_MASKMAX(riscv_xlen(target))

Regards

[TommyMurphyTM1234]

tdata1.timing=0

Did you mean mcontrol.timing?

[en-sc]

Thank you for highlighting the issue!

Your suggestion will cause some troubles with chained triggers: if mcontrol.timing is ignored when setting up the chain, it will be possible to create a chain with different timings, that will never fire.

The mcontrol.timing field can be added to tdata1_ignore_mask conditionally, in case ge_lt triggers are disabled for watchpoints.

But there is still the issue, of mcontrol.timing being recommended to be set to 1 for address watchpoints and HW breakpoints. Data watchpoints are not yet supported in RISC-V OpenOCD for now.

[Dolu1990]

Did you mean mcontrol.timing?

Yes right ^^

Data watchpoints are not yet supported in RISC-V OpenOCD for now.

Right. That would be great to have :D
(as hardware breapoint are also used for microcontroller debugging, where you often can't afford the side-effect of GDB evaluating breakpoint conditions without hardware support)

The mcontrol.timing field can be added to tdata1_ignore_mask conditionally, in case ge_lt triggers are disabled for watchpoints

Ahhh i also need those.
Maybe it would be the job of the functions which setup chains to ensure things goes well ? Or else, as a workaround, could add an riscv target option to set the prefered timing mode, but that isn't so nice i guess.

[en-sc]

Right. That would be great to have :D (as hardware breapoint are also used for microcontroller debugging, where you often can't afford the side-effect of GDB evaluating breakpoint conditions without hardware support)

It most definitely would, however, do you have an access to a HW/simulator that does support such triggers? AFAIK, Spike does not.

Or else, as a workaround, could add an riscv target option to set the prefered timing mode, but that isn't so nice i guess.

I don't see this as a workaround. In fact, I think this is the preferred approach. Currently, the mcontrol.timing field value is selected as recommended by [Table 5.10: Suggested Trigger Timings]. The change in the value changes user-observable behavior of BPs/WPs. So, IMHO, the user should explicitly ask for it.

[Dolu1990]

It most definitely would, however, do you have an access to a HW/simulator that does support such triggers? AFAIK, Spike does not.

I'm working on adding trigger / watchpoint support in VexRiscv (HW cpu).
So far i was testing in HW simulation using verilator.

My first implementation was about implementing all the address / data triggers to happen after the instruction, but that doesn't work well with GDB, as it kinda realy expect address trigger to cancel the instruction commit.
So i'm now reworking the HW implementation to have triggers working before commit, and dropping load data trigger.

[en-sc]

Related to #556