public inbox for linuxppc-dev@ozlabs.org
 help / color / mirror / Atom feed
* [PATCH] powerpc/fadump: Add timeout to RTAS busy-wait loops
@ 2026-04-06  6:15 Adriano Vero
  2026-04-07  4:06 ` Ritesh Harjani
  0 siblings, 1 reply; 3+ messages in thread
From: Adriano Vero @ 2026-04-06  6:15 UTC (permalink / raw)
  To: maddy, mpe; +Cc: npiggin, chleroy, linuxppc-dev, linux-kernel, Adriano Vero

The ibm,configure-kernel-dump RTAS call sites in
rtas_fadump_register(), rtas_fadump_unregister(), and
rtas_fadump_invalidate() polled indefinitely while firmware returned
a busy status. A misbehaving or hung firmware could stall these paths
forever, blocking fadump registration at boot or preventing clean
teardown.

Track the accumulated delay in a total_wait counter and bail out with
-ETIMEDOUT if it reaches RTAS_FADUMP_MAX_WAIT_MS (60 seconds) before
firmware signals completion. This follows the bounded busy-wait pattern
used in rtas-rtc.c.

Signed-off-by: Adriano Vero <litaliano00.contact@gmail.com>
---
 arch/powerpc/platforms/pseries/rtas-fadump.c | 37 ++++++++++++++------
 arch/powerpc/platforms/pseries/rtas-fadump.h |  6 ++++
 2 files changed, 33 insertions(+), 10 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/rtas-fadump.c b/arch/powerpc/platforms/pseries/rtas-fadump.c
index eceb32893..b165f165c 100644
--- a/arch/powerpc/platforms/pseries/rtas-fadump.c
+++ b/arch/powerpc/platforms/pseries/rtas-fadump.c
@@ -181,7 +181,7 @@ static u64 rtas_fadump_get_bootmem_min(void)
 
 static int rtas_fadump_register(struct fw_dump *fadump_conf)
 {
-	unsigned int wait_time, fdm_size;
+	unsigned int wait_time, total_wait, fdm_size;
 	int rc, err = -EIO;
 
 	/*
@@ -192,15 +192,20 @@ static int rtas_fadump_register(struct fw_dump *fadump_conf)
 	fdm_size = sizeof(struct rtas_fadump_section_header);
 	fdm_size += be16_to_cpu(fdm.header.dump_num_sections) * sizeof(struct rtas_fadump_section);
 
-	/* TODO: Add upper time limit for the delay */
+	total_wait = 0;
 	do {
 		rc =  rtas_call(fadump_conf->ibm_configure_kernel_dump, 3, 1,
 				NULL, FADUMP_REGISTER, &fdm, fdm_size);
 
 		wait_time = rtas_busy_delay_time(rc);
-		if (wait_time)
+		if (wait_time) {
+			if (total_wait >= RTAS_FADUMP_MAX_WAIT_MS) {
+				pr_err("Timed out waiting for firmware to register fadump\n");
+				return -ETIMEDOUT;
+			}
+			total_wait += wait_time;
 			mdelay(wait_time);
-
+		}
 	} while (wait_time);
 
 	switch (rc) {
@@ -234,18 +239,24 @@ static int rtas_fadump_register(struct fw_dump *fadump_conf)
 
 static int rtas_fadump_unregister(struct fw_dump *fadump_conf)
 {
-	unsigned int wait_time;
+	unsigned int wait_time, total_wait;
 	int rc;
 
-	/* TODO: Add upper time limit for the delay */
+	total_wait = 0;
 	do {
 		rc =  rtas_call(fadump_conf->ibm_configure_kernel_dump, 3, 1,
 				NULL, FADUMP_UNREGISTER, &fdm,
 				sizeof(struct rtas_fadump_mem_struct));
 
 		wait_time = rtas_busy_delay_time(rc);
-		if (wait_time)
+		if (wait_time) {
+			if (total_wait >= RTAS_FADUMP_MAX_WAIT_MS) {
+				pr_err("Timed out waiting for firmware to unregister fadump\n");
+				return -ETIMEDOUT;
+			}
+			total_wait += wait_time;
 			mdelay(wait_time);
+		}
 	} while (wait_time);
 
 	if (rc) {
@@ -259,18 +270,24 @@ static int rtas_fadump_unregister(struct fw_dump *fadump_conf)
 
 static int rtas_fadump_invalidate(struct fw_dump *fadump_conf)
 {
-	unsigned int wait_time;
+	unsigned int wait_time, total_wait;
 	int rc;
 
-	/* TODO: Add upper time limit for the delay */
+	total_wait = 0;
 	do {
 		rc =  rtas_call(fadump_conf->ibm_configure_kernel_dump, 3, 1,
 				NULL, FADUMP_INVALIDATE, fdm_active,
 				sizeof(struct rtas_fadump_mem_struct));
 
 		wait_time = rtas_busy_delay_time(rc);
-		if (wait_time)
+		if (wait_time) {
+			if (total_wait >= RTAS_FADUMP_MAX_WAIT_MS) {
+				pr_err("Timed out waiting for firmware to invalidate fadump\n");
+				return -ETIMEDOUT;
+			}
+			total_wait += wait_time;
 			mdelay(wait_time);
+		}
 	} while (wait_time);
 
 	if (rc) {
diff --git a/arch/powerpc/platforms/pseries/rtas-fadump.h b/arch/powerpc/platforms/pseries/rtas-fadump.h
index c109abf6b..65fdab7b5 100644
--- a/arch/powerpc/platforms/pseries/rtas-fadump.h
+++ b/arch/powerpc/platforms/pseries/rtas-fadump.h
@@ -41,6 +41,12 @@
 #define MAX_SECTIONS				10
 #define RTAS_FADUMP_MAX_BOOT_MEM_REGS		7
 
+/*
+ * Maximum time to wait for firmware to respond to an
+ * ibm,configure-kernel-dump RTAS call before giving up.
+ */
+#define RTAS_FADUMP_MAX_WAIT_MS			60000U
+
 /* Kernel Dump section info */
 struct rtas_fadump_section {
 	__be32	request_flag;
-- 
2.53.0



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] powerpc/fadump: Add timeout to RTAS busy-wait loops
  2026-04-06  6:15 [PATCH] powerpc/fadump: Add timeout to RTAS busy-wait loops Adriano Vero
@ 2026-04-07  4:06 ` Ritesh Harjani
  2026-04-07  6:28   ` litaliano00
  0 siblings, 1 reply; 3+ messages in thread
From: Ritesh Harjani @ 2026-04-07  4:06 UTC (permalink / raw)
  To: Adriano Vero, maddy, mpe
  Cc: npiggin, chleroy, linuxppc-dev, linux-kernel, Adriano Vero

Adriano Vero <litaliano00.contact@gmail.com> writes:

> The ibm,configure-kernel-dump RTAS call sites in
> rtas_fadump_register(), rtas_fadump_unregister(), and
> rtas_fadump_invalidate() polled indefinitely while firmware returned
> a busy status. A misbehaving or hung firmware could stall these paths
> forever, blocking fadump registration at boot or preventing clean
> teardown.

Was there an issue which you encountered? Can you share the details of
the same please?


>
> Track the accumulated delay in a total_wait counter and bail out with
> -ETIMEDOUT if it reaches RTAS_FADUMP_MAX_WAIT_MS (60 seconds) before
> firmware signals completion. This follows the bounded busy-wait pattern
> used in rtas-rtc.c.
>
> Signed-off-by: Adriano Vero <litaliano00.contact@gmail.com>


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] powerpc/fadump: Add timeout to RTAS busy-wait loops
  2026-04-07  4:06 ` Ritesh Harjani
@ 2026-04-07  6:28   ` litaliano00
  0 siblings, 0 replies; 3+ messages in thread
From: litaliano00 @ 2026-04-07  6:28 UTC (permalink / raw)
  To: Ritesh Harjani; +Cc: maddy, mpe, npiggin, chleroy, linuxppc-dev, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1834 bytes --]

Hi Ritesh,

No, I haven't encountered a specific hang on live hardware in production.

This was a proactive fix that originated from an audit of the RTAS call
sites. I noticed the explicit `/* TODO: Add upper time limit for the delay
*/` comments in rtas-fadump.c that had not yet been implemented.

When cross-referencing this with other parts of the pSeries code (such as
the bounded busy-wait pattern used in rtas-rtc.c), it seemed that adding a
timeout is the standard defensive programming approach for these hypervisor
calls.

Since these fadump registration and teardown paths happen during critical
boot and state transition phases, a misbehaving hypervisor or firmware
anomaly could lead to a hard-to-debug, silent system stall. I implemented
the 60-second timeout to resolve the pending TODOs and ensure the kernel
remains resilient in those specific edge cases.

Thanks,
Adriano

On Tue, Apr 7, 2026 at 6:07 AM Ritesh Harjani <ritesh.list@gmail.com> wrote:

> Adriano Vero <litaliano00.contact@gmail.com> writes:
>
> > The ibm,configure-kernel-dump RTAS call sites in
> > rtas_fadump_register(), rtas_fadump_unregister(), and
> > rtas_fadump_invalidate() polled indefinitely while firmware returned
> > a busy status. A misbehaving or hung firmware could stall these paths
> > forever, blocking fadump registration at boot or preventing clean
> > teardown.
>
> Was there an issue which you encountered? Can you share the details of
> the same please?
>
>
> >
> > Track the accumulated delay in a total_wait counter and bail out with
> > -ETIMEDOUT if it reaches RTAS_FADUMP_MAX_WAIT_MS (60 seconds) before
> > firmware signals completion. This follows the bounded busy-wait pattern
> > used in rtas-rtc.c.
> >
> > Signed-off-by: Adriano Vero <litaliano00.contact@gmail.com>
>

[-- Attachment #2: Type: text/html, Size: 2390 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-04-07  6:28 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-06  6:15 [PATCH] powerpc/fadump: Add timeout to RTAS busy-wait loops Adriano Vero
2026-04-07  4:06 ` Ritesh Harjani
2026-04-07  6:28   ` litaliano00

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox