* [PATCH] Xen/vMCE: remove is_vmce_ready check
@ 2013-05-13 16:02 Liu, Jinsong
2013-05-16 8:15 ` Christoph Egger
0 siblings, 1 reply; 3+ messages in thread
From: Liu, Jinsong @ 2013-05-13 16:02 UTC (permalink / raw)
To: Jan Beulich, Christoph Egger, xen-devel
[-- Attachment #1: Type: text/plain, Size: 5116 bytes --]
>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
[-- Attachment #2: 0001-Xen-vMCE-remove-is_vmce_ready-check.patch --]
[-- Type: application/octet-stream, Size: 4971 bytes --]
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
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] Xen/vMCE: remove is_vmce_ready check
2013-05-13 16:02 [PATCH] Xen/vMCE: remove is_vmce_ready check Liu, Jinsong
@ 2013-05-16 8:15 ` Christoph Egger
2013-05-16 8:30 ` Jan Beulich
0 siblings, 1 reply; 3+ messages in thread
From: Christoph Egger @ 2013-05-16 8:15 UTC (permalink / raw)
To: Liu, Jinsong; +Cc: Jan Beulich, 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);
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Xen/vMCE: remove is_vmce_ready check
2013-05-16 8:15 ` Christoph Egger
@ 2013-05-16 8:30 ` Jan Beulich
0 siblings, 0 replies; 3+ messages in thread
From: Jan Beulich @ 2013-05-16 8:30 UTC (permalink / raw)
To: Christoph Egger; +Cc: Jinsong Liu, xen-devel
>>> 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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-05-16 8:30 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-05-13 16:02 [PATCH] Xen/vMCE: remove is_vmce_ready check Liu, Jinsong
2013-05-16 8:15 ` Christoph Egger
2013-05-16 8:30 ` Jan Beulich
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.