diff mbox series

APEI: GHES: Have GHES honor the panic= setting

Message ID 20250113125224.GFZ4UMiNtWIJvgpveU@fat_crate.local (mailing list archive)
State Queued
Delegated to: Rafael Wysocki
Headers show
Series APEI: GHES: Have GHES honor the panic= setting | expand

Commit Message

Borislav Petkov Jan. 13, 2025, 12:52 p.m. UTC 
The GHES driver overrides the panic= setting by force-rebooting the
system after a fatal hw error has been reported. The intent being that
such an error would be reported earlier.

However, this is not optimal when a hard-to-debug issue requires long
time to reproduce and when that happens, the box will get rebooted after
30 seconds and thus destroy the whole hw context of when the error
happened.

So rip out the default GHES panic timeout and honor the global one.

In the panic disabled (panic=0) case, the error will still be logged to
dmesg for later inspection and if panic after a hw error is really
required, then that can be controlled the usual way - use panic= on the
cmdline or set it in the kernel .config's CONFIG_PANIC_TIMEOUT.

Reported-by: Feng Tang <feng.tang@linux.alibaba.com>
Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
Reviewed-by: Feng Tang <feng.tang@linux.alibaba.com>
---
 drivers/acpi/apei/ghes.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Comments

Ira Weiny Jan. 13, 2025, 8:08 p.m. UTC | #1 
Borislav Petkov wrote:
> The GHES driver overrides the panic= setting by force-rebooting the
> system after a fatal hw error has been reported. The intent being that
> such an error would be reported earlier.
> 
> However, this is not optimal when a hard-to-debug issue requires long
> time to reproduce and when that happens, the box will get rebooted after
> 30 seconds and thus destroy the whole hw context of when the error
> happened.
> 
> So rip out the default GHES panic timeout and honor the global one.
> 
> In the panic disabled (panic=0) case, the error will still be logged to
> dmesg for later inspection and if panic after a hw error is really
> required, then that can be controlled the usual way - use panic= on the
> cmdline or set it in the kernel .config's CONFIG_PANIC_TIMEOUT.
> 
> Reported-by: Feng Tang <feng.tang@linux.alibaba.com>
> Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
> Reviewed-by: Feng Tang <feng.tang@linux.alibaba.com>

Reviewed-by: Ira Weiny <ira.weiny@intel.com>
Rafael J. Wysocki Jan. 14, 2025, 5:29 p.m. UTC | #2 
On Mon, Jan 13, 2025 at 9:08 PM Ira Weiny <ira.weiny@intel.com> wrote:
>
> Borislav Petkov wrote:
> > The GHES driver overrides the panic= setting by force-rebooting the
> > system after a fatal hw error has been reported. The intent being that
> > such an error would be reported earlier.
> >
> > However, this is not optimal when a hard-to-debug issue requires long
> > time to reproduce and when that happens, the box will get rebooted after
> > 30 seconds and thus destroy the whole hw context of when the error
> > happened.
> >
> > So rip out the default GHES panic timeout and honor the global one.
> >
> > In the panic disabled (panic=0) case, the error will still be logged to
> > dmesg for later inspection and if panic after a hw error is really
> > required, then that can be controlled the usual way - use panic= on the
> > cmdline or set it in the kernel .config's CONFIG_PANIC_TIMEOUT.
> >
> > Reported-by: Feng Tang <feng.tang@linux.alibaba.com>
> > Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
> > Reviewed-by: Feng Tang <feng.tang@linux.alibaba.com>
>
> Reviewed-by: Ira Weiny <ira.weiny@intel.com>

Applied as 6.14 material, thanks!
diff mbox series

Patch

diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index 07789f0b59bc..b72772494655 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -173,8 +173,6 @@  static struct gen_pool *ghes_estatus_pool;
 static struct ghes_estatus_cache __rcu *ghes_estatus_caches[GHES_ESTATUS_CACHES_SIZE];
 static atomic_t ghes_estatus_cache_alloced;
 
-static int ghes_panic_timeout __read_mostly = 30;
-
 static void __iomem *ghes_map(u64 pfn, enum fixed_addresses fixmap_idx)
 {
 	phys_addr_t paddr;
@@ -983,14 +981,16 @@  static void __ghes_panic(struct ghes *ghes,
 			 struct acpi_hest_generic_status *estatus,
 			 u64 buf_paddr, enum fixed_addresses fixmap_idx)
 {
+	const char *msg = GHES_PFX "Fatal hardware error";
+
 	__ghes_print_estatus(KERN_EMERG, ghes->generic, estatus);
 
 	ghes_clear_estatus(ghes, estatus, buf_paddr, fixmap_idx);
 
-	/* reboot to log the error! */
 	if (!panic_timeout)
-		panic_timeout = ghes_panic_timeout;
-	panic("Fatal hardware error!");
+		pr_emerg("%s but panic disabled\n", msg);
+
+	panic(msg);
 }
 
 static int ghes_proc(struct ghes *ghes)