From mboxrd@z Thu Jan 1 00:00:00 1970 From: Suravee Suthikulpanit Subject: Re: [PATCH] KVM: split kvm_vcpu_wake_up from kvm_vcpu_kick Date: Fri, 18 Mar 2016 12:13:05 +0700 Message-ID: <56EB8E61.4010702@amd.com> References: <1457124368-2025-1-git-send-email-Suravee.Suthikulpanit@amd.com> <1457124368-2025-8-git-send-email-Suravee.Suthikulpanit@amd.com> <20160309205512.GD19459@potion.brq.redhat.com> <56EB0918.80709@amd.com> <20160317202727.GA6465@potion.brq.redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: , , , , , , , , To: =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= Return-path: In-Reply-To: <20160317202727.GA6465@potion.brq.redhat.com> Sender: linux-kernel-owner@vger.kernel.org List-Id: kvm.vger.kernel.org Hi, On 03/18/2016 03:27 AM, Radim Kr=C4=8Dm=C3=A1=C5=99 wrote: > 2016-03-18 02:44+0700, Suravee Suthikulpanit: >> On 3/10/16 03:55, Radim Kr=C4=8Dm=C3=A1=C5=99 wrote: >>> 2016-03-04 14:46-0600, Suravee Suthikulpanit: >>>>> From: Suravee Suthikulpanit >>>>> >>>>> Introduce VMEXIT handlers, avic_incp_ipi_interception() and >>>>> avic_noaccel_interception(). >>>>> >>>>> Signed-off-by: Suravee Suthikulpanit >>>>> --- >>>>> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c >>>>> @@ -3690,6 +3690,264 @@ static int mwait_interception(struct vcpu= _svm *svm) >>>>> + case AVIC_INCMP_IPI_ERR_TARGET_NOT_RUN: { >>>>> + kvm_for_each_vcpu(i, vcpu, kvm) { >>>>> + if (!kvm_apic_match_dest(vcpu, apic, >>>>> + icrl & APIC_SHORT_MASK, >>>>> + GET_APIC_DEST_FIELD(icrh), >>>>> + icrl & APIC_DEST_MASK)) >>>>> + continue; >>>>> + >>>>> + kvm_vcpu_kick(vcpu); >>> KVM shouldn't kick VCPUs that are running. (Imagine a broadcast wh= en >>> most VCPUs are in guest mode.) >> >> So, besides checking if the vcpu match the destination, I will add t= he check >> to see if the is_running bit is set before calling kvm_vcpu_kick() > > That will do. > >>> I think a new helper might be useful here: we only want to wake up = from >>> wait queue, but never force VCPU out of guest mode ... kvm_vcpu_kic= k() >>> does both. >> >> If I only kick non-running vcpu, do I still need this new helper fun= ction? > > I would prefer it. It's a minor performance optimization (non-runnin= g > VCPUs aren't in guest mode) and makes our intent clear. Please use > include the following patch and use kvm_vcpu_wake_up instead. > > ---8<--- > AVIC has a use for kvm_vcpu_wake_up. > > Signed-off-by: Radim Kr=C4=8Dm=C3=A1=C5=99 > --- > include/linux/kvm_host.h | 1 + > virt/kvm/kvm_main.c | 19 +++++++++++++------ > 2 files changed, 14 insertions(+), 6 deletions(-) > > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h > index 861f690aa791..7b269626a3b3 100644 > --- a/include/linux/kvm_host.h > +++ b/include/linux/kvm_host.h > @@ -650,6 +650,7 @@ void kvm_vcpu_mark_page_dirty(struct kvm_vcpu *vc= pu, gfn_t gfn); > void kvm_vcpu_block(struct kvm_vcpu *vcpu); > void kvm_arch_vcpu_blocking(struct kvm_vcpu *vcpu); > void kvm_arch_vcpu_unblocking(struct kvm_vcpu *vcpu); > +void kvm_vcpu_wake_up(struct kvm_vcpu *vcpu); > void kvm_vcpu_kick(struct kvm_vcpu *vcpu); > int kvm_vcpu_yield_to(struct kvm_vcpu *target); > void kvm_vcpu_on_spin(struct kvm_vcpu *vcpu); > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c > index 1eae05236347..c39c54afdb74 100644 > --- a/virt/kvm/kvm_main.c > +++ b/virt/kvm/kvm_main.c > @@ -2054,13 +2054,8 @@ out: > EXPORT_SYMBOL_GPL(kvm_vcpu_block); > > #ifndef CONFIG_S390 > -/* > - * Kick a sleeping VCPU, or a guest VCPU in guest mode, into host ke= rnel mode. > - */ > -void kvm_vcpu_kick(struct kvm_vcpu *vcpu) > +void kvm_vcpu_wake_up(struct kvm_vcpu *vcpu) > { > - int me; > - int cpu =3D vcpu->cpu; > wait_queue_head_t *wqp; > > wqp =3D kvm_arch_vcpu_wq(vcpu); > @@ -2068,6 +2063,18 @@ void kvm_vcpu_kick(struct kvm_vcpu *vcpu) > wake_up_interruptible(wqp); > ++vcpu->stat.halt_wakeup; > } > +} > +EXPORT_SYMBOL_GPL(kvm_vcpu_wake_up); > + > +/* > + * Kick a sleeping VCPU, or a guest VCPU in guest mode, into host ke= rnel mode. > + */ > +void kvm_vcpu_kick(struct kvm_vcpu *vcpu) > +{ > + int me; > + int cpu =3D vcpu->cpu; > + > + kvm_vcpu_wake_up(vcpu); > > me =3D get_cpu(); > if (cpu !=3D me && (unsigned)cpu < nr_cpu_ids && cpu_online(cpu)) > Sure, I'll include this in my V3. Thanks, Suravee