* [PATCH REVIEW 1/2] tpm: provide a way to override the chip returned durations @ 2018-12-14 13:21 Alexey Klimov 2018-12-14 13:21 ` [PATCH REVIEW 2/2] tpm_tis: override durations for STM tpm with firmware 1.2.8.28 Alexey Klimov 2019-01-03 13:05 ` [PATCH REVIEW 1/2] tpm: provide a way to override the chip returned durations Jarkko Sakkinen 0 siblings, 2 replies; 8+ messages in thread From: Alexey Klimov @ 2018-12-14 13:21 UTC (permalink / raw) To: linux-integrity; +Cc: jsnitsel, jarkko.sakkinen, peterhuewe, jgg, aklimov Patch adds method ->update_durations to override returned durations in case TPM chip misbehaves for TPM 1.2 drivers. Cc: Jerry Snitselaar <jsnitsel@redhat.com> Signed-off-by: Alexey Klimov <aklimov@redhat.com> --- drivers/char/tpm/tpm-interface.c | 18 ++++++++++++++++++ include/linux/tpm.h | 2 ++ 2 files changed, 20 insertions(+) diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c index 129f640424b7..5cb90918938d 100644 --- a/drivers/char/tpm/tpm-interface.c +++ b/drivers/char/tpm/tpm-interface.c @@ -768,6 +768,7 @@ int tpm_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; if (chip->flags & TPM_CHIP_FLAG_HAVE_TIMEOUTS) @@ -877,6 +878,23 @@ int tpm_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 ability for vendor overrides of duration values in case + * of misreporting. + */ + if (chip->ops->update_durations != NULL) + chip->duration_adjusted = + chip->ops->update_durations(chip, durations); + + /* Report and set adjusted timeouts */ + if (chip->duration_adjusted) { + + dev_info(&chip->dev, HW_ERR "Adjusting reported durations\n"); + 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 4609b94142d4..d6018431b478 100644 --- a/include/linux/tpm.h +++ b/include/linux/tpm.h @@ -43,6 +43,8 @@ struct tpm_class_ops { u8 (*status) (struct tpm_chip *chip); bool (*update_timeouts)(struct tpm_chip *chip, unsigned long *timeout_cap); + bool (*update_durations)(struct tpm_chip *chip, + unsigned long *durations_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.14.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH REVIEW 2/2] tpm_tis: override durations for STM tpm with firmware 1.2.8.28 2018-12-14 13:21 [PATCH REVIEW 1/2] tpm: provide a way to override the chip returned durations Alexey Klimov @ 2018-12-14 13:21 ` Alexey Klimov 2019-01-03 13:14 ` Jarkko Sakkinen 2019-01-03 13:05 ` [PATCH REVIEW 1/2] tpm: provide a way to override the chip returned durations Jarkko Sakkinen 1 sibling, 1 reply; 8+ messages in thread From: Alexey Klimov @ 2018-12-14 13:21 UTC (permalink / raw) To: linux-integrity; +Cc: jsnitsel, jarkko.sakkinen, peterhuewe, jgg, aklimov 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: Jerry Snitselaar <jsnitsel@redhat.com> Signed-off-by: Alexey Klimov <aklimov@redhat.com> --- drivers/char/tpm/tpm_tis_core.c | 90 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 90 insertions(+) diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c index d2345d9fd7b5..e0bdca647460 100644 --- a/drivers/char/tpm/tpm_tis_core.c +++ b/drivers/char/tpm/tpm_tis_core.c @@ -514,6 +514,95 @@ 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 durs[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 bool 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; + + 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) + 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 = tpm_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].durs, + sizeof(vendor_dur_overrides[i].durs)); + rc = true; + goto out; + } + } else { + rc = tpm_getcap(chip, TPM_CAP_VERSION_1_1, &cap, + "attempting to determine the 1.1 version", + sizeof(cap.tpm_version)); + if (rc) { + rc = false; + 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].durs, + sizeof(vendor_dur_overrides[i].durs)); + rc = true; + goto out; + } + } + } + + rc = false; + +out: + if (chip->ops->clk_enable != NULL) + chip->ops->clk_enable(chip, false); + + return rc; +} + + struct tis_vendor_timeout_override { u32 did_vid; unsigned long timeout_us[4]; @@ -847,6 +936,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.14.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH REVIEW 2/2] tpm_tis: override durations for STM tpm with firmware 1.2.8.28 2018-12-14 13:21 ` [PATCH REVIEW 2/2] tpm_tis: override durations for STM tpm with firmware 1.2.8.28 Alexey Klimov @ 2019-01-03 13:14 ` Jarkko Sakkinen 2019-01-14 19:39 ` Jerry Snitselaar 0 siblings, 1 reply; 8+ messages in thread From: Jarkko Sakkinen @ 2019-01-03 13:14 UTC (permalink / raw) To: Alexey Klimov; +Cc: linux-integrity, jsnitsel, peterhuewe, jgg On Fri, Dec 14, 2018 at 01:21:15PM +0000, Alexey Klimov 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: Jerry Snitselaar <jsnitsel@redhat.com> > Signed-off-by: Alexey Klimov <aklimov@redhat.com> > --- > drivers/char/tpm/tpm_tis_core.c | 90 +++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 90 insertions(+) > > diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c > index d2345d9fd7b5..e0bdca647460 100644 > --- a/drivers/char/tpm/tpm_tis_core.c > +++ b/drivers/char/tpm/tpm_tis_core.c > @@ -514,6 +514,95 @@ 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 durs[3]; I would rather have just "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 bool 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; > + > + 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) > + 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 = tpm_getcap(chip, TPM_CAP_VERSION_1_2, &cap, > + "attempting to determine the 1.2 version", > + sizeof(cap.tpm_version_1_2)); Not properly aligned. > + 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)) { Same. > + > + memcpy(duration_cap, > + vendor_dur_overrides[i].durs, > + sizeof(vendor_dur_overrides[i].durs)); Same. > + rc = true; > + goto out; > + } > + } else { > + rc = tpm_getcap(chip, TPM_CAP_VERSION_1_1, &cap, > + "attempting to determine the 1.1 version", > + sizeof(cap.tpm_version)); Same. > + if (rc) { > + rc = false; > + 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)) { Same. > + > + memcpy(duration_cap, > + vendor_dur_overrides[i].durs, > + sizeof(vendor_dur_overrides[i].durs)); Same. > + rc = true; > + goto out; > + } > + } > + } > + > + rc = false; Shoud return proper rc instead of bool. > + > +out: > + if (chip->ops->clk_enable != NULL) > + chip->ops->clk_enable(chip, false); > + > + return rc; > +} > + > + > struct tis_vendor_timeout_override { > u32 did_vid; > unsigned long timeout_us[4]; > @@ -847,6 +936,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.14.4 > /Jarkko ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH REVIEW 2/2] tpm_tis: override durations for STM tpm with firmware 1.2.8.28 2019-01-03 13:14 ` Jarkko Sakkinen @ 2019-01-14 19:39 ` Jerry Snitselaar 2019-01-18 14:59 ` Jarkko Sakkinen 0 siblings, 1 reply; 8+ messages in thread From: Jerry Snitselaar @ 2019-01-14 19:39 UTC (permalink / raw) To: Jarkko Sakkinen; +Cc: Alexey Klimov, linux-integrity, peterhuewe, jgg On Thu Jan 03 19, Jarkko Sakkinen wrote: >On Fri, Dec 14, 2018 at 01:21:15PM +0000, Alexey Klimov 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: Jerry Snitselaar <jsnitsel@redhat.com> >> Signed-off-by: Alexey Klimov <aklimov@redhat.com> >> --- >> drivers/char/tpm/tpm_tis_core.c | 90 +++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 90 insertions(+) >> >> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c >> index d2345d9fd7b5..e0bdca647460 100644 >> --- a/drivers/char/tpm/tpm_tis_core.c >> +++ b/drivers/char/tpm/tpm_tis_core.c >> @@ -514,6 +514,95 @@ 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 durs[3]; > >I would rather have just "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 bool 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; >> + >> + 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) >> + 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 = tpm_getcap(chip, TPM_CAP_VERSION_1_2, &cap, >> + "attempting to determine the 1.2 version", >> + sizeof(cap.tpm_version_1_2)); > >Not properly aligned. > >> + 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)) { > >Same. > >> + >> + memcpy(duration_cap, >> + vendor_dur_overrides[i].durs, >> + sizeof(vendor_dur_overrides[i].durs)); > >Same. > >> + rc = true; >> + goto out; >> + } >> + } else { >> + rc = tpm_getcap(chip, TPM_CAP_VERSION_1_1, &cap, >> + "attempting to determine the 1.1 version", >> + sizeof(cap.tpm_version)); > >Same. > >> + if (rc) { >> + rc = false; >> + 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)) { > >Same. > >> + >> + memcpy(duration_cap, >> + vendor_dur_overrides[i].durs, >> + sizeof(vendor_dur_overrides[i].durs)); > >Same. > >> + rc = true; >> + goto out; >> + } >> + } >> + } >> + >> + rc = false; > >Shoud return proper rc instead of bool. > Alexey was following the example of tpm_tis_update_timeouts() which returns true if the timeouts were updated, and otherwise returns false. The bool here makes sense to me, but what rc would you suggest in this case? Regards, Jerry >> + >> +out: >> + if (chip->ops->clk_enable != NULL) >> + chip->ops->clk_enable(chip, false); >> + >> + return rc; >> +} >> + >> + >> struct tis_vendor_timeout_override { >> u32 did_vid; >> unsigned long timeout_us[4]; >> @@ -847,6 +936,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.14.4 >> > >/Jarkko ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH REVIEW 2/2] tpm_tis: override durations for STM tpm with firmware 1.2.8.28 2019-01-14 19:39 ` Jerry Snitselaar @ 2019-01-18 14:59 ` Jarkko Sakkinen 2019-01-20 21:30 ` Jerry Snitselaar 0 siblings, 1 reply; 8+ messages in thread From: Jarkko Sakkinen @ 2019-01-18 14:59 UTC (permalink / raw) To: Jerry Snitselaar; +Cc: Alexey Klimov, linux-integrity, peterhuewe, jgg On Mon, Jan 14, 2019 at 12:39:40PM -0700, Jerry Snitselaar wrote: > Alexey was following the example of tpm_tis_update_timeouts() which > returns true if the timeouts were updated, and otherwise returns > false. The bool here makes sense to me, but what rc would you suggest > in this case? Maybe the pattern used there is not that great then. The callback should simply be update_durations(chip), and it would do whatever updates needed and either return zero or -errno. And of course update durations_adjusted flag because that is needed in sysfs. /Jarkko ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH REVIEW 2/2] tpm_tis: override durations for STM tpm with firmware 1.2.8.28 2019-01-18 14:59 ` Jarkko Sakkinen @ 2019-01-20 21:30 ` Jerry Snitselaar 2019-01-21 12:29 ` Jarkko Sakkinen 0 siblings, 1 reply; 8+ messages in thread From: Jerry Snitselaar @ 2019-01-20 21:30 UTC (permalink / raw) To: Jarkko Sakkinen; +Cc: Alexey Klimov, linux-integrity, peterhuewe, jgg On Fri Jan 18 19, Jarkko Sakkinen wrote: >On Mon, Jan 14, 2019 at 12:39:40PM -0700, Jerry Snitselaar wrote: >> Alexey was following the example of tpm_tis_update_timeouts() which >> returns true if the timeouts were updated, and otherwise returns >> false. The bool here makes sense to me, but what rc would you suggest >> in this case? > >Maybe the pattern used there is not that great then. > >The callback should simply be update_durations(chip), and it would do >whatever updates needed and either return zero or -errno. And of course >update durations_adjusted flag because that is needed in sysfs. > >/Jarkko Taking a quick look, they already track whether the adjustment occurred in the tpm_chip struct, so that could be used instead for what the bool return was being used for. I'll post a patch for the timeout updates code, and work with Alexey to rework his patchset. Regards, Jerry ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH REVIEW 2/2] tpm_tis: override durations for STM tpm with firmware 1.2.8.28 2019-01-20 21:30 ` Jerry Snitselaar @ 2019-01-21 12:29 ` Jarkko Sakkinen 0 siblings, 0 replies; 8+ messages in thread From: Jarkko Sakkinen @ 2019-01-21 12:29 UTC (permalink / raw) To: Jerry Snitselaar; +Cc: Alexey Klimov, linux-integrity, peterhuewe, jgg On Sun, Jan 20, 2019 at 02:30:12PM -0700, Jerry Snitselaar wrote: > On Fri Jan 18 19, Jarkko Sakkinen wrote: > > On Mon, Jan 14, 2019 at 12:39:40PM -0700, Jerry Snitselaar wrote: > > > Alexey was following the example of tpm_tis_update_timeouts() which > > > returns true if the timeouts were updated, and otherwise returns > > > false. The bool here makes sense to me, but what rc would you suggest > > > in this case? > > > > Maybe the pattern used there is not that great then. > > > > The callback should simply be update_durations(chip), and it would do > > whatever updates needed and either return zero or -errno. And of course > > update durations_adjusted flag because that is needed in sysfs. > > > > /Jarkko > > Taking a quick look, they already track whether the adjustment > occurred in the tpm_chip struct, so that could be used instead for > what the bool return was being used for. I'll post a patch for the > timeout updates code, and work with Alexey to rework his patchset. Awesome, thank you! /Jarkko ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH REVIEW 1/2] tpm: provide a way to override the chip returned durations 2018-12-14 13:21 [PATCH REVIEW 1/2] tpm: provide a way to override the chip returned durations Alexey Klimov 2018-12-14 13:21 ` [PATCH REVIEW 2/2] tpm_tis: override durations for STM tpm with firmware 1.2.8.28 Alexey Klimov @ 2019-01-03 13:05 ` Jarkko Sakkinen 1 sibling, 0 replies; 8+ messages in thread From: Jarkko Sakkinen @ 2019-01-03 13:05 UTC (permalink / raw) To: Alexey Klimov; +Cc: linux-integrity, jsnitsel, peterhuewe, jgg On Fri, Dec 14, 2018 at 01:21:14PM +0000, Alexey Klimov wrote: > Patch adds method ->update_durations to override returned > durations in case TPM chip misbehaves for TPM 1.2 drivers. > > Cc: Jerry Snitselaar <jsnitsel@redhat.com> > Signed-off-by: Alexey Klimov <aklimov@redhat.com> > --- > drivers/char/tpm/tpm-interface.c | 18 ++++++++++++++++++ > include/linux/tpm.h | 2 ++ > 2 files changed, 20 insertions(+) > > diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c > index 129f640424b7..5cb90918938d 100644 > --- a/drivers/char/tpm/tpm-interface.c > +++ b/drivers/char/tpm/tpm-interface.c > @@ -768,6 +768,7 @@ int tpm_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; > > if (chip->flags & TPM_CHIP_FLAG_HAVE_TIMEOUTS) > @@ -877,6 +878,23 @@ int tpm_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 ability for vendor overrides of duration values in case > + * of misreporting. > + */ > + if (chip->ops->update_durations != NULL) > + chip->duration_adjusted = > + chip->ops->update_durations(chip, durations); > + > + /* Report and set adjusted timeouts */ > + if (chip->duration_adjusted) { > + Can you remove the extra newline? > + dev_info(&chip->dev, HW_ERR "Adjusting reported durations\n"); > + 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 4609b94142d4..d6018431b478 100644 > --- a/include/linux/tpm.h > +++ b/include/linux/tpm.h > @@ -43,6 +43,8 @@ struct tpm_class_ops { > u8 (*status) (struct tpm_chip *chip); > bool (*update_timeouts)(struct tpm_chip *chip, > unsigned long *timeout_cap); > + bool (*update_durations)(struct tpm_chip *chip, > + unsigned long *durations_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.14.4 > /Jarkko ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-01-21 12:29 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-12-14 13:21 [PATCH REVIEW 1/2] tpm: provide a way to override the chip returned durations Alexey Klimov 2018-12-14 13:21 ` [PATCH REVIEW 2/2] tpm_tis: override durations for STM tpm with firmware 1.2.8.28 Alexey Klimov 2019-01-03 13:14 ` Jarkko Sakkinen 2019-01-14 19:39 ` Jerry Snitselaar 2019-01-18 14:59 ` Jarkko Sakkinen 2019-01-20 21:30 ` Jerry Snitselaar 2019-01-21 12:29 ` Jarkko Sakkinen 2019-01-03 13:05 ` [PATCH REVIEW 1/2] tpm: provide a way to override the chip returned durations Jarkko Sakkinen
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.