[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [Patch 2/6] Xen/MCE: remove mcg_ctl and other adjustment for future vMCE



>>> 
>>> Is there a particular reason to make this access fault here, when
>>> it didn't before? I.e. was there anything wrong with the previous
>>> approach of returning zero on reads and ignoring writes when
>>> !MCG_CTL_P? 
>>> 
>> 
>> Semantically this code is better than previous approach, since
>> !MCG_CTL_P means unimplemented MCG_CTL so access it would generate
>> GP#. 
> 
> Agreed. But nevertheless I'd like to be a little more conservative
> here. After all, "knowing" that this won't break Windows or Linux
> isn't covering all possible HVM guests (and the quotes are there
> to indicate that (a) unless you have access to Windows sources,
> you can't really know, you may at best have empirical data
> suggesting so, and (b) makes you/us dependent on all older
> Windows/Linux versions you didn't try out/look at behave
> correctly here too).

OK, fine to me to use previous approach, updated as attached.

Thanks,
Jinsong

===============================


Xen/MCE: remove mcg_ctl and other adjustment for future vMCE

This patch is a middle-work patch, prepare for future new vMCE model.
It remove mcg_ctl, disable MCG_CTL_P, and set bank number to 2.

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

diff -r 8067891037a6 xen/arch/x86/cpu/mcheck/mce.c
--- a/xen/arch/x86/cpu/mcheck/mce.c     Thu Jul 19 20:48:25 2012 +0800
+++ b/xen/arch/x86/cpu/mcheck/mce.c     Tue Aug 07 04:26:56 2012 +0800
@@ -843,8 +843,6 @@
 
     mctelem_init(sizeof(struct mc_info));
 
-    vmce_init(c);
-
     /* Turn on MCE now */
     set_in_cr4(X86_CR4_MCE);
 
diff -r 8067891037a6 xen/arch/x86/cpu/mcheck/mce.h
--- a/xen/arch/x86/cpu/mcheck/mce.h     Thu Jul 19 20:48:25 2012 +0800
+++ b/xen/arch/x86/cpu/mcheck/mce.h     Tue Aug 07 04:26:56 2012 +0800
@@ -170,8 +170,6 @@
 int inject_vmce(struct domain *d);
 int vmce_domain_inject(struct mcinfo_bank *bank, struct domain *d, struct 
mcinfo_global *global);
 
