riscv: add all available CSRs to 'info registers'

[PATCH 3/3] target/riscv: print all available CSRs in riscv_cpu_dump_state()

Posted by Daniel Henrique Barboza 7 months, 2 weeks ago

At this moment we're printing a small selection of CSRs. There's no
particular reason to not print all of them.

We're ignoring the note about CSR_SSTATUS being ommited because it can
be read via CSR_MSTATUS. There's a huge list of CSRs that would fall in
this category and it would be an extra burden to manage them, not
mentioning having to document "we're not listing X because it's the same
value as Y" to users.

Remove 'dump_csrs' and use the existing 'csr_ops' array to print all
available CSRs. Create two helpers in csr.c to identify FPU and VPU CSRs
and skip them - they'll be printed in the FPU/VPU blocks later.

Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
 target/riscv/cpu.c | 55 ++++++++++++++++------------------------------
 target/riscv/cpu.h |  2 ++
 target/riscv/csr.c | 18 +++++++++++++++
 3 files changed, 39 insertions(+), 36 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 95d0b88937..ed1bf18625 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -544,44 +544,27 @@ static void riscv_cpu_dump_state(CPUState *cs, FILE *f, int flags)
 #endif
     qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "pc      ", env->pc);
 #ifndef CONFIG_USER_ONLY
