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