Skip to contentSkip to conversation

tpm: Rework timeout and duration handling

RFC PATCH In review Surendran Kanagaraj 2 participants 14 messages Nov 12 – Nov 14

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

v3 cover tpm: Rework timeout and duration handling

Review of v2 [1] asked to redesign how the timeouts and durations are handled across TPM families and drivers. This series is my attempt at it. Sorry for the slow progress.

- Interface timeouts live in four fields, TPM1.x duration in an array which uses duration class and TPM2.0 in a different map with raw time values and other duration macros. Move interface timeouts and duration classes to a new struct called struct tpm_limits embedded in struct tpm_chip so that core and most drivers read them from one place.

- Currently it's left up to the driver to initialize the timeout values. During probe, core/driver runs tpm_get_timeouts() which on TPM1.x queries the TPM for timeout values. The transmit function used for this request uses these timeouts for the state changes which can be zero at this time. I found a case with xen-tpmfront where it calls tpm get capability with TIMEOUT_C at zero. Hence the limit table is initialized with default value which is highest among both TPM1.x and TPM2. This makes sure driver is not missing to initialize during probe.

- Add module param option for all duration and timeouts. This gives an option of passing these values via kernel command line.

Current flow:

tpm_chip_alloc() Each driver sets its own default timeouts
or none
tpm1_get_timeouts() reported values, ->update_timeouts()

                               and ->update_durations(), into
                               jiffies fields and chip->duration[]

tpm2_get_timeouts() fixed TPM2_TIMEOUT_A..D, no driver hook
tpm_calc_ordinal_duration() chip->duration[] on TPM 1.x, a fixed
per-ordinal map on TPM 2.0
tpm_transmit() resubmits on TPM2_RC_RETRY until the
delay passes TPM2_DURATION_LONG

The limit table is built in fixed order starting with family defaults, reported by TPM chip, ->adjust_limits, tpm.limits= parameter and finally the validation that checks the bounds. This also adds a visibility to the source which sets the limit eg: default, TPM, driver, param etc.

The flow after this series:

tpm_chip_alloc()
tpm_limits_init() larger of the TPM 1.x and 2.0 defaults,
tpm.limits= and bounds check
tpm1_get_timeouts(), tpm2_get_timeouts()
tpm_limits_update() family defaults
reported values (TPM 1.x) ->adjust_limits() tpm.limits=
bounds check
readers of chip->limits
tpm_try_transmit() polls for completion up to the command's
duration class
tpm_transmit() resubmits on TPM2_RC_RETRY up to the
LONG duration
drivers tpm_poll_timeout(), or the limit in
jiffies where a driver counts jiffies
sysfs timeouts and durations, on TPM 2.0 too

This changes some of the existing behaviour:

- TPM2_VerifySignature waits DEFAULT 120 s instead of 30 s.
- Completion polling first sleeps at most 100 us instead of 1 ms.
- Until the timing query runs, tpm_crb waits up to 750 ms for TIMEOUT_C instead of 200 ms, xen-tpmfront 750 ms instead of 0, and st33zp24, tpm_i2c_infineon and tpm_i2c_nuvoton up to 4 s for TIMEOUT_B instead of 2 s.
- On TPM 1.x, an ordinal with no class waits DEFAULT, which is never shorter than LONG, so a chip that reports LONG above two minutes waits longer than the old fixed two minutes.
- The timeouts and durations sysfs files now print exact microseconds, and their "[adjusted]" marker also covers a value changed by validation. TPM 2.0 chips now get both files too, in /sys/class/tpm/tpmX/. tpm.limits= is documented in kernel-parameters.txt.

I ran each patch in QEMU with swtpm, on TPM 2.0 over CRB and TIS and on TPM 1.2 over TIS. Every boot answered commands and the TPM 1.2 sysfs output stays the same across the series. I also fed tpm.limits= values below the floor, above the ceiling and malformed, and with a local test hook held the TPM in TPM2_RC_RETRY and stalled completions to see each wait end where the table says.

On an EC2 instance with Amazon NitroTPM, I ran a GetCapability and hwrng loop through repeated periods of TPM unavailability. Kernel without these changes logged "tpm2_load_context: failed with a TPM error 0x0922" and every later TPM command failed. With this series applied, no command failed.

The idea for this series is mine. An LLM coding assistant helped draft the code and I reviewed every patch myself.

[1] https://lore.kernel.org/linux-integrity/[email protected]/

Surendran

V2 -> V3:
- Replace the TPM 2.0 duration override with one limits table for both TPM families, built in a fixed order and validated
- Replace ->update_timeouts() and ->update_durations() with ->adjust_limits()
- Bound the tpm_transmit() retry loop by the LONG duration instead of TPM2_DURATION_LONG
- Add tpm.limits=, and show the timeouts and durations files for TPM 2.0 chips
- Move the NitroTPM values to a tpm_crb quirk table, raise its SHORT and MEDIUM durations to 30 s and its LONG duration to 45 s

V1 -> V2:
- Drop chip->busy_timeout_ms. TPM2 durations and the TPM2_RC_RETRY budget now come from chip->duration.
- Map TPM2_CC_SELF_TEST and TPM2_CC_VERIFY_SIGNATURE to TPM_LONG with floors of 3s and 30s since their durations do not fit SHORT, MEDIUM, LONG durations.
- Keep the TPM2 defaults for commands sent before tpm2_get_timeouts() has run such as the one in tpm2_probe() and keep the 2s retry budget for TPM 1.x chips.
- Keep the first TPM2_RC_RETRY wait at TPM2_DURATION_SHORT instead of chip->duration[TPM_SHORT]. No TPM2 command maps to TPM_SHORT today and a driver that raises TPM_SHORT for its commands should not also lengthen the first retry wait.
- Return early from tpm_tis_update_durations() for TPM2 chips, since its overrides are for TPM 1.x.
- Raise TPM_SHORT, TPM_MEDIUM and TPM_LONG to 30s for NitroTPM.
- Replace the vendor quirk table in tpm_crb with a plain vendor ID check.
- Use the TPM_CRB_ prefix for the new tpm_crb macros.
- Rename the patches from "tpm: Add per-chip timeout for transient unavailability" and "tpm_crb: Raise timeouts for Amazon NitroTPM".

v2: https://lore.kernel.org/linux-integrity/[email protected]/
v1: https://lore.kernel.org/linux-integrity/[email protected]/

Surendran Kanagaraj (8):
tpm: Add a single table for all TPM wait limits
tpm: Wire the timing setup to the limits table
tpm: Take command durations from the limits table
tpm: Bound core and CRB waits by the limits table
tpm: Drop the jiffies timing fields from struct tpm_chip
tpm: Add the tpm.limits= parameter
tpm: Show timeouts and durations for TPM 2.0 chips
tpm_crb: Raise the limits for Amazon NitroTPM

 Documentation/ABI/stable/sysfs-class-tpm      |  12 +-
 .../admin-guide/kernel-parameters.txt         |  13 +
 drivers/char/tpm/Makefile                     |   1 +
 drivers/char/tpm/st33zp24/st33zp24.c          |  28 +-
 drivers/char/tpm/tpm-chip.c                   |   1 +
 drivers/char/tpm/tpm-interface.c              |  72 +-
 drivers/char/tpm/tpm-limits.c                 | 481 +++++++++++++
 drivers/char/tpm/tpm-sysfs.c                  |  29 +-
 drivers/char/tpm/tpm.h                        |  25 +-
 drivers/char/tpm/tpm1-cmd.c                   | 669 +++++++++---------
 drivers/char/tpm/tpm2-cmd.c                   |  80 ++-
 drivers/char/tpm/tpm_crb.c                    | 120 +++-
 drivers/char/tpm/tpm_i2c_atmel.c              |   9 -
 drivers/char/tpm/tpm_i2c_infineon.c           |  25 +-
 drivers/char/tpm/tpm_i2c_nuvoton.c            |  32 +-
 drivers/char/tpm/tpm_tis_core.c               | 174 ++---
 drivers/char/tpm/tpm_tis_i2c_cr50.c           |  15 +-
 drivers/char/tpm/xen-tpmfront.c               |  10 +-
 include/linux/tpm.h                           |  72 +-
 include/linux/tpm_ptp.h                       |   9 -
 20 files changed, 1188 insertions(+), 689 deletions(-)
 create mode 100644 drivers/char/tpm/tpm-limits.c

base-commit: ca61ec17cf17a774a11c565c2167619eea32ebfb

Signature · 1 line
2.47.3
#1 lore raw

RFC PATCH v3 1/8 tpm: Add a single table for all TPM wait limits

Every TPM wait takes its limit from a different place. The interface timeouts live in four jiffies fields, the TPM 1.x durations in an array, the TPM 2.0 durations in a per-ordinal map and the retry bound in a constant. A driver cannot see or adjust these values in one place. Apart from few TPM1.x fixups, nothing bounds checks these values. Also a driver can send a command before any timeout is set, xen-tpmfront calls tpm_get_timeouts() from probe before anything has set a timeout, so its TPM 1.x timeout query waits on the vTPM with a TIMEOUT_C of zero.

Add struct tpm_limits with one entry per limit: the interface timeouts TIMEOUT_A to TIMEOUT_D and the duration classes SHORT, MEDIUM, LONG, LONG_LONG and DEFAULT. Each entry holds the value in microseconds, its source (the defaults, the TPM or the driver) and whether validation clamped it.

tpm_chip_alloc() fills the table with the larger of the TPM 1.x and TPM 2.0 defaults, so a wait has a usable limit before the TPM is queried. Once the family is known, tpm_limits_update() rebuilds the table (next patch calls it from timing query) in a fixed order:
the family defaults, the values the TPM reported, the driver's new ->adjust_limits() hook, then validation. Validation applies a floor and a ceiling, keeps SHORT to LONG_LONG in increasing order and keeps DEFAULT no shorter than LONG.

The hook sets values with tpm_limits_driver_set() or from a quirk table through tpm_limits_apply_quirks(). tpm_poll_timeout() waits on a condition for a limit from the table. No limits are read from the table yet. The following patches move them to it.

---
 drivers/char/tpm/Makefile     |   1 +
 drivers/char/tpm/tpm-chip.c   |   1 +
 drivers/char/tpm/tpm-limits.c | 368 ++++++++++++++++++++++++++++++++++
 drivers/char/tpm/tpm.h        |  13 ++
 include/linux/tpm.h           |  54 +++++
 5 files changed, 437 insertions(+)
 create mode 100644 drivers/char/tpm/tpm-limits.c
drivers/char/tpm/Makefile +1 -0
index 5b5cdc0d32e4..92f469218e15 100644
@@ -12,6 +12,7 @@ tpm-y += tpm2-cmd.o
 tpm-y += tpmrm-dev.o
 tpm-y += tpm2-space.o
 tpm-y += tpm-sysfs.o
+tpm-y += tpm-limits.o
 tpm-y += eventlog/common.o
 tpm-y += eventlog/tpm1.o
 tpm-y += eventlog/tpm2.o
drivers/char/tpm/tpm-chip.c +1 -0
index 0be5dbfaa72e..11a050c43e10 100644
@@ -319,6 +319,7 @@ struct tpm_chip *tpm_chip_alloc(struct device *pdev,
 	}
 
 	chip->locality = -1;
+	tpm_limits_init(chip);
 	return chip;
 
 out:
drivers/char/tpm/tpm-limits.c new file +368 -0

368 lines not on this page · raw message

drivers/char/tpm/tpm.h +13 -0
index fa554c5ad80b..be88011f9618 100644
@@ -75,6 +75,19 @@ ssize_t tpm1_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap,
 int tpm1_get_random(struct tpm_chip *chip, u8 *out, size_t max);
 int tpm1_get_pcr_allocation(struct tpm_chip *chip);
 unsigned long tpm_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
+
+void tpm_limits_init(struct tpm_chip *chip);
+void tpm_limits_update(struct tpm_chip *chip, const u32 *reported);
+void tpm_limits_driver_set(struct tpm_limits *p, enum tpm_limit limit, u32 us);
+void tpm_limits_apply_quirks(struct tpm_limits *p,
+			     const struct tpm_limits_quirk *quirks, size_t n,
+			     u32 id);
+int tpm_poll_timeout(struct tpm_chip *chip, enum tpm_limit limit,
+		     bool (*cond)(struct tpm_chip *chip, void *arg), void *arg);
+int tpm_poll_timeout_us(struct tpm_chip *chip, u32 timeout_us,
+			bool (*cond)(struct tpm_chip *chip, void *arg),
+			void *arg);
+
 int tpm_pm_suspend(struct device *dev);
 int tpm_pm_resume(struct device *dev);
 int tpm_class_shutdown(struct device *dev);
include/linux/tpm.h +54 -0

78 lines not on this page · raw message

Signature · 1 line
2.47.3
maintainer
#2 lore raw

Re: [RFC PATCH v3 1/8] tpm: Add a single table for all TPM wait limits

Quoted 25 lines · Surendran Kanagaraj

Every TPM wait takes its limit from a different place. The interface timeouts live in four jiffies fields, the TPM 1.x durations in an array, the TPM 2.0 durations in a per-ordinal map and the retry bound in a constant. A driver cannot see or adjust these values in one place. Apart from few TPM1.x fixups, nothing bounds checks these values. Also a driver can send a command before any timeout is set, xen-tpmfront calls tpm_get_timeouts() from probe before anything has set a timeout, so its TPM 1.x timeout query waits on the vTPM with a TIMEOUT_C of zero.

Add struct tpm_limits with one entry per limit: the interface timeouts TIMEOUT_A to TIMEOUT_D and the duration classes SHORT, MEDIUM, LONG, LONG_LONG and DEFAULT. Each entry holds the value in microseconds, its source (the defaults, the TPM or the driver) and whether validation clamped it.

