Re: [PATCH 06/21] KVM: x86/mmu: Track shadow MMIO value on a per-VM basis

Binbin Wu <binbin.wu@xxxxxxxxxxxxxxx> · Tue, 5 Mar 2024 16:35:34 +0800

On 2/28/2024 7:20 AM, Paolo Bonzini wrote:
From: Sean Christopherson <seanjc@xxxxxxxxxx>

TDX will use a different shadow PTE entry value for MMIO from VMX.  Add
members to kvm_arch and track value for MMIO per-VM instead of global

Nit: members -> a member, since only 'shadow_mmio_value' is added.

variables.  By using the per-VM EPT entry value for MMIO, the existing VMX
logic is kept working.  Introduce a separate setter function so that guest
TD can override later.

Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
Signed-off-by: Isaku Yamahata <isaku.yamahata@xxxxxxxxx>
Message-Id: <229a18434e5d83f45b1fcd7bf1544d79db1becb6.1705965635.git.isaku.yamahata@xxxxxxxxx>
Signed-off-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>

Reviewed-by: Binbin Wu <binbin.wu@xxxxxxxxxxxxxxx>

---
  arch/x86/include/asm/kvm_host.h |  2 ++
  arch/x86/kvm/mmu.h              |  1 +
  arch/x86/kvm/mmu/mmu.c          |  8 +++++---
  arch/x86/kvm/mmu/spte.c         | 10 ++++++++--
  arch/x86/kvm/mmu/spte.h         |  4 ++--
  arch/x86/kvm/mmu/tdp_mmu.c      |  6 +++---
  6 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 85dc0f7d09e3..a4514c2ef0ec 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1313,6 +1313,8 @@ struct kvm_arch {
  	 */
  	spinlock_t mmu_unsync_pages_lock;
  
+	u64 shadow_mmio_value;
+
  	struct iommu_domain *iommu_domain;
  	bool iommu_noncoherent;
  #define __KVM_HAVE_ARCH_NONCOHERENT_DMA
diff --git a/arch/x86/kvm/mmu.h b/arch/x86/kvm/mmu.h
index 60f21bb4c27b..2c54ba5b0a28 100644
--- a/arch/x86/kvm/mmu.h
+++ b/arch/x86/kvm/mmu.h
@@ -101,6 +101,7 @@ static inline u8 kvm_get_shadow_phys_bits(void)
  }
  
  void kvm_mmu_set_mmio_spte_mask(u64 mmio_value, u64 mmio_mask, u64 access_mask);
+void kvm_mmu_set_mmio_spte_value(struct kvm *kvm, u64 mmio_value);
  void kvm_mmu_set_me_spte_mask(u64 me_value, u64 me_mask);
  void kvm_mmu_set_ept_masks(bool has_ad_bits, bool has_exec_only);
  
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index b5baf11359ad..195e46a1f00f 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -2515,7 +2515,7 @@ static int mmu_page_zap_pte(struct kvm *kvm, struct kvm_mmu_page *sp,
  				return kvm_mmu_prepare_zap_page(kvm, child,
  								invalid_list);
  		}
-	} else if (is_mmio_spte(pte)) {
+	} else if (is_mmio_spte(kvm, pte)) {
  		mmu_spte_clear_no_track(spte);
  	}
  	return 0;
@@ -4197,7 +4197,7 @@ static int handle_mmio_page_fault(struct kvm_vcpu *vcpu, u64 addr, bool direct)
  	if (WARN_ON_ONCE(reserved))
  		return -EINVAL;
  
