On 03/19/2018 08:55 AM, Stefan Berger wrote:
On 03/14/2018 03:42 PM, Kevin O'Connor wrote:
On Wed, Mar 14, 2018 at 01:42:41PM -0400, Stefan Berger wrote:
Wait for the tpmRegValidSts flag on the TPM_LOC_STATE_x register.
Since it's not quite clear when this flag may become valid, we request access to the interace on locality 0, which must then make it valid.
Signed-off-by: Stefan Berger stefanb@linux.vnet.ibm.com
src/hw/tpm_drivers.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/src/hw/tpm_drivers.c b/src/hw/tpm_drivers.c index ed58bf5..ad97f67 100644 --- a/src/hw/tpm_drivers.c +++ b/src/hw/tpm_drivers.c @@ -374,12 +374,23 @@ static u32 tis_waitrespready(enum tpmDurationType to_t) return rc; } +#define CRB_STATE_VALID_STS 0b10000000
/* if device is not there, return '0', '1' otherwise */ static u32 crb_probe(void) { if (!CONFIG_TCGBIOS) return 0; + /* request access -- this must cause a valid STS flag */ + writeb(CRB_REG(0, CRB_REG_LOC_CTRL), 1);
Would it be possible to do a sanity check with read operations before doing a write operation? The fear is that the device is not present and the write operation corrupts some other device, causes a bus error, or something else undesirable.
We don't need to force use of locality 0 for this polling. LOC_STATE is a single register aliased to all localities. We can just start polling on LOC_STATE immediately without the write.
There's also a note in the spec that locAssigned and activeLocality will be set to 0 as their initial state after power on.
If this one is controversal, the other two patches fix real bugs and we should consider applying them. I think for testing Stephen's feedback is primarily important on 1/3. Stephen, can you comment?
I think we can change this patch to something like this:
+#define CRB_STATE_VALID_STS 0b10000000 +#define CRB_STATE_LOC_ASSIGNED 0b00000010 +#define CRB_STATE_READY_MASK (CRB_STATE_VALID_STS | CRB_STATE_LOC_ASSIGNED) + /* if device is not there, return '0', '1' otherwise */ static u32 crb_probe(void) { if (!CONFIG_TCGBIOS) return 0;
+ /* Wait for the interface to report it's ready */ + u32 rc = crb_wait_reg(0, CRB_REG_LOC_STATE, TIS2_DEFAULT_TIMEOUT_D, + CRB_STATE_READY_MASK, CRB_STATE_VALID_STS); + if (rc) + return 0; +
I tested this on my system (2.0 TPM using FIFO interface) in the present and absent and it works as expected. This testing misses the CRB device present case though.
-Steve