tpm(4): fix delay units
authorcheloha <cheloha@openbsd.org>
Thu, 28 Jan 2021 17:19:40 +0000 (17:19 +0000)
committercheloha <cheloha@openbsd.org>
Thu, 28 Jan 2021 17:19:40 +0000 (17:19 +0000)
tpm(4) has timeout constants in milliseconds, e.g.

#define TPM_ACCESS_TMO 2000 /* 2sec */

This is fine.

The odd thing is that tpm(4) first converts these timeouts to counts
of ticks via tpm_tmotohz() before using DELAY() to busy-wait.  DELAY()
takes a count of microseconds, which are not equivalent to ticks, so
the units are all screwed up.

Let's correct this:

- Remove tpm_tmotohz().  We're not working with ticks so we don't it.

- Multiply the timeouts to match the delay interval.  tpm_request_locality()
  and tpm_getburst() use intervals of 10 microseconds, so multiply the
  millisecond timeouts by 100.  In tpm_waitfor() the delay interval is 1
  microsecond, so multiply the millisecond timeout by 1000.

- Update the parameter name in tpm_waitfor() to note that we expect a
  count of milliseconds, not "tries".

Discussion: https://marc.info/?l=openbsd-tech&m=160995671326406&w=2

Prompted by kettenis@.

Suspend/resume tested by florian@ on an X1 Gen 2.  For the record, it
looks like this:

tpm0 at acpi0 TPM_ addr 0xfed40000/0x5000, device 0x0000104a rev 0x4e

Earlier versions of this patch were reviewed by kn@, but the patch
became more ambitious when kettenis@ got involved so those reviews
are no longer applicable.

jcs@ notes (https://marc.info/?l=openbsd-tech&m=160834427630142&w=2)
in a related discussion that this driver "sucks" and should be
replaced with NetBSD's rewrite.  This would get us a cleaner driver
with TPM 2.0 support.  So there is future work to do here.

ok kettenis@

sys/dev/acpi/tpm.c

index cf575b1..1e89907 100644 (file)
@@ -1,4 +1,4 @@
-/* $OpenBSD: tpm.c,v 1.10 2020/05/22 10:16:37 kettenis Exp $ */
+/* $OpenBSD: tpm.c,v 1.11 2021/01/28 17:19:40 cheloha Exp $ */
 
 /*
  * Minimal interface to Trusted Platform Module chips implementing the
@@ -158,7 +158,6 @@ int tpm_request_locality(struct tpm_softc *, int);
 void   tpm_release_locality(struct tpm_softc *);
 int    tpm_getburst(struct tpm_softc *);
 uint8_t        tpm_status(struct tpm_softc *);
-int    tpm_tmotohz(int);
 
 struct cfattach tpm_ca = {
        sizeof(struct tpm_softc),
@@ -385,7 +384,7 @@ tpm_request_locality(struct tpm_softc *sc, int l)
        bus_space_write_1(sc->sc_bt, sc->sc_bh, TPM_ACCESS,
            TPM_ACCESS_REQUEST_USE);
 
-       to = tpm_tmotohz(TPM_ACCESS_TMO);
+       to = TPM_ACCESS_TMO * 100;      /* steps of 10 microseconds */
 
        while ((r = bus_space_read_1(sc->sc_bt, sc->sc_bh, TPM_ACCESS) &
            (TPM_ACCESS_VALID | TPM_ACCESS_ACTIVE_LOCALITY)) !=
@@ -420,7 +419,7 @@ tpm_getburst(struct tpm_softc *sc)
 {
        int burst, burst2, to;
 
-       to = tpm_tmotohz(TPM_BURST_TMO);
+       to = TPM_BURST_TMO * 100;       /* steps of 10 microseconds */
 
        burst = 0;
        while (burst == 0 && to--) {
@@ -453,29 +452,21 @@ tpm_status(struct tpm_softc *sc)
 }
 
 int
-tpm_tmotohz(int tmo)
-{
-       struct timeval tv;
-
-       tv.tv_sec = tmo / 1000;
-       tv.tv_usec = 1000 * (tmo % 1000);
-
-       return tvtohz(&tv);
-}
-
-int
-tpm_waitfor(struct tpm_softc *sc, uint8_t mask, int tries)
+tpm_waitfor(struct tpm_softc *sc, uint8_t mask, int msecs)
 {
+       int usecs;
        uint8_t status;
 
+       usecs = msecs * 1000;
+
        while (((status = tpm_status(sc)) & mask) != mask) {
-               if (tries == 0) {
+               if (usecs == 0) {
                        DPRINTF(("%s: %s: timed out, status 0x%x != 0x%x\n",
                            sc->sc_dev.dv_xname, __func__, status, mask));
                        return status;
                }
 
-               tries--;
+               usecs--;
                DELAY(1);
        }