On 6/30/25 9:07 PM, Dr. David Alan Gilbert wrote:
* Daniel Henrique Barboza (dbarb...@ventanamicro.com) wrote:

Hi Daniel,

The RISC-V target has *a lot* of CPU registers, with more registers
being added along the way when new extensions are added. In this world,
'info registers' will throw a wall of text that can be annoying to deal
with when the user wants to verify the value of just a couple of
registers.

Add a new 'info register' HMP command that prints a specific register.
The semantics, and implementation, is similar to what 'info registers'
already does, i.e. '-a' will print a register for all VCPUs and it's
possible to print a reg for a specific VCPU.

A RISC-V implementation is included via riscv_cpu_dump_register().

Here's an example:

Welcome to Buildroot
buildroot login: QEMU 10.0.50 monitor - type 'help' for more information
(qemu) info register mstatus

CPU#0
  mstatus  0000000a000000a0
(qemu) info register mstatus -a

CPU#0
  mstatus  0000000a000000a0

CPU#1
  mstatus  0000000a000000a0
(qemu)

OK, that makes some sense; some comments:

    a) I'd make that a list of register names so you can print a handful out
for whatever you're debugging.
        info register gpr0,gpr1,pc
    b) (But then you start wondering if having some predefined like 'gprs'
would work)
    c) It turns out there's an old, but limited similar thing called 
MonitorDef's
where a target can define a list of registers; so you might want to define
target_monitor_defs() OR target_get_monitor_def() for your architecture anyway,
on x86 you can do things like
           p/x $pc
           x/10i $pc
           p/x $eax

       It doesn't seem very well maintained in the architectures though; the x86
   one is prehistoric for example.

But it's a cool API to have it seems. If we couple this with 'cpu N' commands 
we can
print registers from multiple CPUs.

Perhaps we still want an 'info registers' or an 'info registers -r 
reg1,reg2...' to
print multiple registers anyway, but I don't see a reason to not support the 
MonitorDef
API in RISC-V too. I'll take a look.



   d) Another way would be to modify info registers to take an optional
      -r register-list

Anyway, those are _suggestions_ only.


I don't mind adding new options in 'info registers' to do what this new API I'm
proposing. In fact that was my original idea, and the reason I added a new API 
was
more in a fear of adding too much stuff into it and the potential pushback.

IIUC from the feedbacks we have so far, we could augment 'info registers' as 
follows:

- add a '-h' option to print the names of all available registers
- add a '-r' option followed by a list of registers to be dumped


In parallel I'll take a look in MonitorDef too. Thanks,

Daniel






Dave

The API is introduced as TARGET_RISCV only.

Cc: Dr. David Alan Gilbert <d...@treblig.org>
Cc: Marcel Apfelbaum <marcel.apfelb...@gmail.com>
Cc: Philippe Mathieu-Daudé <phi...@linaro.org>
Signed-off-by: Daniel Henrique Barboza <dbarb...@ventanamicro.com>
---
  hmp-commands-info.hx         | 17 +++++++++++++
  hw/core/cpu-common.c         |  8 ++++++
  include/hw/core/cpu.h        | 11 +++++++++
  include/monitor/hmp-target.h |  1 +
  monitor/hmp-cmds-target.c    | 30 ++++++++++++++++++++++
  target/riscv/cpu.c           | 48 ++++++++++++++++++++++++++++++++++++
  6 files changed, 115 insertions(+)

diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index 639a450ee5..f3561e4a02 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -113,6 +113,23 @@ SRST
      Show the cpu registers.
  ERST
+#if defined(TARGET_RISCV)
+    {
+        .name       = "register",
+        .args_type  = "register:s,cpustate_all:-a,vcpu:i?",
+        .params     = "[register|-a|vcpu]",
+        .help       = "show a cpu register (-a: show the register value for all 
cpus;"
+                      " vcpu: specific vCPU to query; show the current CPU's 
register if"
+                      " no vcpu is specified)",
+        .cmd        = hmp_info_register,
+    },
+
+SRST
+  ``info register``
+    Show a cpu register.
+ERST
+#endif
+
  #if defined(TARGET_I386)
      {
          .name       = "lapic",
diff --git a/hw/core/cpu-common.c b/hw/core/cpu-common.c
index 39e674aca2..9c65ce1537 100644
--- a/hw/core/cpu-common.c
+++ b/hw/core/cpu-common.c
@@ -108,6 +108,14 @@ void cpu_dump_state(CPUState *cpu, FILE *f, int flags)
      }
  }
