Salutations,
Sorry for the late review. Just got informed in another thread that this
was sent back in January and I missed it.
I believe patches 1-18 and 25 can be sent in a separated "debug related fixes"
series. They seem nice to have regardless of the other patches.
Patches 19-24 start to touch into a more sensible area that we're discussing in
other threads, most notably this one:
("[PATCH v3 0/1] target/riscv: deprecate 'debug' CPU property and Debug 0.13")
https://lore.kernel.org/qemu-devel/[email protected]/
The lore behind that change is that we want to support Debug 1.0 extensions, in
particular sdext, because that is a requirement for the Server Reference Board.
But 'debug' is based on spec 0.13, and unfortunately these 2 versions can't play
ball, so my initial reaction was to move away for 0.13.
From what I can see in this series there is a desire to keep the existing debug
0.13 code base around, and that's fine. I also believe that despite the
incompatibility the 0.13 code base can be re-used for the 1.0 sdtrig as well,
with the appropriate "if-then-else" to split between the behaviors we want, so
it's feasible to do both.
IIUC what we seem to be missing in patches 19-24 is a separated flag, i.e. a
new extension flag, for the 1.0 sdtrig. So we would end up with 'debug' being
an alias for 'sdtrig 0.13' and a new 'sdtrig' flag that implements debug 1.0.
Chao can then base his 'sdext' work on top of the new sdtrig flag, and then
we can upstream the riscv-server-ref board. At least this is how I'm seeing
all this pieces playing out ....
Cheers,
Daniel
On 1/14/2026 1:46 AM, Nicholas Piggin wrote:
Hi,
Sorry for the big series. The Ascalon CPU implements Sdtrig with 2
different types of mcontrol6 trigger and the icount trigger, so in
the course of testing and bringing up OpenSBI and Linux support for
this, I've accumulated quite a lot.
My new year resolution is to start being better upstream contributor,
it's taken me a while with changing jobs and architectures. So I don't
expect others to drop everything to review this! Joel has been
prodding me, and noted there is some other Sdtrig work going on
with the v1.0 support patches.
I think the debug v1.0 patches are somewhat orthogonal to this series,
but both are addressing aspects of a common problem of Sdtrig
implementation specifics. I wonder if these should be reconciled or
left separate. Sdtrig v1.00/v0.13 configuration is a single boolean
which is feasible as a CPU property. Whereas the entire space of
Sdtrig implementation seems like too much to make configurable in that
way.
Any thoughts would be welcome.
Thanks,
Nick
Nicholas Piggin (25):
target/riscv/debug: Check only mcontrol triggers for break/watchpoint
matching
target/riscv/debug: Handle changing trigger types
target/riscv/debug: Implement permissive type unavailable trigger
target/riscv/debug: Fix icount trigger privilege check
target/riscv/debug: Update itrigger_enabled after changing privilege
target/riscv/debug: Implement get_trigger_action for icount type
trigger
target/riscv/debug: Fix migration post_load icount_enabled() test
target/riscv/debug: Fix icount privilege matching icount_enabled()
test
target/riscv/debug: Implement icount trigger textra matching
target/riscv/debug: Maintain itrigger_enabled in
helper_itrigger_match()
target/riscv/debug: Fix breakpoint matching action
target/riscv/debug: Put mcontrol load/store match address into tval
target/riscv/debug: Remove breakpoints on reset
target/riscv/debug: Move debug CPU post_load details into debug.c
target/riscv/debug: Insert breakpoints after migration
target/riscv/debug: Remove itrigger icount-enabled mode
target/riscv/debug: Advertise icount trigger type in tinfo
target/riscv/debug: Reset trigger type to unavailable
target/riscv/debug: Add new debug state format
target/riscv/debug: Migrate mcontext using new sdtrig vmstate
target/riscv/debug: Implementation specific Sdtrig configuration
target/riscv/debug: Support heterogeneous trigger types
target/riscv/debug: Support heterogeneous mcontrol access types
target/riscv/debug: Emulate TT Ascalon Sdtrig
target/riscv/debug: Fix minor comment typos
target/riscv/cpu.c | 65 ++++-
target/riscv/cpu.h | 41 ++-
target/riscv/cpu_helper.c | 10 +-
target/riscv/csr.c | 7 +-
target/riscv/debug.c | 571 ++++++++++++++++++++-----------------
target/riscv/debug.h | 19 +-
target/riscv/machine.c | 96 ++++++-
target/riscv/tcg/tcg-cpu.c | 5 +-
8 files changed, 510 insertions(+), 304 deletions(-)