tpm_chip_alloc() fills the table with the larger of the TPM 1.x and TPM 2.0 defaults, so a wait has a usable limit before the TPM is queried. Once the family is known, tpm_limits_update() rebuilds the table (next patch calls it from timing query) in a fixed order:
the family defaults, the values the TPM reported, the driver's new ->adjust_limits() hook, then validation. Validation applies a floor and a ceiling, keeps SHORT to LONG_LONG in increasing order and keeps DEFAULT no shorter than LONG.

Quoted 2 lines

The hook sets values with tpm_limits_driver_set() or from a quirk table through tpm_limits_apply_quirks(). tpm_poll_timeout() waits on

OK, but ...

I.e. I'm not fully following what is the diffence between these call paths.

Quoted 43 lines

a condition for a limit from the table. No limits are read from the table yet. The following patches move them to it.

---
 drivers/char/tpm/Makefile     |   1 +
 drivers/char/tpm/tpm-chip.c   |   1 +
 drivers/char/tpm/tpm-limits.c | 368 ++++++++++++++++++++++++++++++++++
 drivers/char/tpm/tpm.h        |  13 ++
 include/linux/tpm.h           |  54 +++++
 5 files changed, 437 insertions(+)
 create mode 100644 drivers/char/tpm/tpm-limits.c
drivers/char/tpm/Makefile +1 -0
index 5b5cdc0d32e4..92f469218e15 100644
@@ -12,6 +12,7 @@ tpm-y += tpm2-cmd.o
 tpm-y += tpmrm-dev.o
 tpm-y += tpm2-space.o
 tpm-y += tpm-sysfs.o
+tpm-y += tpm-limits.o
 tpm-y += eventlog/common.o
 tpm-y += eventlog/tpm1.o
 tpm-y += eventlog/tpm2.o
drivers/char/tpm/tpm-chip.c +1 -0
index 0be5dbfaa72e..11a050c43e10 100644
@@ -319,6 +319,7 @@ struct tpm_chip *tpm_chip_alloc(struct device *pdev,
 	}
 
 	chip->locality = -1;
+	tpm_limits_init(chip);
 	return chip;
 
 out:
drivers/char/tpm/tpm-limits.c new file +0 -0
new file mode 100644
index 000000000000..3fe5affa1522

drivers/char/tpm/tpm-limits.c line 3

Quoted 3 lines
drivers/char/tpm/tpm-limits.c new file +3 -0
@@ -0,0 +1,368 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Handles time limits for a TPM chip.

I'd extend this to just have brief description how this works. Just a high-level overview.

Quoted 354 lines
drivers/char/tpm/tpm-limits.c new file +354 -0
+ */
+
+#include <linux/kernel.h>
+#include <linux/ktime.h>
+#include <linux/string.h>
+#include <linux/tpm_ptp.h>
+#include "tpm.h"
+
+/* Bounds for the limits */
+#define TPM_LIMIT_FLOOR_US	        (2 * TPM_POLL_DURATION_MAX_US)
+#define TPM_LIMIT_INTF_CEILING_US	(60 * USEC_PER_SEC)
+#define TPM_LIMIT_DURATION_CEILING_US	(30 * 60 * USEC_PER_SEC)
+
+#define TPM_POLL_START_US		100
+#define TPM_POLL_INTF_MAX_US		100
+#define TPM_POLL_DURATION_MAX_US	1000
+
+static const u32 tpm1_defaults[TPM_NR_LIMITS] = {
+	[TPM_LIMIT_A]		= TIS_SHORT_TIMEOUT * USEC_PER_MSEC,
+	[TPM_LIMIT_B]		= TIS_LONG_TIMEOUT * USEC_PER_MSEC,
+	[TPM_LIMIT_C]		= TIS_SHORT_TIMEOUT * USEC_PER_MSEC,
+	[TPM_LIMIT_D]		= TIS_SHORT_TIMEOUT * USEC_PER_MSEC,
+
+	[TPM_LIMIT_SHORT]	= 120 * USEC_PER_SEC,
+	[TPM_LIMIT_MEDIUM]	= 120 * USEC_PER_SEC,
+	[TPM_LIMIT_LONG]	= 120 * USEC_PER_SEC,
+	[TPM_LIMIT_LONG_LONG]	= 120 * USEC_PER_SEC,
+	[TPM_LIMIT_DEFAULT]	= 120 * USEC_PER_SEC,
+};
+
+static const u32 tpm2_defaults[TPM_NR_LIMITS] = {
+	[TPM_LIMIT_A]		= TPM2_TIMEOUT_A * USEC_PER_MSEC,
+	[TPM_LIMIT_B]		= TPM2_TIMEOUT_B * USEC_PER_MSEC,
+	[TPM_LIMIT_C]		= TPM2_TIMEOUT_C * USEC_PER_MSEC,
+	[TPM_LIMIT_D]		= TPM2_TIMEOUT_D * USEC_PER_MSEC,
+
+	[TPM_LIMIT_SHORT]	= 750 * USEC_PER_MSEC,
+	[TPM_LIMIT_MEDIUM]	= 2000 * USEC_PER_MSEC,
+	[TPM_LIMIT_LONG]	= 3000 * USEC_PER_MSEC,
+	[TPM_LIMIT_LONG_LONG]	= TPM2_DURATION_LONG_LONG * USEC_PER_MSEC,
+	[TPM_LIMIT_DEFAULT]	= TPM2_DURATION_DEFAULT * USEC_PER_MSEC,
+};
+
+static const char * const tpm_limit_names[TPM_NR_LIMITS] = {
+	[TPM_LIMIT_A]		= "timeout_a",
+	[TPM_LIMIT_B]		= "timeout_b",
+	[TPM_LIMIT_C]		= "timeout_c",
+	[TPM_LIMIT_D]		= "timeout_d",
+	[TPM_LIMIT_SHORT]	= "short",
+	[TPM_LIMIT_MEDIUM]	= "medium",
+	[TPM_LIMIT_LONG]	= "long",
+	[TPM_LIMIT_LONG_LONG]	= "long_long",
+	[TPM_LIMIT_DEFAULT]	= "default",
+};
+
+static const char * const tpm_limit_labels[TPM_NR_LIMITS] = {
+	[TPM_LIMIT_A]		= "TIMEOUT_A",
+	[TPM_LIMIT_B]		= "TIMEOUT_B",
+	[TPM_LIMIT_C]		= "TIMEOUT_C",
+	[TPM_LIMIT_D]		= "TIMEOUT_D",
+	[TPM_LIMIT_SHORT]	= "SHORT",
+	[TPM_LIMIT_MEDIUM]	= "MEDIUM",
+	[TPM_LIMIT_LONG]	= "LONG",
+	[TPM_LIMIT_LONG_LONG]	= "LONG_LONG",
+	[TPM_LIMIT_DEFAULT]	= "DEFAULT",
+};
+
+static const char * const tpm_limit_src_names[] = {
+	[TPM_LSRC_DEFAULT]	= "DEFAULT",
+	[TPM_LSRC_TPM]		= "TPM",
+	[TPM_LSRC_DRIVER]	= "DRIVER",
+};
+
+static u32 tpm_limits_default_us(enum tpm_family family, enum tpm_limit limit)
+{
+	switch (family) {
+	case TPM_FAMILY_1:
+		return tpm1_defaults[limit];
+	case TPM_FAMILY_2:
+		return tpm2_defaults[limit];
+	default:
+		return max(tpm1_defaults[limit], tpm2_defaults[limit]);
+	}
+}
+
+static u32 tpm_limits_ceiling_us(enum tpm_limit limit)
+{
+	if (TPM_LIMIT_INTF_MASK & BIT(limit))
+		return TPM_LIMIT_INTF_CEILING_US;
+
+	return TPM_LIMIT_DURATION_CEILING_US;
+}
+
+static void tpm_limits_store(struct tpm_limits *p, enum tpm_limit limit,
+			     u32 us, enum tpm_limit_src src, u32 flags)
+{
+	p->entry[limit].time_us = us;
+	p->entry[limit].src = src;
+	p->entry[limit].flags = flags;
+}
+
+static void tpm_limits_raise(struct tpm_limits *p, enum tpm_limit limit,
+			     u32 us)
+{
+	if (p->entry[limit].time_us < us) {
+		p->entry[limit].time_us = us;
+		p->entry[limit].flags |= TPM_LF_CLAMPED;
+	}
+}
+
+static void tpm_limits_stage_defaults(struct tpm_limits *p,
+				      enum tpm_family family)
+{
+	int limit;
+
+	memset(p, 0, sizeof(*p));
+	p->family = family;
+	for (limit = 0; limit < TPM_NR_LIMITS; limit++)
+		tpm_limits_store(p, limit,
+				 tpm_limits_default_us(family, limit),
+				 TPM_LSRC_DEFAULT, 0);
+}
+
+/* The values the TPM reported, in usecs; zero where it reported nothing. */
+static void tpm_limits_stage_reported(struct tpm_limits *p,
+				      const u32 *reported)
+{
+	int limit;
+
+	for (limit = 0; limit < TPM_NR_LIMITS; limit++) {
+		if (reported[limit])
+			tpm_limits_store(p, limit, reported[limit],
+					 TPM_LSRC_TPM, 0);
+	}
+}
+
+static void tpm_limits_stage_validate(struct tpm_limits *p)
+{
+	int limit;
+
+	for (limit = 0; limit < TPM_NR_LIMITS; limit++) {
+		struct tpm_limit_entry *e = &p->entry[limit];
+		u32 lo = TPM_LIMIT_FLOOR_US;
+		u32 hi = tpm_limits_ceiling_us(limit);
+
+		if (e->time_us < lo) {
+			e->time_us = lo;
+			e->flags |= TPM_LF_CLAMPED;
+		} else if (e->time_us > hi) {
+			e->time_us = hi;
+			e->flags |= TPM_LF_CLAMPED;
+		}
+	}
+
+	/*
+	 * Keep SHORT to LONG_LONG in increasing order, and DEFAULT no shorter
+	 * than LONG.
+	 */
+	for (limit = TPM_LIMIT_MEDIUM; limit <= TPM_LIMIT_LONG_LONG; limit++)
+		tpm_limits_raise(p, limit, p->entry[limit - 1].time_us);
+
+	tpm_limits_raise(p, TPM_LIMIT_DEFAULT,
+			 p->entry[TPM_LIMIT_LONG].time_us);
+}
+
+static void tpm_limits_commit(struct tpm_chip *chip,
+			      const struct tpm_limits *p)
+{
+	const struct tpm_limit_entry *e = p->entry;
+	int limit;
+
+	chip->limits = *p;
+
+	for (limit = 0; limit < TPM_NR_LIMITS; limit++)
+		dev_dbg(&chip->dev, "limit %s = %u us (%s%s)\n",
+			tpm_limit_names[limit], e[limit].time_us,
+			tpm_limit_src_names[e[limit].src],
+			e[limit].flags & TPM_LF_CLAMPED ? ", clamped" : "");
+}
+
+/**
+ * tpm_limits_init() - set the boot limits of a new chip
+ * @chip: the chip tpm_chip_alloc() is setting up
+ *
+ * The family is not known yet, so each limit gets the larger of the two
+ * families' defaults before validation applies.
+ * The driver's data is not attached yet, so its ->adjust_limits() hook first
+ * runs from tpm_limits_update().
+ */
+void tpm_limits_init(struct tpm_chip *chip)
+{
+	struct tpm_limits p;
+
+	tpm_limits_stage_defaults(&p, TPM_FAMILY_UNKNOWN);
+	tpm_limits_stage_validate(&p);
+	tpm_limits_commit(chip, &p);
+}
+
+/**
+ * tpm_limits_update() - build the limits once the TPM family is known
+ * @chip:	the TPM chip, started, with TPM_CHIP_FLAG_TPM2 set on TPM 2.0
+ * @reported:	TPM_NR_LIMITS values in usecs, indexed by enum tpm_limit,
+ *		zero where the TPM reported nothing; or NULL
+ *
+ * Runs the stages in order: the family defaults, @reported, the driver's
+ * ->adjust_limits() hook, then validation. The hook may send commands, so
+ * the chip must be started. Call it before the chip is registered or with
+ * tpm_mutex held, so that no command sees half-built limits.
+ */
+void tpm_limits_update(struct tpm_chip *chip, const u32 *reported)
+{
+	enum tpm_family family = chip->flags & TPM_CHIP_FLAG_TPM2 ?
+				 TPM_FAMILY_2 : TPM_FAMILY_1;
+	struct tpm_limits p;
+
+	tpm_limits_stage_defaults(&p, family);
+	if (reported)
+		tpm_limits_stage_reported(&p, reported);
+	if (chip->ops->adjust_limits)
+		chip->ops->adjust_limits(chip, &p);
+	tpm_limits_stage_validate(&p);
+	tpm_limits_commit(chip, &p);
+}
+
+/**
+ * tpm_limits_driver_set() - set a limit from a driver's ->adjust_limits() hook
+ * @p:		the limits the hook received
+ * @limit:	the limit to set
+ * @us:		the new value in microseconds
+ *
+ * Marks the driver as the source of @limit. Validation runs after the hook,
+ * so the value can still be clamped.
+ */
+void tpm_limits_driver_set(struct tpm_limits *p, enum tpm_limit limit, u32 us)
+{
+	if (WARN_ON_ONCE(limit >= TPM_NR_LIMITS))
+		return;
+
+	tpm_limits_store(p, limit, us, TPM_LSRC_DRIVER, 0);
+}
+EXPORT_SYMBOL_GPL(tpm_limits_driver_set);
+
+/**
+ * tpm_limits_apply_quirks() - apply the first matching quirk entry
+ * @p:		the limits the ->adjust_limits() hook received
+ * @quirks:	the driver's quirk table
+ * @n:		the number of entries in @quirks
+ * @id:		the device identity to match, as the driver defines it
+ *
+ * Applies the first entry whose id is @id. Each non-zero value in the entry
+ * is set as tpm_limits_driver_set() does.
+ */
+void tpm_limits_apply_quirks(struct tpm_limits *p,
+			     const struct tpm_limits_quirk *quirks, size_t n,
+			     u32 id)
+{
+	size_t i;
+	int limit;
+
+	for (i = 0; i < n; i++) {
+		if (quirks[i].id != id)
+			continue;
+
+		for (limit = 0; limit < TPM_NR_LIMITS; limit++) {
+			if (quirks[i].time_us[limit])
+				tpm_limits_driver_set(p, limit,
+						      quirks[i].time_us[limit]);
+		}
+		return;
+	}
+}
+EXPORT_SYMBOL_GPL(tpm_limits_apply_quirks);
+
+static u32 tpm_limits_us(struct tpm_chip *chip, enum tpm_limit limit)
+{
+	if (!chip)
+		return tpm_limits_default_us(TPM_FAMILY_UNKNOWN, limit);
+
+	return chip->limits.entry[limit].time_us;
+}
+
+static int tpm_poll(struct tpm_chip *chip, u32 timeout_us,
+		    enum tpm_limit limit,
+		    bool (*cond)(struct tpm_chip *chip, void *arg), void *arg)
+{
+	ktime_t timeout = ktime_add_us(ktime_get(), timeout_us);
+	unsigned int max_us = TPM_POLL_INTF_MAX_US;
+	unsigned int delay = TPM_POLL_START_US;
+	const char *label = "fixed timeout";
+	const char *src = "CALLER";
+
+	might_sleep();
+
+	if (limit < TPM_NR_LIMITS && !(TPM_LIMIT_INTF_MASK & BIT(limit)))
+		max_us = TPM_POLL_DURATION_MAX_US;
+
+	for (;;) {
+		if (cond(chip, arg))
+			return 0;
+		if (ktime_after(ktime_get(), timeout))
+			break;
+
+		usleep_range(delay / 2, delay);
+		delay = min(delay * 2, max_us);
+	}
+
+	if (cond(chip, arg))
+		return 0;
+	if (!chip)
+		return -ETIME;
+
+	if (limit < TPM_NR_LIMITS) {
+		label = tpm_limit_labels[limit];
+		src = tpm_limit_src_names[chip->limits.entry[limit].src];
+	}
+	dev_warn_ratelimited(&chip->dev, "%s expired after %u us (source %s)\n",
+			     label, timeout_us, src);
+	return -ETIME;
+}
+
+/**
+ * tpm_poll_timeout() - poll a condition until it holds or a limit passes
+ * @chip:	the TPM chip, or NULL before tpm_chip_alloc()
+ * @limit:	the limit that bounds the wait
+ * @cond:	returns true once the wait is over
+ * @arg:	passed to @cond
+ *
+ * Drivers pass an interface timeout, TPM_LIMIT_A to TPM_LIMIT_D, and the core
+ * passes a command's execution limit. Without a chip, the larger of the two
+ * families' defaults applies, @cond gets a NULL @chip and nothing is printed
+ * on expiry. @cond is checked once more after the deadline, so a TPM that
+ * finished while the caller was descheduled does not time out.
+ *
+ * Return: 0 once @cond holds, -ETIME if @limit passes first, or -EINVAL for
+ * a limit out of range.
+ */
+int tpm_poll_timeout(struct tpm_chip *chip, enum tpm_limit limit,
+		     bool (*cond)(struct tpm_chip *chip, void *arg), void *arg)
+{
+	if (WARN_ON_ONCE(limit >= TPM_NR_LIMITS))
+		return -EINVAL;
+
+	return tpm_poll(chip, tpm_limits_us(chip, limit), limit, cond, arg);
+}
+EXPORT_SYMBOL_GPL(tpm_poll_timeout);
+
+/**
+ * tpm_poll_timeout_us() - poll a condition with a driver-chosen timeout
+ * @chip:	the TPM chip, or NULL before tpm_chip_alloc()
+ * @timeout_us:	the timeout in microseconds
+ * @cond:	returns true once the wait is over
+ * @arg:	passed to @cond
+ *
+ * For a wait no TCG spec names, such as a mailbox doorbell. Otherwise as

drivers/char/tpm/tpm-limits.c line 367

Quoted 10 lines
drivers/char/tpm/tpm-limits.c new file +10 -0
+ * tpm_poll_timeout().
+ *
+ * Return: 0 once @cond holds, or -ETIME if @timeout_us passes first.
+ */
+int tpm_poll_timeout_us(struct tpm_chip *chip, u32 timeout_us,
+			bool (*cond)(struct tpm_chip *chip, void *arg),
+			void *arg)
+{
+	return tpm_poll(chip, timeout_us, TPM_NR_LIMITS, cond, arg);
+}

I did not check all the details but could this be reorganized into:

1. __tpm_poll(): the same as tpm_poll() but exported and defined in drivers/char/tpm.h.
2. tpm_poll(): a single static inline function declared fully at drivers/char/tpm.h

__tpm_poll() would e.g., make a nicer trace point for a kprobe or kretprobe.

Quoted 88 lines
drivers/char/tpm/tpm-limits.c new file +1 -0
+EXPORT_SYMBOL_GPL(tpm_poll_timeout_us);
drivers/char/tpm/tpm.h +13 -0
index fa554c5ad80b..be88011f9618 100644
@@ -75,6 +75,19 @@ ssize_t tpm1_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap,
 int tpm1_get_random(struct tpm_chip *chip, u8 *out, size_t max);
 int tpm1_get_pcr_allocation(struct tpm_chip *chip);
 unsigned long tpm_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
+
+void tpm_limits_init(struct tpm_chip *chip);
+void tpm_limits_update(struct tpm_chip *chip, const u32 *reported);
+void tpm_limits_driver_set(struct tpm_limits *p, enum tpm_limit limit, u32 us);
+void tpm_limits_apply_quirks(struct tpm_limits *p,
+			     const struct tpm_limits_quirk *quirks, size_t n,
+			     u32 id);
+int tpm_poll_timeout(struct tpm_chip *chip, enum tpm_limit limit,
+		     bool (*cond)(struct tpm_chip *chip, void *arg), void *arg);
+int tpm_poll_timeout_us(struct tpm_chip *chip, u32 timeout_us,
+			bool (*cond)(struct tpm_chip *chip, void *arg),
+			void *arg);
+
 int tpm_pm_suspend(struct device *dev);
 int tpm_pm_resume(struct device *dev);
 int tpm_class_shutdown(struct device *dev);
include/linux/tpm.h +41 -0
index 0db277af45c3..5454d87f43ba 100644
@@ -29,6 +29,7 @@
 #include <linux/tpm_buf.h>
 
 struct tpm_chip;
+struct tpm_limits;
 struct trusted_key_payload;
 struct trusted_key_options;
 /* opaque structure, holds auth session parameters like the session key */
@@ -52,6 +53,7 @@ struct tpm_class_ops {
 				unsigned long *timeout_cap);
 	void (*update_durations)(struct tpm_chip *chip,
 				 unsigned long *duration_cap);
+	void (*adjust_limits)(struct tpm_chip *chip, struct tpm_limits *p);
 	int (*go_idle)(struct tpm_chip *chip);
 	int (*cmd_ready)(struct tpm_chip *chip);
 	int (*request_locality)(struct tpm_chip *chip, int loc);
@@ -71,6 +73,56 @@ enum tpm_duration {
 	TPM_NUM_DURATIONS = TPM_UNDEFINED,
 };
 
+enum tpm_limit {
+	/* Interface handshake timeouts */
+	TPM_LIMIT_A,
+	TPM_LIMIT_B,
+	TPM_LIMIT_C,
+	TPM_LIMIT_D,
+
+	/* Command duration classes */
+	TPM_LIMIT_SHORT,
+	TPM_LIMIT_MEDIUM,
+	TPM_LIMIT_LONG,
+	TPM_LIMIT_LONG_LONG,
+	TPM_LIMIT_DEFAULT,
+
+	TPM_NR_LIMITS,
+};
+
+#define TPM_LIMIT_INTF_MASK		GENMASK(TPM_LIMIT_D, TPM_LIMIT_A)
+
+enum tpm_family {
+	TPM_FAMILY_1,		/* TPM 1.1 and 1.2 */
+	TPM_FAMILY_2,
+	TPM_FAMILY_UNKNOWN,
+};
+
+enum tpm_limit_src {
+	TPM_LSRC_DEFAULT,	/* default values */
+	TPM_LSRC_TPM,		/* reported by the TPM */
+	TPM_LSRC_DRIVER,	/* set by the driver */
+};
+
+#define TPM_LF_CLAMPED		BIT(0)	/* limit validation replaced the value */
+
+struct tpm_limit_entry {
+	u32 time_us;
+	enum tpm_limit_src src;
+	u32 flags;		/* TPM_LF_* */
+};
+

include/linux/tpm.h enum tpm_duration { · line 124

Quoted 10 lines
include/linux/tpm.h +10 -0
+struct tpm_limits {
+	enum tpm_family family;
+	struct tpm_limit_entry entry[TPM_NR_LIMITS];
+};
+
+/* per-device overrides based on id the driver defines e.g. DID/VID */
+struct tpm_limits_quirk {
+	u32 id;				/* driver-defined device identity */
+	u32 time_us[TPM_NR_LIMITS];	/* 0 leaves the limit alone */
+};

I think it would make more sense to fully initialize the database per device, if that is possible.

It is a non-concern if two TPM drivers do some redundant work, as it is not a real-world production use case :-)

I'm just wondering if this quirk/entry division.

I.e. in my world view chip owns its limits (not e.g., tpm2_space).

