Message-Id: <20230522143105.8617-1-LinoSanfilippo@gmx.de>
Date:   Mon, 22 May 2023 16:31:04 +0200
From:   Lino Sanfilippo <LinoSanfilippo@....de>
To:     peterhuewe@....de, jarkko@...nel.org, jgg@...pe.ca
Cc:     jsnitsel@...hat.com, hdegoede@...hat.com, oe-lkp@...ts.linux.dev,
        lkp@...el.com, peter.ujfalusi@...ux.intel.com,
        peterz@...radead.org, linux@...ewoehner.de,
        linux-integrity@...r.kernel.org, linux-kernel@...r.kernel.org,
        l.sanfilippo@...bus.com, LinoSanfilippo@....de, lukas@...ner.de,
        p.rosenberger@...bus.com
Subject: [PATCH 1/2] tpm, tpm_tis: Handle interrupt storm

From: Lino Sanfilippo <l.sanfilippo@...bus.com>

Commit e644b2f498d2 ("tpm, tpm_tis: Enable interrupt test") enabled
interrupts instead of polling on all capable TPMs. Unfortunately, on some
products the interrupt line is either never asserted or never deasserted.

The former causes interrupt timeouts and is detected by
tpm_tis_core_init(). The latter results in interrupt storms.

Recent reports concern the Lenovo ThinkStation P360 Tiny, Lenovo ThinkPad
L490 and Inspur NF5180M6:

https://lore.kernel.org/linux-integrity/20230511005403.24689-1-jsnitsel@redhat.com/
https://lore.kernel.org/linux-integrity/d80b180a569a9f068d3a2614f062cfa3a78af5a6.camel@kernel.org/

The current approach to avoid those storms is to disable interrupts by
adding a DMI quirk for the concerned device.

However this is a maintenance burden in the long run, so use a generic
approach:

Detect an interrupt storm by counting the number of unhandled interrupts
within a 10 ms time interval. In case that more than 1000 were unhandled
deactivate interrupts, deregister the handler and fall back to polling.

This equals the implementation that handles interrupt storms in
note_interrupt() by means of timestamps and counters in struct irq_desc.
However the function to access this structure is private so the logic has
to be reimplemented in the TPM TIS core.

Since handler deregistration would deadlock from within the interrupt
routine trigger a worker thread that executes the unregistration.

Suggested-by: Lukas Wunner <lukas@...ner.de>
Signed-off-by: Lino Sanfilippo <l.sanfilippo@...bus.com>
---
 drivers/char/tpm/tpm_tis_core.c | 71 +++++++++++++++++++++++++++++++--
 drivers/char/tpm/tpm_tis_core.h |  6 +++
 2 files changed, 74 insertions(+), 3 deletions(-)

diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index 558144fa707a..458ebf8c2f16 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -752,6 +752,55 @@ static bool tpm_tis_req_canceled(struct tpm_chip *chip, u8 status)
 	return status == TPM_STS_COMMAND_READY;
 }
 
+static void tpm_tis_handle_irq_storm(struct tpm_chip *chip)
+{
+	struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
+	int intmask = 0;
+
+	dev_err(&chip->dev, HW_ERR
+		"TPM interrupt storm detected, polling instead\n");
+
+	tpm_tis_read32(priv, TPM_INT_ENABLE(priv->locality), &intmask);
+
+	intmask &= ~TPM_GLOBAL_INT_ENABLE;
+
+	tpm_tis_request_locality(chip, 0);
+	tpm_tis_write32(priv, TPM_INT_ENABLE(priv->locality), intmask);
+	tpm_tis_relinquish_locality(chip, 0);
+
+	chip->flags &= ~TPM_CHIP_FLAG_IRQ;
+
+	/*
+	 * We must not call devm_free_irq() from within the interrupt handler,
+	 * since this function waits for running interrupt handlers to finish
+	 * and thus it would deadlock. Instead trigger a worker that does the
+	 * unregistration.
+	 */
+	schedule_work(&priv->free_irq_work);
+}
+
+static void tpm_tis_process_unhandled_interrupt(struct tpm_chip *chip)
+{
+	const unsigned int MAX_UNHANDLED_IRQS = 1000;
+	struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
+	/*
+	 * The worker to free the TPM interrupt (free_irq_work) may already
+	 * be scheduled, so make sure it is not scheduled again.
+	 */
+	if (!(chip->flags & TPM_CHIP_FLAG_IRQ))
+		return;
+
+	if (time_after(jiffies, priv->last_unhandled_irq + HZ/10))
+		priv->unhandled_irqs = 1;
+	else
+		priv->unhandled_irqs++;
+
+	priv->last_unhandled_irq = jiffies;
+
+	if (priv->unhandled_irqs > MAX_UNHANDLED_IRQS)
+		tpm_tis_handle_irq_storm(chip);
+}
+
 static irqreturn_t tis_int_handler(int dummy, void *dev_id)
 {
 	struct tpm_chip *chip = dev_id;
@@ -761,10 +810,10 @@ static irqreturn_t tis_int_handler(int dummy, void *dev_id)
 
 	rc = tpm_tis_read32(priv, TPM_INT_STATUS(priv->locality), &interrupt);
 	if (rc < 0)
-		return IRQ_NONE;
+		goto unhandled;
 
 	if (interrupt == 0)
-		return IRQ_NONE;
+		goto unhandled;
 
 	set_bit(TPM_TIS_IRQ_TESTED, &priv->flags);
 	if (interrupt & TPM_INTF_DATA_AVAIL_INT)
@@ -780,10 +829,14 @@ static irqreturn_t tis_int_handler(int dummy, void *dev_id)
 	rc = tpm_tis_write32(priv, TPM_INT_STATUS(priv->locality), interrupt);
 	tpm_tis_relinquish_locality(chip, 0);
 	if (rc < 0)
-		return IRQ_NONE;
+		goto unhandled;
 
 	tpm_tis_read32(priv, TPM_INT_STATUS(priv->locality), &interrupt);
 	return IRQ_HANDLED;
+
+unhandled:
+	tpm_tis_process_unhandled_interrupt(chip);
+	return IRQ_HANDLED;
 }
 
 static void tpm_tis_gen_interrupt(struct tpm_chip *chip)
