git: 366d6a424e1f - main - ipmi: remove timeout from the ipmi_driver_request method

From: Gleb Smirnoff <glebius_at_FreeBSD.org>
Date: Fri, 18 Oct 2024 19:30:29 UTC
The branch main has been updated by glebius:

URL: https://cgit.FreeBSD.org/src/commit/?id=366d6a424e1faad9c151c5794978e218a79fbed8

commit 366d6a424e1faad9c151c5794978e218a79fbed8
Author:     Gleb Smirnoff <glebius@FreeBSD.org>
AuthorDate: 2024-10-18 19:30:09 +0000
Commit:     Gleb Smirnoff <glebius@FreeBSD.org>
CommitDate: 2024-10-18 19:30:09 +0000

    ipmi: remove timeout from the ipmi_driver_request method
    
    Driver requests are done with stack allocated request.  The request is
    put on the tailq and then we msleep(9) until kernel process processes it.
    If we timeout from this sleep, the kernel process may still read the
    request from our stack, which may already be reused by some other code.
    
    Make this sleep unbound and rely on the kernel process that does all its
    I/O with timouts and will eventually wake us up.
    
    Reviewed by:    jhb
    Differential Revision:  https://reviews.freebsd.org/D47179
---
 sys/dev/ipmi/ipmi.c      | 21 ++++++++++-----------
 sys/dev/ipmi/ipmi_bt.c   |  6 +++---
 sys/dev/ipmi/ipmi_kcs.c  |  8 ++++----
 sys/dev/ipmi/ipmi_opal.c | 10 ++++------
 sys/dev/ipmi/ipmi_smic.c |  2 +-
 sys/dev/ipmi/ipmi_ssif.c |  5 ++---
 sys/dev/ipmi/ipmivars.h  |  5 ++---
 7 files changed, 26 insertions(+), 31 deletions(-)

