[SeaBIOS] Long delay: WARNING - Timeout at wait_reg8:81!

[Stephen Douthit]

> When tis_probe() returns '1', it means the interface was detected.
> If all registers return 0xffffffff in the no-TPM case we should return a '0' from tis_probe since rc was set to 0 from tis_wait_access() and we will not get into the ifaceid test case. If they return 0 then we shouldn't get past the tis_wait_access() because the expected pattern would not show up. I think tis_probe() is correct.

Sounds good, I assumed I had screwed this up based on Paul's original
bug report, and was still carrying the patch for my 'fix' to this
function when I was doing my testing.
  
> static u32 crb_probe(void)
> {
>      if (!CONFIG_TCGBIOS)
>          return 0;
> 
>      u32 ifaceid = readl(CRB_REG(0, CRB_REG_INTF_ID));
> 
>      if ((ifaceid & 0xf) != 0xf) {
>          if ((ifaceid & 0xf) == 1) {
>              /* CRB is active */

We also return here without doing the 64 bit address check.

>              return 1;
>          }
>          if ((ifaceid & (1 << 14)) == 0) {
>              /* CRB cannot be selected */
>              return 0;
>          }
>          /* write of 1 to bits 17-18 selects CRB */
>          writel(CRB_REG(0, CRB_REG_INTF_ID), (1 << 17));
>          /* lock it */
>          writel(CRB_REG(0, CRB_REG_INTF_ID), (1 << 19));
>      }
> 
>      /* no support for 64 bit addressing yet */
>      if (readl(CRB_REG(0, CRB_REG_CTRL_CMD_HADDR)))
>          return 1;
> 
>      u64 addr = readq(CRB_REG(0, CRB_REG_CTRL_RSP_ADDR));
>      if (addr > 0xffffffff)
>          return 1;
> 
>      return 0;
> }
> 
> if ifaceid returns ~0 we may return when reading CRB_REG_CTRL_CMD_HADDR. So probably we should test for ifaceid = ~0 after it is read and return 0.
> 
> 
> 
>>> inverted from what they should be, and the first 64bit address check
>>> returned the wrong value.  Fixing both probe functions got rid of the
>>> timeout for me when the TPM was disconnected.
> 
> I see a problem with the crb_probe function but not the tis_probe.
> 
>>>
>>> It looks like there's a bit in the ACCESS register called Seize that
>>> must always read '0' for the version 1.2/1.3 interfaces.  I'd like to
>>> check that instead of didvid in tis_probe to handle the aborted read all
>>> 0s/Fs case.
> 
> Why would there be a 'aborted read all'? I think the didvid registers should not return either 0 or ~0 for there to be a valid device. Why would that not be a good first test to see whether anything is there?

The old parallel PCI bus referred to the termination of any access to a
bad address where no device responded as 'Master Abort'.  That's where I
got the aborted read term from.  As Kevin mentioned, on x86 this case
normally results in a read of all Fs.

The reason I want to move to Seize (or something like it) instead of
didvid is that the spec actually says that _must_ always return '0', I
don't see any explicit restrictions on didvid.

>>>
>>> I'd like to add a poll for tpmRegValidSts to crb_probe() similar to
>>> what's in tis_probe() to avoid potential races on real hardware.
> 
> That may be something necessary on real hardware, though in case nothing is there we all need to wait... I am not sure whether the checking for these bits has impact on the didvid registers. ValidSts presumably means that the STS register has valid bits. I don't have real hardware anymore -- my Acer died a while ago, so I cannot tell but its hardware didn't need the wait.

For x86 if there's no device then there is no wait.  The valid flag is
still set in what I was calling the abort case.  In this case it's not
returned by a device, but by the bus read logic.

With the ACPI check reorder we only have to wait in the following
scenario.

1) CONFIG_TPM is set
2) There's a bogus TCPA/TPM2 table for a device that doesn't exist.
3) There's no TPM
4) We're on a platform that returns 0 for aborted reads, so we poll
for a tpmRegValidSts that will never be set.

>>> There's a Seize bit in TPM_LOC_CTRL_x which always reads 0 that we could
>>> use as a sanity check against the no device all Fs case.
>>>
>>> Let me know if that sounds like a better way to catch the no device
>>> case, or if there's is some other check that would be better.
>> Thanks for looking at this.  It is common on x86 for invalid memory
>> accesses to return 0xff.  I don't know enough about the TPM hardware
>> to make a judgement call on the best way to test for presence.  I'd
>> like to hear what Stefan's thoughts are on this.
>>
>> -Kevin
>>
>