@@ -804,6 +857,15 @@ static void tpm_tis_gen_interrupt(struct tpm_chip *chip)
 		chip->flags &= ~TPM_CHIP_FLAG_IRQ;
 }
 
+static void tpm_tis_free_irq_func(struct work_struct *work)
+{
+	struct tpm_tis_data *priv = container_of(work, typeof(*priv), free_irq_work);
+	struct tpm_chip *chip = priv->chip;
+
+	devm_free_irq(chip->dev.parent, priv->irq, chip);
+	priv->irq = 0;
+}
+
 /* Register the IRQ and issue a command that will cause an interrupt. If an
  * irq is seen then leave the chip setup for IRQ operation, otherwise reverse
  * everything and leave in polling mode. Returns 0 on success.
@@ -816,6 +878,7 @@ static int tpm_tis_probe_irq_single(struct tpm_chip *chip, u32 intmask,
 	int rc;
 	u32 int_status;
 
+	INIT_WORK(&priv->free_irq_work, tpm_tis_free_irq_func);
 
 	rc = devm_request_threaded_irq(chip->dev.parent, irq, NULL,
 				       tis_int_handler, IRQF_ONESHOT | flags,
@@ -918,6 +981,7 @@ void tpm_tis_remove(struct tpm_chip *chip)
 		interrupt = 0;
 
 	tpm_tis_write32(priv, reg, ~TPM_GLOBAL_INT_ENABLE & interrupt);
+	flush_work(&priv->free_irq_work);
 
 	tpm_tis_clkrun_enable(chip, false);
 
@@ -1021,6 +1085,7 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
 	chip->timeout_b = msecs_to_jiffies(TIS_TIMEOUT_B_MAX);
 	chip->timeout_c = msecs_to_jiffies(TIS_TIMEOUT_C_MAX);
 	chip->timeout_d = msecs_to_jiffies(TIS_TIMEOUT_D_MAX);
+	priv->chip = chip;
 	priv->timeout_min = TPM_TIMEOUT_USECS_MIN;
 	priv->timeout_max = TPM_TIMEOUT_USECS_MAX;
 	priv->phy_ops = phy_ops;
diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
index e978f457fd4d..6fc86baa4398 100644
--- a/drivers/char/tpm/tpm_tis_core.h
+++ b/drivers/char/tpm/tpm_tis_core.h
@@ -91,12 +91,18 @@ enum tpm_tis_flags {
 };
 
 struct tpm_tis_data {
+	struct tpm_chip *chip;
 	u16 manufacturer_id;
 	struct mutex locality_count_mutex;
 	unsigned int locality_count;
 	int locality;
+	/* Interrupts */
 	int irq;
+	struct work_struct free_irq_work;
+	unsigned long last_unhandled_irq;
+	unsigned int unhandled_irqs;
 	unsigned int int_mask;
+
 	unsigned long flags;
 	void __iomem *ilb_base_addr;
 	u16 clkrun_enabled;

base-commit: 44c026a73be8038f03dbdeef028b642880cf1511
-- 
2.40.1

Powered by blists - more mailing lists