From mboxrd@z Thu Jan 1 00:00:00 1970 From: Suravee Suthikulpanit Subject: Re: [PART1 RFC 5/9] svm: Add VMEXIT handlers for AVIC Date: Fri, 19 Feb 2016 18:32:54 +0700 Message-ID: <56C6FD66.8070502@amd.com> References: <1455285574-27892-1-git-send-email-suravee.suthikulpanit@amd.com> <1455285574-27892-6-git-send-email-suravee.suthikulpanit@amd.com> <56BDFC72.7030905@redhat.com> <56C2C1BF.7010700@amd.com> <56C312E1.1080902@redhat.com> <20160216141330.GG10555@potion.brq.redhat.com> <56C354A5.4040807@redhat.com> <20160216180618.GA18952@potion.brq.redhat.com> <56C52B80.5050104@amd.com> <20160218141817.GA6289@potion.brq.redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: Paolo Bonzini , , , , , , , To: =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= Return-path: In-Reply-To: <20160218141817.GA6289@potion.brq.redhat.com> Sender: linux-kernel-owner@vger.kernel.org List-Id: kvm.vger.kernel.org Hi, On 2/18/16 21:18, Radim Kr=C4=8Dm=C3=A1=C5=99 wrote: > 2016-02-18 09:25+0700, Suravee Suthikulpanit: >> On 2/17/16 01:06, Radim Kr=C4=8Dm=C3=A1=C5=99 wrote: >>> 2016-02-16 17:56+0100, Paolo Bonzini: >>>>> On 16/02/2016 15:13, Radim Kr=C4=8Dm=C3=A1=C5=99 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 be= fore >>>>>>> checking IsRunning on every destination, which it seems to be. >>>>> >>>>> More precisely, if AVIC writes all IRRs (5.1) and ANDs all IsRunn= ing >>>>> flags before checking the result of the AND (6). >>>>> >>>>>>> (It would, but I believe that AVIC designers made it sane and t= he 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 disagreei= ng, >>> sorry. >>> >> >> Would you please be a bit more specific on what you think I am not d= oing >> correctly to handle the #VMEXIT in the case of target not running be= low. >> >> + 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 re= sults in >> injecting interrupts to the target core: > > Exactly. Injecting the interrupt in AVIC_INCMP_IPI_ERR_TARGET_NOT_RU= N > 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=20 waiting to hear back from the hardware designer to confirm the HW=20 behavior. Meanwhile, I have tried NOT setting the IRR, and only=20 kick_vcpu(). And things seem to work fine. Therefore, I think your=20 analysis is likely to be correct. Thanks again, Suravee