[char-misc-next 1/2] mei: synchronize irq before initiating a reset.

[char-misc-next 1/2] mei: synchronize irq before initiating a reset.

[Tomas Winkler]

We need to synchronize irqs before issuing reset to make sure that the
clients communication is concluded and doesn't leak to the reset flow
and confusing the state machine.

This issue is happening during suspend/resume stress testing.

Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/misc/mei/hw-me.c   | 13 +++++++++++++
 drivers/misc/mei/hw-txe.c  | 15 ++++++++++++++-
 drivers/misc/mei/init.c    |  6 ++++--
 drivers/misc/mei/mei_dev.h |  8 +++++++-
 4 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/drivers/misc/mei/hw-me.c b/drivers/misc/mei/hw-me.c
index 998f7fc0e920..e2b56e8cf745 100644
--- a/drivers/misc/mei/hw-me.c
+++ b/drivers/misc/mei/hw-me.c
@@ -285,6 +285,18 @@ static void mei_me_intr_disable(struct mei_device *dev)
 }
 
 /**
+ * mei_me_synchronize_irq - wait for pending IRQ handlers
+ *
+ * @dev: the device structure
+ */
+static void mei_me_synchronize_irq(struct mei_device *dev)
+{
+	struct pci_dev *pdev = to_pci_dev(dev->dev);
+
+	synchronize_irq(pdev->irq);
+}
+
+/**
  * mei_me_hw_reset_release - release device from the reset
  *
  * @dev: the device structure
@@ -1238,6 +1250,7 @@ static const struct mei_hw_ops mei_me_hw_ops = {
 	.intr_clear = mei_me_intr_clear,
 	.intr_enable = mei_me_intr_enable,
 	.intr_disable = mei_me_intr_disable,
+	.synchronize_irq = mei_me_synchronize_irq,
 
 	.hbuf_free_slots = mei_me_hbuf_empty_slots,
 	.hbuf_is_ready = mei_me_hbuf_is_empty,
diff --git a/drivers/misc/mei/hw-txe.c b/drivers/misc/mei/hw-txe.c
index b7a2f622f23c..e9f8c0aeec13 100644
--- a/drivers/misc/mei/hw-txe.c
+++ b/drivers/misc/mei/hw-txe.c
@@ -19,7 +19,7 @@
 #include <linux/ktime.h>
 #include <linux/delay.h>
 #include <linux/kthread.h>
-#include <linux/irqreturn.h>
+#include <linux/interrupt.h>
 #include <linux/pm_runtime.h>
 
 #include <linux/mei.h>
@@ -441,6 +441,18 @@ static void mei_txe_intr_enable(struct mei_device *dev)
 }
 
 /**
+ * mei_txe_synchronize_irq - wait for pending IRQ handlers
+ *
+ * @dev: the device structure
+ */
+static void mei_txe_synchronize_irq(struct mei_device *dev)
+{
+	struct pci_dev *pdev = to_pci_dev(dev->dev);
+
+	synchronize_irq(pdev->irq);
+}
+
+/**
  * mei_txe_pending_interrupts - check if there are pending interrupts
  *	only Aliveness, Input ready, and output doorbell are of relevance
  *
@@ -1168,6 +1180,7 @@ static const struct mei_hw_ops mei_txe_hw_ops = {
 	.intr_clear = mei_txe_intr_clear,
 	.intr_enable = mei_txe_intr_enable,
 	.intr_disable = mei_txe_intr_disable,
+	.synchronize_irq = mei_txe_synchronize_irq,
 
 	.hbuf_free_slots = mei_txe_hbuf_empty_slots,
 	.hbuf_is_ready = mei_txe_is_input_ready,
diff --git a/drivers/misc/mei/init.c b/drivers/misc/mei/init.c
index 9a9c2484d107..41e5760a6886 100644
--- a/drivers/misc/mei/init.c
+++ b/drivers/misc/mei/init.c
@@ -122,6 +122,10 @@ int mei_reset(struct mei_device *dev)
 			 mei_dev_state_str(state), fw_sts_str);
 	}
 
+	mei_clear_interrupts(dev);
+
+	mei_synchronize_irq(dev);
+
 	/* 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
@@ -273,8 +277,6 @@ int mei_restart(struct mei_device *dev)
 
 	mutex_lock(&dev->device_lock);
 
-	mei_clear_interrupts(dev);
-
 	dev->dev_state = MEI_DEV_POWER_UP;
 	dev->reset_count = 0;
 
diff --git a/drivers/misc/mei/mei_dev.h b/drivers/misc/mei/mei_dev.h
index f16bd1209848..699693cd8c59 100644
--- a/drivers/misc/mei/mei_dev.h
+++ b/drivers/misc/mei/mei_dev.h
@@ -271,6 +271,7 @@ struct mei_cl {
  * @intr_clear       : clear pending interrupts
  * @intr_enable      : enable interrupts
  * @intr_disable     : disable interrupts
+ * @synchronize_irq  : synchronize irqs
  *
  * @hbuf_free_slots  : query for write buffer empty slots
  * @hbuf_is_ready    : query if write buffer is empty
@@ -292,7 +293,6 @@ struct mei_hw_ops {
 	int (*hw_start)(struct mei_device *dev);
 	void (*hw_config)(struct mei_device *dev);
 
-
 	int (*fw_status)(struct mei_device *dev, struct mei_fw_status *fw_sts);
 	enum mei_pg_state (*pg_state)(struct mei_device *dev);
 	bool (*pg_in_transition)(struct mei_device *dev);
@@ -301,6 +301,7 @@ struct mei_hw_ops {
 	void (*intr_clear)(struct mei_device *dev);
 	void (*intr_enable)(struct mei_device *dev);
 	void (*intr_disable)(struct mei_device *dev);
+	void (*synchronize_irq)(struct mei_device *dev);
 
 	int (*hbuf_free_slots)(struct mei_device *dev);
 	bool (*hbuf_is_ready)(struct mei_device *dev);
@@ -645,6 +646,11 @@ static inline void mei_disable_interrupts(struct mei_device *dev)
 	dev->ops->intr_disable(dev);
 }
 
+static inline void mei_synchronize_irq(struct mei_device *dev)
+{
+	dev->ops->synchronize_irq(dev);
+}
+
 static inline bool mei_host_is_ready(struct mei_device *dev)
 {
 	return dev->ops->host_is_ready(dev);
-- 
2.7.4

[char-misc-next 2/2] mei: fix the back to back interrupt handling

[Tomas Winkler]

From: Alexander Usyskin <alexander.usyskin@intel.com>

Since the newer HW sports two interrupts causes we cannot
just simply acknowledge the interrupts directly in the quick handler
and store the cause in the member variable, as the cause
will be overridden upon next interrupt while the interrupt thread
was not yet scheduled handling the previous interrupt.
The simple fix is to disable interrupts in quick handler
and acknowledge and enabled them in the interrupt thread.

Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/misc/mei/hw-me.c | 67 ++++++++++++++++++++++++++++++++++++------------
 drivers/misc/mei/hw-me.h |  2 --
 2 files changed, 50 insertions(+), 19 deletions(-)

diff --git a/drivers/misc/mei/hw-me.c b/drivers/misc/mei/hw-me.c
index e2b56e8cf745..a05375a3338a 100644
--- a/drivers/misc/mei/hw-me.c
+++ b/drivers/misc/mei/hw-me.c
@@ -246,6 +246,36 @@ static inline enum mei_pg_state mei_me_pg_state(struct mei_device *dev)
 	return hw->pg_state;
 }
 
+static inline u32 me_intr_src(u32 hcsr)
+{
+	return hcsr & H_CSR_IS_MASK;
+}
+
+/**
+ * me_intr_disable - disables mei device interrupts
+ *      using supplied hcsr register value.
+ *
+ * @dev: the device structure
+ * @hcsr: supplied hcsr register value
+ */
+static inline void me_intr_disable(struct mei_device *dev, u32 hcsr)
+{
+	hcsr &= ~H_CSR_IE_MASK;
+	mei_hcsr_set(dev, hcsr);
+}
+
+/**
+ * mei_me_intr_clear - clear and stop interrupts
+ *
+ * @dev: the device structure
+ * @hcsr: supplied hcsr register value
+ */
+static inline void me_intr_clear(struct mei_device *dev, u32 hcsr)
+{
+	if (me_intr_src(hcsr))
+		mei_hcsr_write(dev, hcsr);
+}
+
 /**
  * mei_me_intr_clear - clear and stop interrupts
  *
@@ -255,8 +285,7 @@ static void mei_me_intr_clear(struct mei_device *dev)
 {
 	u32 hcsr = mei_hcsr_read(dev);
 
-	if (hcsr & H_CSR_IS_MASK)
-		mei_hcsr_write(dev, hcsr);
+	me_intr_clear(dev, hcsr);
 }
 /**
  * mei_me_intr_enable - enables mei device interrupts
@@ -280,8 +309,7 @@ static void mei_me_intr_disable(struct mei_device *dev)
 {
 	u32 hcsr = mei_hcsr_read(dev);
 
-	hcsr  &= ~H_CSR_IE_MASK;
-	mei_hcsr_set(dev, hcsr);
+	me_intr_disable(dev, hcsr);
 }
 
 /**
@@ -968,13 +996,14 @@ static void mei_me_pg_legacy_intr(struct mei_device *dev)
  * mei_me_d0i3_intr - perform d0i3 processing in interrupt thread handler
  *
  * @dev: the device structure
+ * @intr_source: interrupt source
  */
