[lkml]   [2012]   [Apr]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [Xen-devel] [PATCH 1/3] Add mcelog support for xen platform
    + 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.



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

     \ /
      Last update: 2012-04-21 12:49    [W:0.022 / U:10.584 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site