Re: [Xen-devel] [PATCH 1/3] Add mcelog support for xen platform

[Borislav Petkov <bp@amd64.org>]

+ x86 people.

On Sat, Apr 21, 2012 at 12:14:54AM -0400, Konrad Rzeszutek Wilk wrote:
> Should this driver reside in arch/x86/kernel/cpu/mcheck?

The fact that you raise such a question is already loaded with problems,
see below.

> I can keep it in drivers/xen, but I realized that perhaps it
> should be in a different location?
> > 
> > When MCA error occurs, it would be handled by xen hypervisor first,
> > and then the error information would be sent to dom0 for logging.
> > 
> > This patch gets error information from xen hypervisor and convert
> > xen format error into linux format mcelog. This logic is basically
> > self-contained, not much touching other kernel components.

I have a problem with that statement above. This thing uses struct mce
and mce_log(), which are internal to x86, and whenever we want to change
anything there, we won't be able to because xen uses it too.

And this has happened already with the whole microcode loading debacle.

So, my suggestion is to copy the pieces you need and create your own xen
version of /dev/mcelog and add it to dom0 so that there's no hooking
into baremetal code and whenever a dom0 kernel is running, you can
reroute the mcelog userspace tool to read /dev/xen_mcelog or whatever
and not hook into the x86 versions.

Because, if you'd hooked into it, just imagine one fine day, when we
remove mcelog support, what screaming the xen people will be doing when
mcelog doesn't work anymore.

Thanks.

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551