[char-misc 0/4] mei: reset fix

[char-misc 0/4] mei: reset fix

[Tomas Winkler]

This series should address possible hiccup in
mei reset flow presenting itself as never ending   
repetition of error line.

unexpected reset: dev_state = RESETTING  

The patches make sure that stall watchdog doesn't jump in 
prior to its time; second the reset flow won't recuse,
and at last  it will allow no more than 3 consecutive resets
for misbehaving hardware.


Tomas Winkler (4):
  mei: do not run reset flow from the interrupt thread
  mei: use hbm idle state to prevent spurious resets
  mei: revamp mei reset state machine
  mei: limit the number of consecutive resets

 drivers/misc/mei/client.c    |   1 +
 drivers/misc/mei/hbm.c       | 221 ++++++++++++++++++++++------------
 drivers/misc/mei/hbm.h       |   7 +-
 drivers/misc/mei/hw-me.c     |  40 +++----
 drivers/misc/mei/init.c      | 280 +++++++++++++++++++++++++++----------------
 drivers/misc/mei/interrupt.c |  45 ++++---
 drivers/misc/mei/mei_dev.h   |  11 +-
 drivers/misc/mei/pci-me.c    |  10 +-
 8 files changed, 386 insertions(+), 229 deletions(-)

-- 
1.8.4.2

[char-misc 1/4] mei: do not run reset flow from the interrupt thread

[Tomas Winkler]

This fixes a potential deadlock in case of a firmware
initiated reset

mei_reset has a dialog with the interrupt thread hence
it has to be run from an another work item

Most of the mei_resets were called from mei_hbm_dispatch
which is called in interrupt thread context so this
function underwent major revamp. The error code is
propagated to the interrupt thread and if needed
the reset is scheduled from there.

Cc: <stable@vger.kernel.org> 
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
---
 drivers/misc/mei/hbm.c       | 200 +++++++++++++++++++++++++++----------------
 drivers/misc/mei/hbm.h       |   6 +-
 drivers/misc/mei/hw-me.c     |  32 ++++---
 drivers/misc/mei/init.c      | 100 +++++++++++++---------
 drivers/misc/mei/interrupt.c |   9 +-
 drivers/misc/mei/mei_dev.h   |   1 +
 6 files changed, 210 insertions(+), 138 deletions(-)

diff --git a/drivers/misc/mei/hbm.c b/drivers/misc/mei/hbm.c
index 9b3a0fb..8109b9a 100644
--- a/drivers/misc/mei/hbm.c
+++ b/drivers/misc/mei/hbm.c
@@ -28,9 +28,9 @@
  *
  * @dev: the device structure
  *
- * returns none.
+ * returns 0 on success -ENOMEM on allocation failure
  */
-static void mei_hbm_me_cl_allocate(struct mei_device *dev)
+static int mei_hbm_me_cl_allocate(struct mei_device *dev)
 {
 	struct mei_me_client *clients;
 	int b;
@@ -44,7 +44,7 @@ static void mei_hbm_me_cl_allocate(struct mei_device *dev)
 		dev->me_clients_num++;
 
 	if (dev->me_clients_num == 0)
-		return;
+		return 0;
 
 	kfree(dev->me_clients);
 	dev->me_clients = NULL;
@@ -56,12 +56,10 @@ static void mei_hbm_me_cl_allocate(struct mei_device *dev)
 			sizeof(struct mei_me_client), GFP_KERNEL);
 	if (!clients) {
 		dev_err(&dev->pdev->dev, "memory allocation for ME clients failed.\n");
-		dev->dev_state = MEI_DEV_RESETTING;
-		mei_reset(dev, 1);
-		return;
+		return -ENOMEM;
 	}
 	dev->me_clients = clients;
