Patchwork [PATCHv2] add "info ioapic" monitor command

login
register
mail settings
Submitter Gleb Natapov
Date Dec. 30, 2009, 11:50 a.m.
Message ID <20091230115043.GD26899@redhat.com>
Download mbox | patch
Permalink /patch/41905/
State New
Headers show

Comments

Gleb Natapov - Dec. 30, 2009, 11:50 a.m.
Knowing ioapic configuration is very useful for the poor soles
how need to debug guest occasionally.

Signed-off-by: Gleb Natapov <gleb@redhat.com>
--

I am starring to learn this QObject kung-fu. One question:
Why qlist_iter(..., func, ...) and not
 FOREACH_QOBJ() {
   do things
 }

--
			Gleb.
Blue Swirl - Dec. 30, 2009, 12:01 p.m.
On Wed, Dec 30, 2009 at 11:50 AM, Gleb Natapov <gleb@redhat.com> wrote:
> Knowing ioapic configuration is very useful for the poor soles
> how need to debug guest occasionally.

> +static struct IOAPICState *ioapic;

Ugly. I really think the monitor interface needs to be changed to take
an opaque parameter.

The parameter can passed to the caller with for example a registration
function, like the patches I sent before QObject conversion.

> +void monitor_print_ioapic(Monitor *mon, const QObject *ret_data)

In this case the function would be
void monitor_print_ioapic(Monitor *mon, const QObject *ret_data, void *opaque)
where opaque replaces the static ioapic variable.

> +void do_info_ioapic(Monitor *mon, QObject **ret_data)

Same here.
Gleb Natapov - Dec. 30, 2009, 12:05 p.m.
On Wed, Dec 30, 2009 at 12:01:28PM +0000, Blue Swirl wrote:
> On Wed, Dec 30, 2009 at 11:50 AM, Gleb Natapov <gleb@redhat.com> wrote:
> > Knowing ioapic configuration is very useful for the poor soles
> > how need to debug guest occasionally.
> 
> > +static struct IOAPICState *ioapic;
> 
> Ugly. I really think the monitor interface needs to be changed to take
> an opaque parameter.
> 
Agree, but that's the reality now. Pic does the same. As long as there
is only one IOAPIC (and there is no indication that there will be more)
that is OK.

> The parameter can passed to the caller with for example a registration
> function, like the patches I sent before QObject conversion.
> 
> > +void monitor_print_ioapic(Monitor *mon, const QObject *ret_data)
> 
> In this case the function would be
> void monitor_print_ioapic(Monitor *mon, const QObject *ret_data, void *opaque)
> where opaque replaces the static ioapic variable.
> 
> > +void do_info_ioapic(Monitor *mon, QObject **ret_data)
> 
> Same here.
> 

--
			Gleb.
Izik Eidus - Dec. 30, 2009, 12:08 p.m.
On Wed, 30 Dec 2009 14:05:10 +0200
Gleb Natapov <gleb@redhat.com> wrote:

> On Wed, Dec 30, 2009 at 12:01:28PM +0000, Blue Swirl wrote:
> > On Wed, Dec 30, 2009 at 11:50 AM, Gleb Natapov <gleb@redhat.com> wrote:
> > > Knowing ioapic configuration is very useful for the poor soles
> > > how need to debug guest occasionally.
> > 
> > > +static struct IOAPICState *ioapic;
> > 
> > Ugly. 
> > 
> Agree

And now it seems like even you dont agree with me!!!
Gleb Natapov - Dec. 30, 2009, 12:13 p.m.
On Wed, Dec 30, 2009 at 02:08:27PM +0200, Izik Eidus wrote:
> On Wed, 30 Dec 2009 14:05:10 +0200
> Gleb Natapov <gleb@redhat.com> wrote:
> 
> > On Wed, Dec 30, 2009 at 12:01:28PM +0000, Blue Swirl wrote:
> > > On Wed, Dec 30, 2009 at 11:50 AM, Gleb Natapov <gleb@redhat.com> wrote:
> > > > Knowing ioapic configuration is very useful for the poor soles
> > > > how need to debug guest occasionally.
> > > 
> > > > +static struct IOAPICState *ioapic;
> > > 
> > > Ugly. 
> > > 
> > Agree
> 
> And now it seems like even you dont agree with me!!!
> 
I agree, but all other info functions in qemu do exactly same:
referencing global data. This is ugly _and_ this nothing to
do with my patch.

