[PATCH 0/3] KVM: x86: Introduce quirk KVM_X86_QUIRK_EPT_IGNORE_GUEST_PAT

[PATCH 0/3] KVM: x86: Introduce quirk KVM_X86_QUIRK_EPT_IGNORE_GUEST_PAT

[Yan Zhao]

This series introduces a quirk KVM_X86_QUIRK_EPT_IGNORE_GUEST_PAT as
suggested by Paolo and Sean [1].

The purpose of introducing this quirk is to allow KVM to honor guest PAT on
Intel platforms with self-snoop feature. This support was previously
reverted by commit 9d70f3fec144 ("Revert "KVM: VMX: Always honor guest PAT
on CPUs that support self-snoop"") due to a reported broken of an old bochs
driver which incorrectly set memory type to UC but did not expect that UC
would be very slow on certain Intel platforms.

Sean previously suggested to bottom out if the UC slowness issue is working
as intended so that we can enable the quirk only when the VMs are affected
by the old unmodifiable guests [2]. After consulting with CPU architects,
it's told that this behavior is expected on ICX/SPR Xeon platforms due to
the snooping implementation.

So, implement the quirk such that KVM enables it by default on all Intel
non-TDX platforms while having the quirk explicitly reference the old
unmodifiable guests that rely on KVM to force memory type to WB. Newer
userspace can disable the quirk by default and only leave it enabled if an
old unmodifiable guest is an concern.

The quirk is platform-specific valid, available only on Intel non-TDX
platforms. It is absent on Intel TDX and AMD platforms, where KVM always
honors guest PAT.

Patch 1 does the preparation of making quirks platform-specific valid.
Patch 2 makes the quirk to be present on Intel and absent on AMD.
Patch 3 makes the quirk to be absent on Intel TDX and self-snoop a hard
        dependency to enable TDX [3].
        As a new platform, TDX is always running on CPUs with self-snoop
        feature. It has no worry to break old yet unmodifiable guests.
        Simply have KVM always honor guest PAT on TDX enabled platforms.
        Attaching/detaching non-coherent DMA devices would not lead to
        mirrored EPTs being zapped for TDs then. A previous attempt for
        this purpose is at [4].


This series is based on kvm-coco-queue. It was supposed to be included in
TDX's "the rest" section. We post it separately to start review earlier.

Patches 1 and 2 are changes to the generic code, which can also be applied
to kvm/queue. A proposal is to have them go into kvm/queue and we rebase on
that.

Patch 3 can be included in TDX's "the rest" section in the end.

Thanks
Yan

[1] https://lore.kernel.org/kvm/CABgObfa=t1dGR5cEhbUqVWTD03vZR4QrzEUgHxq+3JJ7YsA9pA@mail.gmail.com
[2] https://lore.kernel.org/kvm/Zt8cgUASZCN6gP8H@google.com
[3] https://lore.kernel.org/kvm/ZuBSNS33_ck-w6-9@google.com
[4] https://lore.kernel.org/kvm/20241115084600.12174-1-yan.y.zhao@intel.com


Yan Zhao (3):
  KVM: x86: Introduce supported_quirks for platform-specific valid
    quirks
  KVM: x86: Introduce Intel specific quirk
    KVM_X86_QUIRK_EPT_IGNORE_GUEST_PAT
  KVM: TDX: Always honor guest PAT on TDX enabled platforms

 Documentation/virt/kvm/api.rst  | 30 +++++++++++++++++++++++++
 arch/x86/include/asm/kvm_host.h |  2 +-
 arch/x86/include/uapi/asm/kvm.h |  1 +
 arch/x86/kvm/mmu.h              |  2 +-
 arch/x86/kvm/mmu/mmu.c          | 14 +++++++-----
 arch/x86/kvm/vmx/main.c         |  1 +
 arch/x86/kvm/vmx/tdx.c          |  5 +++++
 arch/x86/kvm/vmx/vmx.c          | 39 +++++++++++++++++++++++++++------
 arch/x86/kvm/x86.c              |  7 +++---
 arch/x86/kvm/x86.h              | 12 +++++-----
 10 files changed, 91 insertions(+), 22 deletions(-)

-- 
2.43.2

[PATCH 1/3] KVM: x86: Introduce supported_quirks for platform-specific valid quirks

[Yan Zhao]

Introduce supported_quirks in kvm_caps to store platform-specific valid
quirks.

Rename KVM_X86_VALID_QUIRKS to KVM_X86_VALID_QUIRKS_COMMON, representing
valid quirks common to all x86 platforms. Initialize
kvm_caps.supported_quirks to KVM_X86_VALID_QUIRKS_COMMON in the common
vendor initializer kvm_x86_vendor_init().

Use kvm_caps.supported_quirks to respond to user queries about valid quirks
and to mask out unsupported quirks provided by the user.

In kvm_check_has_quirk(), in additional to check if a quirk is not
explicitly disabled by the user, also verify if the quirk is supported by
the platform. This ensures KVM does not treat a quirk as enabled if it's
not explicitly disabled by the user but is outside the platform supported
mask.

This is a preparation for introducing quirks specific to certain platforms,
e.g., quirks present only on Intel platforms and not on AMD.

No functional changes intended.

Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
---
 arch/x86/include/asm/kvm_host.h |  2 +-
 arch/x86/kvm/x86.c              |  5 +++--
 arch/x86/kvm/x86.h              | 12 +++++++-----
 3 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 089cf2c82414..8d15e604613b 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -2409,7 +2409,7 @@ int memslot_rmap_alloc(struct kvm_memory_slot *slot, unsigned long npages);
 #define KVM_CLOCK_VALID_FLAGS						\
 	(KVM_CLOCK_TSC_STABLE | KVM_CLOCK_REALTIME | KVM_CLOCK_HOST_TSC)
 
-#define KVM_X86_VALID_QUIRKS			\
+#define KVM_X86_VALID_QUIRKS_COMMON		\
 	(KVM_X86_QUIRK_LINT0_REENABLED |	\
 	 KVM_X86_QUIRK_CD_NW_CLEARED |		\
 	 KVM_X86_QUIRK_LAPIC_MMIO_HOLE |	\
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 3078e09fc841..4f1b73620c6a 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4782,7 +4782,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
 		r = enable_pmu ? KVM_CAP_PMU_VALID_MASK : 0;
 		break;
 	case KVM_CAP_DISABLE_QUIRKS2:
-		r = KVM_X86_VALID_QUIRKS;
+		r = kvm_caps.supported_quirks;
 		break;
 	case KVM_CAP_X86_NOTIFY_VMEXIT:
 		r = kvm_caps.has_notify_vmexit;
@@ -6521,7 +6521,7 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
 	switch (cap->cap) {
 	case KVM_CAP_DISABLE_QUIRKS2:
 		r = -EINVAL;
-		if (cap->args[0] & ~KVM_X86_VALID_QUIRKS)
+		if (cap->args[0] & ~kvm_caps.supported_quirks)
 			break;
 		fallthrough;
 	case KVM_CAP_DISABLE_QUIRKS:
@@ -9775,6 +9775,7 @@ int kvm_x86_vendor_init(struct kvm_x86_init_ops *ops)
 		kvm_host.xcr0 = xgetbv(XCR_XFEATURE_ENABLED_MASK);
 		kvm_caps.supported_xcr0 = kvm_host.xcr0 & KVM_SUPPORTED_XCR0;
 	}
+	kvm_caps.supported_quirks = KVM_X86_VALID_QUIRKS_COMMON;
 
 	rdmsrl_safe(MSR_EFER, &kvm_host.efer);
 
diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
index 8ce6da98b5a2..772d5c320be1 100644
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -34,6 +34,7 @@ struct kvm_caps {
 	u64 supported_xcr0;
 	u64 supported_xss;
 	u64 supported_perf_cap;
+	u64 supported_quirks;
 };
 
 struct kvm_host_values {
@@ -354,11 +355,6 @@ static inline void kvm_register_write(struct kvm_vcpu *vcpu,
 	return kvm_register_write_raw(vcpu, reg, val);
 }
 
-static inline bool kvm_check_has_quirk(struct kvm *kvm, u64 quirk)
-{
-	return !(kvm->arch.disabled_quirks & quirk);
-}
-
 void kvm_inject_realmode_interrupt(struct kvm_vcpu *vcpu, int irq, int inc_eip);
 
 u64 get_kvmclock_ns(struct kvm *kvm);
@@ -394,6 +390,12 @@ extern struct kvm_host_values kvm_host;
 
 extern bool enable_pmu;
 
+static inline bool kvm_check_has_quirk(struct kvm *kvm, u64 quirk)
+{
+	return (kvm_caps.supported_quirks & quirk) &&
+	       !(kvm->arch.disabled_quirks & quirk);
+}
+
 /*
  * Get a filtered version of KVM's supported XCR0 that strips out dynamic
  * features for which the current process doesn't (yet) have permission to use.
-- 
2.43.2

[PATCH 2/3] KVM: x86: Introduce Intel specific quirk KVM_X86_QUIRK_EPT_IGNORE_GUEST_PAT

[Yan Zhao]

Introduce a Intel specific quirk KVM_X86_QUIRK_EPT_IGNORE_GUEST_PAT to have
KVM ignore guest PAT when this quirk is enabled.

KVM is able to safely honor guest PAT on Intel platforms when CPU feature
self-snoop is supported. However, KVM honoring guest PAT was reverted after
commit 9d70f3fec144 ("Revert "KVM: VMX: Always honor guest PAT on CPUs that
support self-snoop""), due to UC access on certain Intel platforms being
very slow [1]. Honoring guest PAT on those platforms may break some old
guests that accidentally specify PAT as UC. Those old guests may never
expect the slowness since KVM always forces WB previously. See [2].

So, introduce an Intel specific quirk KVM_X86_QUIRK_EPT_IGNORE_GUEST_PAT.
KVM enables the quirk on all Intel platforms by default to avoid breaking
old unmodifiable guests. Newer userspace can disable this quirk to turn on
honoring guest PAT.

The quirk is only valid on Intel's platforms and is absent on AMD's
platforms as KVM always honors guest PAT when running on AMD.

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Suggested-by: Sean Christopherson <seanjc@google.com>
Cc: Kevin Tian <kevin.tian@intel.com>
Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
Link: https://lore.kernel.org/all/Ztl9NWCOupNfVaCA@yzhao56-desk.sh.intel.com # [1]
Link: https://lore.kernel.org/all/87jzfutmfc.fsf@redhat.com # [2]
---
 Documentation/virt/kvm/api.rst  | 28 +++++++++++++++++++++++
 arch/x86/include/uapi/asm/kvm.h |  1 +
 arch/x86/kvm/mmu.h              |  2 +-
 arch/x86/kvm/mmu/mmu.c          | 14 +++++++-----
 arch/x86/kvm/vmx/vmx.c          | 39 +++++++++++++++++++++++++++------
 arch/x86/kvm/x86.c              |  2 +-
 6 files changed, 72 insertions(+), 14 deletions(-)

diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index d5363d88fa52..c22211c2f54c 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -8164,6 +8164,34 @@ KVM_X86_QUIRK_STUFF_FEATURE_MSRS    By default, at vCPU creation, KVM sets the
                                     and 0x489), as KVM does now allow them to
                                     be set by userspace (KVM sets them based on
                                     guest CPUID, for safety purposes).
+
+KVM_X86_QUIRK_EPT_IGNORE_GUEST_PAT  By default, on Intel platforms, KVM ignores
+                                    guest PAT and forces the effective memory
+                                    type to WB in EPT.  The quirk has no effect
+                                    when KVM runs on Intel platforms which are
+                                    incapable of safely honoring guest PAT
+                                    (i.e., without CPU feature self-snoop, KVM
+                                    always ignores guest PAT and forces
+                                    effective memory type to WB) or when a VM
+                                    has assigned non-coherent DMA devices (KVM
+                                    always honors guest PAT with assigned
+                                    non-coherent DMA devices). On certain Intel
+                                    Xeon platforms (e.g. ICX, SPR), though
+                                    self-snoop feature is supported, UC is slow
+                                    enough to cause issues with some older
+                                    guests (e.g. an old version of bochs driver
+                                    uses ioremap() instead of ioremap_wc() to
+                                    map the video RAM, causing wayland desktop
+                                    to fail to start correctly). To prevent
+                                    breaking older guest software, KVM enables
+                                    the quirk by default on Intel platforms.
+                                    Userspace can disable the quirk to honor
+                                    guest PAT when there is no older
+                                    unmodifiable guest software that relies on
+                                    KVM to force memory type to WB.  Note, the
+                                    quirk is not visible on AMD's platforms,
+                                    i.e., KVM always honors guest PAT when
+                                    running on AMD.
 =================================== ============================================
 
 7.32 KVM_CAP_MAX_VCPU_ID
diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
index 89cc7a18ef45..db55a70e173c 100644
--- a/arch/x86/include/uapi/asm/kvm.h
+++ b/arch/x86/include/uapi/asm/kvm.h
@@ -441,6 +441,7 @@ struct kvm_sync_regs {
 #define KVM_X86_QUIRK_MWAIT_NEVER_UD_FAULTS	(1 << 6)
 #define KVM_X86_QUIRK_SLOT_ZAP_ALL		(1 << 7)
 #define KVM_X86_QUIRK_STUFF_FEATURE_MSRS	(1 << 8)
+#define KVM_X86_QUIRK_EPT_IGNORE_GUEST_PAT	(1 << 9)
 
 #define KVM_STATE_NESTED_FORMAT_VMX	0
 #define KVM_STATE_NESTED_FORMAT_SVM	1
diff --git a/arch/x86/kvm/mmu.h b/arch/x86/kvm/mmu.h
index 47e64a3c4ce3..f999c15d8d3e 100644
--- a/arch/x86/kvm/mmu.h
+++ b/arch/x86/kvm/mmu.h
@@ -232,7 +232,7 @@ static inline u8 permission_fault(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu,
 	return -(u32)fault & errcode;
 }
 
-bool kvm_mmu_may_ignore_guest_pat(void);
+bool kvm_mmu_may_ignore_guest_pat(struct kvm *kvm);
 
 int kvm_mmu_post_init_vm(struct kvm *kvm);
 void kvm_mmu_pre_destroy_vm(struct kvm *kvm);
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index e6eb3a262f8d..28d0b73bf685 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -4663,17 +4663,21 @@ static int kvm_tdp_mmu_page_fault(struct kvm_vcpu *vcpu,
 }
 #endif
 
-bool kvm_mmu_may_ignore_guest_pat(void)
+bool kvm_mmu_may_ignore_guest_pat(struct kvm *kvm)
 {
 	/*
 	 * When EPT is enabled (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 PAT so that guest accesses to
 	 * memory that is DMA'd aren't cached against the guest's wishes.  As a
-	 * result, KVM _may_ ignore guest PAT, whereas without non-coherent DMA,
-	 * KVM _always_ ignores guest PAT (when EPT is enabled).
-	 */
-	return shadow_memtype_mask;
+	 * result, KVM _may_ ignore guest PAT, whereas without non-coherent DMA.
+	 * KVM _always_ ignores guest PAT, when EPT is enabled and when quirk
+	 * KVM_X86_QUIRK_EPT_IGNORE_GUEST_PAT is enabled or the CPU lacks the
+	 * ability to safely honor guest PAT.
+	 */
+	return shadow_memtype_mask &&
+	       (kvm_check_has_quirk(kvm, KVM_X86_QUIRK_EPT_IGNORE_GUEST_PAT) ||
+		!static_cpu_has(X86_FEATURE_SELFSNOOP));
 }
 
 int kvm_tdp_page_fault(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault)
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 486fbdb4365c..9fb884175bfd 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -7599,6 +7599,34 @@ int vmx_vm_init(struct kvm *kvm)
 	return 0;
 }
 
