diff mbox

[tpmdd-devel,v10] tpm_tis: verify interrupt during init

Message ID alpine.DEB.2.11.1409242234220.2311@localhost.localdomain
State Accepted, archived
Headers show

Commit Message

Scot Doyle Sept. 24, 2014, 10:41 p.m. UTC
Some machines, such as the Acer C720 and Toshiba CB35, have TPMs that do
not send IRQs while also having an ACPI TPM entry indicating that they
will be sent. These machines freeze on resume while the tpm_tis module
waits for an IRQ, eventually timing out.

When in interrupt mode, the tpm_tis module should receive an IRQ during
module init. Fall back to polling mode if none is received when expected.

Signed-off-by: Scot Doyle <lkml14@scotdoyle.com>
Tested-by: Michael Mullin <masmullin@gmail.com>
---
 drivers/char/tpm/tpm_tis.c | 75 +++++++++++++++++++++++++++++++++++++---------
 1 file changed, 61 insertions(+), 14 deletions(-)

Comments

Jason Gunthorpe Sept. 29, 2014, 5:24 p.m. UTC | #1
On Wed, Sep 24, 2014 at 10:41:10PM +0000, Scot Doyle wrote:
> Some machines, such as the Acer C720 and Toshiba CB35, have TPMs that do
> not send IRQs while also having an ACPI TPM entry indicating that they
> will be sent. These machines freeze on resume while the tpm_tis module
> waits for an IRQ, eventually timing out.
> 
> When in interrupt mode, the tpm_tis module should receive an IRQ during
> module init. Fall back to polling mode if none is received when expected.
> 
> Signed-off-by: Scot Doyle <lkml14@scotdoyle.com>
> Tested-by: Michael Mullin <masmullin@gmail.com>

Looks good with enable fixed

Reviewed-By: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>

