Re: [PATCH 1/2] tpm, tpm_tis: Handle interrupt storm

[Lino Sanfilippo]

On 24.05.23 at 17:30, Jarkko Sakkinen wrote:
> On Wed May 24, 2023 at 6:58 AM EEST, Jarkko Sakkinen wrote:
>>>  	rc = tpm_tis_read32(priv, TPM_INT_STATUS(priv->locality), &interrupt);
>>>  	if (rc < 0)
>>> -		return IRQ_NONE;
>>> +		goto unhandled;
>>>
>>>  	if (interrupt == 0)
>>> -		return IRQ_NONE;
>>> +		goto unhandled;
>>>
>>>  	set_bit(TPM_TIS_IRQ_TESTED, &priv->flags);
>>>  	if (interrupt & TPM_INTF_DATA_AVAIL_INT)
>>> @@ -780,10 +829,14 @@ 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 unhandled;
>>>
>>>  	tpm_tis_read32(priv, TPM_INT_STATUS(priv->locality), &interrupt);
>>>  	return IRQ_HANDLED;
>>> +
>>> +unhandled:
>>> +	tpm_tis_process_unhandled_interrupt(chip);
>>> +	return IRQ_HANDLED;
>>>  }
>
> Some minor glitches I noticed.
>
> You could simplify the flow by making the helper to return IRQ_NONE.
>
> E.g.
>
> 	tpm_tis_relinquish_locality(chip, 0);
> 	if (rc < 0)
> 		return tpm_tis_process_unhandled_interrupt(chip);
>
> I'd recommend changing the function name simply tpm_tis_rollback_interrupt().
>

IMHO this name is worse, since this function does actually _not_ rollback interrupts
most of the times it is called. Only after an interrupt storm is detected (so currently
after it has been called at least 1000 times without rollback) it actually
rolls back interrupts and falls back to polling.

Maybe rather tpm_tis_check_for_interrupt_storm()?

Regards,
Lino