Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933877AbbLPIKT (ORCPT ); Wed, 16 Dec 2015 03:10:19 -0500 Received: from mga02.intel.com ([134.134.136.20]:3037 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932664AbbLPIKR (ORCPT ); Wed, 16 Dec 2015 03:10:17 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.20,436,1444719600"; d="scan'208";a="872523590" Subject: Re: [PATCH 09/11] KVM: MMU: simplify mmu_need_write_protect To: Xiao Guangrong , pbonzini@redhat.com References: <1448907973-36066-1-git-send-email-guangrong.xiao@linux.intel.com> <1448907973-36066-10-git-send-email-guangrong.xiao@linux.intel.com> <566FD2A1.7010601@linux.intel.com> <566FDC76.1090703@linux.intel.com> Cc: gleb@kernel.org, mtosatti@redhat.com, kvm@vger.kernel.org, linux-kernel@vger.kernel.org From: Kai Huang Message-ID: <56711B52.3070005@linux.intel.com> Date: Wed, 16 Dec 2015 16:05:38 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.4.0 MIME-Version: 1.0 In-Reply-To: <566FDC76.1090703@linux.intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3287 Lines: 90 On 12/15/2015 05:25 PM, Xiao Guangrong wrote: > > > On 12/15/2015 04:43 PM, Kai Huang wrote: >> >> >> On 12/01/2015 02:26 AM, Xiao Guangrong wrote: >>> Now, all non-leaf shadow page are page tracked, if gfn is not tracked >>> there is no non-leaf shadow page of gfn is existed, we can directly >>> make the shadow page of gfn to unsync >>> >>> Signed-off-by: Xiao Guangrong >>> --- >>> arch/x86/kvm/mmu.c | 26 ++++++++------------------ >>> 1 file changed, 8 insertions(+), 18 deletions(-) >>> >>> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c >>> index 5a2ca73..f89e77f 100644 >>> --- a/arch/x86/kvm/mmu.c >>> +++ b/arch/x86/kvm/mmu.c >>> @@ -2461,41 +2461,31 @@ static void __kvm_unsync_page(struct >>> kvm_vcpu *vcpu, struct kvm_mmu_page *sp) >>> kvm_mmu_mark_parents_unsync(sp); >>> } >>> -static void kvm_unsync_pages(struct kvm_vcpu *vcpu, gfn_t gfn) >>> +static bool kvm_unsync_pages(struct kvm_vcpu *vcpu, gfn_t gfn, >>> + bool can_unsync) >>> { >>> struct kvm_mmu_page *s; >>> for_each_gfn_indirect_valid_sp(vcpu->kvm, s, gfn) { >>> + if (!can_unsync) >>> + return true; >> How about moving this right before for_each_gfn_indirect_valid_sp? As >> can_unsync is passed as >> parameter, so there's no point checking it several times. >> > > We can not do this. What we are doing here is checking if we have > shadow page mapping > for 'gfn': > a) if no, it can be writable. I think in this case you should also check whether the GFN is being write protection tracked. Ex, if the spte never exists when you add the GFN to write protection tracking, and in this case I think mmu_need_write_protect should also report true. Right? > b) if yes, check 'can_unsync' to see if these shadow pages can make to > be 'unsync'. > > Your suggestion can break the point a). > >> A further thinking is can we move it to mmu_need_write_protect? >> Passing can_unsync as parameter to >> kvm_unsync_pages sounds a little bit odd. >> >>> + >>> if (s->unsync) >>> continue; >>> WARN_ON(s->role.level != PT_PAGE_TABLE_LEVEL); >> How about large page mapping? Such as if guest uses 2M mapping and >> its shadow is indirect, does >> above WARN_ON still meet? As you removed the PT level check in >> mmu_need_write_protect. > > The lager mapping are on the non-leaf shadow pages which can be > figured out by > kvm_page_track_check_mode() before we call this function. Actually I am not quite understanding how large page mapping is implemented. I see in kvm_mmu_get_page, when sp is allocated, it is large page mapping disabled, but I think we do support large shadow mapping, right? I mean theoretically if guest uses 2M mapping and shadow mapping can certainly use 2M mapping as well, and the 2M shadow mapping can also be 'unsynced' (as a leaf mapping table). But in your series I see if we write protect some GFN, the shadow large page mapping is always disabled. Am I wrong? Thanks, -Kai > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/