From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH RFC] KVM: MMU: Don't use RCU for lockless shadow walking Date: Tue, 24 Apr 2012 13:02:00 +0300 Message-ID: <4F967A18.8030303@redhat.com> References: <1335197812-32064-1-git-send-email-avi@redhat.com> <4F964A2C.7050106@linux.vnet.ibm.com> <4F96703F.4000607@redhat.com> <4F967870.6080806@linux.vnet.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Cc: Marcelo Tosatti , kvm@vger.kernel.org To: Xiao Guangrong Return-path: Received: from mx1.redhat.com ([209.132.183.28]:27351 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932241Ab2DXKCE (ORCPT ); Tue, 24 Apr 2012 06:02:04 -0400 In-Reply-To: <4F967870.6080806@linux.vnet.ibm.com> Sender: kvm-owner@vger.kernel.org List-ID: On 04/24/2012 12:54 PM, Xiao Guangrong wrote: > On 04/24/2012 05:19 PM, Avi Kivity wrote: > > > >>> Turned out to be simpler than expected. However, I think there's a problem > >>> with make_all_cpus_request() possible reading an incorrect vcpu->cpu. > >> > >> > >> It seems possible. > >> > >> Can we fix it by reading vcpu->cpu when the vcpu is in GUEST_MODE or > >> EXITING_GUEST_MODE (IIRC, in these modes, interrupt is disabled)? > >> > >> Like: > >> > >> if (kvm_vcpu_exiting_guest_mode(vcpu) != OUTSIDE_GUEST_MODE) > >> cpumask_set_cpu(vcpu->cpu, cpus); > > > > I think it is actually okay. We are only vulnerable if lockless shadow > > walk started during prepare_zap_page(), and extends past > > kvm_flush_remote_tlbs(), yes? But in that case, vcpu->cpu is stable > > since local_irq_disable() kills preemption. > > > > > This case can happen? > > VCPU 0 VCPU 1 > > kvm_for_each_vcpu(i, vcpu, kvm) { > kvm_make_request(req, vcpu); > > VCPU1 is running on CPU 1 out of guest mode > > cpu = vcpu->cpu; > > /* Set ->requests bit before we read ->mode */ > smp_mb(); > > if (cpus != NULL && cpu != -1 && cpu != me && > > VCPU1 is scheduled to CPU 2, and running in > guest mode > > kvm_vcpu_exiting_guest_mode(vcpu) != OUTSIDE_GUEST_MODE) > cpumask_set_cpu(cpu, cpus); > } > > VCPU 0 send IPI to CPU1, but actually, VCPU1 is running on CPU 2. > It can happen, but it's benign. After migration, vcpu1 will examine vcpu->requests and flush the TLB. -- error compiling committee.c: too many arguments to function