Himanshu,
We spoke offline but let's make everyone aware:
- 'sdtrig' should be marked with 'x-' and be an experimental extension since
the spec isn't yet frozen;
- Alvin sent a patch to the ML adding the 'mcontext' CSR for 'sdtrig' some time
ago:
"[PATCH v2] target/riscv: Implement optional CSR mcontext of debug Sdtrig
extension​"
It would be good to put his patch on top of this series to ease the review for
everyone.
The changes done in patch 2 would also be applicable to the mcontext CSR;
- last but probably the most important: the existing 'debug' flag seems to be
acting as
the actual 'sdtrig' extension due to how the flag is gating trigger code, e.g.:
if (cpu->cfg.debug) {
riscv_trigger_realize(&cpu->env);
}
and
if (cpu->cfg.debug) {
riscv_trigger_reset_hold(env);
}
If that's really the case, all the checks with cpu->cfg.debug will need to also
include
cpu->cfg.ext_sdtrig (one or the other). And now we'll have to make an option:
do we leave
the debug triggers (i.e. the 'debug' flag) as always enabled?
If it's up to me I would make 'debug' as default 'false' and deprecate it.
Users will need
to enable the debug triggers via x-sdtrig=true from now on. This will break
existing behavior,
but the way it is now we're always enabling an extension (via the debug flag)
that isn't even
frozen, so we're already in the wrong.
Alistair, any thoughts?
Thanks,
Daniel
On 1/10/24 01:02, Himanshu Chauhan wrote:
All the CPUs may or may not implement the debug trigger (sdtrig)
extension. The presence of it should be dynamically detectable.
This patch exports the debug triggers as an extension which
can be turned on or off by sdtrig=<true/false> option. It is
turned on by default.
"sdtrig" is concatenated to ISA string when it is enabled.
Like so:
rv64imafdch_zicbom_*_sdtrig_*_sstc_svadu
Himanshu Chauhan (2):
target/riscv: Export sdtrig as an extension and ISA string
target/riscv: Raise an exception when sdtrig is turned off
target/riscv/cpu.c | 2 ++
target/riscv/cpu_cfg.h | 1 +
target/riscv/csr.c | 20 ++++++++++++++++++++
3 files changed, 23 insertions(+)
