Re: [PATCH 1/1] tpm: disable hwrng for fTPM on some AMD designs
From: Mario Limonciello
Date: Mon Jul 31 2023 - 23:04:28 EST
On 7/31/23 18:40, Jason A. Donenfeld wrote:
Hi all,
I've been tracking this issue with Mario on various threads and
bugzilla for a while now. My suggestion over at bugzilla was to just
disable all current AMD fTPMs by bumping the check for a major version
number, so that the hardware people can reenable it i it's ever fixed,
but only if this is something that the hardware people would actually
respect. As I understand it, Mario was going to check into it and see.
Failing that, yea, just disabling hwrng on fTPM seems like a fine
enough thing to do.
The reason I'm not too concerned about that is twofold:
- Systems with fTPM all have RDRAND anyway, so there's no entropy problem.
- fTPM *probably* uses the same random source as RDRAND -- the
TRNG_OUT MMIO register -- so it's not really doing much more than what
we already have available.
Yeah I have conversations ongoing about this topic, but also I concluded
your suspicion is correct. They both get their values from the
integrated CCP HW IP.
So this all seems fine. And Jarkko's patch seems more or less the
straight forward way of disabling it. But with that said, in order of
priority, maybe we should first try these:
1) Adjust the version check to a major-place fTPM version that AMD's
hardware team pinky swears will have this bug fixed. (Though, I can
already imagine somebody on the list shouting, "we don't trust
hardware teams to do anything with unreleased stuff!", which could be
valid.)
I find it very likely the actual root cause is similar to what Linus
suggested. If that's the case I don't think the bug can be fixed
by just an fTPM fix but would rather require a BIOS fix.
This to me strengthens the argument to either not register fTPM as RNG
in the first place or just use TPM for boot time entropy.
2) Remove the version check, but add some other query to detect AMD
fTPM vs realTPM, and ban fTPM.
AMD doesn't make dTPMs, only fTPMs. It's tempting to try to use
TPM2_PT_VENDOR_TPM_TYPE, but this actually is a vendor specific value.
I don't see a reliable way in the spec to do this.
- Remove the version check, and just check for AMD; this is Jarrko's patch.
I have a counter-proposal to Jarkko's patch attached. This has two
notable changes:
1) It only disables RNG generation in the case of having RDRAND or RDSEED.
2) It also matches Intel PTT.
I still do also think Linus' idea of TPMs only providing boot time
entropy is worth weighing out.
From 9f41d3037e37d91d5aaf14208ba43ba76a76031f Mon Sep 17 00:00:00 2001
From: Mario Limonciello <mario.limonciello@xxxxxxx>
Date: Mon, 31 Jul 2023 21:30:13 -0500
Subject: [PATCH] tpm: Disable RNG for fTPMs on SOCs that support RDRAND/RDSEED
The TPM RNG functionality is not necessary for entropy when the CPU
already supports dedicated instructions for RNG.
Furthermore it continues to show problems on some systems causing
stutter.
Cc: stable@xxxxxxxxxxxxxxx # 6.1.y+
Fixes: b006c439d58d ("hwrng: core - start hwrng kthread also for untrusted sources")
Reported-by: daniil.stas@xxxxxxxxxx
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217719
Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx>
---
drivers/char/tpm/tpm-chip.c | 53 ++++++++-----------------------------
1 file changed, 11 insertions(+), 42 deletions(-)
diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index cf5499e51999b..ac39ed8c9704a 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -510,19 +510,10 @@ 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)
+static bool tpm_skip_rng(struct tpm_chip *chip)
{
- u32 val1, val2;
- u64 version;
+ u32 val1;
int ret;
if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
@@ -531,44 +522,22 @@ static bool tpm_amd_is_rng_defective(struct tpm_chip *chip)
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 {
+ /* if CPU supports RDRAND/RDSEED ignore fTPM for RNG */
+ switch (val1) {
+ case 0x414D4400U: /* AMD */
+ case 0x494E5443U: /* INTC */
+ return boot_cpu_has(X86_FEATURE_RDRAND) || boot_cpu_has(X86_FEATURE_RDSEED);
+ default:
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)
+static inline bool tpm_skip_rng(struct tpm_chip *chip)
{
return false;
}
@@ -588,7 +557,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))
+ tpm_skip_rng(chip))
return 0;
snprintf(chip->hwrng_name, sizeof(chip->hwrng_name),
@@ -719,7 +688,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))
+ !tpm_skip_rng(chip))
hwrng_unregister(&chip->hwrng);
tpm_bios_log_teardown(chip);
if (chip->flags & TPM_CHIP_FLAG_TPM2 && !tpm_is_firmware_upgrade(chip))
--
2.34.1