[SeaBIOS] [PATCH v2 1/3] tpm: Wait for tpmRegValidSts flag on CRQ interface before probing
Stefan Berger
stefanb at linux.vnet.ibm.com
Mon Mar 26 19:03:21 CEST 2018
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;
>
More information about the SeaBIOS
mailing list