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 at 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
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
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.
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
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