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

[Stefan Berger]

On 03/13/2018 10:15 AM, Stephen Douthit wrote:
>> 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.

Right...

>
>>              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.

We could just look at the vendor id part of didvid and follow this 
document here. There's neither a 0x0000 nor a 0xffff vendor -- assuming 
the list is complete.

https://trustedcomputinggroup.org/wp-content/uploads/TCG-TPM-Vendor-ID-Registry-Version-1.01-Revision-1.00.pdf

So, following that logic, it cannot return 32bit ~0 (or 0), either. So 
the existing test is fine from what I can see.

I think that should be a first test, maybe also for CRB.

     Stefan