-static void mei_me_d0i3_intr(struct mei_device *dev)
+static void mei_me_d0i3_intr(struct mei_device *dev, u32 intr_source)
 {
 	struct mei_me_hw *hw = to_me_hw(dev);
 
 	if (dev->pg_event == MEI_PG_EVENT_INTR_WAIT &&
-	    (hw->intr_source & H_D0I3C_IS)) {
+	    (intr_source & H_D0I3C_IS)) {
 		dev->pg_event = MEI_PG_EVENT_INTR_RECEIVED;
 		if (hw->pg_state == MEI_PG_ON) {
 			hw->pg_state = MEI_PG_OFF;
@@ -993,7 +1022,7 @@ static void mei_me_d0i3_intr(struct mei_device *dev)
 		wake_up(&dev->wait_pg);
 	}
 
-	if (hw->pg_state == MEI_PG_ON && (hw->intr_source & H_IS)) {
+	if (hw->pg_state == MEI_PG_ON && (intr_source & H_IS)) {
 		/*
 		 * HW sent some data and we are in D0i3, so
 		 * we got here because of HW initiated exit from D0i3.
@@ -1008,13 +1037,14 @@ static void mei_me_d0i3_intr(struct mei_device *dev)
  * mei_me_pg_intr - perform pg processing in interrupt thread handler
  *
  * @dev: the device structure
+ * @intr_source: interrupt source
  */
-static void mei_me_pg_intr(struct mei_device *dev)
+static void mei_me_pg_intr(struct mei_device *dev, u32 intr_source)
 {
 	struct mei_me_hw *hw = to_me_hw(dev);
 
 	if (hw->d0i3_supported)
-		mei_me_d0i3_intr(dev);
+		mei_me_d0i3_intr(dev, intr_source);
 	else
 		mei_me_pg_legacy_intr(dev);
 }
@@ -1133,19 +1163,16 @@ static int mei_me_hw_reset(struct mei_device *dev, bool intr_enable)
 irqreturn_t mei_me_irq_quick_handler(int irq, void *dev_id)
 {
 	struct mei_device *dev = (struct mei_device *)dev_id;
-	struct mei_me_hw *hw = to_me_hw(dev);
 	u32 hcsr;
 
 	hcsr = mei_hcsr_read(dev);
-	if (!(hcsr & H_CSR_IS_MASK))
+	if (!me_intr_src(hcsr))
 		return IRQ_NONE;
 
-	hw->intr_source = hcsr & H_CSR_IS_MASK;
-	dev_dbg(dev->dev, "interrupt source 0x%08X.\n", hw->intr_source);
-
-	/* clear H_IS and H_D0I3C_IS bits in H_CSR to clear the interrupts */
-	mei_hcsr_write(dev, hcsr);
+	dev_dbg(dev->dev, "interrupt source 0x%08X\n", me_intr_src(hcsr));
 
+	/* disable interrupts on device */
+	me_intr_disable(dev, hcsr);
 	return IRQ_WAKE_THREAD;
 }
 
@@ -1164,11 +1191,16 @@ 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;
+	u32 hcsr;
 	int rets = 0;
 
 	dev_dbg(dev->dev, "function called after ISR to handle the interrupt processing.\n");
 	/* initialize our complete list */
 	mutex_lock(&dev->device_lock);
+
+	hcsr = mei_hcsr_read(dev);
+	me_intr_clear(dev, hcsr);
+
 	mei_io_list_init(&complete_list);
 
 	/* check if ME wants a reset */
@@ -1178,7 +1210,7 @@ irqreturn_t mei_me_irq_thread_handler(int irq, void *dev_id)
 		goto end;
 	}
 
-	mei_me_pg_intr(dev);
+	mei_me_pg_intr(dev, me_intr_src(hcsr));
 
 	/*  check if we need to start the dev */
 	if (!mei_host_is_ready(dev)) {
@@ -1228,6 +1260,7 @@ irqreturn_t mei_me_irq_thread_handler(int irq, void *dev_id)
 
 end:
 	dev_dbg(dev->dev, "interrupt thread end ret = %d\n", rets);
+	mei_me_intr_enable(dev);
 	mutex_unlock(&dev->device_lock);
 	return IRQ_HANDLED;
 }
diff --git a/drivers/misc/mei/hw-me.h b/drivers/misc/mei/hw-me.h
index 2ee14dc1b2ea..cf64847a35b9 100644
--- a/drivers/misc/mei/hw-me.h
+++ b/drivers/misc/mei/hw-me.h
@@ -51,14 +51,12 @@ struct mei_cfg {
  *
  * @cfg: per device generation config and ops
  * @mem_addr: io memory address
- * @intr_source: interrupt source
  * @pg_state: power gating state
  * @d0i3_supported: di03 support
  */
 struct mei_me_hw {
 	const struct mei_cfg *cfg;
 	void __iomem *mem_addr;
-	u32 intr_source;
 	enum mei_pg_state pg_state;
 	bool d0i3_supported;
 };
-- 
2.7.4