| 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
| ||
|
Message-ID: <AANLkTimY5hr3DgN9=v83Z289MkzjgX8k0WzdTWZbGorj@mail.gmail.com>
Date: Mon, 27 Sep 2010 20:47:53 +0800
From: huang ying <huang.ying.caritas@...il.com>
To: Robert Richter <robert.richter@....com>
Cc: Huang Ying <ying.huang@...el.com>, Don Zickus <dzickus@...hat.com>,
Ingo Molnar <mingo@...e.hu>, "H. Peter Anvin" <hpa@...or.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Andi Kleen <andi@...stfloor.org>
Subject: Re: [PATCH -v2 6/7] x86, NMI, Add support to notify hardware error
with unknown NMI
On Mon, Sep 27, 2010 at 6:09 PM, Robert Richter <robert.richter@....com> wrote:
> On 26.09.10 20:57:05, Huang Ying wrote:
>> On some platforms, fatal hardware error may be notified via unknown
>> NMI.
>>
>> For example, on some platform with APEI firmware first mode support,
>> firmware generates NMI for fatal error but without error record. The
>> unknown NMI should be treated as notification of fatal hardware
>> error. The unknown_nmi_for_hwerr is added for these platform, if it is
>> not zero, system will treat unknown NMI as notification of fatal
>> hardware error.
>>
>> These platforms are identified via the presentation of APEI HEST or
>> some PCI ID of the host bridge. The PCI ID of host bridge instead of
>> DMI ID is used, so that the checking can be done based on the platform
>> type instead of motherboard. This should be simpler and sufficient.
>>
>> The method to identify the platforms is designed by Andi Kleen.
>>
>> Signed-off-by: Huang Ying <ying.huang@...el.com>
>> ---
>> arch/x86/include/asm/nmi.h | 1
>> arch/x86/kernel/Makefile | 2 +
>> arch/x86/kernel/hwerr.c | 55 +++++++++++++++++++++++++++++++++++++++++++++
>
> Instead of creating this file the code should be implemented in
>
> arch/x86/kernel/cpu/intel.c
>
> Similar AMD NB code is implemented in amd.c and k8.c.
Why? This file is not vendor specific.
>> arch/x86/kernel/traps.c | 10 ++++++++
>> drivers/acpi/apei/hest.c | 8 ++++++
>> 5 files changed, 76 insertions(+)
>> create mode 100644 arch/x86/kernel/hwerr.c
>>
>> --- a/arch/x86/include/asm/nmi.h
>> +++ b/arch/x86/include/asm/nmi.h
>> @@ -44,6 +44,7 @@ struct ctl_table;
>> extern int proc_nmi_enabled(struct ctl_table *, int ,
>> void __user *, size_t *, loff_t *);
>> extern int unknown_nmi_panic;
>> +extern int unknown_nmi_for_hwerr;
>>
>> void arch_trigger_all_cpu_backtrace(void);
>> #define arch_trigger_all_cpu_backtrace arch_trigger_all_cpu_backtrace
>> --- a/arch/x86/kernel/Makefile
>> +++ b/arch/x86/kernel/Makefile
>> @@ -118,6 +118,8 @@ obj-$(CONFIG_X86_CHECK_BIOS_CORRUPTION)
>>
>> obj-$(CONFIG_SWIOTLB) += pci-swiotlb.o
>>
>> +obj-y += hwerr.o
>> +
>> ###
>> # 64 bit specific files
>> ifeq ($(CONFIG_X86_64),y)
>> --- /dev/null
>> +++ b/arch/x86/kernel/hwerr.c
>> @@ -0,0 +1,55 @@
>> +/*
>> + * Hardware error architecture dependent processing
>> + *
>> + * Copyright 2010 Intel Corp.
>> + * Author: Huang Ying <ying.huang@...el.com>
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License version
>> + * 2 as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, write to the Free Software
>> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
>> + */
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/pci.h>
>> +#include <linux/init.h>
>> +#include <linux/nmi.h>
>> +
>> +/*
>> + * On some platform, hardware errors may be notified via unknown
>> + * NMI. These platform is identified via the PCI ID of host bridge.
>> + *
>> + * The PCI ID of host bridge instead of DMI ID is used, so that the
>> + * checking can be done based on the platform instead of motherboard.
>> + * This should be simpler and sufficient.
>> + */
>> +static const
>> +struct pci_device_id unknown_nmi_for_hwerr_platform[] __initdata = {
>> + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x3406) },
>> + { 0, }
>> +};
>> +
>> +int __init check_unknown_nmi_for_hwerr(void)
>> +{
>> + struct pci_dev *dev = NULL;
>> +
>> + for_each_pci_dev(dev) {
>> + if (pci_match_id(unknown_nmi_for_hwerr_platform, dev)) {
>> + pr_info(
>> +"Host bridge is identified, will treat unknown NMI as hardware error!\n");
>> + unknown_nmi_for_hwerr = 1;
>> + break;
>> + }
>> + }
>> +
>> + return 0;
>> +}
>> +late_initcall(check_unknown_nmi_for_hwerr);
>
> Maybe you can use early pci functions like read_pci_config() to avoid
> late init.
I don't think late init is a big issue. Hardware error is rare after all.
>> --- a/arch/x86/kernel/traps.c
>> +++ b/arch/x86/kernel/traps.c
>> @@ -83,6 +83,8 @@ EXPORT_SYMBOL_GPL(used_vectors);
>>
>> static int ignore_nmis;
>>
>> +int unknown_nmi_for_hwerr;
>
> If it is an nmi for hwerr, it is no longer an unknown nmi. So we
> should drop 'unknow' in the naming.
I think unkown NMI is the one we can not identify the source.
Something like anonymous.
>> +
>> /*
>> * Prevent NMI reason port (0x61) being accessed simultaneously, can
>> * only be used in NMI handler.
>> @@ -360,6 +362,14 @@ io_check_error(unsigned char reason, str
>> static notrace __kprobes void
>> unknown_nmi_error(unsigned char reason, struct pt_regs *regs)
>> {
>> + /*
>> + * On some platforms, hardware errors may be notified via
>> + * unknown NMI
>> + */
>> + if (unknown_nmi_for_hwerr)
>> + panic("NMI for hardware error without error record: "
>> + "Not continuing");
>> +
>
> Instead of checking this flag you should implement and register an nmi
> handler for this case.
I think explicit function calls have better readability than notifier chains.
Best Regards,
Huang Ying
--
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/
Powered by blists - more mailing lists