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

[Paolo Bonzini]

Il 25/01/2013 13:34, Eduardo Habkost ha scritto:
> On Fri, Jan 25, 2013 at 10:14:43AM -0200, Luiz Capitulino wrote:
>> 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.
> 
> Maybe a GUI that needs to show that debug info should use gdb instead?

That's a bit heavyweight.

>> 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
> 
> Is it?

I think so.  If a watchdog fires, for example, it may be useful to
include register information in the log without firing up gdb.

>> 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
> 
> We already have CPU QOM objects, we just need to add a
> methods/properties so each per-architecture subclass will implement
> what's necessary for the command.
> 
> We could go even further: just use the qom-* commands. Then if there's
> some set of registers we really want to expose to the outside, just add
> them as properties to the CPU object.

Makes sense.

> QOM properties would be problematic as well, as
> object_property_{get,set}_int() and QInt support only 64-bit values. But
> in the worst case we can work around it using strings or _high/_low
> properties.

Yes, you can use a string.  I assume this to be mostly for human
consumption (ultimately), as opposed to the gdbstub where the values can
be used by gdb to do math.

>>
>>>     * 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.
> 
> QOM is even more flexible: it has inheritance and allows introspection
> of properties at runtime.
> 
>>
>>>     * 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.
> 
> Agreed.

i386-softmmu could have eax and x86_64-softmmu rax, but that's it.  I agree.

Paolo