linux-kernel - Re: [PATCH v5] cper, apei, mce: Pass x86 CPER through the MCA handling chain

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-prev] [day] [month] [year] [list]

Message-ID: <87361e8259.fsf@kokedama.swc.toshiba.co.jp>
Date:   Fri, 13 Nov 2020 10:40:50 +0900
From:   Punit Agrawal <punit1.agrawal@...hiba.co.jp>
To:     Smita Koralahalli Channabasappa <skoralah@....com>
Cc:     Borislav Petkov <bp@...en8.de>,
        Smita Koralahalli <Smita.KoralahalliChannabasappa@....com>,
        <x86@...nel.org>, <linux-kernel@...r.kernel.org>,
        <linux-pm@...r.kernel.org>, <linux-edac@...r.kernel.org>,
        <linux-efi@...r.kernel.org>, <linux-acpi@...r.kernel.org>,
        Tony Luck <tony.luck@...el.com>,
        "Rafael J . Wysocki" <rjw@...ysocki.net>,
        Len Brown <len.brown@...el.com>,
        Ard Biesheuvel <ardb@...nel.org>,
        Yazen Ghannam <yazen.ghannam@....com>
Subject: Re: [PATCH v5] cper, apei, mce: Pass x86 CPER through the MCA handling chain

Smita Koralahalli Channabasappa <skoralah@....com> writes:

> Punit,
>
> On 11/9/20 1:05 PM, Smita Koralahalli Channabasappa wrote:
>
>> On 11/8/20 7:18 PM, Punit Agrawal wrote:
>>> Borislav Petkov <bp@...en8.de> writes:
>>>> On Fri, Nov 06, 2020 at 02:36:46PM +0900, Punit Agrawal wrote:
>>>>>> diff --git a/drivers/firmware/efi/cper-x86.c b/drivers/firmware/efi/cper-x86.c
>>>>>> index 2531de49f56c..438ed9eff6d0 100644
>>>>>> --- a/drivers/firmware/efi/cper-x86.c
>>>>>> +++ b/drivers/firmware/efi/cper-x86.c
>>>>>> @@ -2,6 +2,7 @@
>>>>>>    // Copyright (C) 2018, Advanced Micro Devices, Inc.
>>>>>>      #include <linux/cper.h>
>>>>>> +#include <linux/acpi.h>
>>>>> Did you mean to include <asm/acpi.h>?
>>>> Why?
>>> Because arch_apei_report_x86_error() used in the patch is defined
>>> there. The indirect include works but pulls in additional definitions
>>> not needed by the patch.
>>>
>>> Do you prefer the more generic include?
>> I agree, it's generally a good practice to avoid pulling up additional
>> definitions. I had this when I made the declaration in generic header
>> file and may be I did not consider it changing initially as my build
>> didn't break after moving the declaration from generic header to arch
>> specific header file.
>> I will take care henceforth and make the changes as required.
>
> The asm specific include throws out a warning when I run checkpatch.pl
>
> WARNING: Use #include <linux/acpi.h> instead of <asm/acpi.h>
> #215: FILE: drivers/firmware/efi/cper-x86.c:5:
> +#include <asm/acpi.h>
>
> Should I just keep the generic include?

Thanks for checking.

I had a quick look at checkpatch to understand the reason for the
warning. It seems to warn when "asm" includes are used when a suitable
"linux" include exists[0].

I am not convinced that the rationale for that check applies in this
case as the function being used is indeed an architecture specific one
but also don't feel strongly enough to object.

Feel free to pick up the "Reviewed-by" tag in either case.

Thanks,
Punit

[0] https://github.com/torvalds/linux/blob/master/scripts/checkpatch.pl#L5333