lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <1389205164-13553-4-git-send-email-tomas.winkler@intel.com>
Date:	Wed,  8 Jan 2014 20:19:23 +0200
From:	Tomas Winkler <tomas.winkler@...el.com>
To:	gregkh@...uxfoundation.org
Cc:	arnd@...db.de, linux-kernel@...r.kernel.org,
	stable@...r.kernel.org, Tomas Winkler <tomas.winkler@...el.com>,
	Alexander Usyskin <alexander.usyskin@...el.com>
Subject: [char-misc 3/4] mei: revamp mei reset state machine

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@...r.kernel.org> 
Signed-off-by: Tomas Winkler <tomas.winkler@...el.com>
Signed-off-by: Alexander Usyskin <alexander.usyskin@...el.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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