From nobody Thu Sep 03 04:00:30 2026 X-Original-To: dev-commits-src-all@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 4hb5V70SByz6qW7M for ; Thu, 03 Sep 2026 04:00:31 +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 4hb5V66jZ9z3qH9 for ; Thu, 03 Sep 2026 04:00:30 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1788408030; 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=2FI0yQWXlCiUjPI/RunJAv3Gmdzjtx2DproUSf+NRWU=; b=RFusbd0GxM5qSTQtjIhCTgH0/H6b+o4nPPYqZHC96QaOryGnZqJ/RPQBmRXTsE4McbVjBp URnVhfMCIBENNPM1hrPI6tDKSOKj7tJLdojurjkNrkd4yG2ZOs1Ci8/EwY+RYTNwYsvmuj IsGj8j03S2wS0SfBJZhOGj96yIDWH1hMFC0wUf10lB43oyIdsxL2jyn7FfiTDkPmExm3rc p5JmPjyGj9Tf4TtcJdWFsXJXgPnqOuYjj/GNX+9A1mDp59HzDRxPuGUmjGxkwH459TEtlt 3WRAsgQo9/Ap+YvAtN33P85Iiv+gRKxMKsb31T8vld3wrngpPLGMCjU77R2ZSA== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1788408031; a=rsa-sha256; cv=none; b=NgwdFZwvFyjcxmNnQGIAeZLK+UnSIFeMtmh9TorDHDFatPSaaELtfFh4VqnquMMx4OuHho oWUAw4OoXXuAUSN6/tFCIwDekGBYkhWWv1Y+E6VcPeHpu0kJ9EDSbxT4n54xhWhVdbaYb8 SM4McUnQRm64Q89VXiWGnNz3/df3d9jfOk6IcRb0mk30nITAcK3aYFAbW6aLUM6QRV00WR DAKvPPlSk4g5wc2I2dpFBpYjMP4mWIZnSsLeLRzb9aKd3tXNzlkNL/RrlDQpC7S100gX1U I4EJuTMdfdnGq2OK/j9MGYK9bZ1ZljrRgCrs3Hin11ugoF8ijrp0+4GPkI2Rqw== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1788408030; 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=2FI0yQWXlCiUjPI/RunJAv3Gmdzjtx2DproUSf+NRWU=; b=ABi5WJD5TUeFsbQjVVZF/HFO37UPDtsAFLoLf0r0TQ4cn2hW0WZ66/9cxF+bT8EkL7d+dh irFhNLb9uHZsjEei7YC+3azalRUMu3/w/P17ejNz7D3tIXdiI3byKmjPX1TZZxwplHhAj6 w7dH/zEq0K07fCsKe3zVkn/jRL5u3ndGfu7anjOLrygGM6kb5naW+mj0MDeDpqUNu6UpTF 04vVUixsbU7e5Hw0Wtg9AHFKLEs/6RRmYisw4is4LpkvzITdgHDyP7OwMNun3Gg1baFyxi gPdpCADt+9T6eJzUjKTNdtSuGnyh51cD30jrRTaoEAPsTg8OHOpu6Efad71y3Q== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) by mxrelay.nyi.freebsd.org (Postfix) with ESMTP id 4hb5V65Nj5z1C68 for ; Thu, 03 Sep 2026 04:00:30 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from git (uid 1279) (envelope-from git@FreeBSD.org) id 3a1e0 by gitrepo.freebsd.org (DragonFly Mail Agent v0.13+ on gitrepo.freebsd.org); Thu, 03 Sep 2026 04:00:30 +0000 To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Kevin Bowling Subject: git: add771d5e62a - main - tpm_tis: Close interrupt wait races List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org List-Id: 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/main X-Git-Reftype: branch X-Git-Commit: add771d5e62ae12f2494fb30a93cd5f1067ac34d Auto-Submitted: auto-generated Date: Thu, 03 Sep 2026 04:00:30 +0000 Message-Id: <6a98f0de.3a1e0.756eb55d@gitrepo.freebsd.org> The branch main has been updated by kbowling: URL: https://cgit.FreeBSD.org/src/commit/?id=add771d5e62ae12f2494fb30a93cd5f1067ac34d commit add771d5e62ae12f2494fb30a93cd5f1067ac34d Author: Kevin Bowling AuthorDate: 2026-08-27 12:46:19 +0000 Commit: Kevin Bowling CommitDate: 2026-09-03 04:00:09 +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 MFC after: 2 weeks Sponsored by: BBOX.io Differential Revision: https://reviews.freebsd.org/D59241 --- 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 f4b8ef23b3c7..08d37ebcfe19 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. */