>> If you really think supporting mixed irqchip mode is a must > > I will leave it to Avi to decide since he implicitly suggested it. > But suffice to say, if we *dont* want to support it we will need to > get the other in-kernel stuff into the lapic branch in its entirety > before it can be merged so we dont create an ABI issue. >
Agree, anyway the device model side code (majority for both PIC patch and APIC patch) can be reused. The only debate is the I/F issue. > >> , 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. > > This is where we disagree, as I know you have mentioned this before. > The solution I am proposing (which reverts the userspace injection > method) would not require this ACK that you mention. The > reason is that > the synchronous nature of the injection takes care of things > naturally. Here is how: > > With sync-injection, we can only move a vector from the IRR->ISR if > RFLAGS.IF and !irq.pending. Once we inject, the vector will > move to ISR > and get queued in kernel (thereby setting an irq.pending bit). It may That means IRR->ISR happen, right? If yes, the question I raised is still valid, if not when will be moved? > not inject to the guest immediately, but thats ok because no > new vectors > will be acked in the PIC until that previous vector is processed That violates the IRQ priority. When the previous irq is still queued, a new irq may be generated which may have higher priority than the previous one. > (because irq.pending will remain set). Therefore, we will never > prematurely move IRR->ISR until the previous vector really is > "in-service". Does this make sense? > > My suggestion assumes that a single vector can stay in ISR without > actually being in service without ill effect (Todays v9 code could > presumably have multiple vectors in ISR without being in service, > which I admit is bogus). If this assumption is false, then I agree > with you that we need extra intack I/F. What are you thoughts here? > Yes, only one but this violates irq priority and my concern is still valid if IRR->ISR happened in previous statement. > >>> >>> 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. > > I suppose, but it somewhat defeats the purpose IMO. Every pin in the > 8259 that gets tickled implicitly means an IOAPIC pin was tickled > also. Do we really want to go to userspace for that? Essentially User space can handle this, go to IOAPIC first and then decide to go to in kernel PIC or not. Here the assumption is that an irq line is either serviced by PIC or IOAPIC, it will never be serviced by both. So no back to user space. > what happens > then is the PIT ends up having to go through userspace for > every tick at After PIT is moved to kernel, no this issue. For now, PIT is in user level. > that point. On the flip side, the IOAPIC model is very simple so I > think it makes more sense to move it one to one with the PIC. I agree too, either are OK and just depend on decision. PIC device model is about 400-500 lines of code, IOAPIC should be less than that, APIC is about 800-900 lines of code in Xen but 1500-1800 lines in V09 patch. size of irq abstraction layer could be sperated from above device model for now. 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/ _______________________________________________ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
