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

[Stephen Douthit]

> On 03/13/2018 10:40 AM, Stefan Berger wrote:
>> 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.
> 
> It's fine with the caveat that no vendor in the future is ever assigned
> 0xffff or 0x0000.  It's a low risk, but not a no risk scenario.
> 
>> I think that should be a first test, maybe also for CRB.
> 
> I don't think we can make that the first test.  If we don't wait for
> tpmRegValidSts (qualified by some known zero bit), then we can't tell
> the difference between no-TPM, and reading before the device is ready.
> 
> Note 2 in section 6.6 of the TIS 1.3 spec:
> 
> 2. Within 30 milliseconds of the completion of TPM_Init:
>    a. All fields within the access register and all other registers MUST
>       return with the state of all their fields valid (i.e.
>       TPM_ACCESS_x.tpmRegValidSts is set to ‘1’).
>    b. The TPM MUST be ready to receive a command
> 
> Maybe the best thing to do is try to skip probing entirely based if we
> don't see a related ACPI table.  If we do probe, and have to wait in the
> corrected wait code where we qualify tpmRegValidSts against vendor ID or
> Seize, and we still find no device, we should probably print a message
> to the user.
> 
> I don't see a way around some potential wait if we want to support real
> hardware devices reliably.

Or maybe we could skip polling on QEMU based on CONFIG_QEMU?