Re: [RFC PATCH v4] tpm_tis: verify interrupt during init
From: Jason Gunthorpe
Date: Sat Aug 30 2014 - 13:49:55 EST
On Fri, Aug 29, 2014 at 11:59:32PM +0000, Scot Doyle wrote:
> (current->pending.signal.sig == 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.
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/