On 03/19/2018 08:55 AM, Stefan Berger wrote:
On 03/14/2018 03:42 PM, Kevin O'Connor
> 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(a)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 |
/* if device is not there, return '0', '1' otherwise */
static u32 crb_probe(void)
+ /* 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.
Ok. In that case we'll have to set that tpmRegValidSts field also in
QEMU upon reset. I'll take your changes for a v2 series and post again.