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 <[email protected]> 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 <[email protected]> > > > > Reviewed-by: Sourabh Jain <[email protected]> > > 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/[email protected]/ > > 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; >
