Re: In kernel PIC support: kernel patch

Gregory Haskins wrote:
> On Wed, 2007-06-20 at 15:43 +0800, Dong, Eddie wrote:
>> 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). 
> 
>> 
>> 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/