Re: [PATCH] hmp-cmds-target, target/riscv: add 'info register'

[Daniel Henrique Barboza <dbarboza@ventanamicro.com>]

On 6/30/25 1:00 PM, Philippe Mathieu-Daudé wrote:
> Hi Daniel,
> 
> On 30/6/25 15:22, Daniel Henrique Barboza wrote:
>> 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)
>>
>> The API is introduced as TARGET_RISCV only.
>>
>> Cc: Dr. David Alan Gilbert <dave@treblig.org>
>> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
>> Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
>> Signed-off-by: Daniel Henrique Barboza <dbarboza@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)
> 
> Just make this command available for all targets, displaying
> "No such register" or better when no handler registered.

What about "<targe-name> does not support this command" ?


> 
>> +    {
>> +        .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)",
> 
> I'd invert the default behavior: dump for all vcpus except if a specific
> one is specified.

Fair enough.

> 
> I wonder about a 'info register -h' do list all register names available.

That's a neat idea. I'll see what I can do.


Thanks,

Daniel

> 
>> +        .cmd        = hmp_info_register,
>> +    },
>> +
>> +SRST
>> +  ``info register``
>> +    Show a cpu register.
>> +ERST
>> +#endif
> 
> Regards,
> 
> Phil.