[PATCH] fixup 3dnow! support

[PATCH] fixup 3dnow! support

[Glauber Costa]

qemu recently added support for 3dnow instructions. Because of
that, 3dnow will be featured among cpuid bits. But this will
break kvm in cpus that don't have those instructions (which includes
my laptop). So we fixup our cpuid before exposing it to the guest.

Signed-off-by: Glauber Costa <gcosta@redhat.com>
---
 arch/x86/kvm/x86.c           |   22 ++++++++++++++++++----
 include/asm-x86/cpufeature.h |    2 ++
 2 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 979f983..e79fcd5 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -919,7 +919,7 @@ static int is_efer_nx(void)
 	return efer & EFER_NX;
 }
 
-static void cpuid_fix_nx_cap(struct kvm_vcpu *vcpu)
+static void cpuid_fix_caps(struct kvm_vcpu *vcpu)
 {
 	int i;
 	struct kvm_cpuid_entry2 *e, *entry;
@@ -932,6 +932,20 @@ static void cpuid_fix_nx_cap(struct kvm_vcpu *vcpu)
 			break;
 		}
 	}
+
+	/* 3DNOWEXT */
+	if (entry && (entry->edx & (1 << 30)) && !cpu_has_3dnowext) {
+		entry->edx &= ~(1 << 30);
+		printk(KERN_INFO "kvm: guest 3DNOWEXT capability removed\n");
+	}
+
+	/* 3DNOW */
+	if (entry && (entry->edx & (1 << 31)) && !cpu_has_3dnow) {
+		entry->edx &= ~(1 << 31);
+		printk(KERN_INFO "kvm: guest 3DNOW capability removed\n");
+	}
+
+	/* NX */
 	if (entry && (entry->edx & (1 << 20)) && !is_efer_nx()) {
 		entry->edx &= ~(1 << 20);
 		printk(KERN_INFO "kvm: guest NX capability removed\n");
@@ -970,7 +984,7 @@ static int kvm_vcpu_ioctl_set_cpuid(struct kvm_vcpu *vcpu,
 		vcpu->arch.cpuid_entries[i].padding[2] = 0;
 	}
 	vcpu->arch.cpuid_nent = cpuid->nent;
-	cpuid_fix_nx_cap(vcpu);
+	cpuid_fix_caps(vcpu);
 	r = 0;
 
 out_free:
@@ -1061,8 +1075,8 @@ static void do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
 		bit(X86_FEATURE_LM) |
 #endif
 		bit(X86_FEATURE_MMXEXT) |
-		bit(X86_FEATURE_3DNOWEXT) |
-		bit(X86_FEATURE_3DNOW);
+		(bit(X86_FEATURE_3DNOWEXT) && cpu_has_3dnowext) |
+		(bit(X86_FEATURE_3DNOW) && cpu_has_3dnow);
 	const u32 kvm_supported_word3_x86_features =
 		bit(X86_FEATURE_XMM3) | bit(X86_FEATURE_CX16);
 	const u32 kvm_supported_word6_x86_features =
diff --git a/include/asm-x86/cpufeature.h b/include/asm-x86/cpufeature.h
index 0d609c8..efbc5ce 100644
--- a/include/asm-x86/cpufeature.h
+++ b/include/asm-x86/cpufeature.h
@@ -187,6 +187,8 @@ extern const char * const x86_power_flags[32];
 #define cpu_has_gbpages		boot_cpu_has(X86_FEATURE_GBPAGES)
 #define cpu_has_arch_perfmon	boot_cpu_has(X86_FEATURE_ARCH_PERFMON)
 #define cpu_has_pat		boot_cpu_has(X86_FEATURE_PAT)
+#define cpu_has_3dnow		boot_cpu_has(X86_FEATURE_3DNOW)
+#define cpu_has_3dnowext	boot_cpu_has(X86_FEATURE_3DNOWEXT)
 
 #if defined(CONFIG_X86_INVLPG) || defined(CONFIG_X86_64)
 # define cpu_has_invlpg		1
-- 
1.5.0.6

Re: [kvm-devel] [PATCH] fixup 3dnow! support

[Alexander Graf]

On May 6, 2008, at 6:27 PM, Glauber Costa wrote:

> qemu recently added support for 3dnow instructions. Because of
> that, 3dnow will be featured among cpuid bits. But this will
> break kvm in cpus that don't have those instructions (which includes
> my laptop). So we fixup our cpuid before exposing it to the guest.

I actually don't see where the problem is here. As far as I read the  
code, the CPUID feature function gets received from the host CPU and  
bitwise ANDed with a bunch of features that are known to work. What's  
wrong with that approach?

But I'm pretty sure Dao can tell us a lot more about this. Has there  
been any progress in getting the new CPUID code in? I think I could  
review this sometime soon.

Alex

Re: [kvm-devel] [PATCH] fixup 3dnow! support

[Glauber Costa]

Alexander Graf wrote:
> 
> On May 6, 2008, at 6:27 PM, Glauber Costa wrote:
> 
>> qemu recently added support for 3dnow instructions. Because of
>> that, 3dnow will be featured among cpuid bits. But this will
>> break kvm in cpus that don't have those instructions (which includes
>> my laptop). So we fixup our cpuid before exposing it to the guest.
> 
> I actually don't see where the problem is here. As far as I read the 
> code, the CPUID feature function gets received from the host CPU and 
> bitwise ANDed with a bunch of features that are known to work. What's 
> wrong with that approach?
Probably is that besides that known to work features, there are also 
features that qemu puts in unconditionally. Among them, 3DNOW.

> But I'm pretty sure Dao can tell us a lot more about this.
Sure, it would be welcome.

Re: [kvm-devel] [PATCH] fixup 3dnow! support

[Avi Kivity]

Glauber Costa wrote:
> qemu recently added support for 3dnow instructions. Because of
> that, 3dnow will be featured among cpuid bits. But this will
> break kvm in cpus that don't have those instructions (which includes
> my laptop). So we fixup our cpuid before exposing it to the guest.
>
>   

I've already fixed this in userspace.

In general I don't like silently modifying cpuid options in the kernel, 
since that means we cannot be confident as to what the cpuid the guest 
sees actually is, and so we can't be sure that migration will succeed.  
Instead, the preferred path is to query what cpuid bits the host 
support, pass that to the management app in order to compute the 
greatest common subset, and tell the kernel to use that set.

nx is handled differently due to compatibility issues.  In time we can 
remove the special handling.

-- 
error compiling committee.c: too many arguments to function

Re: [kvm-devel] [PATCH] fixup 3dnow! support

[Avi Kivity]

Alexander Graf wrote:
>
> On May 6, 2008, at 6:27 PM, Glauber Costa wrote:
>
>> qemu recently added support for 3dnow instructions. Because of
>> that, 3dnow will be featured among cpuid bits. But this will
>> break kvm in cpus that don't have those instructions (which includes
>> my laptop). So we fixup our cpuid before exposing it to the guest.
>
> I actually don't see where the problem is here. As far as I read the 
> code, the CPUID feature function gets received from the host CPU and 
> bitwise ANDed with a bunch of features that are known to work. What's 
> wrong with that approach?

Nothing.  Note that "known to work" needs to be queried from the kernel, 
since userspace and the kernel are not guaranteed to match.

> But I'm pretty sure Dao can tell us a lot more about this. Has there 
> been any progress in getting the new CPUID code in? I think I could 
> review this sometime soon.

Who's Dao?  Do you mean Dan?

-- 
error compiling committee.c: too many arguments to function

Re: [kvm-devel] [PATCH] fixup 3dnow! support

[Alexander Graf]

Avi Kivity wrote:
> Who's Dao?  Do you mean Dan

Phew I'm so really bad at remembering names, sorry about that. I was 
actually talking about Dan Kenigsberg, who apparently did patches to use 
the "new" cpuid interface some time ago.