From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 49821C7EE29 for ; Tue, 6 Jun 2023 04:59:35 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234187AbjFFE7b (ORCPT ); Tue, 6 Jun 2023 00:59:31 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:38378 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229693AbjFFE72 (ORCPT ); Tue, 6 Jun 2023 00:59:28 -0400 Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 85940196; Mon, 5 Jun 2023 21:59:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1686027567; x=1717563567; h=date:from:to:cc:subject:message-id:references: mime-version:in-reply-to; bh=7YqIXOEFk6+LSNMwIOYXhRVCF5tOS5Mo1Nr0nwPD3FU=; b=ZJqWpqQMpMOIRNo2bansYhzGHkwC5D4wRSJcKUeIrpY8XNVGr9cT/epx Xd/kqU3F0r5ew0Ooa8JhU1RQH32xNp45hUWqVTBJwi+uTJmyEJsfMMGRx G8V7IW7FZSTr/ze4DbEx+vilt0xAIZu75wPSxXXmK7Vvk5H231GOjwAM5 NJnxNxoVtc8tsrWWLBD40GOMdHiHU06oDNQh3t+YCqErEDTFrQ1xLfvGO 7h/4LpBxwwqM1RQzQYQ9evOSOzmVv2Sn3OR6U4yn0N2iFMUBgT6xNpzxs f3upuuc+6diH3sLl43bU9hP49asi8RZr4kGtxPCtNK0toWvf9XHuJl3/u g==; X-IronPort-AV: E=McAfee;i="6600,9927,10732"; a="336920624" X-IronPort-AV: E=Sophos;i="6.00,219,1681196400"; d="scan'208";a="336920624" Received: from fmsmga007.fm.intel.com ([10.253.24.52]) by fmsmga107.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 05 Jun 2023 21:59:27 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10732"; a="712056414" X-IronPort-AV: E=Sophos;i="6.00,219,1681196400"; d="scan'208";a="712056414" Received: from yy-desk-7060.sh.intel.com (HELO localhost) ([10.239.159.76]) by fmsmga007.fm.intel.com with ESMTP; 05 Jun 2023 21:59:24 -0700 Date: Tue, 6 Jun 2023 12:59:23 +0800 From: Yuan Yao To: isaku.yamahata@intel.com Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org, isaku.yamahata@gmail.com, Paolo Bonzini , erdemaktas@google.com, Sean Christopherson , Sagi Shahar , David Matlack , Kai Huang , Zhi Wang , chen.bo@intel.com, Sean Christopherson Subject: Re: [PATCH v14 031/113] KVM: x86/mmu: Replace hardcoded value 0 for the initial value for SPTE Message-ID: <20230606045923.ol5kjhagiimqksmn@yy-desk-7060> References: <8b4f21e2fada944d041ffee0f27d527e0e447cbb.1685333727.git.isaku.yamahata@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <8b4f21e2fada944d041ffee0f27d527e0e447cbb.1685333727.git.isaku.yamahata@intel.com> User-Agent: NeoMutt/20171215 Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sun, May 28, 2023 at 09:19:13PM -0700, isaku.yamahata@intel.com wrote: > From: Isaku Yamahata > > The TDX support will need the "suppress #VE" bit (bit 63) set as the > initial value for SPTE. To reduce code change size, introduce a new macro > SHADOW_NONPRESENT_VALUE for the initial value for the shadow page table > entry (SPTE) and replace hard-coded value 0 for it. Initialize shadow page > tables with their value. > > The plan is to unconditionally set the "suppress #VE" bit for both AMD and > Intel as: 1) AMD hardware uses the bit 63 as NX for present SPTE and > ignored for non-present SPTE; 2) for conventional VMX guests, KVM never > enables the "EPT-violation #VE" in VMCS control and "suppress #VE" bit is > ignored by hardware. > > Signed-off-by: Sean Christopherson > Signed-off-by: Isaku Yamahata > --- > arch/x86/kvm/mmu/mmu.c | 20 +++++++++++++++----- > arch/x86/kvm/mmu/paging_tmpl.h | 2 +- > arch/x86/kvm/mmu/spte.h | 2 ++ > arch/x86/kvm/mmu/tdp_mmu.c | 14 +++++++------- > 4 files changed, 25 insertions(+), 13 deletions(-) > > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c > index dc2b9a2f717c..1b6fd4434e96 100644 > --- a/arch/x86/kvm/mmu/mmu.c > +++ b/arch/x86/kvm/mmu/mmu.c > @@ -576,9 +576,9 @@ static u64 mmu_spte_clear_track_bits(struct kvm *kvm, u64 *sptep) > > if (!is_shadow_present_pte(old_spte) || > !spte_has_volatile_bits(old_spte)) > - __update_clear_spte_fast(sptep, 0ull); > + __update_clear_spte_fast(sptep, SHADOW_NONPRESENT_VALUE); > else > - old_spte = __update_clear_spte_slow(sptep, 0ull); > + old_spte = __update_clear_spte_slow(sptep, SHADOW_NONPRESENT_VALUE); > > if (!is_shadow_present_pte(old_spte)) > return old_spte; > @@ -612,7 +612,7 @@ static u64 mmu_spte_clear_track_bits(struct kvm *kvm, u64 *sptep) > */ > static void mmu_spte_clear_no_track(u64 *sptep) > { > - __update_clear_spte_fast(sptep, 0ull); > + __update_clear_spte_fast(sptep, SHADOW_NONPRESENT_VALUE); > } > > static u64 mmu_spte_get_lockless(u64 *sptep) > @@ -1969,7 +1969,8 @@ static bool kvm_sync_page_check(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp) > > static int kvm_sync_spte(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp, int i) > { > - if (!sp->spt[i]) > + /* sp->spt[i] has initial value of shadow page table allocation */ > + if (sp->spt[i] != SHADOW_NONPRESENT_VALUE) This should be "sp->spt[i] == SHADOW_NONPRESENT_VALUE" ? Looks all present spt[i] are skipped without sync for shadow paging. > return 0; > > return vcpu->arch.mmu->sync_spte(vcpu, sp, i); > @@ -6120,7 +6121,16 @@ int kvm_mmu_create(struct kvm_vcpu *vcpu) > vcpu->arch.mmu_page_header_cache.kmem_cache = mmu_page_header_cache; > vcpu->arch.mmu_page_header_cache.gfp_zero = __GFP_ZERO; > > - vcpu->arch.mmu_shadow_page_cache.gfp_zero = __GFP_ZERO; > + /* > + * When X86_64, initial SEPT entries are initialized with > + * SHADOW_NONPRESENT_VALUE. Otherwise zeroed. See > + * mmu_memory_cache_alloc_obj(). > + */ > + if (IS_ENABLED(CONFIG_X86_64)) > + vcpu->arch.mmu_shadow_page_cache.init_value = > + SHADOW_NONPRESENT_VALUE; > + if (!vcpu->arch.mmu_shadow_page_cache.init_value) > + vcpu->arch.mmu_shadow_page_cache.gfp_zero = __GFP_ZERO; > > vcpu->arch.mmu = &vcpu->arch.root_mmu; > vcpu->arch.walk_mmu = &vcpu->arch.root_mmu; > diff --git a/arch/x86/kvm/mmu/paging_tmpl.h b/arch/x86/kvm/mmu/paging_tmpl.h > index 0662e0278e70..ef8124bd2f11 100644 > --- a/arch/x86/kvm/mmu/paging_tmpl.h > +++ b/arch/x86/kvm/mmu/paging_tmpl.h > @@ -892,7 +892,7 @@ static int FNAME(sync_spte)(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp, int > gpa_t pte_gpa; > gfn_t gfn; > > - if (WARN_ON_ONCE(!sp->spt[i])) > + if (WARN_ON_ONCE(sp->spt[i] == SHADOW_NONPRESENT_VALUE)) > return 0; > > first_pte_gpa = FNAME(get_level1_sp_gpa)(sp); > diff --git a/arch/x86/kvm/mmu/spte.h b/arch/x86/kvm/mmu/spte.h > index 1279db2eab44..a99eb7d4ae5d 100644 > --- a/arch/x86/kvm/mmu/spte.h > +++ b/arch/x86/kvm/mmu/spte.h > @@ -148,6 +148,8 @@ static_assert(MMIO_SPTE_GEN_LOW_BITS == 8 && MMIO_SPTE_GEN_HIGH_BITS == 11); > > #define MMIO_SPTE_GEN_MASK GENMASK_ULL(MMIO_SPTE_GEN_LOW_BITS + MMIO_SPTE_GEN_HIGH_BITS - 1, 0) > > +#define SHADOW_NONPRESENT_VALUE 0ULL > + > extern u64 __read_mostly shadow_host_writable_mask; > extern u64 __read_mostly shadow_mmu_writable_mask; > extern u64 __read_mostly shadow_nx_mask; > diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c > index 3000ef6d79ea..ddd995885dd3 100644 > --- a/arch/x86/kvm/mmu/tdp_mmu.c > +++ b/arch/x86/kvm/mmu/tdp_mmu.c > @@ -627,7 +627,7 @@ static inline int tdp_mmu_zap_spte_atomic(struct kvm *kvm, > * here since the SPTE is going from non-present to non-present. Use > * the raw write helper to avoid an unnecessary check on volatile bits. > */ > - __kvm_tdp_mmu_write_spte(iter->sptep, 0); > + __kvm_tdp_mmu_write_spte(iter->sptep, SHADOW_NONPRESENT_VALUE); > > return 0; > } > @@ -764,8 +764,8 @@ static void __tdp_mmu_zap_root(struct kvm *kvm, struct kvm_mmu_page *root, > continue; > > if (!shared) > - tdp_mmu_iter_set_spte(kvm, &iter, 0); > - else if (tdp_mmu_set_spte_atomic(kvm, &iter, 0)) > + tdp_mmu_iter_set_spte(kvm, &iter, SHADOW_NONPRESENT_VALUE); > + else if (tdp_mmu_set_spte_atomic(kvm, &iter, SHADOW_NONPRESENT_VALUE)) > goto retry; > } > } > @@ -821,8 +821,8 @@ bool kvm_tdp_mmu_zap_sp(struct kvm *kvm, struct kvm_mmu_page *sp) > if (WARN_ON_ONCE(!is_shadow_present_pte(old_spte))) > return false; > > - tdp_mmu_set_spte(kvm, kvm_mmu_page_as_id(sp), sp->ptep, old_spte, 0, > - sp->gfn, sp->role.level + 1); > + tdp_mmu_set_spte(kvm, kvm_mmu_page_as_id(sp), sp->ptep, old_spte, > + SHADOW_NONPRESENT_VALUE, sp->gfn, sp->role.level + 1); > > return true; > } > @@ -856,7 +856,7 @@ static bool tdp_mmu_zap_leafs(struct kvm *kvm, struct kvm_mmu_page *root, > !is_last_spte(iter.old_spte, iter.level)) > continue; > > - tdp_mmu_iter_set_spte(kvm, &iter, 0); > + tdp_mmu_iter_set_spte(kvm, &iter, SHADOW_NONPRESENT_VALUE); > flush = true; > } > > @@ -1250,7 +1250,7 @@ static bool set_spte_gfn(struct kvm *kvm, struct tdp_iter *iter, > * invariant that the PFN of a present * leaf SPTE can never change. > * See handle_changed_spte(). > */ > - tdp_mmu_iter_set_spte(kvm, iter, 0); > + tdp_mmu_iter_set_spte(kvm, iter, SHADOW_NONPRESENT_VALUE); > > if (!pte_write(range->pte)) { > new_spte = kvm_mmu_changed_pte_notifier_make_spte(iter->old_spte, > -- > 2.25.1 >