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.
Details
Details
- Reviewers
mst_semihalf.com mw imp emaste fabient cem - Group Reviewers
secteam - Commits
- rS342084: Introduce driver for TPM 2.0 in CRB and FIFO (TIS) modes
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
Diff Detail
- Lint
Lint Skipped - Unit
Tests Skipped
Event Timeline
Comment Actions
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 | ||
|---|---|---|
| 42–46 | 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(). | |
| 165–167 | 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 | ||
| 135–136 | 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. | |
| sys/dev/tpm/tpm_crb.c | ||
|---|---|---|
| 241–242 | 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 | 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. | |
Comment Actions
Add commands with lower timeout to tpm20_get_timeout.
Also separate functions are now called when device is about to suspend and shutdown.