Quoted 16 lines
include/linux/tpm.h +3 -0
+
 #define TPM_PPI_VERSION_LEN		3
 
 struct tpm_space {
@@ -140,6 +192,8 @@ struct tpm_chip {
 	unsigned long duration[TPM_NUM_DURATIONS]; /* jiffies */
 	bool duration_adjusted;
 
+	struct tpm_limits limits;
+
 	struct dentry *bios_dir;
 
 	const struct attribute_group *groups[3 + TPM_MAX_HASHES];
Signature · 1 line
2.47.3

This is a great starting point.

Br, Jarkko.

maintainer
#3 lore raw

Re: [RFC PATCH v3 1/8] tpm: Add a single table for all TPM wait limits

Quoted 1 line · Jarkko Sakkinen

This is a great starting point.

I skimmed the other patches but it does not pay off to nitpick the details up until the very first patch is more or less settled.

Br, Jarkko

#4 lore raw

Re: [RFC PATCH v3 1/8] tpm: Add a single table for all TPM wait limits

In reply to Jarkko Sakkinen

Hi Jarkko,
Thanks for the review.

Quoted 8 lines · Jarkko Sakkinen
Quoted 2 lines · Surendran Kanagaraj

The hook sets values with tpm_limits_driver_set() or from a quirk table through tpm_limits_apply_quirks(). tpm_poll_timeout() waits on

OK, but ...

I.e. I'm not fully following what is the diffence between these call paths.

They are used from the same place. tpm_limits_update(), which is called from tpm*_get_timeouts(), builds the table. The driver hook ->adjust_limits() runs from tpm_limits_update() and uses tpm_limits_apply_quirks() to apply the quirks. tpm_limits_apply_quirks() finds the matching quirk entry and calls tpm_limits_driver_set() to update the limit.

tpm_limits_update(...)

{
    tpm_limits_stage_defaults(&p, family);
    if (reported)
        tpm_limits_stage_reported(&p, reported);
    if (chip->ops->adjust_limits)
        chip->ops->adjust_limits(chip, &p);
    tpm_limits_stage_validate(&p);
    tpm_limits_commit(chip, &p);
}

Hope this clarifies.

Quoted 5 lines
Quoted 2 lines

+/*
+ * Handles time limits for a TPM chip.

I'd extend this to just have brief description how this works. Just a high-level overview.

Ack. I will add a short overview in the next rev.

Quoted 23 lines
Quoted 10 lines
drivers/char/tpm/tpm-limits.c new file +9 -0
+static int tpm_poll(struct tpm_chip *chip, u32 timeout_us,
+		    enum tpm_limit limit,
+		    bool (*cond)(struct tpm_chip *chip, void *arg), void *arg)
[...]
+int tpm_poll_timeout(struct tpm_chip *chip, enum tpm_limit limit,
+		     bool (*cond)(struct tpm_chip *chip, void *arg), void *arg)
+{
+	if (WARN_ON_ONCE(limit >= TPM_NR_LIMITS))
+		return -EINVAL;
+

drivers/char/tpm/tpm-limits.c line 367

Quoted 10 lines
drivers/char/tpm/tpm-limits.c new file +9 -0
+	return tpm_poll(chip, tpm_limits_us(chip, limit), limit, cond, arg);
+}
+EXPORT_SYMBOL_GPL(tpm_poll_timeout);
[...]
+int tpm_poll_timeout_us(struct tpm_chip *chip, u32 timeout_us,
+			bool (*cond)(struct tpm_chip *chip, void *arg),
+			void *arg)
+{
+	return tpm_poll(chip, timeout_us, TPM_NR_LIMITS, cond, arg);
+}

I did not check all the details but could this be reorganized into:

Quoted 7 lines

1. __tpm_poll(): the same as tpm_poll() but exported and defined in drivers/char/tpm.h.
2. tpm_poll(): a single static inline function declared fully at drivers/char/tpm.h

__tpm_poll() would e.g., make a nicer trace point for a kprobe or kretprobe.

Good point. __tpm_poll() then becomes the single kprobe point. Will do this in the next rev.

Quoted 9 lines

include/linux/tpm.h enum tpm_duration { · line 124

Quoted 5 lines
include/linux/tpm.h +5 -0
+/* per-device overrides based on id the driver defines e.g. DID/VID */
+struct tpm_limits_quirk {
+	u32 id;				/* driver-defined device identity */
+	u32 time_us[TPM_NR_LIMITS];	/* 0 leaves the limit alone */
+};

I think it would make more sense to fully initialize the database per device, if that is possible.

Quoted 6 lines

It is a non-concern if two TPM drivers do some redundant work, as it is not a real-world production use case :-)

I'm just wondering if this quirk/entry division.

I.e. in my world view chip owns its limits (not e.g., tpm2_space).

Makes sense. The quirk table is just a convenience, a common table fmt for drivers, and the chip still owns the table. Without it the driver calls tpm_limits_driver_set() for each limit it changes. I am happy to drop it in the next rev.

Thanks,
Surendran

maintainer
#5 lore raw

Re: [RFC PATCH v3 1/8] tpm: Add a single table for all TPM wait limits

Quoted 4 lines · Surendran Kanagaraj

Makes sense. The quirk table is just a convenience, a common table fmt for drivers, and the chip still owns the table. Without it the driver calls tpm_limits_driver_set() for each limit it changes. I am happy to drop it in the next rev.

Yes as a general principle optimizing here "data redundancy" at the cost of more complex code does not pay off.

Quoted 3 lines

Thanks,
Surendran

Br, Jarkko

#6 lore raw

RFC PATCH v3 2/8 tpm: Wire the timing setup to the limits table

In reply to Surendran Kanagaraj

tpm1_get_timeouts() and tpm2_get_timeouts() still write the jiffies fields of struct tpm_chip directly and tpm_tis patches the TPM 1.x values through ->update_timeouts() and ->update_durations(). Build the limits table instead.

Both functions now call tpm_limits_update(). The TPM 1.x path passes what the chip reported after the existing fixups for chips. The table already keeps the default where the chip reports a zero. When the final values differ from the reported ones tpm1_get_timeouts() logs both for the durations as well as the timeouts.

The waits/polls still read the jiffies fields so tpm_limits_commit() copies the table into them. Since that already fills them with the defaults at tpm_chip_alloc(), tpm_tis_core_init() no longer uses TIS_TIMEOUT_*_MAX macros which are removed and xen-tpmfront gets a nonzero TIMEOUT_C for its timeout query.

The Atmel 3204 timeout quirk and the STMicroelectronics 0x104a duration quirk move to tpm_tis_adjust_limits() and the two old hooks are removed. The quirks use the DID/VID that tpm_tis_core_init() already reads so they no longer read the register under ->clk_enable().

The "[adjusted]" marker in the timeouts and durations sysfs files now means that a final value differs from what the chip reported. That was already true for the quirks and the fixups above. A value changed by validation is the new case. Update the ABI text which only described the msec scaling.

---
 Documentation/ABI/stable/sysfs-class-tpm |  12 +-
 drivers/char/tpm/tpm-limits.c            |  14 +++
 drivers/char/tpm/tpm1-cmd.c              | 147 +++++++++++------------
 drivers/char/tpm/tpm2-cmd.c              |   5 +-
 drivers/char/tpm/tpm_tis_core.c          | 143 ++++++----------------
 include/linux/tpm.h                      |   5 +-
 include/linux/tpm_ptp.h                  |   9 --
 7 files changed, 129 insertions(+), 206 deletions(-)
Documentation/ABI/stable/sysfs-class-tpm +6 -6
index 411d5895bed4..73b69f63b298 100644
@@ -61,10 +61,10 @@ Description:	The "durations" property shows the 3 vendor-specific values
 		Here the short, medium and long durations are displayed in
 		usecs. "[original]" indicates that the values are displayed
 		unmodified from when they were queried from the chip.
-		Durations can be modified in the case where a buggy chip
-		reports them in msec instead of usec and they need to be
-		scaled to be displayed in usecs. In this case "[adjusted]"
-		will be displayed in place of "[original]".
+		"[adjusted]" is displayed in place of "[original]" when a
+		value differs from what the chip reported, for example
+		because the chip reported msecs instead of usecs or a driver
+		quirk replaced the value.
 
 What:		/sys/class/tpm/tpmX/device/enabled
 Date:		April 2006
@@ -181,8 +181,8 @@ Description:	The "timeouts" property shows the 4 vendor-specific values
 		  750000 750000 750000 750000 [original]
 
 		The four timeout values are shown in usecs, with a trailing
-		"[original]" or "[adjusted]" depending on whether the values
-		were scaled by the driver to be reported in usec from msecs.
+		"[adjusted]" if any value differs from what the chip
+		reported, and "[original]" otherwise.
 
 What:		/sys/class/tpm/tpmX/tpm_version_major
 Date:		October 2019
drivers/char/tpm/tpm-limits.c +14 -0
index 3fe5affa1522..8e3c2afdd439 100644
@@ -174,6 +174,20 @@ static void tpm_limits_commit(struct tpm_chip *chip,
 
 	chip->limits = *p;
 
+	/* Copy the limits into the jiffies fields that the waits still read. */
+	chip->timeout_a = usecs_to_jiffies(e[TPM_LIMIT_A].time_us);
+	chip->timeout_b = usecs_to_jiffies(e[TPM_LIMIT_B].time_us);
+	chip->timeout_c = usecs_to_jiffies(e[TPM_LIMIT_C].time_us);
+	chip->timeout_d = usecs_to_jiffies(e[TPM_LIMIT_D].time_us);
+	chip->duration[TPM_SHORT] =
+		usecs_to_jiffies(e[TPM_LIMIT_SHORT].time_us);
+	chip->duration[TPM_MEDIUM] =
+		usecs_to_jiffies(e[TPM_LIMIT_MEDIUM].time_us);
+	chip->duration[TPM_LONG] =
+		usecs_to_jiffies(e[TPM_LIMIT_LONG].time_us);
+	chip->duration[TPM_LONG_LONG] =
+		usecs_to_jiffies(e[TPM_LIMIT_LONG_LONG].time_us);
+
 	for (limit = 0; limit < TPM_NR_LIMITS; limit++)
 		dev_dbg(&chip->dev, "limit %s = %u us (%s%s)\n",
 			tpm_limit_names[limit], e[limit].time_us,
drivers/char/tpm/tpm1-cmd.c +68 -79

177 lines not on this page · raw message

drivers/char/tpm/tpm2-cmd.c +1 -4
index 4b2bb2ddc321..f943f1c05721 100644
@@ -43,10 +43,7 @@ EXPORT_SYMBOL_GPL(tpm2_find_hash_alg);
 
 int tpm2_get_timeouts(struct tpm_chip *chip)
 {
-	chip->timeout_a = msecs_to_jiffies(TPM2_TIMEOUT_A);
-	chip->timeout_b = msecs_to_jiffies(TPM2_TIMEOUT_B);
-	chip->timeout_c = msecs_to_jiffies(TPM2_TIMEOUT_C);
-	chip->timeout_d = msecs_to_jiffies(TPM2_TIMEOUT_D);
+	tpm_limits_update(chip, NULL);
 	chip->flags |= TPM_CHIP_FLAG_HAVE_TIMEOUTS;
 	return 0;
 }
drivers/char/tpm/tpm_tis_core.c +39 -104

185 lines not on this page · raw message

include/linux/tpm.h +1 -4
index 5454d87f43ba..0b9661ffe45e 100644
@@ -49,10 +49,6 @@ struct tpm_class_ops {
 		    size_t cmd_len);
 	void (*cancel) (struct tpm_chip *chip);
 	u8 (*status) (struct tpm_chip *chip);
-	void (*update_timeouts)(struct tpm_chip *chip,
-				unsigned long *timeout_cap);
-	void (*update_durations)(struct tpm_chip *chip,
-				 unsigned long *duration_cap);
 	void (*adjust_limits)(struct tpm_chip *chip, struct tpm_limits *p);
 	int (*go_idle)(struct tpm_chip *chip);
 	int (*cmd_ready)(struct tpm_chip *chip);
@@ -91,6 +87,7 @@ enum tpm_limit {
 };
 
 #define TPM_LIMIT_INTF_MASK		GENMASK(TPM_LIMIT_D, TPM_LIMIT_A)
+#define TPM1_LIMIT_DURATION_MASK	GENMASK(TPM_LIMIT_LONG, TPM_LIMIT_SHORT)
 
 enum tpm_family {
 	TPM_FAMILY_1,		/* TPM 1.1 and 1.2 */
include/linux/tpm_ptp.h +0 -9
index e80ed810a269..3b6e7a854f70 100644
@@ -79,15 +79,6 @@ enum tis_x86_defaults {
 	LPC_CLKRUN_EN			= (1 << 2),
 };
 
-/*
- * Some timeout values are needed before it is known whether the chip is
- * TPM 1.0 or TPM 2.0.
- */
-#define TIS_TIMEOUT_A_MAX	max_t(int, TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_A)
-#define TIS_TIMEOUT_B_MAX	max_t(int, TIS_LONG_TIMEOUT, TPM2_TIMEOUT_B)
-#define TIS_TIMEOUT_C_MAX	max_t(int, TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_C)
-#define TIS_TIMEOUT_D_MAX	max_t(int, TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_D)
-
 #define	TPM_ACCESS(l)			(0x0000 | ((l) << 12))
 #define	TPM_INT_ENABLE(l)		(0x0008 | ((l) << 12))
 #define	TPM_INT_VECTOR(l)		(0x000C | ((l) << 12))
Signature · 1 line
2.47.3
#7 lore raw

RFC PATCH v3 3/8 tpm: Take command durations from the limits table

In reply to Surendran Kanagaraj

TPM 2.0 commands still get their wait from a fixed map of durations. The limits table never sees those values so they have no bounds and a driver cannot adjust them. TPM 1.x already maps each ordinal to a class through tpm1_ordinal_duration[] but reads the value from the jiffies copies in struct tpm_chip.

Map each TPM 2.0 ordinal to a class too and have tpm_calc_ordinal_duration() read the class's limit from the table for both families. An ordinal neither table knows gets DEFAULT.

With the default limits, TPM2_VerifySignature is the only command whose wait changes. Its 30 s matched no class, so it now gets DEFAULT 120 s. On TPM 1.x an unknown ordinal still waits two minutes, unless LONG ends up above that and validation raises DEFAULT to match.

---
 drivers/char/tpm/tpm-interface.c |  15 +-
 drivers/char/tpm/tpm.h           |   4 +-
 drivers/char/tpm/tpm1-cmd.c      | 518 +++++++++++++++----------------
 drivers/char/tpm/tpm2-cmd.c      |  58 ++--
 4 files changed, 294 insertions(+), 301 deletions(-)
drivers/char/tpm/tpm-interface.c +11 -4
index e0385b16616b..8e6ea9c04019 100644
@@ -40,6 +40,14 @@ module_param_named(suspend_pcr, tpm_suspend_pcr, uint, 0644);
 MODULE_PARM_DESC(suspend_pcr,
 		 "PCR to use for dummy writes to facilitate flush on suspend.");
 
+static enum tpm_limit tpm_duration_limit(struct tpm_chip *chip, u32 ordinal)
+{
+	if (chip->flags & TPM_CHIP_FLAG_TPM2)
+		return tpm2_duration_limit(ordinal);
+
+	return tpm1_duration_limit(ordinal);
+}
+
 /**
  * tpm_calc_ordinal_duration() - calculate the maximum command duration
  * @chip:    TPM chip to use.
@@ -52,10 +60,9 @@ MODULE_PARM_DESC(suspend_pcr,
  */
 unsigned long tpm_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal)
 {
-	if (chip->flags & TPM_CHIP_FLAG_TPM2)
-		return tpm2_calc_ordinal_duration(ordinal);
-	else
-		return tpm1_calc_ordinal_duration(chip, ordinal);
+	enum tpm_limit limit = tpm_duration_limit(chip, ordinal);
+
+	return usecs_to_jiffies(chip->limits.entry[limit].time_us);
 }
 EXPORT_SYMBOL_GPL(tpm_calc_ordinal_duration);
 
drivers/char/tpm/tpm.h +2 -2
index be88011f9618..1e023f12a400 100644
@@ -66,7 +66,7 @@ int tpm1_pm_suspend(struct tpm_chip *chip, u32 tpm_suspend_pcr);
 int tpm1_auto_startup(struct tpm_chip *chip);
 int tpm1_do_selftest(struct tpm_chip *chip);
 int tpm1_get_timeouts(struct tpm_chip *chip);
-unsigned long tpm1_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
+enum tpm_limit tpm1_duration_limit(u32 ordinal);
 int tpm1_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, const u8 *hash,
 		    const char *log_msg);
 int tpm1_pcr_read(struct tpm_chip *chip, u32 pcr_idx, u8 *res_buf);
@@ -132,7 +132,7 @@ ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,
 ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip);
 int tpm2_auto_startup(struct tpm_chip *chip);
 void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type);
-unsigned long tpm2_calc_ordinal_duration(u32 ordinal);
+enum tpm_limit tpm2_duration_limit(u32 ordinal);
 int tpm2_probe(struct tpm_chip *chip);
 int tpm2_get_cc_attrs_tbl(struct tpm_chip *chip);
 int tpm2_find_cc(struct tpm_chip *chip, u32 cc);
drivers/char/tpm/tpm1-cmd.c +253 -265

539 lines not on this page · raw message

drivers/char/tpm/tpm2-cmd.c +28 -30

75 lines not on this page · raw message

Signature · 1 line
2.47.3
#8 lore raw

RFC PATCH v3 4/8 tpm: Bound core and CRB waits by the limits table

In reply to Surendran Kanagaraj

