Messages in this thread | | | Subject | Re: [PART1 RFC 5/9] svm: Add VMEXIT handlers for AVIC | From | Suravee Suthikulpanit <> | Date | Fri, 19 Feb 2016 18:32:54 +0700 |
| |
Hi,
On 2/18/16 21:18, Radim Krčmář wrote: > 2016-02-18 09:25+0700, Suravee Suthikulpanit: >> On 2/17/16 01:06, Radim Krčmář wrote: >>> 2016-02-16 17:56+0100, Paolo Bonzini: >>>>> On 16/02/2016 15:13, Radim Krčmář wrote: >>>>>>> Yeah, I think atomic there means that it won't race with other writes to >>>>>>> the same byte in IRR. We're fine as long as AVIC writes IRR before >>>>>>> checking IsRunning on every destination, which it seems to be. >>>>> >>>>> More precisely, if AVIC writes all IRRs (5.1) and ANDs all IsRunning >>>>> flags before checking the result of the AND (6). >>>>> >>>>>>> (It would, but I believe that AVIC designers made it sane and the spec >>>>>>> doesn't let me read it in a way that supports your theories.) >>>>> >>>>> I hope so as well, and you've probably convinced me. But I still think >>>>> the code is wrong in this patch. Let's look at the spec that you pasted: >>> The code definitely is wrong. I'll be more specific when disagreeing, >>> sorry. >>> >> >> Would you please be a bit more specific on what you think I am not doing >> correctly to handle the #VMEXIT in the case of target not running below. >> >> + case AVIC_INCMP_IPI_ERR_TARGET_NOT_RUN: >> + kvm_lapic_reg_write(apic, APIC_ICR2, icrh); >> + kvm_lapic_reg_write(apic, APIC_ICR, icrl); >> >> This is actually not just writing to the register. Please note that writing >> to APIC_ICR register would also be calling apic_send_ipi(), which results in >> injecting interrupts to the target core: > > Exactly. Injecting the interrupt in AVIC_INCMP_IPI_ERR_TARGET_NOT_RUN > handler is causing the double-injection bug that Paolo described. > >> Am I missing something? > > Probably that AVIC already wrote to all IRRs (and sent appropriate > doorbells) before this VMEXIT, so KVM shouldn't repeat it.
Ah, Ok I got it now. Thanks for the detail description. I am still waiting to hear back from the hardware designer to confirm the HW behavior. Meanwhile, I have tried NOT setting the IRR, and only kick_vcpu(). And things seem to work fine. Therefore, I think your analysis is likely to be correct.
Thanks again, Suravee
| |