From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marcelo Tosatti Subject: Re: [PATCH 08/31] nVMX: Fix local_vcpus_link handling Date: Wed, 18 May 2011 09:08:01 -0300 Message-ID: <20110518120801.GA9176@amt.cnet> References: <201105161948.p4GJm1as001742@rice.haifa.ibm.com> <20110517131918.GA3809@amt.cnet> <4DD27998.1040105@redhat.com> <20110517143532.GA2490@fermat.math.technion.ac.il> <4DD2902C.9050403@redhat.com> <20110517181132.GA16262@fermat.math.technion.ac.il> <20110517184336.GA10394@amt.cnet> <20110517193030.GA21656@fermat.math.technion.ac.il> <20110517195253.GB11065@amt.cnet> <20110518055236.GA1230@fermat.math.technion.ac.il> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Avi Kivity , kvm@vger.kernel.org, gleb@redhat.com To: "Nadav Har'El" Return-path: Received: from mx1.redhat.com ([209.132.183.28]:39750 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932976Ab1ERMIY (ORCPT ); Wed, 18 May 2011 08:08:24 -0400 Content-Disposition: inline In-Reply-To: <20110518055236.GA1230@fermat.math.technion.ac.il> Sender: kvm-owner@vger.kernel.org List-ID: On Wed, May 18, 2011 at 08:52:36AM +0300, Nadav Har'El wrote: > On Tue, May 17, 2011, Marcelo Tosatti wrote about "Re: [PATCH 08/31] nVMX: Fix local_vcpus_link handling": > > On Tue, May 17, 2011 at 10:30:30PM +0300, Nadav Har'El wrote: > > > So we'll have two fields, vmx.vcpu.cpu and vmx.vmcs.cpu, which are supposed > > > to always contain the same value. Are you fine with that? > > > > Yes. Avi? > > Oops, it's even worse than I said, because if the new vmclear_local_vmcss > clears the vmcs currently used on some vcpu, it will update vmcs.cpu on that > vcpu to -1, but will *not* update vmx.vcpu.cpu, which remain its old value, > and potentially cause problems when it is used (e.g., in x86.c) instead > of vmx.vmcs.cpu. Humpf, right. OK, you can handle the x86.c usage with diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 64edf57..b5fd9b4 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -2118,7 +2118,7 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu) if (need_emulate_wbinvd(vcpu)) { if (kvm_x86_ops->has_wbinvd_exit()) cpumask_set_cpu(cpu, vcpu->arch.wbinvd_dirty_mask); - else if (vcpu->cpu != -1 && vcpu->cpu != cpu) + else if (vcpu->cpu != -1 && vcpu->cpu != cpu && cpu_online(vcpu->cpu)) smp_call_function_single(vcpu->cpu, wbinvd_ipi, NULL, 1); } Note this is not just about the code being nicer, but simplicity is crucial, the code is tricky enough with one linked list.