From mboxrd@z Thu Jan  1 00:00:00 1970
From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
Subject: Re: [PART1 RFC 5/9] svm: Add VMEXIT handlers for AVIC
Date: Thu, 18 Feb 2016 09:25:04 +0700
Message-ID: <56C52B80.5050104@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>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8;
	format=flowed
Content-Transfer-Encoding: QUOTED-PRINTABLE
Cc: <joro@8bytes.org>, <alex.williamson@redhat.com>, <gleb@kernel.org>,
	<kvm@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<wei@redhat.com>, <sherry.hurwitz@amd.com>
To: =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= <rkrcmar@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>
Return-path: <linux-kernel-owner@vger.kernel.org>
In-Reply-To: <20160216180618.GA18952@potion.brq.redhat.com>
Sender: linux-kernel-owner@vger.kernel.org
List-Id: kvm.vger.kernel.org

Hi

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 wr=
ites to
>>> >>the same byte in IRR.  We're fine as long as AVIC writes IRR befo=
re
>>> >>checking IsRunning on every destination, which it seems to be.
>> >
>> >More precisely, if AVIC writes all IRRs (5.1) and ANDs all IsRunnin=
g
>> >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 t=
hink
>> >the code is wrong in this patch.  Let's look at the spec that you p=
asted:
> 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 doin=
g=20
correctly to handle the #VMEXIT in the case of target not running below=
=2E

+    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=20
writing to APIC_ICR register would also be calling apic_send_ipi(),=20
which results in injecting interrupts to the target core:

=46rom: arch/x86/kvm/lapic.c: kvm_lapic_write_reg()
[....]
         case APIC_ICR:
                 /* No delay here, so we always clear the pending bit *=
/
                 apic_set_reg(apic, APIC_ICR, val & ~(1 << 12));
                 apic_send_ipi(apic);
                 break;

         case APIC_ICR2:
                 if (!apic_x2apic_mode(apic))
                         val &=3D 0xff000000;
                 apic_set_reg(apic, APIC_ICR2, val);
                 break;

Am I missing something?

Thanks,
Suravee