Re: [PATCH v1 1/1] target/riscv: Print CPU and privledge in disas

[Alistair Francis]

On Fri, Sep 27, 2019 at 10:18 AM Richard Henderson
<address@hidden> wrote:
>
> On 9/26/19 5:45 PM, Alistair Francis wrote:
> > Signed-off-by: Alistair Francis <address@hidden>
> > ---
> >  target/riscv/translate.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> > index adeddb85f6..537af0003e 100644
> > --- a/target/riscv/translate.c
> > +++ b/target/riscv/translate.c
> > @@ -810,7 +810,14 @@ static void riscv_tr_tb_stop(DisasContextBase *dcbase, 
> > CPUState *cpu)
> >
> >  static void riscv_tr_disas_log(const DisasContextBase *dcbase, CPUState 
> > *cpu)
> >  {
> > +#ifndef CONFIG_USER_ONLY
> > +    RISCVCPU *rvcpu = RISCV_CPU(cpu);
> > +    CPURISCVState *env = &rvcpu->env;
> > +#endif
> >      qemu_log("IN: %s\n", lookup_symbol(dcbase->pc_first));
> > +#ifndef CONFIG_USER_ONLY
> > +    qemu_log("CPU: %d; priv: "TARGET_FMT_ld"\n", cpu->cpu_index, 
> > env->priv);
> > +#endif
> >      log_target_disas(cpu, dcbase->pc_first, dcbase->tb->size);
> >  }
>
> I'm curious about the motivation here.
>
> In particular, what difference does it make what cpu the TB is generated for?
> It would seem like the more relevant place to look for this is with -d cpu or
> -d exec where the TB is actually executed, which could well be multiple cpus.

The main reason is that the sifive_u machine now has two RISC-V
clusters so it's useful to see what is happening on each CPU. -d cpu
and -d exec are much more difficult to comprehend so aren't always as
useful.

>
> As for env->priv... is there really anything in the input assembly that 
> depends
> on the privilege?  Or conversely, why priv and not all of the other bits that
> are in tb_flags?  And are you placing that here in -d in_asm because are 
> always
> also using either -d op or -d op_opt, and it somehow looks better to have this
> at the top?

I think it's just clearer to see what privileged mode the instructions
were generated for, it helps with debugging boot issues. Again it
comes back to the other options being difficult to comprehend, in_asm
I find is the clearest to understand (although it has lots of
limitations).

Alistair

>
>
> r~