[RFC PATCH v2] tpm_tis: verify interrupt during init

[Scot Doyle]

On Mon, 25 Aug 2014, Jason Gunthorpe wrote:
> On Mon, Aug 25, 2014, Scot Doyle wrote:
>> 3. Custom SeaBIOS. Blacklist the tpm_tis module so that it doesn't load
>>    and therefore doesn't issue startup(clear) to the TPM chip.
>
> It seems to me at least in this case you should be able to get rid of
> the IRQ entry, people are going to be flashing the custom SeaBIOS
> anyhow.

The person building many of these custom SeaBIOS packages has removed the
TPM section from the DSDT, so this may be addressed.


On Mon, 25 Aug 2014, Jason Gunthorpe wrote:
> I think you'll have to directly test in the tis driver if the
> interrupt is working.
>
> The ordering in the TIS driver is wrong, interrupts should be turned
> on before any TPM commands are issued. This is what other drivers are
> doing.
>
> If you fix this, tis can then just count interrupts recieved and check
> if that is 0 to detect failure and then turn them off.

How about something like this?

It doesn't enable stock SeaBIOS machines to suspend/resume before the 30 
second interrupt timeout, unless using interrupts=0 or force=1.

---

diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
index 2c46734..ae701d8 100644
--- a/drivers/char/tpm/tpm_tis.c
+++ b/drivers/char/tpm/tpm_tis.c
@@ -493,6 +493,8 @@ static irqreturn_t tis_int_probe(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }

+static bool interrupted = false;
+
 static irqreturn_t tis_int_handler(int dummy, void *dev_id)
 {
 	struct tpm_chip *chip = dev_id;
@@ -511,6 +513,8 @@ static irqreturn_t tis_int_handler(int dummy, void *dev_id)
 		for (i = 0; i < 5; i++)
 			if (check_locality(chip, i) >= 0)
 				break;
+	if (interrupt & TPM_INTF_CMD_READY_INT)
+		interrupted = true;
 	if (interrupt &
 	    (TPM_INTF_LOCALITY_CHANGE_INT | TPM_INTF_STS_VALID_INT |
 	     TPM_INTF_CMD_READY_INT))
@@ -612,12 +616,6 @@ static int tpm_tis_init(struct device *dev, resource_size_t start,
 		goto out_err;
 	}

-	if (tpm_do_selftest(chip)) {
-		dev_err(dev, "TPM self test failed\n");
-		rc = -ENODEV;
-		goto out_err;
-	}
-
 	/* INTERRUPT Setup */
 	init_waitqueue_head(&chip->vendor.read_queue);
 	init_waitqueue_head(&chip->vendor.int_queue);
@@ -693,7 +691,7 @@ static int tpm_tis_init(struct device *dev, resource_size_t start,
 			free_irq(i, chip);
 		}
 	}
-	if (chip->vendor.irq) {
+	if (interrupts && chip->vendor.irq) {
 		iowrite8(chip->vendor.irq,
 			 chip->vendor.iobase +
 			 TPM_INT_VECTOR(chip->vendor.locality));
@@ -719,6 +717,32 @@ static int tpm_tis_init(struct device *dev, resource_size_t start,
 		}
 	}

+	/* Test interrupt and/or prepare for later save state */
+	interrupted = false;
+	if (tpm_do_selftest(chip)) {
+		if (!interrupts || interrupted) {
+			dev_err(dev, "TPM self test failed\n");
+			rc = -ENODEV;
+			goto out_err;
+		} else {
+			/* Turn off interrupt */
+			iowrite32(intmask,
+				  chip->vendor.iobase +
+				  TPM_INT_ENABLE(chip->vendor.locality));
+			free_irq(chip->vendor.irq, chip);
+
+			/* Retry in polling mode */
+			chip->vendor.irq = 0;
+			if (tpm_do_selftest(chip)) {
+				dev_err(dev, "TPM self test failed\n");
+				rc = -ENODEV;
+				goto out_err;
+			} else {
+				dev_err(dev, "ACPI DSDT entry incorrect, polling instead\n");
+			}
+		}
+	}
+
 	INIT_LIST_HEAD(&chip->vendor.list);
 	mutex_lock(&tis_lock);
 	list_add(&chip->vendor.list, &tis_chips);

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