linux-kernel - RE: [Xen-devel] [PATCH 1/3] xen/mce: Add mcelog support for Xen platform (v2)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <DE8DF0795D48FD4CA783C40EC82923351F4313@SHSMSX101.ccr.corp.intel.com>
Date:	Mon, 28 May 2012 13:36:45 +0000
From:	"Liu, Jinsong" <jinsong.liu@...el.com>
To:	'Konrad Rzeszutek Wilk' <konrad.wilk@...cle.com>,
	Borislav Petkov <bp@...64.org>
CC:	"'linux-kernel@...r.kernel.org'" <linux-kernel@...r.kernel.org>,
	"Luck, Tony" <tony.luck@...el.com>,
	"'xen-devel@...ts.xensource.com'" <xen-devel@...ts.xensource.com>
Subject: RE: [Xen-devel] [PATCH 1/3] xen/mce: Add mcelog support for Xen
 platform (v2)

Liu, Jinsong wrote:
> Konrad Rzeszutek Wilk wrote:
>> On Fri, May 25, 2012 at 05:56:56PM +0000, Liu, Jinsong wrote:
>>> Konrad Rzeszutek Wilk wrote:
>>>>>>>>> -static struct miscdevice mce_chrdev_device = {
>>>>>>>>> +struct miscdevice mce_chrdev_device = {
>>>>>>>>>  	MISC_MCELOG_MINOR,
>>>>>>>>>  	"mcelog",
>>>>>>>>>  	&mce_chrdev_ops,
>>>>>>>> 
>>>>>>>> You're still reusing those - pls, define your own 'struct
>>>>>>>> miscdevice mce_chrdev_device' in drivers/xen/ or somewhere
>>>>>>>> convenient and your own mce_chrdev_ops. The only thing you
>>>>>>>> should be touching in arch/x86/.../mcheck/ is the export of
>>>>>>>> MISC_MCELOG_MINOR. 
>>>>>>>> 
>>>>>>>> Thanks.
>>>>>>> 
>>>>>>> I'm *not* reuse native code.
>>>>>>> I have defined 'struct miscdevice xen_mce_chrdev_device' in
>>>>>>> drivers/xen, and I also implement xen_mce_chrdev_ops, they are
>>>>>>> all xen-self-contained. 
>>>>>>> 
>>>>>>> The patch just redirect native mce_chrdev_device to
>>>>>>> xen_mce_chrdev_device when running under xen environment.
>>>>>>> It didn't change any native code (except just cancel
>>>>>>> mce_chrdev_device 'static'), and will not break native logic.
>>>>>> 
>>>>>> Why are you doing that?
>>>>>> 
>>>>>> Why don't you do
>>>>>> 
>>>>>> 	misc_register(&xen_mce_chrdev_device);
>>>>>> 
>>>>>> in xen_early_init_mcelog() ?
>>>>>> 
>>>>>> This way there'll be no arch/x86/ dependencies at all.
>>>>> 
>>>>> The reason is, if we do so, it would be covered by native
>>>>> misc_register(&mce_chrdev_device) later when native kernel init
>>>>> (xen init first and then start native kernel).
>>>> 
>>>> Won't the second registration (so the original one) of the major
>>>> fail? So the mce_log would just error out since somebody already
>>>> registered?
>>> 
>>> No, that would be device confliction, the 2nd register return as
>>> -EBUSY and un-predicetable result.
>> 
>> And the existing code does not actually check the 'misc_register'
>> return value? Ah yes. Perhaps then a fix to
>> arch/x86/kernel/cpu/mcheck/mce.c to do the proper de-registration if
>> 'misc_register' fails?
> 
> It's weird. From code point of view, it indeed not check return value
> so it should go silently. mce.c didn't do misc_deregister. 

Have a debug it, the root cause is, 
1). it does misc_register(&xen_mce_chrdev_device) at xen_early_init_mcelog(), it's very early stage (at xen_start_kernel), linux kernel and dd didn't start yet, so it fail to register xen_mce_chrdev_device.
2). After native start_kernel, native mce_chrdev_device registered successfully, then it point to *native* mcelog logic.

> 
>> 
>> Or just set 'mce_disabled=1' in the bootup of Xen, similar to
>> how lguest.c does it?

Have a look at lguest, it disable whole mce logic.
Xen dom0 cannot do so since we need keep dom0 mce logic to handle memory error which belong to dom0 (hypervisor will inject vMCE to dom0 in the case).


===================

Borislav and Konrad,

An approach which basically same as you suggested but w/ slightly update, is
1). at xen/mcelog.c, do misc_register(&xen_mce_chrdev_device) at xen_late_init_mcelog, define it as device_initcall(xen_late_init_mcelog) --> now linux dd ready, so xen mcelog divice would register successfully;
2). at native mce.c, change 1 line from device_initcall(mcheck_init_device) to device_initcall_sync(mcheck_init_device) --> so misc_register(&mce_chrdev_device) would be blocked by xen mcelog device;

I have draft test it and works fine.
Thought?

Thanks,
Jinsong
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/