| Message ID | 1358977550-8217-1-git-send-email-peter@gridcentric.ca |
|---|---|
| State | New |
| Headers | show |
On 01/23/2013 02:45 PM, peter@gridcentric.ca wrote: > From: Peter Feiner <peter@gridcentric.ca> > > Adds control registers that govern virtual address translation to query-cpus. > > Given these registers and the guest's physical memory, which can be obtained > with dump-guest-memory, a client can perform virtual-to-physical translations. > This is useful for debugging and introspection. > > Signed-off-by: Peter Feiner <peter@gridcentric.ca> > --- > +++ b/qapi-schema.json > @@ -569,6 +569,15 @@ > # If the target is Sparc, this is the PC component of the > # instruction pointer. > # > +# @cr0: #optional If the target is i386 or x86_64, this is the CR1 register. Might be worth mentioning that this field (and the others) was added in 1.4.
On Wed, Jan 23, 2013 at 4:49 PM, Eric Blake <eblake@redhat.com> wrote:
> Might be worth mentioning that this field (and the others) was added in 1.4.
Thanks Eric. I'll put that in V2.
On Wed, 23 Jan 2013 16:45:50 -0500 peter@gridcentric.ca wrote: > From: Peter Feiner <peter@gridcentric.ca> > > Adds control registers that govern virtual address translation to query-cpus. > > Given these registers and the guest's physical memory, which can be obtained > with dump-guest-memory, a client can perform virtual-to-physical translations. > This is useful for debugging and introspection. What about converting 'info registers' to QMP (ie. having query-cpu-registers)?
> 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.
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.
* 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).
* 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?
* 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").
On Thu, 24 Jan 2013 13:12:08 -0500 Peter Feiner <peter@gridcentric.ca> 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.
On Fri, Jan 25, 2013 at 10:14:43AM -0200, Luiz Capitulino wrote: > On Thu, 24 Jan 2013 13:12:08 -0500 > Peter Feiner <peter@gridcentric.ca> 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? > > 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? > > 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. > 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? I still don't see why we need this. But if we need it, why aren't the qom-* commands good enough to implement this, instead of adding new commands? > > > 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. 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. > > > * 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.
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 <peter@gridcentric.ca> 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
On Fri, 25 Jan 2013 13:38:18 +0100 Paolo Bonzini <pbonzini@redhat.com> wrote: > 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 <peter@gridcentric.ca> 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. Yes, and the same argument could be made for exposing control registers 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 > > > > 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. Oh, agreed. That would be a lot better and I didn't even consider this. What's the status of the CPU work on QOM? Would it be possible to expose registers info already or some work is still needed? If work is needed, what's missing?
Am 25.01.2013 13:14, schrieb Luiz Capitulino: > On Thu, 24 Jan 2013 13:12:08 -0500 > Peter Feiner <peter@gridcentric.ca> 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? I wonder why this needs to be a custom QMP command? I have done a lot of preparations to make all CPUState QOM objects, so it would be easily possible to add QOM properties for the registers and reuse the existing qom-get QMP command. That would of course require deciding on property names and types ;) and on getting usable QOM paths to access the CPUs. Is there a particular release this feature is being requested for? Regards, Andreas
On Fri, 25 Jan 2013 13:51:14 +0100 Andreas Färber <afaerber@suse.de> wrote: > Am 25.01.2013 13:14, schrieb Luiz Capitulino: > > On Thu, 24 Jan 2013 13:12:08 -0500 > > Peter Feiner <peter@gridcentric.ca> 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? > > I wonder why this needs to be a custom QMP command? It doesn't. I just forgot about QOM when responding to the original email. See my last email to this thread.
On Fri, Jan 25, 2013 at 10:45:11AM -0200, Luiz Capitulino wrote: [...] > > >> 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. > > Oh, agreed. That would be a lot better and I didn't even consider this. > > What's the status of the CPU work on QOM? Would it be possible to expose > registers info already or some work is still needed? If work is needed, > what's missing? The CPUs are already DeviceState objects, but they are not on any bus. I am not sure if they are visible in the device tree and acessible using qom-* without a bus, but IIRC somebody told me that they are. If they are already visible and acessible using qom-list/qom-get, we just need to add properties for each register we want to expose.
Am 25.01.2013 13:45, schrieb Luiz Capitulino: > On Fri, 25 Jan 2013 13:38:18 +0100 > Paolo Bonzini <pbonzini@redhat.com> wrote: > >> Il 25/01/2013 13:34, Eduardo Habkost ha scritto: >>> 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. > > Oh, agreed. That would be a lot better and I didn't even consider this. > > What's the status of the CPU work on QOM? Would it be possible to expose > registers info already or some work is still needed? If work is needed, > what's missing? Should be possible since v1.1. :) I made an demo for ARM once but that didn't get applied at the time due to CP15 rework. Elsewhere it's waiting for creative volunteers. Similarly, my invasive CPUState refactoring series aim at making generic "halted" etc. properties available for introspection. We just need to be aware that inspecting register values is one thing, but that letting QMP qom-set arbitrary registers during runtime could cause unpredictable issues. Checks on dev->realized could be added as precaution (cf. qdev static properties). The work needed mentioned elsewhere was that users need to know which object to qom-get properties on. We thus need to object_property_add_child() the CPUs somewhere, and this is going to be machine-specific as opposed to target-specific as opposed to generic. ;) If needed, we could (given any such paths) have generic link properties. Regards, Andreas
On Fri, Jan 25, 2013 at 8:08 AM, Andreas Färber <afaerber@suse.de> wrote: > We just need to be aware that inspecting register values is one thing, > but that letting QMP qom-set arbitrary registers during runtime could > cause unpredictable issues. Checks on dev->realized could be added as > precaution (cf. qdev static properties). I just want to inspect register values. The implementation of qom-set for CPU registers could just always fail. > The work needed mentioned elsewhere was that users need to know which > object to qom-get properties on. We thus need to > object_property_add_child() the CPUs somewhere, and this is going to be > machine-specific as opposed to target-specific as opposed to generic. ;) > If needed, we could (given any such paths) have generic link properties. I like this QOM approach: there's no point in coming up with a bunch of per-arch data structures and queries when CPUs are already represented in QOM. When I have some time, I'll submit a patch for the QOM approach. On Fri, Jan 25, 2013 at 7:51 AM, Andreas Färber <afaerber@suse.de> wrote: > Is there a particular release this feature is being requested for? We want our product to work with stock qemu-1.4.0 (currently, Gridcentric ships QEMU along with a couple of patches). So I had hoped this would get into qemu-1.4.0. However, since doing this *right* (i.e. using QOM) isn't such a small patch, we aren't holding our breath for this feature making it into 1.4 (i.e., I probably won't get around to submitting the QOM patch in time). Since I submitted this patch, we've decided to glean the information by parsing HMP's 'info registers', which is available in stock QEMU. So, from our perspective, including registers in QMP in 1.4 isn't important. However, it would be nice to spare other people the joy (pain?) of parsing HMP in the future ;-)
diff --git a/cpus.c b/cpus.c index a4390c3..e9cc620 100644 --- a/cpus.c +++ b/cpus.c @@ -1224,6 +1224,14 @@ CpuInfoList *qmp_query_cpus(Error **errp) #if defined(TARGET_I386) info->value->has_pc = true; info->value->pc = env->eip + env->segs[R_CS].base; + info->value->has_cr0 = true; + info->value->cr0 = env->cr[0]; + info->value->has_cr3 = true; + info->value->cr3 = env->cr[3]; + info->value->has_cr4 = true; + info->value->cr4 = env->cr[4]; + info->value->has_efer = true; + info->value->efer = env->efer; #elif defined(TARGET_PPC) info->value->has_nip = true; info->value->nip = env->nip; diff --git a/qapi-schema.json b/qapi-schema.json index 6d7252b..80df503 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -569,6 +569,15 @@ # If the target is Sparc, this is the PC component of the # instruction pointer. # +# @cr0: #optional If the target is i386 or x86_64, this is the CR1 register. +# +# @cr3: #optional If the target is i386 or x86_64, this is the CR3 register. +# +# @cr4: #optional If the target is i386 or x86_64, this is the CR4 register. +# +# @efer: #optional If the target is i386 or x86_64, this is the "efer" +# (extended features) register. +# # @nip: #optional If the target is PPC, the instruction pointer # # @npc: #optional If the target is Sparc, the NPC component of the instruction @@ -585,6 +594,7 @@ ## { 'type': 'CpuInfo', 'data': {'CPU': 'int', 'current': 'bool', 'halted': 'bool', '*pc': 'int', + '*cr0': 'int', '*cr3': 'int', '*cr4': 'int', '*efer', 'int', '*nip': 'int', '*npc': 'int', '*PC': 'int', 'thread_id': 'int'} } ##