Polls in tpm_try_transmit() still use fixed 1 ms sleeps and tpm_transmit() stops retrying once the next backoff would pass TPM2_DURATION_LONG and tpm_crb always waits TPM2_TIMEOUT_C. A value set through ->adjust_limits() reaches neither the retry loop nor the crb waits.

Poll for completion with tpm_poll_timeout(). Its sleeps start at 100 us and double up to 1 ms. It checks the condition once more after the deadline and an expired wait logs the limit and where its value came from. Bound the total sleep of the retry loop by the LONG duration, whose 3 s TPM 2.0 default keeps the current seven retries. Until the timing query sets the family, LONG holds the 120 s TPM 1.x default, so the loop uses the TPM 2.0 one instead. tpm_crb waits on TIMEOUT_C through tpm_poll_timeout(). The Pluton doorbell's completion wait, which no spec names keeps its 200 ms through tpm_poll_timeout_us().

Two waits behave differently. Completion polling first sleeps at most 100 us instead of 1 ms. Until tpm2_get_timeouts() runs, tpm_crb waits up to 750 ms for TIMEOUT_C, the larger of the two families' defaults, instead of 200 ms. From then on it waits 200 ms as before.

---
 drivers/char/tpm/tpm-interface.c | 57 +++++++++++++-------------
 drivers/char/tpm/tpm-limits.c    | 19 +++++++++
 drivers/char/tpm/tpm.h           |  2 +-
 drivers/char/tpm/tpm_crb.c       | 70 +++++++++++++++++---------------
 4 files changed, 87 insertions(+), 61 deletions(-)
drivers/char/tpm/tpm-interface.c +29 -28
index 8e6ea9c04019..0df39f4ba30b 100644
@@ -103,6 +103,16 @@ static void tpm_go_idle(struct tpm_chip *chip)
 		chip->ops->go_idle(chip);
 }
 
+/* Poll condition: the command completed or was canceled. */
+static bool tpm_cmd_done(struct tpm_chip *chip, void *arg)
+{
+	u8 *status = arg;
+
+	*status = tpm_chip_status(chip);
+	return tpm_transmit_completed(*status, chip) ||
+	       tpm_chip_req_canceled(chip, *status);
+}
+
 DEFINE_FREE(tpm_go_idle, struct tpm_chip *, if (_T) tpm_go_idle(_T))
 
 static ssize_t tpm_try_transmit(struct tpm_chip *chip, void *buf, size_t bufsiz)
@@ -112,7 +122,7 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip, void *buf, size_t bufsiz)
 	int rc;
 	ssize_t len = 0;
 	u32 count, ordinal;
-	unsigned long stop;
+	u8 status;
 
 	if (bufsiz < TPM_HEADER_SIZE)
 		return -EINVAL;
@@ -173,31 +183,19 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip, void *buf, size_t bufsiz)
 	if (chip->flags & TPM_CHIP_FLAG_IRQ)
 		goto out_recv;
 
-	stop = jiffies + tpm_calc_ordinal_duration(chip, ordinal);
-	do {
-		u8 status = tpm_chip_status(chip);
-		if (tpm_transmit_completed(status, chip))
-			goto out_recv;
-
-		if (tpm_chip_req_canceled(chip, status)) {
-			dev_err(&chip->dev, "Operation Canceled\n");
-			return -ECANCELED;
-		}
-
-		tpm_msleep(TPM_TIMEOUT_POLL);
-		rmb();
-	} while (time_before(jiffies, stop));
-
-	/*
-	 * Check for completion one more time, just in case the device reported
-	 * it while the driver was sleeping in the busy loop above.
-	 */
-	if (tpm_transmit_completed(tpm_chip_status(chip), chip))
-		goto out_recv;
+	if (tpm_poll_timeout(chip, tpm_duration_limit(chip, ordinal),
+			     tpm_cmd_done, &status)) {
+		tpm_chip_cancel(chip);
+		dev_err(&chip->dev, "Operation Timed out\n");
+		return -ETIME;
+	}
+	/* Read the response only after the status reported completion. */
+	rmb();
 
-	tpm_chip_cancel(chip);
-	dev_err(&chip->dev, "Operation Timed out\n");
-	return -ETIME;
+	if (!tpm_transmit_completed(status, chip)) {
+		dev_err(&chip->dev, "Operation Canceled\n");
+		return -ECANCELED;
+	}
 
 out_recv:
 	len = chip->ops->recv(chip, buf, bufsiz);
@@ -220,8 +218,8 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip, void *buf, size_t bufsiz)
  * @bufsiz:	length of the TPM command buffer
  *
  * A wrapper around tpm_try_transmit() that handles TPM2_RC_RETRY returns from
- * the TPM and retransmits the command after a delay up to a maximum wait of
- * TPM2_DURATION_LONG.
+ * the TPM and retransmits the command after a doubling delay, for as long as
+ * the sleeps fit in the chip's LONG duration limit.
  *
  * Note that TPM 1.x never returns TPM2_RC_RETRY so the retry logic is TPM 2.0
  * only.
@@ -236,6 +234,7 @@ ssize_t tpm_transmit(struct tpm_chip *chip, u8 *buf, size_t bufsiz)
 	/* space for header and handles */
 	u8 save[TPM_HEADER_SIZE + 3*sizeof(u32)];
 	unsigned int delay_msec = TPM2_DURATION_SHORT;
+	unsigned int slept_msec = 0;
 	u32 rc = 0;
 	ssize_t ret;
 	const size_t save_size = min(sizeof(save), bufsiz);
@@ -263,7 +262,8 @@ ssize_t tpm_transmit(struct tpm_chip *chip, u8 *buf, size_t bufsiz)
 		if (rc == TPM2_RC_TESTING && cc == TPM2_CC_SELF_TEST)
 			break;
 
-		if (delay_msec > TPM2_DURATION_LONG) {
+		if ((slept_msec + delay_msec) * USEC_PER_MSEC >
+		    tpm_limits_retry_us(chip)) {
 			if (rc == TPM2_RC_RETRY)
 				dev_err(&chip->dev, "in retry loop\n");
 			else
@@ -272,6 +272,7 @@ ssize_t tpm_transmit(struct tpm_chip *chip, u8 *buf, size_t bufsiz)
 			break;
 		}
 		tpm_msleep(delay_msec);
+		slept_msec += delay_msec;
 		delay_msec *= 2;
 		memcpy(buf, save, save_size);
 	}
drivers/char/tpm/tpm-limits.c +19 -0
index 8e3c2afdd439..a68c39b1e5ea 100644
@@ -288,6 +288,25 @@ void tpm_limits_apply_quirks(struct tpm_limits *p,
 }
 EXPORT_SYMBOL_GPL(tpm_limits_apply_quirks);
 
