[PATCH v4] tpm,tpm_tis: Disable interrupts after 1000 unhandled IRQs

From: Lino Sanfilippo
Date: Thu Jul 13 2023 - 13:40:46 EST

Next message: Mukesh Savaliya: "RE: [PATCH v6 2/2] spi: spi-geni-qcom: Add SPI Device mode support for GENI based QuPv3"
Previous message: Roger Quadros: "Re: [PATCH 0/2] Deprecate ti-serdes header file"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

From: Lino Sanfilippo <l.sanfilippo@xxxxxxxxxx>

After activation of interrupts for TPM TIS drivers 0-day reports an
interrupt storm on an Inspur NF5180M6 server.

Fix this by detecting the storm and falling back to polling:
Count the number of unhandled interrupts within a 10 ms time interval. In
case that more than 1000 were unhandled deactivate interrupts entirely,
deregister the handler and use polling instead.

Also print a note to point to the tpm_tis_dmi_table.

Since the interrupt deregistration function devm_free_irq() waits for all
interrupt handlers to finish, only trigger a worker in the interrupt
handler and do the unregistration in the worker to avoid a deadlock.

Note: the storm detection logic equals the implementation in
note_interrupt() which uses timestamps and counters stored in struct
irq_desc. Since this structure is private to the generic interrupt core
the TPM TIS core uses its own timestamps and counters. Furthermore the TPM
interrupt handler always returns IRQ_HANDLED to prevent the generic
interrupt core from processing the interrupt storm.

Fixes: e644b2f498d2 ("tpm, tpm_tis: Enable interrupt test")
Reported-by: kernel test robot <yujie.liu@xxxxxxxxx>
Closes: https://lore.kernel.org/oe-lkp/202305041325.ae8b0c43-yujie.liu@xxxxxxxxx/
Suggested-by: Lukas Wunner <lukas@xxxxxxxxx>
Signed-off-by: Lino Sanfilippo <l.sanfilippo@xxxxxxxxxx>
---
drivers/char/tpm/tpm_tis_core.c | 103 +++++++++++++++++++++++++++-----
drivers/char/tpm/tpm_tis_core.h | 4 ++
2 files changed, 92 insertions(+), 15 deletions(-)

Changes to v3 (all requested by Jarko):
- remove all inline comments
- rename tpm_tis_reenable_polling() to tpm_tis_revert_interrupts()
- rename tpm_tis_check_for_interrupt_storm() to tpm_tis_update_unhandled_irqs()
- rename label "unhandled" to "err"
- add Fixes: tag

Changes to v2:
- use define for max number of unhandles irqs(requested by Jarko)
- rename intmask to int_mask (requested by Jarko)
- rephrased short summary (requested by Jarko)
- rename disable_interrupts to tpm_tis_disable_interrupts (requested by Jarko)
- print info message concerning adding an entry to tpm_tis_dmi_table
(suggested by Jerry)
- amended commit message
- handle failure of locality request by returning IRQ_NONE
- dont take and release locality in __tpm_tis_disable_interrupts but in its
caller

diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index 558144fa707a..88a5384c09c0 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -24,9 +24,12 @@
#include <linux/wait.h>
#include <linux/acpi.h>
#include <linux/freezer.h>
+#include <linux/dmi.h>
#include "tpm.h"
#include "tpm_tis_core.h"

+#define TPM_TIS_MAX_UNHANDLED_IRQS 1000
+
static void tpm_tis_clkrun_enable(struct tpm_chip *chip, bool value);

static bool wait_for_tpm_stat_cond(struct tpm_chip *chip, u8 mask,
@@ -468,25 +471,29 @@ static int tpm_tis_send_data(struct tpm_chip *chip, const u8 *buf, size_t len)
return rc;
}

-static void disable_interrupts(struct tpm_chip *chip)
+static void __tpm_tis_disable_interrupts(struct tpm_chip *chip)
+{
+ struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
+ u32 int_mask = 0;
+
+ tpm_tis_read32(priv, TPM_INT_ENABLE(priv->locality), &int_mask);
+ int_mask &= ~TPM_GLOBAL_INT_ENABLE;
+ tpm_tis_write32(priv, TPM_INT_ENABLE(priv->locality), int_mask);
+
+ chip->flags &= ~TPM_CHIP_FLAG_IRQ;
+}
+
+static void tpm_tis_disable_interrupts(struct tpm_chip *chip)
{
struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
- u32 intmask;
- int rc;

if (priv->irq == 0)
return;

- rc = tpm_tis_read32(priv, TPM_INT_ENABLE(priv->locality), &intmask);
- if (rc < 0)
- intmask = 0;
-
- intmask &= ~TPM_GLOBAL_INT_ENABLE;
- rc = tpm_tis_write32(priv, TPM_INT_ENABLE(priv->locality), intmask);
+ __tpm_tis_disable_interrupts(chip);

devm_free_irq(chip->dev.parent, priv->irq, chip);
priv->irq = 0;
- chip->flags &= ~TPM_CHIP_FLAG_IRQ;
}

