2019-08-28 00:47:48

by Jerry Snitselaar

[permalink] [raw]
Subject: [PATCH 2/2 v2] tpm_tis: override durations for STM tpm with firmware 1.2.8.28

There was revealed a bug in the STM TPM chipset used in Dell R415s.
Bug is observed so far only on chipset firmware 1.2.8.28
(1.2 TPM, device-id 0x0, rev-id 78). After some number of
operations chipset hangs and stays in inconsistent state:

tpm_tis 00:09: Operation Timed out
tpm_tis 00:09: tpm_transmit: tpm_send: error -5

Durations returned by the chip are the same like on other
firmware revisions but apparently with specifically 1.2.8.28 fw
durations should be reset to 2 minutes to enable tpm chip work
properly. No working way of updating firmware was found.

This patch adds implementation of ->update_durations method
that matches only STM devices with specific firmware version.

Cc: Peter Huewe <[email protected]>
Cc: Jarkko Sakkinen <[email protected]>
Cc: Jason Gunthorpe <[email protected]>
Signed-off-by: Alexey Klimov <[email protected]>
Signed-off-by: Jerry Snitselaar <[email protected]>
---
v2: Make suggested changes from Jarkko
- change struct field name to durations from durs
- formatting cleanups
- turn into void function like update_timeouts and
use chip->duration_adjusted to track whether adjustment occurred.

drivers/char/tpm/tpm_tis_core.c | 91 +++++++++++++++++++++++++++++++++
1 file changed, 91 insertions(+)

diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index c3181ea9f271..81b65ec2a41b 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -506,6 +506,96 @@ static int tpm_tis_send(struct tpm_chip *chip, u8 *buf, size_t len)
return rc;
}

