Re: [PATCH] target/riscv: Dump sstatus CSR in riscv_cpu_dump_state()

[Alistair Francis <alistair23@gmail.com>]

On Wed, Nov 23, 2022 at 2:07 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Alistair,
>
> On Wed, Nov 23, 2022 at 8:03 AM Alistair Francis <alistair23@gmail.com> wrote:
> >
> > On Wed, Nov 23, 2022 at 2:07 AM Bin Meng <bmeng@tinylab.org> wrote:
> > >
> > > sstatus register dump is currently missing in riscv_cpu_dump_state().
> > >
> > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1332
> > > Signed-off-by: Bin Meng <bmeng@tinylab.org>
> > >
> > > ---
> > >
> > >  target/riscv/cpu.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > > index d14e95c9dc..80d76f0181 100644
> > > --- a/target/riscv/cpu.c
> > > +++ b/target/riscv/cpu.c
> > > @@ -382,6 +382,7 @@ static void riscv_cpu_dump_state(CPUState *cs, FILE *f, int flags)
> > >              CSR_MHARTID,
> > >              CSR_MSTATUS,
> > >              CSR_MSTATUSH,
> > > +            CSR_SSTATUS,
> >
> > I don't think we need this. mstatus contains all of the information
> > already and there is limited space to print all of this information
> > out.
> >
>
> I am not sure what limited space restricts this? This is CPU state
> dump, and printing sstatus CSR seems reasonable to me. We do the
> similar thing in the gdb stub too.

Limited space in that there is only so much text we want to dump to
the screen. As new extensions are added this will continue to grow,
and sstatus doesn't provide any more information then the mstatus
register that we already show.

GDB is a little different as people can interactively probe the
registers they are interested in.

Alistair

>
> Regards,
> Bin