/*
@@ -552,7 +559,7 @@ static int tpm_tis_send(struct tpm_chip *chip, u8 *buf, size_t len)
if (!test_bit(TPM_TIS_IRQ_TESTED, &priv->flags))
tpm_msleep(1);
if (!test_bit(TPM_TIS_IRQ_TESTED, &priv->flags))
- disable_interrupts(chip);
+ tpm_tis_disable_interrupts(chip);
set_bit(TPM_TIS_IRQ_TESTED, &priv->flags);
return rc;
}
@@ -752,6 +759,57 @@ static bool tpm_tis_req_canceled(struct tpm_chip *chip, u8 status)
return status == TPM_STS_COMMAND_READY;
}

+static irqreturn_t tpm_tis_revert_interrupts(struct tpm_chip *chip)
+{
+ struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
+ const char *product;
+ const char *vendor;
+
+ dev_warn(&chip->dev, FW_BUG
+ "TPM interrupt storm detected, polling instead\n");
+
+ vendor = dmi_get_system_info(DMI_SYS_VENDOR);
+ product = dmi_get_system_info(DMI_PRODUCT_VERSION);
+
+ if (vendor && product) {
+ dev_info(&chip->dev,
+ "Consider adding the following entry to tpm_tis_dmi_table:\n");
+ dev_info(&chip->dev, "\tDMI_SYS_VENDOR: %s\n", vendor);
+ dev_info(&chip->dev, "\tDMI_PRODUCT_VERSION: %s\n", product);
+ }
+
+ if (tpm_tis_request_locality(chip, 0) != 0)
+ return IRQ_NONE;
+
+ __tpm_tis_disable_interrupts(chip);
+ tpm_tis_relinquish_locality(chip, 0);
+
+ schedule_work(&priv->free_irq_work);
+
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t tpm_tis_update_unhandled_irqs(struct tpm_chip *chip)
+{
+ struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
+ irqreturn_t irqret = IRQ_HANDLED;
+
+ if (!(chip->flags & TPM_CHIP_FLAG_IRQ))
+ return IRQ_HANDLED;
+
+ 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 > TPM_TIS_MAX_UNHANDLED_IRQS)
+ irqret = tpm_tis_revert_interrupts(chip);
+
+ return irqret;
+}
+
static irqreturn_t tis_int_handler(int dummy, void *dev_id)
{
struct tpm_chip *chip = dev_id;
@@ -761,10 +819,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 err;

if (interrupt == 0)
- return IRQ_NONE;
+ goto err;

set_bit(TPM_TIS_IRQ_TESTED, &priv->flags);
if (interrupt & TPM_INTF_DATA_AVAIL_INT)
@@ -780,10 +838,13 @@ 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 err;

tpm_tis_read32(priv, TPM_INT_STATUS(priv->locality), &interrupt);
return IRQ_HANDLED;
+
+err:
+ return tpm_tis_update_unhandled_irqs(chip);
}

static void tpm_tis_gen_interrupt(struct tpm_chip *chip)
@@ -804,6 +865,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 +886,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 +989,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 +1093,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;
@@ -1179,7 +1252,7 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
rc = tpm_tis_request_locality(chip, 0);
if (rc < 0)
goto out_err;
- disable_interrupts(chip);
+ tpm_tis_disable_interrupts(chip);
tpm_tis_relinquish_locality(chip, 0);
}
}
diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
index 610bfadb6acf..b1a169d7d1ca 100644
--- a/drivers/char/tpm/tpm_tis_core.h
+++ b/drivers/char/tpm/tpm_tis_core.h
@@ -91,11 +91,15 @@ 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;
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;

base-commit: 6995e2de6891c724bfeb2db33d7b87775f913ad1
--
2.25.1

Next message: Mukesh Savaliya: "RE: [PATCH v6 2/2] spi: spi-geni-qcom: Add SPI Device mode support for GENI based QuPv3"
Previous message: Roger Quadros: "Re: [PATCH 0/2] Deprecate ti-serdes header file"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]