3770 ipmi drivers hangs due to attach/detach/attach cycle

Reviewed by: Hans Rosenfeld <hans.rosenfeld@nexenta.com>
Reviewed by: Albert Lee <trisk@nexenta.com>
Reviewed by: Saso Kiselkov <skiselkov.ml@gmail.com>
Approved by: Richard Lowe <richlowe@richlowe.net>

3 files changed, 50 insertions, 11 deletions

diff --git a/usr/src/uts/intel/io/ipmi/ipmi.c b/usr/src/uts/intel/io/ipmi/ipmi.c
index 7c59bc2fda..095d8dacda 100644
--- a/usr/src/uts/intel/io/ipmi/ipmi.c
+++ b/usr/src/uts/intel/io/ipmi/ipmi.c
@@ -28,6 +28,7 @@
 
 /*
  * Copyright 2012, Joyent, Inc.  All rights reserved.
+ * Copyright 2013, Nexenta Systems, Inc.  All rights reserved.
  */
 
 #include <sys/devops.h>
@@ -178,6 +179,18 @@ ipmi_polled_enqueue_request(struct ipmi_softc *sc, struct ipmi_request *req)
 }
 
 void
+ipmi_shutdown(struct ipmi_softc *sc)
+{
+	taskq_destroy(sc->ipmi_kthread);
+
+	cv_destroy(&sc->ipmi_request_added);
+	mutex_destroy(&sc->ipmi_lock);
+
+	cv_destroy(&slplock);
+	mutex_destroy(&slpmutex);
+}
+
+boolean_t
 ipmi_startup(struct ipmi_softc *sc)
 {
 	struct ipmi_request *req;
@@ -195,7 +208,7 @@ ipmi_startup(struct ipmi_softc *sc)
 	error = sc->ipmi_startup(sc);
 	if (error) {
 		cmn_err(CE_WARN, "Failed to initialize interface: %d", error);
-		return;
+		return (B_FALSE);
 	}
 
 	/* Send a GET_DEVICE_ID request. */
@@ -206,22 +219,22 @@ ipmi_startup(struct ipmi_softc *sc)
 	if (error == EWOULDBLOCK) {
 		cmn_err(CE_WARN, "Timed out waiting for GET_DEVICE_ID");
 		ipmi_free_request(req);
-		return;
+		return (B_FALSE);
 	} else if (error) {
 		cmn_err(CE_WARN, "Failed GET_DEVICE_ID: %d", error);
 		ipmi_free_request(req);
-		return;
+		return (B_FALSE);
 	} else if (req->ir_compcode != 0) {
 		cmn_err(CE_WARN,
 		    "Bad completion code for GET_DEVICE_ID: %d",
 		    req->ir_compcode);
 		ipmi_free_request(req);
-		return;
+		return (B_FALSE);
 	} else if (req->ir_replylen < 5) {
 		cmn_err(CE_WARN, "Short reply for GET_DEVICE_ID: %d",
 		    req->ir_replylen);
 		ipmi_free_request(req);
-		return;
+		return (B_FALSE);
 	}
 
 	cmn_err(CE_CONT, "!device rev. %d, firmware rev. %d.%d%d, "
@@ -235,8 +248,11 @@ ipmi_startup(struct ipmi_softc *sc)
 	req = ipmi_alloc_driver_request(IPMI_ADDR(IPMI_APP_REQUEST, 0),
 	    IPMI_CLEAR_FLAGS, 1, 0);
 
-	if ((error = ipmi_submit_driver_request(sc, req, 0)) != 0)
+	if ((error = ipmi_submit_driver_request(sc, req, 0)) != 0) {
 		cmn_err(CE_WARN, "Failed to clear IPMI flags: %d\n", error);
+		ipmi_free_request(req);
+		return (B_FALSE);
+	}
 
 	/* Magic numbers */
 	if (req->ir_compcode == 0xc0) {
@@ -272,7 +288,7 @@ ipmi_startup(struct ipmi_softc *sc)
 	if ((error = ipmi_submit_driver_request(sc, req, 0)) != 0) {
 		cmn_err(CE_WARN, "Failed to check IPMI watchdog: %d\n", error);
 		ipmi_free_request(req);
-		return;
+		return (B_FALSE);
 	}
 
 	if (req->ir_compcode == 0x00) {
@@ -284,4 +300,6 @@ ipmi_startup(struct ipmi_softc *sc)
 		 */
 	}
 	ipmi_free_request(req);
+
+	return (B_TRUE);
 }
diff --git a/usr/src/uts/intel/io/ipmi/ipmi_main.c b/usr/src/uts/intel/io/ipmi/ipmi_main.c
index 9d1e2d0031..812314f54e 100644
--- a/usr/src/uts/intel/io/ipmi/ipmi_main.c
+++ b/usr/src/uts/intel/io/ipmi/ipmi_main.c
@@ -21,6 +21,7 @@
 
 /*
  * Copyright 2012, Joyent, Inc.  All rights reserved.
+ * Copyright 2013, Nexenta Systems, Inc.  All rights reserved.
  */
 
 /*
@@ -487,6 +488,14 @@ ipmi_attach(dev_info_t *dip, ddi_attach_cmd_t cmd)
 	if (cmd != DDI_ATTACH)
 		return (DDI_FAILURE);
 
+	/* this driver only supports one device instance */
+	if (ddi_get_instance(dip) != 0) {
+		cmn_err(CE_WARN,
+		    "!not attaching to non-zero device instance %d",
+		    ddi_get_instance(dip));
+		return (DDI_FAILURE);
+	}
+
 	if (get_smbios_ipmi_info() == DDI_FAILURE)
 		return (DDI_FAILURE);
 
@@ -518,7 +527,11 @@ ipmi_attach(dev_info_t *dip, ddi_attach_cmd_t cmd)
 	/* Create ID space for open devs.  ID 0 is reserved. */
 	minor_ids = id_space_create("ipmi_id_space", 1, 128);
 
-	ipmi_startup(sc);
+	if (ipmi_startup(sc) != B_TRUE) {
+		ipmi_shutdown(sc);
+		return (DDI_FAILURE);
+	}
+
 	ipmi_attached = B_TRUE;
 
 	return (DDI_SUCCESS);
@@ -540,13 +553,14 @@ ipmi_detach(dev_info_t *dip, ddi_detach_cmd_t cmd)
 	sc->ipmi_detaching = 1;
 	cv_signal(&sc->ipmi_request_added);
 
+	ipmi_shutdown(sc);
 	ddi_remove_minor_node(dip, NULL);
 	ipmi_dip = NULL;
 
-	taskq_destroy(sc->ipmi_kthread);
 	list_destroy(&dev_list);
 	id_space_destroy(minor_ids);
 
+	sc->ipmi_detaching = 0;
 	ipmi_attached = B_FALSE;
 	return (DDI_SUCCESS);
 }
@@ -566,7 +580,10 @@ static struct cb_ops ipmi_cb_ops = {
 	ipmi_poll,
 	ddi_prop_op,
 	NULL,			/* streamtab */
-	D_NEW | D_MP		/* flags */
+	D_NEW | D_MP,		/* flags */
+	CB_REV,
+	nodev,			/* awread */
+	nodev			/* awrite */
 };
 
 static struct dev_ops ipmi_ops = {
diff --git a/usr/src/uts/intel/io/ipmi/ipmivars.h b/usr/src/uts/intel/io/ipmi/ipmivars.h
index 2943fd1574..b12971f3a2 100644
--- a/usr/src/uts/intel/io/ipmi/ipmivars.h
+++ b/usr/src/uts/intel/io/ipmi/ipmivars.h
@@ -28,6 +28,7 @@
 
 /*
  * Copyright 2012, Joyent, Inc.  All rights reserved.
+ * Copyright 2013, Nexenta Systems, Inc.  All rights reserved.
  */
 
 #ifndef _IPMIVARS_H_
@@ -179,9 +180,12 @@ struct ipmi_request *ipmi_alloc_request(struct ipmi_device *, long msgid,
 void	ipmi_free_request(struct ipmi_request *);
 
 /* Interface attach routines. */
-void	ipmi_startup(struct ipmi_softc *sc);
+boolean_t ipmi_startup(struct ipmi_softc *sc);
 int	ipmi_kcs_attach(struct ipmi_softc *);
 
+/* Interface detach cleanup */
+void	ipmi_shutdown(struct ipmi_softc *sc);
+
 #ifdef	__cplusplus
 }
 #endif