RE: [PATCH] DOM0: Adding MCA Loging support in DOM0

RE: [PATCH] DOM0: Adding MCA Loging support in DOM0

[Ke, Liping]

[-- Attachment #1: Type: text/plain, Size: 2173 bytes --]

Hi, Jan

Thanks a lot for the detailed review!
I resend the verified patch according to the below agreement.

Regards,
Criping

Jan Beulich wrote:
>>>> --- a/arch/x86_64/Kconfig	Tue Jun 09 09:58:11 2009 +0800
>>>> +++ b/arch/x86_64/Kconfig	Wed Jun 10 14:12:55 2009 +0800 @@ -472,7
>>>> +472,6 @@ 
>>>> 
>>>> config X86_MCE
>>>> 	bool "Machine check support" if EMBEDDED
>>>> -	depends on !X86_64_XEN
>>>> 	default y
>>>> 	help
>>>> 	   Include a machine check error handler to report hardware
>>>> errors. 
>>> 
>>> Shouldn't this rather change the dependency to XEN_PRIVILEGED_GUEST?
>>> 
>> Native depends on this config too, so have about change it like this
>> depends on   !X86_64_XEN || ( X86_64_XEN && XEN_PRIVILEGED_GUEST)
> 
> Oh, right. But then why not simply using !XEN_UNPRIVILEGED_GUEST?

Yes, it's very nice.

> 
>>>> @@ -483,7 +482,7 @@
>>>> config X86_MCE_INTEL
>>>> 	bool "Intel MCE features"
>>>> 	depends on X86_MCE && X86_LOCAL_APIC
>>>> -	default y
>>>> +	default n
>>>> 	help
>>>> 	   Additional support for intel specific MCE features such as
>>>> 	   the thermal monitor.
>>> 
>>> This hunk should go.
>> 
>> This option will not appear under XEN, only for native?
>> depends on X86_MCE && X86_LOCAL_APIC && !X86_64_XEN
> 
> Whether it appears is not the question - you're changing the default
> value for no good reason.
> 
Yes. I need not change the default value now, it is true only in 
Native. We will not build those two specifc files.

>>>> 
>>>> +config X86_64_XEN_MCE
>>>> +	def_bool y
>>>> +	depends on X86_64_XEN && (X86_MCE_INTEL || X86_MCE_AMD)
>>>> +
>>> 
>>> I wonder if this wouldn't better be named X86_XEN_MCE (for
>>> consistency with a potential 32-bit implementation).
>>> 
>> 
>> This option will depends on X86_64_XEN && X86_MCE.
>> Yes, we could rename it. Yet this config file is under X86_64 sub
>> dir, also, seems 32bit system will not have mca support?
> 
> They do - see arch/i386/kernel/cpu/mcheck/.

Actually I mean 32 bit system might have rare hardware support. But
for consistency, yes, it's better to rename it as X86_XEN_MCE.
> 
> Jan



[-- Attachment #2: intel_mce_dom0.patch --]
[-- Type: application/octet-stream, Size: 8802 bytes --]

This patch is to add MCA logging support in DOM0

When an MCE/CMCI error happens (or by polling), the related error information
will be sent to DOM0 by XEN. This patch will help to fetch the xen-logged
information by hypercall and then convert XEN-format log into Linux format
MCELOG. It makes using current available mcelog tools for native Linux
possible.

With this patch, after mce/cmci error log information is sent to DOM0,
running mcelog tools in DOM0, you will get same detailed decoded mce
information as in Native Linux.

Signed-Off-By: Liping Ke<liping.ke@intel.com>
Signed-Off-By: Yunhong Jiang<yunhong.jiang@intel.com>

diff -r ef96f99e1084 arch/x86_64/Kconfig
--- a/arch/x86_64/Kconfig	Thu Jun 11 10:14:50 2009 +0800
+++ b/arch/x86_64/Kconfig	Thu Jun 11 10:25:16 2009 +0800
@@ -471,9 +471,9 @@
 	bool
 
 config X86_MCE
-	bool "Machine check support" if EMBEDDED
-	depends on !X86_64_XEN
-	default y
+	bool "Machine check support"
+	depends on (!XEN_UNPRIVILEGED_GUEST)
+	default n
 	help
 	   Include a machine check error handler to report hardware errors.
 	   This version will require the mcelog utility to decode some
@@ -482,7 +482,7 @@
 
 config X86_MCE_INTEL
 	bool "Intel MCE features"
-	depends on X86_MCE && X86_LOCAL_APIC
+	depends on X86_MCE && X86_LOCAL_APIC && !X86_64_XEN
 	default y
 	help
 	   Additional support for intel specific MCE features such as
@@ -490,12 +490,16 @@
 
 config X86_MCE_AMD
 	bool "AMD MCE features"
-	depends on X86_MCE && X86_LOCAL_APIC
+	depends on X86_MCE && X86_LOCAL_APIC && !X86_64_XEN
 	default y
 	help
 	   Additional support for AMD specific MCE features such as
 	   the DRAM Error Threshold.
 
+config X86_XEN_MCE
+	def_bool y
+	depends on X86_64_XEN && X86_MCE
+
 config KEXEC
 	bool "kexec system call (EXPERIMENTAL)"
 	depends on EXPERIMENTAL && !XEN_UNPRIVILEGED_GUEST
diff -r ef96f99e1084 arch/x86_64/kernel/Makefile
--- a/arch/x86_64/kernel/Makefile	Thu Jun 11 10:14:50 2009 +0800
+++ b/arch/x86_64/kernel/Makefile	Thu Jun 11 10:25:16 2009 +0800
@@ -13,6 +13,7 @@
 obj-$(CONFIG_STACKTRACE)	+= stacktrace.o
 obj-$(CONFIG_X86_MCE)         += mce.o
 obj-$(CONFIG_X86_MCE_INTEL)	+= mce_intel.o
+obj-$(CONFIG_X86_XEN_MCE)	+= mce_dom0.o
 obj-$(CONFIG_X86_MCE_AMD)	+= mce_amd.o
 obj-$(CONFIG_MTRR)		+= ../../i386/kernel/cpu/mtrr/
 obj-$(CONFIG_ACPI)		+= acpi/
diff -r ef96f99e1084 arch/x86_64/kernel/entry-xen.S
--- a/arch/x86_64/kernel/entry-xen.S	Thu Jun 11 10:14:50 2009 +0800
+++ b/arch/x86_64/kernel/entry-xen.S	Thu Jun 11 10:25:16 2009 +0800
@@ -1258,13 +1258,8 @@
 
 #ifdef CONFIG_X86_MCE
 	/* runs on exception stack */
-ENTRY(machine_check)
-	INTR_FRAME
-	pushq $0
-	CFI_ADJUST_CFA_OFFSET 8	
-	paranoidentry do_machine_check
-	jmp paranoid_exit1
-	CFI_ENDPROC
+KPROBE_ENTRY(machine_check)
+	zeroentry do_machine_check
 END(machine_check)
 #endif
 
diff -r ef96f99e1084 arch/x86_64/kernel/mce.c
--- a/arch/x86_64/kernel/mce.c	Thu Jun 11 10:14:50 2009 +0800
+++ b/arch/x86_64/kernel/mce.c	Thu Jun 11 10:25:16 2009 +0800
@@ -276,9 +276,16 @@
 
 /*
  * Periodic polling timer for "silent" machine check errors.
- */
+ * We will disable polling in DOM0 since all CMCI/Polling
+ * mechanism will be done in XEN for Intel CPUs
+*/
 
+#if defined (CONFIG_X86_XEN_MCE)
+static int check_interval = 0; /* disable polling */
+#else
 static int check_interval = 5 * 60; /* 5 minutes */
+#endif
+
 static void mcheck_timer(void *data);
 static DECLARE_WORK(mcheck_work, mcheck_timer, NULL);
 
@@ -365,6 +372,7 @@
 
 }			
 
+#ifndef CONFIG_X86_64_XEN
 static void __cpuinit mce_cpu_features(struct cpuinfo_x86 *c)
 {
 	switch (c->x86_vendor) {
@@ -378,7 +386,7 @@
 		break;
 	}
 }
-
+#endif
 /* 
  * Called for each booted CPU to set up machine checks.
  * Must be called with preempt off. 
@@ -395,7 +403,9 @@
 		return;
 
 	mce_init(NULL);
+#ifndef CONFIG_X86_64_XEN
 	mce_cpu_features(c);
+#endif
 }
 
 /*
@@ -649,6 +659,8 @@
 };
 #endif
 
+struct mc_info *g_mi;
+extern void bind_virq_for_mce(void);
 static __init int mce_init_device(void)
 {
 	int err;
@@ -664,6 +676,13 @@
 
 	register_hotcpu_notifier(&mce_cpu_notifier);
 	misc_register(&mce_log_device);
+
+    /*Register vIRQ handler for MCE LOG processing*/
+#if defined(CONFIG_X86_XEN_MCE)
+    printk(KERN_DEBUG "MCE: bind virq for DOM0 Logging\n");
+    bind_virq_for_mce();
+#endif
+
 	return err;
 }
 
diff -r ef96f99e1084 arch/x86_64/kernel/mce_dom0.c
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/arch/x86_64/kernel/mce_dom0.c	Thu Jun 11 10:25:16 2009 +0800
@@ -0,0 +1,133 @@
+#include <linux/init.h>
+#include <linux/types.h>
+#include <linux/kernel.h>
+#include <xen/interface/xen.h>
+#include <xen/evtchn.h>
+#include <xen/interface/vcpu.h>
+#include <asm/hypercall.h>
+#include <asm/mce.h>
+
+static int convert_log(struct mc_info *mi)
+{
+	struct mcinfo_common *mic = NULL;
+	struct mcinfo_global *mc_global;
+	struct mcinfo_bank *mc_bank;
+	struct mce m;
+
+	x86_mcinfo_lookup(mic, mi, MC_TYPE_GLOBAL);
+	if (mic == NULL)
+	{
+		printk(KERN_ERR "DOM0_MCE_LOG: global data is NULL\n");
+		return -1;
+	}
+
+	mc_global = (struct mcinfo_global*)mic;
+	m.mcgstatus = mc_global->mc_gstatus;
+	m.cpu = mc_global->mc_coreid;/*for test*/
+	x86_mcinfo_lookup(mic, mi, MC_TYPE_BANK);
+	do
+	{
+		if (mic == NULL || mic->size == 0)
+			break;
+		if (mic->type == MC_TYPE_BANK)
+		{
+			mc_bank = (struct mcinfo_bank*)mic;
+			m.misc = mc_bank->mc_misc;
+			m.status = mc_bank->mc_status;
+			m.addr = mc_bank->mc_addr;
+			m.tsc = mc_bank->mc_tsc;
+			m.res1 = mc_bank->mc_ctrl2;
+			m.bank = mc_bank->mc_bank;
+			printk(KERN_DEBUG "[CPU%d, BANK%d, addr %llx, state %llx]\n", 
+						m.bank, m.cpu, m.addr, m.status);
+			/*log this record*/
+			mce_log(&m);
+		}
+		mic = x86_mcinfo_next(mic);
+	}while (1);
+
+	return 0;
+}
+
+extern struct mc_info *g_mi;
+
+/*dom0 mce virq handler, logging physical mce error info*/
+
+static irqreturn_t mce_dom0_interrupt(int irq, void *dev_id,
+									struct pt_regs *regs)
+{
+	xen_mc_t mc_op;
+	int result = 0;
+
+	printk(KERN_DEBUG "MCE_DOM0_LOG: enter dom0 mce vIRQ handler\n");
+	mc_op.cmd = XEN_MC_fetch;
+	mc_op.interface_version = XEN_MCA_INTERFACE_VERSION;
+	set_xen_guest_handle(mc_op.u.mc_fetch.data, g_mi);
+urgent:
+	mc_op.u.mc_fetch.flags = XEN_MC_URGENT;
+	result = HYPERVISOR_mca(&mc_op);
+	if (result || mc_op.u.mc_fetch.flags & XEN_MC_NODATA ||
+			mc_op.u.mc_fetch.flags & XEN_MC_FETCHFAILED)
+	{
+		printk(KERN_DEBUG "MCE_DOM0_LOG: No more urgent data\n");
+		goto nonurgent;
+	}
+	else
+	{
+		result = convert_log(g_mi);
+		if (result) {
+			printk(KERN_ERR "MCE_DOM0_LOG: Log conversion failed\n");
+			goto end;
+		}
+		/* After fetching the telem from DOM0, we need to dec the telem's
+		 * refcnt and release the entry. The telem is reserved and inc
+		 * refcnt when filling the telem.
+		 */
+		mc_op.u.mc_fetch.flags = XEN_MC_URGENT | XEN_MC_ACK;
+		result = HYPERVISOR_mca(&mc_op);
+
+		goto urgent;
+	}
+nonurgent:
+	mc_op.u.mc_fetch.flags = XEN_MC_NONURGENT;
+	result = HYPERVISOR_mca(&mc_op);
+	if (result || mc_op.u.mc_fetch.flags & XEN_MC_NODATA ||
+			mc_op.u.mc_fetch.flags & XEN_MC_FETCHFAILED)
+	{
+		printk(KERN_DEBUG "MCE_DOM0_LOG: No more nonurgent data\n");
+		goto end;
+	}
+	else
+	{
+		result = convert_log(g_mi);
+		if (result) {
+			printk(KERN_ERR "MCE_DOM0_LOG: Log conversion failed\n");
+			goto end;
+		}
+		/* After fetching the telem from DOM0, we need to dec the telem's
+		 * refcnt and release the entry. The telem is reserved and inc
+		 * refcnt when filling the telem.
+		 */
+		mc_op.u.mc_fetch.flags = XEN_MC_NONURGENT | XEN_MC_ACK;
+		result = HYPERVISOR_mca(&mc_op);
+
+		goto nonurgent;
+	}
+end:
+	return IRQ_HANDLED;
+}
+
+void bind_virq_for_mce(void)
+{
+	int ret;
+
+	ret  = bind_virq_to_irqhandler(VIRQ_ARCH_0, 0, 
+		mce_dom0_interrupt, 0, "mce", NULL);
+
+	g_mi = kmalloc(sizeof(struct mc_info), GFP_KERNEL);
+	if ( ret<0 )
+	{
+		printk(KERN_ERR "MCE_DOM0_LOG: bind_virq for DOM0 failed\n");
+	}
+}
+
diff -r ef96f99e1084 include/asm-x86_64/mach-xen/asm/hypercall.h
--- a/include/asm-x86_64/mach-xen/asm/hypercall.h	Thu Jun 11 10:14:50 2009 +0800
+++ b/include/asm-x86_64/mach-xen/asm/hypercall.h	Thu Jun 11 10:25:16 2009 +0800
@@ -39,6 +39,7 @@
 
 #include <linux/string.h> /* memcpy() */
 #include <linux/stringify.h>
