From patchwork Tue Aug 19 08:50:21 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Xiao Guangrong X-Patchwork-Id: 4741481 Return-Path: X-Original-To: patchwork-kvm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id B9930C0338 for ; Tue, 19 Aug 2014 08:51:09 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id E184F2014A for ; Tue, 19 Aug 2014 08:51:08 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id AC88E20155 for ; Tue, 19 Aug 2014 08:51:06 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751853AbaHSIue (ORCPT ); Tue, 19 Aug 2014 04:50:34 -0400 Received: from e28smtp04.in.ibm.com ([122.248.162.4]:41170 "EHLO e28smtp04.in.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751795AbaHSIub (ORCPT ); Tue, 19 Aug 2014 04:50:31 -0400 Received: from /spool/local by e28smtp04.in.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 19 Aug 2014 14:20:27 +0530 Received: from d28dlp03.in.ibm.com (9.184.220.128) by e28smtp04.in.ibm.com (192.168.1.134) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Tue, 19 Aug 2014 14:20:24 +0530 Received: from d28relay04.in.ibm.com (d28relay04.in.ibm.com [9.184.220.61]) by d28dlp03.in.ibm.com (Postfix) with ESMTP id DC2D41258017; Tue, 19 Aug 2014 14:20:36 +0530 (IST) Received: from d28av05.in.ibm.com (d28av05.in.ibm.com [9.184.220.67]) by d28relay04.in.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id s7J8olDG42205410; Tue, 19 Aug 2014 14:20:47 +0530 Received: from d28av05.in.ibm.com (localhost [127.0.0.1]) by d28av05.in.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id s7J8oMUE021847; Tue, 19 Aug 2014 14:20:23 +0530 Received: from ericxiao.site (ericxiao.cn.ibm.com [9.111.29.90]) by d28av05.in.ibm.com (8.14.4/8.14.4/NCO v10.0 AVin) with ESMTP id s7J8oI09021586; Tue, 19 Aug 2014 14:20:19 +0530 Message-ID: <53F30FCD.3080109@linux.vnet.ibm.com> Date: Tue, 19 Aug 2014 16:50:21 +0800 From: Xiao Guangrong User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.6.0 MIME-Version: 1.0 To: Paolo Bonzini , David Matlack CC: Gleb Natapov , Avi Kivity , mtosatti@redhat.com, linux-kernel@vger.kernel.org, kvm@vger.kernel.org, stable@vger.kernel.org Subject: Re: [PATCH 1/2] KVM: fix cache stale memslot info with correct mmio generation number References: <1407999713-3726-1-git-send-email-xiaoguangrong@linux.vnet.ibm.com> <53F20653.2030204@redhat.com> <9AD43423-2FF3-422D-A5AD-61CAE6339CCC@linux.vnet.ibm.com> <53F24A49.2010807@redhat.com> <53F2C997.6070605@linux.vnet.ibm.com> <53F30AA4.4050803@redhat.com> In-Reply-To: <53F30AA4.4050803@redhat.com> X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 14081908-5564-0000-0000-000000D948FE Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org X-Spam-Status: No, score=-7.6 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On 08/19/2014 04:28 PM, Paolo Bonzini wrote: > Il 19/08/2014 05:50, Xiao Guangrong ha scritto: >> >> Note in the step *, my approach detects the invalid generation-number which >> will invalidate the mmio spte properly . > > You are right, in fact my mail included another part: "Another > alternative could be to use the low bit to mark an in-progress change, > *and skip the caching if the low bit is set*." Okay, what confused me it that it seems that the single line patch is ok to you. :) Now, do we really need to care the case 2? like David said: "Sorry I didn't explain myself very well: Since we can get a single wrong mmio exit no matter what, it has to be handled in userspace. So my point was, it doesn't really help to fix that one very specific way that it can happen, because it can just happen in other ways. (E.g. update memslots occurs after is_noslot_pfn() and before mmio exit)." What's your idea? > > I think if you always treat the low bit as zero in mmio sptes, you can > do that without losing a bit of the generation. What's you did is avoiding cache a invalid generation number into spte, but actually if we can figure it out when we check mmio access, it's ok. Like the updated patch i posted should fix it, that way avoids doubly increase the number. Okay, if you're interested increasing the number doubly, there is the simpler one: Reviewed-by: Xiao Guangrong --- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c index 9314678..bf49170 100644 --- a/arch/x86/kvm/mmu.c +++ b/arch/x86/kvm/mmu.c @@ -236,6 +236,9 @@ static unsigned int get_mmio_spte_generation(u64 spte) static unsigned int kvm_current_mmio_generation(struct kvm *kvm) { + /* The initialized generation number should be even. */ + BUILD_BUG_ON((MMIO_MAX_GEN - 150) & 0x1); + /* * Init kvm generation close to MMIO_MAX_GEN to easily test the * code of handling generation number wrap-around. @@ -292,6 +295,14 @@ static bool check_mmio_spte(struct kvm *kvm, u64 spte) kvm_gen = kvm_current_mmio_generation(kvm); spte_gen = get_mmio_spte_generation(spte); + /* + * Aha, we cached a being-updated generation number or + * generation number is currnetly being updated, let do the + * real check for mmio access. + */ + if ((kvm_gen | spte_gen) & 0x1) + return false; + trace_check_mmio_spte(spte, kvm_gen, spte_gen); return likely(kvm_gen == spte_gen); } diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index 33712fb..5c3f7b7 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -725,7 +725,7 @@ static struct kvm_memslots *install_new_memslots(struct kvm *kvm, update_memslots(slots, new, kvm->memslots->generation); rcu_assign_pointer(kvm->memslots, slots); synchronize_srcu_expedited(&kvm->srcu); - + kvm->memslots->generation++; kvm_arch_memslots_updated(kvm); return old_memslots;