Re: [PATCH v9] tpm_tis: verify interrupt during init

[Scot Doyle <lkml14@scotdoyle.com>]

On Tue, 23 Sep 2014, 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>
> Reviewed-By: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>

Did I prematurely add your Reviewed-by? If so, I apologize.

> ---
> drivers/char/tpm/tpm_tis.c | 76 +++++++++++++++++++++++++++++++++++++---------
> 1 file changed, 62 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
> index 2c46734..bfe1f1a 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,27 @@ 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_INTF_CMD_READY_INT | TPM_INTF_LOCALITY_CHANGE_INT |
> +		   TPM_INTF_DATA_AVAIL_INT | TPM_INTF_STS_VALID_INT;
> +	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 +392,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 +548,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 +578,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 +654,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 +755,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);
> -- 
> 2.1.0
>