+#include <xen/interface/arch-x86/xen-mca.h>
 
 #ifndef __HYPERVISOR_H__
 # error "please don't include this file directly"
@@ -215,7 +216,13 @@
 	platform_op->interface_version = XENPF_INTERFACE_VERSION;
 	return _hypercall1(int, platform_op, platform_op);
 }
-
+static inline int __must_check
+HYPERVISOR_mca(
+	struct xen_mc *mc_op)
+{
+	mc_op->interface_version = XEN_MCA_INTERFACE_VERSION;
+	return _hypercall1(int, mca, mc_op);
+}
 static inline int __must_check
 HYPERVISOR_set_debugreg(
 	unsigned int reg, unsigned long value)

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

RE: [PATCH] DOM0: Adding MCA Loging support in DOM0

[Jan Beulich]

>>> "Ke, Liping" <liping.ke@intel.com> 11.06.09 04:42 >>>
A few small issue remain:

>--- a/arch/x86_64/Kconfig	Thu Jun 11 10:14:50 2009 +0800
>+++ b/arch/x86_64/Kconfig	Thu Jun 11 10:25:16 2009 +0800
>@@ -471,9 +471,9 @@
> 	bool
> 
> config X86_MCE
>-	bool "Machine check support" if EMBEDDED
>-	depends on !X86_64_XEN
>-	default y
>+	bool "Machine check support"
>+	depends on (!XEN_UNPRIVILEGED_GUEST)
>+	default n
> 	help
> 	   Include a machine check error handler to report hardware errors.
> 	   This version will require the mcelog utility to decode some

Once again you change a default value here for no apparent reason.

>--- a/arch/x86_64/kernel/mce.c	Thu Jun 11 10:14:50 2009 +0800
>+++ b/arch/x86_64/kernel/mce.c	Thu Jun 11 10:25:16 2009 +0800
>...
>@@ -395,7 +403,9 @@
> 		return;
> 
> 	mce_init(NULL);
>+#ifndef CONFIG_X86_64_XEN
> 	mce_cpu_features(c);
>+#endif
> }
> 
> /*

This conditional can be avoided if the preceding one would be moved inside
the function (to cover just the entire function body).

>@@ -649,6 +659,8 @@
> };
> #endif
> 
>+struct mc_info *g_mi;
>+extern void bind_virq_for_mce(void);
> static __init int mce_init_device(void)
> {
> 	int err;

g_mi appears to be no longer used outside of arch/x86_64/kernel/mce_dom0.c,
so it could be (statically) defined there instead of here.

Jan

RE: [PATCH] DOM0: Adding MCA Loging support in DOM0

[Ke, Liping]

[-- Attachment #1: Type: text/plain, Size: 1073 bytes --]

Hi, Jan

Thanks a lot! I modified them and resent it.

Regards,
Criping



>> config X86_MCE
>> -	bool "Machine check support" if EMBEDDED
>> -	depends on !X86_64_XEN
>> -	default y
>> +	bool "Machine check support"
>> +	depends on (!XEN_UNPRIVILEGED_GUEST)
>> +	default n
>> 	help
>> 	   Include a machine check error handler to report hardware errors.
>> 	   This version will require the mcelog utility to decode some
> 
> Once again you change a default value here for no apparent reason.

I originally plan not enable this option by default. But yes, it's fine to enable it.

>> +#ifndef CONFIG_X86_64_XEN
>> 	mce_cpu_features(c);
>> +#endif
>> }
>> 
>> /*
> 
> This conditional can be avoided if the preceding one would be moved
> inside the function (to cover just the entire function body).

Yes. I will change it.

> 
> g_mi appears to be no longer used outside of
> arch/x86_64/kernel/mce_dom0.c, so it could be (statically) defined
> there instead of here. 
> 

Yes. I need to clean this code after changes.

> Jan


[-- Attachment #2: intel_mce_dom0.patch --]
[-- Type: application/octet-stream, Size: 8642 bytes --]

This patch is to add MCA logging support in DOM0

When an MCE/CMCI error happens (or by polling), the related error information
will be sent to DOM0 by XEN. This patch will help to fetch the xen-logged
information by hypercall and then convert XEN-format log into Linux format
MCELOG. It makes using current available mcelog tools for native Linux
possible.

With this patch, after mce/cmci error log information is sent to DOM0,
running mcelog tools in DOM0, you will get same detailed decoded mce
information as in Native Linux.

Signed-Off-By: Liping Ke<liping.ke@intel.com>
Signed-Off-By: Yunhong Jiang<yunhong.jiang@intel.com>

diff -r ef96f99e1084 arch/x86_64/Kconfig
--- a/arch/x86_64/Kconfig	Thu Jun 11 10:14:50 2009 +0800
+++ b/arch/x86_64/Kconfig	Mon Jun 15 15:46:24 2009 +0800
@@ -471,8 +471,8 @@
 	bool
 
 config X86_MCE
-	bool "Machine check support" if EMBEDDED
-	depends on !X86_64_XEN
+	bool "Machine check support"
+	depends on (!XEN_UNPRIVILEGED_GUEST)
 	default y
 	help
 	   Include a machine check error handler to report hardware errors.
@@ -482,7 +482,7 @@
 
 config X86_MCE_INTEL
 	bool "Intel MCE features"
-	depends on X86_MCE && X86_LOCAL_APIC
+	depends on X86_MCE && X86_LOCAL_APIC && !X86_64_XEN
 	default y
 	help
 	   Additional support for intel specific MCE features such as
@@ -490,12 +490,16 @@
 
 config X86_MCE_AMD
 	bool "AMD MCE features"
-	depends on X86_MCE && X86_LOCAL_APIC
+	depends on X86_MCE && X86_LOCAL_APIC && !X86_64_XEN
 	default y
 	help
 	   Additional support for AMD specific MCE features such as
 	   the DRAM Error Threshold.
 
+config X86_XEN_MCE
+	def_bool y
+	depends on X86_64_XEN && X86_MCE
+
 config KEXEC
 	bool "kexec system call (EXPERIMENTAL)"
 	depends on EXPERIMENTAL && !XEN_UNPRIVILEGED_GUEST
diff -r ef96f99e1084 arch/x86_64/kernel/Makefile
--- a/arch/x86_64/kernel/Makefile	Thu Jun 11 10:14:50 2009 +0800
+++ b/arch/x86_64/kernel/Makefile	Mon Jun 15 15:46:24 2009 +0800
@@ -13,6 +13,7 @@
 obj-$(CONFIG_STACKTRACE)	+= stacktrace.o
 obj-$(CONFIG_X86_MCE)         += mce.o
 obj-$(CONFIG_X86_MCE_INTEL)	+= mce_intel.o
+obj-$(CONFIG_X86_XEN_MCE)	+= mce_dom0.o
 obj-$(CONFIG_X86_MCE_AMD)	+= mce_amd.o
 obj-$(CONFIG_MTRR)		+= ../../i386/kernel/cpu/mtrr/
 obj-$(CONFIG_ACPI)		+= acpi/
diff -r ef96f99e1084 arch/x86_64/kernel/entry-xen.S
--- a/arch/x86_64/kernel/entry-xen.S	Thu Jun 11 10:14:50 2009 +0800
+++ b/arch/x86_64/kernel/entry-xen.S	Mon Jun 15 15:46:24 2009 +0800
@@ -1258,13 +1258,8 @@
 
 #ifdef CONFIG_X86_MCE
 	/* runs on exception stack */
-ENTRY(machine_check)
-	INTR_FRAME
-	pushq $0
-	CFI_ADJUST_CFA_OFFSET 8	
-	paranoidentry do_machine_check
-	jmp paranoid_exit1
-	CFI_ENDPROC
+KPROBE_ENTRY(machine_check)
+	zeroentry do_machine_check
 END(machine_check)
 #endif
 
diff -r ef96f99e1084 arch/x86_64/kernel/mce.c
--- a/arch/x86_64/kernel/mce.c	Thu Jun 11 10:14:50 2009 +0800
+++ b/arch/x86_64/kernel/mce.c	Mon Jun 15 15:46:24 2009 +0800
@@ -276,9 +276,16 @@
 
 /*
  * Periodic polling timer for "silent" machine check errors.
- */
+ * We will disable polling in DOM0 since all CMCI/Polling
+ * mechanism will be done in XEN for Intel CPUs
+*/
 
+#if defined (CONFIG_X86_XEN_MCE)
+static int check_interval = 0; /* disable polling */
+#else
 static int check_interval = 5 * 60; /* 5 minutes */
+#endif
+
 static void mcheck_timer(void *data);
 static DECLARE_WORK(mcheck_work, mcheck_timer, NULL);
 
@@ -367,6 +374,7 @@
 
 static void __cpuinit mce_cpu_features(struct cpuinfo_x86 *c)
 {
+#ifndef CONFIG_X86_64_XEN
 	switch (c->x86_vendor) {
 	case X86_VENDOR_INTEL:
 		mce_intel_feature_init(c);
@@ -377,8 +385,8 @@
 	default:
 		break;
 	}
+#endif
 }
-
 /* 
  * Called for each booted CPU to set up machine checks.
  * Must be called with preempt off. 
@@ -649,6 +657,7 @@
 };
 #endif
 
+extern void bind_virq_for_mce(void);
 static __init int mce_init_device(void)
 {
 	int err;
@@ -664,6 +673,13 @@
 
 	register_hotcpu_notifier(&mce_cpu_notifier);
 	misc_register(&mce_log_device);
+
+    /*Register vIRQ handler for MCE LOG processing*/
+#if defined(CONFIG_X86_XEN_MCE)
+    printk(KERN_DEBUG "MCE: bind virq for DOM0 Logging\n");
+    bind_virq_for_mce();
+#endif
+
 	return err;
 }
 
diff -r ef96f99e1084 arch/x86_64/kernel/mce_dom0.c
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/arch/x86_64/kernel/mce_dom0.c	Mon Jun 15 15:46:24 2009 +0800
@@ -0,0 +1,133 @@
+#include <linux/init.h>
+#include <linux/types.h>
+#include <linux/kernel.h>
+#include <xen/interface/xen.h>
+#include <xen/evtchn.h>
+#include <xen/interface/vcpu.h>
+#include <asm/hypercall.h>
+#include <asm/mce.h>
+
+static int convert_log(struct mc_info *mi)
+{
+	struct mcinfo_common *mic = NULL;
+	struct mcinfo_global *mc_global;
+	struct mcinfo_bank *mc_bank;
+	struct mce m;
+
+	x86_mcinfo_lookup(mic, mi, MC_TYPE_GLOBAL);
+	if (mic == NULL)
+	{
+		printk(KERN_ERR "DOM0_MCE_LOG: global data is NULL\n");
+		return -1;
+	}
+
+	mc_global = (struct mcinfo_global*)mic;
+	m.mcgstatus = mc_global->mc_gstatus;
+	m.cpu = mc_global->mc_coreid;/*for test*/
+	x86_mcinfo_lookup(mic, mi, MC_TYPE_BANK);
+	do
+	{
+		if (mic == NULL || mic->size == 0)
+			break;
+		if (mic->type == MC_TYPE_BANK)
+		{
+			mc_bank = (struct mcinfo_bank*)mic;
+			m.misc = mc_bank->mc_misc;
+			m.status = mc_bank->mc_status;
+			m.addr = mc_bank->mc_addr;
+			m.tsc = mc_bank->mc_tsc;
+			m.res1 = mc_bank->mc_ctrl2;
+			m.bank = mc_bank->mc_bank;
+			printk(KERN_DEBUG "[CPU%d, BANK%d, addr %llx, state %llx]\n", 
+						m.bank, m.cpu, m.addr, m.status);
+			/*log this record*/
+			mce_log(&m);
+		}
+		mic = x86_mcinfo_next(mic);
+	}while (1);
+
+	return 0;
+}
+
+static struct mc_info *g_mi;
+
+/*dom0 mce virq handler, logging physical mce error info*/
+
+static irqreturn_t mce_dom0_interrupt(int irq, void *dev_id,
+									struct pt_regs *regs)
+{
+	xen_mc_t mc_op;
+	int result = 0;
+
+	printk(KERN_DEBUG "MCE_DOM0_LOG: enter dom0 mce vIRQ handler\n");
+	mc_op.cmd = XEN_MC_fetch;
+	mc_op.interface_version = XEN_MCA_INTERFACE_VERSION;
+	set_xen_guest_handle(mc_op.u.mc_fetch.data, g_mi);
+urgent:
+	mc_op.u.mc_fetch.flags = XEN_MC_URGENT;
+	result = HYPERVISOR_mca(&mc_op);
+	if (result || mc_op.u.mc_fetch.flags & XEN_MC_NODATA ||
+			mc_op.u.mc_fetch.flags & XEN_MC_FETCHFAILED)
+	{
+		printk(KERN_DEBUG "MCE_DOM0_LOG: No more urgent data\n");
+		goto nonurgent;
+	}
+	else
+	{
+		result = convert_log(g_mi);
+		if (result) {
+			printk(KERN_ERR "MCE_DOM0_LOG: Log conversion failed\n");
+			goto end;
+		}
+		/* After fetching the telem from DOM0, we need to dec the telem's
+		 * refcnt and release the entry. The telem is reserved and inc
+		 * refcnt when filling the telem.
+		 */
+		mc_op.u.mc_fetch.flags = XEN_MC_URGENT | XEN_MC_ACK;
+		result = HYPERVISOR_mca(&mc_op);
+
+		goto urgent;
+	}
+nonurgent:
+	mc_op.u.mc_fetch.flags = XEN_MC_NONURGENT;
+	result = HYPERVISOR_mca(&mc_op);
+	if (result || mc_op.u.mc_fetch.flags & XEN_MC_NODATA ||
+			mc_op.u.mc_fetch.flags & XEN_MC_FETCHFAILED)
+	{
+		printk(KERN_DEBUG "MCE_DOM0_LOG: No more nonurgent data\n");
+		goto end;
+	}
+	else
+	{
+		result = convert_log(g_mi);
+		if (result) {
+			printk(KERN_ERR "MCE_DOM0_LOG: Log conversion failed\n");
+			goto end;
+		}
+		/* After fetching the telem from DOM0, we need to dec the telem's
+		 * refcnt and release the entry. The telem is reserved and inc
+		 * refcnt when filling the telem.
+		 */
+		mc_op.u.mc_fetch.flags = XEN_MC_NONURGENT | XEN_MC_ACK;
+		result = HYPERVISOR_mca(&mc_op);
+
+		goto nonurgent;
+	}
+end:
+	return IRQ_HANDLED;
+}
+
+void bind_virq_for_mce(void)
+{
+	int ret;
+
+	ret  = bind_virq_to_irqhandler(VIRQ_ARCH_0, 0, 
+		mce_dom0_interrupt, 0, "mce", NULL);
+
+	g_mi = kmalloc(sizeof(struct mc_info), GFP_KERNEL);
+	if ( ret<0 )
+	{
+		printk(KERN_ERR "MCE_DOM0_LOG: bind_virq for DOM0 failed\n");
+	}
+}
+
diff -r ef96f99e1084 include/asm-x86_64/mach-xen/asm/hypercall.h
--- a/include/asm-x86_64/mach-xen/asm/hypercall.h	Thu Jun 11 10:14:50 2009 +0800
+++ b/include/asm-x86_64/mach-xen/asm/hypercall.h	Mon Jun 15 15:46:24 2009 +0800
@@ -39,6 +39,7 @@
 
 #include <linux/string.h> /* memcpy() */
 #include <linux/stringify.h>
