Re: In kernel PIC support: kernel patch

Gregory Haskins wrote:

On Wed, 2007-06-20 at 15:43 +0800, Dong, Eddie wrote:

quoted

As we discussed, if we move APIC to kernel while leaving PIC in user
level, we have ABI level holes if the interrupt a user level qemu
injected is not immediately injected to guest for some reason. \

Hi Eddie,

I know you worked hard on this, but I still do not agree with this
statement.  I agree that v9 introduced an issue related to a)
premature IRR->ISR assignment, and b) lack of support for clearing
pending ISR with IMR changes (and thank you for pointing it out!). 

Achitectually not only that. A premature IRR->ISR will cause guest OS
confuse in many place. A guest (say BIOS) may turn from interrupt
enabled mode to polling mode which polls IRR to detect if 
there is pending IRQ to handle. In this case we have trouble.

However, as I mentioned we can fix that issue with just a few new
lines of 
code and by
reverting the userspace injection model to the one used in prior
releases without having to implement an entire in-kernel PIC to do it.

In-kernel PIC gives us a full in kernel irqchip solution and performance
gain. Some OSes may use PIC only.

I think moving the PIC into the kernel has the advantage of allowing
us to move the PIT into the kernel too (which is huge, IMHO), but

Not very big, I just want to do one by one. we have done the code
in Xen already.

that is its sole advantage.  Don't get me wrong: I am in favor of
doing it. However, I wanted to point out that this particular
solution to the problem you found essentially is invalidating
"level-1" mode by only supporting level-0 or level-2, not fixing it. 

Not exactly understand level-0/1/2 meaning? Do u mean we mixed 
irqchip mode is a feature requirement? I didn't see the necessity, 
maybe I am wrong. But isn't both PIC and APIC in kernel much
nature and simple for us?

If you really think supporting mixed irqchip mode is a must, we 
need to introduce an intack I/F to notify user level irqchip if the irq 
is really ACKed or not. We can do that but I doubt the necessity.

Perhaps we are "ok" with that, but I was under the distinct
impression from Avi et. al. that these variable levels of support
were a design goal for debugging purposes. 

I would prefer that we just fix level-1 mode with the changes I
mentioned instead of just disabling it (in addition to adding level-2
mode as Eddie is working on). 

quoted

This patch fixes this by introducing new VM level IOCTL
KVM_SET_ISA_IRQ_LEVEL & KVM_CREATE_PIC to avoid the ABI hole.  The
original KVM_INTERRUPT is still there to be backward compatible.

WIth this patch, old Qemu, new qemu (after this patch), old kvm, new
kvm can work in any combination. Both user level code and kernel
code will automatically decide the irq source base on irqchip
location (user or kernel). 

Some known issues (TODO):
	1: SVM support is on going.
	The code for VMX to inject irq is same with Xen now since the
situation is same now (irqchip in hyprevisor), SVM code should be
able to directly reuse from Xen too.
	2: live migration break.
	3: Temply APIC support is removed in CPUID to wait for the merge
of in kernel APIC patch

Note that you will need more than just the APIC patch.  My patch only
moves the LAPIC down.  The IOAPIC and 8259s are still in userspace.
Your patch only moves the 8259s down.  This means there is a gap where
the IOAPIC used to be that still needs to be addressed.

Why do u think I/O APIC must be moved down too? A new IOCTL like
KVM_IOAPIC_MESSAGE can solve the interface issue IMO and quit 
natual.

Thx,
Eddie

-------------------------------------------------------------------------
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/