Re: [RFC PATCH v4] tpm_tis: verify interrupt during init

[Jason Gunthorpe <jgunthorpe@obsidianresearch.com>]

On Fri, Aug 29, 2014 at 11:59:32PM +0000, Scot Doyle wrote:

> (current->pending.signal.sig[0] == 0x00000100 == SIGKILL?) about 30 
> seconds after module load begins. wait_for_tpm_stat sees that the return 
> value from wait_event_interruptible_timeout is positive and returns 0. 
> tpm_tis_send thinks everything is fine and continues. However, since a 
> signal was received, but not cleared, then the next time 
> wait_event_interruptible_timeout is used within wait_for_tpm_stat it 
> returns with -ERESTARTSYS, and continues doing so until tpm_send_data 
> returns -ETIME.

Oh, I see. That is another bug you've found - ERESTARTSYS should not
be translated into ETIME!

It is also not exciting that udev is overriding the driver timeouts. :(

> [    1.536850] tpm_tis 00:08: 1.2 TPM (device-id 0xB, rev-id 16)
> [    7.650172] tpm_tis 00:08: [Firmware Bug]: TPM interrupt not working, polling instead
> 
> I tried calling tpm_get_timeouts only during the interrupt test, but again 
> was timed out after 30 seconds. The interrupt wait in tis_send calls 
> tpm_calc_ordinal_duration, which uses a default timeout of two minutes 
> when chip->vendor.duration[duration_idx] hasn't been set. Thus the second 
> call to tpm_get_timeouts in tpm_tis_init.

So the strategy is to read the timeouts and hope that the chip reports
something small and reasonable, then do a second read?

Seems reasonable, but with this new arrangement we could also use an
alternate polling logic for 'testing_int' that did the normal polling
loop unconditionally and then checked if the interrupt was
delivered. This would give a minimal dealy.

> What do you think about the guard logic? My intent is to prevent a signal 
> received after the test period from causing a fallback to polling mode. 
> Plus, it seems good to preserve the current logic where practical.

I think this is looking very reasonable now, I'll have to read it
closer next week!

> +	if (priv->testing_int)
> +		priv->int_received = true;

This could just be a simple counter, if the counter is 0 then test
the interrupt otherwise proceed normally.

Jason