+#include <xen/interface/arch-x86/xen-mca.h>
 
 #ifndef __HYPERVISOR_H__
 # error "please don't include this file directly"
@@ -215,7 +216,13 @@
 	platform_op->interface_version = XENPF_INTERFACE_VERSION;
 	return _hypercall1(int, platform_op, platform_op);
 }
-
+static inline int __must_check
+HYPERVISOR_mca(
+	struct xen_mc *mc_op)
+{
+	mc_op->interface_version = XEN_MCA_INTERFACE_VERSION;
+	return _hypercall1(int, mca, mc_op);
+}
 static inline int __must_check
 HYPERVISOR_set_debugreg(
 	unsigned int reg, unsigned long value)

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

RE: [PATCH] DOM0: Adding MCA Loging support in DOM0

[Jan Beulich]

Thanks - looks goo now to me (i.e. Acked-by: Jan Beulich <jbeulich@novell.com>
if we have such a concept in Xen).

Jan

>>> "Ke, Liping" <liping.ke@intel.com> 15.06.09 09:49 >>>
Hi, Jan

Thanks a lot! I modified them and resent it.

Regards,
Criping



>> config X86_MCE
>> -	bool "Machine check support" if EMBEDDED
>> -	depends on !X86_64_XEN
>> -	default y
>> +	bool "Machine check support"
>> +	depends on (!XEN_UNPRIVILEGED_GUEST)
>> +	default n
>> 	help
>> 	   Include a machine check error handler to report hardware errors.
>> 	   This version will require the mcelog utility to decode some
> 
> Once again you change a default value here for no apparent reason.

I originally plan not enable this option by default. But yes, it's fine to enable it.

>> +#ifndef CONFIG_X86_64_XEN
>> 	mce_cpu_features(c);
>> +#endif
>> }
>> 
>> /*
> 
> This conditional can be avoided if the preceding one would be moved
> inside the function (to cover just the entire function body).

Yes. I will change it.

> 
> g_mi appears to be no longer used outside of
> arch/x86_64/kernel/mce_dom0.c, so it could be (statically) defined
> there instead of here. 
> 

Yes. I need to clean this code after changes.

> Jan

RE: [PATCH] DOM0: Adding MCA Loging support in DOM0

[Ke, Liping]

[-- Attachment #1: Type: text/plain, Size: 309 bytes --]

Hi, keir

This is the final patch for adding MCA logging support in DOM0 which is Acked by Jan.
And thanks for Jan's help!

Regards,
Criping

Jan Beulich wrote:
> Thanks - looks goo now to me (i.e. Acked-by: Jan Beulich
> <jbeulich@novell.com> 
> if we have such a concept in Xen).
> 
> Jan
> 

[-- Attachment #2: intel_mce_dom0.patch --]
[-- Type: application/octet-stream, Size: 8685 bytes --]

This patch is to add MCA logging support in DOM0

When an MCE/CMCI error happens (or by polling), the related error information
will be sent to DOM0 by XEN. This patch will help to fetch the xen-logged
information by hypercall and then convert XEN-format log into Linux format
MCELOG. It makes using current available mcelog tools for native Linux
possible.

With this patch, after mce/cmci error log information is sent to DOM0,
running mcelog tools in DOM0, you will get same detailed decoded mce
information as in Native Linux.

Signed-Off-By: Liping Ke<liping.ke@intel.com>
Signed-Off-By: Yunhong Jiang<yunhong.jiang@intel.com>
Acked-By: Jan Beulich<jbeulich@novell.com>

diff -r ef96f99e1084 arch/x86_64/Kconfig
--- a/arch/x86_64/Kconfig	Thu Jun 11 10:14:50 2009 +0800
+++ b/arch/x86_64/Kconfig	Mon Jun 15 15:46:24 2009 +0800
@@ -471,8 +471,8 @@
 	bool
 
 config X86_MCE
-	bool "Machine check support" if EMBEDDED
-	depends on !X86_64_XEN
+	bool "Machine check support"
+	depends on (!XEN_UNPRIVILEGED_GUEST)
 	default y
 	help
 	   Include a machine check error handler to report hardware errors.
@@ -482,7 +482,7 @@
 
 config X86_MCE_INTEL
 	bool "Intel MCE features"
-	depends on X86_MCE && X86_LOCAL_APIC
+	depends on X86_MCE && X86_LOCAL_APIC && !X86_64_XEN
 	default y
 	help
 	   Additional support for intel specific MCE features such as
@@ -490,12 +490,16 @@
 
 config X86_MCE_AMD
 	bool "AMD MCE features"
-	depends on X86_MCE && X86_LOCAL_APIC
+	depends on X86_MCE && X86_LOCAL_APIC && !X86_64_XEN
 	default y
 	help
 	   Additional support for AMD specific MCE features such as
 	   the DRAM Error Threshold.
 
+config X86_XEN_MCE
+	def_bool y
+	depends on X86_64_XEN && X86_MCE
+
 config KEXEC
 	bool "kexec system call (EXPERIMENTAL)"
 	depends on EXPERIMENTAL && !XEN_UNPRIVILEGED_GUEST
diff -r ef96f99e1084 arch/x86_64/kernel/Makefile
--- a/arch/x86_64/kernel/Makefile	Thu Jun 11 10:14:50 2009 +0800
+++ b/arch/x86_64/kernel/Makefile	Mon Jun 15 15:46:24 2009 +0800
@@ -13,6 +13,7 @@
 obj-$(CONFIG_STACKTRACE)	+= stacktrace.o
 obj-$(CONFIG_X86_MCE)         += mce.o
 obj-$(CONFIG_X86_MCE_INTEL)	+= mce_intel.o
+obj-$(CONFIG_X86_XEN_MCE)	+= mce_dom0.o
 obj-$(CONFIG_X86_MCE_AMD)	+= mce_amd.o
 obj-$(CONFIG_MTRR)		+= ../../i386/kernel/cpu/mtrr/
 obj-$(CONFIG_ACPI)		+= acpi/
diff -r ef96f99e1084 arch/x86_64/kernel/entry-xen.S
--- a/arch/x86_64/kernel/entry-xen.S	Thu Jun 11 10:14:50 2009 +0800
+++ b/arch/x86_64/kernel/entry-xen.S	Mon Jun 15 15:46:24 2009 +0800
@@ -1258,13 +1258,8 @@
 
 #ifdef CONFIG_X86_MCE
 	/* runs on exception stack */
-ENTRY(machine_check)
-	INTR_FRAME
-	pushq $0
-	CFI_ADJUST_CFA_OFFSET 8	
-	paranoidentry do_machine_check
-	jmp paranoid_exit1
-	CFI_ENDPROC
+KPROBE_ENTRY(machine_check)
+	zeroentry do_machine_check
 END(machine_check)
 #endif
 
diff -r ef96f99e1084 arch/x86_64/kernel/mce.c
--- a/arch/x86_64/kernel/mce.c	Thu Jun 11 10:14:50 2009 +0800
+++ b/arch/x86_64/kernel/mce.c	Mon Jun 15 15:46:24 2009 +0800
@@ -276,9 +276,16 @@
 
 /*
  * Periodic polling timer for "silent" machine check errors.
- */
+ * We will disable polling in DOM0 since all CMCI/Polling
+ * mechanism will be done in XEN for Intel CPUs
+*/
 
+#if defined (CONFIG_X86_XEN_MCE)
+static int check_interval = 0; /* disable polling */
+#else
 static int check_interval = 5 * 60; /* 5 minutes */
+#endif
+
 static void mcheck_timer(void *data);
 static DECLARE_WORK(mcheck_work, mcheck_timer, NULL);
 
@@ -367,6 +374,7 @@
 
 static void __cpuinit mce_cpu_features(struct cpuinfo_x86 *c)
 {
+#ifndef CONFIG_X86_64_XEN
 	switch (c->x86_vendor) {
 	case X86_VENDOR_INTEL:
 		mce_intel_feature_init(c);
@@ -377,8 +385,8 @@
 	default:
 		break;
 	}
+#endif
 }
-
 /* 
  * Called for each booted CPU to set up machine checks.
  * Must be called with preempt off. 
@@ -649,6 +657,7 @@
 };
 #endif
 
+extern void bind_virq_for_mce(void);
 static __init int mce_init_device(void)
 {
 	int err;
@@ -664,6 +673,13 @@
 
 	register_hotcpu_notifier(&mce_cpu_notifier);
 	misc_register(&mce_log_device);
+
+    /*Register vIRQ handler for MCE LOG processing*/
+#if defined(CONFIG_X86_XEN_MCE)
+    printk(KERN_DEBUG "MCE: bind virq for DOM0 Logging\n");
+    bind_virq_for_mce();
+#endif
+
 	return err;
 }
 
diff -r ef96f99e1084 arch/x86_64/kernel/mce_dom0.c
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/arch/x86_64/kernel/mce_dom0.c	Mon Jun 15 15:46:24 2009 +0800
@@ -0,0 +1,133 @@
+#include <linux/init.h>
+#include <linux/types.h>
+#include <linux/kernel.h>
+#include <xen/interface/xen.h>
+#include <xen/evtchn.h>
+#include <xen/interface/vcpu.h>
+#include <asm/hypercall.h>
+#include <asm/mce.h>
+
+static int convert_log(struct mc_info *mi)
+{
+	struct mcinfo_common *mic = NULL;
+	struct mcinfo_global *mc_global;
+	struct mcinfo_bank *mc_bank;
+	struct mce m;
+
+	x86_mcinfo_lookup(mic, mi, MC_TYPE_GLOBAL);
+	if (mic == NULL)
+	{
+		printk(KERN_ERR "DOM0_MCE_LOG: global data is NULL\n");
+		return -1;
+	}
+
+	mc_global = (struct mcinfo_global*)mic;
+	m.mcgstatus = mc_global->mc_gstatus;
+	m.cpu = mc_global->mc_coreid;/*for test*/
+	x86_mcinfo_lookup(mic, mi, MC_TYPE_BANK);
+	do
+	{
+		if (mic == NULL || mic->size == 0)
+			break;
+		if (mic->type == MC_TYPE_BANK)
+		{
+			mc_bank = (struct mcinfo_bank*)mic;
+			m.misc = mc_bank->mc_misc;
+			m.status = mc_bank->mc_status;
+			m.addr = mc_bank->mc_addr;
+			m.tsc = mc_bank->mc_tsc;
+			m.res1 = mc_bank->mc_ctrl2;
+			m.bank = mc_bank->mc_bank;
+			printk(KERN_DEBUG "[CPU%d, BANK%d, addr %llx, state %llx]\n", 
+						m.bank, m.cpu, m.addr, m.status);
+			/*log this record*/
+			mce_log(&m);
+		}
+		mic = x86_mcinfo_next(mic);
+	}while (1);
+
+	return 0;
+}
+
+static struct mc_info *g_mi;
+
+/*dom0 mce virq handler, logging physical mce error info*/
+
+static irqreturn_t mce_dom0_interrupt(int irq, void *dev_id,
+									struct pt_regs *regs)
+{
+	xen_mc_t mc_op;
+	int result = 0;
+
+	printk(KERN_DEBUG "MCE_DOM0_LOG: enter dom0 mce vIRQ handler\n");
+	mc_op.cmd = XEN_MC_fetch;
+	mc_op.interface_version = XEN_MCA_INTERFACE_VERSION;
+	set_xen_guest_handle(mc_op.u.mc_fetch.data, g_mi);
+urgent:
+	mc_op.u.mc_fetch.flags = XEN_MC_URGENT;
+	result = HYPERVISOR_mca(&mc_op);
+	if (result || mc_op.u.mc_fetch.flags & XEN_MC_NODATA ||
+			mc_op.u.mc_fetch.flags & XEN_MC_FETCHFAILED)
+	{
+		printk(KERN_DEBUG "MCE_DOM0_LOG: No more urgent data\n");
+		goto nonurgent;
+	}
+	else
+	{
+		result = convert_log(g_mi);
+		if (result) {
+			printk(KERN_ERR "MCE_DOM0_LOG: Log conversion failed\n");
+			goto end;
+		}
+		/* After fetching the telem from DOM0, we need to dec the telem's
+		 * refcnt and release the entry. The telem is reserved and inc
+		 * refcnt when filling the telem.
+		 */
+		mc_op.u.mc_fetch.flags = XEN_MC_URGENT | XEN_MC_ACK;
+		result = HYPERVISOR_mca(&mc_op);
+
+		goto urgent;
+	}
+nonurgent:
+	mc_op.u.mc_fetch.flags = XEN_MC_NONURGENT;
+	result = HYPERVISOR_mca(&mc_op);
+	if (result || mc_op.u.mc_fetch.flags & XEN_MC_NODATA ||
+			mc_op.u.mc_fetch.flags & XEN_MC_FETCHFAILED)
+	{
+		printk(KERN_DEBUG "MCE_DOM0_LOG: No more nonurgent data\n");
+		goto end;
+	}
+	else
+	{
+		result = convert_log(g_mi);
+		if (result) {
+			printk(KERN_ERR "MCE_DOM0_LOG: Log conversion failed\n");
+			goto end;
+		}
+		/* After fetching the telem from DOM0, we need to dec the telem's
+		 * refcnt and release the entry. The telem is reserved and inc
+		 * refcnt when filling the telem.
+		 */
+		mc_op.u.mc_fetch.flags = XEN_MC_NONURGENT | XEN_MC_ACK;
+		result = HYPERVISOR_mca(&mc_op);
+
+		goto nonurgent;
+	}
+end:
+	return IRQ_HANDLED;
+}
+
+void bind_virq_for_mce(void)
+{
+	int ret;
+
+	ret  = bind_virq_to_irqhandler(VIRQ_ARCH_0, 0, 
+		mce_dom0_interrupt, 0, "mce", NULL);
+
+	g_mi = kmalloc(sizeof(struct mc_info), GFP_KERNEL);
+	if ( ret<0 )
+	{
+		printk(KERN_ERR "MCE_DOM0_LOG: bind_virq for DOM0 failed\n");
+	}
+}
+
diff -r ef96f99e1084 include/asm-x86_64/mach-xen/asm/hypercall.h
--- a/include/asm-x86_64/mach-xen/asm/hypercall.h	Thu Jun 11 10:14:50 2009 +0800
+++ b/include/asm-x86_64/mach-xen/asm/hypercall.h	Mon Jun 15 15:46:24 2009 +0800
@@ -39,6 +39,7 @@
 
 #include <linux/string.h> /* memcpy() */
 #include <linux/stringify.h>