+struct tis_vendor_durations_override {
+ u32 did_vid;
+ struct tpm_version_t tpm_version;
+ unsigned long durations[3];
+};
+
+static const struct tis_vendor_durations_override vendor_dur_overrides[] = {
+ /* STMicroelectronics 0x104a */
+ { 0x0000104a,
+ { 1, 2, 8, 28 },
+ { (2 * 60 * HZ), (2 * 60 * HZ), (2 * 60 * HZ) } },
+};
+
+static void tpm_tis_update_durations(struct tpm_chip *chip,
+ unsigned long *duration_cap)
+{
+ struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
+ u32 did_vid;
+ int i, rc;
+ cap_t cap;
+
+ chip->duration_adjusted = false;
+
+ if (chip->ops->clk_enable != NULL)
+ chip->ops->clk_enable(chip, true);
+
+ rc = tpm_tis_read32(priv, TPM_DID_VID(0), &did_vid);
+ if (rc < 0) {
+ dev_warn(&chip->dev, "%s: failed to read did_vid. %d\n",
+ __func__, rc);
+ goto out;
+ }
+
+ for (i = 0; i != ARRAY_SIZE(vendor_dur_overrides); i++) {
+ if (vendor_dur_overrides[i].did_vid != did_vid)
+ continue;
+
+ /* Try to get a TPM version 1.2 TPM_CAP_VERSION_INFO */
+ rc = tpm1_getcap(chip, TPM_CAP_VERSION_1_2, &cap,
+ "attempting to determine the 1.2 version",
+ sizeof(cap.tpm_version_1_2));
+ if (!rc) {
+ if ((cap.tpm_version_1_2.Major ==
+ vendor_dur_overrides[i].tpm_version.Major) &&
+ (cap.tpm_version_1_2.Minor ==
+ vendor_dur_overrides[i].tpm_version.Minor) &&
+ (cap.tpm_version_1_2.revMajor ==
+ vendor_dur_overrides[i].tpm_version.revMajor) &&
+ (cap.tpm_version_1_2.revMinor ==
+ vendor_dur_overrides[i].tpm_version.revMinor)) {
+
+ memcpy(duration_cap,
+ vendor_dur_overrides[i].durations,
+ sizeof(vendor_dur_overrides[i].durations));
+
+ chip->duration_adjusted = true;
+ goto out;
+ }
+ } else {
+ rc = tpm1_getcap(chip, TPM_CAP_VERSION_1_1, &cap,
+ "attempting to determine the 1.1 version",
+ sizeof(cap.tpm_version));
+
+ if (rc)
+ goto out;
+
+ if ((cap.tpm_version.Major ==
+ vendor_dur_overrides[i].tpm_version.Major) &&
+ (cap.tpm_version.Minor ==
+ vendor_dur_overrides[i].tpm_version.Minor) &&
+ (cap.tpm_version.revMajor ==
+ vendor_dur_overrides[i].tpm_version.revMajor) &&
+ (cap.tpm_version.revMinor ==
+ vendor_dur_overrides[i].tpm_version.revMinor)) {
+
+ memcpy(duration_cap,
+ vendor_dur_overrides[i].durations,
+ sizeof(vendor_dur_overrides[i].durations));
+
+ chip->duration_adjusted = true;
+ goto out;
+ }
+ }
+ }
+
+out:
+ if (chip->ops->clk_enable != NULL)
+ chip->ops->clk_enable(chip, false);
+}
+
struct tis_vendor_timeout_override {
u32 did_vid;
unsigned long timeout_us[4];
@@ -842,6 +932,7 @@ static const struct tpm_class_ops tpm_tis = {
.send = tpm_tis_send,
.cancel = tpm_tis_ready,
.update_timeouts = tpm_tis_update_timeouts,
+ .update_durations = tpm_tis_update_durations,
.req_complete_mask = TPM_STS_DATA_AVAIL | TPM_STS_VALID,
.req_complete_val = TPM_STS_DATA_AVAIL | TPM_STS_VALID,
.req_canceled = tpm_tis_req_canceled,
--
2.21.0


2019-08-29 14:42:44

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH 2/2 v2] tpm_tis: override durations for STM tpm with firmware 1.2.8.28

On Tue, Aug 27, 2019 at 05:46:21PM -0700, Jerry Snitselaar wrote:
> There was revealed a bug in the STM TPM chipset used in Dell R415s.
> Bug is observed so far only on chipset firmware 1.2.8.28
> (1.2 TPM, device-id 0x0, rev-id 78). After some number of
> operations chipset hangs and stays in inconsistent state:
>
> tpm_tis 00:09: Operation Timed out
> tpm_tis 00:09: tpm_transmit: tpm_send: error -5
>
> Durations returned by the chip are the same like on other
> firmware revisions but apparently with specifically 1.2.8.28 fw
> durations should be reset to 2 minutes to enable tpm chip work
> properly. No working way of updating firmware was found.
>
> This patch adds implementation of ->update_durations method
> that matches only STM devices with specific firmware version.
>
> Cc: Peter Huewe <[email protected]>
> Cc: Jarkko Sakkinen <[email protected]>
> Cc: Jason Gunthorpe <[email protected]>
> Signed-off-by: Alexey Klimov <[email protected]>
> Signed-off-by: Jerry Snitselaar <[email protected]>
> ---
> v2: Make suggested changes from Jarkko
> - change struct field name to durations from durs
> - formatting cleanups
> - turn into void function like update_timeouts and
> use chip->duration_adjusted to track whether adjustment occurred.

The code repetition looks horrible so I wrote a patch that should help:

https://patchwork.kernel.org/patch/11121475/

Read the remar that prepends the diffstat.

/Jarkko

2019-08-29 14:45:07

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH 2/2 v2] tpm_tis: override durations for STM tpm with firmware 1.2.8.28

