On Tue, 2020-04-14 at 10:44 -0700, Jerry Snitselaar wrote:
> On Fri Apr 10 20, Jason Gunthorpe wrote:
>
> > On Thu, Apr 09, 2020 at 11:10:44PM +0200, Hans de Goede wrote:
> > > Since commit dda8b2af395b ("tpm: Revert "tpm_tis_core: Set
> > > TPM_CHIP_FLAG_IRQ before probing for interrupts"") we no longer set
> > > the TPM_CHIP_FLAG_IRQ ever.
> > This all used to work..
>
>
> IIRC this goes all the way back to 570a36097f30 ("tpm: drop 'irq' from struct tpm_vendor_specific")
> when the flag was added. There was never anything initially setting it in the tpm_tis code.
> There were checks added, but the only place it got set was where it did the interrupt test in
> tpm_tis_send, and it can only get down to that code if the flag is set. Stefan's patch was enabling
> the flag, but with the flag enabled some systems were seeing interrupt storms. I believe it has
> been seen with both the t490 and an internal system that Dan Williams was working on at Intel.
> Without access to hw seeing the problem the decision was to revert Stefan's patches while
> we try to figure out the issues.
I stumbled across this when debugging a sporadic "tpm tpm0:
tpm_try_transmit: send(): error -62 (-ETIME)" we see on a i.MX6 board
with a TPM2 connected via SPI. It seems that comes from down in
wait_for_tpm_stat() when the TPM doesn't become ready quickly enough.
As this board actually has the IRQ connected, but is falling back to
polling, I took another look and what's wrong with the IRQ support. I
don't have it working yet, but have found some things I'd like to share
(and maybe get some fresh ideas).
I used Stefan Berger's "tpm_tis_core: Turn on the TPM before probing
IRQ's" and "tpm_tis_core: Set TPM_CHIP_FLAG_IRQ before probing for
interrupts" to get it into the IRQ verification code in tpm_tis_send().
Then I changed the IRQ to threaded with IRQF_ONESHOT, as the handler is
calling into the SPI stack (which can sleep). A related problem exists
in wait_for_tpm_stat(), which uses the potentially sleeping
wait_for_tpm_stat_cond() as the condition check in
wait_event_interruptible_timeout(). This leads to a:
[ 41.799086] do not call blocking ops when !TASK_RUNNING; state=1 set at [<f02406bc>] prepare_to_wait_event+0x84/0x1a4
seems to lead to working transfers, but nevertheless needs to be
changed to be compatible with sleeping busses.
So I think this probably only worked on systems which don't use SPI.
What still confuses me, is that after a few successful transfers I keep
getting IRQs with TPM_INTF_CMD_READY_INT set, although the handler
clear that bit. If i read the TIS spec correctly, it should only be
asserted by a 0->1 transition of
TPM_INT_STATUS_x.commandReadyIntOccured flag, which shouldn't happend
so often.
Regards,
Jan