+void cpu_dump_register(CPUState *cpu, const char *reg, FILE *f)
+{
+    if (cpu->cc->dump_register) {
+        cpu_synchronize_state(cpu);
+        cpu->cc->dump_register(cpu, reg, f);
+    }
+}
+
  void cpu_reset(CPUState *cpu)
  {
      device_cold_reset(DEVICE(cpu));
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 33296a1c08..b9ddce22bd 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -160,6 +160,7 @@ struct CPUClass {
      int (*memory_rw_debug)(CPUState *cpu, vaddr addr,
                             uint8_t *buf, size_t len, bool is_write);
      void (*dump_state)(CPUState *cpu, FILE *, int flags);
+    void (*dump_register)(CPUState *cpu, const char *reg, FILE *);
      void (*query_cpu_fast)(CPUState *cpu, CpuInfoFast *value);
      int64_t (*get_arch_id)(CPUState *cpu);
      void (*set_pc)(CPUState *cpu, vaddr value);
@@ -693,6 +694,16 @@ enum CPUDumpFlags {
   */
  void cpu_dump_state(CPUState *cpu, FILE *f, int flags);
+/**
+ * cpu_dump_register:
+ * @cpu: The CPU whose register state is to be dumped.
+ * @reg: CPU register name to be dumped.
+ * @f: If non-null, dump to this stream, else to current print sink.
+ *
+ * Dumps CPU register state.
+ */
+void cpu_dump_register(CPUState *cpu, const char *reg, FILE *f);
+
  /**
   * cpu_get_phys_page_attrs_debug:
   * @cpu: The CPU to obtain the physical page address for.
diff --git a/include/monitor/hmp-target.h b/include/monitor/hmp-target.h
index b679aaebbf..da9d690f89 100644
--- a/include/monitor/hmp-target.h
+++ b/include/monitor/hmp-target.h
@@ -57,6 +57,7 @@ void hmp_info_via(Monitor *mon, const QDict *qdict);
  void hmp_memory_dump(Monitor *mon, const QDict *qdict);
  void hmp_physical_memory_dump(Monitor *mon, const QDict *qdict);
  void hmp_info_registers(Monitor *mon, const QDict *qdict);
+void hmp_info_register(Monitor *mon, const QDict *qdict);
  void hmp_gva2gpa(Monitor *mon, const QDict *qdict);
  void hmp_gpa2hva(Monitor *mon, const QDict *qdict);
  void hmp_gpa2hpa(Monitor *mon, const QDict *qdict);
diff --git a/monitor/hmp-cmds-target.c b/monitor/hmp-cmds-target.c
index 8eaf70d9c9..43f509aa60 100644
--- a/monitor/hmp-cmds-target.c
+++ b/monitor/hmp-cmds-target.c
@@ -121,6 +121,36 @@ void hmp_info_registers(Monitor *mon, const QDict *qdict)
      }
  }
+/*
+ * Based on hmp_info_registers().
+ */
+void hmp_info_register(Monitor *mon, const QDict *qdict)
+{
+    const char *reg = qdict_get_try_str(qdict, "register");
+    bool all_cpus = qdict_get_try_bool(qdict, "cpustate_all", false);
+    int vcpu = qdict_get_try_int(qdict, "vcpu", -1);
+    CPUState *cs;
+
+    if (all_cpus) {
+        CPU_FOREACH(cs) {
+            cpu_dump_register(cs, reg, NULL);
+        }
+    } else {
+        cs = vcpu >= 0 ? qemu_get_cpu(vcpu) : mon_get_cpu(mon);
+
+        if (!cs) {
+            if (vcpu >= 0) {
+                monitor_printf(mon, "CPU#%d not available\n", vcpu);
+            } else {
+                monitor_printf(mon, "No CPU available\n");
+            }
+            return;
+        }
+
+        cpu_dump_register(cs, reg, NULL);
+    }
+}
+
  static void memory_dump(Monitor *mon, int count, int format, int wsize,
                          hwaddr addr, int is_physical)
  {
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index e3f8ecef68..8b3edf7b23 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -640,6 +640,53 @@ static void riscv_cpu_dump_state(CPUState *cs, FILE *f, 
int flags)
      }
  }
+static void riscv_cpu_dump_register(CPUState *cs, const char *reg, FILE *f)
+{
+    RISCVCPU *cpu = RISCV_CPU(cs);
+    CPURISCVState *env = &cpu->env;
+    bool match_found = false;
+    int i;
+
+    for (i = 0; i < ARRAY_SIZE(csr_ops); i++) {
+        RISCVException res;
+        target_ulong val = 0;
+        int csrno = i;
+
+        /*
+         * Early skip when possible since we're going
+         * through a lot of NULL entries.
+         */
+        if (csr_ops[csrno].predicate == NULL) {
+            continue;
+        }
+
+        /*
+         * We're doing partial register name matching,
+         * e.g. 'mhpm' will match all registers that
+         * starts with 'mhpm'.
+         */
+        if (strncasecmp(csr_ops[csrno].name, reg, strlen(reg)) != 0) {
+            continue;
+        }
+
+        res = riscv_csrrw_debug(env, csrno, &val, 0, 0);
+
+        /*
+         * Rely on the smode, hmode, etc, predicates within csr.c
+         * to do the filtering of the registers that are present.
+         */
+        if (res == RISCV_EXCP_NONE) {
+            if (!match_found) {
+                match_found = true;
+                qemu_fprintf(f, "\nCPU#%d\n", cs->cpu_index);
+            }
+
+            qemu_fprintf(f, " %-8s " TARGET_FMT_lx "\n",
+                         csr_ops[csrno].name, val);
+        }
+    }
+}
+
  static void riscv_cpu_set_pc(CPUState *cs, vaddr value)
  {
      RISCVCPU *cpu = RISCV_CPU(cs);
@@ -2690,6 +2737,7 @@ static void riscv_cpu_common_class_init(ObjectClass *c, 
const void *data)
cc->class_by_name = riscv_cpu_class_by_name;
      cc->dump_state = riscv_cpu_dump_state;
+    cc->dump_register = riscv_cpu_dump_register;
      cc->set_pc = riscv_cpu_set_pc;
      cc->get_pc = riscv_cpu_get_pc;
      cc->gdb_read_register = riscv_cpu_gdb_read_register;
--
2.49.0



Reply via email to