-    {
-        static const int dump_csrs[] = {
-            CSR_MHARTID,
-            CSR_MSTATUS,
-            CSR_MSTATUSH,
-            /*
-             * CSR_SSTATUS is intentionally omitted here as its value
-             * can be figured out by looking at CSR_MSTATUS
-             */
-            CSR_HSTATUS,
-            CSR_VSSTATUS,
-            CSR_MIP,
-            CSR_MIE,
-            CSR_MIDELEG,
-            CSR_HIDELEG,
-            CSR_MEDELEG,
-            CSR_HEDELEG,
-            CSR_MTVEC,
-            CSR_STVEC,
-            CSR_VSTVEC,
-            CSR_MEPC,
-            CSR_SEPC,
-            CSR_VSEPC,
-            CSR_MCAUSE,
-            CSR_SCAUSE,
-            CSR_VSCAUSE,
-            CSR_MTVAL,
-            CSR_STVAL,
-            CSR_HTVAL,
-            CSR_MTVAL2,
-            CSR_MSCRATCH,
-            CSR_SSCRATCH,
-            CSR_SATP,
-        };
+    for (i = 0; i < ARRAY_SIZE(csr_ops); i++) {
+        int csrno = i;
 
-        for (i = 0; i < ARRAY_SIZE(dump_csrs); ++i) {
-            riscv_dump_csr(env, dump_csrs[i], f);
+        /*
+         * Early skip when possible since we're going
+         * through a lot of NULL entries.
+         */
+        if (csr_ops[csrno].predicate == NULL) {
+            continue;
         }
+
+        /*
+         * FPU and VPU CSRs will be printed in the
+         * CPU_DUMP_FPU/CPU_DUMP_VPU blocks later.
+         */
+        if (riscv_csr_is_fpu(csrno) ||
+            riscv_csr_is_vpu(csrno)) {
+            continue;
+        }
+
+        riscv_dump_csr(env, csrno, f);
     }
 #endif
 
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 4a862da615..ecdf709c2d 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -967,6 +967,8 @@ bool riscv_cpu_accelerator_compatible(RISCVCPU *cpu);
 
 /* CSR function table */
 extern riscv_csr_operations csr_ops[CSR_TABLE_SIZE];
+bool riscv_csr_is_fpu(int csrno);
+bool riscv_csr_is_vpu(int csrno);
 
 extern const bool valid_vm_1_10_32[], valid_vm_1_10_64[];
 
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 6296ecd1e1..229257b31b 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -5799,6 +5799,24 @@ static RISCVException write_jvt(CPURISCVState *env, int csrno,
     return RISCV_EXCP_NONE;
 }
 
+bool riscv_csr_is_fpu(int csrno)
+{
+    if (!csr_ops[csrno].predicate) {
+        return false;
+    }
+
+    return csr_ops[csrno].predicate == fs;
+}
+
+bool riscv_csr_is_vpu(int csrno)
+{
+    if (!csr_ops[csrno].predicate) {
+        return false;
+    }
+
+    return csr_ops[csrno].predicate == vs;
+}
+
 /*
  * Control and Status Register function table
  * riscv_csr_operations::predicate() must be provided for an implemented CSR
-- 
2.49.0

Re: [PATCH 3/3] target/riscv: print all available CSRs in riscv_cpu_dump_state()

Posted by Alistair Francis 4 months, 3 weeks ago

On Tue, Jun 24, 2025 at 3:22 AM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
> At this moment we're printing a small selection of CSRs. There's no
> particular reason to not print all of them.

The reason was that there are just so many and printing everything
makes it hard for people to read. So we just print the "main" ones and
people can then probe the more obscure ones they are interested in.

Do we actually want to print all of them?

>
> We're ignoring the note about CSR_SSTATUS being ommited because it can
> be read via CSR_MSTATUS. There's a huge list of CSRs that would fall in
> this category and it would be an extra burden to manage them, not
> mentioning having to document "we're not listing X because it's the same
> value as Y" to users.
>
> Remove 'dump_csrs' and use the existing 'csr_ops' array to print all
> available CSRs. Create two helpers in csr.c to identify FPU and VPU CSRs
> and skip them - they'll be printed in the FPU/VPU blocks later.
>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>

The actual code looks fine though

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  target/riscv/cpu.c | 55 ++++++++++++++++------------------------------
>  target/riscv/cpu.h |  2 ++
>  target/riscv/csr.c | 18 +++++++++++++++
>  3 files changed, 39 insertions(+), 36 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 95d0b88937..ed1bf18625 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -544,44 +544,27 @@ static void riscv_cpu_dump_state(CPUState *cs, FILE *f, int flags)
>  #endif
>      qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "pc      ", env->pc);
>  #ifndef CONFIG_USER_ONLY
> -    {
> -        static const int dump_csrs[] = {
> -            CSR_MHARTID,
> -            CSR_MSTATUS,
> -            CSR_MSTATUSH,
> -            /*
> -             * CSR_SSTATUS is intentionally omitted here as its value
> -             * can be figured out by looking at CSR_MSTATUS
> -             */
> -            CSR_HSTATUS,
> -            CSR_VSSTATUS,
> -            CSR_MIP,
> -            CSR_MIE,
> -            CSR_MIDELEG,
> -            CSR_HIDELEG,
> -            CSR_MEDELEG,
> -            CSR_HEDELEG,
> -            CSR_MTVEC,
> -            CSR_STVEC,
> -            CSR_VSTVEC,
> -            CSR_MEPC,
> -            CSR_SEPC,
> -            CSR_VSEPC,
> -            CSR_MCAUSE,
> -            CSR_SCAUSE,
> -            CSR_VSCAUSE,
> -            CSR_MTVAL,
> -            CSR_STVAL,
> -            CSR_HTVAL,
> -            CSR_MTVAL2,
> -            CSR_MSCRATCH,
> -            CSR_SSCRATCH,
> -            CSR_SATP,
> -        };
> +    for (i = 0; i < ARRAY_SIZE(csr_ops); i++) {
> +        int csrno = i;
>
> -        for (i = 0; i < ARRAY_SIZE(dump_csrs); ++i) {
> -            riscv_dump_csr(env, dump_csrs[i], f);
> +        /*
> +         * Early skip when possible since we're going
> +         * through a lot of NULL entries.
> +         */
> +        if (csr_ops[csrno].predicate == NULL) {
> +            continue;
>          }
> +
> +        /*
> +         * FPU and VPU CSRs will be printed in the
> +         * CPU_DUMP_FPU/CPU_DUMP_VPU blocks later.
> +         */
> +        if (riscv_csr_is_fpu(csrno) ||
> +            riscv_csr_is_vpu(csrno)) {
> +            continue;
> +        }
> +
> +        riscv_dump_csr(env, csrno, f);
>      }
>  #endif
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 4a862da615..ecdf709c2d 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -967,6 +967,8 @@ bool riscv_cpu_accelerator_compatible(RISCVCPU *cpu);
>
>  /* CSR function table */
>  extern riscv_csr_operations csr_ops[CSR_TABLE_SIZE];
> +bool riscv_csr_is_fpu(int csrno);
> +bool riscv_csr_is_vpu(int csrno);
>
>  extern const bool valid_vm_1_10_32[], valid_vm_1_10_64[];
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 6296ecd1e1..229257b31b 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -5799,6 +5799,24 @@ static RISCVException write_jvt(CPURISCVState *env, int csrno,
>      return RISCV_EXCP_NONE;
>  }
>
> +bool riscv_csr_is_fpu(int csrno)
> +{
> +    if (!csr_ops[csrno].predicate) {
> +        return false;
> +    }
> +
> +    return csr_ops[csrno].predicate == fs;
> +}
> +
> +bool riscv_csr_is_vpu(int csrno)
> +{
> +    if (!csr_ops[csrno].predicate) {
> +        return false;
> +    }
> +
> +    return csr_ops[csrno].predicate == vs;
> +}
> +
>  /*
>   * Control and Status Register function table
>   * riscv_csr_operations::predicate() must be provided for an implemented CSR
> --
> 2.49.0
>
>

Re: [PATCH 3/3] target/riscv: print all available CSRs in riscv_cpu_dump_state()

Posted by Daniel Henrique Barboza 4 months, 3 weeks ago


On 9/15/25 12:22 AM, Alistair Francis wrote:
> On Tue, Jun 24, 2025 at 3:22 AM Daniel Henrique Barboza
> <dbarboza@ventanamicro.com> wrote:
>>
>> At this moment we're printing a small selection of CSRs. There's no
>> particular reason to not print all of them.
> 
> The reason was that there are just so many and printing everything
> makes it hard for people to read. So we just print the "main" ones and
> people can then probe the more obscure ones they are interested in.
> 
> Do we actually want to print all of them?

In case we have no idea what is going on with the CPU, e.g. there is something
wrong and you can't quite put your finger on it, outputting all CSRs can help
identify errors in unusual places you wouldn't expect.

After identifying the problematic CSRs we can then use the MonitorDef API to
fetch just the CSRs we want with "p $<reg>".

At this is the sort of use case I was imagining with these patches. They'll be
more effective when used together.


Thanks,

Daniel
  

> 
>>
>> We're ignoring the note about CSR_SSTATUS being ommited because it can
>> be read via CSR_MSTATUS. There's a huge list of CSRs that would fall in
>> this category and it would be an extra burden to manage them, not
>> mentioning having to document "we're not listing X because it's the same
>> value as Y" to users.
>>
>> Remove 'dump_csrs' and use the existing 'csr_ops' array to print all
>> available CSRs. Create two helpers in csr.c to identify FPU and VPU CSRs
>> and skip them - they'll be printed in the FPU/VPU blocks later.
>>
>> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> 
> The actual code looks fine though
> 
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> 
> Alistair
> 
>> ---
>>   target/riscv/cpu.c | 55 ++++++++++++++++------------------------------
>>   target/riscv/cpu.h |  2 ++
>>   target/riscv/csr.c | 18 +++++++++++++++
>>   3 files changed, 39 insertions(+), 36 deletions(-)
>>
>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>> index 95d0b88937..ed1bf18625 100644
>> --- a/target/riscv/cpu.c
>> +++ b/target/riscv/cpu.c
>> @@ -544,44 +544,27 @@ static void riscv_cpu_dump_state(CPUState *cs, FILE *f, int flags)
>>   #endif
>>       qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "pc      ", env->pc);
>>   #ifndef CONFIG_USER_ONLY
>> -    {
>> -        static const int dump_csrs[] = {
>> -            CSR_MHARTID,
>> -            CSR_MSTATUS,
>> -            CSR_MSTATUSH,
>> -            /*
>> -             * CSR_SSTATUS is intentionally omitted here as its value
>> -             * can be figured out by looking at CSR_MSTATUS
>> -             */
>> -            CSR_HSTATUS,
>> -            CSR_VSSTATUS,
>> -            CSR_MIP,
>> -            CSR_MIE,
>> -            CSR_MIDELEG,
>> -            CSR_HIDELEG,
>> -            CSR_MEDELEG,
>> -            CSR_HEDELEG,
>> -            CSR_MTVEC,
>> -            CSR_STVEC,
>> -            CSR_VSTVEC,
>> -            CSR_MEPC,
>> -            CSR_SEPC,
>> -            CSR_VSEPC,
>> -            CSR_MCAUSE,
>> -            CSR_SCAUSE,
>> -            CSR_VSCAUSE,
>> -            CSR_MTVAL,
>> -            CSR_STVAL,
>> -            CSR_HTVAL,
>> -            CSR_MTVAL2,
>> -            CSR_MSCRATCH,
>> -            CSR_SSCRATCH,
>> -            CSR_SATP,
>> -        };
>> +    for (i = 0; i < ARRAY_SIZE(csr_ops); i++) {
>> +        int csrno = i;
>>
>> -        for (i = 0; i < ARRAY_SIZE(dump_csrs); ++i) {
>> -            riscv_dump_csr(env, dump_csrs[i], f);
>> +        /*
>> +         * Early skip when possible since we're going
>> +         * through a lot of NULL entries.
>> +         */
>> +        if (csr_ops[csrno].predicate == NULL) {
>> +            continue;
>>           }
>> +
>> +        /*
>> +         * FPU and VPU CSRs will be printed in the
>> +         * CPU_DUMP_FPU/CPU_DUMP_VPU blocks later.
>> +         */
>> +        if (riscv_csr_is_fpu(csrno) ||
>> +            riscv_csr_is_vpu(csrno)) {
>> +            continue;
>> +        }
>> +
>> +        riscv_dump_csr(env, csrno, f);
>>       }
>>   #endif
>>
>> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
>> index 4a862da615..ecdf709c2d 100644
>> --- a/target/riscv/cpu.h
>> +++ b/target/riscv/cpu.h
>> @@ -967,6 +967,8 @@ bool riscv_cpu_accelerator_compatible(RISCVCPU *cpu);
>>
>>   /* CSR function table */
>>   extern riscv_csr_operations csr_ops[CSR_TABLE_SIZE];
>> +bool riscv_csr_is_fpu(int csrno);
>> +bool riscv_csr_is_vpu(int csrno);
>>
>>   extern const bool valid_vm_1_10_32[], valid_vm_1_10_64[];
>>
>> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
>> index 6296ecd1e1..229257b31b 100644
>> --- a/target/riscv/csr.c
>> +++ b/target/riscv/csr.c
>> @@ -5799,6 +5799,24 @@ static RISCVException write_jvt(CPURISCVState *env, int csrno,
>>       return RISCV_EXCP_NONE;
>>   }
>>
>> +bool riscv_csr_is_fpu(int csrno)
>> +{
>> +    if (!csr_ops[csrno].predicate) {
>> +        return false;
>> +    }
>> +
>> +    return csr_ops[csrno].predicate == fs;
>> +}
>> +
>> +bool riscv_csr_is_vpu(int csrno)
>> +{
>> +    if (!csr_ops[csrno].predicate) {
>> +        return false;
>> +    }
>> +
>> +    return csr_ops[csrno].predicate == vs;
>> +}
>> +
>>   /*
>>    * Control and Status Register function table
>>    * riscv_csr_operations::predicate() must be provided for an implemented CSR
>> --
>> 2.49.0
>>
>>

[PATCH 1/3] target/riscv/cpu: add riscv_dump_csr() helper
[PATCH 2/3] target/riscv/cpu: print all FPU CSRs in riscv_cpu_dump_state()
[PATCH 3/3] target/riscv: print all available CSRs in riscv_cpu_dump_state()