|
[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 _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |