* [PATCH] target/riscv: Dump sstatus CSR in riscv_cpu_dump_state()
@ 2022-11-22 15:46 Bin Meng
2022-11-23 0:02 ` Alistair Francis
0 siblings, 1 reply; 6+ messages in thread
From: Bin Meng @ 2022-11-22 15:46 UTC (permalink / raw)
To: qemu-devel; +Cc: Alistair Francis, Bin Meng, Palmer Dabbelt, qemu-riscv
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,
CSR_HSTATUS,
CSR_VSSTATUS,
CSR_MIP,
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] target/riscv: Dump sstatus CSR in riscv_cpu_dump_state()
2022-11-22 15:46 [PATCH] target/riscv: Dump sstatus CSR in riscv_cpu_dump_state() Bin Meng
@ 2022-11-23 0:02 ` Alistair Francis
2022-11-23 4:07 ` Bin Meng
0 siblings, 1 reply; 6+ messages in thread
From: Alistair Francis @ 2022-11-23 0:02 UTC (permalink / raw)
To: Bin Meng
Cc: qemu-devel, Alistair Francis, Bin Meng, Palmer Dabbelt, qemu-riscv
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.
Alistair
> CSR_HSTATUS,
> CSR_VSSTATUS,
> CSR_MIP,
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] target/riscv: Dump sstatus CSR in riscv_cpu_dump_state()
2022-11-23 0:02 ` Alistair Francis
@ 2022-11-23 4:07 ` Bin Meng
2022-11-24 23:57 ` Alistair Francis
0 siblings, 1 reply; 6+ messages in thread
From: Bin Meng @ 2022-11-23 4:07 UTC (permalink / raw)
To: Alistair Francis
Cc: Bin Meng, qemu-devel, Alistair Francis, Bin Meng, Palmer Dabbelt,
qemu-riscv
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.
Regards,
Bin
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] target/riscv: Dump sstatus CSR in riscv_cpu_dump_state()
2022-11-23 4:07 ` Bin Meng
@ 2022-11-24 23:57 ` Alistair Francis
2022-11-25 3:34 ` Bin Meng
0 siblings, 1 reply; 6+ messages in thread
From: Alistair Francis @ 2022-11-24 23:57 UTC (permalink / raw)
To: Bin Meng
Cc: Bin Meng, qemu-devel, Alistair Francis, Bin Meng, Palmer Dabbelt,
qemu-riscv
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] target/riscv: Dump sstatus CSR in riscv_cpu_dump_state()
2022-11-24 23:57 ` Alistair Francis
@ 2022-11-25 3:34 ` Bin Meng
2022-11-25 4:32 ` Alistair Francis
0 siblings, 1 reply; 6+ messages in thread
From: Bin Meng @ 2022-11-25 3:34 UTC (permalink / raw)
To: Alistair Francis
Cc: Bin Meng, qemu-devel, Alistair Francis, Bin Meng, Palmer Dabbelt,
qemu-riscv
On Fri, Nov 25, 2022 at 7:58 AM Alistair Francis <alistair23@gmail.com> wrote:
>
> 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.
>
On a screen, yes, but we can log to a file so there is no size limitation.
> GDB is a little different as people can interactively probe the
> registers they are interested in.
The dump routine is one of the debug methods too. If gdb stub provides
$sstatus directly I think we should do the same for dump_state for
consistency. Otherwise we can just provide $mstatus in gdb stub and
let user figure out sstatus value.
Regards,
Bin
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] target/riscv: Dump sstatus CSR in riscv_cpu_dump_state()
2022-11-25 3:34 ` Bin Meng
@ 2022-11-25 4:32 ` Alistair Francis
0 siblings, 0 replies; 6+ messages in thread
From: Alistair Francis @ 2022-11-25 4:32 UTC (permalink / raw)
To: Bin Meng
Cc: Bin Meng, qemu-devel, Alistair Francis, Bin Meng, Palmer Dabbelt,
qemu-riscv
On Fri, Nov 25, 2022 at 1:34 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Fri, Nov 25, 2022 at 7:58 AM Alistair Francis <alistair23@gmail.com> wrote:
> >
> > 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.
> >
>
> On a screen, yes, but we can log to a file so there is no size limitation.
And this function is commonly used as part of the HMP `info registers`
command. Which is dumped to the screen.
>
> > GDB is a little different as people can interactively probe the
> > registers they are interested in.
>
> The dump routine is one of the debug methods too. If gdb stub provides
> $sstatus directly I think we should do the same for dump_state for
> consistency. Otherwise we can just provide $mstatus in gdb stub and
> let user figure out sstatus value.
I feel like GDB and text register dumping are different debugging tools.
When using GDB a user should be able to probe all of the registers.
They can read/write to the CSRs and write scripts to interact with
them if they want.
When dumping the CPU state as text, we can't dump all CSRs. There are
too many, it becomes unmanageable. So we have to select a small number
that are the most important and most useful. As sstatus is a copy of
mstatus, which is described in the priv spec, it seems redundant to
print the same information twice.
I feel this list is only going to become longer and longer as we
enable other extensions, so we should try to avoid populating it with
redundant information. It is always clear what bits are propagated to
sstatus, as it's described in the spec.
Alistair
>
> Regards,
> Bin
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-11-25 4:34 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-22 15:46 [PATCH] target/riscv: Dump sstatus CSR in riscv_cpu_dump_state() Bin Meng
2022-11-23 0:02 ` Alistair Francis
2022-11-23 4:07 ` Bin Meng
2022-11-24 23:57 ` Alistair Francis
2022-11-25 3:34 ` Bin Meng
2022-11-25 4:32 ` Alistair Francis
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.