linux-kernel - Re: [PATCH v2 1/2] 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <102d0c75-d642-8f8b-68c7-792499c2a62a@amd.com>
Date:   Wed, 2 Sep 2020 14:29:28 -0500
From:   Smita Koralahalli Channabasappa <skoralah@....com>
To:     Punit Agrawal <punit1.agrawal@...hiba.co.jp>
Cc:     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,
        devel@...ica.org, Borislav Petkov <bp@...en8.de>,
        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 v2 1/2] cper, apei, mce: Pass x86 CPER through the MCA
 handling chain

On 8/31/20 12:05 AM, Punit Agrawal wrote:

> Hi Smita,
>
> A couple of comments below -
>
> Smita Koralahalli <Smita.KoralahalliChannabasappa@....com> writes:
>
> [...]
>
>
>> diff --git a/drivers/firmware/efi/cper-x86.c b/drivers/firmware/efi/cper-x86.c
>> index 2531de49f56c..374b8e18552a 100644
>> --- a/drivers/firmware/efi/cper-x86.c
>> +++ b/drivers/firmware/efi/cper-x86.c
>> @@ -1,7 +1,7 @@
>>   // SPDX-License-Identifier: GPL-2.0
>>   // Copyright (C) 2018, Advanced Micro Devices, Inc.
>>   
>> -#include <linux/cper.h>
> Why is the include dropped? AFAICT, the definitions from there are still
> being used after this patch.

Dropped because <acpi/apei.h> already includes <linux/cper.h>

>> +#include <acpi/apei.h>

[...]

>> diff --git a/include/acpi/apei.h b/include/acpi/apei.h
>> index 680f80960c3d..44d4d08acce0 100644
>> --- a/include/acpi/apei.h
>> +++ b/include/acpi/apei.h
>> @@ -33,8 +33,15 @@ extern bool ghes_disable;
>>   
>>   #ifdef CONFIG_ACPI_APEI
>>   void __init acpi_hest_init(void);
>> +int arch_apei_report_x86_error(struct cper_ia_proc_ctx *ctx_info,
>> +			       u64 lapic_id);
>>   #else
>>   static inline void acpi_hest_init(void) { return; }
>> +static inline int arch_apei_report_x86_error(struct cper_ia_proc_ctx *ctx_info,
>> +					     u64 lapic_id)
>> +{
>> +	return -EINVAL;
>> +}
>>   #endif
> Adding the declaration to this include violates the separation of
> generic and architecture specific code.
>
> Can this be moved to the appropriate architecture specific header?
> Perhaps arch/x86/include/asm/apei.h.

Yes, I have fixed this and moved into arch/x86/include/asm/acpi.h.

>>   typedef int (*apei_hest_func_t)(struct acpi_hest_header *hest_hdr, void *data);
>> @@ -51,6 +58,8 @@ int erst_clear(u64 record_id);
>>   
>>   int arch_apei_enable_cmcff(struct acpi_hest_header *hest_hdr, void *data);
>>   void arch_apei_report_mem_error(int sev, struct cper_sec_mem_err *mem_err);
>> +int arch_apei_report_x86_error(struct cper_ia_proc_ctx *ctx_info,
>> +			       u64 lapic_id);
>
> Why is the additional declaration needed?

Will fix in the next revision.

Thanks,
Smita

[...]