[Xen-devel] [PATCH] Fix vmce MCi_ADDR/MCi_MISC wrmsr bug (v2)

["Liu, Jinsong" <jinsong.liu@xxxxxxxxx>]

Fix vmce MCi_ADDR/MCi_MISC wrmsr bug

This patch fix a bug related to wrmsr vmce MCi_ADDR/MCi_MISC
registers, since they are not read-only.

Intel SDM recommanded os mce driver clear MCi_ADDR/MCi_MISC
So guest mce driver may clear MCi_ADDR/MCi_MISC registers.
Under such case, old vmce wrmsr logic would generate
a #GP fault at guest mce context, and make guest crash.

When wrmsr MCi_ADDR/MCi_MISC, writing 1s will cause #GP.

Signed-off-by: Liu, Jinsong <jinsong.liu@xxxxxxxxx>

diff -r 755f440b3b78 xen/arch/x86/cpu/mcheck/vmce.c
--- a/xen/arch/x86/cpu/mcheck/vmce.c    Wed Apr 18 18:33:36 2012 +0800
+++ b/xen/arch/x86/cpu/mcheck/vmce.c    Tue May 08 03:02:50 2012 +0800
@@ -209,6 +209,14 @@
     struct domain_mca_msrs *vmce = dom_vmce(v->domain);
     struct bank_entry *entry = NULL;
 
+    /* Give the first entry of the list, it corresponds to current
+     * vMCE# injection. When vMCE# is finished processing by the
+     * the guest, this node will be deleted.
+     * Only error bank is written. Non-error banks simply return.
+     */
+    if ( !list_empty(&vmce->impact_header) )
+        entry = list_entry(vmce->impact_header.next, struct bank_entry, list);
+
     switch ( msr & (MSR_IA32_MC0_CTL | 3) )
     {
     case MSR_IA32_MC0_CTL:
@@ -216,32 +224,53 @@
             vmce->mci_ctl[bank] = val;
         break;
     case MSR_IA32_MC0_STATUS:
-        /* Give the first entry of the list, it corresponds to current
-         * vMCE# injection. When vMCE# is finished processing by the
-         * the guest, this node will be deleted.
-         * Only error bank is written. Non-error banks simply return.
-         */
-        if ( !list_empty(&vmce->impact_header) )
+        if ( entry && (entry->bank == bank) )
         {
-            entry = list_entry(vmce->impact_header.next,
-                               struct bank_entry, list);
-            if ( entry->bank == bank )
-                entry->mci_status = val;
+            entry->mci_status = val;
             mce_printk(MCE_VERBOSE,
-                       "MCE: wr MC%u_STATUS %"PRIx64" in vMCE#\n",
-                       bank, val);
+                       "MCE: wr MC%u_STATUS %"PRIx64" in vMCE#\n", bank, val);
         }
         else
             mce_printk(MCE_VERBOSE,
                        "MCE: wr MC%u_STATUS %"PRIx64"\n", bank, val);
         break;
     case MSR_IA32_MC0_ADDR:
-        mce_printk(MCE_QUIET, "MCE: MC%u_ADDR is read-only\n", bank);
-        ret = -1;
+        if ( val == ~0 )
+        {
+            mce_printk(MCE_QUIET,
+                       "MCE: wr MC%u_ADDR with 1s will cause #GP\n", bank);
+            ret = -1;
+            break;
+        }
+
+        if ( entry && (entry->bank == bank) )
+        {
+            entry->mci_addr = val;
+            mce_printk(MCE_VERBOSE,
+                       "MCE: wr MC%u_ADDR %"PRIx64" in vMCE#\n", bank, val);
+        }
+        else
+            mce_printk(MCE_VERBOSE,
+                       "MCE: wr MC%u_ADDR %"PRIx64"\n", bank, val);
         break;
     case MSR_IA32_MC0_MISC:
-        mce_printk(MCE_QUIET, "MCE: MC%u_MISC is read-only\n", bank);
-        ret = -1;
+        if ( val == ~0 )
+        {
+            mce_printk(MCE_QUIET,
+                       "MCE: wr MC%u_MISC with 1s will cause #GP\n", bank);
+            ret = -1;
+            break;
+        }
+
+        if ( entry && (entry->bank == bank) )
+        {
+            entry->mci_misc = val;
+            mce_printk(MCE_VERBOSE,
+                       "MCE: wr MC%u_MISC %"PRIx64" in vMCE#\n", bank, val);
+        }
+        else
+            mce_printk(MCE_VERBOSE,
+                       "MCE: wr MC%u_MISC %"PRIx64"\n", bank, val);
         break;
     default:
         switch ( boot_cpu_data.x86_vendor )

Attachment: vmce-bug-fix.patch
Description: vmce-bug-fix.patch

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel