[SeaBIOS] [PATCH v2 1/3] tpm: Wait for tpmRegValidSts flag on CRQ interface before probing

[Stefan Berger]

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.

>
> -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();
> @@ -108,7 +109,7 @@ static u32 tis_probe(void)
>           return 0;
>
>       /* Wait for the interface to report it's ready */
> -    u32 rc = tis_wait_access(0, TIS_DEFAULT_TIMEOUT_A,
> +    u32 rc = tis_wait_access(0, 0,
>                                TIS_ACCESS_TPM_REG_VALID_STS,
>                                TIS_ACCESS_TPM_REG_VALID_STS);
>       if (rc)
> @@ -385,7 +386,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;
>