tpm: Enable hwrng only for Pluton on AMD CPUs
Subsystems (5)
Jarkko Sakkinen
maintainerPATCH v3 1/1 tpm: Enable hwrng only for Pluton on AMD CPUs
The vendor check introduced by commit 554b841d4703 ("tpm: Disable RNG for all AMD fTPMs") doesn't work properly on a number of Intel fTPMs. On the reported systems the TPM doesn't reply at bootup and returns back the command code. This makes the TPM fail probe.
Since only Microsoft Pluton is the only known combination of AMD CPU and fTPM from other vendor, disable hwrng otherwise. In order to make sysadmin aware of this, print also info message to the klog.
---
v3:
* Forgot to amend config flags.
v2:
* CONFIG_X86
* Removed "Reviewed-by: Jarkko Sakkinen <[email protected]>"
* Removed "Signed-off-by: Mario Limonciello <[email protected]>"
--- drivers/char/tpm/tpm_crb.c | 33 ++++++++------------------------- 1 file changed, 8 insertions(+), 25 deletions(-)
drivers/char/tpm/tpm_crb.c +8 -25
@@ -463,28 +463,6 @@ 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,
@@ -827,9 +805,14 @@ static int crb_acpi_add(struct acpi_device *device)if (rc) goto out; - rc =crb_check_flags (chip); -if (rc) - goto out; +#ifdef CONFIG_X86 +/* A quirk for https://www.amd.com/en/support/kb/faq/pa-410 */ +if (boot_cpu_data.x86_vendor ==X86_VENDOR_AMD && + priv->sm !=ACPI_TPM2_COMMAND_BUFFER_WITH_PLUTON ) { +dev_info (dev,"Disabling hwrng\n" ); + chip->flags |=TPM_CHIP_FLAG_HWRNG_DISABLED ; + } +#endif /* CONFIG_X86 */ rc =tpm_chip_register (chip);
Signature · 1 line
2.39.2
Dear Jarkko,
Thank you for your patch.
Am 23.08.23 um 01:15 schrieb Jarkko Sakkinen:
Quoted 9 lines
The vendor check introduced by commit
554b841d4703("tpm: Disable RNG for all AMD fTPMs") doesn't work properly on a number of Intel fTPMs. On the reported systems the TPM doesn't reply at bootup and returns back the command code. This makes the TPM fail probe.Since only Microsoft Pluton is the only known combination of AMD CPU and fTPM from other vendor, disable hwrng otherwise. In order to make sysadmin aware of this, print also info message to the klog.
Quoted 5 lines
Fixes 554b841d4703 ("tpm: Disable RNG for all AMD fTPMs")Reported-by Todd Brandt <[email protected]>Signed-off-by Jarkko Sakkinen <[email protected]>
Mario’s patch also had the three reporters below listed:
Quoted 46 lines
---
v3:
* Forgot to amend config flags.
v2:
*CONFIG_X86
* Removed "Reviewed-by: Jarkko Sakkinen <[email protected]>"
* Removed "Signed-off-by: Mario Limonciello <[email protected]>"--- drivers/char/tpm/tpm_crb.c | 33 ++++++++------------------------- 1 file changed, 8 insertions(+), 25 deletions(-)drivers/char/tpm/tpm_crb.c +0
-22@@ -463,28 +463,6 @@ 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,@@ -827,9 +805,14 @@ static int crb_acpi_add(struct acpi_device *device)if (rc)
drivers/char/tpm/tpm_crb.c static int crb_acpi_add(struct acpi_device *device) · line 812
Quoted 10 lines
drivers/char/tpm/tpm_crb.c +5
-3goto out; - rc =crb_check_flags (chip); -if (rc) - goto out; +#ifdef CONFIG_X86 +/* A quirk for https://www.amd.com/en/support/kb/faq/pa-410 */ +if (boot_cpu_data.x86_vendor ==X86_VENDOR_AMD && + priv->sm !=ACPI_TPM2_COMMAND_BUFFER_WITH_PLUTON ) { +dev_info (dev,"Disabling hwrng\n" );
A more elaborate log message would be helpful for the user. Maybe:
Disabling hwrng in AMD's fTPM to avoid stutter (AMD article PA 410)
drivers/char/tpm/tpm_crb.c static int crb_acpi_add(struct acpi_device *device) · line 818
Quoted 6 lines
drivers/char/tpm/tpm_crb.c +3
-0+ chip->flags |= TPM_CHIP_FLAG_HWRNG_DISABLED; + } +#endif /* CONFIG_X86 */ rc =tpm_chip_register (chip);
Kind regards,
Paul
Jarkko Sakkinen
maintainerQuoted 22 lines · Paul Menzel
Dear Jarkko,
Thank you for your patch.
Am 23.08.23 um 01:15 schrieb Jarkko Sakkinen:
Quoted 9 lines
The vendor check introduced by commit
554b841d4703("tpm: Disable RNG for all AMD fTPMs") doesn't work properly on a number of Intel fTPMs. On the reported systems the TPM doesn't reply at bootup and returns back the command code. This makes the TPM fail probe.Since only Microsoft Pluton is the only known combination of AMD CPU and fTPM from other vendor, disable hwrng otherwise. In order to make sysadmin aware of this, print also info message to the klog.
Quoted 5 lines
Fixes 554b841d4703 ("tpm: Disable RNG for all AMD fTPMs")Reported-by Todd Brandt <[email protected]>Signed-off-by Jarkko Sakkinen <[email protected]>
Quoted 5 lines
Mario’s patch also had the three reporters below listed:
Reported-by Patrick Steinhardt <[email protected]>Reported-by Ronan Pigott <[email protected]>Reported-by Raymond Jay Golo <[email protected]>
The problem here is that checkpatch throws three warnings:
WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report #19:
WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report #20:
WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report #21:
Since bugzilla is not part of the documented process afaik, I used this field as the guideline:
Reported: 2023-08-17 20:59 UTC by Todd Brandt
How otherwise I should interpret kernel bugzilla?
In any case new version is still needed as the commit message must contain a mention of "Lenovo Legion Y540" as the stimulus for doing this code change in the first place.
BR, Jarkko
Quoted 38 lines · Jarkko Sakkinen
Quoted 22 lines · Paul Menzel
Dear Jarkko,
Thank you for your patch.
Am 23.08.23 um 01:15 schrieb Jarkko Sakkinen:
Quoted 9 lines
The vendor check introduced by commit
554b841d4703("tpm: Disable RNG for all AMD fTPMs") doesn't work properly on a number of Intel fTPMs. On the reported systems the TPM doesn't reply at bootup and returns back the command code. This makes the TPM fail probe.Since only Microsoft Pluton is the only known combination of AMD CPU and fTPM from other vendor, disable hwrng otherwise. In order to make sysadmin aware of this, print also info message to the klog.
Quoted 5 lines
Fixes 554b841d4703 ("tpm: Disable RNG for all AMD fTPMs")Reported-by Todd Brandt <[email protected]>Signed-off-by Jarkko Sakkinen <[email protected]>Quoted 5 lines
Mario’s patch also had the three reporters below listed:
Reported-by Patrick Steinhardt <[email protected]>Reported-by Ronan Pigott <[email protected]>Reported-by Raymond Jay Golo <[email protected]>The problem here is that checkpatch throws three warnings:
WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report #19:
Reported-by Patrick Steinhardt <[email protected]>Reported-by Ronan Pigott <[email protected]>WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report #20:
Quoted 8 lines
Reported-by Ronan Pigott <[email protected]>Reported-by Raymond Jay Golo <[email protected]>WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report #21:
Reported-by Raymond Jay Golo <[email protected]>Signed-off-by Jarkko Sakkinen <[email protected]>
FWIW I observed the same checkpatch warning when I submitted my version of the patch. I figured it's better to ignore the warning and attribute everyone who reported the issue affected them.
If nothing else it gives more people to pull in and check any future fixes if there is a regression caused by this patch that forces it to be reverted.
Quoted 12 lines
Since bugzilla is not part of the documented process afaik, I used this field as the guideline:
Reported: 2023-08-17 20:59 UTC by Todd Brandt
How otherwise I should interpret kernel bugzilla?
In any case new version is still needed as the commit message must contain a mention of "Lenovo Legion Y540" as the stimulus for doing this code change in the first place.
BR, Jarkko
Jarkko Sakkinen
maintainerQuoted 48 lines · Mario Limonciello
Quoted 38 lines · Jarkko Sakkinen
Quoted 22 lines · Paul Menzel
Dear Jarkko,
Thank you for your patch.
Am 23.08.23 um 01:15 schrieb Jarkko Sakkinen:
Quoted 9 lines
The vendor check introduced by commit
554b841d4703("tpm: Disable RNG for all AMD fTPMs") doesn't work properly on a number of Intel fTPMs. On the reported systems the TPM doesn't reply at bootup and returns back the command code. This makes the TPM fail probe.Since only Microsoft Pluton is the only known combination of AMD CPU and fTPM from other vendor, disable hwrng otherwise. In order to make sysadmin aware of this, print also info message to the klog.
Quoted 5 lines
Fixes 554b841d4703 ("tpm: Disable RNG for all AMD fTPMs")Reported-by Todd Brandt <[email protected]>Signed-off-by Jarkko Sakkinen <[email protected]>Quoted 5 lines
Mario’s patch also had the three reporters below listed:
Reported-by Patrick Steinhardt <[email protected]>Reported-by Ronan Pigott <[email protected]>Reported-by Raymond Jay Golo <[email protected]>The problem here is that checkpatch throws three warnings:
WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report #19:
Reported-by Patrick Steinhardt <[email protected]>Reported-by Ronan Pigott <[email protected]>WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report #20:
Quoted 8 lines
Reported-by Ronan Pigott <[email protected]>Reported-by Raymond Jay Golo <[email protected]>WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report #21:
Reported-by Raymond Jay Golo <[email protected]>Signed-off-by Jarkko Sakkinen <[email protected]>
Quoted 3 lines
FWIW I observed the same checkpatch warning when I submitted my version of the patch. I figured it's better to ignore the warning and attribute everyone who reported the issue affected them.
OK so:
1. checkpatch.pl is part of the kernel process.
2. Bugzilla is not part of the kernel process.
Why emphasis on 1?
BR, Jarkko
Quoted 53 lines · Jarkko Sakkinen
Quoted 48 lines · Mario Limonciello
Quoted 38 lines · Jarkko Sakkinen
Quoted 22 lines · Paul Menzel
Dear Jarkko,
Thank you for your patch.
Am 23.08.23 um 01:15 schrieb Jarkko Sakkinen:
Quoted 9 lines
The vendor check introduced by commit
554b841d4703("tpm: Disable RNG for all AMD fTPMs") doesn't work properly on a number of Intel fTPMs. On the reported systems the TPM doesn't reply at bootup and returns back the command code. This makes the TPM fail probe.Since only Microsoft Pluton is the only known combination of AMD CPU and fTPM from other vendor, disable hwrng otherwise. In order to make sysadmin aware of this, print also info message to the klog.
Quoted 5 lines
Fixes 554b841d4703 ("tpm: Disable RNG for all AMD fTPMs")Reported-by Todd Brandt <[email protected]>Signed-off-by Jarkko Sakkinen <[email protected]>Quoted 5 lines
Mario’s patch also had the three reporters below listed:
Reported-by Patrick Steinhardt <[email protected]>Reported-by Ronan Pigott <[email protected]>Reported-by Raymond Jay Golo <[email protected]>The problem here is that checkpatch throws three warnings:
WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report #19:
Reported-by Patrick Steinhardt <[email protected]>Reported-by Ronan Pigott <[email protected]>WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report #20:
Quoted 8 lines
Reported-by Ronan Pigott <[email protected]>Reported-by Raymond Jay Golo <[email protected]>WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report #21:
Reported-by Raymond Jay Golo <[email protected]>Signed-off-by Jarkko Sakkinen <[email protected]>Quoted 3 lines
FWIW I observed the same checkpatch warning when I submitted my version of the patch. I figured it's better to ignore the warning and attribute everyone who reported the issue affected them.
Quoted 8 lines
OK so:
1. checkpatch.pl is part of the kernel process.
2. Bugzilla is not part of the kernel process.Why emphasis on 1?
BR, Jarkko
The reason I submitted it this way is because of this quote from the documentation [1].
"Check your patches with the patch style checker prior to submission (scripts/checkpatch.pl). Note, though, that the style checker should be viewed as a guide, not as a replacement for human judgment. If your code looks better with a violation then its probably best left alone."
I wanted the patch to capture and attribute all those that reported it not just the "first one". Like I said previously, it's better to have a collection of people to ping to notify if something needs to be reverted.
[1] https://www.kernel.org/doc/html/latest/process/submitting-patches.html#style-check-your-changes
[CCing the regression list, as it should be in the loop for regressions:
https://docs.kernel.org/admin-guide/reporting-regressions.html]
Quoted 26 lines · Mario Limonciello
Quoted 25 lines · Jarkko Sakkinen
Quoted 24 lines · Mario Limonciello
Quoted 23 lines · Jarkko Sakkinen
Quoted 22 lines · Paul Menzel
Am 23.08.23 um 01:15 schrieb Jarkko Sakkinen:
Quoted 13 lines
The vendor check introduced by commit
554b841d4703("tpm: Disable RNG for
all AMD fTPMs") doesn't work properly on a number of Intel fTPMs. On the
reported systems the TPM doesn't reply at bootup and returns back the command code. This makes the TPM fail probe.Since only Microsoft Pluton is the only known combination of AMD CPU and
fTPM from other vendor, disable hwrng otherwise. In order to make sysadmin
aware of this, print also info message to the klog.Quoted 5 lines
Fixes 554b841d4703 ("tpm: Disable RNG for all AMD fTPMs")Reported-by Todd Brandt <[email protected]>Signed-off-by Jarkko Sakkinen <[email protected]>Mario’s patch also had the three reporters below listed:
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 3 lines
Reported-by Patrick Steinhardt <[email protected]>Reported-by Ronan Pigott <[email protected]>Reported-by Raymond Jay Golo <[email protected]>The problem here is that checkpatch throws three warnings:
WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report
Note, those are warnings (and not an errors) on purpose and the text says "should" (and not "must"), so this is IMHO something in this case can be ignored, as Mario indicated in his reply.
But I write for a different reason: this seems to become a regression that is annoying quite a few people (in 6.5 and 6.4.y afaics), so it would be good to get the fix merged to mainline rather sooner than later. Are these warnings and the mentioning of affected machines in the patch descriptions the only remaining problems, or is there anything else that needs to be addressed?
Ciao, Thorsten (just back from vacation and catching up) --
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.
#regzbot poke
[CCing Linus, as this triggered my "this is moving to slowly" threshold, as (a) the initial report was two weeks ago by now (b) a fix seems within reach for nearly as long (c) the problem seems to annoy quite a few people, as the culprit of this regression made it into 6.5 and was picked up for 6.1.y and 6.4.y (rightfully so I'd say, as it fixes an earlier regression)]
Quoted 31 lines · Linux regression tracking (Thorsten Leemhuis)
Quoted 24 lines · Mario Limonciello
Quoted 23 lines · Jarkko Sakkinen
Quoted 22 lines · Mario Limonciello
Quoted 21 lines · Jarkko Sakkinen
Quoted 20 lines · Paul Menzel
Am 23.08.23 um 01:15 schrieb Jarkko Sakkinen:
Quoted 18 lines
The vendor check introduced by commit
554b841d4703("tpm: Disable RNG for
all AMD fTPMs") doesn't work properly on a number of Intel fTPMs. On the
reported systems the TPM doesn't reply at bootup and returns back the command code. This makes the TPM fail probe.Since only Microsoft Pluton is the only known combination of AMD CPU and
fTPM from other vendor, disable hwrng otherwise. In order to make sysadmin
aware of this, print also info message to the klog.Fixes 554b841d4703 ("tpm: Disable RNG for all AMD fTPMs")Reported-by Todd Brandt <[email protected]>Signed-off-by Jarkko Sakkinen <[email protected]>Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Mario’s patch also had the three reporters below listed:
Reported-by Patrick Steinhardt <[email protected]>Reported-by Ronan Pigott <[email protected]>Reported-by Raymond Jay Golo <[email protected]>
Quoted 6 lines
[...] this seems to become a regression
that is annoying quite a few people (in 6.5 and 6.4.y afaics), so it would be good to get the fix merged to mainline rather sooner than later. Are these warnings and the mentioning of affected machines in the patch descriptions the only remaining problems, or is there anything else that needs to be addressed?
Hmmm. Quite a bit progress to fix the issue was made in the first week after Todd's report; Jarkko apparently even applied the earlier patch from Mario to his master branch:
https://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git/commit/?id=b1a62d41bdc1d15b0641759717e8c3651f0a810c
But since then (aka in the past week) there was not much progress.
Wondering what's up here -- and if both patches are needed or just one of them (I suspect it's the latter).
Checked lore and noticed that Jarkko was not much active in kernel land during the past few days; happens, *no worries at all*. But still would be good if this could be resolved rather sooner that later. Just not sure how to achieve that.
Mario, could you maybe pick this up in case Jarkko doesn't show up soon soon? From an earlier message in the thread it sounded like all that was missing was a slightly improved patch description? Or am I missing something?
Ciao, Thorsten (who feels bad that he's putting pressure on people; sorry for that, but that duty comes with the "regression tracker" hat) --
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.
#regzbot poke
Jarkko Sakkinen
maintainerQuoted 65 lines · Thorsten Leemhuis
[CCing Linus, as this triggered my "this is moving to slowly" threshold, as (a) the initial report was two weeks ago by now (b) a fix seems within reach for nearly as long (c) the problem seems to annoy quite a few people, as the culprit of this regression made it into 6.5 and was picked up for 6.1.y and 6.4.y (rightfully so I'd say, as it fixes an earlier regression)]
Quoted 31 lines · Linux regression tracking (Thorsten Leemhuis)
Quoted 24 lines · Mario Limonciello
Quoted 23 lines · Jarkko Sakkinen
Quoted 22 lines · Mario Limonciello
Quoted 21 lines · Jarkko Sakkinen
Quoted 20 lines · Paul Menzel
Am 23.08.23 um 01:15 schrieb Jarkko Sakkinen:
Quoted 18 lines
The vendor check introduced by commit
554b841d4703("tpm: Disable RNG for
all AMD fTPMs") doesn't work properly on a number of Intel fTPMs. On the
reported systems the TPM doesn't reply at bootup and returns back the command code. This makes the TPM fail probe.Since only Microsoft Pluton is the only known combination of AMD CPU and
fTPM from other vendor, disable hwrng otherwise. In order to make sysadmin
aware of this, print also info message to the klog.Fixes 554b841d4703 ("tpm: Disable RNG for all AMD fTPMs")Reported-by Todd Brandt <[email protected]>Signed-off-by Jarkko Sakkinen <[email protected]>Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Mario’s patch also had the three reporters below listed:
Reported-by Patrick Steinhardt <[email protected]>Reported-by Ronan Pigott <[email protected]>Reported-by Raymond Jay Golo <[email protected]>Quoted 6 lines
[...] this seems to become a regression
that is annoying quite a few people (in 6.5 and 6.4.y afaics), so it would be good to get the fix merged to mainline rather sooner than later. Are these warnings and the mentioning of affected machines in the patch descriptions the only remaining problems, or is there anything else that needs to be addressed?Hmmm. Quite a bit progress to fix the issue was made in the first week after Todd's report; Jarkko apparently even applied the earlier patch from Mario to his master branch:
https://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git/commit/?id=b1a62d41bdc1d15b0641759717e8c3651f0a810c
But since then (aka in the past week) there was not much progress.Wondering what's up here -- and if both patches are needed or just one of them (I suspect it's the latter).
Checked lore and noticed that Jarkko was not much active in kernel land during the past few days; happens, *no worries at all*. But still would be good if this could be resolved rather sooner that later. Just not sure how to achieve that.
Mario, could you maybe pick this up in case Jarkko doesn't show up soon soon? From an earlier message in the thread it sounded like all that was missing was a slightly improved patch description? Or am I missing something?
Quoted 8 lines
Ciao, Thorsten (who feels bad that he's putting pressure on people; sorry for that, but that duty comes with the "regression tracker" hat) --
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.#regzbot poke
Could it be possible to extend the actual kernel documentation to give at least some guidelines how a maintainer should deal with the bugzilla?
I do not oppose having it but IMHO at least the basics should be in the actualy process documentation. You can even put a link to this URL to that.
I posted a PR today with the hopefully final fix:
https://lore.kernel.org/linux-integrity/[email protected]/T/#u
BR, Jarkko
Quoted 14 lines · Jarkko Sakkinen
Quoted 5 lines · Thorsten Leemhuis
Quoted 4 lines · Linux regression tracking (Thorsten Leemhuis)
Quoted 3 lines · Mario Limonciello
Quoted 2 lines · Jarkko Sakkinen
Quoted 1 line · Mario Limonciello
On 8/23/2023 12:40, Jarkko Sakkinen wrote:
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines · Paul Menzel
Quoted 7 lines
Am 23.08.23 um 01:15 schrieb Jarkko Sakkinen:
Quoted 6 lines
The vendor check introduced by commit
554b841d4703("tpm: Disable RNG for
all AMD fTPMs") doesn't work properly on a number of Intel fTPMs. On the
reported systems the TPM doesn't reply at bootup and returns back the command code. This makes the TPM fail probe.
Quoted 6 lines
[...]
Quoted 5 lines
Hmmm. Quite a bit progress to fix the issue was made in the first week after Todd's report; Jarkko apparently even applied the earlier patch from Mario to his master branch:
https://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git/commit/?id=b1a62d41bdc1d15b0641759717e8c3651f0a810c
But since then (aka in the past week) there was not much progress.
Jarkko, many thx for picking this up and submitting it to Linus, much appreciated. Sorry again for prodding things, but I felt I had to. Hope you didn't mind too much.
Quoted 3 lines
Could it be possible to extend the actual kernel documentation to give at least some guidelines how a maintainer should deal with the bugzilla?
I guess it's best if that is done by somebody that cares about bugzilla (I don't fall into that group[1]) and knows the official status.
But FWIW, I wonder what you actually want to see documented. From
https://lore.kernel.org/all/CVAC8VQPD3PK.1CBS5QTWDSS2C@suppilovahvero/
it sounds like you had trouble with Link:/Closes: tag and Reported-by. From what I can see I don't think bugzilla.kernel.org needs special documentation in that area:
* just use Link:/Closes: to reports to public reports that might be
helpful later in case somebody wants to look at the backstory of a commit, wherever those reports may be (lore, bugzilla.kernel.org,
https://gitlab.freedesktop.org/drm/intel/-/issues,
https://github.com/thesofproject/linux/issues, ...)
* use Reported-by: to give credit to anyone that deserves it, as it is
a nice way to say thx while motivate people to help again in the future. That usually will include the initial reporter, but might also include people that replied to a report from somebody else and helped perceptible with debugging or fixing.
Ciao, Thorsten
[1] I only sometimes help people that report regressions to bugzilla.kernel.org that otherwise would likely would fall through the cracks (among others because many reports are never forwarded to the proper developers otherwise).
Jarkko Sakkinen
maintainerQuoted 45 lines · Thorsten Leemhuis
Quoted 14 lines · Jarkko Sakkinen
Quoted 5 lines · Thorsten Leemhuis
Quoted 4 lines · Linux regression tracking (Thorsten Leemhuis)
Quoted 3 lines · Mario Limonciello
Quoted 2 lines · Jarkko Sakkinen
Quoted 1 line · Mario Limonciello
On 8/23/2023 12:40, Jarkko Sakkinen wrote:
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines · Paul Menzel
Quoted 7 lines
Am 23.08.23 um 01:15 schrieb Jarkko Sakkinen:
Quoted 6 lines
The vendor check introduced by commit
554b841d4703("tpm: Disable RNG for
all AMD fTPMs") doesn't work properly on a number of Intel fTPMs. On the
reported systems the TPM doesn't reply at bootup and returns back the command code. This makes the TPM fail probe.Quoted 6 lines
[...]
Quoted 5 lines
Hmmm. Quite a bit progress to fix the issue was made in the first week after Todd's report; Jarkko apparently even applied the earlier patch from Mario to his master branch:
https://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git/commit/?id=b1a62d41bdc1d15b0641759717e8c3651f0a810c
But since then (aka in the past week) there was not much progress.Jarkko, many thx for picking this up and submitting it to Linus, much appreciated. Sorry again for prodding things, but I felt I had to. Hope you didn't mind too much.
Quoted 3 lines
Could it be possible to extend the actual kernel documentation to give at least some guidelines how a maintainer should deal with the bugzilla?
I guess it's best if that is done by somebody that cares about bugzilla (I don't fall into that group[1]) and knows the official status.
But FWIW, I wonder what you actually want to see documented. From
https://lore.kernel.org/all/CVAC8VQPD3PK.1CBS5QTWDSS2C@suppilovahvero/
it sounds like you had trouble with Link:/Closes: tag and Reported-by. From what I can see I don't think bugzilla.kernel.org needs special documentation in that area:* just use Link:/Closes: to reports to public reports that might be
helpful later in case somebody wants to look at the backstory of a commit, wherever those reports may be (lore, bugzilla.kernel.org,
https://gitlab.freedesktop.org/drm/intel/-/issues,
https://github.com/thesofproject/linux/issues, ...)
Quoted 5 lines
* use Reported-by: to give credit to anyone that deserves it, as it is
a nice way to say thx while motivate people to help again in the future. That usually will include the initial reporter, but might also include people that replied to a report from somebody else and helped perceptible with debugging or fixing.
I *kind of* agree with this but checkpatch.pl disagrees with this :-/
And it is an actual issue that bugzilla is hosted in kernel.org domain, and at the same time it is undocumented process.
AFAIK anything that is not part of the process can be ignored in the process so *theoretically* anything put to kernel bugzilla ca be ignored. This is how it is with e.g. patchwork - some people use it, some people don't.
Personally I think bugzilla, being user approachable system, should be better defined but *theoretically*, at least by the process, it can be fully ignored.
This is where the main source of confusion inherits from, when you put your maintainer hat on.
Quoted 6 lines
Ciao, Thorsten
[1] I only sometimes help people that report regressions to bugzilla.kernel.org that otherwise would likely would fall through the cracks (among others because many reports are never forwarded to the proper developers otherwise).
BR, Jarkko
Jarkko Sakkinen
maintainerQuoted 3 lines · Jarkko Sakkinen
Personally I think bugzilla, being user approachable system, should be better defined but *theoretically*, at least by the process, it can be fully ignored.
I.e. I don't think it should be ignored :-) </disclaimer>
BR, Jarkko
Jarkko Sakkinen
maintainerQuoted 71 lines · Mario Limonciello
Quoted 53 lines · Jarkko Sakkinen
Quoted 48 lines · Mario Limonciello
Quoted 38 lines · Jarkko Sakkinen
Quoted 22 lines · Paul Menzel
Dear Jarkko,
Thank you for your patch.
Am 23.08.23 um 01:15 schrieb Jarkko Sakkinen:
Quoted 9 lines
The vendor check introduced by commit
554b841d4703("tpm: Disable RNG for all AMD fTPMs") doesn't work properly on a number of Intel fTPMs. On the reported systems the TPM doesn't reply at bootup and returns back the command code. This makes the TPM fail probe.Since only Microsoft Pluton is the only known combination of AMD CPU and fTPM from other vendor, disable hwrng otherwise. In order to make sysadmin aware of this, print also info message to the klog.
Quoted 5 lines
Fixes 554b841d4703 ("tpm: Disable RNG for all AMD fTPMs")Reported-by Todd Brandt <[email protected]>Signed-off-by Jarkko Sakkinen <[email protected]>Quoted 5 lines
Mario’s patch also had the three reporters below listed:
Reported-by Patrick Steinhardt <[email protected]>Reported-by Ronan Pigott <[email protected]>Reported-by Raymond Jay Golo <[email protected]>The problem here is that checkpatch throws three warnings:
WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report #19:
Reported-by Patrick Steinhardt <[email protected]>Reported-by Ronan Pigott <[email protected]>WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report #20:
Quoted 8 lines
Reported-by Ronan Pigott <[email protected]>Reported-by Raymond Jay Golo <[email protected]>WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report #21:
Reported-by Raymond Jay Golo <[email protected]>Signed-off-by Jarkko Sakkinen <[email protected]>Quoted 3 lines
FWIW I observed the same checkpatch warning when I submitted my version of the patch. I figured it's better to ignore the warning and attribute everyone who reported the issue affected them.
Quoted 8 lines
OK so:
1. checkpatch.pl is part of the kernel process.
2. Bugzilla is not part of the kernel process.Why emphasis on 1?
BR, Jarkko
The reason I submitted it this way is because of this quote from the documentation [1].
"Check your patches with the patch style checker prior to submission (
scripts/checkpatch.pl). Note, though, that the style checker should be viewed as a guide, not as a replacement for human judgment. If your code looks better with a violation then its probably best left alone."
Quoted 6 lines
I wanted the patch to capture and attribute all those that reported it not just the "first one". Like I said previously, it's better to have a collection of people to ping to notify if something needs to be reverted.
[1]
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#style-check-your-changes
Please denote also that kernel bugzilla is not mentioned in the page that you put as a reference, and only reporter in the LKML has been Todd.
BR, Jarkko
Jarkko Sakkinen
maintainerQuoted 76 lines · Jarkko Sakkinen
Quoted 75 lines · Mario Limonciello
Quoted 53 lines · Jarkko Sakkinen
Quoted 48 lines · Mario Limonciello
Quoted 38 lines · Jarkko Sakkinen
Quoted 22 lines · Paul Menzel
Dear Jarkko,
Thank you for your patch.
Am 23.08.23 um 01:15 schrieb Jarkko Sakkinen:
Quoted 9 lines
The vendor check introduced by commit
554b841d4703("tpm: Disable RNG for all AMD fTPMs") doesn't work properly on a number of Intel fTPMs. On the reported systems the TPM doesn't reply at bootup and returns back the command code. This makes the TPM fail probe.Since only Microsoft Pluton is the only known combination of AMD CPU and fTPM from other vendor, disable hwrng otherwise. In order to make sysadmin aware of this, print also info message to the klog.
Quoted 5 lines
Fixes 554b841d4703 ("tpm: Disable RNG for all AMD fTPMs")Reported-by Todd Brandt <[email protected]>Signed-off-by Jarkko Sakkinen <[email protected]>Quoted 5 lines
Mario’s patch also had the three reporters below listed:
Reported-by Patrick Steinhardt <[email protected]>Reported-by Ronan Pigott <[email protected]>Reported-by Raymond Jay Golo <[email protected]>The problem here is that checkpatch throws three warnings:
WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report #19:
Reported-by Patrick Steinhardt <[email protected]>Reported-by Ronan Pigott <[email protected]>WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report #20:
Quoted 8 lines
Reported-by Ronan Pigott <[email protected]>Reported-by Raymond Jay Golo <[email protected]>WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report #21:
Reported-by Raymond Jay Golo <[email protected]>Signed-off-by Jarkko Sakkinen <[email protected]>Quoted 3 lines
FWIW I observed the same checkpatch warning when I submitted my version of the patch. I figured it's better to ignore the warning and attribute everyone who reported the issue affected them.
Quoted 8 lines
OK so:
1. checkpatch.pl is part of the kernel process.
2. Bugzilla is not part of the kernel process.Why emphasis on 1?
BR, Jarkko
The reason I submitted it this way is because of this quote from the documentation [1].
"Check your patches with the patch style checker prior to submission (
scripts/checkpatch.pl). Note, though, that the style checker should be viewed as a guide, not as a replacement for human judgment. If your code looks better with a violation then its probably best left alone."I wanted the patch to capture and attribute all those that reported it not just the "first one". Like I said previously, it's better to have a collection of people to ping to notify if something needs to be reverted.
Quoted 6 lines
Quoted 2 lines
[1]
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#style-check-your-changesPlease denote also that kernel bugzilla is not mentioned in the page that you put as a reference, and only reporter in the LKML has been Todd.
Also the bugzilla is ambiguous because in this thread I get a picture that any possible commenter is a reporter, and at the same time bugzilla has a *specific field* for a reporter.
How do the comments and the field for the reporter relate, and how they should be interpreted?
BR, Jarkko
checkpatch complains about Reported-by block (was: [PATCH v3] tpm: Enable hwrng only for Pluton on AMD CPUs)
In reply to Jarkko Sakkinen[Cc: +Andy, +Joe]
Dear Jarkko, dear Andy, dear Joe,
Am 23.08.23 um 19:40 schrieb Jarkko Sakkinen:
Quoted 41 lines · Paul Menzel
Quoted 17 lines
Am 23.08.23 um 01:15 schrieb Jarkko Sakkinen:
Quoted 9 lines
The vendor check introduced by commit
554b841d4703("tpm: Disable RNG for all AMD fTPMs") doesn't work properly on a number of Intel fTPMs. On the reported systems the TPM doesn't reply at bootup and returns back the command code. This makes the TPM fail probe.Since only Microsoft Pluton is the only known combination of AMD CPU and fTPM from other vendor, disable hwrng otherwise. In order to make sysadmin aware of this, print also info message to the klog.
Quoted 5 lines
Fixes 554b841d4703 ("tpm: Disable RNG for all AMD fTPMs")Reported-by Todd Brandt <[email protected]>Signed-off-by Jarkko Sakkinen <[email protected]>Quoted 5 lines
Mario’s patch also had the three reporters below listed:
Reported-by Patrick Steinhardt <[email protected]>Reported-by Ronan Pigott <[email protected]>Reported-by Raymond Jay Golo <[email protected]>The problem here is that checkpatch throws three warnings:
WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report #19:
Reported-by Patrick Steinhardt <[email protected]>Reported-by Ronan Pigott <[email protected]>WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report #20:
Reported-by Ronan Pigott <[email protected]>Reported-by Raymond Jay Golo <[email protected]>WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report #21:
Reported-by Raymond Jay Golo <[email protected]>Signed-off-by Jarkko Sakkinen <[email protected]>
Quoted 6 lines
Since bugzilla is not part of the documented process afaik, I used this field as the guideline:
Reported: 2023-08-17 20:59 UTC by Todd Brandt
How otherwise I should interpret kernel bugzilla?
How is the proper process to add more than one reporter (so they are noted and also added to CC), so that checkpatch.pl does not complain?
Kind regards,
Paul
Quoted 5 lines
In any case new version is still needed as the commit message must contain a mention of "Lenovo Legion Y540" as the stimulus for doing this code change in the first place.
BR, Jarkko
Joe Perches
maintainerRe: checkpatch complains about Reported-by block (was: [PATCH v3] tpm: Enable hwrng only for Pluton on AMD CPUs)
Quoted 55 lines · Paul Menzel
[Cc: +Andy, +Joe]
Dear Jarkko, dear Andy, dear Joe,
Am 23.08.23 um 19:40 schrieb Jarkko Sakkinen:
Quoted 41 lines · Paul Menzel
Quoted 17 lines
Am 23.08.23 um 01:15 schrieb Jarkko Sakkinen:
Quoted 9 lines
The vendor check introduced by commit
554b841d4703("tpm: Disable RNG for all AMD fTPMs") doesn't work properly on a number of Intel fTPMs. On the reported systems the TPM doesn't reply at bootup and returns back the command code. This makes the TPM fail probe.Since only Microsoft Pluton is the only known combination of AMD CPU and fTPM from other vendor, disable hwrng otherwise. In order to make sysadmin aware of this, print also info message to the klog.
Quoted 5 lines
Fixes 554b841d4703 ("tpm: Disable RNG for all AMD fTPMs")Reported-by Todd Brandt <[email protected]>Signed-off-by Jarkko Sakkinen <[email protected]>Quoted 5 lines
Mario’s patch also had the three reporters below listed:
Reported-by Patrick Steinhardt <[email protected]>Reported-by Ronan Pigott <[email protected]>Reported-by Raymond Jay Golo <[email protected]>The problem here is that checkpatch throws three warnings:
WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report #19:
Reported-by Patrick Steinhardt <[email protected]>Reported-by Ronan Pigott <[email protected]>WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report #20:
Reported-by Ronan Pigott <[email protected]>Reported-by Raymond Jay Golo <[email protected]>WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report #21:
Reported-by Raymond Jay Golo <[email protected]>Signed-off-by Jarkko Sakkinen <[email protected]>Quoted 6 lines
Since bugzilla is not part of the documented process afaik, I used this field as the guideline:
Reported: 2023-08-17 20:59 UTC by Todd Brandt
How otherwise I should interpret kernel bugzilla?
Quoted 14 lines
How is the proper process to add more than one reporter (so they are noted and also added to CC), so that checkpatch.pl does not complain?
Kind regards,
Paul
Quoted 5 lines
In any case new version is still needed as the commit message must contain a mention of "Lenovo Legion Y540" as the stimulus for doing this code change in the first place.
BR, Jarkko
Well, if it's really desired to allow multiple consecutive reported-by:
lines, maybe:
--- scripts/checkpatch.pl | 2 ++ 1 file changed, 2 insertions(+)
scripts/checkpatch.pl +2 -0
@@ -3179,6 +3179,8 @@ sub process { if (!defined $lines[$linenr]) { WARN("BAD_REPORTED_BY_LINK", "Reported-by: should be immediately followed by Closes: with a URL to the report\n" . $herecurr . "\n"); + } elsif ($rawlines[$linenr] =~ /^\s*reported(?:|-and-tested)-by:/i) { + ; } elsif ($rawlines[$linenr] !~ /^closes:\s*/i) { WARN("BAD_REPORTED_BY_LINK", "Reported-by: should be immediately followed by Closes: with a URL to the report\n" . $herecurr . $rawlines[$linenr] . "\n");
Jarkko Sakkinen
maintainerRe: checkpatch complains about Reported-by block (was: [PATCH v3] tpm: Enable hwrng only for Pluton on AMD CPUs)
Quoted 82 lines · Joe Perches
Quoted 55 lines · Paul Menzel
[Cc: +Andy, +Joe]
Dear Jarkko, dear Andy, dear Joe,
Am 23.08.23 um 19:40 schrieb Jarkko Sakkinen:
Quoted 41 lines · Paul Menzel
Quoted 17 lines
Am 23.08.23 um 01:15 schrieb Jarkko Sakkinen:
Quoted 9 lines
The vendor check introduced by commit
554b841d4703("tpm: Disable RNG for all AMD fTPMs") doesn't work properly on a number of Intel fTPMs. On the reported systems the TPM doesn't reply at bootup and returns back the command code. This makes the TPM fail probe.Since only Microsoft Pluton is the only known combination of AMD CPU and fTPM from other vendor, disable hwrng otherwise. In order to make sysadmin aware of this, print also info message to the klog.
Quoted 5 lines
Fixes 554b841d4703 ("tpm: Disable RNG for all AMD fTPMs")Reported-by Todd Brandt <[email protected]>Signed-off-by Jarkko Sakkinen <[email protected]>Quoted 5 lines
Mario’s patch also had the three reporters below listed:
Reported-by Patrick Steinhardt <[email protected]>Reported-by Ronan Pigott <[email protected]>Reported-by Raymond Jay Golo <[email protected]>The problem here is that checkpatch throws three warnings:
WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report #19:
Reported-by Patrick Steinhardt <[email protected]>Reported-by Ronan Pigott <[email protected]>WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report #20:
Reported-by Ronan Pigott <[email protected]>Reported-by Raymond Jay Golo <[email protected]>WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report #21:
Reported-by Raymond Jay Golo <[email protected]>Signed-off-by Jarkko Sakkinen <[email protected]>Quoted 6 lines
Since bugzilla is not part of the documented process afaik, I used this field as the guideline:
Reported: 2023-08-17 20:59 UTC by Todd Brandt
How otherwise I should interpret kernel bugzilla?
Quoted 14 lines
How is the proper process to add more than one reporter (so they are noted and also added to CC), so that checkpatch.pl does not complain?
Kind regards,
Paul
Quoted 5 lines
In any case new version is still needed as the commit message must contain a mention of "Lenovo Legion Y540" as the stimulus for doing this code change in the first place.
BR, Jarkko
Well, if it's really desired to allow multiple consecutive reported-by:
lines, maybe:--- scripts/checkpatch.pl | 2 ++ 1 file changed, 2 insertions(+)scripts/checkpatch.pl +0
-0
scripts/checkpatch.pl sub process { · line 3186
Quoted 8 lines
scripts/checkpatch.pl +2
-0@@ -3179,6 +3179,8 @@ sub process { if (!defined $lines[$linenr]) { WARN("BAD_REPORTED_BY_LINK", "Reported-by: should be immediately followed by Closes: with a URL to the report\n" . $herecurr . "\n"); + } elsif ($rawlines[$linenr] =~ /^\s*reported(?:|-and-tested)-by:/i) { + ; } elsif ($rawlines[$linenr] !~ /^closes:\s*/i) { WARN("BAD_REPORTED_BY_LINK", "Reported-by: should be immediately followed by Closes: with a URL to the report\n" . $herecurr . $rawlines[$linenr] . "\n");
Kind of opposing this because:
1. Bugzilla has a reporter field.
2. The request is now, if I understood this correctly, to add reported-by field to all people who have left a comment.
3. There is a field for the reporter, which points out to a single person. Why all the possible commenters and not the creator of the report?
BR, Jarkko
Jarkko Sakkinen
maintainerRe: checkpatch complains about Reported-by block (was: [PATCH v3] tpm: Enable hwrng only for Pluton on AMD CPUs)
In reply to Paul MenzelQuoted 51 lines · Paul Menzel
[Cc: +Andy, +Joe]
Dear Jarkko, dear Andy, dear Joe,
Am 23.08.23 um 19:40 schrieb Jarkko Sakkinen:
Quoted 44 lines · Paul Menzel
Quoted 17 lines
Am 23.08.23 um 01:15 schrieb Jarkko Sakkinen:
Quoted 9 lines
The vendor check introduced by commit
554b841d4703("tpm: Disable RNG for all AMD fTPMs") doesn't work properly on a number of Intel fTPMs. On the reported systems the TPM doesn't reply at bootup and returns back the command code. This makes the TPM fail probe.Since only Microsoft Pluton is the only known combination of AMD CPU and fTPM from other vendor, disable hwrng otherwise. In order to make sysadmin aware of this, print also info message to the klog.
Quoted 5 lines
Fixes 554b841d4703 ("tpm: Disable RNG for all AMD fTPMs")Reported-by Todd Brandt <[email protected]>Signed-off-by Jarkko Sakkinen <[email protected]>Quoted 5 lines
Mario’s patch also had the three reporters below listed:
Reported-by Patrick Steinhardt <[email protected]>Reported-by Ronan Pigott <[email protected]>Reported-by Raymond Jay Golo <[email protected]>The problem here is that checkpatch throws three warnings:
WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report #19:
Reported-by Patrick Steinhardt <[email protected]>Reported-by Ronan Pigott <[email protected]>WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report #20:
Reported-by Ronan Pigott <[email protected]>Reported-by Raymond Jay Golo <[email protected]>WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report #21:
Reported-by Raymond Jay Golo <[email protected]>Signed-off-by Jarkko Sakkinen <[email protected]>Since bugzilla is not part of the documented process afaik, I used this field as the guideline:
Quoted 6 lines
Quoted 3 lines
Reported: 2023-08-17 20:59 UTC by Todd Brandt
How otherwise I should interpret kernel bugzilla?
How is the proper process to add more than one reporter (so they are noted and also added to CC), so that checkpatch.pl does not complain?
I have no idea. I actually tried all sorts of combinations with no luck.
Since it exists and is out there, the process documentation should really bring up some clarity to the kernel bugzilla.
BR, Jarkko
Quoted 21 lines · Jarkko Sakkinen
The vendor check introduced by commit
554b841d4703("tpm: Disable RNG for all AMD fTPMs") doesn't work properly on a number of Intel fTPMs. On the reported systems the TPM doesn't reply at bootup and returns back the command code. This makes the TPM fail probe.Since only Microsoft Pluton is the only known combination of AMD CPU and fTPM from other vendor, disable hwrng otherwise. In order to make sysadmin aware of this, print also info message to the klog.
Fixes 554b841d4703 ("tpm: Disable RNG for all AMD fTPMs")Reported-by Todd Brandt <[email protected]>Signed-off-by Jarkko Sakkinen <[email protected]>---
v3:
* Forgot to amend config flags.
v2:
*CONFIG_X86
* Removed "Reviewed-by: Jarkko Sakkinen <[email protected]>"
* Removed "Signed-off-by: Mario Limonciello <[email protected]>"
Quoted 4 lines
--- drivers/char/tpm/tpm_crb.c | 33 ++++++++------------------------- 1 file changed, 8 insertions(+), 25 deletions(-)
It looks like stable should be pinged as well. I saw a report yesterday for Fedora where someone is seeing issue where the tpm device no longer shows up with a 6.4.11 kernel. That kernel pulled in commit 554b841d4703. It pulled in a couple other tpm commits, and there is practically no info in the bug at this point, but I'm guessing the probe is failing due to 554b841d4703.
Quoted 54 lines
drivers/char/tpm/tpm_crb.c +8
-25@@ -463,28 +463,6 @@ 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,@@ -827,9 +805,14 @@ static int crb_acpi_add(struct acpi_device *device)if (rc) goto out; - rc =crb_check_flags (chip); -if (rc) - goto out; +#ifdef CONFIG_X86 +/* A quirk for https://www.amd.com/en/support/kb/faq/pa-410 */ +if (boot_cpu_data.x86_vendor ==X86_VENDOR_AMD && + priv->sm !=ACPI_TPM2_COMMAND_BUFFER_WITH_PLUTON ) { +dev_info (dev,"Disabling hwrng\n" ); + chip->flags |=TPM_CHIP_FLAG_HWRNG_DISABLED ; + } +#endif /* CONFIG_X86 */ rc =tpm_chip_register (chip);Signature · 1 line
2.39.2
Quoted 21 lines · Jarkko Sakkinen
The vendor check introduced by commit
554b841d4703("tpm: Disable RNG for all AMD fTPMs") doesn't work properly on a number of Intel fTPMs. On the reported systems the TPM doesn't reply at bootup and returns back the command code. This makes the TPM fail probe.Since only Microsoft Pluton is the only known combination of AMD CPU and fTPM from other vendor, disable hwrng otherwise. In order to make sysadmin aware of this, print also info message to the klog.
Fixes 554b841d4703 ("tpm: Disable RNG for all AMD fTPMs")Reported-by Todd Brandt <[email protected]>Signed-off-by Jarkko Sakkinen <[email protected]>---
v3:
* Forgot to amend config flags.
v2:
*CONFIG_X86
* Removed "Reviewed-by: Jarkko Sakkinen <[email protected]>"
* Removed "Signed-off-by: Mario Limonciello <[email protected]>"
Quoted 4 lines
--- drivers/char/tpm/tpm_crb.c | 33 ++++++++------------------------- 1 file changed, 8 insertions(+), 25 deletions(-)
Quoted 54 lines
drivers/char/tpm/tpm_crb.c +8
-25@@ -463,28 +463,6 @@ 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,@@ -827,9 +805,14 @@ static int crb_acpi_add(struct acpi_device *device)if (rc) goto out; - rc =crb_check_flags (chip); -if (rc) - goto out; +#ifdef CONFIG_X86 +/* A quirk for https://www.amd.com/en/support/kb/faq/pa-410 */ +if (boot_cpu_data.x86_vendor ==X86_VENDOR_AMD && + priv->sm !=ACPI_TPM2_COMMAND_BUFFER_WITH_PLUTON ) { +dev_info (dev,"Disabling hwrng\n" ); + chip->flags |=TPM_CHIP_FLAG_HWRNG_DISABLED ; + } +#endif /* CONFIG_X86 */ rc =tpm_chip_register (chip);Signature · 1 line
2.39.2
Quoted 31 lines · Jerry Snitselaar
Quoted 23 lines · Jarkko Sakkinen
The vendor check introduced by commit
554b841d4703("tpm: Disable RNG for all AMD fTPMs") doesn't work properly on a number of Intel fTPMs. On the reported systems the TPM doesn't reply at bootup and returns back the command code. This makes the TPM fail probe.Since only Microsoft Pluton is the only known combination of AMD CPU and fTPM from other vendor, disable hwrng otherwise. In order to make sysadmin aware of this, print also info message to the klog.
Fixes 554b841d4703 ("tpm: Disable RNG for all AMD fTPMs")Reported-by Todd Brandt <[email protected]>Signed-off-by Jarkko Sakkinen <[email protected]>---
v3:
* Forgot to amend config flags.
v2:
*CONFIG_X86
* Removed "Reviewed-by: Jarkko Sakkinen <[email protected]>"
* Removed "Signed-off-by: Mario Limonciello <[email protected]>"
---drivers/char/tpm/tpm_crb.c| 33 ++++++++-------------------------Quoted 2 lines
1 file changed, 8 insertions(+), 25 deletions(-)
Quoted 1 line
Reviewed-by Jerry Snitselaar <[email protected]>
It looks like the Fedora folks are getting more reports of the issue.
Jarkko Sakkinen
maintainerQuoted 31 lines · Jerry Snitselaar
Quoted 31 lines · Jerry Snitselaar
Quoted 25 lines · Jarkko Sakkinen
The vendor check introduced by commit
554b841d4703("tpm: Disable RNG for all AMD fTPMs") doesn't work properly on a number of Intel fTPMs. On the reported systems the TPM doesn't reply at bootup and returns back the command code. This makes the TPM fail probe.Since only Microsoft Pluton is the only known combination of AMD CPU and fTPM from other vendor, disable hwrng otherwise. In order to make sysadmin aware of this, print also info message to the klog.
Fixes 554b841d4703 ("tpm: Disable RNG for all AMD fTPMs")Reported-by Todd Brandt <[email protected]>Signed-off-by Jarkko Sakkinen <[email protected]>---
v3:
* Forgot to amend config flags.
v2:
*CONFIG_X86
* Removed "Reviewed-by: Jarkko Sakkinen <[email protected]>"
* Removed "Signed-off-by: Mario Limonciello <[email protected]>"
---drivers/char/tpm/tpm_crb.c| 33 ++++++++-------------------------1 file changed, 8 insertions(+), 25 deletions(-)
Quoted 4 lines
Quoted 1 line
Reviewed-by Jerry Snitselaar <[email protected]>It looks like the Fedora folks are getting more reports of the issue.
https://lore.kernel.org/linux-integrity/[email protected]/T/#u
I have all the possible reported-by's. I still don't fully understand kernel bugzilla's role. I don't oppose having it but e.g. for me reporter has been traditionally someone who reports the bug in LKML, not in bugzilla. Also the ambiguity of the whole discussion has been over the top. E.g. why bugzilla even has a field for reporter if that is not *the* reporter at least according to this discussion?
And in the case of this bug, the reporter in bugzilla was the same exact person who mailed about it to LKML.
I'm actually cool with almost any policy, as long as there is at least some policy in existence. Pretty confusing exercise overally, and very time consuming for a maintainer.
BR, Jarkko