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: <mazejvrglvtqdfsevduahqxvlvjv6hlo3apivossn2jnxpsuds@os6kxmmsq3xt>
Date: Fri, 18 Jul 2025 09:11:15 -0700
From: Breno Leitao <leitao@...ian.org>
To: "Luck, Tony" <tony.luck@...el.com>, xueshuai@...ux.alibaba.com, 
	mahesh@...ux.ibm.com, oohall@...il.com
Cc: Borislav Petkov <bp@...en8.de>, 
	"Rafael J. Wysocki" <rafael@...nel.org>, Len Brown <lenb@...nel.org>, James Morse <james.morse@....com>, 
	"Moore, Robert" <robert.moore@...el.com>, "linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>, 
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, "acpica-devel@...ts.linux.dev" <acpica-devel@...ts.linux.dev>, 
	"kernel-team@...a.com" <kernel-team@...a.com>, osandov@...ndov.com, linuxppc-dev@...ts.ozlabs.org
Subject: Re: [PATCH] ghes: Track number of recovered hardware errors

Hello Tony,

On Thu, Jul 17, 2025 at 05:29:14PM +0000, Luck, Tony wrote:
>
> If the intent is still to add this information to vmcore (as in
> earlier discussions in this thread). Then it could go into
> kernel/vmcore_info.c (and be configured with CONFIG_VMCORE_INFO).
> 
> Would just need an empty stub in some header file for the
> log_recovered_error() function.

Thanks for the suggestion.

I found that I don't need to expose the metrics in vmcore info at all to
be able to read them from vmcore, given crash/drgn can read those
symbols.

Global variable hwerror_tracking will be write-only during kernel
run-time, and only read during post morten analyzes. I am still not sure
if the compiler might not get rid of them completely, given no on reads.
I am wondering if I should EXPORT_SYMBOL_GPL(hwerror_tracking) to avoid
any optimization there.

Anyway, this is the patch I am using and it solves the problem I am
interested in. Any opinion?

Thanks for your support,
--breno

commit 396d9bd5266607731b535f4246fd3e4971df9016
Author: Breno Leitao <leitao@...ian.org>
Date:   Thu Jul 17 07:39:26 2025 -0700

    vmcoreinfo: Track and log recoverable hardware errors
    
    Introduce a generic infrastructure for tracking recoverable hardware
    errors (HW errors that did not cause a panic) and record them for vmcore
    consumption. This aids post-mortem crash analysis tools by preserving
    a count and timestamp for the last occurrence of such errors.
    
    This patch adds centralized logging for three common sources of
    recoverable hardware errors:
    
      - PCIe AER Correctable errors
      - x86 Machine Check Exceptions (MCE)
      - APEI/CPER GHES corrected or recoverable errors
    
    Each source logs to a shared `hwerror_tracking` array, protected by a
    spinlock, and maintains a per-source error count and timestamp of the
    most recent event.
    
    hwerror_tracking is write-only at kernel runtime, and it is meant to be
    read from vmcore using tools like crash/drgn. For example, this is how
    it looks like from drgn:
    
            >>> prog['hwerror_tracking']
            (struct hwerror_tracking_info [3]){
                    {
                            .count = (int)0,
                            .timestamp = (time64_t)0,
                    },
                    {
                            .count = (int)0,
                            .timestamp = (time64_t)0,
                    },
                    {
                            .count = (int)844,
                            .timestamp = (time64_t)1752852018,
                    },
            }
    
    Suggested-by: Tony Luck <tony.luck@...el.com>
    Signed-off-by: Breno Leitao <leitao@...ian.org>

diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c
index 4da4eab56c81d..781cf574642eb 100644
--- a/arch/x86/kernel/cpu/mce/core.c
+++ b/arch/x86/kernel/cpu/mce/core.c
@@ -45,6 +45,7 @@
 #include <linux/task_work.h>
 #include <linux/hardirq.h>
 #include <linux/kexec.h>
+#include <linux/vmcore_info.h>
 
 #include <asm/fred.h>
 #include <asm/cpu_device_id.h>
@@ -1692,6 +1693,8 @@ noinstr void do_machine_check(struct pt_regs *regs)
 out:
 	instrumentation_end();
 
+	/* Given it didn't panic, mark it as recoverable */
+	hwerror_tracking_log(HWE_RECOV_MCE);
 clear:
 	mce_wrmsrq(MSR_IA32_MCG_STATUS, 0);
 }
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index f0584ccad4519..255453cdc72e9 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -43,6 +43,7 @@
 #include <linux/uuid.h>
 #include <linux/ras.h>
 #include <linux/task_work.h>
+#include <linux/vmcore_info.h>
 
 #include <acpi/actbl1.h>
 #include <acpi/ghes.h>
