Hi Eddie,
  I think you are missing a key point here:  irq.deferred is emulating the 
atomic nature of an INTA (*) cycle.  Once an interrupt is acknowledged, it is 
atomically dispatched to the IDT (at least, this is my assumption..you intel 
guys can set me straight if this is not true).  There is no way to put a vector 
that was once obtained in the INTA "back".  However, on a vCPU there is the 
possibility for certain events to interrupt an injection operation.  For 
example: if an external interrupt causes a VMEXIT before the VMENTER/injection 
completes.  There is no equivalent of this on a pCPU.  Therefore, we emulate an 
"in core" queuing mechanism (irq.deferred) to hold vectors that have been 
acked, but not yet injected.  We can only hold one.  

Now here is the important part: When an interrupt is queued here, it is the 
highest priority interrupt in the system.  We dont care about re-evaluating 
ISR, TPR, PPR, or anything else for this matter.  Its sole purpose to to 
complete the atomic INTA->IDT, and therefore we want to inject this deferred 
vector next regardless of the current state of PPR, etc.

So no, during injection failure we would never go back to QEMU/8259, APIC/ISR 
etc.  The vector (regardless of whether it came from the APIC or the PIC) is 
queued in the irq.deferred "in core" holding pattern until the next window.  
This is why I say that both APIC and EXTINT is covered here.

As I mentioned, there is a hole in my implementation related to allowing 
NMI/EXTINT interrupts to prioritize themselves higher than a queued deferred 
interrupt.  I will fix this.  However, this is not the same issue that you are 
talking about.  Does this make sense?

Regards,
-Greg

PS: I am on vacation this week, so I likely wont be able to answer more 
questions until next Monday.

(*) Note that I model both i8259s and LAPICs as having an INTA cycle, even 
though we software guys only explicitly see the INTA cycle on the 8259. But im 
sure there is something that closely resembles an INTA to the LAPIC buried 
inside the silicon so I think this analogy works.

>>> "Dong, Eddie" <[EMAIL PROTECTED]> 06/04/07 8:21 PM >>>
 

>-----Original Message-----
>From: Gregory Haskins [mailto:[EMAIL PROTECTED] 
>Sent: 2007年6月5日 1:23
>To: Dong, Eddie; kvm-devel@lists.sourceforge.net
>Subject: RE: [kvm-devel] [PATCH 3/9] KVM: Add irqdevice object
>
>>>> "Dong, Eddie" <[EMAIL PROTECTED]> 06/04/07 3:13 AM >>>
>>Greg:
>>      Can you explain a bit why we need to distinguish
>> kvm_irqpin_extint from kvm_irqpin_localint?
>
>They are both "vectored interrupts", but localint means "I 
>have the vector" and extint means "someone else has the 
>vector".  They are closely analogous to the LAPICs notion of 
>FIXED and EXTINT interrupts respectfully.  The difference is I 
>named them more generally so that it would apply equally to 
>PICs of other types besides APICs (for abstraction purposes).
>
>> I saw latest V09 has hole here such as handle_exception due to
>> IRQ inject fail. I.e. IDT_VECTORING_INFO_FIELD only push 
>localint back,
>> but no extint. 
>
>Actually, I don't think this is really a hole.  Both 
>interrupts are of the same injection-class and both can be 
>equally deferred.  As explained earlier, the only difference 
>between them is the source of the vectoring data.  If an 
>injection operation fails (regardless of original pin type), 
>we push the vector into our irq.deferred queue and use it next time.

Then you will push the extint back to Qemu ? BTW, PIC is not in 
yet, and I see the patch didn't do push for extirq.

>
>One problem that I *can* see is related to inadvertent 
>reprioritization based on how irq.deferred gets grossly 
>classified as a localint.  The deferred vector should be the 
>highest priority interrupt in the system, since it technically 
>was already dispatched.  However, today (in v9 and earlier) 
>new extint or NMIs would be incorrectly prioritized over 
>irq.deferred.  I doubt this would happen very often (if ever) 
>and I doubt it would cause to many p>However, the fix should be pretty easy: 
>there should probably 
>be something like an irqpin_deferred as the highest priority 
>pin in the system instead of overloading deferred on localint.

This is what I say. Push back an irq cause not only priority issue, 
but actually this irq is already blocked since ISR and TPR are all
 updated now. A higher irq definitely will happen too like you observed too.

WHat I am proposing is to split IDT_VECTORING from APIC logic,do it in irq
injecting logic.


>
>
>> In Xen, when we implementing APIC patch, we simplify it by
>>limiting a single IRQ can either be passed by PIC or APIC, we never
>>handle the combination of both PIC and APIC servicing them in 
>same time.
>>I.e. If APIC is taking the IRQ, PIC must already mask the pin. That
>>works fine so far.  I am just wondering if we should start from simple
>>model and improve later if we find problem.  Same for NMI/SMI 
>handling.
>
>With all due respect, I don't know if I would agree that that 
>approach is either simpler or superior.  Both Xen and QEMU 
>play games with the model w.r.t. APIC vs PIC interactions, and 
>to date, I don't think either one has got it quite right.  I 
>think they both got it "close enough" to work to some degree 
>(partially by luck), but its also (IMHO) a hacky way to do it. 
> What I tried to do was stay truer to the logical model of the 
>ISA architecture as presented by Bochs/QEMU.  So, yes, its 
>true that typically a guest would never enable both interrupts 
>in the PIC and the APIC.  However, if they *did*, my model 
>would work identically to the way real hardware would work.  
>The others would mis-emulate this.

If a guest OS *did* enable both PIC & APIC for a irq, both in the simplified 
approach (Xen) and V09 has same effect: exitirq get the priority and get 
injected. (BTW SDM doesn't explicitly say which one has priority). 

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

Reply via email to