medeleg: the third bit(CAUSE_BREAKPOINT) of medeleg is unwritable when Sdtrig exist.

[NewPaulWalker]

According to the riscv instruction set manual, when the Sdtrig extension exists, the CAUSE_BREAKPOINT bit (the third bit) of the medeleg CSR is unwritable. Otherwise, this bit is writable.

[aswaterman]

I don't think that's entirely true. Making medeleg.breakpoint read-only zero is one implementation choice:

https://github.com/riscv/riscv-debug-spec/blob/bba87500d459e015fc8c9f80c4f9d9043ae8f4de/Sdtrig.adoc#L185-L191

We should be doing the first one in Spike (right @YenHaoChen?) because supporting the delegation of breakpoint exceptions is the more common implementation choice.

[NewPaulWalker]

I don't think that's entirely true. Making medeleg.breakpoint read-only zero is one implementation choice:

https://github.com/riscv/riscv-debug-spec/blob/bba87500d459e015fc8c9f80c4f9d9043ae8f4de/Sdtrig.adoc#L185-L191

We should be doing the first one in Spike (right @YenHaoChen?) because supporting the delegation of breakpoint exceptions is the more common implementation choice.

We hope to use Spike as the golden reference, and our hardware implements the second choice. Can both choice be implemented in Spike? Additionally, add a switch to select which choice to use? You can set the first choice as the default.

[YenHaoChen]

According to the riscv instruction set manual, when the Sdtrig extension exists, the CAUSE_BREAKPOINT bit (the third bit) of the medeleg CSR is unwritable. Otherwise, this bit is writable.

This statement comes from Debug spec Section 5.4, which states the bit is unwritable when implementing tcontrol.mte/mtep (the second choice).

When reading the debug spec, I also needed clarification about what a general implementation should be. Based on my reading, the spec prefers to implement one of the solutions but not both. Additionally, the medeleg[3] is writable in Spike. Thus, I went for the first solution without implementing the optional tcontrol CSR of the second solution. However, people requested implementing the tcontrol CSR #1688. Currently, Spike implements both solutions with the medeleg[3] writable.

@rtwfroody Is the behavior of the current Spike generally expected by the debugger? It is not clear to me whether the debug spec allows the behavior. (I have another question unrelated to this post. The tcontrol is an optional CSR. The default value of tcontrol prevents triggers from M-mode if the tcontrol exists. How does the debugger know whether to write the tcontrol or not? If the tcontrol does not exist, won't writing the tcontrol cause the debugger to fail?)

[rtwfroody]

@rtwfroody Is the behavior of the current Spike generally expected by the debugger?

External debuggers (e.g. OpenOCD) don't care about any of this. These features only exist to facilitate M-Mode debuggers running in spike, debugging M-Mode code.

It is not clear to me whether the debug spec allows the behavior.

The spec says to implement one of the two options. Option 1 is cheap to implement but keeps triggers disabled during interrupt handlers. Option 2 is more costly and gives better options for having triggers fire even during interrupt handlers.

Implementing both at the same time doesn't really make sense. For using spike as a golden model, there should probably be a build or run-time option to select which of the two is active.

(I have another question unrelated to this post. The tcontrol is an optional CSR. The default value of tcontrol prevents triggers from M-mode if the tcontrol exists. How does the debugger know whether to write the tcontrol or not? If the tcontrol does not exist, won't writing the tcontrol cause the debugger to fail?)

If you want triggers to cause a breakpoint while executing from M-Mode, you must have M-Mode privileges to make that happen. In that case you can set up a simple trap handler, write tcontrol, and see if your trap handler was hit or not.

[YenHaoChen]

Both @NewPaulWalker and @rtwfroody have expressed interest in a build option that switches the writability of medeleg[3] and the existence of tcontrol.

However, the current commit won’t compile because we don't have the EXT_SDTRIG. Furthermore, the EXT_SDTRIG doesn’t require a read-only medeleg[3]. To address this, we need an additional build option, such as #ifdef EXIST_TCONTROL. Additionally, the trigger_t::allow_action() should depend on this new build option.

Would anyone like to take on this task? Alternatively, I can provide a prototype for it.

[aswaterman]

We are moving away from build-time options. (Even misaligned loads and stores are no longer a build-time option.)

If we really need to support this, it needs to come in through an ISA string. Specifying that might be a matter for the debug TG; regardless, it seems out of scope for the Spike authors.