From 50fbb875fcf567c75fdbc16c64491aa8e72746b9 Mon Sep 17 00:00:00 2001
From: Liu Jinsong <jinsong.liu@intel.com>
Date: Sat, 27 Apr 2013 22:37:35 +0800
Subject: [PATCH] Xen/vMCE: remove is_vmce_ready check
Remove is_vmce_ready() check since
1. it''s problematic and overkilled: it checks if virq bind to dom0
mcelog
driver. That''s not correct, since mcelog is just a dom0 driver used to
log
error info, irrelated to dom0 vmce injection. It''s also overkilled,
defaulty
dom0 disabled mcelog driver, under such case this checking would resulting
in system crash:
(XEN) MCE: This error page is ownded by DOM 0
(XEN) DOM0 not ready for vMCE
(XEN) domain_crash called from mcaction.c:133
(XEN) Domain 0 reported crashed by domain 32767 on cpu#31:
(XEN) Domain 0 crashed: rebooting machine in 5 seconds.
(XEN) Resetting with ACPI MEMORY or I/O RESET_REG.
2. it''s redundant: hypervisor in fact has checked
1). whether dom0 vmce ready or not (at inject_vmce()), via checking
vmce trap callback, to make sure vmce injection OK;
2). whether dom0 mcelog driver ready or not (at mce_softirq()), via
virq binding, to make sure error log works;
3. it''s deprecated: for hvm, it checks whether guest vcpu has different
virtual family/model with that of host pcpu --> that''s deprecated,
since
vMCE has changed a lot, not bound to host MCE any more.
Signed-off-by: Liu Jinsong <jinsong.liu@intel.com>
---
xen/arch/x86/cpu/mcheck/mcaction.c | 6 ---
xen/arch/x86/cpu/mcheck/vmce.c | 68 ------------------------------------
xen/arch/x86/cpu/mcheck/vmce.h | 1 -
3 files changed, 0 insertions(+), 75 deletions(-)
diff --git a/xen/arch/x86/cpu/mcheck/mcaction.c
b/xen/arch/x86/cpu/mcheck/mcaction.c
index 0ac5b45..adf2ded 100644
--- a/xen/arch/x86/cpu/mcheck/mcaction.c
+++ b/xen/arch/x86/cpu/mcheck/mcaction.c
@@ -83,12 +83,6 @@ mc_memerr_dhandler(struct mca_binfo *binfo,
ASSERT(d);
gfn = get_gpfn_from_mfn((bank->mc_addr) >>
PAGE_SHIFT);
- if ( !is_vmce_ready(bank, d) )
- {
- printk("DOM%d not ready for vMCE\n",
d->domain_id);
- goto vmce_failed;
- }
-
if ( unmmap_broken_page(d, _mfn(mfn), gfn) )
{
printk("Unmap broken memory %lx for DOM%d
failed\n",
diff --git a/xen/arch/x86/cpu/mcheck/vmce.c b/xen/arch/x86/cpu/mcheck/vmce.c
index 7d3fac7..af3b491 100644
--- a/xen/arch/x86/cpu/mcheck/vmce.c
+++ b/xen/arch/x86/cpu/mcheck/vmce.c
@@ -413,74 +413,6 @@ int fill_vmsr_data(struct mcinfo_bank *mc_bank, struct
domain *d,
return 0;
}
-static int is_hvm_vmce_ready(struct mcinfo_bank *bank, struct domain *d)
-{
- struct vcpu *v;
- int no_vmce = 0, i;
-
- if (!is_hvm_domain(d))
- return 0;
-
- /* kill guest if not enabled vMCE */
- for_each_vcpu(d, v)
- {
- if (!(v->arch.hvm_vcpu.guest_cr[4] & X86_CR4_MCE))
- {
- no_vmce = 1;
- break;
- }
-
- if (!mce_broadcast)
- break;
- }
-
- if (no_vmce)
- return 0;
-
- /* Guest has virtualized family/model information */
- for ( i = 0; i < MAX_CPUID_INPUT; i++ )
- {
- if (d->arch.cpuids[i].input[0] == 0x1)
- {
- uint32_t veax = d->arch.cpuids[i].eax, vfam, vmod;
-
- vfam = (veax >> 8) & 15;
- vmod = (veax >> 4) & 15;
-
- if (vfam == 0x6 || vfam == 0xf)
- vmod += ((veax >> 16) & 0xF) << 4;
- if (vfam == 0xf)
- vfam += (veax >> 20) & 0xff;
-
- if ( ( vfam != boot_cpu_data.x86 ) ||
- (vmod != boot_cpu_data.x86_model) )
- {
- dprintk(XENLOG_WARNING,
- "No vmce for different virtual family/model
cpuid\n");
- no_vmce = 1;
- }
- break;
- }
- }
-
- if (no_vmce)
- return 0;
-
- return 1;
-}
-
-int is_vmce_ready(struct mcinfo_bank *bank, struct domain *d)
-{
- if ( d == dom0)
- return dom0_vmce_enabled();
-
- /* No vMCE to HVM guest now */
- if ( is_hvm_domain(d) )
- return is_hvm_vmce_ready(bank, d);
-
- return 0;
-}
-
/* It''s said some ram is setup as mmio_direct for UC cache attribute
*/
#define P2M_UNMAP_TYPES (p2m_to_mask(p2m_ram_rw) \
| p2m_to_mask(p2m_ram_logdirty) \
diff --git a/xen/arch/x86/cpu/mcheck/vmce.h b/xen/arch/x86/cpu/mcheck/vmce.h
index 7263deb..6b2c95a 100644
--- a/xen/arch/x86/cpu/mcheck/vmce.h
+++ b/xen/arch/x86/cpu/mcheck/vmce.h
@@ -8,7 +8,6 @@ int vmce_init(struct cpuinfo_x86 *c);
#define dom0_vmce_enabled() (dom0 && dom0->max_vcpus &&
dom0->vcpu[0] \
&& guest_enabled_event(dom0->vcpu[0], VIRQ_MCA))
-int is_vmce_ready(struct mcinfo_bank *bank, struct domain *d);
int unmmap_broken_page(struct domain *d, mfn_t mfn, unsigned long gfn);
int vmce_intel_rdmsr(const struct vcpu *, uint32_t msr, uint64_t *val);
--
1.7.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
On 13.05.13 18:02, Liu, Jinsong wrote:> From 50fbb875fcf567c75fdbc16c64491aa8e72746b9 Mon Sep 17 00:00:00 2001 > From: Liu Jinsong <jinsong.liu@intel.com> > Date: Sat, 27 Apr 2013 22:37:35 +0800 > Subject: [PATCH] Xen/vMCE: remove is_vmce_ready check > > Remove is_vmce_ready() check since > 1. it''s problematic and overkilled: it checks if virq bind to dom0 mcelog > driver. That''s not correct, since mcelog is just a dom0 driver used to log > error info, irrelated to dom0 vmce injection. It''s also overkilled, defaulty > dom0 disabled mcelog driver, under such case this checking would resulting > in system crash: > (XEN) MCE: This error page is ownded by DOM 0 > (XEN) DOM0 not ready for vMCE > (XEN) domain_crash called from mcaction.c:133 > (XEN) Domain 0 reported crashed by domain 32767 on cpu#31: > (XEN) Domain 0 crashed: rebooting machine in 5 seconds. > (XEN) Resetting with ACPI MEMORY or I/O RESET_REG. > > 2. it''s redundant: hypervisor in fact has checked > 1). whether dom0 vmce ready or not (at inject_vmce()), via checking > vmce trap callback, to make sure vmce injection OK; > 2). whether dom0 mcelog driver ready or not (at mce_softirq()), via > virq binding, to make sure error log works; > > 3. it''s deprecated: for hvm, it checks whether guest vcpu has different > virtual family/model with that of host pcpu --> that''s deprecated, since > vMCE has changed a lot, not bound to host MCE any more. > > Signed-off-by: Liu Jinsong <jinsong.liu@intel.com>Acked-by: Christoph Egger <chegger@amazon.de> P.S.: I like to see this backported to Xen 4.2. However, the Xen 4.2 version may need an adaption as point 3. does not count for Xen 4.2. Christoph> --- > xen/arch/x86/cpu/mcheck/mcaction.c | 6 --- > xen/arch/x86/cpu/mcheck/vmce.c | 68 ------------------------------------ > xen/arch/x86/cpu/mcheck/vmce.h | 1 - > 3 files changed, 0 insertions(+), 75 deletions(-) > > diff --git a/xen/arch/x86/cpu/mcheck/mcaction.c b/xen/arch/x86/cpu/mcheck/mcaction.c > index 0ac5b45..adf2ded 100644 > --- a/xen/arch/x86/cpu/mcheck/mcaction.c > +++ b/xen/arch/x86/cpu/mcheck/mcaction.c > @@ -83,12 +83,6 @@ mc_memerr_dhandler(struct mca_binfo *binfo, > ASSERT(d); > gfn = get_gpfn_from_mfn((bank->mc_addr) >> PAGE_SHIFT); > > - if ( !is_vmce_ready(bank, d) ) > - { > - printk("DOM%d not ready for vMCE\n", d->domain_id); > - goto vmce_failed; > - } > - > if ( unmmap_broken_page(d, _mfn(mfn), gfn) ) > { > printk("Unmap broken memory %lx for DOM%d failed\n", > diff --git a/xen/arch/x86/cpu/mcheck/vmce.c b/xen/arch/x86/cpu/mcheck/vmce.c > index 7d3fac7..af3b491 100644 > --- a/xen/arch/x86/cpu/mcheck/vmce.c > +++ b/xen/arch/x86/cpu/mcheck/vmce.c > @@ -413,74 +413,6 @@ int fill_vmsr_data(struct mcinfo_bank *mc_bank, struct domain *d, > return 0; > } > > -static int is_hvm_vmce_ready(struct mcinfo_bank *bank, struct domain *d) > -{ > - struct vcpu *v; > - int no_vmce = 0, i; > - > - if (!is_hvm_domain(d)) > - return 0; > - > - /* kill guest if not enabled vMCE */ > - for_each_vcpu(d, v) > - { > - if (!(v->arch.hvm_vcpu.guest_cr[4] & X86_CR4_MCE)) > - { > - no_vmce = 1; > - break; > - } > - > - if (!mce_broadcast) > - break; > - } > - > - if (no_vmce) > - return 0; > - > - /* Guest has virtualized family/model information */ > - for ( i = 0; i < MAX_CPUID_INPUT; i++ ) > - { > - if (d->arch.cpuids[i].input[0] == 0x1) > - { > - uint32_t veax = d->arch.cpuids[i].eax, vfam, vmod; > - > - vfam = (veax >> 8) & 15; > - vmod = (veax >> 4) & 15; > - > - if (vfam == 0x6 || vfam == 0xf) > - vmod += ((veax >> 16) & 0xF) << 4; > - if (vfam == 0xf) > - vfam += (veax >> 20) & 0xff; > - > - if ( ( vfam != boot_cpu_data.x86 ) || > - (vmod != boot_cpu_data.x86_model) ) > - { > - dprintk(XENLOG_WARNING, > - "No vmce for different virtual family/model cpuid\n"); > - no_vmce = 1; > - } > - break; > - } > - } > - > - if (no_vmce) > - return 0; > - > - return 1; > -} > - > -int is_vmce_ready(struct mcinfo_bank *bank, struct domain *d) > -{ > - if ( d == dom0) > - return dom0_vmce_enabled(); > - > - /* No vMCE to HVM guest now */ > - if ( is_hvm_domain(d) ) > - return is_hvm_vmce_ready(bank, d); > - > - return 0; > -} > - > /* It''s said some ram is setup as mmio_direct for UC cache attribute */ > #define P2M_UNMAP_TYPES (p2m_to_mask(p2m_ram_rw) \ > | p2m_to_mask(p2m_ram_logdirty) \ > diff --git a/xen/arch/x86/cpu/mcheck/vmce.h b/xen/arch/x86/cpu/mcheck/vmce.h > index 7263deb..6b2c95a 100644 > --- a/xen/arch/x86/cpu/mcheck/vmce.h > +++ b/xen/arch/x86/cpu/mcheck/vmce.h > @@ -8,7 +8,6 @@ int vmce_init(struct cpuinfo_x86 *c); > #define dom0_vmce_enabled() (dom0 && dom0->max_vcpus && dom0->vcpu[0] \ > && guest_enabled_event(dom0->vcpu[0], VIRQ_MCA)) > > -int is_vmce_ready(struct mcinfo_bank *bank, struct domain *d); > int unmmap_broken_page(struct domain *d, mfn_t mfn, unsigned long gfn); > > int vmce_intel_rdmsr(const struct vcpu *, uint32_t msr, uint64_t *val); >
>>> On 16.05.13 at 10:15, Christoph Egger <chegger@amazon.de> wrote: > On 13.05.13 18:02, Liu, Jinsong wrote: >> From 50fbb875fcf567c75fdbc16c64491aa8e72746b9 Mon Sep 17 00:00:00 2001 >> From: Liu Jinsong <jinsong.liu@intel.com> >> Date: Sat, 27 Apr 2013 22:37:35 +0800 >> Subject: [PATCH] Xen/vMCE: remove is_vmce_ready check >> >> Remove is_vmce_ready() check since >> 1. it''s problematic and overkilled: it checks if virq bind to dom0 mcelog >> driver. That''s not correct, since mcelog is just a dom0 driver used to log >> error info, irrelated to dom0 vmce injection. It''s also overkilled, defaulty >> dom0 disabled mcelog driver, under such case this checking would resulting >> in system crash: >> (XEN) MCE: This error page is ownded by DOM 0 >> (XEN) DOM0 not ready for vMCE >> (XEN) domain_crash called from mcaction.c:133 >> (XEN) Domain 0 reported crashed by domain 32767 on cpu#31: >> (XEN) Domain 0 crashed: rebooting machine in 5 seconds. >> (XEN) Resetting with ACPI MEMORY or I/O RESET_REG. >> >> 2. it''s redundant: hypervisor in fact has checked >> 1). whether dom0 vmce ready or not (at inject_vmce()), via checking >> vmce trap callback, to make sure vmce injection OK; >> 2). whether dom0 mcelog driver ready or not (at mce_softirq()), via >> virq binding, to make sure error log works; >> >> 3. it''s deprecated: for hvm, it checks whether guest vcpu has different >> virtual family/model with that of host pcpu --> that''s deprecated, since >> vMCE has changed a lot, not bound to host MCE any more. >> >> Signed-off-by: Liu Jinsong <jinsong.liu@intel.com> > > Acked-by: Christoph Egger <chegger@amazon.de> > > P.S.: I like to see this backported to Xen 4.2. However, > the Xen 4.2 version may need an adaption as point 3. does not > count for Xen 4.2.If you think this requires parts to be kept, then please propose a suitable backport. Jan