diff --git a/sys/dev/ipmi/ipmi.c b/sys/dev/ipmi/ipmi.c
index efb8a7e7669b..5f759017441c 100644
--- a/sys/dev/ipmi/ipmi.c
+++ b/sys/dev/ipmi/ipmi.c
@@ -360,7 +360,7 @@ ipmi_ioctl(struct cdev *cdev, u_long cmd, caddr_t data,
 		kreq->ir_request[req->msg.data_len + 7] =
 		    ipmi_ipmb_checksum(&kreq->ir_request[4],
 		    req->msg.data_len + 3);
-		error = ipmi_submit_driver_request(sc, kreq, MAX_TIMEOUT);
+		error = ipmi_submit_driver_request(sc, kreq);
 		if (error != 0)
 			return (error);
 
@@ -565,11 +565,10 @@ ipmi_complete_request(struct ipmi_softc *sc, struct ipmi_request *req)
 
 /* Perform an internal driver request. */
 int
-ipmi_submit_driver_request(struct ipmi_softc *sc, struct ipmi_request *req,
-    int timo)
+ipmi_submit_driver_request(struct ipmi_softc *sc, struct ipmi_request *req)
 {
 
-	return (sc->ipmi_driver_request(sc, req, timo));
+	return (sc->ipmi_driver_request(sc, req));
 }
 
 /*
@@ -636,7 +635,7 @@ ipmi_reset_watchdog(struct ipmi_softc *sc)
 
 	IPMI_ALLOC_DRIVER_REQUEST(req, IPMI_ADDR(IPMI_APP_REQUEST, 0),
 	    IPMI_RESET_WDOG, 0, 0);
-	error = ipmi_submit_driver_request(sc, req, 0);
+	error = ipmi_submit_driver_request(sc, req);
 	if (error) {
 		device_printf(sc->ipmi_dev, "Failed to reset watchdog\n");
 	} else if (req->ir_compcode == 0x80) {
@@ -677,7 +676,7 @@ ipmi_set_watchdog(struct ipmi_softc *sc, unsigned int sec)
 		req->ir_request[4] = 0;
 		req->ir_request[5] = 0;
 	}
-	error = ipmi_submit_driver_request(sc, req, 0);
+	error = ipmi_submit_driver_request(sc, req);
 	if (error) {
 		device_printf(sc->ipmi_dev, "Failed to set watchdog\n");
 	} else if (req->ir_compcode != 0) {
@@ -812,7 +811,7 @@ ipmi_power_cycle(void *arg, int howto)
 	    IPMI_CHASSIS_CONTROL, 1, 0);
 	req->ir_request[0] = IPMI_CC_POWER_CYCLE;
 
-	ipmi_submit_driver_request(sc, req, MAX_TIMEOUT);
+	ipmi_submit_driver_request(sc, req);
 
 	if (req->ir_error != 0 || req->ir_compcode != 0) {
 		device_printf(sc->ipmi_dev, "Power cycling via IPMI failed code %#x %#x\n",
@@ -859,7 +858,7 @@ ipmi_startup(void *arg)
 	IPMI_ALLOC_DRIVER_REQUEST(req, IPMI_ADDR(IPMI_APP_REQUEST, 0),
 	    IPMI_GET_DEVICE_ID, 0, 15);
 
-	error = ipmi_submit_driver_request(sc, req, MAX_TIMEOUT);
+	error = ipmi_submit_driver_request(sc, req);
 	if (error == EWOULDBLOCK) {
 		device_printf(dev, "Timed out waiting for GET_DEVICE_ID\n");
 		return;
@@ -888,7 +887,7 @@ ipmi_startup(void *arg)
 	IPMI_INIT_DRIVER_REQUEST(req, IPMI_ADDR(IPMI_APP_REQUEST, 0),
 	    IPMI_CLEAR_FLAGS, 1, 0);
 
-	ipmi_submit_driver_request(sc, req, 0);
+	ipmi_submit_driver_request(sc, req);
 
 	/* XXX: Magic numbers */
 	if (req->ir_compcode == 0xc0) {
@@ -903,7 +902,7 @@ ipmi_startup(void *arg)
 		    IPMI_GET_CHANNEL_INFO, 1, 0);
 		req->ir_request[0] = i;
 
-		error = ipmi_submit_driver_request(sc, req, 0);
+		error = ipmi_submit_driver_request(sc, req);
 
 		if (error != 0 || req->ir_compcode != 0)
 			break;
@@ -918,7 +917,7 @@ ipmi_startup(void *arg)
 		IPMI_INIT_DRIVER_REQUEST(req, IPMI_ADDR(IPMI_APP_REQUEST, 0),
 		    IPMI_GET_WDOG, 0, 0);
 
-		error = ipmi_submit_driver_request(sc, req, 0);
+		error = ipmi_submit_driver_request(sc, req);
 
 		if (error == 0 && req->ir_compcode == 0x00) {
 			device_printf(dev, "Attached watchdog\n");
diff --git a/sys/dev/ipmi/ipmi_bt.c b/sys/dev/ipmi/ipmi_bt.c
index 2e92bdb0699e..c13397abd253 100644
--- a/sys/dev/ipmi/ipmi_bt.c
+++ b/sys/dev/ipmi/ipmi_bt.c
@@ -85,7 +85,7 @@
 #define	 BT_IM_BMC_HWRST	(1L << 7)
 
 static int bt_polled_request(struct ipmi_softc *, struct ipmi_request *);
-static int bt_driver_request(struct ipmi_softc *, struct ipmi_request *, int);
+static int bt_driver_request(struct ipmi_softc *, struct ipmi_request *);
 static int bt_wait(struct ipmi_softc *, uint8_t, uint8_t);
 static int bt_reset(struct ipmi_softc *);
 
@@ -247,7 +247,7 @@ bt_loop(void *arg)
 	IPMI_LOCK(sc);
 	while ((req = ipmi_dequeue_request(sc)) != NULL) {
 		IPMI_UNLOCK(sc);
-		(void)bt_driver_request(sc, req, 0);
+		(void)bt_driver_request(sc, req);
 		IPMI_LOCK(sc);
 		sc->ipmi_bt_seq++;
 		ipmi_complete_request(sc, req);
@@ -265,7 +265,7 @@ bt_startup(struct ipmi_softc *sc)
 }
 
 static int
-bt_driver_request(struct ipmi_softc *sc, struct ipmi_request *req, int timo __unused)
+bt_driver_request(struct ipmi_softc *sc, struct ipmi_request *req)
 {
 	int i, ok;
 
diff --git a/sys/dev/ipmi/ipmi_kcs.c b/sys/dev/ipmi/ipmi_kcs.c
index 3f1d84d708ce..be8e6664b717 100644
--- a/sys/dev/ipmi/ipmi_kcs.c
+++ b/sys/dev/ipmi/ipmi_kcs.c
@@ -488,13 +488,13 @@ kcs_startup(struct ipmi_softc *sc)
 }
 
 static int
-kcs_driver_request_queue(struct ipmi_softc *sc, struct ipmi_request *req, int timo)
+kcs_driver_request_queue(struct ipmi_softc *sc, struct ipmi_request *req)
 {
 	int error;
 
 	IPMI_LOCK(sc);
 	ipmi_polled_enqueue_request_highpri(sc, req);
-	error = msleep(req, &sc->ipmi_requests_lock, 0, "ipmireq", timo);
+	error = msleep(req, &sc->ipmi_requests_lock, 0, "ipmireq", 0);
 	if (error == 0)
 		error = req->ir_error;
 	IPMI_UNLOCK(sc);
@@ -517,13 +517,13 @@ kcs_driver_request_poll(struct ipmi_softc *sc, struct ipmi_request *req)
 }
 
 static int
-kcs_driver_request(struct ipmi_softc *sc, struct ipmi_request *req, int timo)
+kcs_driver_request(struct ipmi_softc *sc, struct ipmi_request *req)
 {
 
 	if (KERNEL_PANICKED() || dumping)
 		return (kcs_driver_request_poll(sc, req));
 	else
-		return (kcs_driver_request_queue(sc, req, timo));
+		return (kcs_driver_request_queue(sc, req));
 }
 
 
diff --git a/sys/dev/ipmi/ipmi_opal.c b/sys/dev/ipmi/ipmi_opal.c
index 084caba45184..ab0f14e8b3a8 100644
--- a/sys/dev/ipmi/ipmi_opal.c
+++ b/sys/dev/ipmi/ipmi_opal.c
@@ -142,8 +142,7 @@ opal_ipmi_discard_msgs(struct opal_ipmi_softc *sc)
 }
 
 static int
-opal_ipmi_polled_request(struct opal_ipmi_softc *sc, struct ipmi_request *req,
-    int timo)
+opal_ipmi_polled_request(struct opal_ipmi_softc *sc, struct ipmi_request *req)
 {
 	uint64_t msg_len;
 	int err;
@@ -192,7 +191,7 @@ opal_ipmi_polled_request(struct opal_ipmi_softc *sc, struct ipmi_request *req,
 		goto out;
 	}
 
-	if ((err = opal_ipmi_recv(sc, &msg_len, timo)) == 0) {
+	if ((err = opal_ipmi_recv(sc, &msg_len, 0)) == 0) {
 		/* Subtract one extra for the completion code. */
 		req->ir_replylen = msg_len - sizeof(struct opal_ipmi_msg) - 1;
 		req->ir_replylen = min(req->ir_replylen, req->ir_replybuflen);
@@ -248,15 +247,14 @@ opal_ipmi_startup(struct ipmi_softc *sc)
 }
 
 static int
-opal_ipmi_driver_request(struct ipmi_softc *isc, struct ipmi_request *req,
-    int timo)
+opal_ipmi_driver_request(struct ipmi_softc *isc, struct ipmi_request *req)
 {
 	struct opal_ipmi_softc *sc = (struct opal_ipmi_softc *)isc;
 	int i, err;
 
 	for (i = 0; i < 3; i++) {
 		IPMI_LOCK(&sc->ipmi);
-		err = opal_ipmi_polled_request(sc, req, timo);
+		err = opal_ipmi_polled_request(sc, req);
 		IPMI_UNLOCK(&sc->ipmi);
 		if (err == 0)
 			break;
diff --git a/sys/dev/ipmi/ipmi_smic.c b/sys/dev/ipmi/ipmi_smic.c
index 1bcede44f920..0a80562db7dc 100644
--- a/sys/dev/ipmi/ipmi_smic.c
+++ b/sys/dev/ipmi/ipmi_smic.c
@@ -388,7 +388,7 @@ smic_startup(struct ipmi_softc *sc)
 }
 
 static int
-smic_driver_request(struct ipmi_softc *sc, struct ipmi_request *req, int timo)
+smic_driver_request(struct ipmi_softc *sc, struct ipmi_request *req)
 {
 	int i, ok;
 
diff --git a/sys/dev/ipmi/ipmi_ssif.c b/sys/dev/ipmi/ipmi_ssif.c
index 0c22d35421ef..c83cccc75123 100644
--- a/sys/dev/ipmi/ipmi_ssif.c
+++ b/sys/dev/ipmi/ipmi_ssif.c
@@ -359,15 +359,14 @@ ssif_startup(struct ipmi_softc *sc)
 }
 
 static int
-ssif_driver_request(struct ipmi_softc *sc, struct ipmi_request *req, int timo)
+ssif_driver_request(struct ipmi_softc *sc, struct ipmi_request *req)
 {
 	int error;
 
 	IPMI_LOCK(sc);
 	error = ipmi_polled_enqueue_request(sc, req);
 	if (error == 0)
-		error = msleep(req, &sc->ipmi_requests_lock, 0, "ipmireq",
-		    timo);
+		error = msleep(req, &sc->ipmi_requests_lock, 0, "ipmireq", 0);
 	if (error == 0)
 		error = req->ir_error;
 	IPMI_UNLOCK(sc);
diff --git a/sys/dev/ipmi/ipmivars.h b/sys/dev/ipmi/ipmivars.h
index 17227c2053db..6ab8b128b820 100644
--- a/sys/dev/ipmi/ipmivars.h
+++ b/sys/dev/ipmi/ipmivars.h
@@ -133,7 +133,7 @@ struct ipmi_softc {
 	driver_intr_t		*ipmi_intr;
 	int			(*ipmi_startup)(struct ipmi_softc *);
 	int			(*ipmi_enqueue_request)(struct ipmi_softc *, struct ipmi_request *);
-	int			(*ipmi_driver_request)(struct ipmi_softc *, struct ipmi_request *, int);
+	int			(*ipmi_driver_request)(struct ipmi_softc *, struct ipmi_request *);
 };
 
 #define	ipmi_ssif_smbus_address		_iface.ssif.smbus_address
@@ -247,8 +247,7 @@ struct ipmi_request *ipmi_dequeue_request(struct ipmi_softc *);
 void	ipmi_free_request(struct ipmi_request *);
 int	ipmi_polled_enqueue_request(struct ipmi_softc *, struct ipmi_request *);
 int	ipmi_polled_enqueue_request_highpri(struct ipmi_softc *, struct ipmi_request *);
-int	ipmi_submit_driver_request(struct ipmi_softc *, struct ipmi_request *,
-	    int);
+int	ipmi_submit_driver_request(struct ipmi_softc *, struct ipmi_request *);
 
 /* Identify BMC interface via SMBIOS. */
 int	ipmi_smbios_identify(struct ipmi_get_info *);