RE: [Xen-devel] [PATCH] DOM0: Adding MCA Loging support in DOM0

["Jan Beulich" <JBeulich@xxxxxxxxxx>]

>>> "Jiang, Yunhong" <yunhong.jiang@xxxxxxxxx> 09.06.09 11:30 >>>
>xen-devel-bounces@xxxxxxxxxxxxxxxxxxx wrote:
>>>>> "Ke, Liping" <liping.ke@xxxxxxxxx> 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


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