From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id BF8E93A1CE6 for ; Tue, 7 Apr 2026 17:51:40 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=170.10.129.124 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1775584304; cv=none; b=ddYsnIu6ZO49QV4l93VOOGNm67HM8GuXpTzkBMYfrOrcbqMj8HF1MBhciNFwpFj6FjYZZul7zUuAEnFXAXbiZwF21MiZhpu/9MVBBeQ16Mjs+Sypr/suujuxak1l9k4F4nYlcZVQ3TWr0hhIbK+jh3MO8BSpasyngM3zm7D58mo= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1775584304; c=relaxed/simple; bh=/qRoqUVQqab/1Zqf1oeH5uiG0CWHcYi/NOvtmpDkuc0=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=CMlBwKqm3BUNBeq6aZazO8yJMYWZBmJAddp52QXt7r8SMPnhhQgsg8+yDOuJV2ZUaIfJNGrLWbEhWXGNbhaHxaI/Zsh49s6OKkZRXnbG8PViOOBr+zDJhRQE649/9ZCq1jDrEEhVDYZE9pfocEuPNitXgdL0QLJ54pfXDwWzFAY= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com; spf=pass smtp.mailfrom=redhat.com; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b=TWt6meEp; arc=none smtp.client-ip=170.10.129.124 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="TWt6meEp" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1775584299; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=f+twoVwdNXfHf13CedAqdMJEXuMqs/2V95Av5J0U1Bg=; b=TWt6meEpPpxXcTkyal0Eg0TODH420ceU832dlBEhkr42v1d7DFdx4HI4VFeGSR7+Mw6EKT A2IPaYEVpIBSSDF1jLL6xEPVFwFxwXyQazx3i8ls79frFMZGgo8+2fAzEQxk1kbEiIARlh CpdTOp+q8yKjrViyqXB+OZVvlY0pRv8= Received: from mx-prod-mc-03.mail-002.prod.us-west-2.aws.redhat.com (ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-657-4ErzDG65Ob2CKKvEEJLIHw-1; Tue, 07 Apr 2026 13:51:37 -0400 X-MC-Unique: 4ErzDG65Ob2CKKvEEJLIHw-1 X-Mimecast-MFC-AGG-ID: 4ErzDG65Ob2CKKvEEJLIHw_1775584297 Received: from mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.4]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-03.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id E15CF1956059; Tue, 7 Apr 2026 17:51:36 +0000 (UTC) Received: from hp-dl380pgen9-07.khw.eng.rdu2.dc.redhat.com (hp-dl380pgen9-07.khw.eng.rdu2.dc.redhat.com [10.6.10.143]) by mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id 50F55300019F; Tue, 7 Apr 2026 17:51:36 +0000 (UTC) From: Tony Camuso To: openipmi-developer@lists.sourceforge.net, linux-kernel@vger.kernel.org Cc: minyard@acm.org, tcamuso@redhat.com Subject: [PATCH 1/2] ipmi:watchdog: Reboot cleanly on BMC reset Date: Tue, 7 Apr 2026 13:51:33 -0400 Message-ID: <20260407175134.3367345-2-tcamuso@redhat.com> In-Reply-To: <20260407175134.3367345-1-tcamuso@redhat.com> References: <20260407175134.3367345-1-tcamuso@redhat.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Scanned-By: MIMEDefang 3.4.1 on 10.30.177.4 When the BMC resets while the IPMI watchdog is active, three problems can occur: 1. The static smi_msg and recv_msg structures remain queued in the IPMI layer after a response timeout. If the watchdog daemon retries, the code reuses these structures while still on the IPMI layer's internal lists, causing: list_add double add: new=ffffffffc10063e0, prev=ffffffffc10063e0, ... kernel BUG at lib/list_debug.c:29! 2. Both __ipmi_heartbeat() and _ipmi_set_timeout() use wait_for_completion() with no timeout, blocking indefinitely if the BMC is unresponsive, leaving tasks stuck in D state. 3. When the BMC loses the watchdog timer state, the driver's internal state becomes inconsistent, causing subsequent writes to /dev/watchdog to return -EINVAL, leaving the system without watchdog protection. Fix all three issues: - Add msg_in_flight atomic flag to prevent re-entry into __ipmi_heartbeat() and _ipmi_set_timeout() while message structures are still queued in the IPMI layer. - Convert wait_for_completion() to wait_for_completion_timeout() in both functions to prevent indefinite blocking. - Add reinit_completion() before each use to prevent stale completion events from allowing premature wakeup. - Detect BMC communication failure in ipmi_wdog_msg_handler() via non-zero completion codes and initiate orderly_reboot() when the watchdog is active. This ensures the system reboots cleanly rather than being left without watchdog protection. Error classification distinguishes TIMER_NOT_INIT (0x80), vendor-specific codes (0x81-0xBE), and standard IPMI completion codes. - Guard all BMC communication paths (_ipmi_set_timeout, __ipmi_heartbeat, wdog_reboot_handler) with bmc_reset_shutdown flag to prevent further IPMI operations during shutdown. Signed-off-by: Tony Camuso --- drivers/char/ipmi/ipmi_watchdog.c | 101 ++++++++++++++++++++++++------ 1 file changed, 83 insertions(+), 18 deletions(-) diff --git a/drivers/char/ipmi/ipmi_watchdog.c b/drivers/char/ipmi/ipmi_watchdog.c index a013ddbf1466..1d8277cbe598 100644 --- a/drivers/char/ipmi/ipmi_watchdog.c +++ b/drivers/char/ipmi/ipmi_watchdog.c @@ -123,6 +123,16 @@ #define IPMI_WDOG_TIMER_NOT_INIT_RESP 0x80 +/* Timeout for waiting for a heartbeat response (in jiffies). */ +#define IPMI_HEARTBEAT_WAIT_TIMEOUT (HZ * 5) + +/* + * Set when the BMC becomes unreachable while the watchdog is active. + * Once set, all BMC communication is skipped and an orderly reboot + * is in progress. + */ +static bool bmc_reset_shutdown; + static DEFINE_MUTEX(ipmi_watchdog_mutex); static bool nowayout = WATCHDOG_NOWAYOUT; @@ -339,12 +349,14 @@ static int __ipmi_heartbeat(void); * and freed when both the send and receive messages are free. */ static atomic_t msg_tofree = ATOMIC_INIT(0); +static atomic_t msg_in_flight = ATOMIC_INIT(0); static DECLARE_COMPLETION(msg_wait); static void msg_free_smi(struct ipmi_smi_msg *msg) { if (atomic_dec_and_test(&msg_tofree)) { if (!oops_in_progress) complete(&msg_wait); + atomic_set(&msg_in_flight, 0); } } static void msg_free_recv(struct ipmi_recv_msg *msg) @@ -352,6 +364,7 @@ static void msg_free_recv(struct ipmi_recv_msg *msg) if (atomic_dec_and_test(&msg_tofree)) { if (!oops_in_progress) complete(&msg_wait); + atomic_set(&msg_in_flight, 0); } } static struct ipmi_smi_msg smi_msg = INIT_IPMI_SMI_MSG(msg_free_smi); @@ -429,19 +442,34 @@ static int _ipmi_set_timeout(int do_heartbeat) { int send_heartbeat_now; int rv; + unsigned long ret; if (!watchdog_user) return -ENODEV; + if (bmc_reset_shutdown) + return -ENODEV; + + if (atomic_read(&msg_in_flight)) + return -EBUSY; + + reinit_completion(&msg_wait); + atomic_set(&msg_in_flight, 1); atomic_set(&msg_tofree, 2); rv = __ipmi_set_timeout(&smi_msg, &recv_msg, &send_heartbeat_now); if (rv) { atomic_set(&msg_tofree, 0); + atomic_set(&msg_in_flight, 0); return rv; } - wait_for_completion(&msg_wait); + ret = wait_for_completion_timeout(&msg_wait, + IPMI_HEARTBEAT_WAIT_TIMEOUT); + if (ret == 0) { + atomic_set(&msg_tofree, 0); + return -ETIMEDOUT; + } if ((do_heartbeat == IPMI_SET_TIMEOUT_FORCE_HB) || ((send_heartbeat_now) @@ -510,10 +538,17 @@ static int __ipmi_heartbeat(void) { struct kernel_ipmi_msg msg; int rv; + unsigned long ret; struct ipmi_system_interface_addr addr; int timeout_retries = 0; restart: + if (bmc_reset_shutdown) + return -ENODEV; + + if (atomic_read(&msg_in_flight)) + return -EBUSY; + /* * Don't reset the timer if we have the timer turned off, that * re-enables the watchdog. @@ -521,6 +556,8 @@ static int __ipmi_heartbeat(void) if (ipmi_watchdog_state == WDOG_TIMEOUT_NONE) return 0; + reinit_completion(&msg_wait); + atomic_set(&msg_in_flight, 1); atomic_set(&msg_tofree, 2); addr.addr_type = IPMI_SYSTEM_INTERFACE_ADDR_TYPE; @@ -541,14 +578,17 @@ static int __ipmi_heartbeat(void) 1); if (rv) { atomic_set(&msg_tofree, 0); - pr_warn("heartbeat send failure: %d\n", rv); + atomic_set(&msg_in_flight, 0); return rv; } - /* Wait for the heartbeat to be sent. */ - wait_for_completion(&msg_wait); + ret = wait_for_completion_timeout(&msg_wait, IPMI_HEARTBEAT_WAIT_TIMEOUT); + if (ret == 0) { + atomic_set(&msg_tofree, 0); + return -ETIMEDOUT; + } - if (recv_msg.msg.data[0] == IPMI_WDOG_TIMER_NOT_INIT_RESP) { + if (recv_msg.msg.data[0] >= 0x80) { timeout_retries++; if (timeout_retries > 3) { pr_err("Unable to restore the IPMI watchdog's settings, giving up\n"); @@ -557,12 +597,11 @@ static int __ipmi_heartbeat(void) } /* - * The timer was not initialized, that means the BMC was - * probably reset and lost the watchdog information. Attempt - * to restore the timer's info. Note that we still hold - * the heartbeat lock, to keep a heartbeat from happening - * in this process, so must say no heartbeat to avoid a - * deadlock on this mutex + * The BMC was probably reset and lost the watchdog + * information. Attempt to restore the timer's info. + * Note that we still hold the heartbeat lock, to keep + * a heartbeat from happening in this process, so must + * say no heartbeat to avoid a deadlock on this mutex. */ rv = _ipmi_set_timeout(IPMI_SET_TIMEOUT_NO_HB); if (rv) { @@ -876,15 +915,38 @@ static struct miscdevice ipmi_wdog_miscdev = { static void ipmi_wdog_msg_handler(struct ipmi_recv_msg *msg, void *handler_data) { - if (msg->msg.cmd == IPMI_WDOG_RESET_TIMER && - msg->msg.data[0] == IPMI_WDOG_TIMER_NOT_INIT_RESP) - pr_info("response: The IPMI controller appears to have been reset, will attempt to reinitialize the watchdog timer\n"); - else if (msg->msg.data[0] != 0) - pr_err("response: Error %x on cmd %x\n", - msg->msg.data[0], - msg->msg.cmd); + if (msg->msg.data[0] != 0) { + if (msg->msg.data[0] == IPMI_WDOG_TIMER_NOT_INIT_RESP) + pr_crit("BMC error: watchdog timer not initialized " + "(0x%02x on cmd 0x%02x)\n", + msg->msg.data[0], msg->msg.cmd); + else if (msg->msg.data[0] > 0x80 && + msg->msg.data[0] <= 0xBE) + pr_crit("BMC error: vendor-specific completion code " + "0x%02x on cmd 0x%02x\n", + msg->msg.data[0], msg->msg.cmd); + else + pr_crit("BMC error: completion code 0x%02x " + "on cmd 0x%02x\n", + msg->msg.data[0], msg->msg.cmd); + + if (ipmi_watchdog_state != WDOG_TIMEOUT_NONE && + !bmc_reset_shutdown) { + bmc_reset_shutdown = true; + pr_crit("BMC communication lost with watchdog active, " + "initiating system reboot\n"); + orderly_reboot(); + } + } ipmi_free_recv_msg(msg); + /* + * Ensure the in-flight flag is cleared after the message is freed. + * In the normal path this is redundant (already cleared by the + * recv_msg destructor). For late responses arriving after a + * completion timeout, this is the only path that clears the flag. + */ + atomic_set(&msg_in_flight, 0); } static void ipmi_wdog_pretimeout_handler(void *handler_data) @@ -1106,6 +1168,9 @@ static int wdog_reboot_handler(struct notifier_block *this, /* Make sure we only do this once. */ reboot_event_handled = 1; + if (bmc_reset_shutdown) + return NOTIFY_OK; + if (code == SYS_POWER_OFF || code == SYS_HALT) { /* Disable the WDT if we are shutting down. */ ipmi_watchdog_state = WDOG_TIMEOUT_NONE; -- 2.53.0