[PATCH kvmtool v2] Revert "kvm tools: Filter out CPU vendor string"

* [PATCH kvmtool v2] Revert "kvm tools: Filter out CPU vendor string"
@ 2022-03-18 20:49 Oliver Upton
  2022-03-21 13:57 ` Will Deacon
  0 siblings, 1 reply; 2+ messages in thread
From: Oliver Upton @ 2022-03-18 20:49 UTC (permalink / raw)
  To: kvm
  Cc: Will Deacon, Julien Thierry, Paolo Bonzini, Oliver Upton,
	Dongli Si, Andre Przywara

This reverts commit bc0b99a2a74047707db73ba057743febf458fd90.

Thanks to some digging from Andre [1], we know that kvmtool commit
bc0b99a2a740 ("kvm tools: Filter out CPU vendor string") was intended
to work around a guest kernel bug resulting from kernel commit
5bbc097d8904 ("x86, amd: Disable GartTlbWlkErr when BIOS forgets it").
Critically, KVM does not implement the MC4 mask MSR and instead injects
a #GP into the guest. On guest kernels without commit d47cc0db8fd6
("x86, amd: Use _safe() msr access for GartTlbWlk disable code") this is
unexpected and causes a kernel oops.

Since the kernel has taken the position to fix the bug in the guest and
not KVM, there is no need for CPU vendor string filtering in kvmtool.
Vendor string filtering is highly problematic for feature discovery,
both in the kernel and userspace. As Andre noted, glibc depends on the
vendor string to discover CPU features at runtime [2]. This has been
generally innocuous, but as distributions begin to raise the minimum ISA
guest userspace will quickly crash and burn on kvmtool. Hiding the
vendor string also makes it impossible to test vendor-specific CPU
features in kvmtool guest kernels.

Given the fact that there are known dependencies in kernel and userspace
on the CPU vendor string, allow the guest to see the native CPU vendor
string. This has the potential to break certain guest kernels of 2011
vintage when running on an AMD Fam10h processor. Onus is on the guest to
update its kernel at this point.

Link: https://lore.kernel.org/kvm/20220311121042.010bbb30@donnerap.cambridge.arm.com/
Link: https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/x86/cpu-features.c;h=514226b37889;hb=HEAD#l398
Reported-by: Dongli Si <sidongli1997@gmail.com>
Suggested-by: Andre Przywara <andre.przywara@arm.com>
Signed-off-by: Oliver Upton <oupton@google.com>
---

v1: http://lore.kernel.org/r/20220317192707.59538-1-oupton@google.com

v1 -> v2:
 - Do a git revert of the original patch [Andre]
 - Add more context to the commit message [Andre]

Applies to kvmtool/master

Parent commit: 95f4796 ("arm: pci: Generate "msi-parent" property only with a MSI controller")

 x86/cpuid.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/x86/cpuid.c b/x86/cpuid.c
index aa213d5..f4347a8 100644
--- a/x86/cpuid.c
+++ b/x86/cpuid.c
@@ -10,7 +10,6 @@
 
 static void filter_cpuid(struct kvm_cpuid2 *kvm_cpuid, int cpu_id)
 {
-	unsigned int signature[3];
 	unsigned int i;
 
 	/*
@@ -20,13 +19,6 @@ static void filter_cpuid(struct kvm_cpuid2 *kvm_cpuid, int cpu_id)
 		struct kvm_cpuid_entry2 *entry = &kvm_cpuid->entries[i];
 
 		switch (entry->function) {
-		case 0:
-			/* Vendor name */
-			memcpy(signature, "LKVMLKVMLKVM", 12);
-			entry->ebx = signature[0];
-			entry->ecx = signature[1];
-			entry->edx = signature[2];
-			break;
 		case 1:
 			entry->ebx &= ~(0xff << 24);
 			entry->ebx |= cpu_id << 24;
-- 
2.35.1.894.gb6a874cedc-goog


^ permalink raw reply related	[flat|nested] 2+ messages in thread