On Sun, Mar 25, 2018 at 07:17:33PM -0400, Stefan Berger wrote:
> On 03/25/2018 11:45 AM, Kevin O'Connor wrote:
> > On Thu, Mar 22, 2018 at 08:19:09AM -0400, Stefan Berger wrote:
> > > The timeout to wait for the register change is 30ms. We yield() while
> > > waiting, so we don't block everything entirely... Is the error message
> > > misleading and we should print out that a device was not detected or print
> > > out if it is detected instead?
> > Unfortunately, although the TPM code calls yield(), there isn't
> > anything else running at that point, so the delay still directly
> > impacts the total boot time.  It's not easy to push back the TPM
> > initialization so that other "threads" are running in parallel,
> > because the TPM code wants to be initialized prior to running option
> > roms and other devices.
> > 
> > Could we do something like the below (completely untested)?  I don't
> > think we have to wait for the TPM device to report ready, because in a
> > real world scenario it would take an x86 cpu hundreds of milliseconds
> > from power on to get to this point of the code anyway.
> 
> I had thought about something like that also. Do we have the time in
> milliseconds since power-on or reset? We could subtract the timeout values
> you are discarding below from it and wait for the time difference, ignoring
> negative values of course. TIS_DEFAULT_TIMEOUT_A is 750ms, so more critical
> than TIS2_DEFAULT_TIMEOUT_D with 30ms.

Unfortunately, we don't have that info.  It's not easy to get it
because the clock detection code doesn't run until we're nearly at the
tpmhw_probe() code anyway.

That said, for tis_probe() is there any harm in moving the didvid
check up?  I understand it theoretically isn't setup yet, but I'd
imagine in practice it always would be.  Just as, in practice, I
suspect the tpm would always be ready by the time we got to the
tpmhw_probe() code.  See below.

-Kevin


--- a/src/hw/tpm_drivers.c
+++ b/src/hw/tpm_drivers.c
@@ -78,7 +78,8 @@ static u32 wait_reg8(u8* reg, u32 time, u8 mask, u8 expect)
             break;
         }
         if (timer_check(end)) {
-            warn_timeout();
+            if (time)
+                warn_timeout();
             break;
         }
         yield();
@@ -107,6 +108,10 @@ static u32 tis_probe(void)
     if (!CONFIG_TCGBIOS)
         return 0;
 
+    u32 didvid = readl(TIS_REG(0, TIS_REG_DID_VID));
+    if (didvid == 0 || didvid == 0xffffffff)
+        return 0;
+
     /* Wait for the interface to report it's ready */
     u32 rc = tis_wait_access(0, TIS_DEFAULT_TIMEOUT_A,
                              TIS_ACCESS_TPM_REG_VALID_STS,
@@ -114,11 +119,6 @@ static u32 tis_probe(void)
     if (rc)
         return 0;
 
-    u32 didvid = readl(TIS_REG(0, TIS_REG_DID_VID));
-
-    if ((didvid != 0) && (didvid != 0xffffffff))
-        rc = 1;
-
     /* TPM 2 has an interface register */
     u32 ifaceid = readl(TIS_REG(0, TIS_REG_IFACE_ID));
 
@@ -137,7 +137,7 @@ static u32 tis_probe(void)
         writel(TIS_REG(0, TIS_REG_IFACE_ID), (1 << 19));
     }
 
-    return rc;
+    return 1;
 }
 
 static TPMVersion tis_get_tpm_version(void)
@@ -385,7 +385,7 @@ static u32 crb_probe(void)
         return 0;
 
     /* Wait for the interface to report it's ready */
-    u32 rc = crb_wait_reg(0, CRB_REG_LOC_STATE, TIS2_DEFAULT_TIMEOUT_D,
+    u32 rc = crb_wait_reg(0, CRB_REG_LOC_STATE, 0,
                           CRB_STATE_READY_MASK, CRB_STATE_VALID_STS);
     if (rc)
         return 0;

_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios

Reply via email to