-	return;
+	return 0;
 }
 
 /**
@@ -137,7 +135,7 @@ int mei_hbm_start_wait(struct mei_device *dev)
 	mutex_unlock(&dev->device_lock);
 	ret = wait_event_interruptible_timeout(dev->wait_recvd_msg,
 			dev->hbm_state == MEI_HBM_IDLE ||
-			dev->hbm_state > MEI_HBM_START,
+			dev->hbm_state >= MEI_HBM_STARTED,
 			mei_secs_to_jiffies(MEI_INTEROP_TIMEOUT));
 	mutex_lock(&dev->device_lock);
 
@@ -153,12 +151,15 @@ int mei_hbm_start_wait(struct mei_device *dev)
  * mei_hbm_start_req - sends start request message.
  *
  * @dev: the device structure
+ *
+ * returns 0 on success and < 0 on failure
  */
 int mei_hbm_start_req(struct mei_device *dev)
 {
 	struct mei_msg_hdr *mei_hdr = &dev->wr_msg.hdr;
 	struct hbm_host_version_request *start_req;
 	const size_t len = sizeof(struct hbm_host_version_request);
+	int ret;
 
 	mei_hbm_hdr(mei_hdr, len);
 
@@ -170,12 +171,13 @@ int mei_hbm_start_req(struct mei_device *dev)
 	start_req->host_version.minor_version = HBM_MINOR_VERSION;
 
 	dev->hbm_state = MEI_HBM_IDLE;
-	if (mei_write_message(dev, mei_hdr, dev->wr_msg.data)) {
-		dev_err(&dev->pdev->dev, "version message write failed\n");
-		dev->dev_state = MEI_DEV_RESETTING;
-		mei_reset(dev, 1);
-		return -EIO;
+	ret = mei_write_message(dev, mei_hdr, dev->wr_msg.data);
+	if (ret) {
+		dev_err(&dev->pdev->dev, "version message write failed: ret = %d\n",
+			ret);
+		return ret;
 	}
+
 	dev->hbm_state = MEI_HBM_START;
 	dev->init_clients_timer = MEI_CLIENTS_INIT_TIMEOUT;
 	return 0;
@@ -186,13 +188,15 @@ int mei_hbm_start_req(struct mei_device *dev)
  *
  * @dev: the device structure
  *
- * returns none.
+ * returns 0 on success and < 0 on failure
  */
-static void mei_hbm_enum_clients_req(struct mei_device *dev)
+static int mei_hbm_enum_clients_req(struct mei_device *dev)
 {
 	struct mei_msg_hdr *mei_hdr = &dev->wr_msg.hdr;
 	struct hbm_host_enum_request *enum_req;
 	const size_t len = sizeof(struct hbm_host_enum_request);
+	int ret;
+
 	/* enumerate clients */
 	mei_hbm_hdr(mei_hdr, len);
 
@@ -200,14 +204,15 @@ static void mei_hbm_enum_clients_req(struct mei_device *dev)
 	memset(enum_req, 0, len);
 	enum_req->hbm_cmd = HOST_ENUM_REQ_CMD;
 
-	if (mei_write_message(dev, mei_hdr, dev->wr_msg.data)) {
-		dev->dev_state = MEI_DEV_RESETTING;
-		dev_err(&dev->pdev->dev, "enumeration request write failed.\n");
-		mei_reset(dev, 1);
+	ret = mei_write_message(dev, mei_hdr, dev->wr_msg.data);
+	if (ret) {
+		dev_err(&dev->pdev->dev, "enumeration request write failed: ret = %d.\n",
+			ret);
+		return ret;
 	}
 	dev->hbm_state = MEI_HBM_ENUM_CLIENTS;
 	dev->init_clients_timer = MEI_CLIENTS_INIT_TIMEOUT;
-	return;
+	return 0;
 }
 
 /**
@@ -215,7 +220,7 @@ static void mei_hbm_enum_clients_req(struct mei_device *dev)
  *
  * @dev: the device structure
  *
- * returns none.
+ * returns 0 on success and < 0 on failure
  */
 
 static int mei_hbm_prop_req(struct mei_device *dev)
@@ -226,7 +231,7 @@ static int mei_hbm_prop_req(struct mei_device *dev)
 	const size_t len = sizeof(struct hbm_props_request);
 	unsigned long next_client_index;
 	unsigned long client_num;
-
+	int ret;
 
 	client_num = dev->me_client_presentation_num;
 
@@ -253,12 +258,11 @@ static int mei_hbm_prop_req(struct mei_device *dev)
 	prop_req->hbm_cmd = HOST_CLIENT_PROPERTIES_REQ_CMD;
 	prop_req->address = next_client_index;
 
-	if (mei_write_message(dev, mei_hdr, dev->wr_msg.data)) {
-		dev->dev_state = MEI_DEV_RESETTING;
-		dev_err(&dev->pdev->dev, "properties request write failed\n");
-		mei_reset(dev, 1);
-
-		return -EIO;
+	ret = mei_write_message(dev, mei_hdr, dev->wr_msg.data);
+	if (ret) {
+		dev_err(&dev->pdev->dev, "properties request write failed: ret = %d\n",
+			ret);
+		return ret;
 	}
 
 	dev->init_clients_timer = MEI_CLIENTS_INIT_TIMEOUT;
@@ -559,8 +563,10 @@ bool mei_hbm_version_is_supported(struct mei_device *dev)
  *
  * @dev: the device structure
  * @mei_hdr: header of bus message
+ *
+ * returns 0 on success and < 0 on failure
  */
-void mei_hbm_dispatch(struct mei_device *dev, struct mei_msg_hdr *hdr)
+int mei_hbm_dispatch(struct mei_device *dev, struct mei_msg_hdr *hdr)
 {
 	struct mei_bus_message *mei_msg;
 	struct mei_me_client *me_client;
@@ -579,6 +585,10 @@ void mei_hbm_dispatch(struct mei_device *dev, struct mei_msg_hdr *hdr)
 
 	switch (mei_msg->hbm_cmd) {
 	case HOST_START_RES_CMD:
+		dev_dbg(&dev->pdev->dev, "hbm: start: response message received.\n");
+
+		dev->init_clients_timer = 0;
+
 		version_res = (struct hbm_host_version_response *)mei_msg;
 
 		dev_dbg(&dev->pdev->dev, "HBM VERSION: DRIVER=%02d:%02d DEVICE=%02d:%02d\n",
@@ -597,73 +607,89 @@ void mei_hbm_dispatch(struct mei_device *dev, struct mei_msg_hdr *hdr)
 		}
 
 		if (!mei_hbm_version_is_supported(dev)) {
-			dev_warn(&dev->pdev->dev, "hbm version mismatch: stopping the driver.\n");
+			dev_warn(&dev->pdev->dev, "hbm: start: version mismatch - stopping the driver.\n");
 
-			dev->hbm_state = MEI_HBM_STOP;
+			dev->hbm_state = MEI_HBM_STOPPED;
 			mei_hbm_stop_req_prepare(dev, &dev->wr_msg.hdr,
 						dev->wr_msg.data);
-			mei_write_message(dev, &dev->wr_msg.hdr,
-					dev->wr_msg.data);
+			if (mei_write_message(dev, &dev->wr_msg.hdr,
+					dev->wr_msg.data)) {
+				dev_err(&dev->pdev->dev, "hbm: start: failed to send stop request\n");
+				return -EIO;
+			}
+			break;
+		}
 
-			return;
+		if (dev->dev_state != MEI_DEV_INIT_CLIENTS ||
+		    dev->hbm_state != MEI_HBM_START) {
+			dev_err(&dev->pdev->dev, "hbm: start: state mismatch, [%d, %d]\n",
+				dev->dev_state, dev->hbm_state);
+			return -EPROTO;
 		}
 
-		if (dev->dev_state == MEI_DEV_INIT_CLIENTS &&
-		    dev->hbm_state == MEI_HBM_START) {
-			dev->init_clients_timer = 0;
-			mei_hbm_enum_clients_req(dev);
-		} else {
-			dev_err(&dev->pdev->dev, "reset: wrong host start response\n");
-			mei_reset(dev, 1);
-			return;
+		dev->hbm_state = MEI_HBM_STARTED;
+
+		if (mei_hbm_enum_clients_req(dev)) {
+			dev_err(&dev->pdev->dev, "hbm: start: failed to send enumeration request\n");
+			return -EIO;
 		}
 
 		wake_up_interruptible(&dev->wait_recvd_msg);
-		dev_dbg(&dev->pdev->dev, "host start response message received.\n");
 		break;
 
 	case CLIENT_CONNECT_RES_CMD:
+		dev_dbg(&dev->pdev->dev, "hbm: client connect response: message received.\n");
+
 		connect_res = (struct hbm_client_connect_response *) mei_msg;
 		mei_hbm_cl_connect_res(dev, connect_res);
-		dev_dbg(&dev->pdev->dev, "client connect response message received.\n");
 		wake_up(&dev->wait_recvd_msg);
 		break;
 
 	case CLIENT_DISCONNECT_RES_CMD:
+		dev_dbg(&dev->pdev->dev, "hbm: client disconnect response: message received.\n");
+
 		disconnect_res = (struct hbm_client_connect_response *) mei_msg;
 		mei_hbm_cl_disconnect_res(dev, disconnect_res);
-		dev_dbg(&dev->pdev->dev, "client disconnect response message received.\n");
 		wake_up(&dev->wait_recvd_msg);
 		break;
 
 	case MEI_FLOW_CONTROL_CMD:
+		dev_dbg(&dev->pdev->dev, "hbm: client flow control response: message received.\n");
+
 		flow_control = (struct hbm_flow_control *) mei_msg;
 		mei_hbm_cl_flow_control_res(dev, flow_control);
-		dev_dbg(&dev->pdev->dev, "client flow control response message received.\n");
 		break;
 
 	case HOST_CLIENT_PROPERTIES_RES_CMD:
+		dev_dbg(&dev->pdev->dev, "hbm: properties response: message received.\n");
+
+		dev->init_clients_timer = 0;
+
+		if (dev->me_clients == NULL) {
+			dev_err(&dev->pdev->dev, "hbm: properties response: mei_clients not allocated\n");
+			return -EPROTO;
+		}
+
 		props_res = (struct hbm_props_response *)mei_msg;
 		me_client = &dev->me_clients[dev->me_client_presentation_num];
 
-		if (props_res->status || !dev->me_clients) {
-			dev_err(&dev->pdev->dev, "reset: properties response hbm wrong status.\n");
-			mei_reset(dev, 1);
-			return;
+		if (props_res->status) {
+			dev_err(&dev->pdev->dev, "hbm: properties response: wrong status = %d\n",
+				props_res->status);
+			return -EPROTO;
 		}
 
 		if (me_client->client_id != props_res->address) {
-			dev_err(&dev->pdev->dev, "reset: host properties response address mismatch\n");
-			mei_reset(dev, 1);
-			return;
+			dev_err(&dev->pdev->dev, "hbm: properties response: address mismatch %d ?= %d\n",
+				me_client->client_id, props_res->address);
+			return -EPROTO;
 		}
 
 		if (dev->dev_state != MEI_DEV_INIT_CLIENTS ||
 		    dev->hbm_state != MEI_HBM_CLIENT_PROPERTIES) {
-			dev_err(&dev->pdev->dev, "reset: unexpected properties response\n");
-			mei_reset(dev, 1);
-
-			return;
+			dev_err(&dev->pdev->dev, "hbm: properties response: state mismatch, [%d, %d]\n",
+				dev->dev_state, dev->hbm_state);
+			return -EPROTO;
 		}
 
 		me_client->props = props_res->client_properties;
@@ -671,49 +697,70 @@ void mei_hbm_dispatch(struct mei_device *dev, struct mei_msg_hdr *hdr)
 		dev->me_client_presentation_num++;
 
 		/* request property for the next client */
-		mei_hbm_prop_req(dev);
+		if (mei_hbm_prop_req(dev))
+			return -EIO;
 
 		break;
 
 	case HOST_ENUM_RES_CMD:
+		dev_dbg(&dev->pdev->dev, "hbm: enumeration response: message received\n");
+
+		dev->init_clients_timer = 0;
+
 		enum_res = (struct hbm_host_enum_response *) mei_msg;
 		BUILD_BUG_ON(sizeof(dev->me_clients_map)
 				< sizeof(enum_res->valid_addresses));
 		memcpy(dev->me_clients_map, enum_res->valid_addresses,
 			sizeof(enum_res->valid_addresses));
-		if (dev->dev_state == MEI_DEV_INIT_CLIENTS &&
-		    dev->hbm_state == MEI_HBM_ENUM_CLIENTS) {
-				dev->init_clients_timer = 0;
-				mei_hbm_me_cl_allocate(dev);
-				dev->hbm_state = MEI_HBM_CLIENT_PROPERTIES;
-
-				/* first property reqeust */
-				mei_hbm_prop_req(dev);
-		} else {
-			dev_err(&dev->pdev->dev, "reset: unexpected enumeration response hbm.\n");
-			mei_reset(dev, 1);
-			return;
+
+		if (dev->dev_state != MEI_DEV_INIT_CLIENTS ||
+		    dev->hbm_state != MEI_HBM_ENUM_CLIENTS) {
+			dev_err(&dev->pdev->dev, "hbm: enumeration response: state mismatch, [%d, %d]\n",
+				dev->dev_state, dev->hbm_state);
+			return -EPROTO;
 		}
+
+		if (mei_hbm_me_cl_allocate(dev)) {
+			dev_err(&dev->pdev->dev, "hbm: enumeration response: cannot allocate clients array\n");
+			return -ENOMEM;
+		}
+
+		dev->hbm_state = MEI_HBM_CLIENT_PROPERTIES;
+
+		/* first property request */
+		if (mei_hbm_prop_req(dev))
+			return -EIO;
+
 		break;
 
 	case HOST_STOP_RES_CMD:
+		dev_dbg(&dev->pdev->dev, "hbm: stop response: message received\n");
+
+		dev->init_clients_timer = 0;
+
+		if (dev->hbm_state != MEI_HBM_STOPPED) {
+			dev_err(&dev->pdev->dev, "hbm: stop response: state mismatch, [%d, %d]\n",
+				dev->dev_state, dev->hbm_state);
+			return -EPROTO;
+		}
 
-		if (dev->hbm_state != MEI_HBM_STOP)
-			dev_err(&dev->pdev->dev, "unexpected stop response hbm.\n");
 		dev->dev_state = MEI_DEV_DISABLED;
-		dev_info(&dev->pdev->dev, "reset: FW stop response.\n");
-		mei_reset(dev, 1);
+		dev_info(&dev->pdev->dev, "hbm: stop response: resetting.\n");
+		/* force the reset */
+		return -EPROTO;
 		break;
 
 	case CLIENT_DISCONNECT_REQ_CMD:
-		/* search for client */
+		dev_dbg(&dev->pdev->dev, "hbm: disconnect request: message received\n");
+
 		disconnect_req = (struct hbm_client_connect_request *)mei_msg;
 		mei_hbm_fw_disconnect_req(dev, disconnect_req);
 		break;
 
 	case ME_STOP_REQ_CMD:
+		dev_dbg(&dev->pdev->dev, "hbm: stop request: message received\n");
 
-		dev->hbm_state = MEI_HBM_STOP;
+		dev->hbm_state = MEI_HBM_STOPPED;
 		mei_hbm_stop_req_prepare(dev, &dev->wr_ext_msg.hdr,
 					dev->wr_ext_msg.data);
 		break;
@@ -722,5 +769,6 @@ void mei_hbm_dispatch(struct mei_device *dev, struct mei_msg_hdr *hdr)
 		break;
 
 	}
+	return 0;
 }
 
diff --git a/drivers/misc/mei/hbm.h b/drivers/misc/mei/hbm.h
index 4ae2e56..f2540ff 100644
--- a/drivers/misc/mei/hbm.h
+++ b/drivers/misc/mei/hbm.h
@@ -32,13 +32,13 @@ struct mei_cl;
 enum mei_hbm_state {
 	MEI_HBM_IDLE = 0,
 	MEI_HBM_START,
+	MEI_HBM_STARTED,
 	MEI_HBM_ENUM_CLIENTS,
 	MEI_HBM_CLIENT_PROPERTIES,
-	MEI_HBM_STARTED,
-	MEI_HBM_STOP,
+	MEI_HBM_STOPPED,
 };
 
-void mei_hbm_dispatch(struct mei_device *dev, struct mei_msg_hdr *hdr);
+int mei_hbm_dispatch(struct mei_device *dev, struct mei_msg_hdr *hdr);
 
 static inline void mei_hbm_hdr(struct mei_msg_hdr *hdr, size_t length)
 {
diff --git a/drivers/misc/mei/hw-me.c b/drivers/misc/mei/hw-me.c
index 3412adc..6c07623 100644
--- a/drivers/misc/mei/hw-me.c
+++ b/drivers/misc/mei/hw-me.c
@@ -469,7 +469,7 @@ irqreturn_t mei_me_irq_thread_handler(int irq, void *dev_id)
 	struct mei_device *dev = (struct mei_device *) dev_id;
 	struct mei_cl_cb complete_list;
 	s32 slots;
-	int rets;
+	int rets = 0;
 
 	dev_dbg(&dev->pdev->dev, "function called after ISR to handle the interrupt processing.\n");
 	/* initialize our complete list */
@@ -487,10 +487,9 @@ irqreturn_t mei_me_irq_thread_handler(int irq, void *dev_id)
 	    dev->dev_state != MEI_DEV_INITIALIZING &&
 	    dev->dev_state != MEI_DEV_POWER_DOWN &&
 	    dev->dev_state != MEI_DEV_POWER_UP) {
-		dev_dbg(&dev->pdev->dev, "FW not ready.\n");
-		mei_reset(dev, 1);
-		mutex_unlock(&dev->device_lock);
-		return IRQ_HANDLED;
+		dev_warn(&dev->pdev->dev, "FW not ready: resetting.\n");
+		schedule_work(&dev->reset_work);
+		goto end;
 	}
 
 	/*  check if we need to start the dev */
@@ -500,15 +499,12 @@ irqreturn_t mei_me_irq_thread_handler(int irq, void *dev_id)
 
 			dev->recvd_hw_ready = true;
 			wake_up_interruptible(&dev->wait_hw_ready);
-
-			mutex_unlock(&dev->device_lock);
-			return IRQ_HANDLED;
 		} else {
+
 			dev_dbg(&dev->pdev->dev, "Reset Completed.\n");
 			mei_me_hw_reset_release(dev);
-			mutex_unlock(&dev->device_lock);
-			return IRQ_HANDLED;
 		}
+		goto end;
 	}
 	/* check slots available for reading */
 	slots = mei_count_full_read_slots(dev);
@@ -516,21 +512,23 @@ irqreturn_t mei_me_irq_thread_handler(int irq, void *dev_id)
 		/* we have urgent data to send so break the read */
 		if (dev->wr_ext_msg.hdr.length)
 			break;
-		dev_dbg(&dev->pdev->dev, "slots =%08x\n", slots);
-		dev_dbg(&dev->pdev->dev, "call mei_irq_read_handler.\n");
+		dev_dbg(&dev->pdev->dev, "slots to read = %08x\n", slots);
 		rets = mei_irq_read_handler(dev, &complete_list, &slots);
-		if (rets)
+		if (rets) {
+			schedule_work(&dev->reset_work);
 			goto end;
+		}
 	}
+
 	rets = mei_irq_write_handler(dev, &complete_list);
-end:
-	dev_dbg(&dev->pdev->dev, "end of bottom half function.\n");
-	dev->hbuf_is_ready = mei_hbuf_is_ready(dev);
 
-	mutex_unlock(&dev->device_lock);
+	dev->hbuf_is_ready = mei_hbuf_is_ready(dev);
 
 	mei_irq_compl_handler(dev, &complete_list);
 
+end:
+	dev_dbg(&dev->pdev->dev, "interrupt thread end ret = %d\n", rets);
+	mutex_unlock(&dev->device_lock);
 	return IRQ_HANDLED;
 }
 static const struct mei_hw_ops mei_me_hw_ops = {
diff --git a/drivers/misc/mei/init.c b/drivers/misc/mei/init.c
index f83da33..71cd209 100644
--- a/drivers/misc/mei/init.c
+++ b/drivers/misc/mei/init.c
@@ -43,42 +43,6 @@ const char *mei_dev_state_str(int state)
 #undef MEI_DEV_STATE
 }
 
-void mei_device_init(struct mei_device *dev)
-{
-	/* setup our list array */
-	INIT_LIST_HEAD(&dev->file_list);
-	INIT_LIST_HEAD(&dev->device_list);
-	mutex_init(&dev->device_lock);
-	init_waitqueue_head(&dev->wait_hw_ready);
-	init_waitqueue_head(&dev->wait_recvd_msg);
-	init_waitqueue_head(&dev->wait_stop_wd);
-	dev->dev_state = MEI_DEV_INITIALIZING;
-
-	mei_io_list_init(&dev->read_list);
-	mei_io_list_init(&dev->write_list);
-	mei_io_list_init(&dev->write_waiting_list);
-	mei_io_list_init(&dev->ctrl_wr_list);
-	mei_io_list_init(&dev->ctrl_rd_list);
-
-	INIT_DELAYED_WORK(&dev->timer_work, mei_timer);
-	INIT_WORK(&dev->init_work, mei_host_client_init);
-
-	INIT_LIST_HEAD(&dev->wd_cl.link);
-	INIT_LIST_HEAD(&dev->iamthif_cl.link);
-	mei_io_list_init(&dev->amthif_cmd_list);
-	mei_io_list_init(&dev->amthif_rd_complete_list);
-
-	bitmap_zero(dev->host_clients_map, MEI_CLIENTS_MAX);
-	dev->open_handle_count = 0;
-
-	/*
-	 * Reserving the first client ID
-	 * 0: Reserved for MEI Bus Message communications
-	 */
-	bitmap_set(dev->host_clients_map, 0, 1);
-}
-EXPORT_SYMBOL_GPL(mei_device_init);
-
 /**
  * mei_start - initializes host and fw to start work.
  *
@@ -131,10 +95,15 @@ err:
 }
 EXPORT_SYMBOL_GPL(mei_start);
 
-
+/**
+ * mei_cancel_work. Cancel mei background jobs
+ *
+ * @dev: the device structure
+ */
 void mei_cancel_work(struct mei_device *dev)
 {
 	cancel_work_sync(&dev->init_work);
+	cancel_work_sync(&dev->reset_work);
 
 	cancel_delayed_work(&dev->timer_work);
 }
@@ -216,11 +185,27 @@ void mei_reset(struct mei_device *dev, int interrupts_enabled)
 
 	dev->dev_state = MEI_DEV_INIT_CLIENTS;
 
-	mei_hbm_start_req(dev);
-
+	ret = mei_hbm_start_req(dev);
+	if (ret) {
+		dev_err(&dev->pdev->dev, "hbm_start failed disabling the device\n");
+		dev->dev_state = MEI_DEV_DISABLED;
+		return;
+	}
 }
 EXPORT_SYMBOL_GPL(mei_reset);
 
+static void mei_reset_work(struct work_struct *work)
+{
+	struct mei_device *dev =
+		container_of(work, struct mei_device,  reset_work);
+
+	mutex_lock(&dev->device_lock);
+
+	mei_reset(dev, true);
+
+	mutex_unlock(&dev->device_lock);
+}
+
 void mei_stop(struct mei_device *dev)
 {
 	dev_dbg(&dev->pdev->dev, "stopping the device.\n");
@@ -244,3 +229,40 @@ EXPORT_SYMBOL_GPL(mei_stop);
 
 
 
+void mei_device_init(struct mei_device *dev)
+{
+	/* setup our list array */
+	INIT_LIST_HEAD(&dev->file_list);
+	INIT_LIST_HEAD(&dev->device_list);
+	mutex_init(&dev->device_lock);
+	init_waitqueue_head(&dev->wait_hw_ready);
+	init_waitqueue_head(&dev->wait_recvd_msg);
+	init_waitqueue_head(&dev->wait_stop_wd);
+	dev->dev_state = MEI_DEV_INITIALIZING;
+
+	mei_io_list_init(&dev->read_list);
+	mei_io_list_init(&dev->write_list);
+	mei_io_list_init(&dev->write_waiting_list);
+	mei_io_list_init(&dev->ctrl_wr_list);
+	mei_io_list_init(&dev->ctrl_rd_list);
+
+	INIT_DELAYED_WORK(&dev->timer_work, mei_timer);
+	INIT_WORK(&dev->init_work, mei_host_client_init);
+	INIT_WORK(&dev->reset_work, mei_reset_work);
+
+	INIT_LIST_HEAD(&dev->wd_cl.link);
+	INIT_LIST_HEAD(&dev->iamthif_cl.link);
+	mei_io_list_init(&dev->amthif_cmd_list);
+	mei_io_list_init(&dev->amthif_rd_complete_list);
+
+	bitmap_zero(dev->host_clients_map, MEI_CLIENTS_MAX);
+	dev->open_handle_count = 0;
+
+	/*
+	 * Reserving the first client ID
+	 * 0: Reserved for MEI Bus Message communications
+	 */
+	bitmap_set(dev->host_clients_map, 0, 1);
+}
+EXPORT_SYMBOL_GPL(mei_device_init);
+
diff --git a/drivers/misc/mei/interrupt.c b/drivers/misc/mei/interrupt.c
index 9c8225b..bbb61be 100644
--- a/drivers/misc/mei/interrupt.c
+++ b/drivers/misc/mei/interrupt.c
@@ -329,9 +329,12 @@ int mei_irq_read_handler(struct mei_device *dev,
 
 	/*  HBM message */
 	if (mei_hdr->host_addr == 0 && mei_hdr->me_addr == 0) {
-		mei_hbm_dispatch(dev, mei_hdr);
-		ret = 0;
-		dev_dbg(&dev->pdev->dev, "mei_hbm_dispatch.\n");
+		ret = mei_hbm_dispatch(dev, mei_hdr);
+		if (ret) {
+			dev_dbg(&dev->pdev->dev, "mei_hbm_dispatch failed ret = %d\n",
+					ret);
+			goto end;
+		}
 		goto reset_slots;
 	}
 
diff --git a/drivers/misc/mei/mei_dev.h b/drivers/misc/mei/mei_dev.h
index d92ca0c..3f242e1 100644
--- a/drivers/misc/mei/mei_dev.h
+++ b/drivers/misc/mei/mei_dev.h
@@ -428,6 +428,7 @@ struct mei_device {
 	bool iamthif_canceled;
 
 	struct work_struct init_work;
+	struct work_struct reset_work;
 
 	/* List of bus devices */
 	struct list_head device_list;
-- 
1.8.4.2

[char-misc 2/4] mei: use hbm idle state to prevent spurious resets

[Tomas Winkler]

When reset is caused by hbm protocol mismatch or timeout
we might end up in an endless reset loop and hbm protocol
will never sync

Cc: <stable@vger.kernel.org> 
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
---
 drivers/misc/mei/hbm.c       | 19 +++++++++++++++++++
 drivers/misc/mei/hbm.h       |  1 +
 drivers/misc/mei/init.c      | 12 ++++++++----
 drivers/misc/mei/interrupt.c | 25 +++++++++++++++----------
 4 files changed, 43 insertions(+), 14 deletions(-)

diff --git a/drivers/misc/mei/hbm.c b/drivers/misc/mei/hbm.c
index 8109b9a..836f92d 100644
--- a/drivers/misc/mei/hbm.c
+++ b/drivers/misc/mei/hbm.c
@@ -126,6 +126,17 @@ static bool is_treat_specially_client(struct mei_cl *cl,
 	return false;
 }
 
+/**
+ * mei_hbm_idle - set hbm to idle state
+ *
+ * @dev: the device structure
+ */
+void mei_hbm_idle(struct mei_device *dev)
+{
+	dev->init_clients_timer = 0;
+	dev->hbm_state = MEI_HBM_IDLE;
+}
+
 int mei_hbm_start_wait(struct mei_device *dev)
 {
 	int ret;
@@ -583,6 +594,14 @@ int mei_hbm_dispatch(struct mei_device *dev, struct mei_msg_hdr *hdr)
 	mei_read_slots(dev, dev->rd_msg_buf, hdr->length);
 	mei_msg = (struct mei_bus_message *)dev->rd_msg_buf;
 
+	/* ignore spurious message and prevent reset nesting
+	 * hbm is put to idle during system reset
+	 */
+	if (dev->hbm_state == MEI_HBM_IDLE) {
+		dev_dbg(&dev->pdev->dev, "hbm: state is idle ignore spurious messages\n");
+		return 0;
+	}
+
 	switch (mei_msg->hbm_cmd) {
 	case HOST_START_RES_CMD:
 		dev_dbg(&dev->pdev->dev, "hbm: start: response message received.\n");
diff --git a/drivers/misc/mei/hbm.h b/drivers/misc/mei/hbm.h
index f2540ff..5f92188 100644
--- a/drivers/misc/mei/hbm.h
+++ b/drivers/misc/mei/hbm.h
@@ -49,6 +49,7 @@ static inline void mei_hbm_hdr(struct mei_msg_hdr *hdr, size_t length)
 	hdr->reserved = 0;
 }
 
+void mei_hbm_idle(struct mei_device *dev);
 int mei_hbm_start_req(struct mei_device *dev);
 int mei_hbm_start_wait(struct mei_device *dev);
 int mei_hbm_cl_flow_control_req(struct mei_device *dev, struct mei_cl *cl);
diff --git a/drivers/misc/mei/init.c b/drivers/misc/mei/init.c
index 71cd209..2af08bf 100644
--- a/drivers/misc/mei/init.c
+++ b/drivers/misc/mei/init.c
@@ -129,14 +129,19 @@ void mei_reset(struct mei_device *dev, int interrupts_enabled)
 		dev_warn(&dev->pdev->dev, "unexpected reset: dev_state = %s\n",
 			 mei_dev_state_str(dev->dev_state));
 
+	/* we're already in reset, cancel the init timer
+	 * if the reset was called due the hbm protocol error
+	 * we need to call it before hw start
+	 * so the hbm watchdog won't kick in
+	 */
+	mei_hbm_idle(dev);
+
 	ret = mei_hw_reset(dev, interrupts_enabled);
 	if (ret) {
 		dev_err(&dev->pdev->dev, "hw reset failed disabling the device\n");
 		interrupts_enabled = false;
-		dev->dev_state = MEI_DEV_DISABLED;
 	}
 
-	dev->hbm_state = MEI_HBM_IDLE;
 
 	if (dev->dev_state != MEI_DEV_INITIALIZING &&
 	    dev->dev_state != MEI_DEV_POWER_UP) {
@@ -161,8 +166,6 @@ void mei_reset(struct mei_device *dev, int interrupts_enabled)
 		memset(&dev->wr_ext_msg, 0, sizeof(dev->wr_ext_msg));
 	}
 
-	/* we're already in reset, cancel the init timer */
-	dev->init_clients_timer = 0;
 
 	dev->me_clients_num = 0;
 	dev->rd_msg_hdr = 0;
@@ -170,6 +173,7 @@ void mei_reset(struct mei_device *dev, int interrupts_enabled)
 
 	if (!interrupts_enabled) {
 		dev_dbg(&dev->pdev->dev, "intr not enabled end of reset\n");
+		dev->dev_state = MEI_DEV_DISABLED;
 		return;
 	}
 
diff --git a/drivers/misc/mei/interrupt.c b/drivers/misc/mei/interrupt.c
index bbb61be..206dbe9 100644
--- a/drivers/misc/mei/interrupt.c
+++ b/drivers/misc/mei/interrupt.c
@@ -537,7 +537,6 @@ EXPORT_SYMBOL_GPL(mei_irq_write_handler);
  *
  * @work: pointer to the work_struct structure
  *
- * NOTE: This function is called by timer interrupt work
  */
 void mei_timer(struct work_struct *work)
 {
@@ -552,18 +551,24 @@ void mei_timer(struct work_struct *work)
 
 
 	mutex_lock(&dev->device_lock);
-	if (dev->dev_state != MEI_DEV_ENABLED) {
-		if (dev->dev_state == MEI_DEV_INIT_CLIENTS) {
-			if (dev->init_clients_timer) {
-				if (--dev->init_clients_timer == 0) {
-					dev_err(&dev->pdev->dev, "reset: init clients timeout hbm_state = %d.\n",
-						dev->hbm_state);
-					mei_reset(dev, 1);
-				}
+
+	/* Catch interrupt stalls during HBM init handshake */
+	if (dev->dev_state == MEI_DEV_INIT_CLIENTS &&
+	    dev->hbm_state != MEI_HBM_IDLE) {
+
+		if (dev->init_clients_timer) {
+			if (--dev->init_clients_timer == 0) {
+				dev_err(&dev->pdev->dev, "timer: init clients timeout hbm_state = %d.\n",
+					dev->hbm_state);
+				mei_reset(dev, 1);
+				goto out;
 			}
 		}
-		goto out;
 	}
+
+	if (dev->dev_state != MEI_DEV_ENABLED)
+		goto out;
+
 	/*** connect/disconnect timeouts ***/
 	list_for_each_entry_safe(cl_pos, cl_next, &dev->file_list, link) {
 		if (cl_pos->timer_count) {
-- 
1.8.4.2

[char-misc 3/4] mei: revamp mei reset state machine

[Tomas Winkler]

1. MEI_DEV_RESETTING device state spans only hardware reset flow
while starting dev state is saved into a local variable for further
reference, this let us to reduce big if statements in case we
are trying to avoid nested resets

2. During initializations if the reset ended in MEI_DEV_DISABLED device
state we bail out with -ENODEV

3. Remove redundant interrupts_enabled parameter as this
 can be deduced from the starting dev_state

4. mei_reset propagates error code to the caller

5. Add mei_restart function to wrap the pci resume

Cc: <stable@vger.kernel.org> 
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
---
 drivers/misc/mei/hbm.c       |   2 +-
 drivers/misc/mei/hw-me.c     |  10 +--
 drivers/misc/mei/init.c      | 210 ++++++++++++++++++++++++++-----------------
 drivers/misc/mei/interrupt.c |  13 +--
 drivers/misc/mei/mei_dev.h   |   3 +-
 drivers/misc/mei/pci-me.c    |  10 +--
 6 files changed, 143 insertions(+), 105 deletions(-)

diff --git a/drivers/misc/mei/hbm.c b/drivers/misc/mei/hbm.c
index 836f92d..32f2adf 100644
--- a/drivers/misc/mei/hbm.c
+++ b/drivers/misc/mei/hbm.c
@@ -763,7 +763,7 @@ int mei_hbm_dispatch(struct mei_device *dev, struct mei_msg_hdr *hdr)
 			return -EPROTO;
 		}
 
-		dev->dev_state = MEI_DEV_DISABLED;
+		dev->dev_state = MEI_DEV_POWER_DOWN;
 		dev_info(&dev->pdev->dev, "hbm: stop response: resetting.\n");
 		/* force the reset */
 		return -EPROTO;
diff --git a/drivers/misc/mei/hw-me.c b/drivers/misc/mei/hw-me.c
index 6c07623..6f656c0 100644
--- a/drivers/misc/mei/hw-me.c
+++ b/drivers/misc/mei/hw-me.c
@@ -185,7 +185,7 @@ static int mei_me_hw_reset(struct mei_device *dev, bool intr_enable)
 
 	mei_me_reg_write(hw, H_CSR, hcsr);
 
-	if (dev->dev_state == MEI_DEV_POWER_DOWN)
+	if (intr_enable == false)
 		mei_me_hw_reset_release(dev);
 
 	return 0;
@@ -482,11 +482,7 @@ irqreturn_t mei_me_irq_thread_handler(int irq, void *dev_id)
 		mei_clear_interrupts(dev);
 
 	/* check if ME wants a reset */
-	if (!mei_hw_is_ready(dev) &&
-	    dev->dev_state != MEI_DEV_RESETTING &&
-	    dev->dev_state != MEI_DEV_INITIALIZING &&
-	    dev->dev_state != MEI_DEV_POWER_DOWN &&
-	    dev->dev_state != MEI_DEV_POWER_UP) {
+	if (!mei_hw_is_ready(dev) && dev->dev_state != MEI_DEV_RESETTING) {
 		dev_warn(&dev->pdev->dev, "FW not ready: resetting.\n");
 		schedule_work(&dev->reset_work);
 		goto end;
@@ -514,7 +510,7 @@ irqreturn_t mei_me_irq_thread_handler(int irq, void *dev_id)
 			break;
 		dev_dbg(&dev->pdev->dev, "slots to read = %08x\n", slots);
 		rets = mei_irq_read_handler(dev, &complete_list, &slots);
-		if (rets) {
+		if (rets && dev->dev_state != MEI_DEV_RESETTING) {
 			schedule_work(&dev->reset_work);
 			goto end;
 		}
diff --git a/drivers/misc/mei/init.c b/drivers/misc/mei/init.c
index 2af08bf..059133d 100644
--- a/drivers/misc/mei/init.c
+++ b/drivers/misc/mei/init.c
@@ -43,62 +43,13 @@ const char *mei_dev_state_str(int state)
 #undef MEI_DEV_STATE
 }
 
-/**
- * mei_start - initializes host and fw to start work.
- *
- * @dev: the device structure
- *
- * returns 0 on success, <0 on failure.
- */
-int mei_start(struct mei_device *dev)
-{
-	mutex_lock(&dev->device_lock);
-
-	/* acknowledge interrupt and stop interupts */
-	mei_clear_interrupts(dev);
-
-	mei_hw_config(dev);
-
-	dev_dbg(&dev->pdev->dev, "reset in start the mei device.\n");
-
-	mei_reset(dev, 1);
-
-	if (mei_hbm_start_wait(dev)) {
-		dev_err(&dev->pdev->dev, "HBM haven't started");
-		goto err;
-	}
-
-	if (!mei_host_is_ready(dev)) {
-		dev_err(&dev->pdev->dev, "host is not ready.\n");
-		goto err;
-	}
-
-	if (!mei_hw_is_ready(dev)) {
-		dev_err(&dev->pdev->dev, "ME is not ready.\n");
-		goto err;
-	}
-
-	if (!mei_hbm_version_is_supported(dev)) {
-		dev_dbg(&dev->pdev->dev, "MEI start failed.\n");
-		goto err;
-	}
-
-	dev_dbg(&dev->pdev->dev, "link layer has been established.\n");
-
-	mutex_unlock(&dev->device_lock);
-	return 0;
-err:
-	dev_err(&dev->pdev->dev, "link layer initialization failed.\n");
-	dev->dev_state = MEI_DEV_DISABLED;
-	mutex_unlock(&dev->device_lock);
-	return -ENODEV;
-}
-EXPORT_SYMBOL_GPL(mei_start);
 
 /**
  * mei_cancel_work. Cancel mei background jobs
  *
  * @dev: the device structure
+ *
+ * returns 0 on success or < 0 if the reset hasn't succeeded
  */
 void mei_cancel_work(struct mei_device *dev)
 {
@@ -113,21 +64,19 @@ EXPORT_SYMBOL_GPL(mei_cancel_work);
  * mei_reset - resets host and fw.
  *
  * @dev: the device structure
- * @interrupts_enabled: if interrupt should be enabled after reset.
  */
-void mei_reset(struct mei_device *dev, int interrupts_enabled)
+int mei_reset(struct mei_device *dev)
 {
-	bool unexpected;
+	enum mei_dev_state state = dev->dev_state;
+	bool interrupts_enabled;
 	int ret;
 
-	unexpected = (dev->dev_state != MEI_DEV_INITIALIZING &&
-			dev->dev_state != MEI_DEV_DISABLED &&
-			dev->dev_state != MEI_DEV_POWER_DOWN &&
-			dev->dev_state != MEI_DEV_POWER_UP);
-
-	if (unexpected)
+	if (state != MEI_DEV_INITIALIZING &&
+	    state != MEI_DEV_DISABLED &&
+	    state != MEI_DEV_POWER_DOWN &&
+	    state != MEI_DEV_POWER_UP)
 		dev_warn(&dev->pdev->dev, "unexpected reset: dev_state = %s\n",
-			 mei_dev_state_str(dev->dev_state));
+			 mei_dev_state_str(state));
 
 	/* we're already in reset, cancel the init timer
 	 * if the reset was called due the hbm protocol error
@@ -136,26 +85,23 @@ void mei_reset(struct mei_device *dev, int interrupts_enabled)
 	 */
 	mei_hbm_idle(dev);
 
-	ret = mei_hw_reset(dev, interrupts_enabled);
-	if (ret) {
-		dev_err(&dev->pdev->dev, "hw reset failed disabling the device\n");
-		interrupts_enabled = false;
-	}
-
+	/* enter reset flow */
+	interrupts_enabled = state != MEI_DEV_POWER_DOWN;
+	dev->dev_state = MEI_DEV_RESETTING;
 
-	if (dev->dev_state != MEI_DEV_INITIALIZING &&
-	    dev->dev_state != MEI_DEV_POWER_UP) {
+	ret = mei_hw_reset(dev, interrupts_enabled);
+	/* fall through and remove the sw state even if hw reset has failed */
 
-		if (dev->dev_state != MEI_DEV_DISABLED &&
-		    dev->dev_state != MEI_DEV_POWER_DOWN)
-			dev->dev_state = MEI_DEV_RESETTING;
+	/* no need to clean up software state in case of power up */
+	if (state != MEI_DEV_INITIALIZING &&
+	    state != MEI_DEV_POWER_UP) {
 
 		/* remove all waiting requests */
 		mei_cl_all_write_clear(dev);
 
 		mei_cl_all_disconnect(dev);
 
-		/* wake up all readings so they can be interrupted */
+		/* wake up all readers and writers so they can be interrupted */
 		mei_cl_all_wakeup(dev);
 
 		/* remove entry if already in list */
@@ -171,33 +117,126 @@ void mei_reset(struct mei_device *dev, int interrupts_enabled)
 	dev->rd_msg_hdr = 0;
 	dev->wd_pending = false;
 
-	if (!interrupts_enabled) {
-		dev_dbg(&dev->pdev->dev, "intr not enabled end of reset\n");
+	if (ret) {
+		dev_err(&dev->pdev->dev, "hw_reset failed ret = %d\n", ret);
 		dev->dev_state = MEI_DEV_DISABLED;
-		return;
+		return ret;
+	}
+
+	if (state == MEI_DEV_POWER_DOWN) {
+		dev_dbg(&dev->pdev->dev, "powering down: end of reset\n");
+		dev->dev_state = MEI_DEV_DISABLED;
+		return 0;
 	}
 
 	ret = mei_hw_start(dev);
 	if (ret) {
-		dev_err(&dev->pdev->dev, "hw_start failed disabling the device\n");
+		dev_err(&dev->pdev->dev, "hw_start failed ret = %d\n", ret);
 		dev->dev_state = MEI_DEV_DISABLED;
-		return;
+		return ret;
 	}
 
 	dev_dbg(&dev->pdev->dev, "link is established start sending messages.\n");
-	/* link is established * start sending messages.  */
 
 	dev->dev_state = MEI_DEV_INIT_CLIENTS;
-
 	ret = mei_hbm_start_req(dev);
 	if (ret) {
-		dev_err(&dev->pdev->dev, "hbm_start failed disabling the device\n");
+		dev_err(&dev->pdev->dev, "hbm_start failed ret = %d\n", ret);
 		dev->dev_state = MEI_DEV_DISABLED;
-		return;
+		return ret;
 	}
+
+	return 0;
 }
 EXPORT_SYMBOL_GPL(mei_reset);
 
+/**
+ * mei_start - initializes host and fw to start work.
+ *
+ * @dev: the device structure
+ *
+ * returns 0 on success, <0 on failure.
+ */
+int mei_start(struct mei_device *dev)
+{
+	mutex_lock(&dev->device_lock);
+
+	/* acknowledge interrupt and stop interrupts */
+	mei_clear_interrupts(dev);
+
+	mei_hw_config(dev);
+
+	dev_dbg(&dev->pdev->dev, "reset in start the mei device.\n");
+
+	dev->dev_state = MEI_DEV_INITIALIZING;
+	mei_reset(dev);
+
+	if (dev->dev_state == MEI_DEV_DISABLED) {
+		dev_err(&dev->pdev->dev, "reset failed");
+		goto err;
+	}
+
+	if (mei_hbm_start_wait(dev)) {
+		dev_err(&dev->pdev->dev, "HBM haven't started");
+		goto err;
+	}
+
+	if (!mei_host_is_ready(dev)) {
+		dev_err(&dev->pdev->dev, "host is not ready.\n");
+		goto err;
+	}
+
+	if (!mei_hw_is_ready(dev)) {
+		dev_err(&dev->pdev->dev, "ME is not ready.\n");
+		goto err;
+	}
+
+	if (!mei_hbm_version_is_supported(dev)) {
+		dev_dbg(&dev->pdev->dev, "MEI start failed.\n");
+		goto err;
+	}
+
+	dev_dbg(&dev->pdev->dev, "link layer has been established.\n");
+
+	mutex_unlock(&dev->device_lock);
+	return 0;
+err:
+	dev_err(&dev->pdev->dev, "link layer initialization failed.\n");
+	dev->dev_state = MEI_DEV_DISABLED;
+	mutex_unlock(&dev->device_lock);
+	return -ENODEV;
+}
+EXPORT_SYMBOL_GPL(mei_start);
+
+/**
+ * mei_restart - restart device after suspend
+ *
+ * @dev: the device structure
+ *
+ * returns 0 on success or -ENODEV if the restart hasn't succeeded
+ */
+int mei_restart(struct mei_device *dev)
+{
+	int err;
+
+	mutex_lock(&dev->device_lock);
+
+	mei_clear_interrupts(dev);
+
+	dev->dev_state = MEI_DEV_POWER_UP;
+
+	err = mei_reset(dev);
+
+	mutex_unlock(&dev->device_lock);
+
+	if (err || dev->dev_state == MEI_DEV_DISABLED)
+		return -ENODEV;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(mei_restart);
+
+
 static void mei_reset_work(struct work_struct *work)
 {
 	struct mei_device *dev =
@@ -205,9 +244,12 @@ static void mei_reset_work(struct work_struct *work)
 
 	mutex_lock(&dev->device_lock);
 
-	mei_reset(dev, true);
+	mei_reset(dev);
 
 	mutex_unlock(&dev->device_lock);
+
+	if (dev->dev_state == MEI_DEV_DISABLED)
+		dev_err(&dev->pdev->dev, "reset failed");
 }
 
 void mei_stop(struct mei_device *dev)
@@ -223,7 +265,7 @@ void mei_stop(struct mei_device *dev)
 	mei_wd_stop(dev);
 
 	dev->dev_state = MEI_DEV_POWER_DOWN;
-	mei_reset(dev, 0);
+	mei_reset(dev);
 
 	mutex_unlock(&dev->device_lock);
 
diff --git a/drivers/misc/mei/interrupt.c b/drivers/misc/mei/interrupt.c
index 206dbe9..b5ceba4 100644
--- a/drivers/misc/mei/interrupt.c
+++ b/drivers/misc/mei/interrupt.c
@@ -560,7 +560,7 @@ void mei_timer(struct work_struct *work)
 			if (--dev->init_clients_timer == 0) {
 				dev_err(&dev->pdev->dev, "timer: init clients timeout hbm_state = %d.\n",
 					dev->hbm_state);
-				mei_reset(dev, 1);
+				mei_reset(dev);
 				goto out;
 			}
 		}
@@ -573,8 +573,8 @@ void mei_timer(struct work_struct *work)
 	list_for_each_entry_safe(cl_pos, cl_next, &dev->file_list, link) {
 		if (cl_pos->timer_count) {
 			if (--cl_pos->timer_count == 0) {
-				dev_err(&dev->pdev->dev, "reset: connect/disconnect timeout.\n");
-				mei_reset(dev, 1);
+				dev_err(&dev->pdev->dev, "timer: connect/disconnect timeout.\n");
+				mei_reset(dev);
 				goto out;
 			}
 		}
@@ -582,8 +582,8 @@ void mei_timer(struct work_struct *work)
 
 	if (dev->iamthif_stall_timer) {
 		if (--dev->iamthif_stall_timer == 0) {
-			dev_err(&dev->pdev->dev, "reset: amthif  hanged.\n");
-			mei_reset(dev, 1);
+			dev_err(&dev->pdev->dev, "timer: amthif  hanged.\n");
+			mei_reset(dev);
 			dev->iamthif_msg_buf_size = 0;
 			dev->iamthif_msg_buf_index = 0;
 			dev->iamthif_canceled = false;
@@ -636,7 +636,8 @@ void mei_timer(struct work_struct *work)
 		}
 	}
 out:
-	schedule_delayed_work(&dev->timer_work, 2 * HZ);
+	if (dev->dev_state != MEI_DEV_DISABLED)
+		schedule_delayed_work(&dev->timer_work, 2 * HZ);
 	mutex_unlock(&dev->device_lock);
 }
 
diff --git a/drivers/misc/mei/mei_dev.h b/drivers/misc/mei/mei_dev.h
index 3f242e1..fa45a58 100644
--- a/drivers/misc/mei/mei_dev.h
+++ b/drivers/misc/mei/mei_dev.h
@@ -472,8 +472,9 @@ static inline u32 mei_slots2data(int slots)
  * mei init function prototypes
  */
 void mei_device_init(struct mei_device *dev);
-void mei_reset(struct mei_device *dev, int interrupts);
+int mei_reset(struct mei_device *dev);
 int mei_start(struct mei_device *dev);
+int mei_restart(struct mei_device *dev);
 void mei_stop(struct mei_device *dev);
 void mei_cancel_work(struct mei_device *dev);
 
diff --git a/drivers/misc/mei/pci-me.c b/drivers/misc/mei/pci-me.c
index 8b1deea..ddadd08 100644
--- a/drivers/misc/mei/pci-me.c
+++ b/drivers/misc/mei/pci-me.c
@@ -321,16 +321,14 @@ static int mei_me_pci_resume(struct device *device)
 		return err;
 	}
 
-	mutex_lock(&dev->device_lock);
-	dev->dev_state = MEI_DEV_POWER_UP;
-	mei_clear_interrupts(dev);
-	mei_reset(dev, 1);
-	mutex_unlock(&dev->device_lock);
+	err = mei_restart(dev);
+	if (err)
+		return err;
 
 	/* Start timer if stopped in suspend */
 	schedule_delayed_work(&dev->timer_work, HZ);
 
-	return err;
+	return 0;
 }
 static SIMPLE_DEV_PM_OPS(mei_me_pm_ops, mei_me_pci_suspend, mei_me_pci_resume);
 #define MEI_ME_PM_OPS	(&mei_me_pm_ops)
-- 
1.8.4.2

[char-misc 4/4] mei: limit the number of consecutive resets

[Tomas Winkler]

give up reseting after 3 unsuccessful tries

Cc: <stable@vger.kernel.org> 
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
---
 drivers/misc/mei/client.c  |  1 +
 drivers/misc/mei/init.c    | 10 ++++++++++
 drivers/misc/mei/mei_dev.h |  7 +++++++
 3 files changed, 18 insertions(+)

diff --git a/drivers/misc/mei/client.c b/drivers/misc/mei/client.c
index 585f46b..9b8975b 100644
--- a/drivers/misc/mei/client.c
+++ b/drivers/misc/mei/client.c
@@ -370,6 +370,7 @@ void mei_host_client_init(struct work_struct *work)
 	}
 
 	dev->dev_state = MEI_DEV_ENABLED;
+	dev->reset_count = 0;
 
 	mutex_unlock(&dev->device_lock);
 }
diff --git a/drivers/misc/mei/init.c b/drivers/misc/mei/init.c
index 059133d..cdd31c2 100644
--- a/drivers/misc/mei/init.c
+++ b/drivers/misc/mei/init.c
@@ -89,6 +89,13 @@ int mei_reset(struct mei_device *dev)
 	interrupts_enabled = state != MEI_DEV_POWER_DOWN;
 	dev->dev_state = MEI_DEV_RESETTING;
 
+	dev->reset_count++;
+	if (dev->reset_count > MEI_MAX_CONSEC_RESET) {
+		dev_err(&dev->pdev->dev, "reset: reached maximal consecutive resets: disabling the device\n");
+		dev->dev_state = MEI_DEV_DISABLED;
+		return -ENODEV;
+	}
+
 	ret = mei_hw_reset(dev, interrupts_enabled);
 	/* fall through and remove the sw state even if hw reset has failed */
 
@@ -169,6 +176,7 @@ int mei_start(struct mei_device *dev)
 	dev_dbg(&dev->pdev->dev, "reset in start the mei device.\n");
 
 	dev->dev_state = MEI_DEV_INITIALIZING;
+	dev->reset_count = 0;
 	mei_reset(dev);
 
 	if (dev->dev_state == MEI_DEV_DISABLED) {
@@ -224,6 +232,7 @@ int mei_restart(struct mei_device *dev)
 	mei_clear_interrupts(dev);
 
 	dev->dev_state = MEI_DEV_POWER_UP;
+	dev->reset_count = 0;
 
 	err = mei_reset(dev);
 
@@ -285,6 +294,7 @@ void mei_device_init(struct mei_device *dev)
 	init_waitqueue_head(&dev->wait_recvd_msg);
 	init_waitqueue_head(&dev->wait_stop_wd);
 	dev->dev_state = MEI_DEV_INITIALIZING;
+	dev->reset_count = 0;
 
 	mei_io_list_init(&dev->read_list);
 	mei_io_list_init(&dev->write_list);
diff --git a/drivers/misc/mei/mei_dev.h b/drivers/misc/mei/mei_dev.h
index fa45a58..d3f4c5f 100644
--- a/drivers/misc/mei/mei_dev.h
+++ b/drivers/misc/mei/mei_dev.h
@@ -61,6 +61,11 @@ extern const uuid_le mei_wd_guid;
 #define MEI_CLIENTS_MAX 256
 
 /*
+ * maximum number of consecutive resets
+ */
+#define MEI_MAX_CONSEC_RESET  3
+
+/*
  * Number of File descriptors/handles
  * that can be opened to the driver.
  *
@@ -327,6 +332,7 @@ struct mei_cl_device {
 /**
  * struct mei_device -  MEI private device struct
 
+ * @reset_count - limits the number of consecutive resets
  * @hbm_state - state of host bus message protocol
  * @mem_addr - mem mapped base register address
 
@@ -370,6 +376,7 @@ struct mei_device {
 	/*
 	 * mei device  states
 	 */
+	unsigned long reset_count;
 	enum mei_dev_state dev_state;
 	enum mei_hbm_state hbm_state;
 	u16 init_clients_timer;
-- 
1.8.4.2

Re: [char-misc 1/4] mei: do not run reset flow from the interrupt thread

[Greg KH]

On Wed, Jan 08, 2014 at 08:19:21PM +0200, Tomas Winkler wrote:
> This fixes a potential deadlock in case of a firmware
> initiated reset
> 
> mei_reset has a dialog with the interrupt thread hence
> it has to be run from an another work item
> 
> Most of the mei_resets were called from mei_hbm_dispatch
> which is called in interrupt thread context so this
> function underwent major revamp. The error code is
> propagated to the interrupt thread and if needed
> the reset is scheduled from there.
> 
> Cc: <stable@vger.kernel.org> 
> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
> ---
>  drivers/misc/mei/hbm.c       | 200 +++++++++++++++++++++++++++----------------
>  drivers/misc/mei/hbm.h       |   6 +-
>  drivers/misc/mei/hw-me.c     |  32 ++++---
>  drivers/misc/mei/init.c      | 100 +++++++++++++---------
>  drivers/misc/mei/interrupt.c |   9 +-
>  drivers/misc/mei/mei_dev.h   |   1 +
>  6 files changed, 210 insertions(+), 138 deletions(-)

This is a _really_ big patch for a stable kernel tree.  Are you sure
it's needed there?  Please go read Documentation/stable_kernel_rules.txt
again.

greg k-h

Re: [char-misc 3/4] mei: revamp mei reset state machine

[Greg KH]

On Wed, Jan 08, 2014 at 08:19:23PM +0200, Tomas Winkler wrote:
> 1. MEI_DEV_RESETTING device state spans only hardware reset flow
> while starting dev state is saved into a local variable for further
> reference, this let us to reduce big if statements in case we
> are trying to avoid nested resets
> 
> 2. During initializations if the reset ended in MEI_DEV_DISABLED device
> state we bail out with -ENODEV
> 
> 3. Remove redundant interrupts_enabled parameter as this
>  can be deduced from the starting dev_state
> 
> 4. mei_reset propagates error code to the caller
> 
> 5. Add mei_restart function to wrap the pci resume
> 
> Cc: <stable@vger.kernel.org> 
> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
> ---
>  drivers/misc/mei/hbm.c       |   2 +-
>  drivers/misc/mei/hw-me.c     |  10 +--
>  drivers/misc/mei/init.c      | 210 ++++++++++++++++++++++++++-----------------
>  drivers/misc/mei/interrupt.c |  13 +--
>  drivers/misc/mei/mei_dev.h   |   3 +-
>  drivers/misc/mei/pci-me.c    |  10 +--
>  6 files changed, 143 insertions(+), 105 deletions(-)

Again, way too big for stable kernels, sorry.

greg k-h

Re: [char-misc 3/4] mei: revamp mei reset state machine

[Greg KH]

On Wed, Jan 08, 2014 at 08:19:23PM +0200, Tomas Winkler wrote:
> 1. MEI_DEV_RESETTING device state spans only hardware reset flow
> while starting dev state is saved into a local variable for further
> reference, this let us to reduce big if statements in case we
> are trying to avoid nested resets
> 
> 2. During initializations if the reset ended in MEI_DEV_DISABLED device
> state we bail out with -ENODEV
> 
> 3. Remove redundant interrupts_enabled parameter as this
>  can be deduced from the starting dev_state
> 
> 4. mei_reset propagates error code to the caller
> 
> 5. Add mei_restart function to wrap the pci resume
> 
> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
> ---
>  drivers/misc/mei/hbm.c       |   2 +-
>  drivers/misc/mei/hw-me.c     |  10 +--
>  drivers/misc/mei/init.c      | 210 ++++++++++++++++++++++++++-----------------
>  drivers/misc/mei/interrupt.c |  13 +--
>  drivers/misc/mei/mei_dev.h   |   3 +-
>  drivers/misc/mei/pci-me.c    |  10 +--
>  6 files changed, 143 insertions(+), 105 deletions(-)

This patch fails to apply to my tree:

checking file drivers/misc/mei/hbm.c
checking file drivers/misc/mei/hw-me.c
checking file drivers/misc/mei/init.c
Hunk #3 FAILED at 85.
Hunk #4 succeeded at 119 (offset -1 lines).
Hunk #5 succeeded at 246 (offset -1 lines).
Hunk #6 succeeded at 267 (offset -1 lines).
1 out of 6 hunks FAILED
checking file drivers/misc/mei/interrupt.c
checking file drivers/misc/mei/mei_dev.h
checking file drivers/misc/mei/pci-me.c

Care to fix it up and resend?

thanks,

greg k-h

Re: [char-misc 4/4] mei: limit the number of consecutive resets

[Greg KH]

On Wed, Jan 08, 2014 at 08:19:24PM +0200, Tomas Winkler wrote:
> give up reseting after 3 unsuccessful tries
> 
> Cc: <stable@vger.kernel.org> 
> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
> ---
>  drivers/misc/mei/client.c  |  1 +
>  drivers/misc/mei/init.c    | 10 ++++++++++
>  drivers/misc/mei/mei_dev.h |  7 +++++++
>  3 files changed, 18 insertions(+)

Doesn't apply as patch 3/4 didn't apply.

Which means it shouldn't be for a stable kernel either...

greg k-h

RE: [char-misc 3/4] mei: revamp mei reset state machine

[Winkler, Tomas]

> On Wed, Jan 08, 2014 at 08:19:23PM +0200, Tomas Winkler wrote:
> > 1. MEI_DEV_RESETTING device state spans only hardware reset flow
> > while starting dev state is saved into a local variable for further
> > reference, this let us to reduce big if statements in case we
> > are trying to avoid nested resets
> >
> > 2. During initializations if the reset ended in MEI_DEV_DISABLED device
> > state we bail out with -ENODEV
> >
> > 3. Remove redundant interrupts_enabled parameter as this
> >  can be deduced from the starting dev_state
> >
> > 4. mei_reset propagates error code to the caller
> >
> > 5. Add mei_restart function to wrap the pci resume
> >
> > Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> > Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
> > ---
> >  drivers/misc/mei/hbm.c       |   2 +-
> >  drivers/misc/mei/hw-me.c     |  10 +--
> >  drivers/misc/mei/init.c      | 210 ++++++++++++++++++++++++++-----------------
> >  drivers/misc/mei/interrupt.c |  13 +--
> >  drivers/misc/mei/mei_dev.h   |   3 +-
> >  drivers/misc/mei/pci-me.c    |  10 +--
> >  6 files changed, 143 insertions(+), 105 deletions(-)
> 
> This patch fails to apply to my tree:

Hmm, this is due to the blank line that was dropped (https://lkml.org/lkml/2013/7/29/668) , forgot to adjust it in my tree. 

> 
> Care to fix it up and resend?

On the way.

RE: [char-misc 1/4] mei: do not run reset flow from the interrupt thread

[Winkler, Tomas]

> 
> This is a _really_ big patch for a stable kernel tree.  Are you sure
> it's needed there?  Please go read Documentation/stable_kernel_rules.txt
> again.

I understand it's big though currently I do not have significantly simpler fix for the issue, all 4 patches are needed to make the reset issue  go away.
 
Thanks
Tomas

Re: [char-misc 1/4] mei: do not run reset flow from the interrupt thread

[Greg KH]

On Sat, Jan 11, 2014 at 10:46:14PM +0000, Winkler, Tomas wrote:
> 
> 
> > 
> > This is a _really_ big patch for a stable kernel tree.  Are you sure
> > it's needed there?  Please go read Documentation/stable_kernel_rules.txt
> > again.
> 
> I understand it's big though currently I do not have significantly
> simpler fix for the issue, all 4 patches are needed to make the reset
> issue  go away.

What is the "reset issue"?  Who is it affecting?  What is the problem?
Is there a work-around without this patchset?

As it is, I can't take something this big for the stable tree, sorry.

greg k-h

RE: [char-misc 1/4] mei: do not run reset flow from the interrupt thread

[Winkler, Tomas]

> On Sat, Jan 11, 2014 at 10:46:14PM +0000, Winkler, Tomas wrote:
> >
> >
> > >
> > > This is a _really_ big patch for a stable kernel tree.  Are you sure
> > > it's needed there?  Please go read Documentation/stable_kernel_rules.txt
> > > again.
> >
> > I understand it's big though currently I do not have significantly
> > simpler fix for the issue, all 4 patches are needed to make the reset
> > issue  go away.
> 
> What is the "reset issue"?  Who is it affecting?  What is the problem?

Reported here: 

https://lkml.org/lkml/2013/12/22/102
https://lkml.org/lkml/2013/11/1/190
https://bugzilla.novell.com/show_bug.cgi?id=821619

maybe few more other places, I've already tried to address the issue but it looks I didn't hit the nail till now.. 

> Is there a work-around without this patchset?

Disabling timer_work has same effect 

https://lkml.org/lkml/2013/11/7/179


but  I haven't checked fi it does have another side effects mostly in AMTHI. 

> 
> As it is, I can't take something this big for the stable tree, sorry.
> 

Understood, I will try to find smaller fix to target stable.

Thanks
Tomas