Re: [PATCH v2 1/2] cper, apei, mce: Pass x86 CPER through the MCA handling chain

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

Hi Smita,

A couple of comments below -

Smita Koralahalli <Smita.KoralahalliChannabasappa@xxxxxxx> 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

[...]




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux