Page MenuHomeFreeBSD

Introduce driver for TPM 2.0 in CRB and FIFO (TIS) modes.
ClosedPublic

Authored by mindal_semihalf.com on Nov 19 2018, 4:01 PM.

Details

Summary

Introduce driver for TPM 2.0 in CRB and FIFO (TIS) modes.

It was written basing on:
TCG PC Client Platform TPM Profile (PTP) Specification Version 22, Revision 1.03.
It only supports Locality 0.
Interrupts are only supported in FIFO mode.

Test Plan

The driver in FIFO mode was tested on x86 with Infineon SLB9665 discrete TPM chip.
Driver in both modes was also tested on qemu with swtpm running on host.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

cem added a comment.Dec 3 2018, 5:12 PM

Some initial comments. I'm not necessarily a good reviewer to wait on for this, sorry. I'm unfamiliar with TPM and don't have time to study it now.

sys/dev/tpm/tpm20.c
41–45 ↗(On Diff #50605)

With make_dev_s (see below), these checks shouldn't be needed (or could be made assertions, if you prefer). The devfs object lifetime is such that any threads in methods like these (d_read, etc) are drained before the object is destroyed. The logic is in destroy_devl().

164–166 ↗(On Diff #50605)

I'd encourage using make_dev_s, which avoids the race condition between make_dev and si_drv1 being set.

sys/dev/tpm/tpm20.h
134–135 ↗(On Diff #50605)

This seems like a really verbose and error-prone way of specifying a table of mostly defaults, with a few non-defaults. There are a couple ways you could represent this more succinctly and clearly: a table of only non-default cmd+timeout combinations, plus initialization of the larger table at load; (ab)using zero initialization to represent timeout_b and specifying the others with C99 array initializer syntax ([123] = TPM_TIMEOUT_C,); I'm sure you could think of others.

mindal_semihalf.com updated this revision to Diff 51576.EditedDec 4 2018, 1:00 PM

Addressed @cem comments.

mindal_semihalf.com marked 3 inline comments as done.Dec 4 2018, 1:01 PM
cem added inline comments.Dec 4 2018, 5:06 PM
sys/dev/tpm/tpm_crb.c
241–242 ↗(On Diff #51576)

Probably don't need two copies of this routine.

Did we lose some granularity? (I thought there were three different timeouts before.)

sys/dev/tpm/tpm_crb.c
241–242 ↗(On Diff #51576)

In case of a few commands the timeouts were smaller. This however shouldn't impact the performance very much. The main reason for having different timeouts is that the key creations can take a really long time - longer than specified in standard.

Move tpm_get_timeout to tpm20.c to remove duplication.

mindal_semihalf.com updated this revision to Diff 51728.EditedDec 7 2018, 5:10 PM

Add commands with lower timeout to tpm20_get_timeout.
Also separate functions are now called when device is about to suspend and shutdown.

This revision was not accepted when it landed; it landed in state Needs Review.Dec 14 2018, 4:14 PM
This revision was automatically updated to reflect the committed changes.