More interrupt problems with TIS TPM

Commit Message

James Bottomley Sept. 29, 2020, 12:52 a.m. UTC

I've got hold of an infineon TIS TPM which actually has a working
interrupt.  I find even with the other fix I still need the patch below
to get the interrupt to fire because without it nothing ever sets
TPM_CHIP_FLAG_IRQ which means the interrupt test code is never
executed.

Finally with all this probing fixed, I'm seeing interrupt storms.  The
way this TPM seems to work is that if you allow it to send command
ready interrupts, it will send them any time it can accept a command. 
The problem is if you clear the interrupt and it can accept a command,
it will send another command ready interrupt ... hence the storm since
the TPM is pretty much always in the command ready state.  The only way
to mitigate this seems to be *only* to enable the command ready
interrupt when you're preparing to wait for the TPM to become ready. 
i.e. these interrupts have to be treated as one shot enable, so the
interrupt routine has to mask the command ready interrupt before doing
a TPM_EOI in our way of doing things.  There seems to be support for
this in the TIS spec around line 1135 where it advises us to keep all
interrupts masked until polling says we have to wait for a particular
state.

James

---

Comments

Jarkko Sakkinen Sept. 30, 2020, 2:14 a.m. UTC | #1

On Mon, Sep 28, 2020 at 05:52:43PM -0700, James Bottomley wrote:
> I've got hold of an infineon TIS TPM which actually has a working
> interrupt.  I find even with the other fix I still need the patch below
> to get the interrupt to fire because without it nothing ever sets
> TPM_CHIP_FLAG_IRQ which means the interrupt test code is never
> executed.
> 
> Finally with all this probing fixed, I'm seeing interrupt storms.  The
> way this TPM seems to work is that if you allow it to send command
> ready interrupts, it will send them any time it can accept a command. 
> The problem is if you clear the interrupt and it can accept a command,
> it will send another command ready interrupt ... hence the storm since
> the TPM is pretty much always in the command ready state.  The only way
> to mitigate this seems to be *only* to enable the command ready
> interrupt when you're preparing to wait for the TPM to become ready. 
> i.e. these interrupts have to be treated as one shot enable, so the
> interrupt routine has to mask the command ready interrupt before doing
> a TPM_EOI in our way of doing things.  There seems to be support for
> this in the TIS spec around line 1135 where it advises us to keep all
> interrupts masked until polling says we have to wait for a particular
> state.
> 
> James

OK, this makes a lot of sense. I'll go through the patch set that you
posted. Thank you.

> 
> ---
> 
> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
> index 6b884badabe7..1578d158416c 100644
> --- a/drivers/char/tpm/tpm_tis_core.c
> +++ b/drivers/char/tpm/tpm_tis_core.c
> @@ -804,6 +810,7 @@ static int tpm_tis_probe_irq_single(struct tpm_chip *chip, u32 intmask,
>  		return rc;
>  
>  	priv->irq_tested = false;
> +	chip->flags |= TPM_CHIP_FLAG_IRQ;
>  
>  	/* Generate an interrupt by having the core call through to
>  	 * tpm_tis_send
> 
> 

/Jarkko

Patch

diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index 6b884badabe7..1578d158416c 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -804,6 +810,7 @@  static int tpm_tis_probe_irq_single(struct tpm_chip *chip, u32 intmask,
 		return rc;
 
 	priv->irq_tested = false;
+	chip->flags |= TPM_CHIP_FLAG_IRQ;
 
 	/* Generate an interrupt by having the core call through to
 	 * tpm_tis_send