Message ID | 20230714065006.20201-1-yan.y.zhao@intel.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:a6b2:0:b0:3e4:2afc:c1 with SMTP id c18csp2341092vqm; Fri, 14 Jul 2023 00:54:49 -0700 (PDT) X-Google-Smtp-Source: APBJJlFAkEQq2Z+hc8nCRu26cuzgJKXhHn1hOYt5pn5lT54BE5hvoQFs2OBU7b89Rm20/H7m6YvE X-Received: by 2002:aa7:c3d8:0:b0:51f:f1a4:edc6 with SMTP id l24-20020aa7c3d8000000b0051ff1a4edc6mr3119464edr.37.1689321289286; Fri, 14 Jul 2023 00:54:49 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1689321289; cv=none; d=google.com; s=arc-20160816; b=NRuCI15IZ4dSMEIPgxEL91h4k7p5xTufO5SYeB3C1wJeNyxj8rqRlAap0bn+pYBIMw HYZ6zfFMthNudMQa6fXW7uXYcR1j92AZKTnBky/7t0YITX3lFrhlElA0raLF2DkBdgNL SBON1puhQuT+oF7+Vl5rgIpIZGEZ/vFO/gtpvuvY/i08XwaNtjD3SM0r8meXLvGZ0fHj DMww6XpNTeuTjip1gcFOUgMf2YmrHmnTBRApAcNiJGVyW/oXhLhTpiOAmJm6CDZT9DwE iQcyDSc/AT2RuV2YRJBd3U6nCOmJmIYkEfkTkFjxcu+jkMc3zRJ386K0Iie+QzPI9L6k 5tOQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:references:in-reply-to:message-id:date:subject :cc:to:from:dkim-signature; bh=tq5Q9F2gLxJLnmUdRuEyz1Rj9zYdg2CJUzYnXrBGmrg=; fh=I6wrbN01dP2yWc/eoJu3Jm+A24tUYxebxIQloyExA+I=; b=L0YU/b2Tqo6yeZ+5XSkq5QNDQj7KKn455bGH8iWPsbIxTM+zZPHDLxAc6v5b2RuC3m 3KmcTRQ5T27zbV05ENUKd2i6xnZs/AYUe+kRwSS8krqFCeR2HeViNh3uB91qJ5IV8/px egl3Fa+2YtBeYNcAu6ZdVBNOI3SOVcl5m+E7EFrbKAOfFaW7fFcwoQaMojXvgImbYwpq urMz9T8a0oGhyy1LZHJoltU76CT+tMJwSut47xMPFroMxFldBzDsXd8/tn6AEJbcPuBX h5elrwTCmfXHKPeOcFMFK5DUMA74YPtJ5TuqzZMQuYNYXPeFw0HIZ0wE/tBpf9uzC2HY YCsg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=YndcM5mn; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id u18-20020aa7d892000000b0051e5cabfebdsi1867596edq.373.2023.07.14.00.54.24; Fri, 14 Jul 2023 00:54:49 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=YndcM5mn; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S235247AbjGNHR7 (ORCPT <rfc822;tebrre53rla2o@gmail.com> + 99 others); Fri, 14 Jul 2023 03:17:59 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54750 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235221AbjGNHRu (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Fri, 14 Jul 2023 03:17:50 -0400 Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 9049126B3; Fri, 14 Jul 2023 00:17:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1689319069; x=1720855069; h=from:to:cc:subject:date:message-id:in-reply-to: references; bh=ut2e3J98JDctUs6MMT7+OPH8GV0+N2ela+vuLlRy0YU=; b=YndcM5mn3lBZKgh+LzVexAQHPVUIrMy0yPzV8Y+ZJ984DB1+ZbgWnBeK CJmgltDgKzDYgSqh7vXXh/2vur7DpnZA3rZsFV6VO4WQR2Qwg8rML8oNK xOTu9J9VDNyvXaGTbbwQzNFruaTeHhVnCJtqyjisbZOfzOeuqG9AVcNIh 3VzCy7MsIJo8aWdWbHXCymuc7Om3npoXKonmgPmkBx5jaMZl+Y2LcxAte RUJDi+Kfe0ACzHnNOOlHoK/irI/OmnJW406nvdj3eSue4dcPduv9wO4BD r5JlEVwBFDVreMms+9dPal+LmaF1BCcpd/tP7ImwTwqnvH3urOUiXbN+A Q==; X-IronPort-AV: E=McAfee;i="6600,9927,10770"; a="396221566" X-IronPort-AV: E=Sophos;i="6.01,204,1684825200"; d="scan'208";a="396221566" Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga101.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 14 Jul 2023 00:16:48 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10770"; a="1052955616" X-IronPort-AV: E=Sophos;i="6.01,204,1684825200"; d="scan'208";a="1052955616" Received: from yzhao56-desk.sh.intel.com ([10.239.159.62]) by fmsmga005-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 14 Jul 2023 00:16:46 -0700 From: Yan Zhao <yan.y.zhao@intel.com> To: kvm@vger.kernel.org, linux-kernel@vger.kernel.org Cc: pbonzini@redhat.com, seanjc@google.com, chao.gao@intel.com, kai.huang@intel.com, robert.hoo.linux@gmail.com, yuan.yao@linux.intel.com, Yan Zhao <yan.y.zhao@intel.com> Subject: [PATCH v4 01/12] KVM: x86/mmu: helpers to return if KVM honors guest MTRRs Date: Fri, 14 Jul 2023 14:50:06 +0800 Message-Id: <20230714065006.20201-1-yan.y.zhao@intel.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20230714064656.20147-1-yan.y.zhao@intel.com> References: <20230714064656.20147-1-yan.y.zhao@intel.com> X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF, RCVD_IN_DNSWL_BLOCKED,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL, SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1771381760268571705 X-GMAIL-MSGID: 1771381760268571705 |
Series |
KVM: x86/mmu: refine memtype related mmu zap
|
|
Commit Message
Yan Zhao
July 14, 2023, 6:50 a.m. UTC
Added helpers to check if KVM honors guest MTRRs.
The inner helper __kvm_mmu_honors_guest_mtrrs() is also provided to
outside callers for the purpose of checking if guest MTRRs were honored
before stopping non-coherent DMA.
Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
---
arch/x86/kvm/mmu.h | 7 +++++++
arch/x86/kvm/mmu/mmu.c | 15 +++++++++++++++
2 files changed, 22 insertions(+)
Comments
On 14/7/2023 2:50 pm, Yan Zhao wrote: > Added helpers to check if KVM honors guest MTRRs. > The inner helper __kvm_mmu_honors_guest_mtrrs() is also provided to > outside callers for the purpose of checking if guest MTRRs were honored > before stopping non-coherent DMA. > > Suggested-by: Sean Christopherson <seanjc@google.com> > Signed-off-by: Yan Zhao <yan.y.zhao@intel.com> > --- > arch/x86/kvm/mmu.h | 7 +++++++ > arch/x86/kvm/mmu/mmu.c | 15 +++++++++++++++ > 2 files changed, 22 insertions(+) > > diff --git a/arch/x86/kvm/mmu.h b/arch/x86/kvm/mmu.h > index 92d5a1924fc1..38bd449226f6 100644 > --- a/arch/x86/kvm/mmu.h > +++ b/arch/x86/kvm/mmu.h > @@ -235,6 +235,13 @@ static inline u8 permission_fault(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu, > return -(u32)fault & errcode; > } > > +bool __kvm_mmu_honors_guest_mtrrs(struct kvm *kvm, bool vm_has_noncoherent_dma); > + > +static inline bool kvm_mmu_honors_guest_mtrrs(struct kvm *kvm) > +{ > + return __kvm_mmu_honors_guest_mtrrs(kvm, kvm_arch_has_noncoherent_dma(kvm)); > +} > + > void kvm_zap_gfn_range(struct kvm *kvm, gfn_t gfn_start, gfn_t gfn_end); > > int kvm_arch_write_log_dirty(struct kvm_vcpu *vcpu); > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c > index 1e5db621241f..b4f89f015c37 100644 > --- a/arch/x86/kvm/mmu/mmu.c > +++ b/arch/x86/kvm/mmu/mmu.c > @@ -4516,6 +4516,21 @@ static int kvm_tdp_mmu_page_fault(struct kvm_vcpu *vcpu, > } > #endif > > +bool __kvm_mmu_honors_guest_mtrrs(struct kvm *kvm, bool vm_has_noncoherent_dma) According to the motivation provided in the comment, the function will no longer need to be passed the parameter "struct kvm *kvm" but will rely on the global parameters (plus vm_has_noncoherent_dma), removing "*kvm" ? > +{ > + /* > + * If the TDP is enabled, the host MTRRs are ignored by TDP > + * (shadow_memtype_mask is non-zero), and the VM has non-coherent DMA > + * (DMA doesn't snoop CPU caches), KVM's ABI is to honor the memtype > + * from the guest's MTRRs so that guest accesses to memory that is > + * DMA'd aren't cached against the guest's wishes. > + * > + * Note, KVM may still ultimately ignore guest MTRRs for certain PFNs, > + * e.g. KVM will force UC memtype for host MMIO. > + */ > + return vm_has_noncoherent_dma && tdp_enabled && shadow_memtype_mask; > +} > + > int kvm_tdp_page_fault(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault) > { > /*
On Sat, Oct 07, 2023, Like Xu wrote: > On 14/7/2023 2:50 pm, Yan Zhao wrote: > > Added helpers to check if KVM honors guest MTRRs. > > The inner helper __kvm_mmu_honors_guest_mtrrs() is also provided to > > outside callers for the purpose of checking if guest MTRRs were honored > > before stopping non-coherent DMA. > > > > Suggested-by: Sean Christopherson <seanjc@google.com> > > Signed-off-by: Yan Zhao <yan.y.zhao@intel.com> > > --- > > arch/x86/kvm/mmu.h | 7 +++++++ > > arch/x86/kvm/mmu/mmu.c | 15 +++++++++++++++ > > 2 files changed, 22 insertions(+) > > > > diff --git a/arch/x86/kvm/mmu.h b/arch/x86/kvm/mmu.h > > index 92d5a1924fc1..38bd449226f6 100644 > > --- a/arch/x86/kvm/mmu.h > > +++ b/arch/x86/kvm/mmu.h > > @@ -235,6 +235,13 @@ static inline u8 permission_fault(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu, > > return -(u32)fault & errcode; > > } > > +bool __kvm_mmu_honors_guest_mtrrs(struct kvm *kvm, bool vm_has_noncoherent_dma); > > + > > +static inline bool kvm_mmu_honors_guest_mtrrs(struct kvm *kvm) > > +{ > > + return __kvm_mmu_honors_guest_mtrrs(kvm, kvm_arch_has_noncoherent_dma(kvm)); > > +} > > + > > void kvm_zap_gfn_range(struct kvm *kvm, gfn_t gfn_start, gfn_t gfn_end); > > int kvm_arch_write_log_dirty(struct kvm_vcpu *vcpu); > > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c > > index 1e5db621241f..b4f89f015c37 100644 > > --- a/arch/x86/kvm/mmu/mmu.c > > +++ b/arch/x86/kvm/mmu/mmu.c > > @@ -4516,6 +4516,21 @@ static int kvm_tdp_mmu_page_fault(struct kvm_vcpu *vcpu, > > } > > #endif > > +bool __kvm_mmu_honors_guest_mtrrs(struct kvm *kvm, bool vm_has_noncoherent_dma) > > According to the motivation provided in the comment, the function will no > longer need to be passed the parameter "struct kvm *kvm" but will rely on > the global parameters (plus vm_has_noncoherent_dma), removing "*kvm" ? Yeah, I'll fixup the commit to drop @kvm from the inner helper. Thanks!
On Mon, Oct 09, 2023, Sean Christopherson wrote: > On Sat, Oct 07, 2023, Like Xu wrote: > > On 14/7/2023 2:50 pm, Yan Zhao wrote: > > > diff --git a/arch/x86/kvm/mmu.h b/arch/x86/kvm/mmu.h > > > index 92d5a1924fc1..38bd449226f6 100644 > > > --- a/arch/x86/kvm/mmu.h > > > +++ b/arch/x86/kvm/mmu.h > > > @@ -235,6 +235,13 @@ static inline u8 permission_fault(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu, > > > return -(u32)fault & errcode; > > > } > > > +bool __kvm_mmu_honors_guest_mtrrs(struct kvm *kvm, bool vm_has_noncoherent_dma); > > > + > > > +static inline bool kvm_mmu_honors_guest_mtrrs(struct kvm *kvm) > > > +{ > > > + return __kvm_mmu_honors_guest_mtrrs(kvm, kvm_arch_has_noncoherent_dma(kvm)); > > > +} > > > + > > > void kvm_zap_gfn_range(struct kvm *kvm, gfn_t gfn_start, gfn_t gfn_end); > > > int kvm_arch_write_log_dirty(struct kvm_vcpu *vcpu); > > > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c > > > index 1e5db621241f..b4f89f015c37 100644 > > > --- a/arch/x86/kvm/mmu/mmu.c > > > +++ b/arch/x86/kvm/mmu/mmu.c > > > @@ -4516,6 +4516,21 @@ static int kvm_tdp_mmu_page_fault(struct kvm_vcpu *vcpu, > > > } > > > #endif > > > +bool __kvm_mmu_honors_guest_mtrrs(struct kvm *kvm, bool vm_has_noncoherent_dma) > > > > According to the motivation provided in the comment, the function will no > > longer need to be passed the parameter "struct kvm *kvm" but will rely on > > the global parameters (plus vm_has_noncoherent_dma), removing "*kvm" ? > > Yeah, I'll fixup the commit to drop @kvm from the inner helper. Thanks! Gah, and I gave more bad advice when I suggested this idea. There's no need to explicitly check tdp_enabled, as shadow_memtype_mask is set to zero if TDP is disabled. And that must be the case, e.g. make_spte() would generate a corrupt shadow_memtype_mask were non-zero on Intel with shadow paging. Yan, can you take a look at what I ended up with (see below) to make sure it looks sane/acceptable to you? New hashes (assuming I didn't botch things and need even more fixup). [1/5] KVM: x86/mmu: Add helpers to return if KVM honors guest MTRRs https://github.com/kvm-x86/linux/commit/ec1d8217d59b [2/5] KVM: x86/mmu: Zap SPTEs when CR0.CD is toggled iff guest MTRRs are honored https://github.com/kvm-x86/linux/commit/40de16c10b9d [3/5] KVM: x86/mmu: Zap SPTEs on MTRR update iff guest MTRRs are honored https://github.com/kvm-x86/linux/commit/defc3fae8d0f [4/5] KVM: x86/mmu: Zap KVM TDP when noncoherent DMA assignment starts/stops https://github.com/kvm-x86/linux/commit/b344d331adeb [5/5] KVM: VMX: drop IPAT in memtype when CD=1 for KVM_X86_QUIRK_CD_NW_CLEARED https://github.com/kvm-x86/linux/commit/a4d14445c47d commit ec1d8217d59bd7cb03ae4e80551fee987be98a4e Author: Yan Zhao <yan.y.zhao@intel.com> Date: Fri Jul 14 14:50:06 2023 +0800 KVM: x86/mmu: Add helpers to return if KVM honors guest MTRRs Add helpers to check if KVM honors guest MTRRs instead of open coding the logic in kvm_tdp_page_fault(). Future fixes and cleanups will also need to determine if KVM should honor guest MTRRs, e.g. for CR0.CD toggling and and non-coherent DMA transitions. Provide an inner helper, __kvm_mmu_honors_guest_mtrrs(), so that KVM can if guest MTRRs were honored when stopping non-coherent DMA. Note, there is no need to explicitly check that TDP is enabled, KVM clears shadow_memtype_mask when TDP is disabled, i.e. it's non-zero if and only if EPT is enabled. Suggested-by: Sean Christopherson <seanjc@google.com> Signed-off-by: Yan Zhao <yan.y.zhao@intel.com> Link: https://lore.kernel.org/r/20230714065006.20201-1-yan.y.zhao@intel.com Link: https://lore.kernel.org/r/20230714065043.20258-1-yan.y.zhao@intel.com [sean: squash into a one patch, drop explicit TDP check massage changelog] Signed-off-by: Sean Christopherson <seanjc@google.com> diff --git a/arch/x86/kvm/mmu.h b/arch/x86/kvm/mmu.h index 253fb2093d5d..bb8c86eefac0 100644 --- a/arch/x86/kvm/mmu.h +++ b/arch/x86/kvm/mmu.h @@ -237,6 +237,13 @@ static inline u8 permission_fault(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu, return -(u32)fault & errcode; } +bool __kvm_mmu_honors_guest_mtrrs(bool vm_has_noncoherent_dma); + +static inline bool kvm_mmu_honors_guest_mtrrs(struct kvm *kvm) +{ + return __kvm_mmu_honors_guest_mtrrs(kvm_arch_has_noncoherent_dma(kvm)); +} + void kvm_zap_gfn_range(struct kvm *kvm, gfn_t gfn_start, gfn_t gfn_end); int kvm_arch_write_log_dirty(struct kvm_vcpu *vcpu); diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c index f7901cb4d2fa..5d3dc7119e57 100644 --- a/arch/x86/kvm/mmu/mmu.c +++ b/arch/x86/kvm/mmu/mmu.c @@ -4479,21 +4479,28 @@ static int kvm_tdp_mmu_page_fault(struct kvm_vcpu *vcpu, } #endif +bool __kvm_mmu_honors_guest_mtrrs(bool vm_has_noncoherent_dma) +{ + /* + * If host MTRRs are ignored (shadow_memtype_mask is non-zero), and the + * VM has non-coherent DMA (DMA doesn't snoop CPU caches), KVM's ABI is + * to honor the memtype from the guest's MTRRs so that guest accesses + * to memory that is DMA'd aren't cached against the guest's wishes. + * + * Note, KVM may still ultimately ignore guest MTRRs for certain PFNs, + * e.g. KVM will force UC memtype for host MMIO. + */ + return vm_has_noncoherent_dma && shadow_memtype_mask; +} + int kvm_tdp_page_fault(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault) { /* * If the guest's MTRRs may be used to compute the "real" memtype, * restrict the mapping level to ensure KVM uses a consistent memtype - * across the entire mapping. If the host MTRRs are ignored by TDP - * (shadow_memtype_mask is non-zero), and the VM has non-coherent DMA - * (DMA doesn't snoop CPU caches), KVM's ABI is to honor the memtype - * from the guest's MTRRs so that guest accesses to memory that is - * DMA'd aren't cached against the guest's wishes. - * - * Note, KVM may still ultimately ignore guest MTRRs for certain PFNs, - * e.g. KVM will force UC memtype for host MMIO. + * across the entire mapping. */ - if (shadow_memtype_mask && kvm_arch_has_noncoherent_dma(vcpu->kvm)) { + if (kvm_mmu_honors_guest_mtrrs(vcpu->kvm)) { for ( ; fault->max_level > PG_LEVEL_4K; --fault->max_level) { int page_num = KVM_PAGES_PER_HPAGE(fault->max_level); gfn_t base = gfn_round_for_level(fault->gfn,
On Mon, Oct 09, 2023, Sean Christopherson wrote: > On Mon, Oct 09, 2023, Sean Christopherson wrote: > > On Sat, Oct 07, 2023, Like Xu wrote: > > > On 14/7/2023 2:50 pm, Yan Zhao wrote: > > > > diff --git a/arch/x86/kvm/mmu.h b/arch/x86/kvm/mmu.h > > > > index 92d5a1924fc1..38bd449226f6 100644 > > > > --- a/arch/x86/kvm/mmu.h > > > > +++ b/arch/x86/kvm/mmu.h > > > > @@ -235,6 +235,13 @@ static inline u8 permission_fault(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu, > > > > return -(u32)fault & errcode; > > > > } > > > > +bool __kvm_mmu_honors_guest_mtrrs(struct kvm *kvm, bool vm_has_noncoherent_dma); > > > > + > > > > +static inline bool kvm_mmu_honors_guest_mtrrs(struct kvm *kvm) > > > > +{ > > > > + return __kvm_mmu_honors_guest_mtrrs(kvm, kvm_arch_has_noncoherent_dma(kvm)); > > > > +} > > > > + > > > > void kvm_zap_gfn_range(struct kvm *kvm, gfn_t gfn_start, gfn_t gfn_end); > > > > int kvm_arch_write_log_dirty(struct kvm_vcpu *vcpu); > > > > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c > > > > index 1e5db621241f..b4f89f015c37 100644 > > > > --- a/arch/x86/kvm/mmu/mmu.c > > > > +++ b/arch/x86/kvm/mmu/mmu.c > > > > @@ -4516,6 +4516,21 @@ static int kvm_tdp_mmu_page_fault(struct kvm_vcpu *vcpu, > > > > } > > > > #endif > > > > +bool __kvm_mmu_honors_guest_mtrrs(struct kvm *kvm, bool vm_has_noncoherent_dma) > > > > > > According to the motivation provided in the comment, the function will no > > > longer need to be passed the parameter "struct kvm *kvm" but will rely on > > > the global parameters (plus vm_has_noncoherent_dma), removing "*kvm" ? > > > > Yeah, I'll fixup the commit to drop @kvm from the inner helper. Thanks! > > Gah, and I gave more bad advice when I suggested this idea. There's no need to > explicitly check tdp_enabled, as shadow_memtype_mask is set to zero if TDP is > disabled. And that must be the case, e.g. make_spte() would generate a corrupt > shadow_memtype_mask were non-zero on Intel with shadow paging. > > Yan, can you take a look at what I ended up with (see below) to make sure it > looks sane/acceptable to you? > > New hashes (assuming I didn't botch things and need even more fixup). Oof, today is not my day. I forgot to fix the missing "check" in the changelog that Yan reported. So *these* are the new hashes, barring yet another goof on my end. [1/5] KVM: x86/mmu: Add helpers to return if KVM honors guest MTRRs https://github.com/kvm-x86/linux/commit/1affe455d66d [2/5] KVM: x86/mmu: Zap SPTEs when CR0.CD is toggled iff guest MTRRs are honored https://github.com/kvm-x86/linux/commit/7a18c7c2b69a [3/5] KVM: x86/mmu: Zap SPTEs on MTRR update iff guest MTRRs are honored https://github.com/kvm-x86/linux/commit/9a3768191d95 [4/5] KVM: x86/mmu: Zap KVM TDP when noncoherent DMA assignment starts/stops https://github.com/kvm-x86/linux/commit/68c320298404 [5/5] KVM: VMX: drop IPAT in memtype when CD=1 for KVM_X86_QUIRK_CD_NW_CLEARED https://github.com/kvm-x86/linux/commit/8925b3194512
On Mon, Oct 09, 2023 at 02:27:16PM -0700, Sean Christopherson wrote: > On Mon, Oct 09, 2023, Sean Christopherson wrote: > > On Sat, Oct 07, 2023, Like Xu wrote: > > > On 14/7/2023 2:50 pm, Yan Zhao wrote: > > > > diff --git a/arch/x86/kvm/mmu.h b/arch/x86/kvm/mmu.h > > > > index 92d5a1924fc1..38bd449226f6 100644 > > > > --- a/arch/x86/kvm/mmu.h > > > > +++ b/arch/x86/kvm/mmu.h > > > > @@ -235,6 +235,13 @@ static inline u8 permission_fault(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu, > > > > return -(u32)fault & errcode; > > > > } > > > > +bool __kvm_mmu_honors_guest_mtrrs(struct kvm *kvm, bool vm_has_noncoherent_dma); > > > > + > > > > +static inline bool kvm_mmu_honors_guest_mtrrs(struct kvm *kvm) > > > > +{ > > > > + return __kvm_mmu_honors_guest_mtrrs(kvm, kvm_arch_has_noncoherent_dma(kvm)); > > > > +} > > > > + > > > > void kvm_zap_gfn_range(struct kvm *kvm, gfn_t gfn_start, gfn_t gfn_end); > > > > int kvm_arch_write_log_dirty(struct kvm_vcpu *vcpu); > > > > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c > > > > index 1e5db621241f..b4f89f015c37 100644 > > > > --- a/arch/x86/kvm/mmu/mmu.c > > > > +++ b/arch/x86/kvm/mmu/mmu.c > > > > @@ -4516,6 +4516,21 @@ static int kvm_tdp_mmu_page_fault(struct kvm_vcpu *vcpu, > > > > } > > > > #endif > > > > +bool __kvm_mmu_honors_guest_mtrrs(struct kvm *kvm, bool vm_has_noncoherent_dma) > > > > > > According to the motivation provided in the comment, the function will no > > > longer need to be passed the parameter "struct kvm *kvm" but will rely on > > > the global parameters (plus vm_has_noncoherent_dma), removing "*kvm" ? > > > > Yeah, I'll fixup the commit to drop @kvm from the inner helper. Thanks! > > Gah, and I gave more bad advice when I suggested this idea. There's no need to > explicitly check tdp_enabled, as shadow_memtype_mask is set to zero if TDP is > disabled. And that must be the case, e.g. make_spte() would generate a corrupt > shadow_memtype_mask were non-zero on Intel with shadow paging. > > Yan, can you take a look at what I ended up with (see below) to make sure it > looks sane/acceptable to you? yes, tested working on my side. I think why we added the checking of tdp_enabled was because of the existing check in patch 3. As noncoherent DMAs checking is not on hot paths, the previous double checking is also good :) BTW, as param "kvm" is now removed from the helper, better to remove the word "second" in comment in patch 4, i.e. - * So, specify the second parameter as true here to indicate - * non-coherent DMAs are/were involved and TDP zap might be - * necessary. + * So, specify the parameter as true here to indicate non-coherent + * DMAs are/were involved and TDP zap might be necessary. Sorry and thanks a lot for helps on this series!
On Tue, Oct 10, 2023, Yan Zhao wrote: > BTW, as param "kvm" is now removed from the helper, better to remove the word > "second" in comment in patch 4, i.e. > > - * So, specify the second parameter as true here to indicate > - * non-coherent DMAs are/were involved and TDP zap might be > - * necessary. > + * So, specify the parameter as true here to indicate non-coherent > + * DMAs are/were involved and TDP zap might be necessary. > > Sorry and thanks a lot for helps on this series! Heh, don't be sorry, it's not your fault I can't get this quite right. Fixed up yet again, hopefully for the last time. This is what I ended up with for the comment: /* * Non-coherent DMA assignment and de-assignment will affect * whether KVM honors guest MTRRs and cause changes in memtypes * in TDP. * So, pass %true unconditionally to indicate non-coherent DMA was, * or will be involved, and that zapping SPTEs might be necessary. */ and the hashes: [1/5] KVM: x86/mmu: Add helpers to return if KVM honors guest MTRRs https://github.com/kvm-x86/linux/commit/1affe455d66d [2/5] KVM: x86/mmu: Zap SPTEs when CR0.CD is toggled iff guest MTRRs are honored https://github.com/kvm-x86/linux/commit/7a18c7c2b69a [3/5] KVM: x86/mmu: Zap SPTEs on MTRR update iff guest MTRRs are honored https://github.com/kvm-x86/linux/commit/9a3768191d95 [4/5] KVM: x86/mmu: Zap KVM TDP when noncoherent DMA assignment starts/stops https://github.com/kvm-x86/linux/commit/362ff6dca541 [5/5] KVM: VMX: drop IPAT in memtype when CD=1 for KVM_X86_QUIRK_CD_NW_CLEARED https://github.com/kvm-x86/linux/commit/c9f65a3f2d92
On Tue, Oct 10, 2023 at 05:08:08PM -0700, Sean Christopherson wrote: > On Tue, Oct 10, 2023, Yan Zhao wrote: > > BTW, as param "kvm" is now removed from the helper, better to remove the word > > "second" in comment in patch 4, i.e. > > > > - * So, specify the second parameter as true here to indicate > > - * non-coherent DMAs are/were involved and TDP zap might be > > - * necessary. > > + * So, specify the parameter as true here to indicate non-coherent > > + * DMAs are/were involved and TDP zap might be necessary. > > > > Sorry and thanks a lot for helps on this series! > > Heh, don't be sorry, it's not your fault I can't get this quite right. Fixed > up yet again, hopefully for the last time. This is what I ended up with for the > comment: > > /* > * Non-coherent DMA assignment and de-assignment will affect > * whether KVM honors guest MTRRs and cause changes in memtypes > * in TDP. > * So, pass %true unconditionally to indicate non-coherent DMA was, > * or will be involved, and that zapping SPTEs might be necessary. > */ > > and the hashes: > > [1/5] KVM: x86/mmu: Add helpers to return if KVM honors guest MTRRs > https://github.com/kvm-x86/linux/commit/1affe455d66d > [2/5] KVM: x86/mmu: Zap SPTEs when CR0.CD is toggled iff guest MTRRs are honored > https://github.com/kvm-x86/linux/commit/7a18c7c2b69a > [3/5] KVM: x86/mmu: Zap SPTEs on MTRR update iff guest MTRRs are honored > https://github.com/kvm-x86/linux/commit/9a3768191d95 > [4/5] KVM: x86/mmu: Zap KVM TDP when noncoherent DMA assignment starts/stops > https://github.com/kvm-x86/linux/commit/362ff6dca541 > [5/5] KVM: VMX: drop IPAT in memtype when CD=1 for KVM_X86_QUIRK_CD_NW_CLEARED > https://github.com/kvm-x86/linux/commit/c9f65a3f2d92 Looks good to me, thanks!
diff --git a/arch/x86/kvm/mmu.h b/arch/x86/kvm/mmu.h index 92d5a1924fc1..38bd449226f6 100644 --- a/arch/x86/kvm/mmu.h +++ b/arch/x86/kvm/mmu.h @@ -235,6 +235,13 @@ static inline u8 permission_fault(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu, return -(u32)fault & errcode; } +bool __kvm_mmu_honors_guest_mtrrs(struct kvm *kvm, bool vm_has_noncoherent_dma); + +static inline bool kvm_mmu_honors_guest_mtrrs(struct kvm *kvm) +{ + return __kvm_mmu_honors_guest_mtrrs(kvm, kvm_arch_has_noncoherent_dma(kvm)); +} + void kvm_zap_gfn_range(struct kvm *kvm, gfn_t gfn_start, gfn_t gfn_end); int kvm_arch_write_log_dirty(struct kvm_vcpu *vcpu); diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c index 1e5db621241f..b4f89f015c37 100644 --- a/arch/x86/kvm/mmu/mmu.c +++ b/arch/x86/kvm/mmu/mmu.c @@ -4516,6 +4516,21 @@ static int kvm_tdp_mmu_page_fault(struct kvm_vcpu *vcpu, } #endif +bool __kvm_mmu_honors_guest_mtrrs(struct kvm *kvm, bool vm_has_noncoherent_dma) +{ + /* + * If the TDP is enabled, the host MTRRs are ignored by TDP + * (shadow_memtype_mask is non-zero), and the VM has non-coherent DMA + * (DMA doesn't snoop CPU caches), KVM's ABI is to honor the memtype + * from the guest's MTRRs so that guest accesses to memory that is + * DMA'd aren't cached against the guest's wishes. + * + * Note, KVM may still ultimately ignore guest MTRRs for certain PFNs, + * e.g. KVM will force UC memtype for host MMIO. + */ + return vm_has_noncoherent_dma && tdp_enabled && shadow_memtype_mask; +} + int kvm_tdp_page_fault(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault) { /*