>  drivers/char/tpm/tpm_tis.c | 75 +++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 61 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
> index 2c46734..cbef80e 100644
> +++ b/drivers/char/tpm/tpm_tis.c
> @@ -75,6 +75,10 @@ enum tis_defaults {
>  #define	TPM_DID_VID(l)			(0x0F00 | ((l) << 12))
>  #define	TPM_RID(l)			(0x0F04 | ((l) << 12))
> 
> +struct priv_data {
> +	bool irq_tested;
> +};
> +
>  static LIST_HEAD(tis_chips);
>  static DEFINE_MUTEX(tis_lock);
> 
> @@ -338,12 +342,26 @@ out_err:
>  	return rc;
>  }
> 
> +static void disable_interrupts(struct tpm_chip *chip)
> +{
> +	u32 intmask;
> +	intmask =
> +	    ioread32(chip->vendor.iobase +
> +		     TPM_INT_ENABLE(chip->vendor.locality));
> +	intmask &= ~TPM_GLOBAL_INT_ENABLE;
> +	iowrite32(intmask,
> +		  chip->vendor.iobase +
> +		  TPM_INT_ENABLE(chip->vendor.locality));
> +	free_irq(chip->vendor.irq, chip);
> +	chip->vendor.irq = 0;
> +}
> +
>  /*
>   * If interrupts are used (signaled by an irq set in the vendor structure)
>   * tpm.c can skip polling for the data to be available as the interrupt is
>   * waited for here
>   */
> -static int tpm_tis_send(struct tpm_chip *chip, u8 *buf, size_t len)
> +static int tpm_tis_send_main(struct tpm_chip *chip, u8 *buf, size_t len)
>  {
>  	int rc;
>  	u32 ordinal;
> @@ -373,6 +391,30 @@ out_err:
>  	return rc;
>  }
> 
> +static int tpm_tis_send(struct tpm_chip *chip, u8 *buf, size_t len)
> +{
> +	int rc, irq;
> +	struct priv_data *priv = chip->vendor.priv;
> +
> +	if (!chip->vendor.irq || priv->irq_tested)
> +		return tpm_tis_send_main(chip, buf, len);
> +
> +	/* Verify receipt of the expected IRQ */
> +	irq = chip->vendor.irq;
> +	chip->vendor.irq = 0;
> +	rc = tpm_tis_send_main(chip, buf, len);
> +	chip->vendor.irq = irq;
> +	if (!priv->irq_tested)
> +		msleep(1);
> +	if (!priv->irq_tested) {
> +		disable_interrupts(chip);
> +		dev_err(chip->dev,
> +			FW_BUG "TPM interrupt not working, polling instead\n");
> +	}
> +	priv->irq_tested = true;
> +	return rc;
> +}
> +
>  struct tis_vendor_timeout_override {
>  	u32 did_vid;
>  	unsigned long timeout_us[4];
> @@ -505,6 +547,7 @@ static irqreturn_t tis_int_handler(int dummy, void *dev_id)
>  	if (interrupt == 0)
>  		return IRQ_NONE;
> 
> +	((struct priv_data*)chip->vendor.priv)->irq_tested = true;
>  	if (interrupt & TPM_INTF_DATA_AVAIL_INT)
>  		wake_up_interruptible(&chip->vendor.read_queue);
>  	if (interrupt & TPM_INTF_LOCALITY_CHANGE_INT)
> @@ -534,9 +577,14 @@ static int tpm_tis_init(struct device *dev, resource_size_t start,
>  	u32 vendor, intfcaps, intmask;
>  	int rc, i, irq_s, irq_e, probe;
>  	struct tpm_chip *chip;
> +	struct priv_data *priv;
> 
> +	priv = devm_kzalloc(dev, sizeof(struct priv_data), GFP_KERNEL);
> +	if (priv == NULL)
> +		return -ENOMEM;
>  	if (!(chip = tpm_register_hardware(dev, &tpm_tis)))
>  		return -ENODEV;
> +	chip->vendor.priv = priv;
> 
>  	chip->vendor.iobase = ioremap(start, len);
>  	if (!chip->vendor.iobase) {
> @@ -605,19 +653,6 @@ static int tpm_tis_init(struct device *dev, resource_size_t start,
>  	if (intfcaps & TPM_INTF_DATA_AVAIL_INT)
>  		dev_dbg(dev, "\tData Avail Int Support\n");
> 
> -	/* get the timeouts before testing for irqs */
> -	if (tpm_get_timeouts(chip)) {
> -		dev_err(dev, "Could not get TPM timeouts and durations\n");
> -		rc = -ENODEV;
> -		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);
> @@ -719,6 +754,18 @@ static int tpm_tis_init(struct device *dev, resource_size_t start,
>  		}
>  	}
> 
> +	if (tpm_get_timeouts(chip)) {
> +		dev_err(dev, "Could not get TPM timeouts and durations\n");
> +		rc = -ENODEV;
> +		goto out_err;
> +	}
> +
> +	if (tpm_do_selftest(chip)) {
> +		dev_err(dev, "TPM self test failed\n");
> +		rc = -ENODEV;
> +		goto out_err;
> +	}
> +
>  	INIT_LIST_HEAD(&chip->vendor.list);
>  	mutex_lock(&tis_lock);
>  	list_add(&chip->vendor.list, &tis_chips);

------------------------------------------------------------------------------
Slashdot TV.  Videos for Nerds.  Stuff that Matters.
http://pubads.g.doubleclick.net/gampad/clk?id=160591471&iu=/4140/ostg.clktrk
Peter Hüwe Nov. 30, 2014, 2:24 p.m. UTC | #2
Hi Scot,

Am Montag, 29. September 2014, 19:24:57 schrieb Jason Gunthorpe:
> On Wed, Sep 24, 2014 at 10:41:10PM +0000, Scot Doyle wrote:
> > Some machines, such as the Acer C720 and Toshiba CB35, have TPMs that do
> > not send IRQs while also having an ACPI TPM entry indicating that they
> > will be sent. These machines freeze on resume while the tpm_tis module
> > waits for an IRQ, eventually timing out.
> > 
> > When in interrupt mode, the tpm_tis module should receive an IRQ during
> > module init. Fall back to polling mode if none is received when expected.
> > 
> > Signed-off-by: Scot Doyle <lkml14@scotdoyle.com>
> > Tested-by: Michael Mullin <masmullin@gmail.com>
> 
> Looks good with enable fixed
> 
> Reviewed-By: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>


Applied to my tree:
https://github.com/PeterHuewe/linux-tpmdd for-james

Will be included in the next pull-request.

Thanks,
Peter

------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=157005751&iu=/4140/ostg.clktrk
diff mbox

Patch

diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
index 2c46734..cbef80e 100644
--- a/drivers/char/tpm/tpm_tis.c
+++ b/drivers/char/tpm/tpm_tis.c
@@ -75,6 +75,10 @@  enum tis_defaults {
 #define	TPM_DID_VID(l)			(0x0F00 | ((l) << 12))
 #define	TPM_RID(l)			(0x0F04 | ((l) << 12))

+struct priv_data {
+	bool irq_tested;
+};
+
 static LIST_HEAD(tis_chips);
 static DEFINE_MUTEX(tis_lock);

@@ -338,12 +342,26 @@  out_err:
 	return rc;
 }

+static void disable_interrupts(struct tpm_chip *chip)
+{
+	u32 intmask;
+	intmask =
+	    ioread32(chip->vendor.iobase +
+		     TPM_INT_ENABLE(chip->vendor.locality));
+	intmask &= ~TPM_GLOBAL_INT_ENABLE;
+	iowrite32(intmask,
+		  chip->vendor.iobase +
+		  TPM_INT_ENABLE(chip->vendor.locality));
+	free_irq(chip->vendor.irq, chip);
+	chip->vendor.irq = 0;
+}
+
 /*
  * If interrupts are used (signaled by an irq set in the vendor structure)
  * tpm.c can skip polling for the data to be available as the interrupt is
  * waited for here
  */
-static int tpm_tis_send(struct tpm_chip *chip, u8 *buf, size_t len)
+static int tpm_tis_send_main(struct tpm_chip *chip, u8 *buf, size_t len)
 {
 	int rc;
 	u32 ordinal;
@@ -373,6 +391,30 @@  out_err:
 	return rc;
 }

+static int tpm_tis_send(struct tpm_chip *chip, u8 *buf, size_t len)
+{
+	int rc, irq;
+	struct priv_data *priv = chip->vendor.priv;
+
+	if (!chip->vendor.irq || priv->irq_tested)
+		return tpm_tis_send_main(chip, buf, len);
+
+	/* Verify receipt of the expected IRQ */
+	irq = chip->vendor.irq;
+	chip->vendor.irq = 0;
+	rc = tpm_tis_send_main(chip, buf, len);
+	chip->vendor.irq = irq;
+	if (!priv->irq_tested)
+		msleep(1);
+	if (!priv->irq_tested) {
+		disable_interrupts(chip);
+		dev_err(chip->dev,
+			FW_BUG "TPM interrupt not working, polling instead\n");
+	}
+	priv->irq_tested = true;
+	return rc;
+}
+
 struct tis_vendor_timeout_override {
 	u32 did_vid;
 	unsigned long timeout_us[4];
@@ -505,6 +547,7 @@  static irqreturn_t tis_int_handler(int dummy, void *dev_id)
 	if (interrupt == 0)
 		return IRQ_NONE;

+	((struct priv_data*)chip->vendor.priv)->irq_tested = true;
 	if (interrupt & TPM_INTF_DATA_AVAIL_INT)
 		wake_up_interruptible(&chip->vendor.read_queue);
 	if (interrupt & TPM_INTF_LOCALITY_CHANGE_INT)
@@ -534,9 +577,14 @@  static int tpm_tis_init(struct device *dev, resource_size_t start,
 	u32 vendor, intfcaps, intmask;
 	int rc, i, irq_s, irq_e, probe;
 	struct tpm_chip *chip;
+	struct priv_data *priv;

+	priv = devm_kzalloc(dev, sizeof(struct priv_data), GFP_KERNEL);
+	if (priv == NULL)
+		return -ENOMEM;
 	if (!(chip = tpm_register_hardware(dev, &tpm_tis)))
 		return -ENODEV;
+	chip->vendor.priv = priv;

 	chip->vendor.iobase = ioremap(start, len);
 	if (!chip->vendor.iobase) {
@@ -605,19 +653,6 @@  static int tpm_tis_init(struct device *dev, resource_size_t start,
 	if (intfcaps & TPM_INTF_DATA_AVAIL_INT)
 		dev_dbg(dev, "\tData Avail Int Support\n");

-	/* get the timeouts before testing for irqs */
-	if (tpm_get_timeouts(chip)) {
-		dev_err(dev, "Could not get TPM timeouts and durations\n");
-		rc = -ENODEV;
-		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);
@@ -719,6 +754,18 @@  static int tpm_tis_init(struct device *dev, resource_size_t start,
 		}
 	}

+	if (tpm_get_timeouts(chip)) {
+		dev_err(dev, "Could not get TPM timeouts and durations\n");
+		rc = -ENODEV;
+		goto out_err;
+	}
+
+	if (tpm_do_selftest(chip)) {
+		dev_err(dev, "TPM self test failed\n");
+		rc = -ENODEV;
+		goto out_err;
+	}
+
 	INIT_LIST_HEAD(&chip->vendor.list);
 	mutex_lock(&tis_lock);
 	list_add(&chip->vendor.list, &tis_chips);