+#include <xen/interface/arch-x86/xen-mca.h>
 
 #ifndef __HYPERVISOR_H__
 # error "please don't include this file directly"
@@ -215,7 +216,13 @@
 	platform_op->interface_version = XENPF_INTERFACE_VERSION;
 	return _hypercall1(int, platform_op, platform_op);
 }
-
+static inline int __must_check
+HYPERVISOR_mca(
+	struct xen_mc *mc_op)
+{
+	mc_op->interface_version = XEN_MCA_INTERFACE_VERSION;
+	return _hypercall1(int, mca, mc_op);
+}
 static inline int __must_check
 HYPERVISOR_set_debugreg(
 	unsigned int reg, unsigned long value)

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

RE: [PATCH] DOM0: Adding MCA Loging support in DOM0

[Jan Beulich]

Actually, there are still two issues with this:

First, shouldn't bind_virq_for_mce() and the associated message printing
really only happen on Dom0 (i.e. be guarded by is_initial_xendomain()),
just like the name suggests?

Second, I'm now seeing

(XEN) traps.c:2263:d0 Domain attempted WRMSR 000000000000017b from 00000000:0000003f to ffffffff:ffffffff.
(XEN) traps.c:2263:d0 Domain attempted WRMSR 0000000000000400 from 00000000:000000ff to ffffffff:ffffffff.
(XEN) traps.c:2263:d0 Domain attempted WRMSR 0000000000000404 from 00000000:ffffffff to ffffffff:ffffffff.
(XEN) traps.c:2263:d0 Domain attempted WRMSR 0000000000000408 from 00000000:00000fff to ffffffff:ffffffff.
(XEN) traps.c:2263:d0 Domain attempted WRMSR 000000000000040c from 00000000:00000003 to ffffffff:ffffffff.
(XEN) traps.c:2263:d0 Domain attempted WRMSR 0000000000000410 from 00000000:3fffffff to ffffffff:ffffffff.
(XEN) traps.c:2263:d0 Domain attempted WRMSR 0000000000000414 from 00000000:00000001 to ffffffff:ffffffff.

which isn't really nice.

Jan

>>> "Ke, Liping" <liping.ke@intel.com> 15.06.09 10:44 >>>
Hi, keir

This is the final patch for adding MCA logging support in DOM0 which is Acked by Jan.
And thanks for Jan's help!

Regards,
Criping

Jan Beulich wrote:
> Thanks - looks goo now to me (i.e. Acked-by: Jan Beulich
> <jbeulich@novell.com> 
> if we have such a concept in Xen).
> 
> Jan
> 

RE: [PATCH] DOM0: Adding MCA Loging support in DOM0

[Ke, Liping]

Hi, Jan
Please see below answers.

Regards,
Criping

Jan Beulich wrote:
> Actually, there are still two issues with this:
> 
> First, shouldn't bind_virq_for_mce() and the associated message
> printing 
> really only happen on Dom0 (i.e. be guarded by
> is_initial_xendomain()), 
> just like the name suggests?

Yes. DOM0 is the only one who's responsible for MCE logging (Even 
the error address belongs to other Guest). Currently mce_dom0.o is 
only compiled when it is a privileged guest [depends on (!XEN_UNPRIVILEGED_GUEST)
according to Kconfig]. bind_virq_for_mce() is also protected by the same thing
 [#if defined(CONFIG_X86_XEN_MCE)] You mean we still need other guard?

> 
> Second, I'm now seeing
> 
> (XEN) traps.c:2263:d0 Domain attempted WRMSR 000000000000017b from
> 00000000:0000003f to ffffffff:ffffffff. (XEN) traps.c:2263:d0 Domain
> attempted WRMSR 0000000000000400 from 00000000:000000ff to
> ffffffff:ffffffff. (XEN) traps.c:2263:d0 Domain attempted WRMSR
> 0000000000000404 from 00000000:ffffffff to ffffffff:ffffffff. (XEN)
> traps.c:2263:d0 Domain attempted WRMSR 0000000000000408 from
> 00000000:00000fff to ffffffff:ffffffff. (XEN) traps.c:2263:d0 Domain
> attempted WRMSR 000000000000040c from 00000000:00000003 to
> ffffffff:ffffffff. (XEN) traps.c:2263:d0 Domain attempted WRMSR
> 0000000000000410 from 00000000:3fffffff to ffffffff:ffffffff. (XEN)
> traps.c:2263:d0 Domain attempted WRMSR 0000000000000414 from
> 00000000:00000001 to ffffffff:ffffffff.     
> 
> which isn't really nice.

I looked into the code, current CONFIG_X86_MCE is defaultly 'y', so mce.o 
DOM0 will be compiled by default. Yet we now only support intel 64 bit for 
DOM0 vMSR. I guess this is the reason why you see those printings. 
Shall I sent a patch to make stricker limitations for compiling mce.o? 

> 
> Jan
> 
>>>> "Ke, Liping" <liping.ke@intel.com> 15.06.09 10:44 >>> Hi, keir
> 
> This is the final patch for adding MCA logging support in DOM0 which
> is Acked by Jan. 
> And thanks for Jan's help!
> 
> Regards,
> Criping
> 
> Jan Beulich wrote:
>> Thanks - looks goo now to me (i.e. Acked-by: Jan Beulich
>> <jbeulich@novell.com> if we have such a concept in Xen).
>> 
>> Jan

RE: [PATCH] DOM0: Adding MCA Loging support in DOM0

[Jan Beulich]

>>> "Ke, Liping" <liping.ke@intel.com> 01.07.09 03:42 >>>
>Jan Beulich wrote:
>> Actually, there are still two issues with this:
>> 
>> First, shouldn't bind_virq_for_mce() and the associated message
>> printing 
>> really only happen on Dom0 (i.e. be guarded by
>> is_initial_xendomain()), 
>> just like the name suggests?
>
>Yes. DOM0 is the only one who's responsible for MCE logging (Even 
>the error address belongs to other Guest). Currently mce_dom0.o is 
>only compiled when it is a privileged guest [depends on (!XEN_UNPRIVILEGED_GUEST)
>according to Kconfig]. bind_virq_for_mce() is also protected by the same thing
> [#if defined(CONFIG_X86_XEN_MCE)] You mean we still need other guard?

!XEN_UNPRIVILEGED_GUEST doesn't mean the kernel can't run as DomU,
it's only the other way around. Hence a build-time check only isn't sufficient.

>> 
>> Second, I'm now seeing
>> 
>> (XEN) traps.c:2263:d0 Domain attempted WRMSR 000000000000017b from
>> 00000000:0000003f to ffffffff:ffffffff. (XEN) traps.c:2263:d0 Domain
>> attempted WRMSR 0000000000000400 from 00000000:000000ff to
>> ffffffff:ffffffff. (XEN) traps.c:2263:d0 Domain attempted WRMSR
>> 0000000000000404 from 00000000:ffffffff to ffffffff:ffffffff. (XEN)
>> traps.c:2263:d0 Domain attempted WRMSR 0000000000000408 from
>> 00000000:00000fff to ffffffff:ffffffff. (XEN) traps.c:2263:d0 Domain
>> attempted WRMSR 000000000000040c from 00000000:00000003 to
>> ffffffff:ffffffff. (XEN) traps.c:2263:d0 Domain attempted WRMSR
>> 0000000000000410 from 00000000:3fffffff to ffffffff:ffffffff. (XEN)
>> traps.c:2263:d0 Domain attempted WRMSR 0000000000000414 from
>> 00000000:00000001 to ffffffff:ffffffff.     
>> 
>> which isn't really nice.
>
>I looked into the code, current CONFIG_X86_MCE is defaultly 'y', so mce.o 
>DOM0 will be compiled by default. Yet we now only support intel 64 bit for 
>DOM0 vMSR. I guess this is the reason why you see those printings. 
>Shall I sent a patch to make stricker limitations for compiling mce.o? 

I'm not sure if you can avoid compiling mce.o altogether - if that's possible,
it would certainly be the best approach. If not, some other mechanism to
suppress the actual hardware touching bits would be desirable.

Jan

RE: [PATCH] DOM0: Adding MCA Loging support in DOM0

[Ke, Liping]

Jan Beulich wrote:
>>>> "Ke, Liping" <liping.ke@intel.com> 01.07.09 03:42 >>>
>> Jan Beulich wrote:
>>> Actually, there are still two issues with this:
>>> 
>>> First, shouldn't bind_virq_for_mce() and the associated message
>>> printing really only happen on Dom0 (i.e. be guarded by
>>> is_initial_xendomain()),
>>> just like the name suggests?
>> 
>> Yes. DOM0 is the only one who's responsible for MCE logging (Even
>> the error address belongs to other Guest). Currently mce_dom0.o is
>> only compiled when it is a privileged guest [depends on
>> (!XEN_UNPRIVILEGED_GUEST) according to Kconfig]. bind_virq_for_mce()
>> is also protected by the same thing [#if
>> defined(CONFIG_X86_XEN_MCE)] You mean we still need other guard? 
> 
> !XEN_UNPRIVILEGED_GUEST doesn't mean the kernel can't run as DomU,
> it's only the other way around. Hence a build-time check only isn't
> sufficient. 
> 

Ok, I will send out a patch adding guard before calling bind_virq_for_mce.

>>> 
>>> Second, I'm now seeing
>>> 
>>> (XEN) traps.c:2263:d0 Domain attempted WRMSR 000000000000017b from
>>> 00000000:0000003f to ffffffff:ffffffff. (XEN) traps.c:2263:d0 Domain
>>> attempted WRMSR 0000000000000400 from 00000000:000000ff to
>>> ffffffff:ffffffff. (XEN) traps.c:2263:d0 Domain attempted WRMSR
>>> 0000000000000404 from 00000000:ffffffff to ffffffff:ffffffff. (XEN)
>>> traps.c:2263:d0 Domain attempted WRMSR 0000000000000408 from
>>> 00000000:00000fff to ffffffff:ffffffff. (XEN) traps.c:2263:d0 Domain
>>> attempted WRMSR 000000000000040c from 00000000:00000003 to
>>> ffffffff:ffffffff. (XEN) traps.c:2263:d0 Domain attempted WRMSR
>>> 0000000000000410 from 00000000:3fffffff to ffffffff:ffffffff. (XEN)
>>> traps.c:2263:d0 Domain attempted WRMSR 0000000000000414 from
>>> 00000000:00000001 to ffffffff:ffffffff.
>>> 
>>> which isn't really nice.
>> 
>> I looked into the code, current CONFIG_X86_MCE is defaultly 'y', so
>> mce.o 
>> DOM0 will be compiled by default. Yet we now only support intel 64
>> bit for 
>> DOM0 vMSR. I guess this is the reason why you see those printings.
>> Shall I sent a patch to make stricker limitations for compiling
>> mce.o? 
> 
> I'm not sure if you can avoid compiling mce.o altogether - if that's
> possible, it would certainly be the best approach. If not, some other
> mechanism to suppress the actual hardware touching bits would be
> desirable. 

I plan first send a patch making mce.o compiled for x86_64 Intel arch. 
Since we do plan to make AMD and Intel have a common MCA handling
logic. Both AMD and Intel arch shall support vMSR then. This common
handler might need some time. How do you think?

> 
> Jan

RE: [PATCH] DOM0: Adding MCA Loging support in DOM0

[Ke, Liping]

Hi, Jan

When enabing mce in DOM0,  we only want to fully reuse machine check handler in mce.c as
guest virtual mca handler. 
mce_intel.c and mce_amd.c contain two AMD and Intel hardware features which is 
no use to (both mce virq log and virtual mca enabling) actually.

Problem is that mce_XXX_feature_init in (mce_intel.c and mce_amd.c) is called in mce.c 
when do mce init. So how about:
1) Make those two files only compile under native linux
2) in mce.c, don't call mce_XXX_feature_init under XEN envrionment? 

Then many of problems will not exist, please see below answers.

Thanks!
Criping

Jan Beulich wrote:
>>>> "Ke, Liping" <liping.ke@intel.com> 10.06.09 08:52 >>>
>> According to the discussion result, I modified the patch
>> 1)  use Kconfig to identify when mce_dom.c and mce_XXX.c will be
>>     built. mce_dom.c will only be build when it is under XEN MCE
>> environment. 2) virq bind function will only be called when
>> mce_dom.c is used. 
>> 
>> I test the compiling both under native/dom0 with all possible
>> combinations. 
> 
> I think that's not covering all the issue I pointed out:
> 
>> --- a/arch/x86_64/Kconfig	Tue Jun 09 09:58:11 2009 +0800
>> +++ b/arch/x86_64/Kconfig	Wed Jun 10 14:12:55 2009 +0800 @@ -472,7
>> +472,6 @@ 
>> 
>> config X86_MCE
>> 	bool "Machine check support" if EMBEDDED
>> -	depends on !X86_64_XEN
>> 	default y
>> 	help
>> 	   Include a machine check error handler to report hardware errors.
> 
> Shouldn't this rather change the dependency to XEN_PRIVILEGED_GUEST?
> 
Native depends on this config too, so have about change it like this
depends on   !X86_64_XEN || ( X86_64_XEN && XEN_PRIVILEGED_GUEST)