+/*
+ * Ignore guest PAT when the CPU doesn't support self-snoop to safely honor
+ * guest PAT, or quirk KVM_X86_QUIRK_EPT_IGNORE_GUEST_PAT is turned on.  Always
+ * honor guest PAT when there's non-coherent DMA device attached.
+ *
+ * Honoring guest PAT means letting the guest control memory types.
+ * - On Intel CPUs that lack self-snoop feature, honoring guest PAT may result
+ *   in unexpected behavior. So always ignore guest PAT on those CPUs.
+ *
+ * - KVM's ABI is to trust the guest for attached non-coherent DMA devices to
+ *   function correctly (non-coherent DMA devices need the guest to flush CPU
+ *   caches properly). So honoring guest PAT to avoid breaking existing ABI.
+ *
+ * - On certain Intel CPUs (e.g. SPR, ICX), though self-snoop feature is
+ *   supported, UC is slow enough to cause issues with some older guests (e.g.
+ *   an old version of bochs driver uses ioremap() instead of ioremap_wc() to
+ *   map the video RAM, causing wayland desktop to fail to get started
+ *   correctly). To avoid breaking those old guests that rely on KVM to force
+ *   memory type to WB, only honoring guest PAT when quirk
+ *   KVM_X86_QUIRK_EPT_IGNORE_GUEST_PAT is disabled.
+ */
+static inline bool vmx_ignore_guest_pat(struct kvm *kvm)
+{
+	return !kvm_arch_has_noncoherent_dma(kvm) &&
+	       (kvm_check_has_quirk(kvm, KVM_X86_QUIRK_EPT_IGNORE_GUEST_PAT) ||
+		!static_cpu_has(X86_FEATURE_SELFSNOOP));
+}
+
 u8 vmx_get_mt_mask(struct kvm_vcpu *vcpu, gfn_t gfn, bool is_mmio)
 {
 	/*
@@ -7608,13 +7636,8 @@ u8 vmx_get_mt_mask(struct kvm_vcpu *vcpu, gfn_t gfn, bool is_mmio)
 	if (is_mmio)
 		return MTRR_TYPE_UNCACHABLE << VMX_EPT_MT_EPTE_SHIFT;
 
-	/*
-	 * Force WB and ignore guest PAT if the VM does NOT have a non-coherent
-	 * device attached.  Letting the guest control memory types on Intel
-	 * CPUs may result in unexpected behavior, and so KVM's ABI is to trust
-	 * the guest to behave only as a last resort.
-	 */
-	if (!kvm_arch_has_noncoherent_dma(vcpu->kvm))
+	/* Force WB if ignoring guest PAT */
+	if (vmx_ignore_guest_pat(vcpu->kvm))
 		return (MTRR_TYPE_WRBACK << VMX_EPT_MT_EPTE_SHIFT) | VMX_EPT_IPAT_BIT;
 
 	return (MTRR_TYPE_WRBACK << VMX_EPT_MT_EPTE_SHIFT);
@@ -8498,6 +8521,8 @@ __init int vmx_hardware_setup(void)
 			return r;
 	}
 