--
			Gleb.
Luiz Capitulino - Dec. 30, 2009, 1:19 p.m.
On Wed, 30 Dec 2009 13:50:43 +0200
Gleb Natapov <gleb@redhat.com> wrote:

> I am starring to learn this QObject kung-fu.

 Nice, you really got how to do it. Just two minor comments.

> One question:
> Why qlist_iter(..., func, ...) and not
>  FOREACH_QOBJ() {
>    do things
>  }

 Well, when I started working on the QObjects I was still getting
familiar with QEMU internals and just ignored the FOREACH_ loops.

 Later, I realized that they could be better but then we were
planning to have libqmp and now I'm wondering if it's ok to expose
data structure members to the public.

> diff --git a/hw/ioapic.c b/hw/ioapic.c
> index b0ad78f..efb9744 100644
> --- a/hw/ioapic.c
> +++ b/hw/ioapic.c
> @@ -24,6 +24,12 @@
>  #include "pc.h"
>  #include "qemu-timer.h"
>  #include "host-utils.h"
> +#include "monitor.h"
> +#include "qint.h"
> +#include "qlist.h"
> +#include "qdict.h"
> +#include "qstring.h"
> +#include "qjson.h"

 You can include qemu-objects.h.

> +void do_info_ioapic(Monitor *mon, QObject **ret_data)
> +{
> +    int i;
> +    QList *list;
> +
> +    *ret_data = NULL;
> +
> +    if (!ioapic)
> +        return;
> +
> +    list = qlist_new();
> +
> +    for (i = 0; i < IOAPIC_NUM_PINS; i++) {
> +        QObject *obj;
> +        uint64 e = ioapic->ioredtbl[i];
> +        if (e & IOAPIC_LVT_MASKED) {
> +            obj = qobject_from_jsonf("{'index': %d, 'masked': 1}", i);

 'masked' should be a bool, using %i will do it, like:


 obj = qobject_from_jsonf("{'index': %d, 'masked': %i}", i);
Gleb Natapov - Dec. 30, 2009, 1:22 p.m.
On Wed, Dec 30, 2009 at 11:19:30AM -0200, Luiz Capitulino wrote:
> On Wed, 30 Dec 2009 13:50:43 +0200
> Gleb Natapov <gleb@redhat.com> wrote:
> 
> > I am starring to learn this QObject kung-fu.
> 
>  Nice, you really got how to do it. Just two minor comments.
> 
> > One question:
> > Why qlist_iter(..., func, ...) and not
> >  FOREACH_QOBJ() {
> >    do things
> >  }
> 
>  Well, when I started working on the QObjects I was still getting
> familiar with QEMU internals and just ignored the FOREACH_ loops.
> 
>  Later, I realized that they could be better but then we were
> planning to have libqmp and now I'm wondering if it's ok to expose
> data structure members to the public.
> 
> > diff --git a/hw/ioapic.c b/hw/ioapic.c
> > index b0ad78f..efb9744 100644
> > --- a/hw/ioapic.c
> > +++ b/hw/ioapic.c
> > @@ -24,6 +24,12 @@
> >  #include "pc.h"
> >  #include "qemu-timer.h"
> >  #include "host-utils.h"
> > +#include "monitor.h"
> > +#include "qint.h"
> > +#include "qlist.h"
> > +#include "qdict.h"
> > +#include "qstring.h"
> > +#include "qjson.h"
> 
>  You can include qemu-objects.h.
> 
Hm. Copied all this from monitor.c

> > +void do_info_ioapic(Monitor *mon, QObject **ret_data)
> > +{
> > +    int i;
> > +    QList *list;
> > +
> > +    *ret_data = NULL;
> > +
> > +    if (!ioapic)
> > +        return;
> > +
> > +    list = qlist_new();
> > +
> > +    for (i = 0; i < IOAPIC_NUM_PINS; i++) {
> > +        QObject *obj;
> > +        uint64 e = ioapic->ioredtbl[i];
> > +        if (e & IOAPIC_LVT_MASKED) {
> > +            obj = qobject_from_jsonf("{'index': %d, 'masked': 1}", i);
> 
>  'masked' should be a bool, using %i will do it, like:
> 
> 
>  obj = qobject_from_jsonf("{'index': %d, 'masked': %i}", i);
No need to put anything here? ------------------------------^?
OR should it be
obj = qobject_from_jsonf("{'index': %d, 'masked': %i}", i, true);

--
			Gleb.
Luiz Capitulino - Dec. 30, 2009, 1:25 p.m.
On Wed, 30 Dec 2009 12:01:28 +0000
Blue Swirl <blauwirbel@gmail.com> wrote:

> On Wed, Dec 30, 2009 at 11:50 AM, Gleb Natapov <gleb@redhat.com> wrote:
> > Knowing ioapic configuration is very useful for the poor soles
> > how need to debug guest occasionally.
> 
> > +static struct IOAPICState *ioapic;
> 
> Ugly. I really think the monitor interface needs to be changed to take
> an opaque parameter.
> 
> The parameter can passed to the caller with for example a registration
> function, like the patches I sent before QObject conversion.

 Do you mind updating your series and sending it again?

 You could make the opaque parameter part of info_new or cmd_new, provided
the new handlers have global data so that we can see better the benefit
in practice.
Luiz Capitulino - Dec. 30, 2009, 1:50 p.m.
On Wed, 30 Dec 2009 15:22:19 +0200
Gleb Natapov <gleb@redhat.com> wrote:

> > > +void do_info_ioapic(Monitor *mon, QObject **ret_data)
> > > +{
> > > +    int i;
> > > +    QList *list;
> > > +
> > > +    *ret_data = NULL;
> > > +
> > > +    if (!ioapic)
> > > +        return;
> > > +
> > > +    list = qlist_new();
> > > +
> > > +    for (i = 0; i < IOAPIC_NUM_PINS; i++) {
> > > +        QObject *obj;
> > > +        uint64 e = ioapic->ioredtbl[i];
> > > +        if (e & IOAPIC_LVT_MASKED) {
> > > +            obj = qobject_from_jsonf("{'index': %d, 'masked': 1}", i);
> > 
> >  'masked' should be a bool, using %i will do it, like:
> > 
> > 
> >  obj = qobject_from_jsonf("{'index': %d, 'masked': %i}", i);
> No need to put anything here? ------------------------------^?
> OR should it be
> obj = qobject_from_jsonf("{'index': %d, 'masked': %i}", i, true);

 Oh yes, you right. Actually, you can do:

obj = qobject_from_jsonf("{'index': %d, 'masked': true }", i);

 I would also include this key in the unmasked dict (the one
you fill in the else clause), just for consistency in the protocol.

 And yes, having to worry about the protocol in the handler is a sign
that we have to improve.
Avi Kivity - Dec. 30, 2009, 3:04 p.m.
On 12/30/2009 01:50 PM, Gleb Natapov wrote:
> Knowing ioapic configuration is very useful for the poor soles
> how need to debug guest occasionally.
> +
> +void do_info_ioapic(Monitor *mon, QObject **ret_data)
> +{
> +    int i;
> +    QList *list;
> +
> +    *ret_data = NULL;
> +
> +    if (!ioapic)
> +        return;
> +
> +    list = qlist_new();
> +
> +    for (i = 0; i<  IOAPIC_NUM_PINS; i++) {
> +        QObject *obj;
>    


This assumes there is only one ioapic.  While I don't think there's a 
good reason to add more (the one we have is causing sufficient trouble), 
I suggest returning an array of ioapics (or include gsibase: [{ ioapic: 
object, gsibase: 0 }, { ioapic: object, gsibase: 24 }].
Gleb Natapov - Dec. 30, 2009, 3:53 p.m.
On Wed, Dec 30, 2009 at 05:04:08PM +0200, Avi Kivity wrote:
> On 12/30/2009 01:50 PM, Gleb Natapov wrote:
> >Knowing ioapic configuration is very useful for the poor soles
> >how need to debug guest occasionally.
> >+
> >+void do_info_ioapic(Monitor *mon, QObject **ret_data)
> >+{
> >+    int i;
> >+    QList *list;
> >+
> >+    *ret_data = NULL;
> >+
> >+    if (!ioapic)
> >+        return;
> >+
> >+    list = qlist_new();
> >+
> >+    for (i = 0; i<  IOAPIC_NUM_PINS; i++) {
> >+        QObject *obj;
> 
> 
> This assumes there is only one ioapic.  While I don't think there's
> a good reason to add more (the one we have is causing sufficient
> trouble), I suggest returning an array of ioapics (or include
> gsibase: [{ ioapic: object, gsibase: 0 }, { ioapic: object, gsibase:
> 24 }].
> 
For the case of multiple ioapics I thought about extending the command
to get ioapic id as a parameter when time comes. gsibase is not know to
ioapic itself, so this info does not belong here.

--
			Gleb.
Avi Kivity - Dec. 30, 2009, 3:58 p.m.
On 12/30/2009 05:53 PM, Gleb Natapov wrote:
>
>> This assumes there is only one ioapic.  While I don't think there's
>> a good reason to add more (the one we have is causing sufficient
>> trouble), I suggest returning an array of ioapics (or include
>> gsibase: [{ ioapic: object, gsibase: 0 }, { ioapic: object, gsibase:
>> 24 }].
>>
>>      
> For the case of multiple ioapics I thought about extending the command
> to get ioapic id as a parameter when time comes. gsibase is not know to
> ioapic itself, so this info does not belong here.
>
>    

Ok.
Anthony Liguori - Dec. 30, 2009, 11:04 p.m.
On 12/30/2009 09:58 AM, Avi Kivity wrote:
> On 12/30/2009 05:53 PM, Gleb Natapov wrote:
>>
>>> This assumes there is only one ioapic. While I don't think there's
>>> a good reason to add more (the one we have is causing sufficient
>>> trouble), I suggest returning an array of ioapics (or include
>>> gsibase: [{ ioapic: object, gsibase: 0 }, { ioapic: object, gsibase:
>>> 24 }].
>>>
>> For the case of multiple ioapics I thought about extending the command
>> to get ioapic id as a parameter when time comes. gsibase is not know to
>> ioapic itself, so this info does not belong here.
>>
>
> Ok.

Here's a good example of why attacking device-info is a better general 
approach.

Right now, you only support one ioapic and you only display a portion of 
the device's state.  In the future, it's likely that you'll want to 
support multiple ioapic (okay, maybe not likely, but possible), and it's 
very likely you'll want to include more state for the ioapic.

By definition, VMState contains all of the state of a device that's 
guest visible.  That means from a debugging perspective, you are 
guaranteed to have all of the information you need for the particular 
device.  Also, because it's necessarily to save multiple instances of a 
device with VMState, it automatically supports the ability to view 
multiple instances of a device.

Regards,

Anthony Liguori
Anthony Liguori - Dec. 30, 2009, 11:07 p.m.
On 12/30/2009 06:13 AM, Gleb Natapov wrote:
> On Wed, Dec 30, 2009 at 02:08:27PM +0200, Izik Eidus wrote:
>> On Wed, 30 Dec 2009 14:05:10 +0200
>> Gleb Natapov<gleb@redhat.com>  wrote:
>>
>>> On Wed, Dec 30, 2009 at 12:01:28PM +0000, Blue Swirl wrote:
>>>> On Wed, Dec 30, 2009 at 11:50 AM, Gleb Natapov<gleb@redhat.com>  wrote:
>>>>> Knowing ioapic configuration is very useful for the poor soles
>>>>> how need to debug guest occasionally.
>>>>
>>>>> +static struct IOAPICState *ioapic;
>>>>
>>>> Ugly.
>>>>
>>> Agree
>>
>> And now it seems like even you dont agree with me!!!
>>
> I agree, but all other info functions in qemu do exactly same:
> referencing global data. This is ugly _and_ this nothing to
> do with my patch.

This goes away if you use VMState since we already have a global table 
that contains all of the registered VMState instances.  It also maps 
device names/instance ids to the actual field contents.

Regards,

Anthony Liguori

> --
> 			Gleb.
>
>
Gleb Natapov - Dec. 31, 2009, 12:20 a.m.
On Wed, Dec 30, 2009 at 05:04:53PM -0600, Anthony Liguori wrote:
> On 12/30/2009 09:58 AM, Avi Kivity wrote:
> >On 12/30/2009 05:53 PM, Gleb Natapov wrote:
> >>
> >>>This assumes there is only one ioapic. While I don't think there's
> >>>a good reason to add more (the one we have is causing sufficient
> >>>trouble), I suggest returning an array of ioapics (or include
> >>>gsibase: [{ ioapic: object, gsibase: 0 }, { ioapic: object, gsibase:
> >>>24 }].
> >>>
> >>For the case of multiple ioapics I thought about extending the command
> >>to get ioapic id as a parameter when time comes. gsibase is not know to
> >>ioapic itself, so this info does not belong here.
> >>
> >
> >Ok.
> 
> Here's a good example of why attacking device-info is a better
> general approach.
> 
> Right now, you only support one ioapic and you only display a
> portion of the device's state.  In the future, it's likely that
> you'll want to support multiple ioapic (okay, maybe not likely, but
> possible), and it's very likely you'll want to include more state
> for the ioapic.
I included only the state I need for debugging. I don't what to see
complete state. It will just clatter important info.

> 
> By definition, VMState contains all of the state of a device that's
> guest visible.  That means from a debugging perspective, you are
> guaranteed to have all of the information you need for the
> particular device.  Also, because it's necessarily to save multiple
> instances of a device with VMState, it automatically supports the
> ability to view multiple instances of a device.
> 

--
			Gleb.
Gleb Natapov - Dec. 31, 2009, 12:23 a.m.
On Wed, Dec 30, 2009 at 05:07:14PM -0600, Anthony Liguori wrote:
> On 12/30/2009 06:13 AM, Gleb Natapov wrote:
> >On Wed, Dec 30, 2009 at 02:08:27PM +0200, Izik Eidus wrote:
> >>On Wed, 30 Dec 2009 14:05:10 +0200
> >>Gleb Natapov<gleb@redhat.com>  wrote:
> >>
> >>>On Wed, Dec 30, 2009 at 12:01:28PM +0000, Blue Swirl wrote:
> >>>>On Wed, Dec 30, 2009 at 11:50 AM, Gleb Natapov<gleb@redhat.com>  wrote:
> >>>>>Knowing ioapic configuration is very useful for the poor soles
> >>>>>how need to debug guest occasionally.
> >>>>
> >>>>>+static struct IOAPICState *ioapic;
> >>>>
> >>>>Ugly.
> >>>>
> >>>Agree
> >>
> >>And now it seems like even you dont agree with me!!!
> >>
> >I agree, but all other info functions in qemu do exactly same:
> >referencing global data. This is ugly _and_ this nothing to
> >do with my patch.
> 
> This goes away if you use VMState since we already have a global
> table that contains all of the registered VMState instances.  It
> also maps device names/instance ids to the actual field contents.
> 
I've got you point. When VMState will have this cool powers you are talking
about I'll use it. I don't see the point of bragging about something
that doesn't exists.

--
			Gleb.
Anthony Liguori - Dec. 31, 2009, 2:16 p.m.
On 12/30/2009 06:23 PM, Gleb Natapov wrote:
> I've got you point. When VMState will have this cool powers you are talking
> about I'll use it. I don't see the point of bragging about something
> that doesn't exists.

It *does* exist.  The ioapic has already been converted to VMState.

Regards,

Anthony Liguori

> --
> 			Gleb.
Anthony Liguori - Dec. 31, 2009, 2:20 p.m.
On 12/30/2009 06:20 PM, Gleb Natapov wrote:
> I included only the state I need for debugging. I don't what to see
> complete state. It will just clatter important info.

Someone may find that full state useful though.  There's no good reason 
to not show the entire device's state.

Regards,

Anthony Liguori
Gleb Natapov - Dec. 31, 2009, 3:30 p.m.
On Thu, Dec 31, 2009 at 08:16:27AM -0600, Anthony Liguori wrote:
> On 12/30/2009 06:23 PM, Gleb Natapov wrote:
> >I've got you point. When VMState will have this cool powers you are talking
> >about I'll use it. I don't see the point of bragging about something
> >that doesn't exists.
> 
> It *does* exist.  The ioapic has already been converted to VMState.
> 
It *does not*, VMstate has nothing to do with displaying device state as of today.
And implementing of general "device state printing facility" that dumps binary
data was possible event before VMstate: just call device's save
function with fake QEMUFile and dump it in binary. This haven't stopped
addition of info functions that understand device internals and print
relevant information in human readable form. This has nothing to do with
VMState.

--
			Gleb.
Gleb Natapov - Dec. 31, 2009, 3:42 p.m.
On Thu, Dec 31, 2009 at 08:20:06AM -0600, Anthony Liguori wrote:
> On 12/30/2009 06:20 PM, Gleb Natapov wrote:
> >I included only the state I need for debugging. I don't what to see
> >complete state. It will just clatter important info.
> 
> Someone may find that full state useful though.  There's no good
> reason to not show the entire device's state.
> 
There is no good reason to not show the entire device's state in
addition to nicely formated most useful part of it. Here I fixed it for
you.

--
			Gleb.
Anthony Liguori - Dec. 31, 2009, 7:05 p.m.
On 12/31/2009 09:42 AM, Gleb Natapov wrote:
> On Thu, Dec 31, 2009 at 08:20:06AM -0600, Anthony Liguori wrote:
>> On 12/30/2009 06:20 PM, Gleb Natapov wrote:
>>> I included only the state I need for debugging. I don't what to see
>>> complete state. It will just clatter important info.
>>
>> Someone may find that full state useful though.  There's no good
>> reason to not show the entire device's state.
>>
> There is no good reason to not show the entire device's state in
> addition to nicely formated most useful part of it. Here I fixed it for
> you.


Have you even attempted to look at what the generic implementation would 
be? ioapic has three vmstate fields.  If you decode ioredtbl into six 
subfields, then you're only adding two addition fields to be printed.

I can't believe that having those two extra fields is really going to 
make it any more difficult to debug.

And being able to dump any device state is certainly going to be useful 
in the future.

Regards,

Anthony Liguori

> --
> 			Gleb.
Gleb Natapov - Jan. 1, 2010, 10:27 a.m.
On Thu, Dec 31, 2009 at 01:05:18PM -0600, Anthony Liguori wrote:
> On 12/31/2009 09:42 AM, Gleb Natapov wrote:
> >On Thu, Dec 31, 2009 at 08:20:06AM -0600, Anthony Liguori wrote:
> >>On 12/30/2009 06:20 PM, Gleb Natapov wrote:
> >>>I included only the state I need for debugging. I don't what to see
> >>>complete state. It will just clatter important info.
> >>
> >>Someone may find that full state useful though.  There's no good
> >>reason to not show the entire device's state.
> >>
> >There is no good reason to not show the entire device's state in
> >addition to nicely formated most useful part of it. Here I fixed it for
> >you.
> 
> 
> Have you even attempted to look at what the generic implementation
> would be? ioapic has three vmstate fields.  If you decode ioredtbl
> into six subfields, then you're only adding two addition fields to
> be printed.
> 
I you seriously suggesting that we should tie the way device state is
migrated to the way device information is printed? 

> I can't believe that having those two extra fields is really going
> to make it any more difficult to debug.
> 
Can't parse that.

> And being able to dump any device state is certainly going to be
> useful in the future.
> 
No arguing with that.

--
			Gleb.

Patch

diff --git a/hw/ioapic.c b/hw/ioapic.c
index b0ad78f..efb9744 100644
--- a/hw/ioapic.c
+++ b/hw/ioapic.c
@@ -24,6 +24,12 @@ 
 #include "pc.h"
 #include "qemu-timer.h"
 #include "host-utils.h"
+#include "monitor.h"
+#include "qint.h"
+#include "qlist.h"
+#include "qdict.h"
+#include "qstring.h"
+#include "qjson.h"
 
 //#define DEBUG_IOAPIC
 
@@ -50,6 +56,8 @@  struct IOAPICState {
     uint64_t ioredtbl[IOAPIC_NUM_PINS];
 };
 
+static struct IOAPICState *ioapic;
+
 static void ioapic_service(IOAPICState *s)
 {
     uint8_t i;
@@ -232,7 +240,7 @@  qemu_irq *ioapic_init(void)
     qemu_irq *irq;
     int io_memory;
 
-    s = qemu_mallocz(sizeof(IOAPICState));
+    ioapic = s = qemu_mallocz(sizeof(IOAPICState));
     ioapic_reset(s);
 
     io_memory = cpu_register_io_memory(ioapic_mem_read,
@@ -245,3 +253,70 @@  qemu_irq *ioapic_init(void)
 
     return irq;
 }
+
+static void qemu_ioapic_qlist_iter(QObject *data, void *opaque)
+{
+    QDict *qdict = qobject_to_qdict(data);
+    Monitor *mon = opaque;
+
+    monitor_printf(mon, "%2"PRId64": ", qdict_get_int(qdict, "index"));
+    if (qdict_haskey(qdict, "masked")) {
+        monitor_printf(mon, "masked\n");
+    } else {
+        monitor_printf(mon, "vec=%3"PRId64" %s %s acive-%s %s dest=%"PRId64"\n",
+                       qdict_get_int(qdict, "vector"),
+                       qdict_get_str(qdict, "delivery_mode"),
+                       qdict_get_str(qdict, "dest_mode"),
+                       qdict_get_str(qdict, "polarity"),
+                       qdict_get_str(qdict, "trig_mode"),
+                       qdict_get_int(qdict, "destination"));
+    }
+}
+
+void monitor_print_ioapic(Monitor *mon, const QObject *ret_data)
+{
+    qlist_iter(qobject_to_qlist(ret_data), qemu_ioapic_qlist_iter, mon);
+}
+
+static const char *delivery_mode_string[] = {"fixed", "lowprio", "smi", "res",
+                                             "nmi", "init", "res", "extint"};
+
+void do_info_ioapic(Monitor *mon, QObject **ret_data)
+{
+    int i;
+    QList *list;
+
+    *ret_data = NULL;
+
+    if (!ioapic)
+        return;
+
+    list = qlist_new();
+
+    for (i = 0; i < IOAPIC_NUM_PINS; i++) {
+        QObject *obj;
+        uint64 e = ioapic->ioredtbl[i];
+        if (e & IOAPIC_LVT_MASKED) {
+            obj = qobject_from_jsonf("{'index': %d, 'masked': 1}", i);
+        } else {
+            uint8_t vec = e & 0xff;
+            uint8_t trig_mode = ((e >> 15) & 1);
+            uint8_t dest = e >> 56;
+            uint8_t dest_mode = (e >> 11) & 1;
+            uint8_t delivery_mode = (e >> 8) & 7;
+            uint8_t polarity = (e >> 13) & 1;
+            obj = qobject_from_jsonf("{'index': %d, 'vector': %d,"
+                                     "'delivery_mode': %s, 'dest_mode': %s,"
+                                     "'polarity': %s, 'trig_mode': %s,"
+                                     "'destination': %d}",
+                                     i, vec,
+                                     delivery_mode_string[delivery_mode],
+                                     dest_mode ? "logical":"physical",
+                                     polarity ? "low" : "high",
+                                     trig_mode ? "level": "edge",
+                                     dest);
+        }
+        qlist_append_obj(list, obj);
+    }
+    *ret_data = QOBJECT(list);
+}
diff --git a/hw/pc.h b/hw/pc.h
index 03ffc91..3e39444 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -2,6 +2,7 @@ 
 #define HW_PC_H
 
 #include "qemu-common.h"
+#include "qobject.h"
 
 /* PC-style peripherals (also used by other machines).  */
 
@@ -45,6 +46,8 @@  int apic_accept_pic_intr(CPUState *env);
 void apic_deliver_pic_intr(CPUState *env, int level);
 int apic_get_interrupt(CPUState *env);
 qemu_irq *ioapic_init(void);
+void do_info_ioapic(Monitor *mon, QObject **ret_data);
+void monitor_print_ioapic(Monitor *mon, const QObject *data);
 void ioapic_set_irq(void *opaque, int vector, int level);
 void apic_reset_irq_delivered(void);
 int apic_get_irq_delivered(void);
diff --git a/monitor.c b/monitor.c
index c0dc48e..367e330 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2625,6 +2625,14 @@  static const mon_cmd_t info_cmds[] = {
         .mhandler.info = do_info_roms,
     },
     {
+        .name       = "ioapic",
+        .args_type  = "",
+        .params     = "",
+        .help       = "show ioapic config",
+        .user_print = monitor_print_ioapic,
+        .mhandler.info_new = do_info_ioapic,
+    },
+    {
         .name       = NULL,
     },
 };