From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marcelo Tosatti Subject: Re: KVM: Questions and comments on make_all_cpus_request Date: Wed, 16 Jan 2013 16:19:02 -0200 Message-ID: <20130116181902.GA19604@amt.cnet> References: <1358297020-1548-1-git-send-email-cdall@cs.columbia.edu> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: kvm@vger.kernel.org, kvmarm@lists.cs.columbia.edu, nicolas@viennot.biz To: Christoffer Dall Return-path: Received: from mx1.redhat.com ([209.132.183.28]:44024 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754064Ab3APS0Z (ORCPT ); Wed, 16 Jan 2013 13:26:25 -0500 Content-Disposition: inline In-Reply-To: <1358297020-1548-1-git-send-email-cdall@cs.columbia.edu> Sender: kvm-owner@vger.kernel.org List-ID: On Tue, Jan 15, 2013 at 07:43:40PM -0500, Christoffer Dall wrote: > Hi KVM guys, > > I've had a bit of challenges figuring out the exact functinality and > synchronization of vcpu->requests and friends. In lack of a better > method, I wrote some comments as a patch. > > I think this code really deserves some explaining, as it is really hard > to understand otherwise. Unfortunately, I wasn't able to write down > concise and exact comments, but I hope someone else feels up to the > challenge. > > Let me know if I just got this completely wrong and upside down. > > Thanks, > Christoffer > --- > include/linux/kvm_host.h | 4 ++++ > virt/kvm/kvm_main.c | 29 +++++++++++++++++++++++++++-- > 2 files changed, 31 insertions(+), 2 deletions(-) > > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h > index cbe0d68..25deef8 100644 > --- a/include/linux/kvm_host.h > +++ b/include/linux/kvm_host.h > @@ -252,6 +252,10 @@ struct kvm_vcpu { > struct kvm_vcpu_arch arch; > }; > > +/* > + * XXX: Could we explain what we're trying to achieve? Is this an > + * optimization as to not send multiple IPIs? > + */ > static inline int kvm_vcpu_exiting_guest_mode(struct kvm_vcpu *vcpu) > { > return cmpxchg(&vcpu->mode, IN_GUEST_MODE, EXITING_GUEST_MODE); > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c > index e45c20c..ccc292d 100644 > --- a/virt/kvm/kvm_main.c > +++ b/virt/kvm/kvm_main.c > @@ -165,7 +165,18 @@ static void ack_flush(void *_completed) > { > } > > -static bool make_all_cpus_request(struct kvm *kvm, unsigned int req) > +/** > + * make_all_cpus_request - place request on vcpus > + * @kvm: KVM Struct > + * @req: Request to make on the VCPU > + * > + * This function places a request on a VCPU and ensures that the VCPU request > + * is handled before returning from the function, if the VCPU is in guest > + * mode (or exiting, or reading shadow page tables?). > + * > + * Returns true if at least of the vcpus were sent an IPI and responded to it, > + */ > +static bool make_all_vcpus_request(struct kvm *kvm, unsigned int req) > { > int i, cpu, me; > cpumask_var_t cpus; > @@ -179,9 +190,19 @@ static bool make_all_cpus_request(struct kvm *kvm, unsigned int req) > kvm_make_request(req, vcpu); > cpu = vcpu->cpu; > > + /* > + * Is the following really true? Can we have an example of the > + * race that would otherwise happen? Doesn't the make_request > + * pair against the IPI and interrupt disabling, not the mode? > + */ > /* Set ->requests bit before we read ->mode */ > smp_mb(); cpu0 cpu1 make_all_reqs mode = vcpu->mode = OUTSIDE_GUEST_MODE vcpu->mode = IN_GUEST_MODE if (vcpu->requests) set_bit(KVM_REQ_y, vcpu) no kick, its outside guest mode