@@ -1100,13 +1101,16 @@ static int ghes_proc(struct ghes *ghes)
 {
 	struct acpi_hest_generic_status *estatus = ghes->estatus;
 	u64 buf_paddr;
-	int rc;
+	int rc, sev;
 
 	rc = ghes_read_estatus(ghes, estatus, &buf_paddr, FIX_APEI_GHES_IRQ);
 	if (rc)
 		goto out;
 
-	if (ghes_severity(estatus->error_severity) >= GHES_SEV_PANIC)
+	sev = ghes_severity(estatus->error_severity);
+	if (sev == GHES_SEV_RECOVERABLE || sev ==  GHES_SEV_CORRECTED)
+		hwerror_tracking_log(HWE_RECOV_GHES);
+	else if (sev >= GHES_SEV_PANIC)
 		__ghes_panic(ghes, estatus, buf_paddr, FIX_APEI_GHES_IRQ);
 
 	if (!ghes_estatus_cached(estatus)) {
diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index 70ac661883672..9d4fa1cb8afb9 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -30,6 +30,7 @@
 #include <linux/kfifo.h>
 #include <linux/ratelimit.h>
 #include <linux/slab.h>
+#include <linux/vmcore_info.h>
 #include <acpi/apei.h>
 #include <acpi/ghes.h>
 #include <ras/ras_event.h>
@@ -746,6 +747,7 @@ static void pci_dev_aer_stats_incr(struct pci_dev *pdev,
 	switch (info->severity) {
 	case AER_CORRECTABLE:
 		aer_info->dev_total_cor_errs++;
+		hwerror_tracking_log(HWE_RECOV_AER);
 		counter = &aer_info->dev_cor_errs[0];
 		max = AER_MAX_TYPEOF_COR_ERRS;
 		break;
diff --git a/include/linux/vmcore_info.h b/include/linux/vmcore_info.h
index 37e003ae52626..5894da92a6ba4 100644
--- a/include/linux/vmcore_info.h
+++ b/include/linux/vmcore_info.h
@@ -77,4 +77,18 @@ extern u32 *vmcoreinfo_note;
 Elf_Word *append_elf_note(Elf_Word *buf, char *name, unsigned int type,
 			  void *data, size_t data_len);
 void final_note(Elf_Word *buf);
+
+enum hwerror_tracking_source {
+	HWE_RECOV_AER,
+	HWE_RECOV_MCE,
+	HWE_RECOV_GHES,
+	HWE_RECOV_MAX,
+};
+
+#ifdef CONFIG_VMCORE_INFO
+void hwerror_tracking_log(enum hwerror_tracking_source src);
+#else
+void hwerror_tracking_log(enum hwerror_tracking_source src) {};
+#endif
+
 #endif /* LINUX_VMCORE_INFO_H */
diff --git a/kernel/vmcore_info.c b/kernel/vmcore_info.c
index e066d31d08f89..c3d2bfffec298 100644
--- a/kernel/vmcore_info.c
+++ b/kernel/vmcore_info.c
@@ -31,6 +31,14 @@ u32 *vmcoreinfo_note;
 /* trusted vmcoreinfo, e.g. we can make a copy in the crash memory */
 static unsigned char *vmcoreinfo_data_safecopy;
 
+static struct hwerror_tracking_info {
+	int         count;
+	time64_t    timestamp;
+};
+
+static struct hwerror_tracking_info hwerror_tracking[HWE_RECOV_MAX];
+static DEFINE_SPINLOCK(hwerror_tracking_lock);
+
 Elf_Word *append_elf_note(Elf_Word *buf, char *name, unsigned int type,
 			  void *data, size_t data_len)
 {
@@ -118,6 +126,23 @@ phys_addr_t __weak paddr_vmcoreinfo_note(void)
 }
 EXPORT_SYMBOL(paddr_vmcoreinfo_note);
 
+void hwerror_tracking_log(enum hwerror_tracking_source src)
+{
+	struct hwerror_tracking_info *hwet;
+	unsigned long flags;
+
+	if (src < 0 || src >= HWE_RECOV_MAX)
+		return;
+
+	hwet = &hwerror_tracking[src];
+
+	spin_lock_irqsave(&hwerror_tracking_lock, flags);
+	hwet->count++;
+	hwet->timestamp = ktime_get_real_seconds();
+	spin_unlock_irqrestore(&hwerror_tracking_lock, flags);
+}
+EXPORT_SYMBOL_GPL(hwerror_tracking_log);
+
 static int __init crash_save_vmcoreinfo_init(void)
 {
 	vmcoreinfo_data = (unsigned char *)get_zeroed_page(GFP_KERNEL);

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