From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f41.google.com (mail-wr1-f41.google.com [209.85.221.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 1CEFB284B4F for ; Thu, 7 May 2026 02:52:34 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=pass smtp.client-ip=209.85.221.41 ARC-Seal:i=2; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1778122356; cv=pass; b=tdbrzkzkRBfxfm0A03onJW9dN6xWj71ZRYk9RMSSmfDPdQauxwVbWq+XCHvT2uygsBZuGUKyw9psnsx0Hs++G06E+88S8+eOeJdLF21agoTkLolx5DuxvpyvUAT+Cjy5QzZ7MvSY+PrsDeTerw9TLZQBRL/XvHBMJsINb0LCGHQ= ARC-Message-Signature:i=2; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1778122356; c=relaxed/simple; bh=d247ReGvQHtcweUJELlG6mbExVWOLrAsHJv7CTMGcWs=; h=MIME-Version:References:In-Reply-To:From:Date:Message-ID:Subject: To:Cc:Content-Type; b=Oo7T8EqfbiqhaOWLmXBKR9fgIw/cThZ7DfCJnFjhh+z5E479TZfd3yJlv0/GVGX77I2IhgH3UldNEQu5eLfOwMGLlrhMFLnaA2FMoxMfMAOuCyhYflEbv8kdY17Q+6XO/JtU0sPZFKvgTZBNU3I/laaOQy8gS2ITTd48HXG+9vs= ARC-Authentication-Results:i=2; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=MtZxmJVo; arc=pass smtp.client-ip=209.85.221.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="MtZxmJVo" Received: by mail-wr1-f41.google.com with SMTP id ffacd0b85a97d-43d76dd4ee8so285472f8f.2 for ; Wed, 06 May 2026 19:52:34 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1778122353; cv=none; d=google.com; s=arc-20240605; b=IAgWuVflMH28a9vAmD5GIcRCLMDQavenYawxQleU26sVPuT0fgtJS4tbwhFcU1XnHX cG5xbdCGAR6kxT0accqlfuQrDQWSluzuHXebRTBguilcWNHLQzqi5MaE1MwNu6Etyerm eq1hM2lu1wltCbDB5YTsDd99Us7HjMzYJ10yOim0K24Ta2RqItBmYgqEnW5apTF58efk Bsq30Q/WeuIeC1WtZUUbTPtqrmBTNxeHB6nro+A5oLYnFYM7qbbfI0DnTvNSrZlves8Q pWBR5dlYoolqD/IPuHIHSN8mOXxYSo+FCKeHAudGzSXPzLtj1dsJYTnLRJojk82dK6xR GCiA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20240605; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:dkim-signature; bh=uW6UXusY5B7wnfgwtpuv3i16n+MFEhNRrfK3KAuOPZ0=; fh=pAcbRj5FcnjV/vgZYNexXabNQLDBP6Tx6oPuKHIQ8FY=; b=gscQGMM7m/gTd9Zcvw3WNbH3Hx7BATpHCuV38AehXQc4iZllHOrMK1iB6HpFDYinFq M/5YBy2sQ9l/PhgIc8nGq2ARzZhgjOlPUfyGF6saISu2/6K6On39IyXMP3DQOLpd9TTP TMqK4ikAEOJSfZW/hqoCNsAAjFuO8y6o3Otfzr0yuZDPuKLSYk+tcctcl2fRNG8gW151 09dve4BdAsQQuCIEJnzQoH1neQDUntci9Nl/xTzjOSTSnMPmR32EhcSr73WtGky78fBv 4eo0sa6HgltHqF4kBheee+0TPKJ5FKPW4roU9BIYQKDqcY/bUb93Q25GhmLB9Jit74Gn dzPQ==; darn=vger.kernel.org ARC-Authentication-Results: i=1; mx.google.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1778122353; x=1778727153; darn=vger.kernel.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=uW6UXusY5B7wnfgwtpuv3i16n+MFEhNRrfK3KAuOPZ0=; b=MtZxmJVox9am/81bseNEMgX8eHfzXc+n/zHuB5vjb194FI/nnN0xCeXJUWHQPWXuVm pCqgDU4ITHUc2bwTdvNcREg1qdYpqJ37b6bNciIIxTMGN2Ni8fuaD78gqkieJ/Wf6+KX 15v3sHk8GXEYOQQxupIs7zkJDfuHy8n/wqXCX1ZLM+KPaEEgHb/tLxTE+awM5V3WxZEP WMKEo1PkFs/y5QOsbkN+DR9Bja2cZhkaqLC28k/ozzEcso5gLB+jk/L56bpIBlwYeYHS qkQMn8hXZ82m/lCykJ0iRbnneDEL3kstcDLHeSMQSQoAP/LG+TbUyf3Zjl8B7vWrw5hG dfBA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1778122353; x=1778727153; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=uW6UXusY5B7wnfgwtpuv3i16n+MFEhNRrfK3KAuOPZ0=; b=gqS/dK4vle378A193GOC1SPcW+eyTvyxQOhMgpSF6b+hDccQEDX47nKKiL2LQzv/ox z4RcrRGOMCSwRwQ2aNuG5oQ/r2NsI08sZ0gWf4GXCeJAOkPINdVn3SRAopaH+onhMCIQ YbmuI6VjNXCqogTGCBJ6/nH5Fl1MxOnc17spQ5RTkYSf6TsSJg8iIJonIAsRqW7itlFX f7HX3gUQPjJbJrd6vxRFURxUT6s4Mo/8skkJhXktcCGo0OiWn/9fYG2oNnVMMlCwnRiq xsk+gNB+2dRegPAGEXxGKBcw7JURs7ElLiQY38JyuRXGievI+Mhq3xXAK8BCRvHiM1cP QlAA== X-Forwarded-Encrypted: i=1; AFNElJ+9z4anTo8xKS/jQkeUyBEtLOd+g//fHm4JcGaKLAHmgh4C4xlCqTWjqrTz7Fb1qI9HWiRW5dfCTpSQiEs=@vger.kernel.org X-Gm-Message-State: AOJu0YyrK1ipEqnq3x++XM5ENEOlyANQ5TBQN1zVq3nTsB1/YtAkzjj8 OOuCOo3uuBtdzSXqlj5qtUWgLAiYLpLWRiSrUoMQpmkz5MKg0yk7yG/RERoEIK1e1JQ3inrVDaL ssOV8plXe+ppBNToTtVskGs1ME5iEfmo= X-Gm-Gg: AeBDieuBB1WvqlUFyVqBKAKjhLIZkzTyQX6Lekg0j2cA2bDrolwHFC/Nc0l+wMGrvrz 4hesOoHedf5htVOphgJWWNZQhd69BsgBVemyOVCPTGqy/lae+b1za37O6PGH944p6CwNCHOLAi5 EMFzO6+hp7c6rBhCGvMocVvrwA+ek6krY+fDhadlPTHtLTx7U/ARZAloCXH+SIuHzWzP8k29WUT FwR+Fwdztd/djVOwGE21cek8FLEpS1wIYkpSNQgZg7gCOpTsKFU23JoAthOTL5XxZwlLPsJM28i f3TdLtYTIXMHcMkg3NKe8g3sEfE= X-Received: by 2002:a05:6000:2310:b0:43d:773d:7908 with SMTP id ffacd0b85a97d-4515d3dc438mr9631396f8f.32.1778122353253; Wed, 06 May 2026 19:52:33 -0700 (PDT) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 References: <20260406061542.22354-1-litaliano00.contact@gmail.com> <20260419065039.23495-1-adri.vero.dev@gmail.com> <7bef2ce2-1fd3-4648-8c78-d3c5e07a1f9f@linux.ibm.com> In-Reply-To: <7bef2ce2-1fd3-4648-8c78-d3c5e07a1f9f@linux.ibm.com> From: Adriano Vero Date: Thu, 7 May 2026 04:52:21 +0200 X-Gm-Features: AVHnY4LgTIvR1AI6EIc-T4r21ztuWBHZNGBeQ7vGohG8sS8W5mspgVlodsa6JrM Message-ID: Subject: Re: [PATCH v2] powerpc/fadump: Add timeout to RTAS busy-wait loops To: Sourabh Jain Cc: maddy@linux.ibm.com, mpe@ellerman.id.au, npiggin@gmail.com, chleroy@kernel.org, linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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=E2=80=AFPM Sourabh Jain 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 > > --- > > 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/powerp= c/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 bounde= d > > + * 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 =3D 0; > > + int rc; > > + > > + do { > > + rc =3D rtas_call(fadump_conf->ibm_configure_kernel_dump, = 3, 1, > > + NULL, operation, fdm_ptr, fdm_size); > > + wait_time =3D 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 >=3D RTAS_FADUMP_MAX_WAIT_MS) { > > + pr_err("Timed out waiting for firmware to= complete fadump %s\n", > > + op_name); > > + return -ETIMEDOUT; > > + } > > + total_wait +=3D 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 =3D -EIO; > > > > /* > > @@ -192,16 +225,10 @@ static int rtas_fadump_register(struct fw_dump *f= adump_conf) > > fdm_size =3D sizeof(struct rtas_fadump_section_header); > > fdm_size +=3D be16_to_cpu(fdm.header.dump_num_sections) * sizeof(= struct rtas_fadump_section); > > > > - /* TODO: Add upper time limit for the delay */ > > - do { > > - rc =3D rtas_call(fadump_conf->ibm_configure_kernel_dump,= 3, 1, > > - NULL, FADUMP_REGISTER, &fdm, fdm_size); > > - > > - wait_time =3D rtas_busy_delay_time(rc); > > - if (wait_time) > > - mdelay(wait_time); > > - > > - } while (wait_time); > > + rc =3D rtas_fadump_call(fadump_conf, FADUMP_REGISTER, &fdm, fdm_s= ize, > > + "register"); > > + if (rc =3D=3D -ETIMEDOUT) > > + return -ETIMEDOUT; > > > > switch (rc) { > > case 0: > > @@ -234,19 +261,12 @@ static int rtas_fadump_register(struct fw_dump *f= adump_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 =3D rtas_call(fadump_conf->ibm_configure_kernel_dump,= 3, 1, > > - NULL, FADUMP_UNREGISTER, &fdm, > > - sizeof(struct rtas_fadump_mem_struct)); > > - > > - wait_time =3D rtas_busy_delay_time(rc); > > - if (wait_time) > > - mdelay(wait_time); > > - } while (wait_time); > > + rc =3D rtas_fadump_call(fadump_conf, FADUMP_UNREGISTER, &fdm, > > + sizeof(struct rtas_fadump_mem_struct), "unr= egister"); > > + if (rc =3D=3D -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 =3D rtas_call(fadump_conf->ibm_configure_kernel_dump,= 3, 1, > > - NULL, FADUMP_INVALIDATE, fdm_active, > > - sizeof(struct rtas_fadump_mem_struct)); > > - > > - wait_time =3D rtas_busy_delay_time(rc); > > - if (wait_time) > > - mdelay(wait_time); > > - } while (wait_time); > > + rc =3D rtas_fadump_call(fadump_conf, FADUMP_INVALIDATE, > > + (void *)fdm_active, > > + sizeof(struct rtas_fadump_mem_struct), "inv= alidate"); > > + if (rc =3D=3D -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/powerp= c/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 > > While testing this patch I realized that fadump prints below message > when registered is successfully > 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