+	kvm_caps.supported_quirks |= KVM_X86_QUIRK_EPT_IGNORE_GUEST_PAT;
+
 	vmx_set_cpu_caps();
 
 	r = alloc_kvm_area();
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 4f1b73620c6a..8ae96449e6e2 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -13550,7 +13550,7 @@ static void kvm_noncoherent_dma_assignment_start_or_stop(struct kvm *kvm)
 	 * (or last) non-coherent device is (un)registered to so that new SPTEs
 	 * with the correct "ignore guest PAT" setting are created.
 	 */
-	if (kvm_mmu_may_ignore_guest_pat())
+	if (kvm_mmu_may_ignore_guest_pat(kvm))
 		kvm_zap_gfn_range(kvm, gpa_to_gfn(0), gpa_to_gfn(~0ULL));
 }
 
-- 
2.43.2

[PATCH 3/3] KVM: TDX: Always honor guest PAT on TDX enabled platforms

[Yan Zhao]

Always honor guest PAT in KVM-managed EPTs on TDX enabled platforms by
making self-snoop feature a hard dependency for TDX and making quirk
KVM_X86_QUIRK_EPT_IGNORE_GUEST_PAT not a valid quirk once TDX is enabled.

The quirk KVM_X86_QUIRK_EPT_IGNORE_GUEST_PAT only affects memory type of
KVM-managed EPTs. For the TDX-module-managed private EPT, memory type is
always forced to WB now.

Honoring guest PAT in KVM-managed EPTs ensures KVM does not invoke
kvm_zap_gfn_range() when attaching/detaching non-coherent DMA devices,
which would cause mirrored EPTs for TDs to be zapped, leading to the
TDX-module-managed private EPT being incorrectly zapped.

As a new platform, TDX is always with self-snoop feature supported and has
no worry to break old not-well-written yet unmodifiable guests. So, simply
make the quirk KVM_X86_QUIRK_EPT_IGNORE_GUEST_PAT invalid on TDX enabled
platforms.

Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
---
 Documentation/virt/kvm/api.rst | 20 +++++++++++---------
 arch/x86/kvm/vmx/main.c        |  1 +
 arch/x86/kvm/vmx/tdx.c         |  5 +++++
 3 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index c22211c2f54c..5954c5cde33d 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -8165,9 +8165,11 @@ KVM_X86_QUIRK_STUFF_FEATURE_MSRS    By default, at vCPU creation, KVM sets the
                                     be set by userspace (KVM sets them based on
                                     guest CPUID, for safety purposes).
 
-KVM_X86_QUIRK_EPT_IGNORE_GUEST_PAT  By default, on Intel platforms, KVM ignores
-                                    guest PAT and forces the effective memory
-                                    type to WB in EPT.  The quirk has no effect
+KVM_X86_QUIRK_EPT_IGNORE_GUEST_PAT  By default, on Intel platforms except TDX,
+                                    KVM ignores guest PAT and forces the
+                                    effective memory type to WB in EPT. The
+                                    quirk only affects the memory type of
+                                    KVM-managed EPTs.  The quirk has no effect
                                     when KVM runs on Intel platforms which are
                                     incapable of safely honoring guest PAT
                                     (i.e., without CPU feature self-snoop, KVM
@@ -8184,14 +8186,14 @@ KVM_X86_QUIRK_EPT_IGNORE_GUEST_PAT  By default, on Intel platforms, KVM ignores
                                     map the video RAM, causing wayland desktop
                                     to fail to start correctly). To prevent
                                     breaking older guest software, KVM enables
-                                    the quirk by default on Intel platforms.
-                                    Userspace can disable the quirk to honor
-                                    guest PAT when there is no older
+                                    the quirk by default on Intel platforms
+                                    except TDX. Userspace can disable the quirk
+                                    to honor guest PAT when there is no older
                                     unmodifiable guest software that relies on
                                     KVM to force memory type to WB.  Note, the
-                                    quirk is not visible on AMD's platforms,
-                                    i.e., KVM always honors guest PAT when
-                                    running on AMD.
+                                    quirk is not visible on Intel TDX or AMD's
+                                    platforms, i.e., KVM always honors guest PAT
+                                    when running on Intel TDX or AMD.
 =================================== ============================================
 
 7.32 KVM_CAP_MAX_VCPU_ID
