From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753044AbdJaHe5 (ORCPT ); Tue, 31 Oct 2017 03:34:57 -0400 Received: from mga06.intel.com ([134.134.136.31]:54465 "EHLO mga06.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751798AbdJaHez (ORCPT ); Tue, 31 Oct 2017 03:34:55 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.44,322,1505804400"; d="scan'208";a="144171665" Date: Tue, 31 Oct 2017 15:35:04 +0800 From: Haozhong Zhang To: Ingo Molnar Cc: kvm@vger.kernel.org, x86@kernel.org, linux-kernel@vger.kernel.org, Paolo Bonzini , rkrcmar@redhat.com, Xiao Guangrong , Dan Williams , ivan.d.cuevas.escareno@intel.com, karthik.kumar@intel.com Subject: Re: [PATCH 3/3] KVM: MMU: consider host cache type in MMIO pfn check Message-ID: <20171031073504.xunicgpm57k7qyk2@hz-desktop> Mail-Followup-To: Ingo Molnar , kvm@vger.kernel.org, x86@kernel.org, linux-kernel@vger.kernel.org, Paolo Bonzini , rkrcmar@redhat.com, Xiao Guangrong , Dan Williams , ivan.d.cuevas.escareno@intel.com, karthik.kumar@intel.com References: <20171027022524.22589-1-haozhong.zhang@intel.com> <20171027022524.22589-4-haozhong.zhang@intel.com> <20171027084038.j45wuqcw5sb2h3ag@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20171027084038.j45wuqcw5sb2h3ag@gmail.com> User-Agent: NeoMutt/20170714 (1.8.3) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 10/27/17 10:40 +0200, Ingo Molnar wrote: > > * Haozhong Zhang wrote: > > > By default, KVM treats a reserved page as for MMIO purpose, and maps > > it to guest with UC memory type. However, some reserved pages are not > > for MMIO, such as pages of DAX device (e.g., /dev/daxX.Y). Mapping > > them with UC memory type will harm the performance. In order to > > exclude those cases, we check the host cache mode in addition and only > > treat UC/UC- pages as MMIO. > > > > Signed-off-by: Haozhong Zhang > > Reported-by: Cuevas Escareno, Ivan D > > Reported-by: Kumar, Karthik > > --- > > arch/x86/kvm/mmu.c | 32 +++++++++++++++++++++++++++++--- > > 1 file changed, 29 insertions(+), 3 deletions(-) > > > > diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c > > index 0b481cc9c725..d4c821a6df3d 100644 > > --- a/arch/x86/kvm/mmu.c > > +++ b/arch/x86/kvm/mmu.c > > @@ -2707,10 +2707,36 @@ static bool mmu_need_write_protect(struct kvm_vcpu *vcpu, gfn_t gfn, > > > > static bool kvm_is_mmio_pfn(kvm_pfn_t pfn) > > { > > - if (pfn_valid(pfn)) > > - return !is_zero_pfn(pfn) && PageReserved(pfn_to_page(pfn)); > > + bool is_mmio = true; > > > > - return true; > > + if (pfn_valid(pfn)) { > > + is_mmio = !is_zero_pfn(pfn) && PageReserved(pfn_to_page(pfn)); > > + > > + /* > > + * By default, KVM treats a reserved page as for MMIO > > + * purpose, and maps it to guest with UC memory type. > > + * However, some reserved pages are not for MMIO, such > > + * as pages of DAX device (e.g., /dev/daxX.Y). Mapping > > + * them with UC memory type will harm the performance. > > + * In order to exclude those cases, we check the host > > + * cache mode in addition and only treat UC/UC- pages > > + * as MMIO. > > + * > > + * track_pfn_insert() works only when PAT is enabled, > > + * so add pat_enabled() here. > > + */ > > + if (is_mmio && pat_enabled()) { > > + pgprot_t prot; > > + enum page_cache_mode cm; > > + > > + track_pfn_insert(NULL, &prot, kvm_pfn_to_pfn(pfn)); > > + cm = pgprot2cachemode(prot); > > + is_mmio = (cm == _PAGE_CACHE_MODE_UC || > > + cm == _PAGE_CACHE_MODE_UC_MINUS); > > + } > > + } > > + > > + return is_mmio; > > } > > > > static int set_spte(struct kvm_vcpu *vcpu, u64 *sptep, > > s/harm the performance > /harm performance> > > But I suspect the rest of the comment should be rewritten too to be more fluid. I'll refactor the comment. > > Beyond that - I think instead of exposing these low level details a properly named > helper function should be put into pat.c instead - and KVM can use that. > KVM only needs the memory type information, so lookup_memtype() in pat.c is probably a better one to be exposed. Haozhong