-	if (is_mmio_spte(spte)) {
+	if (is_mmio_spte(vcpu->kvm, spte)) {
  		gfn_t gfn = get_mmio_spte_gfn(spte);
  		unsigned int access = get_mmio_spte_access(spte);
  
@@ -4813,7 +4813,7 @@ EXPORT_SYMBOL_GPL(kvm_mmu_new_pgd);
  static bool sync_mmio_spte(struct kvm_vcpu *vcpu, u64 *sptep, gfn_t gfn,
  			   unsigned int access)
  {
-	if (unlikely(is_mmio_spte(*sptep))) {
+	if (unlikely(is_mmio_spte(vcpu->kvm, *sptep))) {
  		if (gfn != get_mmio_spte_gfn(*sptep)) {
  			mmu_spte_clear_no_track(sptep);
  			return true;
@@ -6320,6 +6320,8 @@ static bool kvm_has_zapped_obsolete_pages(struct kvm *kvm)
  
  void kvm_mmu_init_vm(struct kvm *kvm)
  {
+
+	kvm->arch.shadow_mmio_value = shadow_mmio_value;
  	INIT_LIST_HEAD(&kvm->arch.active_mmu_pages);
  	INIT_LIST_HEAD(&kvm->arch.zapped_obsolete_pages);
  	INIT_LIST_HEAD(&kvm->arch.possible_nx_huge_pages);
diff --git a/arch/x86/kvm/mmu/spte.c b/arch/x86/kvm/mmu/spte.c
index 02a466de2991..318135daf685 100644
--- a/arch/x86/kvm/mmu/spte.c
+++ b/arch/x86/kvm/mmu/spte.c
@@ -74,10 +74,10 @@ u64 make_mmio_spte(struct kvm_vcpu *vcpu, u64 gfn, unsigned int access)
  	u64 spte = generation_mmio_spte_mask(gen);
  	u64 gpa = gfn << PAGE_SHIFT;
  
-	WARN_ON_ONCE(!shadow_mmio_value);
+	WARN_ON_ONCE(!vcpu->kvm->arch.shadow_mmio_value);
  
  	access &= shadow_mmio_access_mask;
-	spte |= shadow_mmio_value | access;
+	spte |= vcpu->kvm->arch.shadow_mmio_value | access;
  	spte |= gpa | shadow_nonpresent_or_rsvd_mask;
  	spte |= (gpa & shadow_nonpresent_or_rsvd_mask)
  		<< SHADOW_NONPRESENT_OR_RSVD_MASK_LEN;
@@ -411,6 +411,12 @@ void kvm_mmu_set_mmio_spte_mask(u64 mmio_value, u64 mmio_mask, u64 access_mask)
  }
  EXPORT_SYMBOL_GPL(kvm_mmu_set_mmio_spte_mask);
  
+void kvm_mmu_set_mmio_spte_value(struct kvm *kvm, u64 mmio_value)
+{
+	kvm->arch.shadow_mmio_value = mmio_value;
+}
+EXPORT_SYMBOL_GPL(kvm_mmu_set_mmio_spte_value);
+
  void kvm_mmu_set_me_spte_mask(u64 me_value, u64 me_mask)
  {
  	/* shadow_me_value must be a subset of shadow_me_mask */
diff --git a/arch/x86/kvm/mmu/spte.h b/arch/x86/kvm/mmu/spte.h
index 26bc95bbc962..1a163aee9ec6 100644
--- a/arch/x86/kvm/mmu/spte.h
+++ b/arch/x86/kvm/mmu/spte.h
@@ -264,9 +264,9 @@ static inline struct kvm_mmu_page *root_to_sp(hpa_t root)
  	return spte_to_child_sp(root);
  }
  
-static inline bool is_mmio_spte(u64 spte)
+static inline bool is_mmio_spte(struct kvm *kvm, u64 spte)
  {
-	return (spte & shadow_mmio_mask) == shadow_mmio_value &&
+	return (spte & shadow_mmio_mask) == kvm->arch.shadow_mmio_value &&
  	       likely(enable_mmio_caching);
  }
  
diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
index c8a4d92497b4..d15c44a8e123 100644
--- a/arch/x86/kvm/mmu/tdp_mmu.c
+++ b/arch/x86/kvm/mmu/tdp_mmu.c
@@ -495,8 +495,8 @@ static void handle_changed_spte(struct kvm *kvm, int as_id, gfn_t gfn,
  		 * impact the guest since both the former and current SPTEs
  		 * are nonpresent.
  		 */
-		if (WARN_ON_ONCE(!is_mmio_spte(old_spte) &&
-				 !is_mmio_spte(new_spte) &&
+		if (WARN_ON_ONCE(!is_mmio_spte(kvm, old_spte) &&
+				 !is_mmio_spte(kvm, new_spte) &&
  				 !is_removed_spte(new_spte)))
  			pr_err("Unexpected SPTE change! Nonpresent SPTEs\n"
  			       "should not be replaced with another,\n"
@@ -1028,7 +1028,7 @@ static int tdp_mmu_map_handle_target_level(struct kvm_vcpu *vcpu,
  	}
  
  	/* If a MMIO SPTE is installed, the MMIO will need to be emulated. */
-	if (unlikely(is_mmio_spte(new_spte))) {
+	if (unlikely(is_mmio_spte(vcpu->kvm, new_spte))) {
  		vcpu->stat.pf_mmio_spte_created++;
  		trace_mark_mmio_spte(rcu_dereference(iter->sptep), iter->gfn,
  				     new_spte);








