lkml.org 
[lkml]   [2014]   [Aug]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC PATCH v4] tpm_tis: verify interrupt during init
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


\
 
 \ /
  Last update: 2014-08-30 20:01    [W:0.096 / U:2.092 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site