We've run into a case where a customer has an STM TPM 1.2 chip
(version 1.2.8.28), that is getting into an inconsistent state and
they end up getting tpm transmit errors. In really old tpm code this
wasn't seen because the code that grabbed the duration values from the
chip could fail silently, and would proceed to just use default values
and move forward. More recent code though successfully gets the
duration values from the chip, and using those values this particular
chip version gets into the state seen by the customer.
The idea with this patchset is to provide a facility like the
update_timeouts operation to allow the override of chip supplied
values.
changes from v3:
* Assign value to version when tpm1_getcap is successful for TPM 1.1 device
not when it fails.
changes from v2:
* Added patch 1/3
* Rework tpm_tis_update_durations to make use of new version structs
and pull tpm1_getcap calls out of loop.
changes from v1:
* Remove unneeded newline
* Formatting cleanups
* Change tpm_tis_update_durations to be a void function, and
use chip->duration_adjusted to track whether adjustment was
made.
Jarkko Sakkinen (1):
tpm: Remove duplicate code from caps_show() in tpm-sysfs.c
Jerry Snitselaar (2):
tpm: provide a way to override the chip returned durations
tpm_tis: override durations for STM tpm with firmware 1.2.8.28
Patch adds method ->update_durations to override returned
durations in case TPM chip misbehaves for TPM 1.2 drivers.
Cc: Peter Huewe <[email protected]>
Cc: Jason Gunthorpe <[email protected]>
Signed-off-by: Alexey Klimov <[email protected]>
Signed-off-by: Jerry Snitselaar <[email protected]>
Reviewed-by: Jarkko Sakkinen <[email protected]>
---
drivers/char/tpm/tpm1-cmd.c | 15 +++++++++++++++
include/linux/tpm.h | 2 ++
2 files changed, 17 insertions(+)
diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c
index 149e953ca369..ca7158fa6e6c 100644
--- a/drivers/char/tpm/tpm1-cmd.c
+++ b/drivers/char/tpm/tpm1-cmd.c
@@ -343,6 +343,7 @@ int tpm1_get_timeouts(struct tpm_chip *chip)
{
cap_t cap;
unsigned long timeout_old[4], timeout_chip[4], timeout_eff[4];
+ unsigned long durations[3];
ssize_t rc;
rc = tpm1_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, &cap, NULL,
@@ -427,6 +428,20 @@ int tpm1_get_timeouts(struct tpm_chip *chip)
usecs_to_jiffies(be32_to_cpu(cap.duration.tpm_long));
chip->duration[TPM_LONG_LONG] = 0; /* not used under 1.2 */
+ /*
+ * Provide the ability for vendor overrides of duration values in case
+ * of misreporting.
+ */
+ if (chip->ops->update_durations)
+ chip->ops->update_durations(chip, durations);
+
+ if (chip->duration_adjusted) {
+ dev_info(&chip->dev, HW_ERR "Adjusting reported durations.");
+ chip->duration[TPM_SHORT] = durations[0];
+ chip->duration[TPM_MEDIUM] = durations[1];
+ chip->duration[TPM_LONG] = durations[2];
+ }
+
/* The Broadcom BCM0102 chipset in a Dell Latitude D820 gets the above
* value wrong and apparently reports msecs rather than usecs. So we
* fix up the resulting too-small TPM_SHORT value to make things work.
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index 53c0ea9ec9df..bb1d1ac7081d 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -67,6 +67,8 @@ struct tpm_class_ops {
u8 (*status) (struct tpm_chip *chip);
void (*update_timeouts)(struct tpm_chip *chip,
unsigned long *timeout_cap);
+ void (*update_durations)(struct tpm_chip *chip,
+ unsigned long *duration_cap);
int (*go_idle)(struct tpm_chip *chip);
int (*cmd_ready)(struct tpm_chip *chip);
int (*request_locality)(struct tpm_chip *chip, int loc);
--
2.21.0
On Mon, 2019-09-02 at 07:27 -0700, Jerry Snitselaar wrote:
> Patch adds method ->update_durations to override returned
> durations in case TPM chip misbehaves for TPM 1.2 drivers.
>
> Cc: Peter Huewe <[email protected]>
> Cc: Jason Gunthorpe <[email protected]>
> Signed-off-by: Alexey Klimov <[email protected]>
> Signed-off-by: Jerry Snitselaar <[email protected]>
> Reviewed-by: Jarkko Sakkinen <[email protected]>
Tested-by: Jarkko Sakkinen <[email protected]> (!update_durations path)
/Jarkko
On Mon Sep 02 19, Jerry Snitselaar wrote:
>We've run into a case where a customer has an STM TPM 1.2 chip
>(version 1.2.8.28), that is getting into an inconsistent state and
>they end up getting tpm transmit errors. In really old tpm code this
>wasn't seen because the code that grabbed the duration values from the
>chip could fail silently, and would proceed to just use default values
>and move forward. More recent code though successfully gets the
>duration values from the chip, and using those values this particular
>chip version gets into the state seen by the customer.
>
>The idea with this patchset is to provide a facility like the
>update_timeouts operation to allow the override of chip supplied
>values.
>
>changes from v3:
> * Assign value to version when tpm1_getcap is successful for TPM 1.1 device
> not when it fails.
>
>changes from v2:
> * Added patch 1/3
> * Rework tpm_tis_update_durations to make use of new version structs
> and pull tpm1_getcap calls out of loop.
>
>changes from v1:
> * Remove unneeded newline
> * Formatting cleanups
> * Change tpm_tis_update_durations to be a void function, and
> use chip->duration_adjusted to track whether adjustment was
> made.
>
>Jarkko Sakkinen (1):
> tpm: Remove duplicate code from caps_show() in tpm-sysfs.c
>
>Jerry Snitselaar (2):
> tpm: provide a way to override the chip returned durations
> tpm_tis: override durations for STM tpm with firmware 1.2.8.28
>
>
Anyone else have any feedback on this patchset?
On Sat, Sep 28, 2019 at 10:45:43AM -0700, Jerry Snitselaar wrote:
> On Mon Sep 02 19, Jerry Snitselaar wrote:
> > We've run into a case where a customer has an STM TPM 1.2 chip
> > (version 1.2.8.28), that is getting into an inconsistent state and
> > they end up getting tpm transmit errors. In really old tpm code this
> > wasn't seen because the code that grabbed the duration values from the
> > chip could fail silently, and would proceed to just use default values
> > and move forward. More recent code though successfully gets the
> > duration values from the chip, and using those values this particular
> > chip version gets into the state seen by the customer.
> >
> > The idea with this patchset is to provide a facility like the
> > update_timeouts operation to allow the override of chip supplied
> > values.
> >
> > changes from v3:
> > * Assign value to version when tpm1_getcap is successful for TPM 1.1 device
> > not when it fails.
> >
> > changes from v2:
> > * Added patch 1/3
> > * Rework tpm_tis_update_durations to make use of new version structs
> > and pull tpm1_getcap calls out of loop.
> >
> > changes from v1:
> > * Remove unneeded newline
> > * Formatting cleanups
> > * Change tpm_tis_update_durations to be a void function, and
> > use chip->duration_adjusted to track whether adjustment was
> > made.
> >
> > Jarkko Sakkinen (1):
> > tpm: Remove duplicate code from caps_show() in tpm-sysfs.c
> >
> > Jerry Snitselaar (2):
> > tpm: provide a way to override the chip returned durations
> > tpm_tis: override durations for STM tpm with firmware 1.2.8.28
> >
> >
>
> Anyone else have any feedback on this patchset?
Thanks for reminding. I'll put this to my master soonish.
/Jarkko
On Mon, Sep 02, 2019 at 07:27:32AM -0700, Jerry Snitselaar wrote:
> We've run into a case where a customer has an STM TPM 1.2 chip
> (version 1.2.8.28), that is getting into an inconsistent state and
> they end up getting tpm transmit errors. In really old tpm code this
> wasn't seen because the code that grabbed the duration values from the
> chip could fail silently, and would proceed to just use default values
> and move forward. More recent code though successfully gets the
> duration values from the chip, and using those values this particular
> chip version gets into the state seen by the customer.
>
> The idea with this patchset is to provide a facility like the
> update_timeouts operation to allow the override of chip supplied
> values.
>
> changes from v3:
> * Assign value to version when tpm1_getcap is successful for TPM 1.1 device
> not when it fails.
>
> changes from v2:
> * Added patch 1/3
> * Rework tpm_tis_update_durations to make use of new version structs
> and pull tpm1_getcap calls out of loop.
>
> changes from v1:
> * Remove unneeded newline
> * Formatting cleanups
> * Change tpm_tis_update_durations to be a void function, and
> use chip->duration_adjusted to track whether adjustment was
> made.
>
> Jarkko Sakkinen (1):
> tpm: Remove duplicate code from caps_show() in tpm-sysfs.c
>
> Jerry Snitselaar (2):
> tpm: provide a way to override the chip returned durations
> tpm_tis: override durations for STM tpm with firmware 1.2.8.28
>
>
I applied to my master branch.
Probably hard to get wide testing given the "niche" case when the
issue happens. Should be sufficient that the commonc case still
works.
/Jarkko
On Wed Oct 02 19, Jarkko Sakkinen wrote:
>On Mon, Sep 02, 2019 at 07:27:32AM -0700, Jerry Snitselaar wrote:
>> We've run into a case where a customer has an STM TPM 1.2 chip
>> (version 1.2.8.28), that is getting into an inconsistent state and
>> they end up getting tpm transmit errors. In really old tpm code this
>> wasn't seen because the code that grabbed the duration values from the
>> chip could fail silently, and would proceed to just use default values
>> and move forward. More recent code though successfully gets the
>> duration values from the chip, and using those values this particular
>> chip version gets into the state seen by the customer.
>>
>> The idea with this patchset is to provide a facility like the
>> update_timeouts operation to allow the override of chip supplied
>> values.
>>
>> changes from v3:
>> * Assign value to version when tpm1_getcap is successful for TPM 1.1 device
>> not when it fails.
>>
>> changes from v2:
>> * Added patch 1/3
>> * Rework tpm_tis_update_durations to make use of new version structs
>> and pull tpm1_getcap calls out of loop.
>>
>> changes from v1:
>> * Remove unneeded newline
>> * Formatting cleanups
>> * Change tpm_tis_update_durations to be a void function, and
>> use chip->duration_adjusted to track whether adjustment was
>> made.
>>
>> Jarkko Sakkinen (1):
>> tpm: Remove duplicate code from caps_show() in tpm-sysfs.c
>>
>> Jerry Snitselaar (2):
>> tpm: provide a way to override the chip returned durations
>> tpm_tis: override durations for STM tpm with firmware 1.2.8.28
>>
>>
>
>I applied to my master branch.
>
>Probably hard to get wide testing given the "niche" case when the
>issue happens. Should be sufficient that the commonc case still
>works.
>
>/Jarkko
Yeah, it is a pain. The people with the problem systems tested an
earlier version of Alexey's patches. I have a system with a different
rev STM device, so I did some testing with a modified patch that keyed
off that revision, but it will be hard to get it wide exposure.
On Thu, Oct 03, 2019 at 09:55:51AM -0700, Jerry Snitselaar wrote:
> On Wed Oct 02 19, Jarkko Sakkinen wrote:
> > On Mon, Sep 02, 2019 at 07:27:32AM -0700, Jerry Snitselaar wrote:
> > > We've run into a case where a customer has an STM TPM 1.2 chip
> > > (version 1.2.8.28), that is getting into an inconsistent state and
> > > they end up getting tpm transmit errors. In really old tpm code this
> > > wasn't seen because the code that grabbed the duration values from the
> > > chip could fail silently, and would proceed to just use default values
> > > and move forward. More recent code though successfully gets the
> > > duration values from the chip, and using those values this particular
> > > chip version gets into the state seen by the customer.
> > >
> > > The idea with this patchset is to provide a facility like the
> > > update_timeouts operation to allow the override of chip supplied
> > > values.
> > >
> > > changes from v3:
> > > * Assign value to version when tpm1_getcap is successful for TPM 1.1 device
> > > not when it fails.
> > >
> > > changes from v2:
> > > * Added patch 1/3
> > > * Rework tpm_tis_update_durations to make use of new version structs
> > > and pull tpm1_getcap calls out of loop.
> > >
> > > changes from v1:
> > > * Remove unneeded newline
> > > * Formatting cleanups
> > > * Change tpm_tis_update_durations to be a void function, and
> > > use chip->duration_adjusted to track whether adjustment was
> > > made.
> > >
> > > Jarkko Sakkinen (1):
> > > tpm: Remove duplicate code from caps_show() in tpm-sysfs.c
> > >
> > > Jerry Snitselaar (2):
> > > tpm: provide a way to override the chip returned durations
> > > tpm_tis: override durations for STM tpm with firmware 1.2.8.28
> > >
> > >
> >
> > I applied to my master branch.
> >
> > Probably hard to get wide testing given the "niche" case when the
> > issue happens. Should be sufficient that the commonc case still
> > works.
> >
> > /Jarkko
>
> Yeah, it is a pain. The people with the problem systems tested an
> earlier version of Alexey's patches. I have a system with a different
> rev STM device, so I did some testing with a modified patch that keyed
> off that revision, but it will be hard to get it wide exposure.
I think this is sufficient for me as it
1. Fixes the issue.
2. I've verified that it doesn't break systems that don't have the
issue
The worst case scenario is that you break something that is broken
already...
/Jarkko