From patchwork Wed Jan 16 00:43:40 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Christoffer Dall X-Patchwork-Id: 1986161 Return-Path: X-Original-To: patchwork-kvm@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork1.kernel.org (Postfix) with ESMTP id 1C7B13FE1F for ; Wed, 16 Jan 2013 00:43:56 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756414Ab3APAnw (ORCPT ); Tue, 15 Jan 2013 19:43:52 -0500 Received: from mail-qa0-f48.google.com ([209.85.216.48]:61274 "EHLO mail-qa0-f48.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754817Ab3APAnv (ORCPT ); Tue, 15 Jan 2013 19:43:51 -0500 Received: by mail-qa0-f48.google.com with SMTP id l8so745960qaq.0 for ; Tue, 15 Jan 2013 16:43:50 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=x-received:sender:from:to:cc:subject:date:message-id:x-mailer :x-gm-message-state; bh=w9qASmzkCBF5A5zI4RsTxzuVW08eIvX6N9lWDWpiRVw=; b=Pxg25qZeiMHx6Nqd+H6AZuEDPO2dzEre1uaXUMmI5BGQQ+WvSnfL/DAnkO0FA1zuJ1 uoZkU+oTpcyxwUBa6+9/BlwrPTkSStCzOzkBQmUOiTBkUdKNH0+vSy30hmlYRMS+DGMF D1tLtg5Dha65l3x9JEMU8lky8anAQQecbOiEs4o5vSPGdFylJd1yfg0yQ5BJMbQK+7Ct LrjiWoNKh5p4mF8wb/GlCDzxX/rktqj0DGJNxGBlTr+S03JascutisvcHNhnmsN1MHnU uV1VwHLNhvSgmTBKZzQGI9iVBRU4dpa4f7sg4AJx5z6GfbOl6vV2FGMKRiwNuthTq/An 2fLw== X-Received: by 10.49.84.104 with SMTP id x8mr92935233qey.5.1358297030618; Tue, 15 Jan 2013 16:43:50 -0800 (PST) Received: from ubuntu.columbia.edu (chazy.cs.columbia.edu. [128.59.22.176]) by mx.google.com with ESMTPS id o6sm5073980qaj.14.2013.01.15.16.43.49 (version=TLSv1.1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Tue, 15 Jan 2013 16:43:49 -0800 (PST) From: Christoffer Dall To: kvm@vger.kernel.org Cc: kvmarm@lists.cs.columbia.edu, nicolas@viennot.biz, Christoffer Dall Subject: KVM: Questions and comments on make_all_cpus_request Date: Tue, 15 Jan 2013 19:43:40 -0500 Message-Id: <1358297020-1548-1-git-send-email-cdall@cs.columbia.edu> X-Mailer: git-send-email 1.7.9.5 X-Gm-Message-State: ALoCoQlHGSV7VWkx45YuurN18bR5BMd51HrJSeQoIX8m163RVIsjScMpLV/G3Ko2hlmd/QH15uCB Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org 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(); + /* + * Set the bit on the CPU mask for all CPUs which are somehow + * running a guest (IN_GUEST_MODE, EXITING_GUEST_MODE, and + * READING_SHADOW_PAGE_TABLES). + */ if (cpus != NULL && cpu != -1 && cpu != me && kvm_vcpu_exiting_guest_mode(vcpu) != OUTSIDE_GUEST_MODE) cpumask_set_cpu(cpu, cpus); @@ -201,9 +222,13 @@ void kvm_flush_remote_tlbs(struct kvm *kvm) { long dirty_count = kvm->tlbs_dirty; - smp_mb(); + smp_mb(); /* TODO: Someone should explain this! */ if (make_all_cpus_request(kvm, KVM_REQ_TLB_FLUSH)) ++kvm->stat.remote_tlb_flush; + /* + * TODO: Someone should explain this, why is it a cmpxchg, what + * happens if the dirty is different from dirty_count? + */ cmpxchg(&kvm->tlbs_dirty, dirty_count, 0); }