On Wed Aug 2, 2023 at 3:25 PM EEST, Mario Limonciello wrote: > The TPM RNG functionality is not necessary for entropy when the CPU > already supports the RDRAND instruction. The TPM RNG functionality > was previously disabled on a subset of AMD fTPM series, but reports > continue to show problems on some systems causing stutter root caused > to TPM RNG functionality. > > Expand disabling TPM RNG use for all AMD fTPMs whether they have versions > that claim to have fixed or not. To accomplish this, move the detection > into part of the TPM CRB registration and add a flag indicating that > the TPM should opt-out of registration to hwrng. > > Cc: stable@xxxxxxxxxxxxxxx # 6.1.y+ > Fixes: b006c439d58d ("hwrng: core - start hwrng kthread also for untrusted sources") > Fixes: f1324bbc4011 ("tpm: disable hwrng for fTPM on some AMD designs") > Reported-by: daniil.stas@xxxxxxxxxx > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217719 > Reported-by: bitlord0xff@xxxxxxxxx > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217212 > Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx> > --- > v1->v2: > * switch from callback to everything in tpm_crb > * switch to open coded flags check instead of new inline > --- > drivers/char/tpm/tpm-chip.c | 68 ++----------------------------------- > drivers/char/tpm/tpm_crb.c | 30 ++++++++++++++++ > include/linux/tpm.h | 1 + > 3 files changed, 33 insertions(+), 66 deletions(-) > > diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c > index cf5499e51999b..e904aae9771be 100644 > --- a/drivers/char/tpm/tpm-chip.c > +++ b/drivers/char/tpm/tpm-chip.c > @@ -510,70 +510,6 @@ static int tpm_add_legacy_sysfs(struct tpm_chip *chip) > return 0; > } > > -/* > - * Some AMD fTPM versions may cause stutter > - * https://www.amd.com/en/support/kb/faq/pa-410 > - * > - * Fixes are available in two series of fTPM firmware: > - * 6.x.y.z series: 6.0.18.6 + > - * 3.x.y.z series: 3.57.y.5 + > - */ > -#ifdef CONFIG_X86 > -static bool tpm_amd_is_rng_defective(struct tpm_chip *chip) > -{ > - u32 val1, val2; > - u64 version; > - int ret; > - > - if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) > - return false; > - > - ret = tpm_request_locality(chip); > - if (ret) > - return false; > - > - ret = tpm2_get_tpm_pt(chip, TPM2_PT_MANUFACTURER, &val1, NULL); > - if (ret) > - goto release; > - if (val1 != 0x414D4400U /* AMD */) { > - ret = -ENODEV; > - goto release; > - } > - ret = tpm2_get_tpm_pt(chip, TPM2_PT_FIRMWARE_VERSION_1, &val1, NULL); > - if (ret) > - goto release; > - ret = tpm2_get_tpm_pt(chip, TPM2_PT_FIRMWARE_VERSION_2, &val2, NULL); > - > -release: > - tpm_relinquish_locality(chip); > - > - if (ret) > - return false; > - > - version = ((u64)val1 << 32) | val2; > - if ((version >> 48) == 6) { > - if (version >= 0x0006000000180006ULL) > - return false; > - } else if ((version >> 48) == 3) { > - if (version >= 0x0003005700000005ULL) > - return false; > - } else { > - return false; > - } > - > - dev_warn(&chip->dev, > - "AMD fTPM version 0x%llx causes system stutter; hwrng disabled\n", > - version); > - > - return true; > -} > -#else > -static inline bool tpm_amd_is_rng_defective(struct tpm_chip *chip) > -{ > - return false; > -} > -#endif /* CONFIG_X86 */ > - > static int tpm_hwrng_read(struct hwrng *rng, void *data, size_t max, bool wait) > { > struct tpm_chip *chip = container_of(rng, struct tpm_chip, hwrng); > @@ -588,7 +524,7 @@ static int tpm_hwrng_read(struct hwrng *rng, void *data, size_t max, bool wait) > static int tpm_add_hwrng(struct tpm_chip *chip) > { > if (!IS_ENABLED(CONFIG_HW_RANDOM_TPM) || tpm_is_firmware_upgrade(chip) || > - tpm_amd_is_rng_defective(chip)) > + chip->flags & TPM_CHIP_FLAG_HWRNG_DISABLED) > return 0; > > snprintf(chip->hwrng_name, sizeof(chip->hwrng_name), > @@ -719,7 +655,7 @@ void tpm_chip_unregister(struct tpm_chip *chip) > { > tpm_del_legacy_sysfs(chip); > if (IS_ENABLED(CONFIG_HW_RANDOM_TPM) && !tpm_is_firmware_upgrade(chip) && > - !tpm_amd_is_rng_defective(chip)) > + !(chip->flags & TPM_CHIP_FLAG_HWRNG_DISABLED)) > hwrng_unregister(&chip->hwrng); > tpm_bios_log_teardown(chip); > if (chip->flags & TPM_CHIP_FLAG_TPM2 && !tpm_is_firmware_upgrade(chip)) > diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c > index 1a5d09b185134..9eb1a18590123 100644 > --- a/drivers/char/tpm/tpm_crb.c > +++ b/drivers/char/tpm/tpm_crb.c > @@ -463,6 +463,28 @@ static bool crb_req_canceled(struct tpm_chip *chip, u8 status) > return (cancel & CRB_CANCEL_INVOKE) == CRB_CANCEL_INVOKE; > } > > +static int crb_check_flags(struct tpm_chip *chip) > +{ > + u32 val; > + int ret; > + > + ret = crb_request_locality(chip, 0); > + if (ret) > + return ret; > + > + ret = tpm2_get_tpm_pt(chip, TPM2_PT_MANUFACTURER, &val, NULL); > + if (ret) > + goto release; > + > + if (val == 0x414D4400U /* AMD */) > + chip->flags |= TPM_CHIP_FLAG_HWRNG_DISABLED; > + > +release: > + crb_relinquish_locality(chip, 0); > + > + return ret; > +} > + > static const struct tpm_class_ops tpm_crb = { > .flags = TPM_OPS_AUTO_STARTUP, > .status = crb_status, > @@ -800,6 +822,14 @@ static int crb_acpi_add(struct acpi_device *device) > chip->acpi_dev_handle = device->handle; > chip->flags = TPM_CHIP_FLAG_TPM2; > > + rc = tpm_chip_bootstrap(chip); > + if (rc) > + goto out; > + > + rc = crb_check_flags(chip); > + if (rc) > + goto out; > + > rc = tpm_chip_register(chip); > > out: > diff --git a/include/linux/tpm.h b/include/linux/tpm.h > index 6a1e8f1572551..4ee9d13749adc 100644 > --- a/include/linux/tpm.h > +++ b/include/linux/tpm.h > @@ -283,6 +283,7 @@ enum tpm_chip_flags { > TPM_CHIP_FLAG_FIRMWARE_POWER_MANAGED = BIT(6), > TPM_CHIP_FLAG_FIRMWARE_UPGRADE = BIT(7), > TPM_CHIP_FLAG_SUSPENDED = BIT(8), > + TPM_CHIP_FLAG_HWRNG_DISABLED = BIT(9), > }; > > #define to_tpm_chip(d) container_of(d, struct tpm_chip, dev) > -- > 2.34.1 Reviewed-by: Jarkko Sakkinen <jarkko@xxxxxxxxxx> Thank you, great work. I pushed the patch to my next branch: https://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git/log/?h=next I'll hold on for tested-by's from AMD users, and send a pull request tomorrow afternoon (GMT+3). BR, Jarkko