[PATCH char-misc-next 3/4] mei: me: avoid hw access in polling when not active

[Alexander Usyskin]

Polling thread should not access hardware when not active.
Check for active state under device lock to avoid such
access when hardware is powered down.

Reviewed-by: Menachem Adin <menachem.adin@xxxxxxxxx>
Signed-off-by: Alexander Usyskin <alexander.usyskin@xxxxxxxxx>
---
 drivers/misc/mei/hw-me.c | 99 +++++++++++++++++++++++++++++-------------------
 1 file changed, 59 insertions(+), 40 deletions(-)

diff --git a/drivers/misc/mei/hw-me.c b/drivers/misc/mei/hw-me.c
index ef7e1289ad7e..f3d833be0e1a 100644
--- a/drivers/misc/mei/hw-me.c
+++ b/drivers/misc/mei/hw-me.c
@@ -5,6 +5,7 @@
  */
 
 #include <linux/bitfield.h>
+#include <linux/cleanup.h>
 #include <linux/delay.h>
 #include <linux/interrupt.h>
 #include <linux/kthread.h>
@@ -1290,28 +1291,15 @@ irqreturn_t mei_me_irq_quick_handler(int irq, void *dev_id)
 }
 EXPORT_SYMBOL_GPL(mei_me_irq_quick_handler);
 
-/**
- * mei_me_irq_thread_handler - function called after ISR to handle the interrupt
- * processing.
- *
- * @irq: The irq number
- * @dev_id: pointer to the device structure
- *
- * Return: irqreturn_t
- *
- */
-irqreturn_t mei_me_irq_thread_handler(int irq, void *dev_id)
+static irqreturn_t __mei_me_irq_thread_handler(struct mei_device *dev)
 {
-	struct mei_device *dev = (struct mei_device *) dev_id;
 	struct list_head cmpl_list;
 	bool pg_blocked;
 	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);
@@ -1406,13 +1394,33 @@ 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;
 }
+
+/**
+ * mei_me_irq_thread_handler - function called after ISR to handle the interrupt
+ * processing.
+ *
+ * @irq: The irq number
+ * @dev_id: pointer to the device structure
+ *
+ * Return: irqreturn_t
+ *
+ */
+irqreturn_t mei_me_irq_thread_handler(int irq, void *dev_id)
+{
+	struct mei_device *dev = (struct mei_device *) dev_id;
+
+	dev_dbg(&dev->dev, "function called after ISR to handle the interrupt processing.\n");
+
+	guard(mutex)(&dev->device_lock);
+
+	return __mei_me_irq_thread_handler(dev);
+}
 EXPORT_SYMBOL_GPL(mei_me_irq_thread_handler);
 
-#define MEI_POLLING_TIMEOUT_ACTIVE 100
-#define MEI_POLLING_TIMEOUT_IDLE   500
+#define MEI_POLL_ACTIVE_MS 100
+#define MEI_POLL_IDLE_MS   500
 
 /**
  * mei_me_polling_thread - interrupt register polling thread
@@ -1420,48 +1428,59 @@ EXPORT_SYMBOL_GPL(mei_me_irq_thread_handler);
  * @_dev: mei device
  *
  * The thread monitors the interrupt source register and calls
- * mei_me_irq_thread_handler() to handle the firmware
+ * __mei_me_irq_thread_handler() to handle the firmware
  * input.
  *
- * The function polls in MEI_POLLING_TIMEOUT_ACTIVE timeout
+ * The function polls in MEI_POLL_ACTIVE_MS timeout
  * in case there was an event, in idle case the polling
- * time increases yet again by MEI_POLLING_TIMEOUT_ACTIVE
- * up to MEI_POLLING_TIMEOUT_IDLE.
+ * time increases yet again by MEI_POLL_ACTIVE_MS
+ * up to MEI_POLL_IDLE_MS.
  *
  * Return: always 0
  */
 int mei_me_polling_thread(void *_dev)
 {
 	struct mei_device *dev = _dev;
-	irqreturn_t irq_ret;
-	long polling_timeout = MEI_POLLING_TIMEOUT_ACTIVE;
+	long polling_timeout = MEI_POLL_ACTIVE_MS;
+	bool have_interrupt;
 
 	dev_dbg(&dev->dev, "kernel thread is running\n");
 	while (!kthread_should_stop()) {
 		struct mei_me_hw *hw = to_me_hw(dev);
 		u32 hcsr;
 
-		wait_event_interruptible_timeout(hw->wait_active,
-						 hw->is_active || kthread_should_stop(),
-						 msecs_to_jiffies(MEI_POLLING_TIMEOUT_IDLE));
+		if (!hw->is_active)
+			wait_event_interruptible(hw->wait_active,
+						 hw->is_active || kthread_should_stop());
+		else
+			wait_event_interruptible_timeout(hw->wait_active,
+							 hw->is_active || kthread_should_stop(),
+							 msecs_to_jiffies(MEI_POLL_IDLE_MS));
 
 		if (kthread_should_stop())
 			break;
 
-		hcsr = mei_hcsr_read(dev);
-		if (me_intr_src(hcsr)) {
-			polling_timeout = MEI_POLLING_TIMEOUT_ACTIVE;
-			irq_ret = mei_me_irq_thread_handler(1, dev);
-			if (irq_ret != IRQ_HANDLED)
-				dev_err(&dev->dev, "irq_ret %d\n", irq_ret);
-		} else {
-			/*
-			 * Increase timeout by MEI_POLLING_TIMEOUT_ACTIVE
-			 * up to MEI_POLLING_TIMEOUT_IDLE
-			 */
-			polling_timeout = clamp_val(polling_timeout + MEI_POLLING_TIMEOUT_ACTIVE,
-						    MEI_POLLING_TIMEOUT_ACTIVE,
-						    MEI_POLLING_TIMEOUT_IDLE);
+		scoped_guard(mutex, &dev->device_lock) {
+			if (!hw->is_active) {
+				have_interrupt = false;
+			} else {
+				hcsr = mei_hcsr_read(dev);
+				have_interrupt = !!me_intr_src(hcsr);
+			}
+
+			if (have_interrupt) {
+				polling_timeout = MEI_POLL_ACTIVE_MS;
+				dev_dbg(&dev->dev, "polling interrupt processing\n");
+				__mei_me_irq_thread_handler(dev);
+			} else {
+				/*
+				 * Increase timeout by MEI_POLL_ACTIVE_MS
+				 * up to MEI_POLL_IDLE_MS
+				 */
+				polling_timeout = clamp_val(polling_timeout + MEI_POLL_ACTIVE_MS,
+							    MEI_POLL_ACTIVE_MS,
+							    MEI_POLL_IDLE_MS);
+			}
 		}
 
 		schedule_timeout_interruptible(msecs_to_jiffies(polling_timeout));

-- 
2.53.0