[Qemu-devel] Re: [PATCH] add "info ioapic" monitor command

[Luiz Capitulino]

On Tue, 29 Dec 2009 20:16:10 +0200
"Michael S. Tsirkin" <address@hidden> wrote:

> On Tue, Dec 29, 2009 at 03:39:29PM -0200, Luiz Capitulino wrote:
> > On Tue, 29 Dec 2009 18:53:44 +0200
> > Gleb Natapov <address@hidden> wrote:
> > 
> > > On Tue, Dec 29, 2009 at 02:37:20PM -0200, Luiz Capitulino wrote:
> > > > On Thu, 24 Dec 2009 17:02:42 +0200
> > > > Gleb Natapov <address@hidden> wrote:
> > > > 
> > > > > +
> > > > > +static const char *delivery_mode_string[] = {"fixed", "lowprio", 
> > > > > "smi", "res",
> > > > > +                                             "nmi", "init", "res", 
> > > > > "extint"};
> > > > > +
> > > > > +void do_info_ioapic(Monitor *mon)
> > > > > +{
> > > > > +    int i;
> > > > > +
> > > > > +    if (!ioapic)
> > > > > +        return;
> > > > > +    for (i = 0; i < IOAPIC_NUM_PINS; i++) {
> > > > > +        uint64 e = ioapic->ioredtbl[i];
> > > > > +        monitor_printf(mon, "%2d: ", i);
> > > > > +        if (e & IOAPIC_LVT_MASKED) {
> > > > > +            monitor_printf(mon, "masked\n");
> > > > > +        } 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;
> > > > > +            monitor_printf(mon, "vec=%3d %s %s acive-%s %s 
> > > > > dest=%d\n",
> > > > > +                           vec,
> > > > > +                           delivery_mode_string[delivery_mode],
> > > > > +                           dest_mode ? "logical":"physical",
> > > > > +                           polarity ? "low" : "high",
> > > > > +                           trig_mode ? "level": "edge",
> > > > > +                           dest);
> > > > > +        }
> > > > > +    }
> > > > > +}
> > > > 
> > > >  New Monitor handlers should return QObjects, monitor_printf()
> > > > can only be used in the print handler.
> > > > 
> > > So I want to add only print handler. This is debug info only, no
> > > management should ever care about it.
> > 
> >  Well, this is possible (at least for now) but there are plans to
> > have an external shell.
> > 
> >  Also, I don't want people to avoid writing handlers because
> > they feel it's difficult.
> > 
> > > >  You can take a look at qemu_chr_info() for an example, as it does
> > > > what I think you should do here: build a qdict for each pin and
> > > > return a qlist or return another qdict if it makes sense (or will
> > > > make in the future) to have more the one ioapic.
> > > I don't understand a single word from what you are saying :(, but
> > > qemu_chr_info() looks scary. Actually I don't understand what it does at
> > > all.
> > 
> >  Ouch. :((
> > 
> > > >  I'm still thinking in ways to make the work of writing the new
> > > > Monitor handlers easier..
> > > Something more simple is definitely needed. If I need to pars the data
> > > structure to some intermediate language and then parse it back again just 
> > > to add
> > > debug command I will not event consider adding it. One more tracepoint
> > > in the kernel will take 10min to add and will accomplish the same goal
> > > to me albeit in less elegant way. 
> > 
> >  Actually, you're building objects and iterating over them to get them
> > printed, it's not parsing and it's common to do that in high level
> > languages.
> > 
> >  But I agree that it's not as easy as calling monitor_printf().
> > 
> >  Something that is on my TODO list is to add a default printing
> > function. This will make things easier a bit, but you'll have to
> > build the objects and the user output won't be pretty.
> > 
> >  We could consider allowing monitor_printf() in certain handlers,
> > but as I said above this has a number of problems and doing this
> > because writing handlers became harder only hides the real
> > problem (which will come up again, soon or later).
> > 
> >  So, we'll have to wait for people to come back from vacations
> > to discuss this issue.
> 
> Maybe we need a "debug" monitor command.
> That would just print a string, no objects,
> and no guarantees for management about format.

 I don't think a single string will make it, Gleb's new command
is an example of that.