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




_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios

Reply via email to