Page MenuHomeFreeBSD

tpm: Validate firmware interrupt routes and fall back cleanly
Needs ReviewPublic

Authored by kbowling on Mon, Sep 14, 12:42 AM.
Tags
None
Referenced Files
F172473841: D59657.diff
Fri, Sep 18, 5:39 PM
F172441741: D59657.diff
Fri, Sep 18, 12:01 PM
F172441697: D59657.diff
Fri, Sep 18, 12:01 PM
F172385751: D59657.diff
Fri, Sep 18, 3:17 AM
Unknown Object (File)
Thu, Sep 17, 7:44 PM
Unknown Object (File)
Wed, Sep 16, 7:17 PM
Unknown Object (File)
Wed, Sep 16, 11:22 AM
Unknown Object (File)
Mon, Sep 14, 1:43 AM
Subscribers

Details

Summary
tpm: Validate firmware interrupt routes and fall back cleanly

A host IRQ resource is not necessarily an LPC SIRQ selector.  Let the
frontend configure the route: ACPI uses the allocated _CRS interrupt and
_SRS when present, preserving the firmware selector, while ISA supplies a
separate, range-checked SIRQ.  Match trigger and polarity to the host route
and TPM capabilities, including opaque INTRNG resource mappings.
Pass the firmware descriptor unchanged to _SRS; apply the x86 ISA polarity
override only when computing the TPM mode.  ISA chips without rising-edge
support now poll instead of substituting a level mode for the host route.

For both FIFO drivers, require a complete GetRandom transport response and
a fresh DATA_AVAIL interrupt before selecting interrupt waits.  Perform
the test with polling before publishing the device, and repeat it after
resume.  Missing or failed routes release the handler and IRQ resource;
TPM 1.2 handler-setup failure now also falls back to polling.

Arm only DATA_AVAIL before GO.  Mask delivery on the first interrupt and
before relinquishing locality, preventing idle and repeated-source storms.
Bound interrupt sleeps so lost delivery can recover the current command
through polling.  Defer unexpected-source cleanup to a task that can
acquire locality, and drain it before destroying driver locks.  Preserve
an unread TPM 1.2 response when quiescing an already owned locality.

If the ithread cannot mask the TPM, yield for a tick with the host source
masked so a level storm cannot starve the command owner or cleanup worker.
Register INTR_SLEEPABLE | INTR_EXCL for both drivers, and reject firmware
shared routes before installing a handler.  Such routes now poll: FreeBSD
does not allow sleepable shared handlers.  Drain maskable events during
firmware programming without letting them validate or reject the IRQ test.

Retain the initial hardware-quiesce guarantee, including after _SRS:
attach fails when delivery cannot be disabled, and a later unrecoverable
quiesce failure suspends command access.  Known TPM 1.2 NOINTS quirks and
the TPM 2.0 polling hint remain effective.  Frontends without an explicit
route method use polling.
Test Plan

Validated with 158 mocked FIFO/IRQ/ACPI scenarios under ASan/UBSan and tpm.ko builds for amd64, i386, and arm64. Hardware tests passed 2,700 TPM 2.0 commands and 1,200 TPM 1.2 commands, including lifecycle cycles, TPM 2.0 harvesting, and S3 resume with WITNESS and INVARIANTS.

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

kbowling created this revision.

Note this builds on the parent D59653 and that should probably send as is since this is a lot more invasive. From a survey of my machines none present an IRQ.. @adrian you should see IRQ attachment if everything is sufficient here. I also lack arm64 hw.

without D_TRACKCLOSE, the callback is skipped and the exclusive-open flag stays set forever.

That sounds like a bug in devfs. I guess devfs_open() should bump the usecount only after a successful d_open, though that might not be quite right, the usecount is used for ttys as well and maybe that depends on the existing behaviour. I wrote D59725, could you please check whether that fixes the problem without D_TRACKCLOSE?

It probably makes sense for this to ditch d_close entirely in favor of devfs_set_cdevpriv(9). This is how we typically get reliable semantics. @kib has a writeup of why d_close is shit for this somewhere, but I always have to go trolling through mail history to find it

kbowling edited the summary of this revision. (Show Details)

Drop D_TRACKCLOSE