* [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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).