Page MenuHomeFreeBSD

tpm: Do not use timed tsleep() while polling during cold boot
AcceptedPublic

Authored by jrm on Sat, Sep 26, 7:37 PM.
Tags
None
Referenced Files
F173607520: D60050.id187771.diff
Sun, Sep 27, 3:56 AM
F173590668: D60050.id187771.diff
Sun, Sep 27, 1:09 AM
F173590311: D60050.diff
Sun, Sep 27, 1:06 AM
F173590219: D60050.id187771.diff
Sun, Sep 27, 1:05 AM
F173590101: D60050.diff
Sun, Sep 27, 1:04 AM
F173584195: D60050.diff
Sun, Sep 27, 12:09 AM
F173574597: D60050.diff
Sat, Sep 26, 10:45 PM
F173574526: D60050.diff
Sat, Sep 26, 10:45 PM
Subscribers

Details

Summary

Commit 4e0f283fb97a made tpm_tis12_init() wait for TPM_STS_CMD_READY
after aborting any in-flight command. The wait is implemented by the
driver's existing tpm_waitfor_poll() loop, which sleeps with a one-tick
tsleep() between status reads. Until now that loop only ran from the
resume and command paths, after boot. From tpm_attach() it can panic
with "timed sleep before timers are working" when the TPM is attached
from ACPI during cold boot and the chip does not report ready on the
first status read.

Before 4e0f283fb97a, tpm_tis12_init() wrote TPM_STS_CMD_READY and
returned without waiting, so the polling loops only ran after boot.
tpm_request_locality() had the same latent hazard but its fast path
returns before sleeping whenever locality is already active.

Nothing calls wakeup() on the channels used by these polling loops, so
the sleeps are pure delays. Use pause_sig(), which falls back to
DELAY() while the kernel is cold and returns EWOULDBLOCK, a value these
loops already tolerate.

The c argument to tpm_waitfor_poll() is now unused. It is left in place
to keep this change minimal for MFC and can be removed in a follow-up.

Fixes: 4e0f283fb97a (tpm: Bound TPM 1.2 locality ownership)
MFC after: 1 week

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 77363
Build 74246: arc lint + arc unit

Event Timeline

jrm requested review of this revision.Sat, Sep 26, 7:37 PM

I tested this on an Intel Core i3-6100T desktop with a Winbond WEC WPCT200 rev 0x2 (TPM 1.2, TIS 1.2, no interrupt resource), attached via ACPI PNP0C31.

Without the patch, GENERIC from main at 53fe018630d0 panics during acpi_probe_children() every cold boot, immediately after printing "tpm: WEC WPCT200 rev 0x2". Booting with hint.tpm.0.disabled=1 avoids the panic.

With the patch applied on top of 53fe018630d0, the same machine boots with the hint unset.

One behavioural difference is that tsleep() was called with PRIBIO. I think this boost is meant for disk I/O completion, and I wonder if it was intentional here.

This is sufficient as a fix.

You have the right premise to remove the tsleep (it comes from old shared *BSD project). It ultimately should look like tpm20 tis. I rebased D59653 and D59657 on this with cold IRQ handling and started some of the conversion. Can you test those after landing this and make sure your hw still boots?

This revision is now accepted and ready to land.Sat, Sep 26, 11:16 PM