On Thu, Aug 29, 2019 at 05:40:40PM +0300, Jarkko Sakkinen wrote:
> On Tue, Aug 27, 2019 at 05:46:21PM -0700, Jerry Snitselaar wrote:
> > There was revealed a bug in the STM TPM chipset used in Dell R415s.
> > Bug is observed so far only on chipset firmware 1.2.8.28
> > (1.2 TPM, device-id 0x0, rev-id 78). After some number of
> > operations chipset hangs and stays in inconsistent state:
> >
> > tpm_tis 00:09: Operation Timed out
> > tpm_tis 00:09: tpm_transmit: tpm_send: error -5
> >
> > Durations returned by the chip are the same like on other
> > firmware revisions but apparently with specifically 1.2.8.28 fw
> > durations should be reset to 2 minutes to enable tpm chip work
> > properly. No working way of updating firmware was found.
> >
> > This patch adds implementation of ->update_durations method
> > that matches only STM devices with specific firmware version.
> >
> > Cc: Peter Huewe <[email protected]>
> > Cc: Jarkko Sakkinen <[email protected]>
> > Cc: Jason Gunthorpe <[email protected]>
> > Signed-off-by: Alexey Klimov <[email protected]>
> > Signed-off-by: Jerry Snitselaar <[email protected]>
> > ---
> > v2: Make suggested changes from Jarkko
> > - change struct field name to durations from durs
> > - formatting cleanups
> > - turn into void function like update_timeouts and
> > use chip->duration_adjusted to track whether adjustment occurred.
>
> The code repetition looks horrible so I wrote a patch that should help:
>
> https://patchwork.kernel.org/patch/11121475/
>
> Read the remar that prepends the diffstat.

Forgot from that remark that I did not have TPM 1.x available at hand
(WFH today) so please also review and test it.

/Jrakko

2019-08-29 18:05:53

by Jerry Snitselaar

[permalink] [raw]
Subject: Re: [PATCH 2/2 v2] tpm_tis: override durations for STM tpm with firmware 1.2.8.28

On Thu Aug 29 19, Jarkko Sakkinen wrote:
>On Thu, Aug 29, 2019 at 05:40:40PM +0300, Jarkko Sakkinen wrote:
>> On Tue, Aug 27, 2019 at 05:46:21PM -0700, Jerry Snitselaar wrote:
>> > There was revealed a bug in the STM TPM chipset used in Dell R415s.
>> > Bug is observed so far only on chipset firmware 1.2.8.28
>> > (1.2 TPM, device-id 0x0, rev-id 78). After some number of
>> > operations chipset hangs and stays in inconsistent state:
>> >
>> > tpm_tis 00:09: Operation Timed out
>> > tpm_tis 00:09: tpm_transmit: tpm_send: error -5
>> >
>> > Durations returned by the chip are the same like on other
>> > firmware revisions but apparently with specifically 1.2.8.28 fw
>> > durations should be reset to 2 minutes to enable tpm chip work
>> > properly. No working way of updating firmware was found.
>> >
>> > This patch adds implementation of ->update_durations method
>> > that matches only STM devices with specific firmware version.
>> >
>> > Cc: Peter Huewe <[email protected]>
>> > Cc: Jarkko Sakkinen <[email protected]>
>> > Cc: Jason Gunthorpe <[email protected]>
>> > Signed-off-by: Alexey Klimov <[email protected]>
>> > Signed-off-by: Jerry Snitselaar <[email protected]>
>> > ---
>> > v2: Make suggested changes from Jarkko
>> > - change struct field name to durations from durs
>> > - formatting cleanups
>> > - turn into void function like update_timeouts and
>> > use chip->duration_adjusted to track whether adjustment occurred.
>>
>> The code repetition looks horrible so I wrote a patch that should help:
>>
>> https://patchwork.kernel.org/patch/11121475/
>>
>> Read the remar that prepends the diffstat.
>
>Forgot from that remark that I did not have TPM 1.x available at hand
>(WFH today) so please also review and test it.
>
>/Jrakko

I will test it this morning, and once that is done I'll submit a v3 that
cleans up the version comparison in the update_durations function.

2019-08-29 20:26:42

by Jerry Snitselaar

[permalink] [raw]
Subject: Re: [PATCH 2/2 v2] tpm_tis: override durations for STM tpm with firmware 1.2.8.28

