Re: [PATCH v1 1/1] riscv: Convert interrupt logs to use qemu_log_mask()

[Alistair Francis]

On Sun, Sep 27, 2020 at 9:22 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> On 9/27/20 3:47 PM, Alistair Francis wrote:
> > Currently we log interrupts and exceptions using the trace backed in
>
> s/backed/backend/
>
> > riscv_cpu_do_interrupt(). We also log execptions using the interrupt log
>
> Typo "exceptions".
>
> > mask (-d int) in riscv_raise_exception().
> >
> > This PR converts riscv_cpu_do_interrupt() to log both interrupts and
>
> s/PR/patch/

Fixed!

>
> > exceptions with the interrupt log mask, so that both are printed when a
> > user runs QEMU with -d int.
> >
> > Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> > ---
> >  target/riscv/cpu_helper.c | 7 +++++--
> >  target/riscv/op_helper.c  | 1 -
> >  target/riscv/trace-events | 3 ---
> >  3 files changed, 5 insertions(+), 6 deletions(-)
> >
> > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > index 904899054d..9df3238213 100644
> > --- a/target/riscv/cpu_helper.c
> > +++ b/target/riscv/cpu_helper.c
> > @@ -894,8 +894,11 @@ void riscv_cpu_do_interrupt(CPUState *cs)
> >          }
> >      }
> >
> > -    trace_riscv_trap(env->mhartid, async, cause, env->pc, tval,
> > -        riscv_cpu_get_trap_name(cause, async));
> > +    qemu_log_mask(CPU_LOG_INT,
> > +                  "%s: hart:"TARGET_FMT_ld", async:%d, 
> > cause:"TARGET_FMT_lx", "
> > +                  "epc:0x"TARGET_FMT_lx", tval:0x"TARGET_FMT_lx", 
> > desc=%s\n",
> > +                  __func__, env->mhartid, async, cause, env->pc, tval,
> > +                  riscv_cpu_get_trap_name(cause, async));
>
> Maybe you want to keep the trace event, as the trace framework allow you
> to select traced events at runtime, events come with timestamp and for
> some backends the events are machine-parsable.

Good point, I have kept this in.

>
> Regardless:
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Thanks!

Alistair

>
> >
> >      if (env->priv <= PRV_S &&
> >              cause < TARGET_LONG_BITS && ((deleg >> cause) & 1)) {
> > diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
> > index 9b9ada45a9..e987bd262f 100644
> > --- a/target/riscv/op_helper.c
> > +++ b/target/riscv/op_helper.c
> > @@ -29,7 +29,6 @@ void QEMU_NORETURN riscv_raise_exception(CPURISCVState 
> > *env,
> >                                            uint32_t exception, uintptr_t pc)
> >  {
> >      CPUState *cs = env_cpu(env);
> > -    qemu_log_mask(CPU_LOG_INT, "%s: %d\n", __func__, exception);
> >      cs->exception_index = exception;
> >      cpu_loop_exit_restore(cs, pc);
> >  }
> > diff --git a/target/riscv/trace-events b/target/riscv/trace-events
> > index b7e371ee97..6be2147c8f 100644
> > --- a/target/riscv/trace-events
> > +++ b/target/riscv/trace-events
> > @@ -1,6 +1,3 @@
> > -# cpu_helper.c
> > -riscv_trap(uint64_t hartid, bool async, uint64_t cause, uint64_t epc, 
> > uint64_t tval, const char *desc) "hart:%"PRId64", async:%d, 
> > cause:%"PRId64", epc:0x%"PRIx64", tval:0x%"PRIx64", desc=%s"
> > -
> >  # pmp.c
> >  pmpcfg_csr_read(uint64_t mhartid, uint32_t reg_index, uint64_t val) "hart 
> > %" PRIu64 ": read reg%" PRIu32", val: 0x%" PRIx64
> >  pmpcfg_csr_write(uint64_t mhartid, uint32_t reg_index, uint64_t val) "hart 
> > %" PRIu64 ": write reg%" PRIu32", val: 0x%" PRIx64
> >
>