* Daniel Henrique Barboza (dbarb...@ventanamicro.com) wrote:
>
>
> 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.
If you were doing MonitorDef, I think of the two interfaces,
target_get_monitor_def
looks easier to me, but I might be wrong.
> > 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
Yeh.
> In parallel I'll take a look in MonitorDef too. Thanks,
Nice.
Dave
>
> 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
> > >
>
--
-----Open up your eyes, open up your mind, open up your code -------
/ Dr. David Alan Gilbert | Running GNU/Linux | Happy \
\ dave @ treblig.org | | In Hex /
\ _________________________|_____ http://www.treblig.org |_______/