+/**
+ * tpm_limits_retry_us() - the total sleep allowed between resubmissions
+ * @chip:	the TPM chip
+ *
+ * The LONG duration bounds how long tpm_transmit() keeps resubmitting a
+ * command the TPM asked to retry. Until the timing query sets the family,
+ * LONG holds the larger TPM 1.x default, so use the TPM 2.0 one: only a
+ * TPM 2.0 chip asks for a retry.
+ *
+ * Return: the bound in microseconds.
+ */
+u32 tpm_limits_retry_us(struct tpm_chip *chip)
+{
+	if (chip->limits.family == TPM_FAMILY_UNKNOWN)
+		return tpm2_defaults[TPM_LIMIT_LONG];
+
+	return chip->limits.entry[TPM_LIMIT_LONG].time_us;
+}
+
 static u32 tpm_limits_us(struct tpm_chip *chip, enum tpm_limit limit)
 {
 	if (!chip)
drivers/char/tpm/tpm.h +1 -1
index 1e023f12a400..152cd392fa22 100644
@@ -40,7 +40,6 @@ enum tpm_timeout {
 	TPM_TIMEOUT = 5,	/* msecs */
 	TPM_TIMEOUT_RETRY = 100, /* msecs */
 	TPM_TIMEOUT_RANGE_US = 300,	/* usecs */
-	TPM_TIMEOUT_POLL = 1,	/* msecs */
 	TPM_TIMEOUT_USECS_MIN = 100,      /* usecs */
 	TPM_TIMEOUT_USECS_MAX = 500      /* usecs */
 };
@@ -82,6 +81,7 @@ void tpm_limits_driver_set(struct tpm_limits *p, enum tpm_limit limit, u32 us);
 void tpm_limits_apply_quirks(struct tpm_limits *p,
 			     const struct tpm_limits_quirk *quirks, size_t n,
 			     u32 id);
+u32 tpm_limits_retry_us(struct tpm_chip *chip);
 int tpm_poll_timeout(struct tpm_chip *chip, enum tpm_limit limit,
 		     bool (*cond)(struct tpm_chip *chip, void *arg), void *arg);
 int tpm_poll_timeout_us(struct tpm_chip *chip, u32 timeout_us,
drivers/char/tpm/tpm_crb.c +38 -32

149 lines not on this page · raw message

Signature · 1 line
2.47.3
#9 lore raw

RFC PATCH v3 5/8 tpm: Drop the jiffies timing fields from struct tpm_chip

In reply to Surendran Kanagaraj

timeout_a to timeout_d and duration[] in struct tpm_chip now only mirror the limits table. Some drivers still write initial values into the timeout fields at probe and tpm_limits_update() overwrites them.

Remove the fields and enum tpm_duration which only indexed duration[]. Add tpm_limits_jiffies(), which returns the value the field held, for the drivers that wait in jiffies. The timeouts and durations sysfs files read the table directly.

Two things behave differently. Until the timing query runs at registration, st33zp24, tpm_i2c_infineon and tpm_i2c_nuvoton wait up to 4 s for TIMEOUT_B, the larger of the two families' defaults, instead of 2 s. tpm_i2c_atmel also set 2 s but never waits on an interface timeout, and tpm_tis_i2c_cr50 already used 4 s. The timeouts and durations files print exact microseconds instead of a value rounded to a jiffy.

---
 drivers/char/tpm/st33zp24/st33zp24.c | 28 ++++++++----------------
 drivers/char/tpm/tpm-limits.c        | 14 ------------
 drivers/char/tpm/tpm-sysfs.c         | 27 ++++++++++-------------
 drivers/char/tpm/tpm.h               |  6 ++++++
 drivers/char/tpm/tpm1-cmd.c          |  4 ++--
 drivers/char/tpm/tpm_i2c_atmel.c     |  9 --------
 drivers/char/tpm/tpm_i2c_infineon.c  | 25 +++++++---------------
 drivers/char/tpm/tpm_i2c_nuvoton.c   | 32 +++++++++++++---------------
 drivers/char/tpm/tpm_tis_core.c      | 31 +++++++++++++++------------
 drivers/char/tpm/tpm_tis_i2c_cr50.c  | 15 +++++--------
 drivers/char/tpm/xen-tpmfront.c      | 10 +++++----
 include/linux/tpm.h                  | 15 -------------
 12 files changed, 79 insertions(+), 137 deletions(-)
drivers/char/tpm/st33zp24/st33zp24.c +9 -19
index 91d1278fe95a..5a2cf76ddea7 100644
@@ -56,11 +56,6 @@ enum st33zp24_int_flags {
 	TPM_INTF_DATA_AVAIL_INT = 0x001,
 };
 
-enum tis_defaults {
-	TIS_SHORT_TIMEOUT = 750,
-	TIS_LONG_TIMEOUT = 2000,
-};
-
 /*
  * clear the pending interrupt.
  */
@@ -132,7 +127,7 @@ static int request_locality(struct tpm_chip *chip)
 	if (ret < 0)
 		return ret;
 
-	stop = jiffies + chip->timeout_a;
+	stop = jiffies + tpm_limits_jiffies(chip, TPM_LIMIT_A);
 
 	/* Request locality is usually effective after the request */
 	do {
@@ -165,7 +160,7 @@ static int get_burstcount(struct tpm_chip *chip)
 	int burstcnt, status;
 	u8 temp;
 
-	stop = jiffies + chip->timeout_d;
+	stop = jiffies + tpm_limits_jiffies(chip, TPM_LIMIT_D);
 	do {
 		status = tpm_dev->ops->recv(tpm_dev->phy_id, TPM_STS + 1,
 					    &temp, 1);
@@ -270,7 +265,7 @@ static int recv_data(struct tpm_chip *chip, u8 *buf, size_t count)
 	while (size < count &&
 	       wait_for_stat(chip,
 			     TPM_STS_DATA_AVAIL | TPM_STS_VALID,
-			     chip->timeout_c,
+			     tpm_limits_jiffies(chip, TPM_LIMIT_C),
 			     &tpm_dev->read_queue, true) == 0) {
 		burstcnt = get_burstcount(chip);
 		if (burstcnt < 0)
@@ -320,9 +315,9 @@ static int st33zp24_send(struct tpm_chip *chip, unsigned char *buf,
 	status = st33zp24_status(chip);
 	if ((status & TPM_STS_COMMAND_READY) == 0) {
 		st33zp24_cancel(chip);
-		if (wait_for_stat
-		    (chip, TPM_STS_COMMAND_READY, chip->timeout_b,
-		     &tpm_dev->read_queue, false) < 0) {
+		if (wait_for_stat(chip, TPM_STS_COMMAND_READY,
+				  tpm_limits_jiffies(chip, TPM_LIMIT_B),
+				  &tpm_dev->read_queue, false) < 0) {
 			ret = -ETIME;
 			goto out_err;
 		}
@@ -467,11 +462,6 @@ int st33zp24_probe(void *phy_id, const struct st33zp24_phy_ops *ops,
 	tpm_dev->ops = ops;
 	dev_set_drvdata(&chip->dev, tpm_dev);
 
-	chip->timeout_a = msecs_to_jiffies(TIS_SHORT_TIMEOUT);
-	chip->timeout_b = msecs_to_jiffies(TIS_LONG_TIMEOUT);
-	chip->timeout_c = msecs_to_jiffies(TIS_SHORT_TIMEOUT);
-	chip->timeout_d = msecs_to_jiffies(TIS_SHORT_TIMEOUT);
-
 	tpm_dev->locality = LOCALITY0;
 
 	if (ACPI_COMPANION(dev)) {
@@ -569,9 +559,9 @@ int st33zp24_pm_resume(struct device *dev)
 
 	if (tpm_dev->io_lpcpd) {
 		gpiod_set_value_cansleep(tpm_dev->io_lpcpd, 1);
-		ret = wait_for_stat(chip,
-				TPM_STS_VALID, chip->timeout_b,
-				&tpm_dev->read_queue, false);
+		ret = wait_for_stat(chip, TPM_STS_VALID,
+				    tpm_limits_jiffies(chip, TPM_LIMIT_B),
+				    &tpm_dev->read_queue, false);
 	} else {
 		ret = tpm_pm_resume(dev);
 		if (!ret)
drivers/char/tpm/tpm-limits.c +0 -14
index a68c39b1e5ea..2cd51a9f27ba 100644
@@ -174,20 +174,6 @@ static void tpm_limits_commit(struct tpm_chip *chip,
 
 	chip->limits = *p;
 
-	/* Copy the limits into the jiffies fields that the waits still read. */
-	chip->timeout_a = usecs_to_jiffies(e[TPM_LIMIT_A].time_us);
-	chip->timeout_b = usecs_to_jiffies(e[TPM_LIMIT_B].time_us);
-	chip->timeout_c = usecs_to_jiffies(e[TPM_LIMIT_C].time_us);
-	chip->timeout_d = usecs_to_jiffies(e[TPM_LIMIT_D].time_us);
-	chip->duration[TPM_SHORT] =
-		usecs_to_jiffies(e[TPM_LIMIT_SHORT].time_us);
-	chip->duration[TPM_MEDIUM] =
-		usecs_to_jiffies(e[TPM_LIMIT_MEDIUM].time_us);
-	chip->duration[TPM_LONG] =
-		usecs_to_jiffies(e[TPM_LIMIT_LONG].time_us);
-	chip->duration[TPM_LONG_LONG] =
-		usecs_to_jiffies(e[TPM_LIMIT_LONG_LONG].time_us);
-
 	for (limit = 0; limit < TPM_NR_LIMITS; limit++)
 		dev_dbg(&chip->dev, "limit %s = %u us (%s%s)\n",
 			tpm_limit_names[limit], e[limit].time_us,
drivers/char/tpm/tpm-sysfs.c +11 -16
index d915307ae833..b289a7f6f085 100644
@@ -275,16 +275,13 @@ static ssize_t durations_show(struct device *dev, struct device_attribute *attr,
 			      char *buf)
 {
 	struct tpm_chip *chip = to_tpm_chip(dev);
+	const struct tpm_limit_entry *e = chip->limits.entry;
 
-	if (chip->duration[TPM_LONG] == 0)
-		return 0;
-
-	return sprintf(buf, "%d %d %d [%s]\n",
-		       jiffies_to_usecs(chip->duration[TPM_SHORT]),
-		       jiffies_to_usecs(chip->duration[TPM_MEDIUM]),
-		       jiffies_to_usecs(chip->duration[TPM_LONG]),
-		       chip->duration_adjusted
-		       ? "adjusted" : "original");
+	return sysfs_emit(buf, "%u %u %u [%s]\n",
+			  e[TPM_LIMIT_SHORT].time_us,
+			  e[TPM_LIMIT_MEDIUM].time_us,
+			  e[TPM_LIMIT_LONG].time_us,
+			  chip->duration_adjusted ? "adjusted" : "original");
 }
 static DEVICE_ATTR_RO(durations);
 
@@ -292,14 +289,12 @@ static ssize_t timeouts_show(struct device *dev, struct device_attribute *attr,
 			     char *buf)
 {
 	struct tpm_chip *chip = to_tpm_chip(dev);
+	const struct tpm_limit_entry *e = chip->limits.entry;
 
-	return sprintf(buf, "%d %d %d %d [%s]\n",
-		       jiffies_to_usecs(chip->timeout_a),
-		       jiffies_to_usecs(chip->timeout_b),
-		       jiffies_to_usecs(chip->timeout_c),
-		       jiffies_to_usecs(chip->timeout_d),
-		       chip->timeout_adjusted
-		       ? "adjusted" : "original");
+	return sysfs_emit(buf, "%u %u %u %u [%s]\n",
+			  e[TPM_LIMIT_A].time_us, e[TPM_LIMIT_B].time_us,
+			  e[TPM_LIMIT_C].time_us, e[TPM_LIMIT_D].time_us,
+			  chip->timeout_adjusted ? "adjusted" : "original");
 }
 static DEVICE_ATTR_RO(timeouts);
 
drivers/char/tpm/tpm.h +6 -0
index 152cd392fa22..a53c04b641fa 100644
@@ -82,6 +82,12 @@ void tpm_limits_apply_quirks(struct tpm_limits *p,
 			     const struct tpm_limits_quirk *quirks, size_t n,
 			     u32 id);
 u32 tpm_limits_retry_us(struct tpm_chip *chip);
+static inline unsigned long tpm_limits_jiffies(struct tpm_chip *chip,
+					       enum tpm_limit limit)
+{
+	return usecs_to_jiffies(chip->limits.entry[limit].time_us);
+}
+
 int tpm_poll_timeout(struct tpm_chip *chip, enum tpm_limit limit,
 		     bool (*cond)(struct tpm_chip *chip, void *arg), void *arg);
 int tpm_poll_timeout_us(struct tpm_chip *chip, u32 timeout_us,
drivers/char/tpm/tpm1-cmd.c +2 -2
index 117948a2b318..82a626866827 100644
@@ -406,8 +406,8 @@ int tpm1_get_timeouts(struct tpm_chip *chip)
 
 	/* The Broadcom BCM0102 chipset in a Dell Latitude D820 gets the above
 	 * value wrong and apparently reports msecs rather than usecs. So we
-	 * fix up the resulting too-small TPM_SHORT value to make things work.
-	 * We also scale the TPM_MEDIUM and -_LONG values by 1000.
+	 * fix up the resulting too-small SHORT value to make things work.
+	 * We also scale the MEDIUM and LONG values by 1000.
 	 */
 	if ((mask & BIT(TPM_LIMIT_SHORT)) &&
 	    val[TPM_LIMIT_SHORT] < 10 * USEC_PER_MSEC) {
drivers/char/tpm/tpm_i2c_atmel.c +0 -9
index ab058bfc9c99..78b193eb3da6 100644
@@ -24,9 +24,6 @@
 
 #define I2C_DRIVER_NAME "tpm_i2c_atmel"
 
-#define TPM_I2C_SHORT_TIMEOUT  750     /* ms */
-#define TPM_I2C_LONG_TIMEOUT   2000    /* 2 sec */
-
 #define ATMEL_STS_OK 1
 
 struct priv_data {
@@ -174,12 +171,6 @@ static int i2c_atmel_probe(struct i2c_client *client)
 	if (!priv)
 		return -ENOMEM;
 
-	/* Default timeouts */
-	chip->timeout_a = msecs_to_jiffies(TPM_I2C_SHORT_TIMEOUT);
-	chip->timeout_b = msecs_to_jiffies(TPM_I2C_LONG_TIMEOUT);
-	chip->timeout_c = msecs_to_jiffies(TPM_I2C_SHORT_TIMEOUT);
-	chip->timeout_d = msecs_to_jiffies(TPM_I2C_SHORT_TIMEOUT);
-
 	dev_set_drvdata(&chip->dev, priv);
 
 	/*
drivers/char/tpm/tpm_i2c_infineon.c +8 -17
index fffa235068df..0e7d08e0851d 100644
@@ -299,11 +299,6 @@ enum tis_status {
 	TPM_STS_DATA_EXPECT = 0x08,
 };
 
-enum tis_defaults {
-	TIS_SHORT_TIMEOUT = 750,	/* ms */
-	TIS_LONG_TIMEOUT = 2000,	/* 2 sec */
-};
-
 #define	TPM_ACCESS(l)			(0x0000 | ((l) << 4))
 #define	TPM_STS(l)			(0x0001 | ((l) << 4))
 #define	TPM_DATA_FIFO(l)		(0x0005 | ((l) << 4))
@@ -352,7 +347,7 @@ static int request_locality(struct tpm_chip *chip, int loc)
 	iic_tpm_write(TPM_ACCESS(loc), &buf, 1);
 
 	/* wait for burstcount */
-	stop = jiffies + chip->timeout_a;
+	stop = jiffies + tpm_limits_jiffies(chip, TPM_LIMIT_A);
 	do {
 		if (check_locality(chip, loc))
 			return loc;
@@ -394,7 +389,7 @@ static ssize_t get_burstcount(struct tpm_chip *chip)
 
 	/* wait for burstcount */
 	/* which timeout value, spec has 2 answers (c & d) */
-	stop = jiffies + chip->timeout_d;
+	stop = jiffies + tpm_limits_jiffies(chip, TPM_LIMIT_D);
 	do {
 		/* Note: STS is little endian */
 		if (iic_tpm_read(TPM_STS(tpm_dev.locality)+1, buf, 3) < 0)
@@ -497,7 +492,8 @@ static int tpm_tis_i2c_recv(struct tpm_chip *chip, u8 *buf, size_t count)
 		goto out;
 	}
 
-	wait_for_stat(chip, TPM_STS_VALID, chip->timeout_c, &status);
+	wait_for_stat(chip, TPM_STS_VALID,
+		      tpm_limits_jiffies(chip, TPM_LIMIT_C), &status);
 	if (status & TPM_STS_DATA_AVAIL) {	/* retry? */
 		dev_err(&chip->dev, "Error left over data\n");
 		size = -EIO;
@@ -534,7 +530,7 @@ static int tpm_tis_i2c_send(struct tpm_chip *chip, u8 *buf, size_t bufsiz,
 		tpm_tis_i2c_ready(chip);
 		if (wait_for_stat
 		    (chip, TPM_STS_COMMAND_READY,
-		     chip->timeout_b, &status) < 0) {
+		     tpm_limits_jiffies(chip, TPM_LIMIT_B), &status) < 0) {
 			rc = -ETIME;
 			goto out_err;
 		}
@@ -566,7 +562,7 @@ static int tpm_tis_i2c_send(struct tpm_chip *chip, u8 *buf, size_t bufsiz,
 		}
 
 		wait_for_stat(chip, TPM_STS_VALID,
-			      chip->timeout_c, &status);
+			      tpm_limits_jiffies(chip, TPM_LIMIT_C), &status);
 
 		if ((status & TPM_STS_DATA_EXPECT) == 0) {
 			rc = -EIO;
@@ -576,7 +572,8 @@ static int tpm_tis_i2c_send(struct tpm_chip *chip, u8 *buf, size_t bufsiz,
 
 	/* write last byte */
 	iic_tpm_write(TPM_DATA_FIFO(tpm_dev.locality), &(buf[count]), 1);
-	wait_for_stat(chip, TPM_STS_VALID, chip->timeout_c, &status);
+	wait_for_stat(chip, TPM_STS_VALID,
+		      tpm_limits_jiffies(chip, TPM_LIMIT_C), &status);
 	if ((status & TPM_STS_DATA_EXPECT) != 0) {
 		rc = -EIO;
 		goto out_err;
@@ -622,12 +619,6 @@ static int tpm_tis_i2c_init(struct device *dev)
 	if (IS_ERR(chip))
 		return PTR_ERR(chip);
 
-	/* Default timeouts */
-	chip->timeout_a = msecs_to_jiffies(TIS_SHORT_TIMEOUT);
-	chip->timeout_b = msecs_to_jiffies(TIS_LONG_TIMEOUT);
-	chip->timeout_c = msecs_to_jiffies(TIS_SHORT_TIMEOUT);
-	chip->timeout_d = msecs_to_jiffies(TIS_SHORT_TIMEOUT);
-
 	if (request_locality(chip, 0) != 0) {
 		dev_err(dev, "could not request locality\n");
 		rc = -ENODEV;
drivers/char/tpm/tpm_i2c_nuvoton.c +15 -17
index b01e42ad121e..15b70939ee39 100644
@@ -82,9 +82,6 @@ static s32 i2c_nuvoton_write_buf(struct i2c_client *client, u8 offset, u8 size,
 #define TPM_STS_RESPONSE_RETRY 0x02
 #define TPM_STS_ERR_VAL        0x07    /* bit2...bit0 reads always 0 */
 
-#define TPM_I2C_SHORT_TIMEOUT  750     /* ms */
-#define TPM_I2C_LONG_TIMEOUT   2000    /* 2 sec */
-
 /* read TPM_STS register */
 static u8 i2c_nuvoton_read_status(struct tpm_chip *chip)
 {
@@ -136,7 +133,7 @@ static void i2c_nuvoton_ready(struct tpm_chip *chip)
 static int i2c_nuvoton_get_burstcount(struct i2c_client *client,
 				      struct tpm_chip *chip)
 {
-	unsigned long stop = jiffies + chip->timeout_d;
+	unsigned long stop = jiffies + tpm_limits_jiffies(chip, TPM_LIMIT_D);
 	s32 status;
 	int burst_count = -1;
 	u8 data;
@@ -235,10 +232,11 @@ static int i2c_nuvoton_recv_data(struct i2c_client *client,
 	struct priv_data *priv = dev_get_drvdata(&chip->dev);
 	s32 rc;
 	int burst_count, bytes2read, size = 0;
+	unsigned long timeout_c = tpm_limits_jiffies(chip, TPM_LIMIT_C);
 
 	while (size < count &&
 	       i2c_nuvoton_wait_for_data_avail(chip,
-					       chip->timeout_c,
+					       timeout_c,
 					       &priv->read_queue) == 0) {
 		burst_count = i2c_nuvoton_get_burstcount(client, chip);
 		if (burst_count < 0) {
@@ -275,6 +273,7 @@ static int i2c_nuvoton_recv(struct tpm_chip *chip, u8 *buf, size_t count)
 	int retries;
 	int size = 0;
 	u32 expected;
+	unsigned long timeout_c = tpm_limits_jiffies(chip, TPM_LIMIT_C);
 
 	if (count < TPM_HEADER_SIZE) {
 		i2c_nuvoton_ready(chip);    /* return to idle */
@@ -292,7 +291,7 @@ static int i2c_nuvoton_recv(struct tpm_chip *chip, u8 *buf, size_t count)
 		 * tag, paramsize, and result
 		 */
 		status = i2c_nuvoton_wait_for_data_avail(
-			chip, chip->timeout_c, &priv->read_queue);
+			chip, timeout_c, &priv->read_queue);
 		if (status != 0) {
 			dev_err(dev, "%s() timeout on dataAvail\n", __func__);
 			size = -ETIMEDOUT;
@@ -332,7 +331,7 @@ static int i2c_nuvoton_recv(struct tpm_chip *chip, u8 *buf, size_t count)
 		}
 		if (i2c_nuvoton_wait_for_stat(
 			    chip, TPM_STS_VALID | TPM_STS_DATA_AVAIL,
-			    TPM_STS_VALID, chip->timeout_c,
+			    TPM_STS_VALID, timeout_c,
 			    NULL)) {
 			dev_err(dev, "%s() error left over data\n", __func__);
 			size = -ETIMEDOUT;
@@ -362,12 +361,14 @@ static int i2c_nuvoton_send(struct tpm_chip *chip, u8 *buf, size_t bufsiz,
 	unsigned long duration;
 	size_t count = 0;
 	int burst_count, bytes2write, retries, rc = -EIO;
+	unsigned long timeout_b = tpm_limits_jiffies(chip, TPM_LIMIT_B);
+	unsigned long timeout_c = tpm_limits_jiffies(chip, TPM_LIMIT_C);
 
 	for (retries = 0; retries < TPM_RETRY; retries++) {
 		i2c_nuvoton_ready(chip);
 		if (i2c_nuvoton_wait_for_stat(chip, TPM_STS_COMMAND_READY,
 					      TPM_STS_COMMAND_READY,
-					      chip->timeout_b, NULL)) {
+					      timeout_b, NULL)) {
 			dev_err(dev, "%s() timeout on commandReady\n",
 				__func__);
 			rc = -EIO;
@@ -399,7 +400,7 @@ static int i2c_nuvoton_send(struct tpm_chip *chip, u8 *buf, size_t bufsiz,
 						       TPM_STS_EXPECT,
 						       TPM_STS_VALID |
 						       TPM_STS_EXPECT,
-						       chip->timeout_c,
+						       timeout_c,
 						       NULL);
 			if (rc < 0) {
 				dev_err(dev, "%s() timeout on Expect\n",
@@ -424,7 +425,7 @@ static int i2c_nuvoton_send(struct tpm_chip *chip, u8 *buf, size_t bufsiz,
 		rc = i2c_nuvoton_wait_for_stat(chip,
 					       TPM_STS_VALID | TPM_STS_EXPECT,
 					       TPM_STS_VALID,
-					       chip->timeout_c, NULL);
+					       timeout_c, NULL);
 		if (rc) {
 			dev_err(dev, "%s() timeout on Expect to clear\n",
 				__func__);
@@ -554,12 +555,6 @@ static int i2c_nuvoton_probe(struct i2c_client *client)
 
 	init_waitqueue_head(&priv->read_queue);
 
-	/* Default timeouts */
-	chip->timeout_a = msecs_to_jiffies(TPM_I2C_SHORT_TIMEOUT);
-	chip->timeout_b = msecs_to_jiffies(TPM_I2C_LONG_TIMEOUT);
-	chip->timeout_c = msecs_to_jiffies(TPM_I2C_SHORT_TIMEOUT);
-	chip->timeout_d = msecs_to_jiffies(TPM_I2C_SHORT_TIMEOUT);
-
 	dev_set_drvdata(&chip->dev, priv);
 
 	/*
@@ -578,6 +573,9 @@ static int i2c_nuvoton_probe(struct i2c_client *client)
 		if (rc) {
 			priv->irq = 0;
 		} else {
+			unsigned long timeout_b =
+				tpm_limits_jiffies(chip, TPM_LIMIT_B);
+
 			chip->flags |= TPM_CHIP_FLAG_IRQ;
 			/* Clear any pending interrupt */
 			i2c_nuvoton_ready(chip);
@@ -585,7 +583,7 @@ static int i2c_nuvoton_probe(struct i2c_client *client)
 			rc = i2c_nuvoton_wait_for_stat(chip,
 						       TPM_STS_COMMAND_READY,
 						       TPM_STS_COMMAND_READY,
-						       chip->timeout_b,
+						       timeout_b,
 						       NULL);
 			if (rc == 0) {
 				/*
drivers/char/tpm/tpm_tis_core.c +17 -14
index 64ae82e435d0..5da9446c788a 100644
@@ -137,7 +137,7 @@ static int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask,
 static int wait_startup(struct tpm_chip *chip, int l)
 {
 	struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
-	unsigned long stop = jiffies + chip->timeout_a;
+	unsigned long stop = jiffies + tpm_limits_jiffies(chip, TPM_LIMIT_A);
 
 	do {
 		int rc;
@@ -210,7 +210,7 @@ static int __tpm_tis_request_locality(struct tpm_chip *chip, int l)
 	if (rc < 0)
 		return rc;
 
-	stop = jiffies + chip->timeout_a;
+	stop = jiffies + tpm_limits_jiffies(chip, TPM_LIMIT_A);
 
 	if (chip->flags & TPM_CHIP_FLAG_IRQ) {
 again:
@@ -303,9 +303,9 @@ static int get_burstcount(struct tpm_chip *chip)
 
 	/* wait for burstcount */
 	if (chip->flags & TPM_CHIP_FLAG_TPM2)
-		stop = jiffies + chip->timeout_a;
+		stop = jiffies + tpm_limits_jiffies(chip, TPM_LIMIT_A);
 	else
-		stop = jiffies + chip->timeout_d;
+		stop = jiffies + tpm_limits_jiffies(chip, TPM_LIMIT_D);
 	do {
 		rc = tpm_tis_read32(priv, TPM_STS(priv->locality), &value);
 		if (rc < 0)
@@ -327,7 +327,7 @@ static int recv_data(struct tpm_chip *chip, u8 *buf, size_t count)
 	while (size < count) {
 		rc = wait_for_tpm_stat(chip,
 				 TPM_STS_DATA_AVAIL | TPM_STS_VALID,
-				 chip->timeout_c,
+				 tpm_limits_jiffies(chip, TPM_LIMIT_C),
 				 &priv->read_queue, true);
 		if (rc < 0)
 			return rc;
@@ -382,8 +382,9 @@ static int tpm_tis_try_recv(struct tpm_chip *chip, u8 *buf, size_t count)
 		goto out;
 	}
 
-	if (wait_for_tpm_stat(chip, TPM_STS_VALID, chip->timeout_c,
-				&priv->int_queue, false) < 0) {
+	if (wait_for_tpm_stat(chip, TPM_STS_VALID,
+			      tpm_limits_jiffies(chip, TPM_LIMIT_C),
+			      &priv->int_queue, false) < 0) {
 		size = -ETIME;
 		goto out;
 	}
@@ -447,9 +448,9 @@ static int tpm_tis_send_data(struct tpm_chip *chip, const u8 *buf, size_t len)
 	status = tpm_tis_status(chip);
 	if ((status & TPM_STS_COMMAND_READY) == 0) {
 		tpm_tis_ready(chip);
-		if (wait_for_tpm_stat
-		    (chip, TPM_STS_COMMAND_READY, chip->timeout_b,
-		     &priv->int_queue, false) < 0) {
+		if (wait_for_tpm_stat(chip, TPM_STS_COMMAND_READY,
+				      tpm_limits_jiffies(chip, TPM_LIMIT_B),
+				      &priv->int_queue, false) < 0) {
 			rc = -ETIME;
 			goto out_err;
 		}
@@ -470,8 +471,9 @@ static int tpm_tis_send_data(struct tpm_chip *chip, const u8 *buf, size_t len)
 
 		count += burstcnt;
 
-		if (wait_for_tpm_stat(chip, TPM_STS_VALID, chip->timeout_c,
-					&priv->int_queue, false) < 0) {
+		if (wait_for_tpm_stat(chip, TPM_STS_VALID,
+				      tpm_limits_jiffies(chip, TPM_LIMIT_C),
+				      &priv->int_queue, false) < 0) {
 			if (test_bit(TPM_TIS_STATUS_VALID_RETRY, &priv->flags))
 				rc = -EAGAIN;
 			else
@@ -492,8 +494,9 @@ static int tpm_tis_send_data(struct tpm_chip *chip, const u8 *buf, size_t len)
 	if (rc < 0)
 		goto out_err;
 
-	if (wait_for_tpm_stat(chip, TPM_STS_VALID, chip->timeout_c,
-				&priv->int_queue, false) < 0) {
+	if (wait_for_tpm_stat(chip, TPM_STS_VALID,
+			      tpm_limits_jiffies(chip, TPM_LIMIT_C),
+			      &priv->int_queue, false) < 0) {
 		if (test_bit(TPM_TIS_STATUS_VALID_RETRY, &priv->flags))
 			rc = -EAGAIN;
 		else
drivers/char/tpm/tpm_tis_i2c_cr50.c +5 -10
index 0acfc36eff16..3a39ec49d3f3 100644
@@ -95,6 +95,7 @@ static irqreturn_t tpm_cr50_i2c_int_handler(int dummy, void *tpm_info)
 static int tpm_cr50_i2c_wait_tpm_ready(struct tpm_chip *chip)
 {
 	struct tpm_i2c_cr50_priv_data *priv = dev_get_drvdata(&chip->dev);
+	unsigned long timeout = tpm_limits_jiffies(chip, TPM_LIMIT_A);
 
 	/* Use a safe fixed delay if interrupt is not supported */
 	if (priv->irq <= 0) {
@@ -103,7 +104,7 @@ static int tpm_cr50_i2c_wait_tpm_ready(struct tpm_chip *chip)
 	}
 
 	/* Wait for interrupt to indicate TPM is ready to respond */
-	if (!wait_for_completion_timeout(&priv->tpm_ready, chip->timeout_a)) {
+	if (!wait_for_completion_timeout(&priv->tpm_ready, timeout)) {
 		dev_warn(&chip->dev, "Timeout waiting for TPM ready\n");
 		return -ETIMEDOUT;
 	}
@@ -362,7 +363,7 @@ static int tpm_cr50_request_locality(struct tpm_chip *chip, int loc)
 	if (rc < 0)
 		goto unlock_out;
 
-	stop = jiffies + chip->timeout_a;
+	stop = jiffies + tpm_limits_jiffies(chip, TPM_LIMIT_A);
 	do {
 		if (tpm_cr50_check_locality(chip, loc) == loc)
 			return loc;
@@ -433,7 +434,7 @@ static int tpm_cr50_i2c_get_burst_and_status(struct tpm_chip *chip, u8 mask,
 	*status = 0;
 
 	/* wait for burstcount */
-	stop = jiffies + chip->timeout_b;
+	stop = jiffies + tpm_limits_jiffies(chip, TPM_LIMIT_B);
 
 	do {
 		if (tpm_cr50_i2c_read(chip, TPM_I2C_STS(chip->locality), buf, sizeof(buf)) < 0) {
@@ -563,7 +564,7 @@ static int tpm_cr50_i2c_tis_send(struct tpm_chip *chip, u8 *buf, size_t bufsiz,
 	int rc;
 
 	/* Wait until TPM is ready for a command */
-	stop = jiffies + chip->timeout_b;
+	stop = jiffies + tpm_limits_jiffies(chip, TPM_LIMIT_B);
 	while (!(tpm_cr50_i2c_tis_status(chip) & TPM_STS_COMMAND_READY)) {
 		if (time_after(jiffies, stop)) {
 			rc = -ETIMEDOUT;
@@ -738,12 +739,6 @@ static int tpm_cr50_i2c_probe(struct i2c_client *client)
 	if (tpm_cr50_i2c_is_firmware_power_managed(dev))
 		chip->flags |= TPM_CHIP_FLAG_FIRMWARE_POWER_MANAGED;
 
-	/* Default timeouts */
-	chip->timeout_a = msecs_to_jiffies(TIS_SHORT_TIMEOUT);
-	chip->timeout_b = msecs_to_jiffies(TIS_LONG_TIMEOUT);
-	chip->timeout_c = msecs_to_jiffies(TIS_SHORT_TIMEOUT);
-	chip->timeout_d = msecs_to_jiffies(TIS_SHORT_TIMEOUT);
-
 	dev_set_drvdata(&chip->dev, priv);
 	init_completion(&priv->tpm_ready);
 
drivers/char/tpm/xen-tpmfront.c +6 -4
index 3c0cce57c4ae..b1624345891a 100644
@@ -148,8 +148,9 @@ static int vtpm_send(struct tpm_chip *chip, u8 *buf, size_t bufsiz,
 		return -EINVAL;
 
 	/* Wait for completion of any existing command or cancellation */
-	if (wait_for_tpm_stat(chip, VTPM_STATUS_IDLE, chip->timeout_c,
-			&priv->read_queue, true) < 0) {
+	if (wait_for_tpm_stat(chip, VTPM_STATUS_IDLE,
+			      tpm_limits_jiffies(chip, TPM_LIMIT_C),
+			      &priv->read_queue, true) < 0) {
 		vtpm_cancel(chip);
 		return -ETIME;
 	}
@@ -185,8 +186,9 @@ static int vtpm_recv(struct tpm_chip *chip, u8 *buf, size_t count)
 		return -ECANCELED;
 
 	/* In theory the wait at the end of _send makes this one unnecessary */
-	if (wait_for_tpm_stat(chip, VTPM_STATUS_RESULT, chip->timeout_c,
-			&priv->read_queue, true) < 0) {
+	if (wait_for_tpm_stat(chip, VTPM_STATUS_RESULT,
+			      tpm_limits_jiffies(chip, TPM_LIMIT_C),
+			      &priv->read_queue, true) < 0) {
 		vtpm_cancel(chip);
 		return -ETIME;
 	}
include/linux/tpm.h +0 -15
index 0b9661ffe45e..8ff1683f21ea 100644
@@ -59,16 +59,6 @@ struct tpm_class_ops {
 
 #define TPM_NUM_EVENT_LOG_FILES		3
 
-/* Indexes the duration array */
-enum tpm_duration {
-	TPM_SHORT = 0,
-	TPM_MEDIUM = 1,
-	TPM_LONG = 2,
-	TPM_LONG_LONG = 3,
-	TPM_UNDEFINED,
-	TPM_NUM_DURATIONS = TPM_UNDEFINED,
-};
-
 enum tpm_limit {
 	/* Interface handshake timeouts */
 	TPM_LIMIT_A,
@@ -181,12 +171,7 @@ struct tpm_chip {
 
 	struct mutex tpm_mutex;	/* tpm is processing */
 
-	unsigned long timeout_a; /* jiffies */
-	unsigned long timeout_b; /* jiffies */
-	unsigned long timeout_c; /* jiffies */
-	unsigned long timeout_d; /* jiffies */
 	bool timeout_adjusted;
-	unsigned long duration[TPM_NUM_DURATIONS]; /* jiffies */
 	bool duration_adjusted;
 
 	struct tpm_limits limits;
Signature · 1 line
2.47.3
#10 lore raw

RFC PATCH v3 6/8 tpm: Add the tpm.limits= parameter

In reply to Surendran Kanagaraj

When a TPM needs more time than its limits allow, the only way to give it more is to change its driver and rebuild the kernel.

Add tpm.limits=, which takes name:usecs pairs such as tpm.limits=timeout_c:2000000,long:600000000. The values apply when the chip is allocated and again after the driver's ->adjust_limits() hook, both times before validation, so they carry the new PARAM source and still get the floor and the ceiling. Document the parameter in kernel-parameters.txt.

---
 .../admin-guide/kernel-parameters.txt         |  13 +++
 drivers/char/tpm/tpm-limits.c                 | 102 +++++++++++++++++-
 include/linux/tpm.h                           |   1 +
 3 files changed, 112 insertions(+), 4 deletions(-)
Documentation/admin-guide/kernel-parameters.txt +13 -0
index 0b3c140babd9..3c4d6f3101d4 100644
@@ -7667,6 +7667,19 @@ Kernel parameters
 			causing a major performance hit, and the space where
 			machines are deployed is by other means guarded.
 
+	tpm.limits=	[HW,TPM]
+			Format: <name>:<usecs>[,<name>:<usecs>...]
+			Override how long the kernel waits for the TPM.
+			The names are timeout_a, timeout_b, timeout_c and
+			timeout_d for the interface timeouts, and short,
+			medium, long, long_long and default for command
+			execution. long also bounds the total sleep between
+			resubmissions of a command a TPM 2.0 chip asked to
+			retry. Values are in microseconds, and the TPM core
+			still applies its floor and ceiling. A value of 0
+			leaves that limit unchanged.
+			Example: tpm.limits=timeout_c:2000000,long:600000000
+
 	tpm_crb_ffa.busy_timeout_ms= [ARM64,TPM]
 			Maximum time in milliseconds to retry sending a message
 			to the TPM service before giving up. This parameter controls
drivers/char/tpm/tpm-limits.c +98 -4

146 lines not on this page · raw message

include/linux/tpm.h +1 -0
index 8ff1683f21ea..a4b7282d88b5 100644
@@ -89,6 +89,7 @@ enum tpm_limit_src {
 	TPM_LSRC_DEFAULT,	/* default values */
 	TPM_LSRC_TPM,		/* reported by the TPM */
 	TPM_LSRC_DRIVER,	/* set by the driver */
+	TPM_LSRC_PARAM,		/* tpm.limits= */
 };
 
 #define TPM_LF_CLAMPED		BIT(0)	/* limit validation replaced the value */
Signature · 1 line
2.47.3
#11 lore raw

RFC PATCH v3 7/8 tpm: Show timeouts and durations for TPM 2.0 chips

In reply to Surendran Kanagaraj

The timeouts and durations files exist only for TPM 1.x chips, so a TPM 2.0 chip exposes none of its wait limits, even when tpm.limits= or a driver's ->adjust_limits() hook changes them.

Both files read the limits table, so add them to the TPM 2.0 attributes too. A TPM 2.0 chip reports none of these values, so for it "[adjusted]" means that tpm.limits= or a driver's ->adjust_limits() hook set one of the shown values, or that validation changed one. TPM 2.0 chips get no device/ links, so the files are /sys/class/tpm/tpmX/timeouts and durations.

---
 drivers/char/tpm/tpm-sysfs.c |  2 ++
 drivers/char/tpm/tpm2-cmd.c  | 17 +++++++++++++++++
 2 files changed, 19 insertions(+)
drivers/char/tpm/tpm-sysfs.c +2 -0
index b289a7f6f085..6bfc366fa120 100644
@@ -340,6 +340,8 @@ static struct attribute *tpm1_dev_attrs[] = {
 
 static struct attribute *tpm2_dev_attrs[] = {
 	&dev_attr_tpm_version_major.attr,
+	&dev_attr_durations.attr,
+	&dev_attr_timeouts.attr,
 #ifdef CONFIG_TCG_TPM2_HMAC
 	&dev_attr_null_name.attr,
 #endif
drivers/char/tpm/tpm2-cmd.c +17 -0
index 629caaeffa17..8cc1a262d668 100644
@@ -41,9 +41,26 @@ int tpm2_find_hash_alg(unsigned int crypto_id)
 }
 EXPORT_SYMBOL_GPL(tpm2_find_hash_alg);
 
+static bool tpm2_limits_adjusted(struct tpm_chip *chip, unsigned long mask)
+{
+	const struct tpm_limit_entry *e = chip->limits.entry;
+	int limit;
+
+	for_each_set_bit(limit, &mask, TPM_NR_LIMITS) {
+		if (e[limit].src != TPM_LSRC_DEFAULT ||
+		    e[limit].flags & TPM_LF_CLAMPED)
+			return true;
+	}
+
+	return false;
+}
+
 int tpm2_get_timeouts(struct tpm_chip *chip)
 {
 	tpm_limits_update(chip, NULL);
+	chip->timeout_adjusted = tpm2_limits_adjusted(chip, TPM_LIMIT_INTF_MASK);
+	chip->duration_adjusted =
+		tpm2_limits_adjusted(chip, TPM1_LIMIT_DURATION_MASK);
 	chip->flags |= TPM_CHIP_FLAG_HAVE_TIMEOUTS;
 	return 0;
 }
Signature · 1 line
2.47.3
#12 lore raw

RFC PATCH v3 8/8 tpm_crb: Raise the limits for Amazon NitroTPM

In reply to Surendran Kanagaraj

In rare scenarios NitroTPM can enter a mode where it either sends TPM2_RC_RETRY for longer than LONG duration or delays command completion for longer than the TPM 2.0 durations allow. When this happens during auth session start, the driver disables the chip and every subsequent request fails:

tpm tpm0: in retry loop
tpm tpm0: tpm2_load_context: failed with a TPM error 0x0922

Add a quirk table to the CRB driver keyed by the vendor ID from the interface ID register, and apply it from ->adjust_limits(). For NitroTPM, raise the SHORT and MEDIUM durations to 30 s, which covers the longest unavailability with some headroom, and LONG to 45 s. With the back-off doubling from 20 ms, a 45 s LONG duration lets the driver resubmit a command for about 40.9 s, so the retry loop covers the entire unavailability window.

These are upper bounds, not expected waits. A command that completes sooner ends the wait as soon as the poll sees it. Add TPM_VID_AMZN next to the vendor ID list in tpm.h.

Tested on an EC2 instance with NitroTPM by inducing multiple unavailability windows with no TPM errors, and in QEMU with swtpm by holding the TPM in TPM2_RC_RETRY and stalling command completion for longer than 2s.

---
 drivers/char/tpm/tpm_crb.c | 50 ++++++++++++++++++++++++++++++++++++++
 include/linux/tpm.h        |  1 +
 2 files changed, 51 insertions(+)
drivers/char/tpm/tpm_crb.c +50 -0
index 7c1011eb855a..0e18f61cfddf 100644
@@ -13,6 +13,7 @@
 
 #include <linux/acpi.h>
 #include <linux/highmem.h>
+#include <linux/io-64-nonatomic-lo-hi.h>
 #include <linux/rculist.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
@@ -26,6 +27,14 @@
 #define ACPI_SIG_TPM2 "TPM2"
 #define TPM_CRB_MAX_RESOURCES 3
 
+/* TPM_CRB_INTF_ID_x vendor ID field, bits 47:32 of the 64-bit register */
+#define TPM_CRB_INTF_ID_VID(intf_id)	(((intf_id) >> 32) & 0xffff)
+
+/* Longest NitroTPM unavailability, with some headroom */
+#define TPM_CRB_AMZN_LIMIT_US		(30 * USEC_PER_SEC)
+/* LONG also bounds the retry loop: 45 s lets it resubmit for 40.9 s */
+#define TPM_CRB_AMZN_LONG_US		(45 * USEC_PER_SEC)
+
 static const guid_t crb_acpi_start_guid =
 	GUID_INIT(0x6BBF6CAB, 0x5463, 0x4714,
 		  0xB7, 0xCD, 0xF0, 0x20, 0x3C, 0x03, 0x68, 0xD4);
@@ -663,6 +672,46 @@ static bool crb_req_canceled(struct tpm_chip *chip, u8 status)
 	return (cancel & CRB_CANCEL_INVOKE) == CRB_CANCEL_INVOKE;
 }
 
+/*
+ * Read the vendor ID from the interface ID register. Returns -ENODEV when
+ * the head registers are not mapped for this start method.
+ */
+static int crb_vendor_id(struct crb_priv *priv)
+{
+	u64 intf_id;
+
+	if (!priv->regs_h)
+		return -ENODEV;
+
+	intf_id = lo_hi_readq(&priv->regs_h->intf_id);
+
+	return TPM_CRB_INTF_ID_VID(intf_id);
+}
+
+/* Limits keyed by the vendor ID of the interface ID register */
+static const struct tpm_limits_quirk crb_limits_quirks[] = {
+	{
+		.id = TPM_VID_AMZN,
+		.time_us = {
+			[TPM_LIMIT_SHORT]	= TPM_CRB_AMZN_LIMIT_US,
+			[TPM_LIMIT_MEDIUM]	= TPM_CRB_AMZN_LIMIT_US,
+			[TPM_LIMIT_LONG]	= TPM_CRB_AMZN_LONG_US,
+		},
+	},
+};
+
+static void crb_adjust_limits(struct tpm_chip *chip, struct tpm_limits *p)
+{
+	struct crb_priv *priv = dev_get_drvdata(&chip->dev);
+	int vid = crb_vendor_id(priv);
+
+	if (vid < 0)
+		return;
+
+	tpm_limits_apply_quirks(p, crb_limits_quirks,
+				ARRAY_SIZE(crb_limits_quirks), vid);
+}
+
 static const struct tpm_class_ops tpm_crb = {
 	.flags = TPM_OPS_AUTO_STARTUP,
 	.status = crb_status,
@@ -674,6 +723,7 @@ static const struct tpm_class_ops tpm_crb = {
 	.cmd_ready = crb_cmd_ready,
 	.request_locality = crb_request_locality,
 	.relinquish_locality = crb_relinquish_locality,
+	.adjust_limits = crb_adjust_limits,
 	.req_complete_mask = CRB_DRV_STS_COMPLETE,
 	.req_complete_val = CRB_DRV_STS_COMPLETE,
 };
include/linux/tpm.h +1 -0
index a4b7282d88b5..f557c1a32388 100644
@@ -220,6 +220,7 @@ static inline enum tpm2_mso_type tpm2_handle_mso(u32 handle)
 #define TPM_VID_STM      0x104A
 #define TPM_VID_ATML     0x1114
 #define TPM_VID_IFX      0x15D1
+#define TPM_VID_AMZN     0x0EC2
 
 enum tpm_chip_flags {
 	TPM_CHIP_FLAG_BOOTSTRAPPED		= BIT(0),
Signature · 1 line
2.47.3
maintainer
#13 lore raw
In reply to Surendran Kanagaraj
Quoted 3 lines · Surendran Kanagaraj

Review of v2 [1] asked to redesign how the timeouts and durations are handled across TPM families and drivers. This series is my attempt at it. Sorry for the slow progress.

No this is great! I mean it is right even if it was in some ways wrong. I have zero expectations that this would be exactly right at first, but now we have something more relevant to debate.

And I think this type of paradigm shift towards more architectural changes makes kernel development much more rewarding also for you as an author. I don't think anyone loves that much competing with LLMs when it comes to smaller details and random bugs.

If we don't do this, we'll all become check-and-stamp machines for LLMs. That would be sad.

A bit more intrusive but at the same time well thought changes make LLM attention to work for our benefit as these bots help to quickly stabilize the landed code.

Br, Jarkko