Sorry for the late response. Marcelo Tosatti wrote on 2013-02-04: > On Thu, Jan 31, 2013 at 03:55:56PM +0200, Gleb Natapov wrote: >> On Thu, Jan 31, 2013 at 11:44:43AM -0200, Marcelo Tosatti wrote: >>> On Thu, Jan 31, 2013 at 03:38:37PM +0200, Gleb Natapov wrote: >>>> On Thu, Jan 31, 2013 at 11:32:45AM -0200, Marcelo Tosatti wrote: >>>>> On Thu, Jan 31, 2013 at 11:43:48AM +0200, Gleb Natapov wrote: >>>>>> On Wed, Jan 30, 2013 at 09:03:11PM -0200, Marcelo Tosatti wrote: >>>>>>> Posted interrupt patch: >>>>>>> 2) Must move IN_GUEST_MODE assignment after local_irq_disable, in >>>>>>> vcpu_enter_guest function. Otherwise: >>>>>>> >>>>>>> cpu0 vcpu1<->cpu1 >>>>>>> >>>>>>> vcpu->mode = IN_GUEST_MODE >>>>>>> >>>>>>> if IN_GUEST_MODE == true >>>>>>> send IPI >>>>>>> local_irq_disable >>>>>>> >>>>>>> PIR not transferred to VIRR, misses interrupt. >>>>>>> >>>>>> cpu0 will set KVM_REQ_EVENT, so vmentry will be aborted after >>>>>> local_irq_disable() by ->requests check. >>>>> >>>>> Yes, but you don't want KVM_REQ_EVENT+kick. It defeats the purpose >>>>> of posted interrupts. You want >>>>> >>>>> if vcpu in guest mode >>>>> send posted interrupt IPI >>>>> else >>>>> KVM_REQ_EVENT+kick >>>>> >>>> I am thinking: >>>> >>>> set KVM_REQ_EVENT >>>> if pi is enabled >>>> send posted interrupt IPI else kick >>> >>> KVM_REQ_EVENT must be after sending posted interrupt IPI. Otherwise on >>> the vcpu entry side >>> >>> test_and_clear(KVM_REQ_EVENT) { >>> No bits set in PIR >>> } >> It should be after updating PIR, but before sending posted interrupt >> IPI. Otherwise: >> >> cpu0 cpu1/vcpu >> KVM_REQ_EVENT is not set >> set pir >> send IPI >> irq_disable() >> ->request is empty. >> set KVM_REQ_EVENT >> >> That's the same sequence as with IRR update, KVM_REQ_EVENT and kick >> today. > > Can only send IPI if vcpu->mode == IN_GUEST_MODE, which must be set > after interrupt flag is cleared as noted. > > Also KVM_REQ_EVENT is processed outside of interrupt disabled region today. > > Or maybe i don't get what you say... write a complete description? This version patch is too old. And I already noticed this problem and the current solution is like this: +static bool vmx_send_notification_event(struct kvm_vcpu *vcpu, + int vector, int *result) +{ + struct vcpu_vmx *vmx = to_vmx(vcpu); + + *result = !pi_test_and_set_pir(vector, vmx->pi); + if (!pi_test_and_set_on(vmx->pi) && (vcpu->mode == IN_GUEST_MODE)) { + kvm_make_request(KVM_REQ_PENDING_PIR, vcpu); + apic->send_IPI_mask(get_cpu_mask(vcpu->cpu), + POSTED_INTR_VECTOR); + if (!pi_test_on(vmx->pi)) + clear_bit(KVM_REQ_PENDING_PIR, &vcpu->requests) ; + return true; + } + return false; +}
For detail, please check the v2 patches. >>> What about item 4 below? >>> >> That's for Yang to answer :) > > "If more than one interrupt is generated with the same vector number, > the local APIC can set the bit for the vector both in the IRR and ISR. > This means that for the Pentium 4 and Intel Xeon processors, the IRR > and ISR can queue two interrupts for each interrupt vector: one in the > IRR and one in the ISR. Any additional interrupts issued for the same > interrupt vector are collapsed into the single bit in IRR" > > Which would mean KVM emulation differs... "Eventually 3 interrupts can > be queued: one in IRR, one in ISR, one in PIR". > > Any example how software relies on such two-interrupts-queued-in-IRR/ISR > behaviour? Before PI, the interrupt handle logic is IRR->ISR. And now it is : PIR->IRR->ISR. So, if we can ensure PIR sync to IRR before each access to IRR, I think there should no problem. Best regards, Yang -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html