-extern int vmce_init(struct cpuinfo_x86 *c);
-
 static inline int mce_vendor_bank_msr(const struct vcpu *v, uint32_t msr)
 {
     if ( boot_cpu_data.x86_vendor == X86_VENDOR_INTEL &&
diff -r 8067891037a6 xen/arch/x86/cpu/mcheck/vmce.c
--- a/xen/arch/x86/cpu/mcheck/vmce.c    Thu Jul 19 20:48:25 2012 +0800
+++ b/xen/arch/x86/cpu/mcheck/vmce.c    Tue Aug 07 04:26:56 2012 +0800
@@ -19,13 +19,18 @@
 #include "mce.h"
 #include "x86_mca.h"
 
+/*
+ * Emulalte 2 banks for guest
+ * Bank0: reserved for 'bank0 quirk' occur at some very old processors:
+ *   1). Intel cpu whose family-model value < 06-1A;
+ *   2). AMD K7
+ * Bank1: used to transfer error info to guest
+ */
+#define GUEST_BANK_NUM 2
+#define GUEST_MCG_CAP (MCG_TES_P | MCG_SER_P | GUEST_BANK_NUM)
+
 #define dom_vmce(x)   ((x)->arch.vmca_msrs)
 
-static uint64_t __read_mostly g_mcg_cap;
-
-/* Real value in physical CTL MSR */
-static uint64_t __read_mostly h_mcg_ctl;
-
 int vmce_init_msr(struct domain *d)
 {
     dom_vmce(d) = xmalloc(struct domain_mca_msrs);
@@ -33,7 +38,6 @@
         return -ENOMEM;
 
     dom_vmce(d)->mcg_status = 0x0;
-    dom_vmce(d)->mcg_ctl = ~(uint64_t)0x0;
     dom_vmce(d)->nr_injection = 0;
 
     INIT_LIST_HEAD(&dom_vmce(d)->impact_header);
@@ -52,17 +56,17 @@
 
 void vmce_init_vcpu(struct vcpu *v)
 {
-    v->arch.mcg_cap = g_mcg_cap;
+    v->arch.mcg_cap = GUEST_MCG_CAP;
 }
 
 int vmce_restore_vcpu(struct vcpu *v, uint64_t caps)
 {
-    if ( caps & ~g_mcg_cap & ~MCG_CAP_COUNT & ~MCG_CTL_P )
+    if ( caps & ~GUEST_MCG_CAP & ~MCG_CAP_COUNT & ~MCG_CTL_P )
     {
         dprintk(XENLOG_G_ERR, "%s restore: unsupported MCA capabilities"
                 " %#" PRIx64 " for d%d:v%u (supported: %#Lx)\n",
                 is_hvm_vcpu(v) ? "HVM" : "PV", caps, v->domain->domain_id,
-                v->vcpu_id, g_mcg_cap & ~MCG_CAP_COUNT);
+                v->vcpu_id, GUEST_MCG_CAP & ~MCG_CAP_COUNT);
         return -EPERM;
     }
 
@@ -175,11 +179,10 @@
                    *val);
         break;
     case MSR_IA32_MCG_CTL:
-        /* Always 0 if no CTL support */
+        /* Stick all 1's when CTL support, and 0's when no CTL support */
         if ( cur->arch.mcg_cap & MCG_CTL_P )
-            *val = vmce->mcg_ctl & h_mcg_ctl;
-        mce_printk(MCE_VERBOSE, "MCE: rdmsr MCG_CTL 0x%"PRIx64"\n",
-                   *val);
+            *val = ~0ULL;
+        mce_printk(MCE_VERBOSE, "MCE: rdmsr MCG_CTL 0x%"PRIx64"\n", *val);
         break;
     default:
         ret = mce_bank_msr(cur, msr) ? bank_mce_rdmsr(cur, msr, val) : 0;
@@ -287,15 +290,11 @@
     struct domain_mca_msrs *vmce = dom_vmce(cur->domain);
     int ret = 1;
 
-    if ( !g_mcg_cap )
-        return 0;
-
     spin_lock(&vmce->lock);
 
     switch ( msr )
     {
     case MSR_IA32_MCG_CTL:
-        vmce->mcg_ctl = val;
         break;
     case MSR_IA32_MCG_STATUS:
         vmce->mcg_status = val;
@@ -510,31 +509,6 @@
 }
 #endif
 
-int vmce_init(struct cpuinfo_x86 *c)
-{
-    u64 value;
-
-    rdmsrl(MSR_IA32_MCG_CAP, value);
-    /* For Guest vMCE usage */
-    g_mcg_cap = value & (MCG_CAP_COUNT | MCG_CTL_P | MCG_TES_P | MCG_SER_P);
-    if (value & MCG_CTL_P)
-        rdmsrl(MSR_IA32_MCG_CTL, h_mcg_ctl);
-
-    return 0;
-}
-
-static int mca_ctl_conflict(struct mcinfo_bank *bank, struct domain *d)
-{
-    if ( !bank || !d )
-        return 1;
-
-    /* Will MCE happen in host if If host mcg_ctl is 0? */
-    if ( ~d->arch.vmca_msrs->mcg_ctl & h_mcg_ctl )
-        return 1;
-
-    return 0;
-}
-
 static int is_hvm_vmce_ready(struct mcinfo_bank *bank, struct domain *d)
 {
     struct vcpu *v;
@@ -588,14 +562,6 @@
     if (no_vmce)
         return 0;
 
-    /* Guest has different MCE ctl value setting */
-    if (mca_ctl_conflict(bank, d))
-    {
-        dprintk(XENLOG_WARNING,
-          "No vmce, guest has different mca control setting\n");
-        return 0;
-    }
-
     return 1;
 }
 
diff -r 8067891037a6 xen/include/asm-x86/mce.h
--- a/xen/include/asm-x86/mce.h Thu Jul 19 20:48:25 2012 +0800
+++ b/xen/include/asm-x86/mce.h Tue Aug 07 04:26:56 2012 +0800
@@ -16,7 +16,6 @@
 struct domain_mca_msrs
 {
     /* Guest should not change below values after DOM boot up */
-    uint64_t mcg_ctl;
     uint64_t mcg_status;
     uint16_t nr_injection;
     struct list_head impact_header;

Attachment: 2_vmce_mcg_ctl_compatible.patch
Description: 2_vmce_mcg_ctl_compatible.patch

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.