From nobody Sun Oct 11 01:07:29 2026 X-Original-To: dev-commits-src-branches@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4j2Mrx5HpMz6w8Vl for ; Sun, 11 Oct 2026 01:07:29 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange x25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "YR2" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id 4j2Mrx4HmZz3J4g for ; Sun, 11 Oct 2026 01:07:29 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1791680849; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=W7WO9JiiUWsmANzHPZIdMrYtE12J8577iT9My26jwX0=; b=Rb2mwnWnUy1DewgGhAqKZ9STaQ0wqHOP5xFh65+kzEGvjTm3iGFHB2+C4CrA6MvmRGuC3D 0Nlt864yZYt4W86nZm2yc8kNIrW+s56/3e0Y0sVhK05t3d3XcJWJ7YmnBi9FHAVhFaFjCl Nuj7p5nyCYGx9bs9IZUCYd6DaeyOhSKQ6GZrT0sCxt4CeE0xeD1w4KhIIH+hdcc8grg0KH pWMj84/GyLGKV/ddTVGqQr1WGSwxdp3OUzAhyaTFEgg/2foxxfQ9L1jc4d0FbO0uIzR7sf BEp6skYfNV/r8V5ZxpWkKkvZw8//5PUiUo3m4iQFT222Jb2hxthcFYcRvlGz/g== ARC-Seal: i=1; a=rsa-sha256; d=freebsd.org; s=dkim; cv=none; t=1791680849; b=LqVWZQdhDeXzM/uyKvUlO+p8WvVHTvz+u74SIZcvl9ej+bY7VfVGpYy3Unct5IqGLqy9nT RBZ71J4z8uRhT26vglv9k6oFx3ReCJdQF15iPTry4v/LSttyndIAeWNM8GEjPsy/+OLZUT oTPCEgSvGXUg8B8JXVU6qRr3nVEm08TR3xqFrJqp++S8KhocqBiHO8AXq85n0jwzoTjBzc QU+nK4cqJdcsfAvgjSiLQTKJmpKx5J9rF69oDptD87kEJNQFa4P79SZCzEXQlE3j3Vd2h8 VziaIPAonBOq2t8s4HHFP9wtHUH3OcuZRQWl6LBmq/Oq+wycMsAtIzWeRvxkXg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1791680849; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=W7WO9JiiUWsmANzHPZIdMrYtE12J8577iT9My26jwX0=; b=uX3wucn25irdFlK+N6iVWpOaUbtUGGRh1ngCPHdJ5On1DGwooy1ok4qwIR80ahYqcg4bz6 vb2okWiiRQNve4kMB2D7IBHZcz5fBju99dx73jAY3JA+W5tsty6CXCb0BRsAT4tzVok5L+ u2UN35oPPIO3Wm2HyhfZCVB+aNDztgbJV9nYBtNUyopqJCrhgjBXm+zEZSBACcY7+MG7KK z5xdC+ixPo3ZUpo3k3Ei6Y07JdKedfd1scxURXEugwuNyqrgZ0KSGAr7t5kTpiUu8NGnfd pqH2XgRkYHiDWYRO+LjTHjFdwQgh9TT1jCLzSj6IdFqdRqR6lXhqIlIEIhabcA== ARC-Authentication-Results: i=1; mx1.freebsd.org; none Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) by mxrelay.nyi.freebsd.org (Postfix) with ESMTP id 4j2Mrx3Lx9zJ5v for ; Sun, 11 Oct 2026 01:07:29 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from git (uid 1279) (envelope-from git@FreeBSD.org) id 32ad5 by gitrepo.freebsd.org (DragonFly Mail Agent v0.13+ on gitrepo.freebsd.org); Sun, 11 Oct 2026 01:07:29 +0000 To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Kevin Bowling Subject: git: 48d997f4366f - stable/15 - tpm_tis: Close interrupt wait races List-Archive: https://lists.freebsd.org/archives/dev-commits-src-branches X-BeenThere: dev-commits-src-branches@freebsd.org Sender: owner-dev-commits-src-branches@FreeBSD.org List-Id: Commits to the stable branches of the FreeBSD src repository List-Post: List-Help: List-Subscribe: List-Unsubscribe: List-Owner: Precedence: list MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: kbowling X-Git-Repository: src X-Git-Refname: refs/heads/stable/15 X-Git-Reftype: branch X-Git-Commit: 48d997f4366f65501f875ee5100d2b2be01eac7a Auto-Submitted: auto-generated Date: Sun, 11 Oct 2026 01:07:29 +0000 Message-Id: <6acae151.32ad5.2f64415b@gitrepo.freebsd.org> The branch stable/15 has been updated by kbowling: URL: https://cgit.FreeBSD.org/src/commit/?id=48d997f4366f65501f875ee5100d2b2be01eac7a commit 48d997f4366f65501f875ee5100d2b2be01eac7a Author: Kevin Bowling AuthorDate: 2026-08-27 12:46:19 +0000 Commit: Kevin Bowling CommitDate: 2026-10-11 01:07:20 +0000 tpm_tis: Close interrupt wait races The TIS interrupt handler can acknowledge and signal an event after the waiter checks the device status but before it enters tsleep(). Since the handler is MPSAFE, the command lock does not close this window. A lost wakeup can delay a completed command for its full timeout, up to 40 seconds for long TPM 2.0 operations. Publish the expected event under an interrupt mutex and use a generation counter to record matching interrupts. Recheck the device predicate without the mutex because register access may sleep on a SPI transport, then compare the generation before atomically waiting on a condition variable. This closes the check-to-sleep race without placing sleeping bus operations under a mutex. Use an absolute deadline while retrying the predicate after wakeups. Apply the same scheme to locality acquisition, which had an equivalent race. Leave the expected event published while polling so the attach-time test can still prove that an advertised interrupt arrived. Regression-tested the polling fallback on two TPM 2.0 FIFO systems with 200 concurrent PCR reads per system and repeated device suspend/resume. Neither ACPI device exposes an IRQ, so the interrupt-mode path remains hardware unvalidated. Reviewed by: kevans Sponsored by: BBOX.io Differential Revision: https://reviews.freebsd.org/D59241 (cherry picked from commit add771d5e62ae12f2494fb30a93cd5f1067ac34d) --- sys/dev/tpm/tpm20.h | 6 ++ sys/dev/tpm/tpm_tis_core.c | 146 ++++++++++++++++++++++++++++----------------- 2 files changed, 98 insertions(+), 54 deletions(-) diff --git a/sys/dev/tpm/tpm20.h b/sys/dev/tpm/tpm20.h index 2aa23318b5e5..36f41e6b9e0b 100644 --- a/sys/dev/tpm/tpm20.h +++ b/sys/dev/tpm/tpm20.h @@ -40,8 +40,10 @@ #include #include #include +#include #include #include +#include #include #include #include @@ -123,10 +125,14 @@ struct tpm_sc { struct cdev *sc_cdev; + /* Serialize commands and lifecycle; intr_lock nests inside. */ struct sx dev_lock; + struct mtx intr_lock; + struct cv intr_cv; void *intr_cookie; int intr_type; /* Current event type */ + uint32_t intr_generation; uint32_t intr_mask; /* Saved TIS interrupt configuration */ bool interrupts; bool common_initialized; diff --git a/sys/dev/tpm/tpm_tis_core.c b/sys/dev/tpm/tpm_tis_core.c index 66263a13ce97..d831339342dd 100644 --- a/sys/dev/tpm/tpm_tis_core.c +++ b/sys/dev/tpm/tpm_tis_core.c @@ -89,8 +89,9 @@ static bool tpmtis_request_locality(struct tpm_sc *sc, int locality); static void tpmtis_relinquish_locality(struct tpm_sc *sc); static bool tpmtis_go_ready(struct tpm_sc *sc); -static bool tpm_wait_for_u32(struct tpm_sc *sc, bus_size_t off, - uint32_t mask, uint32_t val, int32_t timeout); +static bool tpm_wait_for_reg(struct tpm_sc *sc, bus_size_t off, + uint32_t mask, uint32_t val, int32_t timeout, int intr_type, + bool reg32); static uint16_t tpmtis_wait_for_burst(struct tpm_sc *sc); @@ -104,8 +105,11 @@ tpmtis_attach(device_t dev) sc = device_get_softc(dev); sc->dev = dev; sc->intr_type = -1; + sc->intr_generation = 0; sx_init(&sc->dev_lock, "TPM driver lock"); + mtx_init(&sc->intr_lock, "TPM interrupt lock", NULL, MTX_DEF); + cv_init(&sc->intr_cv, "tpmtis_intr"); resource_int_value("tpm", device_get_unit(dev), "use_polling", &poll); if (poll != 0) { @@ -148,6 +152,8 @@ tpmtis_detach(device_t dev) if (sc->intr_cookie != NULL) bus_teardown_intr(dev, sc->irq_res, sc->intr_cookie); + cv_destroy(&sc->intr_cv); + mtx_destroy(&sc->intr_lock); if (sc->irq_res != NULL) bus_release_resource(dev, SYS_RES_IRQ, @@ -240,9 +246,12 @@ tpmtis_setup_intr(struct tpm_sc *sc) { bool configured, enable; - sc->interrupts = false; enable = sc->intr_cookie != NULL; sx_xlock(&sc->dev_lock); + mtx_lock(&sc->intr_lock); + sc->interrupts = false; + sc->intr_type = -1; + mtx_unlock(&sc->intr_lock); configured = tpmtis_program_intr(sc, enable); sx_xunlock(&sc->dev_lock); if (!configured || !enable) @@ -258,6 +267,7 @@ tpmtis_resume(device_t dev) sc = device_get_softc(dev); sx_xlock(&sc->dev_lock); + mtx_lock(&sc->intr_lock); restore_intr = sc->interrupts; /* @@ -266,6 +276,8 @@ tpmtis_resume(device_t dev) * observed by the handler. */ sc->interrupts = false; + sc->intr_type = -1; + mtx_unlock(&sc->intr_lock); if (!tpmtis_program_intr(sc, restore_intr)) device_printf(dev, "failed to %s interrupts; using polling\n", @@ -286,40 +298,74 @@ tpmtis_intr_handler(void *arg) TPM_WRITE_4(sc->dev, TPM_INT_STS, status); + mtx_lock(&sc->intr_lock); /* Check for stray interrupts. */ - if (sc->intr_type == -1 || (sc->intr_type & status) == 0) - return; + if (sc->intr_type != -1 && (sc->intr_type & status) != 0) { + sc->interrupts = true; + sc->intr_generation++; + cv_broadcast(&sc->intr_cv); + } + mtx_unlock(&sc->intr_lock); +} - sc->interrupts = true; - wakeup(sc); +static uint32_t +tpmtis_read_wait_reg(struct tpm_sc *sc, bus_size_t off, bool reg32) +{ + + if (reg32) + return (TPM_READ_4(sc->dev, off)); + return (TPM_READ_1(sc->dev, off)); } static bool -tpm_wait_for_u32(struct tpm_sc *sc, bus_size_t off, uint32_t mask, uint32_t val, - int32_t timeout) +tpm_wait_for_reg(struct tpm_sc *sc, bus_size_t off, uint32_t mask, + uint32_t val, int32_t timeout, int intr_type, bool reg32) { + sbintime_t deadline; + uint32_t generation; + bool interrupts, result; - /* Check for condition */ - if ((TPM_READ_4(sc->dev, off) & mask) == val) - return (true); - - /* If interrupts are enabled sleep for timeout duration */ - if(sc->interrupts && sc->intr_type != -1) { - tsleep(sc, PWAIT, "TPM WITH INTERRUPTS", timeout / tick); + sx_assert(&sc->dev_lock, SA_XLOCKED); + deadline = sbinuptime() + ustosbt(timeout); + mtx_lock(&sc->intr_lock); + sc->intr_type = intr_type; + generation = sc->intr_generation; + interrupts = sc->interrupts; + mtx_unlock(&sc->intr_lock); + + for (;;) { + result = (tpmtis_read_wait_reg(sc, off, reg32) & mask) == val; + if (result) + break; + if (sbinuptime() >= deadline) + break; + if (!interrupts) { + pause("TPM POLLING", 1); + mtx_lock(&sc->intr_lock); + generation = sc->intr_generation; + interrupts = sc->interrupts; + mtx_unlock(&sc->intr_lock); + continue; + } - sc->intr_type = -1; - return ((TPM_READ_4(sc->dev, off) & mask) == val); + /* + * Register access may sleep for a SPI TPM, so evaluate the + * predicate without intr_lock. The generation check closes the + * resulting window before cv_timedwait_sbt() atomically sleeps. + */ + mtx_lock(&sc->intr_lock); + if (generation == sc->intr_generation) + (void)cv_timedwait_sbt(&sc->intr_cv, &sc->intr_lock, + deadline, 0, C_ABSOLUTE | C_HARDCLOCK); + generation = sc->intr_generation; + interrupts = sc->interrupts; + mtx_unlock(&sc->intr_lock); } - /* If we don't have interrupts poll the device every tick */ - while (timeout > 0) { - if ((TPM_READ_4(sc->dev, off) & mask) == val) - return (true); - - pause("TPM POLLING", 1); - timeout -= tick; - } - return (false); + mtx_lock(&sc->intr_lock); + sc->intr_type = -1; + mtx_unlock(&sc->intr_lock); + return (result); } static uint16_t @@ -385,44 +431,36 @@ static bool tpmtis_request_locality(struct tpm_sc *sc, int locality) { uint8_t mask; - int timeout; + sx_assert(&sc->dev_lock, SA_XLOCKED); /* Currently we only support Locality 0 */ if (locality != 0) return (false); mask = TPM_ACCESS_LOC_ACTIVE | TPM_ACCESS_VALID; - timeout = TPM_TIMEOUT_A; - sc->intr_type = TPM_INT_STS_LOC_CHANGE; TPM_WRITE_1(sc->dev, TPM_ACCESS, TPM_ACCESS_LOC_REQ); TPM_WRITE_BARRIER(sc->dev, TPM_ACCESS, 1); - if(sc->interrupts) { - tsleep(sc, PWAIT, "TPMLOCREQUEST with INTR", timeout / tick); - return ((TPM_READ_1(sc->dev, TPM_ACCESS) & mask) == mask); - } else { - while(timeout > 0) { - if ((TPM_READ_1(sc->dev, TPM_ACCESS) & mask) == mask) - return (true); - - pause("TPMLOCREQUEST POLLING", 1); - timeout -= tick; - } - } - - return (false); + return (tpm_wait_for_reg(sc, TPM_ACCESS, mask, mask, TPM_TIMEOUT_A, + TPM_INT_STS_LOC_CHANGE, false)); } static void tpmtis_relinquish_locality(struct tpm_sc *sc) { + bool interrupts; + sx_assert(&sc->dev_lock, SA_XLOCKED); /* * Interrupts can only be cleared when a locality is active. * Clear them now in case interrupt handler didn't make it in time. */ - if(sc->interrupts) - AND4(sc, TPM_INT_STS, TPM_READ_4(sc->dev, TPM_INT_STS)); + mtx_lock(&sc->intr_lock); + interrupts = sc->interrupts; + mtx_unlock(&sc->intr_lock); + if (interrupts) + TPM_WRITE_4(sc->dev, TPM_INT_STS, + TPM_READ_4(sc->dev, TPM_INT_STS)); OR1(sc, TPM_ACCESS, TPM_ACCESS_LOC_RELINQUISH); } @@ -433,11 +471,11 @@ tpmtis_go_ready(struct tpm_sc *sc) uint32_t mask; mask = TPM_STS_CMD_RDY; - sc->intr_type = TPM_INT_STS_CMD_RDY; TPM_WRITE_4(sc->dev, TPM_STS, TPM_STS_CMD_RDY); TPM_WRITE_BARRIER(sc->dev, TPM_STS, 4); - if (!tpm_wait_for_u32(sc, TPM_STS, mask, mask, TPM_TIMEOUT_B)) + if (!tpm_wait_for_reg(sc, TPM_STS, mask, mask, TPM_TIMEOUT_B, + TPM_INT_STS_CMD_RDY, true)) return (false); return (true); @@ -471,8 +509,8 @@ tpmtis_transmit(device_t dev, struct tpm_priv *priv, size_t length) } mask = TPM_STS_VALID; - sc->intr_type = TPM_INT_STS_VALID; - if (!tpm_wait_for_u32(sc, TPM_STS, mask, mask, TPM_TIMEOUT_C)) { + if (!tpm_wait_for_reg(sc, TPM_STS, mask, mask, TPM_TIMEOUT_C, + TPM_INT_STS_VALID, true)) { device_printf(dev, "Timeout while waiting for valid bit\n"); return (EIO); @@ -495,8 +533,8 @@ tpmtis_transmit(device_t dev, struct tpm_priv *priv, size_t length) TPM_WRITE_BARRIER(dev, TPM_STS, 4); mask = TPM_STS_DATA_AVAIL | TPM_STS_VALID; - sc->intr_type = TPM_INT_STS_DATA_AVAIL; - if (!tpm_wait_for_u32(sc, TPM_STS, mask, mask, timeout)) { + if (!tpm_wait_for_reg(sc, TPM_STS, mask, mask, timeout, + TPM_INT_STS_DATA_AVAIL, true)) { device_printf(dev, "Timeout while waiting for device to process cmd\n"); /* @@ -510,8 +548,8 @@ tpmtis_transmit(device_t dev, struct tpm_priv *priv, size_t length) * After canceling a command we should get a response, * check if there is one. */ - sc->intr_type = TPM_INT_STS_DATA_AVAIL; - if (!tpm_wait_for_u32(sc, TPM_STS, mask, mask, TPM_TIMEOUT_C)) + if (!tpm_wait_for_reg(sc, TPM_STS, mask, mask, TPM_TIMEOUT_C, + TPM_INT_STS_DATA_AVAIL, true)) return (EIO); } /* Read response header. Length is passed in bytes 2 - 6. */