Re: [PATCH 0/8] in-kernel APIC support "v1"

[Avi Kivity <avi-atKUWr5tajBWk0Htik3J/w@public.gmane.org>]

Gregory Haskins wrote:
> Here is my latest series incorporating the feedback and numerous bugfixes.  I
> did not keep an official change-log, so its difficult to say what changed off
> the top of my head without an interdiff.  I will keep a changelog from here on
> out.  Lets call this drop officially "v1".  I will start tracking versions of
> the drop so its easier to refer to them in review notes, etc.
>
> Here are a few notes:
>
> A) I implemented Avi's idea for a fd-based signaling mechanism.  I didnt quite
> get what he meant by "writable-fd".  The way I saw it, it should be readable
> so that is how I implemented it.  If that is not satisfactory, please
> elaborate on the writable idea and I will change it over.
>   

I think that it should be writable, as the vcpu wants interrupts to be 
pushed into it (a write op) rather than it indicates it wants data to be 
pulled out of it.

> B) I changed the controversial kvm_irqdevice_ack() mechanism to use an "out"
> structure, instead of an int pointer + return bitmap.  Hopefully, this design
> puts Avi's mind at ease as the return code is more standard now.  In addition,
> this API makes it easier to extend, which I take advantage of later in the
> series for the TPR-shadow stuff.
>   

It should certainly be cleaner code-wise.   It's probably still complex 
in terms of the number of cases it generates, but as I said before, I 
have no better idea to offer.

> C) I changed the irq.task assignment from a lock to a barrier, per review
> comments.  However, I left the irq.guestmode = 0 assignment in a lock because
> I believe it is actually required to eliminate a race.  E.g. We want to make
> sure that the irq.pending and IPI-method are decided atomically and the
> irq.guest-mode is essentially identifiying a critical section.  I could be
> convinced otherwise, but for now its still there.
>   

Ok.

> D) Patch #8 is for demonstration purposes only.  Dont apply it (yet) as it
> causes the system to error on VMENTRY.  I include it purely so its clear where
> I am going.
>   

It's certainly the right direction.  But we don't support Windows x84 
yet, which would be the primary (only?) beneficiary?

> Overall, this code (excluding patch #8) seems to be working quite well from a
> pure functional standpoint.  One problem that I see is QEMU remains pretty
> busy even when the guest is idle.  I have a feeling it has something to do
> with the way signals are delivered...TBD.  Otherwise, its working from my
> perspective.  I would love to hear feedback from testers.
>   

Is this on all guests?  Windows+APIC only?  What exactly do you mean by 
high?

An oprofile run should clear the mystery.

> An interesting discovery on my part while working on this is that there is an
> aparent mis-emulation in the QEMU LAPIC code.  The kernel that ships as the
> SLED-10 installer (2.6.16.21, I think) maps LINT0 as an NMI and masks off all
> interrupts in the 8259 except the PIT.  It also leaves the PIT input on the
> IOAPIC active. 
>
> This means that every timer tick gets delivered both as a FIXED vector from
> the IOAPIC, and as an NMI.  As far as I can tell from reading google, this is
> what linux intended.  Note, however, that under QEMU LAPIC, LINT0 is dropped
> if the vector is not EXTINT whereas the in-kernel APIC emulates both.
> Therefore, cat'ing /proc/interrupts under stock KVM shows only IRQ: 0, and LOC
> incrementing, with NMI at 0.  The in-kernel patches show NMIs also
> incrementing. 
>
> I could generate a patch to fix the QEMU code, but what I am not sure of is
> whether this was intentionally coded to ignore the LINT0 NMI programming? 
>
>   

See 654501f79be082925c623806c00a27021565035f in kvm-userspace.git.  I 
confess I didn't delve deeply into it, but it seems related.

I wouldn't be surprised if indeed there's a qemu bug in this area.

-- 
error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/