Re: [Qemu-devel] [PATCH] qapi/x86: add control registers to query-cpus

[Luiz Capitulino]

On Thu, 24 Jan 2013 13:12:08 -0500
Peter Feiner <address@hidden> wrote:

> > What about converting 'info registers' to QMP (ie. having 
> > query-cpu-registers)?
> 
> We had thought about it, but we decided to go with this lower hanging fruit
> because it provides immediately useful functionality at a low implementation
> cost. It's harder (for us) to think of why would anyone want to know XMM12 or
> r10 in the general case outside of gdb, which is already supported.

For the same reason you need the other registers now. Besides, it would be
nice to allow GUIs to have more debug info like this.

Let me re-state the problem for the CC'ed people: you're adding x86 control
registers to the query-cpus command. I think this has a few problems:

 1. Won't "scale", as query-cpus will become a huge mess if people start
    doing this for other archs

 2. query-cpus is bad designed. I'd prefer adding new commands instead of
    extending it (unless the information is general enough)

 3. It's very desirable to have registers info in QMP

The obvious suggestion is to add query-cpu-registers. I understand this has a
few problems (see questions below), but I think the following incremental
approach could work:

 1. Add a CPURegisters union
 2. Each CPU arch is added as a type to the union (eg. CPUX86Registers)
 3. query-cpu-registers returns the union
 4. Move do_info_registers() to hmp.c as hmp_info_registers()
 5. Change hmp_info_registers() to first call qmp_query_cpu_registers(), if
    this returns the CPU arch it expects, then print it. Otherwise fallback
    to cpu_dump_state()

You start by adding CPUX86Registers. Other CPUs are added as needed.

What do the CC'ed people think?

> Additionally, porting over the entire functionality of 'info registers' has a
> bunch of wrinkles:
> 
>     * I'm afraid that 'info registers' couldn't so much be converted from HMP 
> to
>       QMP as added. That is, most of the work done by each target's various
>       'info registers' implementation (i.e., the target's cpu_dump_state
>       function) is formatting the output nicely. So most of the existing
>       'info registers' logic would remain and a qmp_query_cpu_registers would
>       have to be added for each target.

Explained above how this could be solved.

>     * How do you represent 128bit registers (e.g., XMM)?  Perhaps add a
>       'int128' type to QMP? The simplest solution is 64-bit components (e.g.,
>       XMM0_low, XMM0_high).

Yes, that's where json is problematic. We could have _low and _high you
suggest or represent 128bit registers as strings.

>     * Like query-cpus, does the schema make all registers optional because
>       they're architecture specific? This would entail hundreds of data 
> fields.
>       Or should query-cpu-registers return a list of (name, value) pairs?

QAPI has union support, I think that's the best way to solve this. Search
for 'union' in qapi-schema.json.

>     * Should register names change depending on processor mode (e.g., eax vs
>       rax), or should they have canonical names (e.g., always use "a" or 
> "rax").

I don't think they should change.