>> @@ -483,7 +482,7 @@
>> config X86_MCE_INTEL
>> 	bool "Intel MCE features"
>> 	depends on X86_MCE && X86_LOCAL_APIC
>> -	default y
>> +	default n
>> 	help
>> 	   Additional support for intel specific MCE features such as
>> 	   the thermal monitor.
> 
> This hunk should go.

This option will not appear under XEN, only for native?
depends on X86_MCE && X86_LOCAL_APIC && !X86_64_XEN

> 
>> @@ -491,11 +490,15 @@
>> config X86_MCE_AMD
>> 	bool "AMD MCE features"
>> 	depends on X86_MCE && X86_LOCAL_APIC
>> -	default y
>> +	default n
>> 	help
>> 	   Additional support for AMD specific MCE features such as
>> 	   the DRAM Error Threshold.
> 
> And likewise this part.
> 
This option will not appear under XEN, only for native.
depends on X86_MCE && X86_LOCAL_APIC && !X86_64_XEN

So above two files will not be compiled for DOM0. Compiling hunks for them will not be needed any more.

>> 
>> +config X86_64_XEN_MCE
>> +	def_bool y
>> +	depends on X86_64_XEN && (X86_MCE_INTEL || X86_MCE_AMD)
>> +
> 
> I wonder if this wouldn't better be named X86_XEN_MCE (for consistency
> with a potential 32-bit implementation).
> 

This option will depends on X86_64_XEN && X86_MCE.
Yes, we could rename it. Yet this config file is under X86_64 sub dir, also, seems 32bit system will not 
have mca support?

