[PATCH] powerpc/fadump: Add timeout to RTAS busy-wait loops

[PATCH] powerpc/fadump: Add timeout to RTAS busy-wait loops

[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

Re: [PATCH] powerpc/fadump: Add timeout to RTAS busy-wait loops

[Ritesh Harjani]

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>

Re: [PATCH] powerpc/fadump: Add timeout to RTAS busy-wait loops

[Sourabh Jain]

Hello Adriano

On 06/04/26 11:45, Adriano Vero wrote:
> 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.

I agree that it is a good idea to avoid calling rtas_call for
fadump operations indefinitely. However, so far I have not come
across a case where the kernel gets stuck during fadump
registration, unregistration, or invalidation due to phyp/RTAS
continuously returning a wait time on an LPAR.

That said, since fadump support has recently been extended to
QEMU, this change might possibly prove useful in that environment.

>
> Track the accumulated delay in a total_wait counter and bail out with
> -ETIMEDOUT if it reaches RTAS_FADUMP_MAX_WAIT_MS (60 seconds)

What is the rationale behind choosing a 60-second limit?


> 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);

This do...while loop is almost identical in all three places.
Would it make sense to introduce a helper function to wrap the
rtas_call, along with handling the wait time and timeout?

- Sourabh Jain

>   	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;

Re: [PATCH] powerpc/fadump: Add timeout to RTAS busy-wait loops

[Sourabh Jain]

On 14/04/26 03:07, litaliano00 wrote:
> On Mon, Apr 13, 2026, 15: 50 Sourabh Jain 
> <sourabhjain@ linux. ibm. com> wrote: > What is the rationale behind 
> choosing a 60-second limit? The 60-second value was chosen somewhat 
> arbitrarily. Looking at rtas_busy_delay_time(), the maximum
> 
> On Mon, Apr 13, 2026, 15:50 Sourabh Jain <sourabhjain@linux.ibm.com> 
> wrote:
>
>     > What is the rationale behind choosing a 60-second limit? 
>
>
>     The 60-second value was chosen somewhat arbitrarily. Looking at
>
>     rtas_busy_delay_time(), the maximum single delay returned is 100000ms
>
>     (status 9905). In practice, legitimate busy delays are expected to be
>
>     much shorter. I was following the spirit of rtas-rtc.c which uses a
>
>     5-second limit for simpler operations; fadump is a heavier firmware
>
>     operation, so I went higher. I am open to suggestions on a more
>
>     principled value.
>


phyp/RTAS is a black box to me, so it is hard to conclude
what happens during fadump register/unregister/invalidate.
However, based on QEMU's fadump implementation, all three
operations do not seem very complex.

That said, I am OK with a 60 sec timeout.

I would also recommend adding pr_debug logs when the wait
time is hit. It might help debug cases where the kernel
reaches the 60 sec limit especially during boot.

- Sourabh Jain

>
>     > Would it make sense to introduce a helper function to wrap the
>
>     > rtas_call, along with handling the wait time and timeout?
>
>
>     Absolutely, I will introduce a helper in v2. The three sites are
>
>     indeed identical except for the FADUMP_REGISTER/UNREGISTER/INVALIDATE
>
>     argument and the struct pointer/size. I will factor out the common
>
>     busy-wait loop into a static helper.
>
>
>     Will send v2 shortly from my new email adri.vero.dev@gmail.com
>
>
>     Adriano
>
>

[PATCH v2] powerpc/fadump: Add timeout to RTAS busy-wait loops

[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.

Introduce rtas_fadump_call(), a helper that wraps the common
busy-wait pattern shared by all three sites. The helper accumulates
the total delay and returns -ETIMEDOUT if firmware keeps returning a
busy status beyond RTAS_FADUMP_MAX_WAIT_MS (60 seconds). A pr_debug()
message is emitted on each busy iteration to aid diagnosis when the
timeout is hit.

Signed-off-by: Adriano Vero <adri.vero.dev@gmail.com>
---
 arch/powerpc/platforms/pseries/rtas-fadump.c | 80 ++++++++++++--------
 arch/powerpc/platforms/pseries/rtas-fadump.h |  6 ++
 2 files changed, 53 insertions(+), 33 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/rtas-fadump.c b/arch/powerpc/platforms/pseries/rtas-fadump.c
index eceb32893..3bb4ac2ab 100644
--- a/arch/powerpc/platforms/pseries/rtas-fadump.c
+++ b/arch/powerpc/platforms/pseries/rtas-fadump.c
@@ -179,9 +179,42 @@ static u64 rtas_fadump_get_bootmem_min(void)
 	return RTAS_FADUMP_MIN_BOOT_MEM;
 }
 
