Liu, Jinsong
2012-Jul-06 18:28 UTC
[PATCH] Xen/MCE: remove mcg_ctl and other adjustment for future vMCE
Jan,
This patch is an incremental patch on top of the former uncontroversial one
(remove_mci_ctl_1.patch), updated according to your comments.
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@intel.com>
diff -r 6691bddc1c7a xen/arch/x86/cpu/mcheck/mce.c
--- a/xen/arch/x86/cpu/mcheck/mce.c Sat Jul 07 09:40:37 2012 +0800
+++ b/xen/arch/x86/cpu/mcheck/mce.c Sat Jul 07 09:49:13 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 6691bddc1c7a xen/arch/x86/cpu/mcheck/mce.h
--- a/xen/arch/x86/cpu/mcheck/mce.h Sat Jul 07 09:40:37 2012 +0800
+++ b/xen/arch/x86/cpu/mcheck/mce.h Sat Jul 07 09:49:13 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 6691bddc1c7a xen/arch/x86/cpu/mcheck/vmce.c
--- a/xen/arch/x86/cpu/mcheck/vmce.c Sat Jul 07 09:40:37 2012 +0800
+++ b/xen/arch/x86/cpu/mcheck/vmce.c Sat Jul 07 09:49:13 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,16 @@
*val);
break;
case MSR_IA32_MCG_CTL:
- /* Always 0 if 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 = ~0UL;
+ mce_printk(MCE_VERBOSE, "MCE: rdmsr MCG_CTL
0x%"PRIx64"\n", *val);
+ }
+ else
+ {
+ mce_printk(MCE_QUIET, "MCE: no MCG_CTL\n");
+ ret = -1;
+ }
break;
default:
ret = mce_bank_msr(cur, msr) ? bank_mce_rdmsr(cur, msr, val) : 0;
@@ -287,15 +296,18 @@
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;
+ if ( cur->arch.mcg_cap & MCG_CTL_P )
+ ;
+ else
+ {
+ mce_printk(MCE_QUIET, "MCE: no MCG_CTL\n");
+ ret = -1;
+ }
break;
case MSR_IA32_MCG_STATUS:
vmce->mcg_status = val;
@@ -510,31 +522,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 +575,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 6691bddc1c7a xen/include/asm-x86/mce.h
--- a/xen/include/asm-x86/mce.h Sat Jul 07 09:40:37 2012 +0800
+++ b/xen/include/asm-x86/mce.h Sat Jul 07 09:49:13 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;
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Auld, Will
2012-Jul-09 20:40 UTC
Re: [PATCH] Xen/MCE: remove mcg_ctl and other adjustment for future vMCE
[This email is either empty or too large to be displayed at this time]
Luck, Tony
2012-Jul-09 20:57 UTC
Re: [PATCH] Xen/MCE: remove mcg_ctl and other adjustment for future vMCE
Looks OK.
-Tony
-----Original Message-----
From: Auld, Will
Sent: Monday, July 09, 2012 1:40 PM
To: Liu, Jinsong; Jan Beulich
Cc: Keir (Xen.org); Ian Campbell; Christoph Egger; Luck, Tony; Jiang, Yunhong;
xen-devel@lists.xensource.com; Auld, Will
Subject: RE: [PATCH] Xen/MCE: remove mcg_ctl and other adjustment for future
vMCE
Jinsong and I now agree that we need to handle the case of guest migrating from
older versions of Xen (current version) that are executing with a higher MC bank
count than the newer version of Xen would create. As such I have updated the
implementation document to include this functionality. The added text is at the
bottom of the Live Migration section that starts on page 8. The new text is
offset by "-=-=" above and below it.
Comments?
Thanks,
Will
-----Original Message-----
From: Liu, Jinsong
Sent: Friday, July 06, 2012 11:28 AM
To: Jan Beulich
Cc: Keir (Xen.org); Ian Campbell; Christoph Egger; Luck, Tony; Auld, Will;
Jiang, Yunhong; xen-devel@lists.xensource.com
Subject: [PATCH] Xen/MCE: remove mcg_ctl and other adjustment for future vMCE
Jan,
This patch is an incremental patch on top of the former uncontroversial one
(remove_mci_ctl_1.patch), updated according to your comments.
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@intel.com>
diff -r 6691bddc1c7a xen/arch/x86/cpu/mcheck/mce.c
--- a/xen/arch/x86/cpu/mcheck/mce.c Sat Jul 07 09:40:37 2012 +0800
+++ b/xen/arch/x86/cpu/mcheck/mce.c Sat Jul 07 09:49:13 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 6691bddc1c7a xen/arch/x86/cpu/mcheck/mce.h
--- a/xen/arch/x86/cpu/mcheck/mce.h Sat Jul 07 09:40:37 2012 +0800
+++ b/xen/arch/x86/cpu/mcheck/mce.h Sat Jul 07 09:49:13 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
6691bddc1c7a xen/arch/x86/cpu/mcheck/vmce.c
--- a/xen/arch/x86/cpu/mcheck/vmce.c Sat Jul 07 09:40:37 2012 +0800
+++ b/xen/arch/x86/cpu/mcheck/vmce.c Sat Jul 07 09:49:13 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,16 @@
*val);
break;
case MSR_IA32_MCG_CTL:
- /* Always 0 if 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 = ~0UL;
+ mce_printk(MCE_VERBOSE, "MCE: rdmsr MCG_CTL
0x%"PRIx64"\n", *val);
+ }
+ else
+ {
+ mce_printk(MCE_QUIET, "MCE: no MCG_CTL\n");
+ ret = -1;
+ }
break;
default:
ret = mce_bank_msr(cur, msr) ? bank_mce_rdmsr(cur, msr, val) : 0; @@
-287,15 +296,18 @@
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;
+ if ( cur->arch.mcg_cap & MCG_CTL_P )
+ ;
+ else
+ {
+ mce_printk(MCE_QUIET, "MCE: no MCG_CTL\n");
+ ret = -1;
+ }
break;
case MSR_IA32_MCG_STATUS:
vmce->mcg_status = val;
@@ -510,31 +522,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 +575,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 6691bddc1c7a xen/include/asm-x86/mce.h
--- a/xen/include/asm-x86/mce.h Sat Jul 07 09:40:37 2012 +0800
+++ b/xen/include/asm-x86/mce.h Sat Jul 07 09:49:13 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;