diff --git a/arch/x86/kvm/vmx/main.c b/arch/x86/kvm/vmx/main.c
index f586e09b5acf..1fa0364faa60 100644
--- a/arch/x86/kvm/vmx/main.c
+++ b/arch/x86/kvm/vmx/main.c
@@ -1092,6 +1092,7 @@ static int __init vt_init(void)
 		vcpu_align = max_t(unsigned, vcpu_align,
 				__alignof__(struct vcpu_tdx));
 		kvm_caps.supported_vm_types |= BIT(KVM_X86_TDX_VM);
+		kvm_caps.supported_quirks &= ~KVM_X86_QUIRK_EPT_IGNORE_GUEST_PAT;
 	}
 
 	/*
diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
index e73c9fcf213c..7d063cacc9c9 100644
--- a/arch/x86/kvm/vmx/tdx.c
+++ b/arch/x86/kvm/vmx/tdx.c
@@ -3483,6 +3483,11 @@ int __init tdx_bringup(void)
 		goto success_disable_tdx;
 	}
 
+	if (!cpu_feature_enabled(X86_FEATURE_SELFSNOOP)) {
+		pr_err("Self-snoop is reqiured for TDX\n");
+		goto success_disable_tdx;
+	}
+
 	if (!kvm_can_support_tdx()) {
 		pr_err("tdx: no TDX private KeyIDs available\n");
 		goto success_disable_tdx;
-- 
2.43.2

Re: [PATCH 0/3] KVM: x86: Introduce quirk KVM_X86_QUIRK_EPT_IGNORE_GUEST_PAT

[Paolo Bonzini]

On 2/24/25 08:07, Yan Zhao wrote:
> This series introduces a quirk KVM_X86_QUIRK_EPT_IGNORE_GUEST_PAT as
> suggested by Paolo and Sean [1].
> 
> The purpose of introducing this quirk is to allow KVM to honor guest PAT on
> Intel platforms with self-snoop feature. This support was previously
> reverted by commit 9d70f3fec144 ("Revert "KVM: VMX: Always honor guest PAT
> on CPUs that support self-snoop"") due to a reported broken of an old bochs
> driver which incorrectly set memory type to UC but did not expect that UC
> would be very slow on certain Intel platforms.

Hi Yan,

the main issue with this series is that the quirk is not disabled only 
for TDX VMs, but for *all* VMs if TDX is available.

There are two concepts here:

- which quirks can be disabled

- which quirks are active

I agree with making the first vendor-dependent, but for a different 
reason: the new KVM_X86_QUIRK_EPT_IGNORE_GUEST_PAT must be hidden if 
self-snoop is not present.

As to the second, we already have an example of a quirk that is also 
active, though we don't represent that in kvm->arch.disabled_quirks: 
that's KVM_X86_QUIRK_CD_NW_CLEARED which is for AMD only and is 
effectively always disabled on Intel platforms.  For those cases, we 
need to expose the quirk anyway in KVM_CAP_DISABLE_QUIRKS2, so that 
userspace knows that KVM is *aware* of a particular issue.  In other 
words, even if disabling it has no effect, userspace may want to know 
that it can rely on the problematic behavior not being present.

I'm testing an alternative series and will post it shortly.

Paolo

> Sean previously suggested to bottom out if the UC slowness issue is working
> as intended so that we can enable the quirk only when the VMs are affected
> by the old unmodifiable guests [2]. After consulting with CPU architects,
> it's told that this behavior is expected on ICX/SPR Xeon platforms due to
> the snooping implementation.
> 
> So, implement the quirk such that KVM enables it by default on all Intel
> non-TDX platforms while having the quirk explicitly reference the old
> unmodifiable guests that rely on KVM to force memory type to WB. Newer
> userspace can disable the quirk by default and only leave it enabled if an
> old unmodifiable guest is an concern.
> 
> The quirk is platform-specific valid, available only on Intel non-TDX
> platforms. It is absent on Intel TDX and AMD platforms, where KVM always
> honors guest PAT.
> 
> Patch 1 does the preparation of making quirks platform-specific valid.
> Patch 2 makes the quirk to be present on Intel and absent on AMD.
> Patch 3 makes the quirk to be absent on Intel TDX and self-snoop a hard
>          dependency to enable TDX [3].
>          As a new platform, TDX is always running on CPUs with self-snoop
>          feature. It has no worry to break old yet unmodifiable guests.
>          Simply have KVM always honor guest PAT on TDX enabled platforms.
>          Attaching/detaching non-coherent DMA devices would not lead to
>          mirrored EPTs being zapped for TDs then. A previous attempt for
>          this purpose is at [4].
> 
> 
> This series is based on kvm-coco-queue. It was supposed to be included in
> TDX's "the rest" section. We post it separately to start review earlier.
> 
> Patches 1 and 2 are changes to the generic code, which can also be applied
> to kvm/queue. A proposal is to have them go into kvm/queue and we rebase on
> that.
> 
> Patch 3 can be included in TDX's "the rest" section in the end.
> 
> Thanks
> Yan
> 
> [1] https://lore.kernel.org/kvm/CABgObfa=t1dGR5cEhbUqVWTD03vZR4QrzEUgHxq+3JJ7YsA9pA@mail.gmail.com
> [2] https://lore.kernel.org/kvm/Zt8cgUASZCN6gP8H@google.com
> [3] https://lore.kernel.org/kvm/ZuBSNS33_ck-w6-9@google.com
> [4] https://lore.kernel.org/kvm/20241115084600.12174-1-yan.y.zhao@intel.com
> 
> 
> Yan Zhao (3):
>    KVM: x86: Introduce supported_quirks for platform-specific valid
>      quirks
>    KVM: x86: Introduce Intel specific quirk
>      KVM_X86_QUIRK_EPT_IGNORE_GUEST_PAT
>    KVM: TDX: Always honor guest PAT on TDX enabled platforms
> 
>   Documentation/virt/kvm/api.rst  | 30 +++++++++++++++++++++++++
>   arch/x86/include/asm/kvm_host.h |  2 +-
>   arch/x86/include/uapi/asm/kvm.h |  1 +
>   arch/x86/kvm/mmu.h              |  2 +-
>   arch/x86/kvm/mmu/mmu.c          | 14 +++++++-----
>   arch/x86/kvm/vmx/main.c         |  1 +
>   arch/x86/kvm/vmx/tdx.c          |  5 +++++
>   arch/x86/kvm/vmx/vmx.c          | 39 +++++++++++++++++++++++++++------
>   arch/x86/kvm/x86.c              |  7 +++---
>   arch/x86/kvm/x86.h              | 12 +++++-----
>   10 files changed, 91 insertions(+), 22 deletions(-)
> 

Re: [PATCH 0/3] KVM: x86: Introduce quirk KVM_X86_QUIRK_EPT_IGNORE_GUEST_PAT

[Yan Zhao]

On Sat, Mar 01, 2025 at 07:49:13AM +0100, Paolo Bonzini wrote:
> On 2/24/25 08:07, Yan Zhao wrote:
> > This series introduces a quirk KVM_X86_QUIRK_EPT_IGNORE_GUEST_PAT as
> > suggested by Paolo and Sean [1].
> > 
> > The purpose of introducing this quirk is to allow KVM to honor guest PAT on
> > Intel platforms with self-snoop feature. This support was previously
> > reverted by commit 9d70f3fec144 ("Revert "KVM: VMX: Always honor guest PAT
> > on CPUs that support self-snoop"") due to a reported broken of an old bochs
> > driver which incorrectly set memory type to UC but did not expect that UC
> > would be very slow on certain Intel platforms.
> 
> Hi Yan,
Hi Paolo,

> the main issue with this series is that the quirk is not disabled only for
> TDX VMs, but for *all* VMs if TDX is available.
Yes, once TDX is enabled, the quirk is disabled for all VMs.
My thought is that on TDX as a new platform, users have the option to update
guest software to address bugs caused by incorrect guest PAT settings.

If you think it's a must to support old unmodifiable non-TDX VMs on TDX
platforms, then it's indeed an issue of this series.

> 
> There are two concepts here:
> 
> - which quirks can be disabled
> 
> - which quirks are active
> 
> I agree with making the first vendor-dependent, but for a different reason:
> the new KVM_X86_QUIRK_EPT_IGNORE_GUEST_PAT must be hidden if self-snoop is
> not present.
I think it's a good idea to make KVM_X86_QUIRK_EPT_IGNORE_GUEST_PAT out of
KVM_CAP_DISABLE_QUIRKS2, so that the quirk is always enabled when self-snoop is
not present as userspace has no way to disable this quirk.

However, this seems to contradict your point below, especially since it is even
present on AMD platforms.

"we need to expose the quirk anyway in KVM_CAP_DISABLE_QUIRKS2, so that
userspace knows that KVM is *aware* of a particular issue",  "even if disabling
it has no effect, userspace may want to know that it can rely on the problematic
behavior not being present".

So, could we also expose KVM_X86_QUIRK_EPT_IGNORE_GUEST_PAT in
KVM_CAP_DISABLE_QUIRKS2 on Intel platforms without self-snoop, but ensure that
disabling the quirk has no effect?

 
> As to the second, we already have an example of a quirk that is also active,
> though we don't represent that in kvm->arch.disabled_quirks: that's
> KVM_X86_QUIRK_CD_NW_CLEARED which is for AMD only and is effectively always
> disabled on Intel platforms.  For those cases, we need to expose the quirk
I also have a concern about this one. Please find my comments in v2.

> anyway in KVM_CAP_DISABLE_QUIRKS2, so that userspace knows that KVM is
> *aware* of a particular issue.  In other words, even if disabling it has no
> effect, userspace may want to know that it can rely on the problematic
> behavior not being present.
>
> I'm testing an alternative series and will post it shortly.
 
Thanks a lot for helping with refining the patches!

> 
> > Sean previously suggested to bottom out if the UC slowness issue is working
> > as intended so that we can enable the quirk only when the VMs are affected
> > by the old unmodifiable guests [2]. After consulting with CPU architects,
> > it's told that this behavior is expected on ICX/SPR Xeon platforms due to
> > the snooping implementation.
> > 
> > So, implement the quirk such that KVM enables it by default on all Intel
> > non-TDX platforms while having the quirk explicitly reference the old
> > unmodifiable guests that rely on KVM to force memory type to WB. Newer
> > userspace can disable the quirk by default and only leave it enabled if an
> > old unmodifiable guest is an concern.
> > 
> > The quirk is platform-specific valid, available only on Intel non-TDX
> > platforms. It is absent on Intel TDX and AMD platforms, where KVM always
> > honors guest PAT.
> > 
> > Patch 1 does the preparation of making quirks platform-specific valid.
> > Patch 2 makes the quirk to be present on Intel and absent on AMD.
> > Patch 3 makes the quirk to be absent on Intel TDX and self-snoop a hard
> >          dependency to enable TDX [3].
> >          As a new platform, TDX is always running on CPUs with self-snoop
> >          feature. It has no worry to break old yet unmodifiable guests.
> >          Simply have KVM always honor guest PAT on TDX enabled platforms.
> >          Attaching/detaching non-coherent DMA devices would not lead to
> >          mirrored EPTs being zapped for TDs then. A previous attempt for
> >          this purpose is at [4].
> > 
> > 
> > This series is based on kvm-coco-queue. It was supposed to be included in
> > TDX's "the rest" section. We post it separately to start review earlier.
> > 
> > Patches 1 and 2 are changes to the generic code, which can also be applied
> > to kvm/queue. A proposal is to have them go into kvm/queue and we rebase on
> > that.
> > 
> > Patch 3 can be included in TDX's "the rest" section in the end.
> > 
> > Thanks
> > Yan
> > 
> > [1] https://lore.kernel.org/kvm/CABgObfa=t1dGR5cEhbUqVWTD03vZR4QrzEUgHxq+3JJ7YsA9pA@mail.gmail.com
> > [2] https://lore.kernel.org/kvm/Zt8cgUASZCN6gP8H@google.com
> > [3] https://lore.kernel.org/kvm/ZuBSNS33_ck-w6-9@google.com
> > [4] https://lore.kernel.org/kvm/20241115084600.12174-1-yan.y.zhao@intel.com
> > 
> > 
> > Yan Zhao (3):
> >    KVM: x86: Introduce supported_quirks for platform-specific valid
> >      quirks
> >    KVM: x86: Introduce Intel specific quirk
> >      KVM_X86_QUIRK_EPT_IGNORE_GUEST_PAT
> >    KVM: TDX: Always honor guest PAT on TDX enabled platforms
> > 
> >   Documentation/virt/kvm/api.rst  | 30 +++++++++++++++++++++++++
> >   arch/x86/include/asm/kvm_host.h |  2 +-
> >   arch/x86/include/uapi/asm/kvm.h |  1 +
> >   arch/x86/kvm/mmu.h              |  2 +-
> >   arch/x86/kvm/mmu/mmu.c          | 14 +++++++-----
> >   arch/x86/kvm/vmx/main.c         |  1 +
> >   arch/x86/kvm/vmx/tdx.c          |  5 +++++
> >   arch/x86/kvm/vmx/vmx.c          | 39 +++++++++++++++++++++++++++------
> >   arch/x86/kvm/x86.c              |  7 +++---
> >   arch/x86/kvm/x86.h              | 12 +++++-----
> >   10 files changed, 91 insertions(+), 22 deletions(-)
> > 
> 

Re: [PATCH 0/3] KVM: x86: Introduce quirk KVM_X86_QUIRK_EPT_IGNORE_GUEST_PAT

[Paolo Bonzini]

On 3/3/25 02:11, Yan Zhao wrote:
>> the main issue with this series is that the quirk is not disabled only for
>> TDX VMs, but for *all* VMs if TDX is available.
> Yes, once TDX is enabled, the quirk is disabled for all VMs.
> My thought is that on TDX as a new platform, users have the option to update
> guest software to address bugs caused by incorrect guest PAT settings.
> 
> If you think it's a must to support old unmodifiable non-TDX VMs on TDX
> platforms, then it's indeed an issue of this series.

Yeah, unfortunately I think we need to keep the quirk for old VMs.  But 
I think the code changes needed to do so are small and good to have anyway.

>> There are two concepts here:
>>
>> - which quirks can be disabled
>>
>> - which quirks are active
>>
>> I agree with making the first vendor-dependent, but for a different reason:
>> the new KVM_X86_QUIRK_EPT_IGNORE_GUEST_PAT must be hidden if self-snoop is
>> not present.
>
> I think it's a good idea to make KVM_X86_QUIRK_EPT_IGNORE_GUEST_PAT out of
> KVM_CAP_DISABLE_QUIRKS2, so that the quirk is always enabled when self-snoop is
> not present as userspace has no way to disable this quirk.
> 
> However, this seems to contradict your point below, especially since it is even
> present on AMD platforms.
> 
> "we need to expose the quirk anyway in KVM_CAP_DISABLE_QUIRKS2, so that
> userspace knows that KVM is *aware* of a particular issue",  "even if disabling
> it has no effect, userspace may want to know that it can rely on the problematic
> behavior not being present".

There are four cases:

* quirk cannot be disabled: example, "ignore guest PAT" on 
non-self-snoop machines: the quirk must not be in KVM_CAP_DISABLE_QUIRKS2

* quirk can be disabled: the quirk must be in KVM_CAP_DISABLE_QUIRKS2

* quirk is always disabled: right now we're always exposing those in 
KVM_CAP_DISABLE_QUIRKS2, so we should keep that behavior.  If desired we 
could add a capability like KVM_CAP_DISABLED_QUIRKS

* for some VMs, quirk is always disabled: this is the case also for the 
zap_all quirk that you have previously introduced.  Right now there's no 
way to query it, but KVM_CAP_DISABLED_QUIRKS would also cover this.  If 
KVM_CAP_DISABLED_QUIRKS was introduced, zap_all could be added too.

> So, could we also expose KVM_X86_QUIRK_EPT_IGNORE_GUEST_PAT in
> KVM_CAP_DISABLE_QUIRKS2 on Intel platforms without self-snoop, but ensure that
> disabling the quirk has no effect?

To keep the API clear, disabling the quirk should *always* have the 
effect of going to the non-quirky behavior.  Which may be no effect at 
all if the non-quirky behavior is the only one---but the important thing 
is that you don't want the quirky/buggy/non-architectural behavior after 
a successful KVM_ENABLE_CAP(KVM_CAP_DISABLE_QUIRKS2).

There is a pre-existing bug in that I think 
KVM_ENABLE_CAP(KVM_CAP_DISABLE_QUIRKS2) should be cumulative, i.e. 
should not allow re-enabling a previously-disabled quirk.  I think we 
can change that without worrying about breaking userspace there, as the 
current behavior is the most surprising.

>> As to the second, we already have an example of a quirk that is also active,
>> though we don't represent that in kvm->arch.disabled_quirks: that's
>> KVM_X86_QUIRK_CD_NW_CLEARED which is for AMD only and is effectively always
>> disabled on Intel platforms.  For those cases, we need to expose the quirk
> I also have a concern about this one. Please find my comments in v2.

Ok, I'll reply there too.

>> anyway in KVM_CAP_DISABLE_QUIRKS2, so that userspace knows that KVM is
>> *aware* of a particular issue.  In other words, even if disabling it has no
>> effect, userspace may want to know that it can rely on the problematic
>> behavior not being present.
>>
>> I'm testing an alternative series and will post it shortly.
>   
> Thanks a lot for helping with refining the patches!

Thanks to you and sorry that the patches weren't of the best quality - I 
mostly wanted to start the discussion on the userspace API side before 
the beginning of the week in your time zone.

Paolo

Re: [PATCH 0/3] KVM: x86: Introduce quirk KVM_X86_QUIRK_EPT_IGNORE_GUEST_PAT

[Yan Zhao]

On Mon, Mar 03, 2025 at 11:25:08AM +0100, Paolo Bonzini wrote:
> On 3/3/25 02:11, Yan Zhao wrote:
> > > the main issue with this series is that the quirk is not disabled only for
> > > TDX VMs, but for *all* VMs if TDX is available.
> > Yes, once TDX is enabled, the quirk is disabled for all VMs.
> > My thought is that on TDX as a new platform, users have the option to update
> > guest software to address bugs caused by incorrect guest PAT settings.
> > 
> > If you think it's a must to support old unmodifiable non-TDX VMs on TDX
> > platforms, then it's indeed an issue of this series.
> 
> Yeah, unfortunately I think we need to keep the quirk for old VMs.  But I
> think the code changes needed to do so are small and good to have anyway.
> 
> > > There are two concepts here:
> > > 
> > > - which quirks can be disabled
> > > 
> > > - which quirks are active
> > > 
> > > I agree with making the first vendor-dependent, but for a different reason:
> > > the new KVM_X86_QUIRK_EPT_IGNORE_GUEST_PAT must be hidden if self-snoop is
> > > not present.
> > 
> > I think it's a good idea to make KVM_X86_QUIRK_EPT_IGNORE_GUEST_PAT out of
> > KVM_CAP_DISABLE_QUIRKS2, so that the quirk is always enabled when self-snoop is
> > not present as userspace has no way to disable this quirk.
> > 
> > However, this seems to contradict your point below, especially since it is even
> > present on AMD platforms.
> > 
> > "we need to expose the quirk anyway in KVM_CAP_DISABLE_QUIRKS2, so that
> > userspace knows that KVM is *aware* of a particular issue",  "even if disabling
> > it has no effect, userspace may want to know that it can rely on the problematic
> > behavior not being present".
> 
> There are four cases:
> 
> * quirk cannot be disabled: example, "ignore guest PAT" on non-self-snoop
> machines: the quirk must not be in KVM_CAP_DISABLE_QUIRKS2
> 
> * quirk can be disabled: the quirk must be in KVM_CAP_DISABLE_QUIRKS2
> 
> * quirk is always disabled: right now we're always exposing those in
> KVM_CAP_DISABLE_QUIRKS2, so we should keep that behavior.  If desired we
> could add a capability like KVM_CAP_DISABLED_QUIRKS
> 
> * for some VMs, quirk is always disabled: this is the case also for the
> zap_all quirk that you have previously introduced.  Right now there's no way
> to query it, but KVM_CAP_DISABLED_QUIRKS would also cover this.  If
> KVM_CAP_DISABLED_QUIRKS was introduced, zap_all could be added too.
> 
> > So, could we also expose KVM_X86_QUIRK_EPT_IGNORE_GUEST_PAT in
> > KVM_CAP_DISABLE_QUIRKS2 on Intel platforms without self-snoop, but ensure that
> > disabling the quirk has no effect?
> 
> To keep the API clear, disabling the quirk should *always* have the effect
> of going to the non-quirky behavior.  Which may be no effect at all if the
> non-quirky behavior is the only one---but the important thing is that you
> don't want the quirky/buggy/non-architectural behavior after a successful
> KVM_ENABLE_CAP(KVM_CAP_DISABLE_QUIRKS2).
Thanks for this clarification!

> 
> There is a pre-existing bug in that I think
> KVM_ENABLE_CAP(KVM_CAP_DISABLE_QUIRKS2) should be cumulative, i.e. should
> not allow re-enabling a previously-disabled quirk.  I think we can change
> that without worrying about breaking userspace there, as the current
> behavior is the most surprising.
That would be better.

> > > As to the second, we already have an example of a quirk that is also active,
> > > though we don't represent that in kvm->arch.disabled_quirks: that's
> > > KVM_X86_QUIRK_CD_NW_CLEARED which is for AMD only and is effectively always
> > > disabled on Intel platforms.  For those cases, we need to expose the quirk
> > I also have a concern about this one. Please find my comments in v2.
> 
> Ok, I'll reply there too.
> 
> > > anyway in KVM_CAP_DISABLE_QUIRKS2, so that userspace knows that KVM is
> > > *aware* of a particular issue.  In other words, even if disabling it has no
> > > effect, userspace may want to know that it can rely on the problematic
> > > behavior not being present.
> > > 
> > > I'm testing an alternative series and will post it shortly.
> > Thanks a lot for helping with refining the patches!
> 
> Thanks to you and sorry that the patches weren't of the best quality - I
> mostly wanted to start the discussion on the userspace API side before the
> beginning of the week in your time zone.
No problem.

I realized the problem in my implementation of excluding quirk IGNORE_GUEST_PAT
from KVM_CAP_DISABLE_QUIRKS2 on TDX platforms.
This could lead to confusion for userspace, which wouldn't be able to determine
whether:
- it's an old KVM that does not support quirk IGNORE_GUEST_PAT, meaning KVM will
  ignore guest PAT, or
- it's a new KVM that supports IGNORE_GUEST_PAT, meaning KVM will honor guest
  PAT on TDX platforms.

Looking back, I was too KVM-centric. I just thought users wouldn't need to invoke
KVM_ENABLE_CAP(KVM_CAP_DISABLE_QUIRKS2) on AMD or TDX, but that was wrong --
I did not consider the issue from the user's perspective.