Re: kern/56568: ipmi.c 1.7 causes large boot delays

[Taylor R Campbell <campbell+netbsd-tech-kern%mumble.net@localhost>]

The attached patch -- compile-tested only -- tries to resolve the
present conflict between long ipmi(4) initialization time and early
use of wdogctl(8) at boot by:

(a) allowing boot to proceed with config_pending_decr sooner, before
    device initialization has completed; and

(b) making watchdog configuration wait until initialization has
    completed.

Testing welcome!

# HG changeset patch
# User Taylor R Campbell <riastradh%NetBSD.org@localhost>
# Date 1733184792 0
#      Tue Dec 03 00:13:12 2024 +0000
# Branch trunk
# Node ID 32dee81c8e034a77811fddb38816c166f46bb530
# Parent  9e743b738fb8d4ce4e33ddccdc9acb6e8e81f9df
# EXP-Topic riastradh-pr56568-ipmidelay
ipmi(4): Don't hold up boot -- just hold up wdogctl(8).

While here, omit needless volatile qualifier on variables serialized
by mutex (or by config_pending_incr/decr -- ipmi_detach can't run
until ipmi_thread does config_pending_decr).

PR kern/56568: ipmi.c 1.7 causes large boot delays

diff -r 9e743b738fb8 -r 32dee81c8e03 sys/dev/ipmi.c
--- a/sys/dev/ipmi.c	Sun Dec 01 11:10:34 2024 +0000
+++ b/sys/dev/ipmi.c	Tue Dec 03 00:13:12 2024 +0000
@@ -1998,6 +1998,23 @@ ipmi_thread(void *cookie)
 	/* Map registers */
 	ipmi_map_regs(sc, ia);
 
+	/* Setup Watchdog timer */
+	sc->sc_wdog.smw_name = device_xname(sc->sc_dev);
+	sc->sc_wdog.smw_cookie = sc;
+	sc->sc_wdog.smw_setmode = ipmi_watchdog_setmode;
+	sc->sc_wdog.smw_tickle = ipmi_watchdog_tickle;
+	sysmon_wdog_register(&sc->sc_wdog);
+
+	/* Set up a power handler so we can possibly sleep */
+	if (!pmf_device_register(self, ipmi_suspend, NULL))
+                aprint_error_dev(self, "couldn't establish a power handler\n");
+
+	/*
+	 * Allow boot to proceed -- we'll do the rest asynchronously
+	 * since it requires talking to the device.
+	 */
+	config_pending_decr(self);
+
 	memset(&id, 0, sizeof(id));
 	if (ipmi_get_device_id(sc, &id))
 		aprint_error_dev(self, "Failed to re-query device ID\n");
@@ -2098,20 +2115,9 @@ ipmi_thread(void *cookie)
 	/* setup flag to exclude iic */
 	ipmi_enabled = 1;
 
-	/* Setup Watchdog timer */
-	sc->sc_wdog.smw_name = device_xname(sc->sc_dev);
-	sc->sc_wdog.smw_cookie = sc;
-	sc->sc_wdog.smw_setmode = ipmi_watchdog_setmode;
-	sc->sc_wdog.smw_tickle = ipmi_watchdog_tickle;
-	sysmon_wdog_register(&sc->sc_wdog);
-
-	/* Set up a power handler so we can possibly sleep */
-	if (!pmf_device_register(self, ipmi_suspend, NULL))
-                aprint_error_dev(self, "couldn't establish a power handler\n");
-
-	config_pending_decr(self);
-
 	mutex_enter(&sc->sc_poll_mtx);
+	sc->sc_thread_ready = true;
+	cv_broadcast(&sc->sc_mode_cv);
 	while (sc->sc_thread_running) {
 		while (sc->sc_mode == IPMI_MODE_COMMAND)
 			cv_wait(&sc->sc_mode_cv, &sc->sc_poll_mtx);
@@ -2259,6 +2265,15 @@ ipmi_watchdog_setmode(struct sysmon_wdog
 	else
 		sc->sc_wdog.smw_period = smwdog->smw_period;
 
+	/* Wait until the device is initialized */
+	rc = 0;
+	mutex_enter(&sc->sc_poll_mtx);
+	while (sc->sc_thread_ready)
+		rc = cv_wait_sig(&sc->sc_mode_cv, &sc->sc_poll_mtx);
+	mutex_exit(&sc->sc_poll_mtx);
+	if (rc)
+		return rc;
+
 	mutex_enter(&sc->sc_cmd_mtx);
 	/* see if we can properly task to the watchdog */
 	rc = ipmi_sendcmd(sc, BMC_SA, BMC_LUN, APP_NETFN,
diff -r 9e743b738fb8 -r 32dee81c8e03 sys/dev/ipmivar.h
--- a/sys/dev/ipmivar.h	Sun Dec 01 11:10:34 2024 +0000
+++ b/sys/dev/ipmivar.h	Tue Dec 03 00:13:12 2024 +0000
@@ -101,8 +101,9 @@ struct ipmi_softc {
 	struct ipmi_bmc_args	*sc_iowait_args;
 
 	struct ipmi_sensor	*current_sensor;
-	volatile bool		sc_thread_running;
-	volatile bool		sc_tickle_due;
+	bool			sc_thread_running;
+	bool			sc_thread_ready;
+	bool			sc_tickle_due;
 	struct sysmon_wdog	sc_wdog;
 	struct sysmon_envsys	*sc_envsys;
 	envsys_data_t		*sc_sensor;