Marcelo Tosatti wrote on 2012-12-21: > On Thu, Dec 20, 2012 at 03:12:32PM +0200, Gleb Natapov wrote: >> On Thu, Dec 20, 2012 at 10:53:16AM -0200, Marcelo Tosatti wrote: >>> On Thu, Dec 20, 2012 at 08:42:06AM +0200, Gleb Natapov wrote: >>>> On Wed, Dec 19, 2012 at 10:59:36PM -0200, Marcelo Tosatti wrote: >>>>> On Mon, Dec 17, 2012 at 01:30:49PM +0800, Yang Zhang wrote: >>>>>> From: Yang Zhang <yang.z.zhang@xxxxxxxxx> >>>>>> >>>>>> Virtual interrupt delivery avoids KVM to inject vAPIC interrupts >>>>>> manually, which is fully taken care of by the hardware. This needs >>>>>> some special awareness into existing interrupr injection path: >>>>>> >>>>>> - for pending interrupt, instead of direct injection, we may need >>>>>> update architecture specific indicators before resuming to guest. >>>>>> - A pending interrupt, which is masked by ISR, should be also >>>>>> considered in above update action, since hardware will decide >>>>>> when to inject it at right time. Current has_interrupt and >>>>>> get_interrupt only returns a valid vector from injection p.o.v. >>>>>> Signed-off-by: Kevin Tian <kevin.tian@xxxxxxxxx> >>>>>> Signed-off-by: Yang Zhang <yang.z.zhang@xxxxxxxxx> >>>>> >>>>> >>>>> Resuming previous discussion: >>>>> >>>>>>> How about to recaculate irr_pending according the VIRR on each >>>>>>> vmexit? >>>>>>> >>>>>> No need really. Since HW can only clear VIRR the only situation that >>>>>> may >>>>>> happen is that irr_pending will be true but VIRR is empty and >>>>>> apic_find_highest_irr() will return correct result in this case. >>>>> >>>>> Self-IPI does cause VIRR to be set, see "29.1.5 Self-IPI >>>>> Virtualization". >>>>> >>>> True. But as I said later in that discussion once irr_pending is set >>>> to true it never becomes false, so the optimization is effectively >>>> disable. We can set it to true doing apic initialization to make it >>>> explicit. >>> >>> Its just confusing, to have a variable which has different meanings >>> in different configurations. I would rather have it explicit that >>> its not used rather than check every time the i read the code. >>> >>> if (apic_vid() == 0 && !apic->irr_pending) >>> return -1; >>> >> I'd prefer to avoid this additional if() especially as its sole purpose >> is documentation. We can add comment instead. Note that irr_pending >> is just a hint anyway. It can be true when no interrupt is pending in >> irr. We can even rename it to irr_pending_hint or something. > > Works for me (documentation). > >>> Not sure if you can skip it, its probably necessary to calculate it >>> before HW does so (say migration etc). >> kvm_apic_has_interrupt() is not called during migration and >> kvm_apic_post_state_restore() calls apic_update_ppr() explicitly. >> I am not sure it is needed though since migrated value should be already >> correct anyway. > > Ok, best force isr_count to 1 if apic vintr enabled (and add a comment, > please). Sorry for the later reply. I will address those problems according your comments. Best regards, Yang -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
