[SeaBIOS] [PATCH 1/3] tpm: Wait for tpmRegValidSts flags on CRQ interface before probing
stephend at silicom-usa.com
Mon Mar 19 15:48:37 CET 2018
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 at 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)
+ /* 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.
More information about the SeaBIOS