>> config KEXEC
>> 	bool "kexec system call (EXPERIMENTAL)"
>> 	depends on EXPERIMENTAL && !XEN_UNPRIVILEGED_GUEST ...
>> --- a/arch/x86_64/kernel/apic-xen.c	Tue Jun 09 09:58:11 2009 +0800
>> +++ b/arch/x86_64/kernel/apic-xen.c	Wed Jun 10 14:12:55 2009 +0800
>> @@ -195,3 +195,13 @@ 
>> 
>> 	return 1;
>> }
>> +
>> +/* Pull back for compiling arch/x86_64/kernel/mce_amd.c */
>> +void setup_APIC_extened_lvt(unsigned char lvt_off, unsigned char
>> vector, +			    unsigned char msg_type, unsigned char mask) +{
>> +	unsigned long reg = (lvt_off << 4) + K8_APIC_EXT_LVT_BASE;
>> +	unsigned int  v   = (mask << 16) | (msg_type << 8) | vector;
>> +	apic_write(reg, v); +}
> 
> This continues to be wrong (yes, I realize
> arch/x86_64/kernel/apic-xen.c is full of such broken code, but that
> doesn't mean more should be added). 
> 

We no long need this change.

>> ...
>> --- a/arch/x86_64/kernel/mce.c	Tue Jun 09 09:58:11 2009 +0800
>> +++ b/arch/x86_64/kernel/mce.c	Wed Jun 10 14:12:55 2009 +0800 @@
>> -165,7 +165,7 @@ 
>>  * The actual machine check handler
>>  */
>> 
>> -void do_machine_check(struct pt_regs * regs, long error_code)
>> +asmlinkage void do_machine_check(struct pt_regs * regs, long
>> 	error_code) { struct mce m, panicm;
>> 	int nowayout = (tolerant < 1);
> 
> Why?

Yes, this change is not necessary, I will remove it.
> 
>> @@ -276,9 +276,16 @@
>> 
>> /*
>>  * Periodic polling timer for "silent" machine check errors. - */
>> + * We will disable polling in DOM0 since all CMCI/Polling
>> + * mechanism will be done in XEN for Intel CPUs
>> +*/
>> 
>> +#if defined (CONFIG_XEN) && defined(CONFIG_X86_MCE_INTEL)
>> +static int check_interval = 0; /* disable polling */ +#else
>> static int check_interval = 5 * 60; /* 5 minutes */ +#endif
>> +
>> static void mcheck_timer(void *data);
>> static DECLARE_WORK(mcheck_work, mcheck_timer, NULL);
> 
> Shouldn't that now simply be #ifdef CONFIG_X86_64_XEN_MCE?
> 

yes, you're right. Missed this chunk.

>> ...
>> --- a/include/asm-x86_64/mach-xen/asm/hw_irq.h	Tue Jun 09 09:58:11
>> 2009 +0800 +++ b/include/asm-x86_64/mach-xen/asm/hw_irq.h	Wed Jun 10
>> 14:12:55 2009 +0800 @@ -60,6 +60,9 @@ #define
>> NUM_INVALIDATE_TLB_VECTORS	8 #endif
>> 
>> +/* Pull back for compiling arch/x86_64/kernel/mce_amd.c */
>> +#define THRESHOLD_APIC_VECTOR   0xf9
>> +#define THERMAL_APIC_VECTOR	0xfa
>> /*
>>  * Local APIC timer IRQ vector is on a different priority level,
>>  * to work around the 'lost local interrupt if more than 2 IRQ
> 
> These vectors mean nothing under Xen.

no longer need this change.

> 
> Jan

RE: [PATCH] DOM0: Adding MCA Loging support in DOM0

[Jan Beulich]

>>> "Ke, Liping" <liping.ke@intel.com> 10.06.09 08:52 >>>
>According to the discussion result, I modified the patch
>1)  use Kconfig to identify when mce_dom.c and mce_XXX.c will be built. mce_dom.c
>     will only be build when it is under XEN MCE environment.
>2) virq bind function will only be called when mce_dom.c is used.
>
>I test the compiling both under native/dom0 with all possible combinations.

I think that's not covering all the issue I pointed out:

>--- a/arch/x86_64/Kconfig	Tue Jun 09 09:58:11 2009 +0800
>+++ b/arch/x86_64/Kconfig	Wed Jun 10 14:12:55 2009 +0800
>@@ -472,7 +472,6 @@
> 
> config X86_MCE
> 	bool "Machine check support" if EMBEDDED
>-	depends on !X86_64_XEN
> 	default y
> 	help
> 	   Include a machine check error handler to report hardware errors.

Shouldn't this rather change the dependency to XEN_PRIVILEGED_GUEST?

>@@ -483,7 +482,7 @@
> config X86_MCE_INTEL
> 	bool "Intel MCE features"
> 	depends on X86_MCE && X86_LOCAL_APIC
>-	default y
>+	default n
> 	help
> 	   Additional support for intel specific MCE features such as
> 	   the thermal monitor.

This hunk should go.

>@@ -491,11 +490,15 @@
> config X86_MCE_AMD
> 	bool "AMD MCE features"
> 	depends on X86_MCE && X86_LOCAL_APIC
>-	default y
>+	default n
> 	help
> 	   Additional support for AMD specific MCE features such as
> 	   the DRAM Error Threshold.

And likewise this part.

> 
>+config X86_64_XEN_MCE
>+	def_bool y
>+	depends on X86_64_XEN && (X86_MCE_INTEL || X86_MCE_AMD)
>+

I wonder if this wouldn't better be named X86_XEN_MCE (for consistency
with a potential 32-bit implementation).

> config KEXEC
> 	bool "kexec system call (EXPERIMENTAL)"
> 	depends on EXPERIMENTAL && !XEN_UNPRIVILEGED_GUEST
>...
>--- a/arch/x86_64/kernel/apic-xen.c	Tue Jun 09 09:58:11 2009 +0800
>+++ b/arch/x86_64/kernel/apic-xen.c	Wed Jun 10 14:12:55 2009 +0800
>@@ -195,3 +195,13 @@
> 
> 	return 1;
> }
>+
>+/* Pull back for compiling arch/x86_64/kernel/mce_amd.c */
>+void setup_APIC_extened_lvt(unsigned char lvt_off, unsigned char vector,
>+			    unsigned char msg_type, unsigned char mask)
>+{
>+	unsigned long reg = (lvt_off << 4) + K8_APIC_EXT_LVT_BASE;
>+	unsigned int  v   = (mask << 16) | (msg_type << 8) | vector;
>+	apic_write(reg, v);
>+}

This continues to be wrong (yes, I realize arch/x86_64/kernel/apic-xen.c is
full of such broken code, but that doesn't mean more should be added).

>...
>--- a/arch/x86_64/kernel/mce.c	Tue Jun 09 09:58:11 2009 +0800
>+++ b/arch/x86_64/kernel/mce.c	Wed Jun 10 14:12:55 2009 +0800
>@@ -165,7 +165,7 @@
>  * The actual machine check handler
>  */
> 
>-void do_machine_check(struct pt_regs * regs, long error_code)
>+asmlinkage void do_machine_check(struct pt_regs * regs, long error_code)
> {
> 	struct mce m, panicm;
> 	int nowayout = (tolerant < 1); 

Why?

>@@ -276,9 +276,16 @@
> 
> /*
>  * Periodic polling timer for "silent" machine check errors.
>- */
>+ * We will disable polling in DOM0 since all CMCI/Polling
>+ * mechanism will be done in XEN for Intel CPUs
>+*/
> 
>+#if defined (CONFIG_XEN) && defined(CONFIG_X86_MCE_INTEL)
>+static int check_interval = 0; /* disable polling */
>+#else
> static int check_interval = 5 * 60; /* 5 minutes */
>+#endif
>+
> static void mcheck_timer(void *data);
> static DECLARE_WORK(mcheck_work, mcheck_timer, NULL);
 
Shouldn't that now simply be #ifdef CONFIG_X86_64_XEN_MCE?

>...
>--- a/include/asm-x86_64/mach-xen/asm/hw_irq.h	Tue Jun 09 09:58:11 2009 +0800
>+++ b/include/asm-x86_64/mach-xen/asm/hw_irq.h	Wed Jun 10 14:12:55 2009 +0800
>@@ -60,6 +60,9 @@
> #define NUM_INVALIDATE_TLB_VECTORS	8
> #endif
> 
>+/* Pull back for compiling arch/x86_64/kernel/mce_amd.c */
>+#define THRESHOLD_APIC_VECTOR   0xf9
>+#define THERMAL_APIC_VECTOR	0xfa
> /*
>  * Local APIC timer IRQ vector is on a different priority level,
>  * to work around the 'lost local interrupt if more than 2 IRQ

These vectors mean nothing under Xen.

Jan

RE: [PATCH] DOM0: Adding MCA Loging support in DOM0

[Jiang, Yunhong]

The issue here is, Kernel has same config option/file for MCE/Thermal. Although Xen HV provide virtual MCE to dom0 so that we don't need anything changes for dom0, but that's not ture forthermal. We will consider that. Really thanks for pointing that.

--jyh

Jan Beulich wrote:
>>>>> "Jiang, Yunhong" <yunhong.jiang@intel.com> 10.06.09 04:45 >>>
>> Jan Beulich wrote:
>>>>>> "Jiang, Yunhong" <yunhong.jiang@intel.com> 09.06.09 11:30 >>>
>>>> Jan, I think the native sub-options (X86_MCE_INTEL and
>>> X86_MCE_AMD) needs be enabled still.
>>>> When a MCE happens, and dom0 is effected, that MCE will be
>>> injected to dom0 as virtual MCE, and that requires to enable these
>>>> otions.
>>>> When a MCE happens and dom0 is not effected, that MCE will be
>>> sent to dom0 as vIRQ so that dom0 can log that event for
>>>> analysis.
>>> 
>>> Oh, okay, if the code is indeed needed, than that's fine. But
>>> the hacks needed
>>> to get mce_amd.c to compile look suspicious. As much as e.g.
>>> changing the defaults for the MCE Kconfig sub-options - those
>>> should be 
>>> kept as is, and if
>>> the new code has any kind of dependency on them, the to-be-added new
>>> Kconfig option should express this accurately.
>> 
>> Maybe we should change the "add" to "copy back". That two code are
>> from native kernel, but it is missed when copy the apic.c to
>> apic-xen.c, we just add it back. (After all, these hunks just change
>> xen specific code, apic-xen.c and mach-xen/asm/hw-irq.h)
> 
> But both changes are wrong for Xen: Neither can you do APIC writes in
> Dom0 (or setup an APIC local vector entry in particular), nor do
> vectors 0xf9 and 0xfa have any significance there.
> 
> Jan

RE: [PATCH] DOM0: Adding MCA Loging support in DOM0

[Jiang, Yunhong]

Jan Beulich wrote:
>>>>> "Jiang, Yunhong" <yunhong.jiang@intel.com> 10.06.09 04:45 >>>
>> Jan Beulich wrote:
>>>>>> "Jiang, Yunhong" <yunhong.jiang@intel.com> 09.06.09 11:30 >>>
>>>> Jan, I think the native sub-options (X86_MCE_INTEL and
>>> X86_MCE_AMD) needs be enabled still.
>>>> When a MCE happens, and dom0 is effected, that MCE will be
>>> injected to dom0 as virtual MCE, and that requires to enable these
>>>> otions.
>>>> When a MCE happens and dom0 is not effected, that MCE will be
>>> sent to dom0 as vIRQ so that dom0 can log that event for
>>>> analysis.
>>> 
>>> Oh, okay, if the code is indeed needed, than that's fine. But
>>> the hacks needed
>>> to get mce_amd.c to compile look suspicious. As much as e.g.
>>> changing the defaults for the MCE Kconfig sub-options - those
>>> should be 
>>> kept as is, and if
>>> the new code has any kind of dependency on them, the to-be-added new
>>> Kconfig option should express this accurately.
>> 
>> Maybe we should change the "add" to "copy back". That two code are
>> from native kernel, but it is missed when copy the apic.c to
>> apic-xen.c, we just add it back. (After all, these hunks just change
>> xen specific code, apic-xen.c and mach-xen/asm/hw-irq.h)
> 
> But both changes are wrong for Xen: Neither can you do APIC writes in
> Dom0 (or setup an APIC local vector entry in particular), nor do
> vectors 0xf9 and 0xfa have any significance there.

Aha, yes. We need check our patch again for similar potential issue. Thanks for pointing that.


> 
> Jan

RE: [PATCH] DOM0: Adding MCA Loging support in DOM0

[Jan Beulich]

>>>> "Jiang, Yunhong" <yunhong.jiang@intel.com> 10.06.09 04:45 >>>
>Jan Beulich wrote:
>>>>> "Jiang, Yunhong" <yunhong.jiang@intel.com> 09.06.09 11:30 >>>
>>> Jan, I think the native sub-options (X86_MCE_INTEL and
>> X86_MCE_AMD) needs be enabled still.
>>> When a MCE happens, and dom0 is effected, that MCE will be
>> injected to dom0 as virtual MCE, and that requires to enable these
>>> otions.
>>> When a MCE happens and dom0 is not effected, that MCE will be
>> sent to dom0 as vIRQ so that dom0 can log that event for
>>> analysis.
>> 
>> Oh, okay, if the code is indeed needed, than that's fine. But
>> the hacks needed
>> to get mce_amd.c to compile look suspicious. As much as e.g. changing
>> the defaults for the MCE Kconfig sub-options - those should be
>> kept as is, and if
>> the new code has any kind of dependency on them, the to-be-added new
>> Kconfig option should express this accurately.
>
>Maybe we should change the "add" to "copy back". That two code are from
>native kernel, but it is missed when copy the apic.c to apic-xen.c, we just
>add it back. (After all, these hunks just change xen specific code,
>apic-xen.c and mach-xen/asm/hw-irq.h)

But both changes are wrong for Xen: Neither can you do APIC writes in Dom0
(or setup an APIC local vector entry in particular), nor do vectors 0xf9 and
0xfa have any significance there.

Jan

RE: [PATCH] DOM0: Adding MCA Loging support in DOM0

[Ke, Liping]

[-- Attachment #1: Type: text/plain, Size: 3454 bytes --]

Hi, Jan and yunhong

According to the discussion result, I modified the patch
1)  use Kconfig to identify when mce_dom.c and mce_XXX.c will be built. mce_dom.c
     will only be build when it is under XEN MCE environment.
2) virq bind function will only be called when mce_dom.c is used.

I test the compiling both under native/dom0 with all possible combinations.

Thanks& Regards,
Criping  

-----Original Message-----
From: Jiang, Yunhong 
Sent: 2009年6月10日 10:46
To: Jan Beulich; Ke, Liping
Cc: Keir Fraser; xen-devel
Subject: RE: [Xen-devel] [PATCH] DOM0: Adding MCA Loging support in DOM0

Jan Beulich wrote:
>>>> "Jiang, Yunhong" <yunhong.jiang@intel.com> 09.06.09 11:30 >>>
>> xen-devel-bounces@lists.xensource.com wrote:
>>>>>> "Ke, Liping" <liping.ke@intel.com> 09.06.09 07:10 >>>
>>>> This patch is to support MCA info logging in DOM0. When an MCE/CMCI
>>>> error happens (or by polling), the related error info will be sent
>>>> to DOM0 by XEN. This patch will help to fetch the xen-logged
>>>> information by hypercall and then convert XEN-format log into Linux
>>>> format MCELOG. So we can reuse current available mcelog tools for
>>>> native Linux now.
>>> 
>>> Could this patch please be re-worked to not needlessly touch non-Xen
>>> code? This would include a separate config option for the new code,
>>> while disabling the native sub-options (which at once would avoid
>>> the hacks you appear to need to make mce_amd.c build).
>> 
>> Jan, I think the native sub-options (X86_MCE_INTEL and
> X86_MCE_AMD) needs be enabled still.
>> When a MCE happens, and dom0 is effected, that MCE will be
> injected to dom0 as virtual MCE, and that requires to enable these
>> otions.
>> When a MCE happens and dom0 is not effected, that MCE will be
> sent to dom0 as vIRQ so that dom0 can log that event for
>> analysis.
> 
> Oh, okay, if the code is indeed needed, than that's fine. But
> the hacks needed
> to get mce_amd.c to compile look suspicious. As much as e.g. changing
> the defaults for the MCE Kconfig sub-options - those should be
> kept as is, and if
> the new code has any kind of dependency on them, the to-be-added new
> Kconfig option should express this accurately.

Maybe we should change the "add" to "copy back". That two code are from native kernel, but it is missed when copy the apic.c to apic-xen.c, we just add it back. (After all, these hunks just change xen specific code, apic-xen.c and mach-xen/asm/hw-irq.h)

> 
> Basically, behavior for a native kernel built from the same
> sources should not
> be modified at all.
> 
>> But yes, maybe following hunk can be done in xen-code to avoid 
>> needless touch. 
>> 
>> +
>> +    /*Register vIRQ handler for MCE LOG processing*/
>> +#if defined (CONFIG_XEN) && defined(CONFIG_X86_MCE_INTEL)
>> +    printk(KERN_DEBUG "MCE: bind virq for DOM0 Logging\n"); +   
>> bind_virq_for_mce(); +#endif
>> +
>> 	return err;
>> 
>> And following code can be removed, although it may cause dom0's
>> needless polling. 
>> 
>> +#if defined (CONFIG_XEN) && defined(CONFIG_X86_MCE_INTEL)
>> +static int check_interval = 0; /* disable polling */ +#else
>> static int check_interval = 5 * 60; /* 5 minutes */ +#endif
>> +
> 
> Actually, these two hunks actually represent examples of what I'm not
> concerned about . 

Remove it to xen-specific file will help us in future for the PV_ops dom0.

> 
> Jan

[-- Attachment #2: intel_mce_dom0.patch --]
[-- Type: application/octet-stream, Size: 9605 bytes --]

This patch is to add MCA logging support in DOM0

When an MCE/CMCI error happens (or by polling), the related error information
will be sent to DOM0 by XEN. This patch will help to fetch the xen-logged
information by hypercall and then convert XEN-format log into Linux format
MCELOG. It makes using current available mcelog tools for native Linux
possible.

With this patch, after mce/cmci error log information is sent to DOM0,
running mcelog tools in DOM0, you will get same detailed decoded mce
information as in Native Linux.

Signed-Off-By: Liping Ke<liping.ke@intel.com>
Signed-Off-By: Yunhong Jiang<yunhong.jiang@intel.com>

diff -r 7bea4500e612 arch/x86_64/Kconfig
--- a/arch/x86_64/Kconfig	Tue Jun 09 09:58:11 2009 +0800
+++ b/arch/x86_64/Kconfig	Wed Jun 10 14:12:55 2009 +0800
@@ -472,7 +472,6 @@
 
 config X86_MCE
 	bool "Machine check support" if EMBEDDED
-	depends on !X86_64_XEN
 	default y
 	help
 	   Include a machine check error handler to report hardware errors.
@@ -483,7 +482,7 @@
 config X86_MCE_INTEL
 	bool "Intel MCE features"
 	depends on X86_MCE && X86_LOCAL_APIC
-	default y
+	default n
 	help
 	   Additional support for intel specific MCE features such as
 	   the thermal monitor.
@@ -491,11 +490,15 @@
 config X86_MCE_AMD
 	bool "AMD MCE features"
 	depends on X86_MCE && X86_LOCAL_APIC
-	default y
+	default n
 	help
 	   Additional support for AMD specific MCE features such as
 	   the DRAM Error Threshold.
 
+config X86_64_XEN_MCE
+	def_bool y
+	depends on X86_64_XEN && (X86_MCE_INTEL || X86_MCE_AMD)
+
 config KEXEC
 	bool "kexec system call (EXPERIMENTAL)"
 	depends on EXPERIMENTAL && !XEN_UNPRIVILEGED_GUEST
diff -r 7bea4500e612 arch/x86_64/kernel/Makefile
--- a/arch/x86_64/kernel/Makefile	Tue Jun 09 09:58:11 2009 +0800
+++ b/arch/x86_64/kernel/Makefile	Wed Jun 10 14:12:55 2009 +0800
@@ -13,6 +13,7 @@
 obj-$(CONFIG_STACKTRACE)	+= stacktrace.o
 obj-$(CONFIG_X86_MCE)         += mce.o
 obj-$(CONFIG_X86_MCE_INTEL)	+= mce_intel.o
+obj-$(CONFIG_X86_64_XEN_MCE)	+= mce_dom0.o
 obj-$(CONFIG_X86_MCE_AMD)	+= mce_amd.o
 obj-$(CONFIG_MTRR)		+= ../../i386/kernel/cpu/mtrr/
 obj-$(CONFIG_ACPI)		+= acpi/
diff -r 7bea4500e612 arch/x86_64/kernel/apic-xen.c
--- a/arch/x86_64/kernel/apic-xen.c	Tue Jun 09 09:58:11 2009 +0800
+++ b/arch/x86_64/kernel/apic-xen.c	Wed Jun 10 14:12:55 2009 +0800
@@ -195,3 +195,13 @@
 
 	return 1;
 }
+
+/* Pull back for compiling arch/x86_64/kernel/mce_amd.c */
+void setup_APIC_extened_lvt(unsigned char lvt_off, unsigned char vector,
+			    unsigned char msg_type, unsigned char mask)
+{
+	unsigned long reg = (lvt_off << 4) + K8_APIC_EXT_LVT_BASE;
+	unsigned int  v   = (mask << 16) | (msg_type << 8) | vector;
+	apic_write(reg, v);
+}
+
diff -r 7bea4500e612 arch/x86_64/kernel/entry-xen.S
--- a/arch/x86_64/kernel/entry-xen.S	Tue Jun 09 09:58:11 2009 +0800
+++ b/arch/x86_64/kernel/entry-xen.S	Wed Jun 10 14:12:55 2009 +0800
@@ -1258,13 +1258,8 @@
 
 #ifdef CONFIG_X86_MCE
 	/* runs on exception stack */
-ENTRY(machine_check)
-	INTR_FRAME
-	pushq $0
-	CFI_ADJUST_CFA_OFFSET 8	
-	paranoidentry do_machine_check
-	jmp paranoid_exit1
-	CFI_ENDPROC
+KPROBE_ENTRY(machine_check)
+	zeroentry do_machine_check
 END(machine_check)
 #endif
 
diff -r 7bea4500e612 arch/x86_64/kernel/mce.c
--- a/arch/x86_64/kernel/mce.c	Tue Jun 09 09:58:11 2009 +0800
+++ b/arch/x86_64/kernel/mce.c	Wed Jun 10 14:12:55 2009 +0800
@@ -165,7 +165,7 @@
  * The actual machine check handler
  */
 
-void do_machine_check(struct pt_regs * regs, long error_code)
+asmlinkage void do_machine_check(struct pt_regs * regs, long error_code)
 {
 	struct mce m, panicm;
 	int nowayout = (tolerant < 1); 
@@ -276,9 +276,16 @@
 
 /*
  * Periodic polling timer for "silent" machine check errors.
- */
+ * We will disable polling in DOM0 since all CMCI/Polling
+ * mechanism will be done in XEN for Intel CPUs
+*/
 
+#if defined (CONFIG_XEN) && defined(CONFIG_X86_MCE_INTEL)
+static int check_interval = 0; /* disable polling */
+#else
 static int check_interval = 5 * 60; /* 5 minutes */
+#endif
+
 static void mcheck_timer(void *data);
 static DECLARE_WORK(mcheck_work, mcheck_timer, NULL);
 
@@ -649,6 +656,8 @@
 };
 #endif
 
+struct mc_info *g_mi;
+extern void bind_virq_for_mce(void);
 static __init int mce_init_device(void)
 {
 	int err;
@@ -664,6 +673,13 @@
 
 	register_hotcpu_notifier(&mce_cpu_notifier);
 	misc_register(&mce_log_device);
+
+    /*Register vIRQ handler for MCE LOG processing*/
+#if defined(CONFIG_X86_64_XEN_MCE)
+    printk(KERN_DEBUG "MCE: bind virq for DOM0 Logging\n");
+    bind_virq_for_mce();
+#endif
+
 	return err;
 }
 
diff -r 7bea4500e612 arch/x86_64/kernel/mce_dom0.c
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/arch/x86_64/kernel/mce_dom0.c	Wed Jun 10 14:12:55 2009 +0800
@@ -0,0 +1,133 @@
+#include <linux/init.h>
+#include <linux/types.h>
+#include <linux/kernel.h>
+#include <xen/interface/xen.h>
+#include <xen/evtchn.h>
+#include <xen/interface/vcpu.h>
+#include <asm/hypercall.h>
+#include <asm/mce.h>
+
+static int convert_log(struct mc_info *mi)
+{
+	struct mcinfo_common *mic = NULL;
+	struct mcinfo_global *mc_global;
+	struct mcinfo_bank *mc_bank;
+	struct mce m;
+
+	x86_mcinfo_lookup(mic, mi, MC_TYPE_GLOBAL);
+	if (mic == NULL)
+	{
+		printk(KERN_ERR "DOM0_MCE_LOG: global data is NULL\n");
+		return -1;
+	}
+
+	mc_global = (struct mcinfo_global*)mic;
+	m.mcgstatus = mc_global->mc_gstatus;
+	m.cpu = mc_global->mc_coreid;/*for test*/
+	x86_mcinfo_lookup(mic, mi, MC_TYPE_BANK);
+	do
+	{
+		if (mic == NULL || mic->size == 0)
+			break;
+		if (mic->type == MC_TYPE_BANK)
+		{
+			mc_bank = (struct mcinfo_bank*)mic;
+			m.misc = mc_bank->mc_misc;
+			m.status = mc_bank->mc_status;
+			m.addr = mc_bank->mc_addr;
+			m.tsc = mc_bank->mc_tsc;
+			m.res1 = mc_bank->mc_ctrl2;
+			m.bank = mc_bank->mc_bank;
+			printk(KERN_DEBUG "[CPU%d, BANK%d, addr %llx, state %llx]\n", 
+						m.bank, m.cpu, m.addr, m.status);
+			/*log this record*/
+			mce_log(&m);
+		}
+		mic = x86_mcinfo_next(mic);
+	}while (1);
+
+	return 0;
+}
+
+extern struct mc_info *g_mi;
+
+/*dom0 mce virq handler, logging physical mce error info*/
+
+static irqreturn_t mce_dom0_interrupt(int irq, void *dev_id,
+									struct pt_regs *regs)
+{
+	xen_mc_t mc_op;
+	int result = 0;
+
+	printk(KERN_DEBUG "MCE_DOM0_LOG: enter dom0 mce vIRQ handler\n");
+	mc_op.cmd = XEN_MC_fetch;
+	mc_op.interface_version = XEN_MCA_INTERFACE_VERSION;
+	set_xen_guest_handle(mc_op.u.mc_fetch.data, g_mi);
+urgent:
+	mc_op.u.mc_fetch.flags = XEN_MC_URGENT;
+	result = HYPERVISOR_mca(&mc_op);
+	if (result || mc_op.u.mc_fetch.flags & XEN_MC_NODATA ||
+			mc_op.u.mc_fetch.flags & XEN_MC_FETCHFAILED)
+	{
+		printk(KERN_DEBUG "MCE_DOM0_LOG: No more urgent data\n");
+		goto nonurgent;
+	}
+	else
+	{
+		result = convert_log(g_mi);
+		if (result) {
+			printk(KERN_ERR "MCE_DOM0_LOG: Log conversion failed\n");
+			goto end;
+		}
+		/* After fetching the telem from DOM0, we need to dec the telem's
+		 * refcnt and release the entry. The telem is reserved and inc
+		 * refcnt when filling the telem.
+		 */
+		mc_op.u.mc_fetch.flags = XEN_MC_URGENT | XEN_MC_ACK;
+		result = HYPERVISOR_mca(&mc_op);
+
+		goto urgent;
+	}
+nonurgent:
+	mc_op.u.mc_fetch.flags = XEN_MC_NONURGENT;
+	result = HYPERVISOR_mca(&mc_op);
+	if (result || mc_op.u.mc_fetch.flags & XEN_MC_NODATA ||
+			mc_op.u.mc_fetch.flags & XEN_MC_FETCHFAILED)
+	{
+		printk(KERN_DEBUG "MCE_DOM0_LOG: No more nonurgent data\n");
+		goto end;
+	}
+	else
+	{
+		result = convert_log(g_mi);
+		if (result) {
+			printk(KERN_ERR "MCE_DOM0_LOG: Log conversion failed\n");
+			goto end;
+		}
+		/* After fetching the telem from DOM0, we need to dec the telem's
+		 * refcnt and release the entry. The telem is reserved and inc
+		 * refcnt when filling the telem.
+		 */
+		mc_op.u.mc_fetch.flags = XEN_MC_NONURGENT | XEN_MC_ACK;
+		result = HYPERVISOR_mca(&mc_op);
+
+		goto nonurgent;
+	}
+end:
+	return IRQ_HANDLED;
+}
+
+void bind_virq_for_mce(void)
+{
+	int ret;
+
+	ret  = bind_virq_to_irqhandler(VIRQ_ARCH_0, 0, 
+		mce_dom0_interrupt, 0, "mce", NULL);
+
+	g_mi = kmalloc(sizeof(struct mc_info), GFP_KERNEL);
+	if ( ret<0 )
+	{
+		printk(KERN_ERR "MCE_DOM0_LOG: bind_virq for DOM0 failed\n");
+	}
+}
+
diff -r 7bea4500e612 include/asm-x86_64/mach-xen/asm/hw_irq.h
--- a/include/asm-x86_64/mach-xen/asm/hw_irq.h	Tue Jun 09 09:58:11 2009 +0800
+++ b/include/asm-x86_64/mach-xen/asm/hw_irq.h	Wed Jun 10 14:12:55 2009 +0800
@@ -60,6 +60,9 @@
 #define NUM_INVALIDATE_TLB_VECTORS	8
 #endif
 
+/* Pull back for compiling arch/x86_64/kernel/mce_amd.c */
+#define THRESHOLD_APIC_VECTOR   0xf9
+#define THERMAL_APIC_VECTOR	0xfa
 /*
  * Local APIC timer IRQ vector is on a different priority level,
  * to work around the 'lost local interrupt if more than 2 IRQ
diff -r 7bea4500e612 include/asm-x86_64/mach-xen/asm/hypercall.h
--- a/include/asm-x86_64/mach-xen/asm/hypercall.h	Tue Jun 09 09:58:11 2009 +0800
+++ b/include/asm-x86_64/mach-xen/asm/hypercall.h	Wed Jun 10 14:12:55 2009 +0800
@@ -39,6 +39,7 @@
 
 #include <linux/string.h> /* memcpy() */
 #include <linux/stringify.h>
+#include <xen/interface/arch-x86/xen-mca.h>
 
 #ifndef __HYPERVISOR_H__
 # error "please don't include this file directly"
@@ -215,7 +216,13 @@
 	platform_op->interface_version = XENPF_INTERFACE_VERSION;
 	return _hypercall1(int, platform_op, platform_op);
 }
-
+static inline int __must_check
+HYPERVISOR_mca(
+	struct xen_mc *mc_op)
+{
+	mc_op->interface_version = XEN_MCA_INTERFACE_VERSION;
+	return _hypercall1(int, mca, mc_op);
+}
 static inline int __must_check
 HYPERVISOR_set_debugreg(
 	unsigned int reg, unsigned long value)

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

RE: [PATCH] DOM0: Adding MCA Loging support in DOM0

[Jiang, Yunhong]

Jan Beulich wrote:
>>>> "Jiang, Yunhong" <yunhong.jiang@intel.com> 09.06.09 11:30 >>>
>> xen-devel-bounces@lists.xensource.com wrote:
>>>>>> "Ke, Liping" <liping.ke@intel.com> 09.06.09 07:10 >>>
>>>> This patch is to support MCA info logging in DOM0. When an MCE/CMCI
>>>> error happens (or by polling), the related error info will be sent
>>>> to DOM0 by XEN. This patch will help to fetch the xen-logged
>>>> information by hypercall and then convert XEN-format log into Linux
>>>> format MCELOG. So we can reuse current available mcelog tools for
>>>> native Linux now.
>>> 
>>> Could this patch please be re-worked to not needlessly touch non-Xen
>>> code? This would include a separate config option for the new code,
>>> while disabling the native sub-options (which at once would avoid
>>> the hacks you appear to need to make mce_amd.c build).
>> 
>> Jan, I think the native sub-options (X86_MCE_INTEL and
> X86_MCE_AMD) needs be enabled still.
>> When a MCE happens, and dom0 is effected, that MCE will be
> injected to dom0 as virtual MCE, and that requires to enable these
>> otions.
>> When a MCE happens and dom0 is not effected, that MCE will be
> sent to dom0 as vIRQ so that dom0 can log that event for
>> analysis.
> 
> Oh, okay, if the code is indeed needed, than that's fine. But
> the hacks needed
> to get mce_amd.c to compile look suspicious. As much as e.g. changing
> the defaults for the MCE Kconfig sub-options - those should be
> kept as is, and if
> the new code has any kind of dependency on them, the to-be-added new
> Kconfig option should express this accurately.

Maybe we should change the "add" to "copy back". That two code are from native kernel, but it is missed when copy the apic.c to apic-xen.c, we just add it back. (After all, these hunks just change xen specific code, apic-xen.c and mach-xen/asm/hw-irq.h)

> 
> Basically, behavior for a native kernel built from the same
> sources should not
> be modified at all.
> 
>> But yes, maybe following hunk can be done in xen-code to avoid 
>> needless touch. 
>> 
>> +
>> +    /*Register vIRQ handler for MCE LOG processing*/
>> +#if defined (CONFIG_XEN) && defined(CONFIG_X86_MCE_INTEL)
>> +    printk(KERN_DEBUG "MCE: bind virq for DOM0 Logging\n"); +   
>> bind_virq_for_mce(); +#endif
>> +
>> 	return err;
>> 
>> And following code can be removed, although it may cause dom0's
>> needless polling. 
>> 
>> +#if defined (CONFIG_XEN) && defined(CONFIG_X86_MCE_INTEL)
>> +static int check_interval = 0; /* disable polling */ +#else
>> static int check_interval = 5 * 60; /* 5 minutes */ +#endif
>> +
> 
> Actually, these two hunks actually represent examples of what I'm not
> concerned about . 

Remove it to xen-specific file will help us in future for the PV_ops dom0.

> 
> Jan

RE: [PATCH] DOM0: Adding MCA Loging support in DOM0

[Jan Beulich]

>>> "Jiang, Yunhong" <yunhong.jiang@intel.com> 09.06.09 11:30 >>>
>xen-devel-bounces@lists.xensource.com wrote:
>>>>> "Ke, Liping" <liping.ke@intel.com> 09.06.09 07:10 >>>
>>> This patch is to support MCA info logging in DOM0. When an MCE/CMCI
>>> error happens (or by polling), the related error info will be sent
>>> to DOM0 by XEN. This patch will help to fetch the xen-logged
>>> information by hypercall and then convert XEN-format log into Linux
>>> format MCELOG. So we can reuse current available mcelog tools for
>>> native Linux now. 
>> 
>> Could this patch please be re-worked to not needlessly touch non-Xen
>> code? This would include a separate config option for the new code,
>> while disabling the native sub-options (which at once would avoid the
>> hacks you appear to need to make mce_amd.c build).
>
>Jan, I think the native sub-options (X86_MCE_INTEL and X86_MCE_AMD) needs be enabled still.
>When a MCE happens, and dom0 is effected, that MCE will be injected to dom0 as virtual MCE, and that requires to enable these
>otions.
>When a MCE happens and dom0 is not effected, that MCE will be sent to dom0 as vIRQ so that dom0 can log that event for
>analysis.

Oh, okay, if the code is indeed needed, than that's fine. But the hacks needed
to get mce_amd.c to compile look suspicious. As much as e.g. changing the
defaults for the MCE Kconfig sub-options - those should be kept as is, and if
the new code has any kind of dependency on them, the to-be-added new
Kconfig option should express this accurately.

Basically, behavior for a native kernel built from the same sources should not
be modified at all.

>But yes, maybe following hunk can be done in xen-code to avoid  needless touch.
>
>+
>+    /*Register vIRQ handler for MCE LOG processing*/
>+#if defined (CONFIG_XEN) && defined(CONFIG_X86_MCE_INTEL)
>+    printk(KERN_DEBUG "MCE: bind virq for DOM0 Logging\n");
>+    bind_virq_for_mce();
>+#endif
>+
> 	return err;
>
>And following code can be removed, although it may cause dom0's needless polling.
>
>+#if defined (CONFIG_XEN) && defined(CONFIG_X86_MCE_INTEL)
>+static int check_interval = 0; /* disable polling */
>+#else
> static int check_interval = 5 * 60; /* 5 minutes */
>+#endif
>+

Actually, these two hunks actually represent examples of what I'm not
concerned about .

Jan

RE: [PATCH] DOM0: Adding MCA Loging support in DOM0

[Jiang, Yunhong]

xen-devel-bounces@lists.xensource.com wrote:
>>>> "Ke, Liping" <liping.ke@intel.com> 09.06.09 07:10 >>>
>> This patch is to support MCA info logging in DOM0. When an MCE/CMCI
>> error happens (or by polling), the related error info will be sent
>> to DOM0 by XEN. This patch will help to fetch the xen-logged
>> information by hypercall and then convert XEN-format log into Linux
>> format MCELOG. So we can reuse current available mcelog tools for
>> native Linux now. 
> 
> Could this patch please be re-worked to not needlessly touch non-Xen
> code? This would include a separate config option for the new code,
> while disabling the native sub-options (which at once would avoid the
> hacks you appear to need to make mce_amd.c build).

Jan, I think the native sub-options (X86_MCE_INTEL and X86_MCE_AMD) needs be enabled still.
When a MCE happens, and dom0 is effected, that MCE will be injected to dom0 as virtual MCE, and that requires to enable these otions.
When a MCE happens and dom0 is not effected, that MCE will be sent to dom0 as vIRQ so that dom0 can log that event for analysis.

But yes, maybe following hunk can be done in xen-code to avoid  needless touch.

+
+    /*Register vIRQ handler for MCE LOG processing*/
+#if defined (CONFIG_XEN) && defined(CONFIG_X86_MCE_INTEL)
+    printk(KERN_DEBUG "MCE: bind virq for DOM0 Logging\n");
+    bind_virq_for_mce();
+#endif
+
 	return err;

And following code can be removed, although it may cause dom0's needless polling.

+#if defined (CONFIG_XEN) && defined(CONFIG_X86_MCE_INTEL)
+static int check_interval = 0; /* disable polling */
+#else
 static int check_interval = 5 * 60; /* 5 minutes */
+#endif
+

Thanks
Yunhong Jiang




> 
> Thanks, Jan
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

RE: [PATCH] DOM0: Adding MCA Loging support in DOM0

[Ke, Liping]

[-- Attachment #1: Type: text/plain, Size: 1002 bytes --]

sure. I will resend the patch a little later.

Thanks a lot!
Criping 

-----Original Message-----
From: Jan Beulich [mailto:JBeulich@novell.com] 
Sent: 2009年6月9日 17:17
To: Ke, Liping
Cc: Keir Fraser; xen-devel
Subject: Re: [Xen-devel] [PATCH] DOM0: Adding MCA Loging support in DOM0

>>> "Ke, Liping" <liping.ke@intel.com> 09.06.09 07:10 >>>
>This patch is to support MCA info logging in DOM0. When an MCE/CMCI error happens
> (or by polling), the related error info will be sent to DOM0 by XEN. This patch will help 
>to fetch the xen-logged information by hypercall and then convert XEN-format log into 
>Linux format MCELOG. So we can reuse current available mcelog tools for native Linux 
>now.
 
Could this patch please be re-worked to not needlessly touch non-Xen code?
This would include a separate config option for the new code, while disabling
the native sub-options (which at once would avoid the hacks you appear to
need to make mce_amd.c build).

Thanks, Jan


[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] DOM0: Adding MCA Loging support in DOM0

[Jan Beulich]

>>> "Ke, Liping" <liping.ke@intel.com> 09.06.09 07:10 >>>
>This patch is to support MCA info logging in DOM0. When an MCE/CMCI error happens
> (or by polling), the related error info will be sent to DOM0 by XEN. This patch will help 
>to fetch the xen-logged information by hypercall and then convert XEN-format log into 
>Linux format MCELOG. So we can reuse current available mcelog tools for native Linux 
>now.
 
Could this patch please be re-worked to not needlessly touch non-Xen code?
This would include a separate config option for the new code, while disabling
the native sub-options (which at once would avoid the hacks you appear to
need to make mce_amd.c build).

Thanks, Jan

[PATCH] DOM0: Adding MCA Loging support in DOM0

[Ke, Liping]

[-- Attachment #1: Type: text/plain, Size: 723 bytes --]

Hi, all
 
This patch is to support MCA info logging in DOM0. When an MCE/CMCI error happens
 (or by polling), the related error info will be sent to DOM0 by XEN. This patch will help 
to fetch the xen-logged information by hypercall and then convert XEN-format log into 
Linux format MCELOG. So we can reuse current available mcelog tools for native Linux 
now.
 
With this patch, after mce/cmci error log information is sent to DOM0, running mcelog 
tools in DOM0, you will get same detailed decoded mce information as in Native Linux.
 
We test this patch on latest Intel platform with newest version of mcelog getting from
git://git.kernel.org/pub/scm/utils/cpu/mce/mcelog.git
 
Thanks& Regards,
Criping

[-- Attachment #2: intel_mce_dom0.patch --]
[-- Type: application/octet-stream, Size: 9413 bytes --]

This patch is to add MCA logging support in DOM0

When an MCE/CMCI error happens (or by polling), the related error information 
will be sent to DOM0 by XEN. This patch will help to fetch the xen-logged 
information by hypercall and then convert XEN-format log into Linux format 
MCELOG. It makes using current available mcelog tools for native Linux 
possible.
 
With this patch, after mce/cmci error log information is sent to DOM0, 
running mcelog tools in DOM0, you will get same detailed decoded mce 
information as in Native Linux.

Signed-Off-By: Liping Ke<liping.ke@intel.com>
Signed-Off-By: Yunhong Jiang<yunhong.jiang@intel.com>

diff -r a66aa1d590ba arch/x86_64/Kconfig
--- a/arch/x86_64/Kconfig	Mon Jun 08 12:53:25 2009 +0800
+++ b/arch/x86_64/Kconfig	Mon Jun 08 17:05:18 2009 +0800
@@ -472,7 +472,6 @@
 
 config X86_MCE
 	bool "Machine check support" if EMBEDDED
-	depends on !X86_64_XEN
 	default y
 	help
 	   Include a machine check error handler to report hardware errors.
@@ -483,7 +482,7 @@
 config X86_MCE_INTEL
 	bool "Intel MCE features"
 	depends on X86_MCE && X86_LOCAL_APIC
-	default y
+	default n
 	help
 	   Additional support for intel specific MCE features such as
 	   the thermal monitor.
@@ -491,7 +490,7 @@
 config X86_MCE_AMD
 	bool "AMD MCE features"
 	depends on X86_MCE && X86_LOCAL_APIC
-	default y
+	default n
 	help
 	   Additional support for AMD specific MCE features such as
 	   the DRAM Error Threshold.
diff -r a66aa1d590ba arch/x86_64/kernel/Makefile
--- a/arch/x86_64/kernel/Makefile	Mon Jun 08 12:53:25 2009 +0800
+++ b/arch/x86_64/kernel/Makefile	Mon Jun 08 17:05:18 2009 +0800
@@ -13,6 +13,7 @@
 obj-$(CONFIG_STACKTRACE)	+= stacktrace.o
 obj-$(CONFIG_X86_MCE)         += mce.o
 obj-$(CONFIG_X86_MCE_INTEL)	+= mce_intel.o
+obj-$(CONFIG_X86_MCE_INTEL)	+= mce_dom0.o
 obj-$(CONFIG_X86_MCE_AMD)	+= mce_amd.o
 obj-$(CONFIG_MTRR)		+= ../../i386/kernel/cpu/mtrr/
 obj-$(CONFIG_ACPI)		+= acpi/
diff -r a66aa1d590ba arch/x86_64/kernel/apic-xen.c
--- a/arch/x86_64/kernel/apic-xen.c	Mon Jun 08 12:53:25 2009 +0800
+++ b/arch/x86_64/kernel/apic-xen.c	Mon Jun 08 17:05:18 2009 +0800
@@ -195,3 +195,13 @@
 
 	return 1;
 }
+
+/* Add for compiling arch/x86_64/kernel/mce_amd.c */
+void setup_APIC_extened_lvt(unsigned char lvt_off, unsigned char vector,
+			    unsigned char msg_type, unsigned char mask)
+{
+	unsigned long reg = (lvt_off << 4) + K8_APIC_EXT_LVT_BASE;
+	unsigned int  v   = (mask << 16) | (msg_type << 8) | vector;
+	apic_write(reg, v);
+}
+
diff -r a66aa1d590ba arch/x86_64/kernel/entry-xen.S
--- a/arch/x86_64/kernel/entry-xen.S	Mon Jun 08 12:53:25 2009 +0800
+++ b/arch/x86_64/kernel/entry-xen.S	Mon Jun 08 17:05:18 2009 +0800
@@ -1258,13 +1258,8 @@
 
 #ifdef CONFIG_X86_MCE
 	/* runs on exception stack */
-ENTRY(machine_check)
-	INTR_FRAME
-	pushq $0
-	CFI_ADJUST_CFA_OFFSET 8	
-	paranoidentry do_machine_check
-	jmp paranoid_exit1
-	CFI_ENDPROC
+KPROBE_ENTRY(machine_check)
+	zeroentry do_machine_check
 END(machine_check)
 #endif
 
diff -r a66aa1d590ba arch/x86_64/kernel/mce.c
--- a/arch/x86_64/kernel/mce.c	Mon Jun 08 12:53:25 2009 +0800
+++ b/arch/x86_64/kernel/mce.c	Mon Jun 08 17:05:18 2009 +0800
@@ -165,7 +165,7 @@
  * The actual machine check handler
  */
 
-void do_machine_check(struct pt_regs * regs, long error_code)
+asmlinkage void do_machine_check(struct pt_regs * regs, long error_code)
 {
 	struct mce m, panicm;
 	int nowayout = (tolerant < 1); 
@@ -276,9 +276,16 @@
 
 /*
  * Periodic polling timer for "silent" machine check errors.
- */
+ * We will disable polling in DOM0 since all CMCI/Polling
+ * mechanism will be done in XEN for Intel CPUs
+*/
 
+#if defined (CONFIG_XEN) && defined(CONFIG_X86_MCE_INTEL)
+static int check_interval = 0; /* disable polling */
+#else
 static int check_interval = 5 * 60; /* 5 minutes */
+#endif
+
 static void mcheck_timer(void *data);
 static DECLARE_WORK(mcheck_work, mcheck_timer, NULL);
 
@@ -649,6 +656,8 @@
 };
 #endif
 
+struct mc_info *g_mi;
+extern void bind_virq_for_mce(void);
 static __init int mce_init_device(void)
 {
 	int err;
@@ -664,6 +673,13 @@
 
 	register_hotcpu_notifier(&mce_cpu_notifier);
 	misc_register(&mce_log_device);
+
+    /*Register vIRQ handler for MCE LOG processing*/
+#if defined (CONFIG_XEN) && defined(CONFIG_X86_MCE_INTEL)
+    printk(KERN_DEBUG "MCE: bind virq for DOM0 Logging\n");
+    bind_virq_for_mce();
+#endif
+
 	return err;
 }
 
diff -r a66aa1d590ba arch/x86_64/kernel/mce_dom0.c
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/arch/x86_64/kernel/mce_dom0.c	Mon Jun 08 17:05:18 2009 +0800
@@ -0,0 +1,133 @@
+#include <linux/init.h>
+#include <linux/types.h>
+#include <linux/kernel.h>
+#include <xen/interface/xen.h>
+#include <xen/evtchn.h>
+#include <xen/interface/vcpu.h>
+#include <asm/hypercall.h>
+#include <asm/mce.h>
+
+static int convert_log(struct mc_info *mi)
+{
+	struct mcinfo_common *mic = NULL;
+	struct mcinfo_global *mc_global;
+	struct mcinfo_bank *mc_bank;
+	struct mce m;
+
+	x86_mcinfo_lookup(mic, mi, MC_TYPE_GLOBAL);
+	if (mic == NULL)
+	{
+		printk(KERN_ERR "DOM0_MCE_LOG: global data is NULL\n");
+		return -1;
+	}
+
+	mc_global = (struct mcinfo_global*)mic;
+	m.mcgstatus = mc_global->mc_gstatus;
+	m.cpu = mc_global->mc_coreid;/*for test*/
+	x86_mcinfo_lookup(mic, mi, MC_TYPE_BANK);
+	do
+	{
+		if (mic == NULL || mic->size == 0)
+			break;
+		if (mic->type == MC_TYPE_BANK)
+		{
+			mc_bank = (struct mcinfo_bank*)mic;
+			m.misc = mc_bank->mc_misc;
+			m.status = mc_bank->mc_status;
+			m.addr = mc_bank->mc_addr;
+			m.tsc = mc_bank->mc_tsc;
+			m.res1 = mc_bank->mc_ctrl2;
+			m.bank = mc_bank->mc_bank;
+			printk(KERN_DEBUG "[CPU%d, BANK%d, addr %llx, state %llx]\n", 
+						m.bank, m.cpu, m.addr, m.status);
+			/*log this record*/
+			mce_log(&m);
+		}
+		mic = x86_mcinfo_next(mic);
+	}while (1);
+
+	return 0;
+}
+
+extern struct mc_info *g_mi;
+
+/*dom0 mce virq handler, logging physical mce error info*/
+
+static irqreturn_t mce_dom0_interrupt(int irq, void *dev_id,
+									struct pt_regs *regs)
+{
+	xen_mc_t mc_op;
+	int result = 0;
+
+	printk(KERN_DEBUG "MCE_DOM0_LOG: enter dom0 mce vIRQ handler\n");
+	mc_op.cmd = XEN_MC_fetch;
+	mc_op.interface_version = XEN_MCA_INTERFACE_VERSION;
+	set_xen_guest_handle(mc_op.u.mc_fetch.data, g_mi);
+urgent:
+	mc_op.u.mc_fetch.flags = XEN_MC_URGENT;
+	result = HYPERVISOR_mca(&mc_op);
+	if (result || mc_op.u.mc_fetch.flags & XEN_MC_NODATA ||
+			mc_op.u.mc_fetch.flags & XEN_MC_FETCHFAILED)
+	{
+		printk(KERN_DEBUG "MCE_DOM0_LOG: No more urgent data\n");
+		goto nonurgent;
+	}
+	else
+	{
+		result = convert_log(g_mi);
+		if (result) {
+			printk(KERN_ERR "MCE_DOM0_LOG: Log conversion failed\n");
+			goto end;
+		}
+		/* After fetching the telem from DOM0, we need to dec the telem's
+		 * refcnt and release the entry. The telem is reserved and inc
+		 * refcnt when filling the telem.
+		 */
+		mc_op.u.mc_fetch.flags = XEN_MC_URGENT | XEN_MC_ACK;
+		result = HYPERVISOR_mca(&mc_op);
+
+		goto urgent;
+	}
+nonurgent:
+	mc_op.u.mc_fetch.flags = XEN_MC_NONURGENT;
+	result = HYPERVISOR_mca(&mc_op);
+	if (result || mc_op.u.mc_fetch.flags & XEN_MC_NODATA ||
+			mc_op.u.mc_fetch.flags & XEN_MC_FETCHFAILED)
+	{
+		printk(KERN_DEBUG "MCE_DOM0_LOG: No more nonurgent data\n");
+		goto end;
+	}
+	else
+	{
+		result = convert_log(g_mi);
+		if (result) {
+			printk(KERN_ERR "MCE_DOM0_LOG: Log conversion failed\n");
+			goto end;
+		}
+		/* After fetching the telem from DOM0, we need to dec the telem's
+		 * refcnt and release the entry. The telem is reserved and inc
+		 * refcnt when filling the telem.
+		 */
+		mc_op.u.mc_fetch.flags = XEN_MC_NONURGENT | XEN_MC_ACK;
+		result = HYPERVISOR_mca(&mc_op);
+
+		goto nonurgent;
+	}
+end:
+	return IRQ_HANDLED;
+}
+
+void bind_virq_for_mce(void)
+{
+	int ret;
+
+	ret  = bind_virq_to_irqhandler(VIRQ_ARCH_0, 0, 
+		mce_dom0_interrupt, 0, "mce", NULL);
+
+	g_mi = kmalloc(sizeof(struct mc_info), GFP_KERNEL);
+	if ( ret<0 )
+	{
+		printk(KERN_ERR "MCE_DOM0_LOG: bind_virq for DOM0 failed\n");
+	}
+}
+
diff -r a66aa1d590ba include/asm-x86_64/mach-xen/asm/hw_irq.h
--- a/include/asm-x86_64/mach-xen/asm/hw_irq.h	Mon Jun 08 12:53:25 2009 +0800
+++ b/include/asm-x86_64/mach-xen/asm/hw_irq.h	Mon Jun 08 17:05:18 2009 +0800
@@ -60,6 +60,9 @@
 #define NUM_INVALIDATE_TLB_VECTORS	8
 #endif
 
+/* Add for compiling arch/x86_64/kernel/mce_amd.c */
+#define THRESHOLD_APIC_VECTOR   0xf9
+#define THERMAL_APIC_VECTOR	0xfa
 /*
  * Local APIC timer IRQ vector is on a different priority level,
  * to work around the 'lost local interrupt if more than 2 IRQ
diff -r a66aa1d590ba include/asm-x86_64/mach-xen/asm/hypercall.h
--- a/include/asm-x86_64/mach-xen/asm/hypercall.h	Mon Jun 08 12:53:25 2009 +0800
+++ b/include/asm-x86_64/mach-xen/asm/hypercall.h	Mon Jun 08 17:05:18 2009 +0800
@@ -39,6 +39,7 @@
 
 #include <linux/string.h> /* memcpy() */
 #include <linux/stringify.h>
+#include <xen/interface/arch-x86/xen-mca.h>
 
 #ifndef __HYPERVISOR_H__
 # error "please don't include this file directly"
@@ -215,7 +216,13 @@
 	platform_op->interface_version = XENPF_INTERFACE_VERSION;
 	return _hypercall1(int, platform_op, platform_op);
 }
-
+static inline int __must_check
+HYPERVISOR_mca(
+	struct xen_mc *mc_op)
+{
+	mc_op->interface_version = XEN_MCA_INTERFACE_VERSION;
+	return _hypercall1(int, mca, mc_op);
+}
 static inline int __must_check
 HYPERVISOR_set_debugreg(
 	unsigned int reg, unsigned long value)

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel