* [PATCH] powerpc/fadump: Add timeout to RTAS busy-wait loops
@ 2026-04-06 6:15 Adriano Vero
2026-04-07 4:06 ` Ritesh Harjani
` (2 more replies)
0 siblings, 3 replies; 13+ 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] 13+ 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
2026-04-13 13:50 ` Sourabh Jain
2026-04-19 6:50 ` [PATCH v2] " Adriano Vero
2 siblings, 1 reply; 13+ 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] 13+ 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; 13+ 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] 13+ 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-13 13:50 ` Sourabh Jain
2026-04-13 21:37 ` litaliano00
2026-04-19 6:50 ` [PATCH v2] " Adriano Vero
2 siblings, 1 reply; 13+ messages in thread
From: Sourabh Jain @ 2026-04-13 13:50 UTC (permalink / raw)
To: Adriano Vero, maddy, mpe; +Cc: npiggin, chleroy, linuxppc-dev, linux-kernel
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;
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] powerpc/fadump: Add timeout to RTAS busy-wait loops
2026-04-13 13:50 ` Sourabh Jain
@ 2026-04-13 21:37 ` litaliano00
2026-04-14 10:29 ` Sourabh Jain
0 siblings, 1 reply; 13+ messages in thread
From: litaliano00 @ 2026-04-13 21:37 UTC (permalink / raw)
To: Sourabh Jain; +Cc: maddy, mpe, npiggin, chleroy, linuxppc-dev, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1025 bytes --]
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.
> > 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
[-- Attachment #2: Type: text/html, Size: 3699 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] powerpc/fadump: Add timeout to RTAS busy-wait loops
2026-04-13 21:37 ` litaliano00
@ 2026-04-14 10:29 ` Sourabh Jain
0 siblings, 0 replies; 13+ messages in thread
From: Sourabh Jain @ 2026-04-14 10:29 UTC (permalink / raw)
To: litaliano00; +Cc: maddy, mpe, npiggin, chleroy, linuxppc-dev, linux-kernel
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
>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2] 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-13 13:50 ` Sourabh Jain
@ 2026-04-19 6:50 ` Adriano Vero
2026-05-06 19:21 ` Sourabh Jain
2026-05-06 22:20 ` [PATCH v3] " Adriano Vero
2 siblings, 2 replies; 13+ messages in thread
From: Adriano Vero @ 2026-04-19 6:50 UTC (permalink / raw)
To: maddy, mpe, sourabhjain
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.
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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2] powerpc/fadump: Add timeout to RTAS busy-wait loops
2026-04-19 6:50 ` [PATCH v2] " Adriano Vero
@ 2026-05-06 19:21 ` Sourabh Jain
2026-05-07 2:52 ` Adriano Vero
2026-05-06 22:20 ` [PATCH v3] " Adriano Vero
1 sibling, 1 reply; 13+ messages in thread
From: Sourabh Jain @ 2026-05-06 19:21 UTC (permalink / raw)
To: Adriano Vero, maddy, mpe; +Cc: npiggin, chleroy, linuxppc-dev, linux-kernel
On 19/04/26 12:20, 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.
>
> 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;
Changes look good to me.
Feel free to add:
Reviewed-by: Sourabh Jain <sourabhjain@linux•ibm.com>
While testing this patch I realized that fadump prints below message
when registered is successfully
<rtas/opal> fadump: Registration is successful
But there is no message when fadump is unregistered successfully. I
think it is good to
log that event. I will send a separate patch for this.
- Sourabh Jain
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v3] powerpc/fadump: Add timeout to RTAS busy-wait loops
2026-04-19 6:50 ` [PATCH v2] " Adriano Vero
2026-05-06 19:21 ` Sourabh Jain
@ 2026-05-06 22:20 ` Adriano Vero
2026-05-07 4:15 ` Sourabh Jain
1 sibling, 1 reply; 13+ messages in thread
From: Adriano Vero @ 2026-05-06 22:20 UTC (permalink / raw)
To: maddy, mpe, sourabhjain
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.
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>
Reviewed-by: Sourabh Jain <sourabhjain@linux•ibm.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 eceb3289383e..3bb4ac2ab6cc 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 c109abf6befd..65fdab7b5b8d 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.54.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2] powerpc/fadump: Add timeout to RTAS busy-wait loops
2026-05-06 19:21 ` Sourabh Jain
@ 2026-05-07 2:52 ` Adriano Vero
2026-05-07 4:07 ` Sourabh Jain
0 siblings, 1 reply; 13+ messages in thread
From: Adriano Vero @ 2026-05-07 2:52 UTC (permalink / raw)
To: Sourabh Jain; +Cc: maddy, mpe, npiggin, chleroy, linuxppc-dev, linux-kernel
Hi Sourabh,
Thank you for the review. v3 with your Reviewed-by tag has been sent.
Adriano
On Wed, May 6, 2026 at 9:22 PM Sourabh Jain <sourabhjain@linux•ibm.com> wrote:
>
>
>
> On 19/04/26 12:20, 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.
> >
> > 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;
>
> Changes look good to me.
>
> Feel free to add:
> Reviewed-by: Sourabh Jain <sourabhjain@linux•ibm.com>
>
> While testing this patch I realized that fadump prints below message
> when registered is successfully
> <rtas/opal> fadump: Registration is successful
>
> But there is no message when fadump is unregistered successfully. I
> think it is good to
> log that event. I will send a separate patch for this.
>
> - Sourabh Jain
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] powerpc/fadump: Add timeout to RTAS busy-wait loops
2026-05-07 2:52 ` Adriano Vero
@ 2026-05-07 4:07 ` Sourabh Jain
0 siblings, 0 replies; 13+ messages in thread
From: Sourabh Jain @ 2026-05-07 4:07 UTC (permalink / raw)
To: Adriano Vero; +Cc: maddy, mpe, npiggin, chleroy, linuxppc-dev, linux-kernel
On 07/05/26 08:22, Adriano Vero wrote:
> Hi Sourabh,
>
> Thank you for the review. v3 with your Reviewed-by tag has been sent.
Thanks, but a new version just to include tags is not really needed.
Maintainers generally take care of including them.
- Sourabh Jain
>
> Adriano
>
>
> On Wed, May 6, 2026 at 9:22 PM Sourabh Jain <sourabhjain@linux•ibm.com> wrote:
>>
>>
>> On 19/04/26 12:20, 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.
>>>
>>> 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;
>> Changes look good to me.
>>
>> Feel free to add:
>> Reviewed-by: Sourabh Jain <sourabhjain@linux•ibm.com>
>>
>> While testing this patch I realized that fadump prints below message
>> when registered is successfully
>> <rtas/opal> fadump: Registration is successful
>>
>> But there is no message when fadump is unregistered successfully. I
>> think it is good to
>> log that event. I will send a separate patch for this.
>>
>> - Sourabh Jain
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3] powerpc/fadump: Add timeout to RTAS busy-wait loops
2026-05-06 22:20 ` [PATCH v3] " Adriano Vero
@ 2026-05-07 4:15 ` Sourabh Jain
2026-05-07 8:08 ` Adriano Vero
0 siblings, 1 reply; 13+ messages in thread
From: Sourabh Jain @ 2026-05-07 4:15 UTC (permalink / raw)
To: Adriano Vero, maddy, mpe; +Cc: npiggin, chleroy, linuxppc-dev, linux-kernel
Hello Adriano,
On 07/05/26 03:50, 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.
>
> 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>
>
> Reviewed-by: Sourabh Jain <sourabhjain@linux•ibm.com>
New line between tags is unnecessary.
Including a Changelog in subsequent versions of the same patch is
helpful for the maintainer and reviewers to understand what has changed
in this version compared to the previous version.
For example:
https://lore.kernel.org/all/20260407124349.1698552-1-sourabhjain@linux.ibm.com/
Thanks,
Sourabh Jain
> ---
> 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 eceb3289383e..3bb4ac2ab6cc 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 c109abf6befd..65fdab7b5b8d 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;
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3] powerpc/fadump: Add timeout to RTAS busy-wait loops
2026-05-07 4:15 ` Sourabh Jain
@ 2026-05-07 8:08 ` Adriano Vero
0 siblings, 0 replies; 13+ messages in thread
From: Adriano Vero @ 2026-05-07 8:08 UTC (permalink / raw)
To: Sourabh Jain; +Cc: maddy, mpe, npiggin, chleroy, linuxppc-dev, linux-kernel
Hi Sourabh,
Understood, I'll keep that in mind for future patches. Thanks for
the guidance.
Adriano
On Thu, May 7, 2026 at 6:16 AM Sourabh Jain <sourabhjain@linux•ibm.com> wrote:
>
> Hello Adriano,
>
> On 07/05/26 03:50, 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.
> >
> > 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>
> >
> > Reviewed-by: Sourabh Jain <sourabhjain@linux•ibm.com>
>
> New line between tags is unnecessary.
>
> Including a Changelog in subsequent versions of the same patch is
> helpful for the maintainer and reviewers to understand what has changed
> in this version compared to the previous version.
>
> For example:
> https://lore.kernel.org/all/20260407124349.1698552-1-sourabhjain@linux.ibm.com/
>
> Thanks,
> Sourabh Jain
>
> > ---
> > 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 eceb3289383e..3bb4ac2ab6cc 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 c109abf6befd..65fdab7b5b8d 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;
>
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2026-05-07 8:08 UTC | newest]
Thread overview: 13+ 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
2026-04-13 13:50 ` Sourabh Jain
2026-04-13 21:37 ` litaliano00
2026-04-14 10:29 ` Sourabh Jain
2026-04-19 6:50 ` [PATCH v2] " Adriano Vero
2026-05-06 19:21 ` Sourabh Jain
2026-05-07 2:52 ` Adriano Vero
2026-05-07 4:07 ` Sourabh Jain
2026-05-06 22:20 ` [PATCH v3] " Adriano Vero
2026-05-07 4:15 ` Sourabh Jain
2026-05-07 8:08 ` Adriano Vero
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox