Re: [RFT] Fix for unhandled msr c0000081 problems under Intel cpus

[Avi Kivity <avi-atKUWr5tajBWk0Htik3J/w@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 938 bytes --]

Avi Kivity wrote:
> Michael Riepe wrote:
>> Oh well...
>>
>> I was wondering which stupid piece of sh^H^Hcode generated so many wrong
>> rdmsr instructions without looking at the CPU feature flags. So I
>> grepped through the linux kernel and glibc sources as well as some other
>> likely suspects and found - nothing.
>>
>> Guess what? It's qemu!
>>
>> In qemu-kvm.c (around line 329 in save_regs()) it starts to call
>> kvm_get_msrs(), and one of the MSRs it wants to read is MSR_STAR. When I
>> removed MSR_STAR from the list (which fortunately has no consequences on
>> my Core Duo), the messages were gone.
>>   
>
> Yes, you're right.
>
> I'll commit the previous patch (which is correct, even if no guest 
> actually uses syscall), and teach qemu not to read MSR_STAR when it 
> isn't available.
>

A patch is attached.  Please test.

(note it changes libkvm too)


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


[-- Attachment #2: avoid-msr-star-if-not-available.patch --]
[-- Type: text/x-patch, Size: 4120 bytes --]

Index: qemu/qemu-kvm.c
===================================================================
--- qemu/qemu-kvm.c	(revision 4110)
+++ qemu/qemu-kvm.c	(working copy)
@@ -16,6 +16,8 @@
 
 int kvm_allowed = 1;
 kvm_context_t kvm_context;
+static struct kvm_msr_list *kvm_msr_list;
+static int kvm_has_msr_star;
 
 #define NR_CPU 16
 static CPUState *saved_env[NR_CPU];
@@ -127,7 +129,7 @@
     struct kvm_regs regs;
     struct kvm_sregs sregs;
     struct kvm_msr_entry msrs[MSR_COUNT];
-    int rc;
+    int rc, n;
 
     /* hack: save env */
     if (!saved_env[0])
@@ -201,19 +203,21 @@
     kvm_set_sregs(kvm_context, 0, &sregs);
 
     /* msrs */
-    set_msr_entry(&msrs[0], MSR_IA32_SYSENTER_CS,  env->sysenter_cs);
-    set_msr_entry(&msrs[1], MSR_IA32_SYSENTER_ESP, env->sysenter_esp);
-    set_msr_entry(&msrs[2], MSR_IA32_SYSENTER_EIP, env->sysenter_eip);
-    set_msr_entry(&msrs[3], MSR_STAR,              env->star);
-    set_msr_entry(&msrs[4], MSR_IA32_TSC, env->tsc);
+    n = 0;
+    set_msr_entry(&msrs[n++], MSR_IA32_SYSENTER_CS,  env->sysenter_cs);
+    set_msr_entry(&msrs[n++], MSR_IA32_SYSENTER_ESP, env->sysenter_esp);
+    set_msr_entry(&msrs[n++], MSR_IA32_SYSENTER_EIP, env->sysenter_eip);
+    if (kvm_has_msr_star)
+	set_msr_entry(&msrs[n++], MSR_STAR,              env->star);
+    set_msr_entry(&msrs[n++], MSR_IA32_TSC, env->tsc);
 #ifdef TARGET_X86_64
-    set_msr_entry(&msrs[5], MSR_CSTAR,             env->cstar);
-    set_msr_entry(&msrs[6], MSR_KERNELGSBASE,      env->kernelgsbase);
-    set_msr_entry(&msrs[7], MSR_FMASK,             env->fmask);
-    set_msr_entry(&msrs[8], MSR_LSTAR  ,           env->lstar);
+    set_msr_entry(&msrs[n++], MSR_CSTAR,             env->cstar);
+    set_msr_entry(&msrs[n++], MSR_KERNELGSBASE,      env->kernelgsbase);
+    set_msr_entry(&msrs[n++], MSR_FMASK,             env->fmask);
+    set_msr_entry(&msrs[n++], MSR_LSTAR  ,           env->lstar);
 #endif
 
-    rc = kvm_set_msrs(kvm_context, 0, msrs, MSR_COUNT);
+    rc = kvm_set_msrs(kvm_context, 0, msrs, n);
     if (rc == -1)
         perror("kvm_set_msrs FAILED");
 }
@@ -326,18 +330,20 @@
     tlb_flush(env, 1);
 
     /* msrs */    
-    msrs[0].index = MSR_IA32_SYSENTER_CS;
-    msrs[1].index = MSR_IA32_SYSENTER_ESP;
-    msrs[2].index = MSR_IA32_SYSENTER_EIP;
-    msrs[3].index = MSR_STAR;
-    msrs[4].index = MSR_IA32_TSC;
+    n = 0;
+    msrs[n++].index = MSR_IA32_SYSENTER_CS;
+    msrs[n++].index = MSR_IA32_SYSENTER_ESP;
+    msrs[n++].index = MSR_IA32_SYSENTER_EIP;
+    if (kvm_has_msr_star)
+	msrs[n++].index = MSR_STAR;
+    msrs[n++].index = MSR_IA32_TSC;
 #ifdef TARGET_X86_64
-    msrs[5].index = MSR_CSTAR;
-    msrs[6].index = MSR_KERNELGSBASE;
-    msrs[7].index = MSR_FMASK;
-    msrs[8].index = MSR_LSTAR;
+    msrs[n++].index = MSR_CSTAR;
+    msrs[n++].index = MSR_KERNELGSBASE;
+    msrs[n++].index = MSR_FMASK;
+    msrs[n++].index = MSR_LSTAR;
 #endif
-    rc = kvm_get_msrs(kvm_context, 0, msrs, MSR_COUNT);
+    rc = kvm_get_msrs(kvm_context, 0, msrs, n);
     if (rc == -1) {
         perror("kvm_get_msrs FAILED");
     }
@@ -597,11 +603,20 @@
 
 int kvm_qemu_create_context(void)
 {
+    int i;
+
     if (kvm_create(kvm_context, phys_ram_size, (void**)&phys_ram_base) < 0) {
 	kvm_qemu_destroy();
 	return -1;
     }
-
+    kvm_msr_list = kvm_get_msr_list(kvm_context);
+    if (!kvm_msr_list) {
+	kvm_qemu_destroy();
+	return -1;
+    }
+    for (i = 0; i < kvm_msr_list->nmsrs; ++i)
+	if (kvm_msr_list->indices[i] == MSR_STAR)
+	    kvm_has_msr_star = 1;
     return 0;
 }
 
Index: user/kvmctl.c
===================================================================
--- user/kvmctl.c	(revision 4089)
+++ user/kvmctl.c	(working copy)
@@ -339,13 +339,14 @@
 
     sizer.nmsrs = 0;
     r = ioctl(kvm->fd, KVM_GET_MSR_INDEX_LIST, &sizer);
-    if (r == -1)
+    if (r == -1 && errno != E2BIG)
 	return 0;
     msrs = malloc(sizeof *msrs + sizer.nmsrs * sizeof *msrs->indices);
     if (!msrs) {
 	errno = ENOMEM;
 	return 0;
     }
+    msrs->nmsrs = sizer.nmsrs;
     r = ioctl(kvm->fd, KVM_GET_MSR_INDEX_LIST, msrs);
     if (r == -1) {
 	e = errno;

[-- Attachment #3: Type: text/plain, Size: 347 bytes --]

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

[-- Attachment #4: Type: text/plain, Size: 186 bytes --]

_______________________________________________
kvm-devel mailing list
kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/kvm-devel