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?
_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios
