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