Re: [PATCH v3 0/2] Export debug triggers as an extension

[Alistair Francis]

On Fri, Mar 1, 2024 at 2:20 AM Anup Patel <anup@brainfault.org> wrote:
>
> On Thu, Feb 29, 2024 at 8:42 PM Andrew Jones <ajones@ventanamicro.com> wrote:
> >
> > On Thu, Feb 29, 2024 at 07:07:43PM +0530, Himanshu Chauhan wrote:
> > > All the CPUs may or may not implement the debug triggers (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
> > >
> > > Changes from v1:
> > >    - Replaced the debug property with ext_sdtrig
> > >    - Marked it experimenatal by naming it x-sdtrig
> > >    - x-sdtrig is added to ISA string
> > >    - Reversed the patch order
> > >
> > > Changes from v2:
> > >    - Mark debug property as deprecated and replace internally with sdtrig 
> > > extension
> >
> > I'm getting lost in our discussions, but I thought we needed both in case
> > a machine only implements debug 0.13, since sdtrig is at least 'more than'
> > debug, even if backwards compatible (which I also wasn't sure was the
> > case). If, OTOH, QEMU's debug implementation exactly implements sdtrig's
> > specification, then I'm in favor of deprecating the 'debug' extension.
>
> The QEMU's debug implementation aligns more with Sdtrig v1.0 specification
> because we have mcontrol6 which was not present in debug 0.13

I'm not sure that's exactly the case. I think QEMU implements the
debug 0.13 specification and also the mcontrol6. That's really a bug
that we support mcontrol6, it snuck in.

We can just support both and wrap the mcontrol6 section behind a
sdtrig check. That seems to be the easiest option. That way we can
support the current ratified debug spec and the experimental soon to
be ratified sdtrig and friends spec.

Alistair

>
> IMO, we should definitely depricate debug 0.13
>
> Regards,
> Anup
>
> >
> > Thanks,
> > drew
> >
> >
> > >    - setting/unsetting debug property shows warning and sets/unsets 
> > > ext_sdtrig
> > >    - sdtrig is added to ISA string as RISC-V debug specification is frozen
> > >
> > > Himanshu Chauhan (2):
> > >   target/riscv: Mark debug property as deprecated
> > >   target/riscv: Export sdtrig in ISA string
> > >
> > >  target/riscv/cpu.c        | 38 +++++++++++++++++++++++++++++++++++---
> > >  target/riscv/cpu_cfg.h    |  2 +-
> > >  target/riscv/cpu_helper.c |  2 +-
> > >  target/riscv/csr.c        |  2 +-
> > >  target/riscv/machine.c    |  2 +-
> > >  5 files changed, 39 insertions(+), 7 deletions(-)
> > >
> > > --
> > > 2.34.1
> > >
> > >
> >
>