Re: [RESEND PATCH v3 1/7] target/riscv: Add initial support for native debug

[Bin Meng]

On Wed, Jan 19, 2022 at 11:16 AM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Wed, Jan 5, 2022 at 1:09 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > From: Bin Meng <bin.meng@windriver.com>
> >
> > This adds initial support for the native debug via the Trigger Module,
> > as defined in the RISC-V Debug Specification [1].
>
> Doesn't this mean we are just supporting the Sdtrig extension?

I was looking at where Sdtrig is defined. It turns out this new name
was assigned in a later debug spec and when this patch series was
worked on the Sdtrig extention was not invented yet ...

So the answer is yes, only Sdtrig is supported. Sdext does not make
sense in the QEMU context as it is for the on-chip-debugging.

>
> >
> > Only "Address / Data Match" trigger (type 2) is implemented as of now,
> > which is mainly used for hardware breakpoint and watchpoint. The number
> > of type 2 triggers implemented is 2, which is the number that we can
> > find in the SiFive U54/U74 cores.
> >
> > [1] 
> > https://github.com/riscv/riscv-debug-spec/raw/master/riscv-debug-stable.pdf
> >
> > Signed-off-by: Bin Meng <bin.meng@windriver.com>
> > ---
> >
> > Changes in v3:
> > - drop riscv_trigger_init(), which will be moved to patch #5
> >
> >  target/riscv/cpu.h       |   5 +
> >  target/riscv/debug.h     | 108 +++++++++++++
> >  target/riscv/debug.c     | 339 +++++++++++++++++++++++++++++++++++++++
> >  target/riscv/meson.build |   1 +
> >  4 files changed, 453 insertions(+)
> >  create mode 100644 target/riscv/debug.h
> >  create mode 100644 target/riscv/debug.c
> >
> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > index dc10f27093..0f3b3a4219 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -98,6 +98,7 @@ typedef struct CPURISCVState CPURISCVState;
> >
> >  #if !defined(CONFIG_USER_ONLY)
> >  #include "pmp.h"
> > +#include "debug.h"
> >  #endif
> >
> >  #define RV_VLEN_MAX 1024
> > @@ -234,6 +235,10 @@ struct CPURISCVState {
> >      pmp_table_t pmp_state;
> >      target_ulong mseccfg;
> >
> > +    /* trigger module */
> > +    target_ulong trigger_cur;
> > +    trigger_type2_t trigger_type2[TRIGGER_TYPE2_NUM];
> > +
> >      /* machine specific rdtime callback */
> >      uint64_t (*rdtime_fn)(uint32_t);
> >      uint32_t rdtime_fn_arg;
> > diff --git a/target/riscv/debug.h b/target/riscv/debug.h
> > new file mode 100644
> > index 0000000000..0a3fda6c72
> > --- /dev/null
> > +++ b/target/riscv/debug.h
> > @@ -0,0 +1,108 @@
> > +/*
> > + * QEMU RISC-V Native Debug Support
> > + *
> > + * Copyright (c) 2022 Wind River Systems, Inc.
> > + *
> > + * Author:
> > + *   Bin Meng <bin.meng@windriver.com>
> > + *
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms and conditions of the GNU General Public License,
> > + * version 2 or later, as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope it will be useful, but WITHOUT
> > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> > + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License 
> > for
> > + * more details.
> > + *
> > + * You should have received a copy of the GNU General Public License along 
> > with
> > + * this program.  If not, see <http://www.gnu.org/licenses/>.
> > + */
> > +
> > +#ifndef RISCV_DEBUG_H
> > +#define RISCV_DEBUG_H
> > +
> > +/* trigger indexes implemented */
> > +enum {
> > +    TRIGGER_TYPE2_IDX_0 = 0,
> > +    TRIGGER_TYPE2_IDX_1,
> > +    TRIGGER_TYPE2_NUM,
> > +    TRIGGER_NUM = TRIGGER_TYPE2_NUM
> > +};
> > +
> > +/* register index of tdata CSRs */
> > +enum {
> > +    TDATA1 = 0,
> > +    TDATA2,
> > +    TDATA3,
> > +    TDATA_NUM
> > +};
> > +
> > +typedef enum {
> > +    TRIGGER_TYPE_NO_EXIST = 0,      /* trigger does not exist */
> > +    TRIGGER_TYPE_AD_MATCH = 2,      /* address/data match trigger */
> > +    TRIGGER_TYPE_INST_CNT = 3,      /* instruction count trigger */
> > +    TRIGGER_TYPE_INT = 4,           /* interrupt trigger */
> > +    TRIGGER_TYPE_EXCP = 5,          /* exception trigger */
> > +    TRIGGER_TYPE_AD_MATCH6 = 6,     /* new address/data match trigger */
> > +    TRIGGER_TYPE_EXT_SRC = 7,       /* external source trigger */
> > +    TRIGGER_TYPE_UNAVAIL = 15       /* trigger exists, but unavailable */
> > +} trigger_type_t;
> > +
> > +typedef struct {
> > +    target_ulong mcontrol;
> > +    target_ulong maddress;
> > +    struct CPUBreakpoint *bp;
> > +    struct CPUWatchpoint *wp;
> > +} trigger_type2_t;
>
> This is a confusing name
>

I will change it to type2_tigger,

Regards,
Bin