On 03/25/2018 07:46 PM, Kevin O'Connor wrote:
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;
-
The didvid worked well for TPM 1.2. I think we should duplicate the
reading of it, once before the tis_wait_access and keep it after.
Stefan
/* 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
