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

Himanshu Chauhan posted 2 patches 6 months, 3 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20240229133745.771154-1-hchauhan@ventanamicro.com
Maintainers: Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Bin Meng <bin.meng@windriver.com>, Weiwei Li <liwei1518@gmail.com>, Daniel Henrique Barboza <dbarboza@ventanamicro.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>
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(-)
[PATCH v3 0/2] Export debug triggers as an extension
Posted by Himanshu Chauhan 6 months, 3 weeks ago
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
   - 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
Re: [PATCH v3 0/2] Export debug triggers as an extension
Posted by Andrew Jones 6 months, 3 weeks ago
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.

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
> 
>
Re: [PATCH v3 0/2] Export debug triggers as an extension
Posted by Anup Patel 6 months, 3 weeks ago
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

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
> >
> >
>
Re: [PATCH v3 0/2] Export debug triggers as an extension
Posted by Alistair Francis 6 months, 2 weeks ago
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
> > >
> > >
> >
>