From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753782AbbJSJhf (ORCPT ); Mon, 19 Oct 2015 05:37:35 -0400 Received: from mx1.redhat.com ([209.132.183.28]:35629 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753532AbbJSJhb (ORCPT ); Mon, 19 Oct 2015 05:37:31 -0400 Subject: Re: [PATCH] KVM: x86: MMU: Initialize force_pt_level before calling mapping_level() To: Takuya Yoshikawa References: <20151019151329.cf4b7cbbfe1fdb15bb756d93@lab.ntt.co.jp> Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org From: Paolo Bonzini Message-ID: <5624B9D8.1040804@redhat.com> Date: Mon, 19 Oct 2015 11:37:28 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: <20151019151329.cf4b7cbbfe1fdb15bb756d93@lab.ntt.co.jp> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 19/10/2015 08:13, Takuya Yoshikawa wrote: > Commit fd1369021878 ("KVM: x86: MMU: Move mapping_level_dirty_bitmap() > call in mapping_level()") forgot to initialize force_pt_level to false > in FNAME(page_fault)() before calling mapping_level() like > nonpaging_map() does. This can sometimes result in forcing page table > level mapping unnecessarily. > > Fix this and move the first *force_pt_level check in mapping_level() > before kvm_vcpu_gfn_to_memslot() call to make it a bit clearer that > the variable must be initialized before mapping_level() gets called. > > This change can also avoid calling kvm_vcpu_gfn_to_memslot() when > !check_hugepage_cache_consistency() check in tdp_page_fault() forces > page table level mapping. > > Signed-off-by: Takuya Yoshikawa > --- > arch/x86/kvm/mmu.c | 7 ++++--- > arch/x86/kvm/paging_tmpl.h | 2 +- > 2 files changed, 5 insertions(+), 4 deletions(-) > > diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c > index dd2a7c6..7d85bca 100644 > --- a/arch/x86/kvm/mmu.c > +++ b/arch/x86/kvm/mmu.c > @@ -886,10 +886,11 @@ static int mapping_level(struct kvm_vcpu *vcpu, gfn_t large_gfn, > int host_level, level, max_level; > struct kvm_memory_slot *slot; > > - slot = kvm_vcpu_gfn_to_memslot(vcpu, large_gfn); > + if (unlikely(*force_pt_level)) > + return PT_PAGE_TABLE_LEVEL; > > - if (likely(!*force_pt_level)) > - *force_pt_level = !memslot_valid_for_gpte(slot, true); > + slot = kvm_vcpu_gfn_to_memslot(vcpu, large_gfn); > + *force_pt_level = !memslot_valid_for_gpte(slot, true); > if (unlikely(*force_pt_level)) > return PT_PAGE_TABLE_LEVEL; > > diff --git a/arch/x86/kvm/paging_tmpl.h b/arch/x86/kvm/paging_tmpl.h > index bf39d0f..b41faa9 100644 > --- a/arch/x86/kvm/paging_tmpl.h > +++ b/arch/x86/kvm/paging_tmpl.h > @@ -698,7 +698,7 @@ static int FNAME(page_fault)(struct kvm_vcpu *vcpu, gva_t addr, u32 error_code, > int r; > pfn_t pfn; > int level = PT_PAGE_TABLE_LEVEL; > - bool force_pt_level; > + bool force_pt_level = false; > unsigned long mmu_seq; > bool map_writable, is_self_change_mapping; > > Looks good, thanks. Paolo