Jarkko Sakkinen <jarkko@xxxxxxxxxx> writes: > On Thu, 2021-01-28 at 14:07 +0100, Lukasz Majczak wrote: >> There is a missing call to tpm_request_locality before the call to >> the tpm_get_timeouts() and tpm_tis_probe_irq_single(). As the current >> approach might work for tpm2, it fails for tpm1.x - in that case >> call to tpm_get_timeouts() or tpm_tis_probe_irq_single() >> without locality fails and in turn causes tpm_tis_core_init() to fail. >> Tested on Samsung Chromebook Pro (Caroline). >> >> Signed-off-by: Lukasz Majczak <lma@xxxxxxxxxxxx> > > Is it possible that you test against linux-next and see if any > problems still arise? I've applied the locality fixes from James. I tested current linux-next and the warning still appears, unfortunately. I then incrementally applied further patches from James' series [1] and after "[PATCH v2 4/5] tpm_tis: fix IRQ probing" the warning has gone: # dmesg | grep tpm [ 7.220410] tpm_tis STM0125:00: 2.0 TPM (device-id 0x0, rev-id 78) [ 7.322564] Modules linked in: iwlmvm(+) btusb btrtl btbcm btintel mac80211 amdgpu(+) iwlwifi drm_ttm_helper tpm_crb sdhci_pci ttm cqhci gpu_sched sdhci ccp cfg80211 rng_core tpm_tis tpm_tis_core tpm thinkpad_acpi(+) wmi nvram pinctrl_amd You might notice there is another warning but that is rtc related and I still have to find out if that is something I should report. Dirk [1] https://lore.kernel.org/linux-integrity/20201001180925.13808-1-James.Bottomley@xxxxxxxxxxxxxxxxxxxxx/ >> --- >> Jarkko, James, Guenter >> >> I’m aware about the other thread, but it seems to be dead for a few months. >> Here is the small patch as fixing this specific issue >> would allow us to unblock the ChromeOs development. >> We want to upstream all of our patches, >> so the ChromeOs will not diverge even more, >> so I'm hoping this could be applied, if you see it neat enough. > > The usual approach is that you construct a series picking the pre-existing > fixes and on top of that create your own, if any required. > >> Best regards, >> Lukasz > > /Jarkko > >> >> v1 -> v2: >> - fixed typos >> - as there is no need to enable clock, switched to >> use only tpm_request/relinquish_locality calls >> - narrowed down boundaries of tpm_request/relinquish_locality calls >> >> drivers/char/tpm/tpm-chip.c | 4 ++-- >> drivers/char/tpm/tpm-interface.c | 11 +++++++++-- >> drivers/char/tpm/tpm.h | 2 ++ >> drivers/char/tpm/tpm_tis_core.c | 12 ++++++++++-- >> 4 files changed, 23 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c >> index ddaeceb7e109..5351963a4b19 100644 >> --- a/drivers/char/tpm/tpm-chip.c >> +++ b/drivers/char/tpm/tpm-chip.c >> @@ -32,7 +32,7 @@ struct class *tpm_class; >> struct class *tpmrm_class; >> dev_t tpm_devt; >> >> -static int tpm_request_locality(struct tpm_chip *chip) >> +int tpm_request_locality(struct tpm_chip *chip) >> { >> int rc; >> >> @@ -47,7 +47,7 @@ static int tpm_request_locality(struct tpm_chip *chip) >> return 0; >> } >> >> -static void tpm_relinquish_locality(struct tpm_chip *chip) >> +void tpm_relinquish_locality(struct tpm_chip *chip) >> { >> int rc; >> >> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c >> index 1621ce818705..69309b2bea6a 100644 >> --- a/drivers/char/tpm/tpm-interface.c >> +++ b/drivers/char/tpm/tpm-interface.c >> @@ -243,8 +243,15 @@ int tpm_get_timeouts(struct tpm_chip *chip) >> >> if (chip->flags & TPM_CHIP_FLAG_TPM2) >> return tpm2_get_timeouts(chip); >> - else >> - return tpm1_get_timeouts(chip); >> + else { >> + ssize_t ret = tpm_request_locality(chip); >> + >> + if (ret) >> + return ret; >> + ret = tpm1_get_timeouts(chip); >> + tpm_relinquish_locality(chip); >> + return ret; >> + } >> } >> EXPORT_SYMBOL_GPL(tpm_get_timeouts); >> >> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h >> index 947d1db0a5cc..8c13008437dd 100644 >> --- a/drivers/char/tpm/tpm.h >> +++ b/drivers/char/tpm/tpm.h >> @@ -193,6 +193,8 @@ static inline void tpm_msleep(unsigned int delay_msec) >> >> int tpm_chip_start(struct tpm_chip *chip); >> void tpm_chip_stop(struct tpm_chip *chip); >> +int tpm_request_locality(struct tpm_chip *chip); >> +void tpm_relinquish_locality(struct tpm_chip *chip); >> struct tpm_chip *tpm_find_get_ops(struct tpm_chip *chip); >> __must_check int tpm_try_get_ops(struct tpm_chip *chip); >> void tpm_put_ops(struct tpm_chip *chip); >> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c >> index 92c51c6cfd1b..0ae675e8cf2f 100644 >> --- a/drivers/char/tpm/tpm_tis_core.c >> +++ b/drivers/char/tpm/tpm_tis_core.c >> @@ -754,9 +754,17 @@ static int tpm_tis_gen_interrupt(struct tpm_chip *chip) >> >> if (chip->flags & TPM_CHIP_FLAG_TPM2) >> return tpm2_get_tpm_pt(chip, 0x100, &cap2, desc); >> - else >> - return tpm1_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, &cap, desc, >> + else { >> + ssize_t ret = tpm_request_locality(chip); >> + >> + if (ret) >> + return ret; >> + ret = tpm1_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, &cap, desc, >> 0); >> + tpm_relinquish_locality(chip); >> + return ret; >> + } >> + >> } >> >> /* Register the IRQ and issue a command that will cause an interrupt. If an