On Thu Aug 29 19, Jarkko Sakkinen wrote:
>On Tue, Aug 27, 2019 at 05:46:21PM -0700, Jerry Snitselaar wrote:
>> There was revealed a bug in the STM TPM chipset used in Dell R415s.
>> Bug is observed so far only on chipset firmware 1.2.8.28
>> (1.2 TPM, device-id 0x0, rev-id 78). After some number of
>> operations chipset hangs and stays in inconsistent state:
>>
>> tpm_tis 00:09: Operation Timed out
>> tpm_tis 00:09: tpm_transmit: tpm_send: error -5
>>
>> Durations returned by the chip are the same like on other
>> firmware revisions but apparently with specifically 1.2.8.28 fw
>> durations should be reset to 2 minutes to enable tpm chip work
>> properly. No working way of updating firmware was found.
>>
>> This patch adds implementation of ->update_durations method
>> that matches only STM devices with specific firmware version.
>>
>> Cc: Peter Huewe <[email protected]>
>> Cc: Jarkko Sakkinen <[email protected]>
>> Cc: Jason Gunthorpe <[email protected]>
>> Signed-off-by: Alexey Klimov <[email protected]>
>> Signed-off-by: Jerry Snitselaar <[email protected]>
>> ---
>> v2: Make suggested changes from Jarkko
>> - change struct field name to durations from durs
>> - formatting cleanups
>> - turn into void function like update_timeouts and
>> use chip->duration_adjusted to track whether adjustment occurred.
>
>The code repetition looks horrible so I wrote a patch that should help:
>
>https://patchwork.kernel.org/patch/11121475/
>
>Read the remar that prepends the diffstat.
>
>/Jarkko

LGTM, and testing it on a 1.2 tpm system here worked fine. You can add my
Reviewed-by and Tested-by.

I have reworked this 2/2 patch to make use of these new structs and
pull the tpm1_getcap calls out of the for loop. So I will submit a v3
to go on top of your patch.

2019-08-29 22:24:25

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH 2/2 v2] tpm_tis: override durations for STM tpm with firmware 1.2.8.28

On Thu, Aug 29, 2019 at 01:25:27PM -0700, Jerry Snitselaar wrote:
> On Thu Aug 29 19, Jarkko Sakkinen wrote:
> > On Tue, Aug 27, 2019 at 05:46:21PM -0700, Jerry Snitselaar wrote:
> > > There was revealed a bug in the STM TPM chipset used in Dell R415s.
> > > Bug is observed so far only on chipset firmware 1.2.8.28
> > > (1.2 TPM, device-id 0x0, rev-id 78). After some number of
> > > operations chipset hangs and stays in inconsistent state:
> > >
> > > tpm_tis 00:09: Operation Timed out
> > > tpm_tis 00:09: tpm_transmit: tpm_send: error -5
> > >
> > > Durations returned by the chip are the same like on other
> > > firmware revisions but apparently with specifically 1.2.8.28 fw
> > > durations should be reset to 2 minutes to enable tpm chip work
> > > properly. No working way of updating firmware was found.
> > >
> > > This patch adds implementation of ->update_durations method
> > > that matches only STM devices with specific firmware version.
> > >
> > > Cc: Peter Huewe <[email protected]>
> > > Cc: Jarkko Sakkinen <[email protected]>
> > > Cc: Jason Gunthorpe <[email protected]>
> > > Signed-off-by: Alexey Klimov <[email protected]>
> > > Signed-off-by: Jerry Snitselaar <[email protected]>
> > > ---
> > > v2: Make suggested changes from Jarkko
> > > - change struct field name to durations from durs
> > > - formatting cleanups
> > > - turn into void function like update_timeouts and
> > > use chip->duration_adjusted to track whether adjustment occurred.
> >
> > The code repetition looks horrible so I wrote a patch that should help:
> >
> > https://patchwork.kernel.org/patch/11121475/
> >
> > Read the remar that prepends the diffstat.
> >
> > /Jarkko
>
> LGTM, and testing it on a 1.2 tpm system here worked fine. You can add my
> Reviewed-by and Tested-by.
>
> I have reworked this 2/2 patch to make use of these new structs and
> pull the tpm1_getcap calls out of the for loop. So I will submit a v3
> to go on top of your patch.

Add my patch to the patch set with the tags.

/Jarkko