+/*
+ * Helper to make an ibm,configure-kernel-dump RTAS call with a bounded
+ * busy-wait loop. Returns the RTAS return code on completion, or
+ * -ETIMEDOUT if firmware keeps returning a busy status beyond
+ * RTAS_FADUMP_MAX_WAIT_MS milliseconds.
+ */
+static int rtas_fadump_call(struct fw_dump *fadump_conf, int operation,
+			    void *fdm_ptr, unsigned int fdm_size,
+			    const char *op_name)
+{
+	unsigned int wait_time, total_wait = 0;
+	int rc;
+
+	do {
+		rc = rtas_call(fadump_conf->ibm_configure_kernel_dump, 3, 1,
+			       NULL, operation, fdm_ptr, fdm_size);
+		wait_time = rtas_busy_delay_time(rc);
+		if (wait_time) {
+			pr_debug("Firmware busy during fadump %s, waiting %ums (total %ums)\n",
+				 op_name, wait_time, total_wait);
+			if (total_wait >= RTAS_FADUMP_MAX_WAIT_MS) {
+				pr_err("Timed out waiting for firmware to complete fadump %s\n",
+				       op_name);
+				return -ETIMEDOUT;
+			}
+			total_wait += wait_time;
+			mdelay(wait_time);
+		}
+	} while (wait_time);
+
+	return rc;
+}
+
 static int rtas_fadump_register(struct fw_dump *fadump_conf)
 {
-	unsigned int wait_time, fdm_size;
+	unsigned int fdm_size;
 	int rc, err = -EIO;
 
 	/*
@@ -192,16 +225,10 @@ 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 */
-	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)
-			mdelay(wait_time);
-
-	} while (wait_time);
+	rc = rtas_fadump_call(fadump_conf, FADUMP_REGISTER, &fdm, fdm_size,
+			      "register");
+	if (rc == -ETIMEDOUT)
+		return -ETIMEDOUT;
 
 	switch (rc) {
 	case 0:
@@ -234,19 +261,12 @@ static int rtas_fadump_register(struct fw_dump *fadump_conf)
 
 static int rtas_fadump_unregister(struct fw_dump *fadump_conf)
 {
-	unsigned int wait_time;
 	int rc;
 
-	/* TODO: Add upper time limit for the delay */
-	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)
-			mdelay(wait_time);
-	} while (wait_time);
+	rc = rtas_fadump_call(fadump_conf, FADUMP_UNREGISTER, &fdm,
+			      sizeof(struct rtas_fadump_mem_struct), "unregister");
+	if (rc == -ETIMEDOUT)
+		return -ETIMEDOUT;
 
 	if (rc) {
 		pr_err("Failed to un-register - unexpected error(%d).\n", rc);
@@ -259,19 +279,13 @@ static int rtas_fadump_unregister(struct fw_dump *fadump_conf)
 
 static int rtas_fadump_invalidate(struct fw_dump *fadump_conf)
 {
-	unsigned int wait_time;
 	int rc;
 
-	/* TODO: Add upper time limit for the delay */
-	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)
-			mdelay(wait_time);
-	} while (wait_time);
+	rc = rtas_fadump_call(fadump_conf, FADUMP_INVALIDATE,
+			      (void *)fdm_active,
+			      sizeof(struct rtas_fadump_mem_struct), "invalidate");
+	if (rc == -ETIMEDOUT)
+		return -ETIMEDOUT;
 
 	if (rc) {
 		pr_err("Failed to invalidate - unexpected error (%d).\n", 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