[PATCH v3] tpm: Enable hwrng only for Pluton on AMD CPUs

[PATCH v3] tpm: Enable hwrng only for Pluton on AMD CPUs

[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.

Cc: stable@vger.kernel.org
Fixes: 554b841d4703 ("tpm: Disable RNG for all AMD fTPMs")
Reported-by: Todd Brandt <todd.e.brandt@intel.com>
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217804
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
v3:
* Forgot to amend config flags.
v2:
* CONFIG_X86
* Removed "Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>"
* Removed "Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>"
---
 drivers/char/tpm/tpm_crb.c | 33 ++++++++-------------------------
 1 file changed, 8 insertions(+), 25 deletions(-)

diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index 65ff4d2fbe8d..ea085b14ab7c 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -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);
 
-- 
2.39.2

Re: [PATCH v3] tpm: Enable hwrng only for Pluton on AMD CPUs

[Paul Menzel]

Dear Jarkko,


Thank you for your patch.


Am 23.08.23 um 01:15 schrieb 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.
> 
> Cc: stable@vger.kernel.org
> Fixes: 554b841d4703 ("tpm: Disable RNG for all AMD fTPMs")
> Reported-by: Todd Brandt <todd.e.brandt@intel.com>
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217804
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>

Mario’s patch also had the three reporters below listed:

Reported-by: Patrick Steinhardt <ps@pks.im>
Reported-by: Ronan Pigott <ronan@rjp.ie>
Reported-by: Raymond Jay Golo <rjgolo@gmail.com>

> ---
> v3:
> * Forgot to amend config flags.
> v2:
> * CONFIG_X86
> * Removed "Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>"
> * Removed "Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>"
> ---
>   drivers/char/tpm/tpm_crb.c | 33 ++++++++-------------------------
>   1 file changed, 8 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
> index 65ff4d2fbe8d..ea085b14ab7c 100644
> --- a/drivers/char/tpm/tpm_crb.c
> +++ b/drivers/char/tpm/tpm_crb.c
> @@ -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");

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)

> +		chip->flags |= TPM_CHIP_FLAG_HWRNG_DISABLED;
> +	}
> +#endif /* CONFIG_X86 */
>   
>   	rc = tpm_chip_register(chip);
>   


Kind regards,

Paul

Re: [PATCH v3] tpm: Enable hwrng only for Pluton on AMD CPUs

[Jarkko Sakkinen]

On Wed Aug 23, 2023 at 11:23 AM EEST, Paul Menzel wrote:
> Dear Jarkko,
>
>
> Thank you for your patch.
>
>
> Am 23.08.23 um 01:15 schrieb 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.
> > 
> > Cc: stable@vger.kernel.org
> > Fixes: 554b841d4703 ("tpm: Disable RNG for all AMD fTPMs")
> > Reported-by: Todd Brandt <todd.e.brandt@intel.com>
> > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217804
> > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
>
> Mario’s patch also had the three reporters below listed:
>
> Reported-by: Patrick Steinhardt <ps@pks.im>
> Reported-by: Ronan Pigott <ronan@rjp.ie>
> Reported-by: Raymond Jay Golo <rjgolo@gmail.com>

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 <ps@pks.im>
Reported-by: Ronan Pigott <ronan@rjp.ie>

WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report
#20:
Reported-by: Ronan Pigott <ronan@rjp.ie>
Reported-by: Raymond Jay Golo <rjgolo@gmail.com>

WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report
#21:
Reported-by: Raymond Jay Golo <rjgolo@gmail.com>
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>

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

Re: [PATCH v3] tpm: Enable hwrng only for Pluton on AMD CPUs

[Mario Limonciello]

On 8/23/2023 12:40, Jarkko Sakkinen wrote:
> On Wed Aug 23, 2023 at 11:23 AM EEST, Paul Menzel wrote:
>> Dear Jarkko,
>>
>>
>> Thank you for your patch.
>>
>>
>> Am 23.08.23 um 01:15 schrieb 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.
>>>
>>> Cc: stable@vger.kernel.org
>>> Fixes: 554b841d4703 ("tpm: Disable RNG for all AMD fTPMs")
>>> Reported-by: Todd Brandt <todd.e.brandt@intel.com>
>>> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217804
>>> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
>>
>> Mario’s patch also had the three reporters below listed:
>>
>> Reported-by: Patrick Steinhardt <ps@pks.im>
>> Reported-by: Ronan Pigott <ronan@rjp.ie>
>> Reported-by: Raymond Jay Golo <rjgolo@gmail.com>
> 
> 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 <ps@pks.im>
> Reported-by: Ronan Pigott <ronan@rjp.ie>
> 
> WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report
> #20:
> Reported-by: Ronan Pigott <ronan@rjp.ie>
> Reported-by: Raymond Jay Golo <rjgolo@gmail.com>
> 
> WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report
> #21:
> Reported-by: Raymond Jay Golo <rjgolo@gmail.com>
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> 

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.

> 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

Re: [PATCH v3] tpm: Enable hwrng only for Pluton on AMD CPUs

[Jarkko Sakkinen]

On Wed Aug 23, 2023 at 9:58 PM EEST, Mario Limonciello wrote:
> On 8/23/2023 12:40, Jarkko Sakkinen wrote:
> > On Wed Aug 23, 2023 at 11:23 AM EEST, Paul Menzel wrote:
> >> Dear Jarkko,
> >>
> >>
> >> Thank you for your patch.
> >>
> >>
> >> Am 23.08.23 um 01:15 schrieb 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.
> >>>
> >>> Cc: stable@vger.kernel.org
> >>> Fixes: 554b841d4703 ("tpm: Disable RNG for all AMD fTPMs")
> >>> Reported-by: Todd Brandt <todd.e.brandt@intel.com>
> >>> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217804
> >>> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> >>
> >> Mario’s patch also had the three reporters below listed:
> >>
> >> Reported-by: Patrick Steinhardt <ps@pks.im>
> >> Reported-by: Ronan Pigott <ronan@rjp.ie>
> >> Reported-by: Raymond Jay Golo <rjgolo@gmail.com>
> > 
> > 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 <ps@pks.im>
> > Reported-by: Ronan Pigott <ronan@rjp.ie>
> > 
> > WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report
> > #20:
> > Reported-by: Ronan Pigott <ronan@rjp.ie>
> > Reported-by: Raymond Jay Golo <rjgolo@gmail.com>
> > 
> > WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report
> > #21:
> > Reported-by: Raymond Jay Golo <rjgolo@gmail.com>
> > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> > 
>
> 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

Re: [PATCH v3] tpm: Enable hwrng only for Pluton on AMD CPUs

[Mario Limonciello]

On 8/27/2023 13:12, Jarkko Sakkinen wrote:
> On Wed Aug 23, 2023 at 9:58 PM EEST, Mario Limonciello wrote:
>> On 8/23/2023 12:40, Jarkko Sakkinen wrote:
>>> On Wed Aug 23, 2023 at 11:23 AM EEST, Paul Menzel wrote:
>>>> Dear Jarkko,
>>>>
>>>>
>>>> Thank you for your patch.
>>>>
>>>>
>>>> Am 23.08.23 um 01:15 schrieb 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.
>>>>>
>>>>> Cc: stable@vger.kernel.org
>>>>> Fixes: 554b841d4703 ("tpm: Disable RNG for all AMD fTPMs")
>>>>> Reported-by: Todd Brandt <todd.e.brandt@intel.com>
>>>>> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217804
>>>>> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
>>>>
>>>> Mario’s patch also had the three reporters below listed:
>>>>
>>>> Reported-by: Patrick Steinhardt <ps@pks.im>
>>>> Reported-by: Ronan Pigott <ronan@rjp.ie>
>>>> Reported-by: Raymond Jay Golo <rjgolo@gmail.com>
>>>
>>> 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 <ps@pks.im>
>>> Reported-by: Ronan Pigott <ronan@rjp.ie>
>>>
>>> WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report
>>> #20:
>>> Reported-by: Ronan Pigott <ronan@rjp.ie>
>>> Reported-by: Raymond Jay Golo <rjgolo@gmail.com>
>>>
>>> WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report
>>> #21:
>>> Reported-by: Raymond Jay Golo <rjgolo@gmail.com>
>>> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
>>>
>>
>> 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

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

Re: [PATCH v3] tpm: Enable hwrng only for Pluton on AMD CPUs

[Linux regression tracking (Thorsten Leemhuis)]

[CCing the regression list, as it should be in the loop for regressions:
https://docs.kernel.org/admin-guide/reporting-regressions.html]

On 28.08.23 02:35, Mario Limonciello wrote:
> On 8/27/2023 13:12, Jarkko Sakkinen wrote:
>> On Wed Aug 23, 2023 at 9:58 PM EEST, Mario Limonciello wrote:
>>> On 8/23/2023 12:40, Jarkko Sakkinen wrote:
>>>> On Wed Aug 23, 2023 at 11:23 AM EEST, Paul Menzel wrote:
>>>>> Am 23.08.23 um 01:15 schrieb 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.
>>>>>>
>>>>>> Cc: stable@vger.kernel.org
>>>>>> Fixes: 554b841d4703 ("tpm: Disable RNG for all AMD fTPMs")
>>>>>> Reported-by: Todd Brandt <todd.e.brandt@intel.com>
>>>>>> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217804
>>>>>> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
>>>>>
>>>>> Mario’s patch also had the three reporters below listed:
>>>>>
>>>>> Reported-by: Patrick Steinhardt <ps@pks.im>
>>>>> Reported-by: Ronan Pigott <ronan@rjp.ie>
>>>>> Reported-by: Raymond Jay Golo <rjgolo@gmail.com>
>>>>
>>>> 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

Re: [PATCH v3] tpm: Enable hwrng only for Pluton on AMD CPUs

[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)]

On 29.08.23 10:38, Linux regression tracking (Thorsten Leemhuis) wrote:
> On 28.08.23 02:35, Mario Limonciello wrote:
>> On 8/27/2023 13:12, Jarkko Sakkinen wrote:
>>> On Wed Aug 23, 2023 at 9:58 PM EEST, Mario Limonciello wrote:
>>>> On 8/23/2023 12:40, Jarkko Sakkinen wrote:
>>>>> On Wed Aug 23, 2023 at 11:23 AM EEST, Paul Menzel wrote:
>>>>>> Am 23.08.23 um 01:15 schrieb 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.
>>>>>>>
>>>>>>> Cc: stable@vger.kernel.org
>>>>>>> Fixes: 554b841d4703 ("tpm: Disable RNG for all AMD fTPMs")
>>>>>>> Reported-by: Todd Brandt <todd.e.brandt@intel.com>
>>>>>>> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217804
>>>>>>> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
>>>>>>
>>>>>> Mario’s patch also had the three reporters below listed:
>>>>>>
>>>>>> Reported-by: Patrick Steinhardt <ps@pks.im>
>>>>>> Reported-by: Ronan Pigott <ronan@rjp.ie>
>>>>>> Reported-by: Raymond Jay Golo <rjgolo@gmail.com>
>
> [...] 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

Re: [PATCH v3] tpm: Enable hwrng only for Pluton on AMD CPUs

[Jarkko Sakkinen]

On Fri Sep 1, 2023 at 11:49 AM EEST, Thorsten Leemhuis wrote:
> [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)]
>
> On 29.08.23 10:38, Linux regression tracking (Thorsten Leemhuis) wrote:
> > On 28.08.23 02:35, Mario Limonciello wrote:
> >> On 8/27/2023 13:12, Jarkko Sakkinen wrote:
> >>> On Wed Aug 23, 2023 at 9:58 PM EEST, Mario Limonciello wrote:
> >>>> On 8/23/2023 12:40, Jarkko Sakkinen wrote:
> >>>>> On Wed Aug 23, 2023 at 11:23 AM EEST, Paul Menzel wrote:
> >>>>>> Am 23.08.23 um 01:15 schrieb 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.
> >>>>>>>
> >>>>>>> Cc: stable@vger.kernel.org
> >>>>>>> Fixes: 554b841d4703 ("tpm: Disable RNG for all AMD fTPMs")
> >>>>>>> Reported-by: Todd Brandt <todd.e.brandt@intel.com>
> >>>>>>> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217804
> >>>>>>> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> >>>>>>
> >>>>>> Mario’s patch also had the three reporters below listed:
> >>>>>>
> >>>>>> Reported-by: Patrick Steinhardt <ps@pks.im>
> >>>>>> Reported-by: Ronan Pigott <ronan@rjp.ie>
> >>>>>> Reported-by: Raymond Jay Golo <rjgolo@gmail.com>
> >
> > [...] 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

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/20230904202512.29825-1-jarkko@kernel.org/T/#u

BR, Jarkko

Re: [PATCH v3] tpm: Enable hwrng only for Pluton on AMD CPUs

[Thorsten Leemhuis]

On 05.09.23 00:32, Jarkko Sakkinen wrote:
> On Fri Sep 1, 2023 at 11:49 AM EEST, Thorsten Leemhuis wrote:
>> On 29.08.23 10:38, Linux regression tracking (Thorsten Leemhuis) wrote:
>>> On 28.08.23 02:35, Mario Limonciello wrote:
>>>> On 8/27/2023 13:12, Jarkko Sakkinen wrote:
>>>>> On Wed Aug 23, 2023 at 9:58 PM EEST, Mario Limonciello wrote:
>>>>>> On 8/23/2023 12:40, Jarkko Sakkinen wrote:
>>>>>>> On Wed Aug 23, 2023 at 11:23 AM EEST, Paul Menzel wrote:
>>>>>>>> Am 23.08.23 um 01:15 schrieb 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.
> [...]
>> 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.

> 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).

Re: [PATCH v3] tpm: Enable hwrng only for Pluton on AMD CPUs

[Jarkko Sakkinen]

On Tue Sep 5, 2023 at 3:01 PM EEST, Thorsten Leemhuis wrote:
> On 05.09.23 00:32, Jarkko Sakkinen wrote:
> > On Fri Sep 1, 2023 at 11:49 AM EEST, Thorsten Leemhuis wrote:
> >> On 29.08.23 10:38, Linux regression tracking (Thorsten Leemhuis) wrote:
> >>> On 28.08.23 02:35, Mario Limonciello wrote:
> >>>> On 8/27/2023 13:12, Jarkko Sakkinen wrote:
> >>>>> On Wed Aug 23, 2023 at 9:58 PM EEST, Mario Limonciello wrote:
> >>>>>> On 8/23/2023 12:40, Jarkko Sakkinen wrote:
> >>>>>>> On Wed Aug 23, 2023 at 11:23 AM EEST, Paul Menzel wrote:
> >>>>>>>> Am 23.08.23 um 01:15 schrieb 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.
> > [...]
> >> 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.
>
> > 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.

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.

> 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

Re: [PATCH v3] tpm: Enable hwrng only for Pluton on AMD CPUs

[Jarkko Sakkinen]

On Mon Sep 11, 2023 at 1:40 PM EEST, Jarkko Sakkinen wrote:
> 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

Re: [PATCH v3] tpm: Enable hwrng only for Pluton on AMD CPUs

[Jarkko Sakkinen]

On Mon Aug 28, 2023 at 3:35 AM EEST, Mario Limonciello wrote:
> On 8/27/2023 13:12, Jarkko Sakkinen wrote:
> > On Wed Aug 23, 2023 at 9:58 PM EEST, Mario Limonciello wrote:
> >> On 8/23/2023 12:40, Jarkko Sakkinen wrote:
> >>> On Wed Aug 23, 2023 at 11:23 AM EEST, Paul Menzel wrote:
> >>>> Dear Jarkko,
> >>>>
> >>>>
> >>>> Thank you for your patch.
> >>>>
> >>>>
> >>>> Am 23.08.23 um 01:15 schrieb 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.
> >>>>>
> >>>>> Cc: stable@vger.kernel.org
> >>>>> Fixes: 554b841d4703 ("tpm: Disable RNG for all AMD fTPMs")
> >>>>> Reported-by: Todd Brandt <todd.e.brandt@intel.com>
> >>>>> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217804
> >>>>> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> >>>>
> >>>> Mario’s patch also had the three reporters below listed:
> >>>>
> >>>> Reported-by: Patrick Steinhardt <ps@pks.im>
> >>>> Reported-by: Ronan Pigott <ronan@rjp.ie>
> >>>> Reported-by: Raymond Jay Golo <rjgolo@gmail.com>
> >>>
> >>> 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 <ps@pks.im>
> >>> Reported-by: Ronan Pigott <ronan@rjp.ie>
> >>>
> >>> WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report
> >>> #20:
> >>> Reported-by: Ronan Pigott <ronan@rjp.ie>
> >>> Reported-by: Raymond Jay Golo <rjgolo@gmail.com>
> >>>
> >>> WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report
> >>> #21:
> >>> Reported-by: Raymond Jay Golo <rjgolo@gmail.com>
> >>> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> >>>
> >>
> >> 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
>
> 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

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

Re: [PATCH v3] tpm: Enable hwrng only for Pluton on AMD CPUs

[Jarkko Sakkinen]

On Mon Sep 4, 2023 at 9:00 PM EEST, Jarkko Sakkinen wrote:
> On Mon Aug 28, 2023 at 3:35 AM EEST, Mario Limonciello wrote:
> > On 8/27/2023 13:12, Jarkko Sakkinen wrote:
> > > On Wed Aug 23, 2023 at 9:58 PM EEST, Mario Limonciello wrote:
> > >> On 8/23/2023 12:40, Jarkko Sakkinen wrote:
> > >>> On Wed Aug 23, 2023 at 11:23 AM EEST, Paul Menzel wrote:
> > >>>> Dear Jarkko,
> > >>>>
> > >>>>
> > >>>> Thank you for your patch.
> > >>>>
> > >>>>
> > >>>> Am 23.08.23 um 01:15 schrieb 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.
> > >>>>>
> > >>>>> Cc: stable@vger.kernel.org
> > >>>>> Fixes: 554b841d4703 ("tpm: Disable RNG for all AMD fTPMs")
> > >>>>> Reported-by: Todd Brandt <todd.e.brandt@intel.com>
> > >>>>> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217804
> > >>>>> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> > >>>>
> > >>>> Mario’s patch also had the three reporters below listed:
> > >>>>
> > >>>> Reported-by: Patrick Steinhardt <ps@pks.im>
> > >>>> Reported-by: Ronan Pigott <ronan@rjp.ie>
> > >>>> Reported-by: Raymond Jay Golo <rjgolo@gmail.com>
> > >>>
> > >>> 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 <ps@pks.im>
> > >>> Reported-by: Ronan Pigott <ronan@rjp.ie>
> > >>>
> > >>> WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report
> > >>> #20:
> > >>> Reported-by: Ronan Pigott <ronan@rjp.ie>
> > >>> Reported-by: Raymond Jay Golo <rjgolo@gmail.com>
> > >>>
> > >>> WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report
> > >>> #21:
> > >>> Reported-by: Raymond Jay Golo <rjgolo@gmail.com>
> > >>> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> > >>>
> > >>
> > >> 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
> >
> > 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
>
> 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.

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)

[Paul Menzel]

[Cc: +Andy, +Joe]


Dear Jarkko, dear Andy, dear Joe,


Am 23.08.23 um 19:40 schrieb Jarkko Sakkinen:
> On Wed Aug 23, 2023 at 11:23 AM EEST, Paul Menzel wrote:

>> Am 23.08.23 um 01:15 schrieb 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.
>>>
>>> Cc: stable@vger.kernel.org
>>> Fixes: 554b841d4703 ("tpm: Disable RNG for all AMD fTPMs")
>>> Reported-by: Todd Brandt <todd.e.brandt@intel.com>
>>> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217804
>>> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
>>
>> Mario’s patch also had the three reporters below listed:
>>
>> Reported-by: Patrick Steinhardt <ps@pks.im>
>> Reported-by: Ronan Pigott <ronan@rjp.ie>
>> Reported-by: Raymond Jay Golo <rjgolo@gmail.com>
> 
> 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 <ps@pks.im>
> Reported-by: Ronan Pigott <ronan@rjp.ie>
> 
> WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report
> #20:
> Reported-by: Ronan Pigott <ronan@rjp.ie>
> Reported-by: Raymond Jay Golo <rjgolo@gmail.com>
> 
> WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report
> #21:
> Reported-by: Raymond Jay Golo <rjgolo@gmail.com>
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> 
> 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


> 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

Re: checkpatch complains about Reported-by block (was: [PATCH v3] tpm: Enable hwrng only for Pluton on AMD CPUs)

[Joe Perches]

On Wed, 2023-08-23 at 21:24 +0200, Paul Menzel wrote:
> [Cc: +Andy, +Joe]
> 
> 
> Dear Jarkko, dear Andy, dear Joe,
> 
> 
> Am 23.08.23 um 19:40 schrieb Jarkko Sakkinen:
> > On Wed Aug 23, 2023 at 11:23 AM EEST, Paul Menzel wrote:
> 
> > > Am 23.08.23 um 01:15 schrieb 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.
> > > > 
> > > > Cc: stable@vger.kernel.org
> > > > Fixes: 554b841d4703 ("tpm: Disable RNG for all AMD fTPMs")
> > > > Reported-by: Todd Brandt <todd.e.brandt@intel.com>
> > > > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217804
> > > > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> > > 
> > > Mario’s patch also had the three reporters below listed:
> > > 
> > > Reported-by: Patrick Steinhardt <ps@pks.im>
> > > Reported-by: Ronan Pigott <ronan@rjp.ie>
> > > Reported-by: Raymond Jay Golo <rjgolo@gmail.com>
> > 
> > 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 <ps@pks.im>
> > Reported-by: Ronan Pigott <ronan@rjp.ie>
> > 
> > WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report
> > #20:
> > Reported-by: Ronan Pigott <ronan@rjp.ie>
> > Reported-by: Raymond Jay Golo <rjgolo@gmail.com>
> > 
> > WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report
> > #21:
> > Reported-by: Raymond Jay Golo <rjgolo@gmail.com>
> > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> > 
> > 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
> 
> 
> > 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(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 528f619520eb9..5b5c11ad04087 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -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");

Re: checkpatch complains about Reported-by block (was: [PATCH v3] tpm: Enable hwrng only for Pluton on AMD CPUs)

[Jarkko Sakkinen]

On Thu Aug 24, 2023 at 7:43 AM EEST, Joe Perches wrote:
> On Wed, 2023-08-23 at 21:24 +0200, Paul Menzel wrote:
> > [Cc: +Andy, +Joe]
> > 
> > 
> > Dear Jarkko, dear Andy, dear Joe,
> > 
> > 
> > Am 23.08.23 um 19:40 schrieb Jarkko Sakkinen:
> > > On Wed Aug 23, 2023 at 11:23 AM EEST, Paul Menzel wrote:
> > 
> > > > Am 23.08.23 um 01:15 schrieb 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.
> > > > > 
> > > > > Cc: stable@vger.kernel.org
> > > > > Fixes: 554b841d4703 ("tpm: Disable RNG for all AMD fTPMs")
> > > > > Reported-by: Todd Brandt <todd.e.brandt@intel.com>
> > > > > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217804
> > > > > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> > > > 
> > > > Mario’s patch also had the three reporters below listed:
> > > > 
> > > > Reported-by: Patrick Steinhardt <ps@pks.im>
> > > > Reported-by: Ronan Pigott <ronan@rjp.ie>
> > > > Reported-by: Raymond Jay Golo <rjgolo@gmail.com>
> > > 
> > > 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 <ps@pks.im>
> > > Reported-by: Ronan Pigott <ronan@rjp.ie>
> > > 
> > > WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report
> > > #20:
> > > Reported-by: Ronan Pigott <ronan@rjp.ie>
> > > Reported-by: Raymond Jay Golo <rjgolo@gmail.com>
> > > 
> > > WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report
> > > #21:
> > > Reported-by: Raymond Jay Golo <rjgolo@gmail.com>
> > > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> > > 
> > > 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
> > 
> > 
> > > 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(+)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 528f619520eb9..5b5c11ad04087 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -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

Re: checkpatch complains about Reported-by block (was: [PATCH v3] tpm: Enable hwrng only for Pluton on AMD CPUs)

[Jarkko Sakkinen]

On Wed Aug 23, 2023 at 10:24 PM EEST, Paul Menzel wrote:
> [Cc: +Andy, +Joe]
>
>
> Dear Jarkko, dear Andy, dear Joe,
>
>
> Am 23.08.23 um 19:40 schrieb Jarkko Sakkinen:
> > On Wed Aug 23, 2023 at 11:23 AM EEST, Paul Menzel wrote:
>
> >> Am 23.08.23 um 01:15 schrieb 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.
> >>>
> >>> Cc: stable@vger.kernel.org
> >>> Fixes: 554b841d4703 ("tpm: Disable RNG for all AMD fTPMs")
> >>> Reported-by: Todd Brandt <todd.e.brandt@intel.com>
> >>> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217804
> >>> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> >>
> >> Mario’s patch also had the three reporters below listed:
> >>
> >> Reported-by: Patrick Steinhardt <ps@pks.im>
> >> Reported-by: Ronan Pigott <ronan@rjp.ie>
> >> Reported-by: Raymond Jay Golo <rjgolo@gmail.com>
> > 
> > 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 <ps@pks.im>
> > Reported-by: Ronan Pigott <ronan@rjp.ie>
> > 
> > WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report
> > #20:
> > Reported-by: Ronan Pigott <ronan@rjp.ie>
> > Reported-by: Raymond Jay Golo <rjgolo@gmail.com>
> > 
> > WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report
> > #21:
> > Reported-by: Raymond Jay Golo <rjgolo@gmail.com>
> > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> > 
> > 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?

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

Re: [PATCH v3] tpm: Enable hwrng only for Pluton on AMD CPUs

[Jerry Snitselaar]

On Wed, Aug 23, 2023 at 02:15:10AM +0300, Jarkko Sakkinen wrote:
> 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.
> 
> Cc: stable@vger.kernel.org
> Fixes: 554b841d4703 ("tpm: Disable RNG for all AMD fTPMs")
> Reported-by: Todd Brandt <todd.e.brandt@intel.com>
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217804
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> ---
> v3:
> * Forgot to amend config flags.
> v2:
> * CONFIG_X86
> * Removed "Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>"
> * Removed "Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>"
> ---
>  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.


> diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
> index 65ff4d2fbe8d..ea085b14ab7c 100644
> --- a/drivers/char/tpm/tpm_crb.c
> +++ b/drivers/char/tpm/tpm_crb.c
> @@ -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);
>  
> -- 
> 2.39.2
> 

Re: [PATCH v3] tpm: Enable hwrng only for Pluton on AMD CPUs

[Jerry Snitselaar]

On Wed, Aug 23, 2023 at 02:15:10AM +0300, Jarkko Sakkinen wrote:
> 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.
> 
> Cc: stable@vger.kernel.org
> Fixes: 554b841d4703 ("tpm: Disable RNG for all AMD fTPMs")
> Reported-by: Todd Brandt <todd.e.brandt@intel.com>
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217804
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> ---
> v3:
> * Forgot to amend config flags.
> v2:
> * CONFIG_X86
> * Removed "Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>"
> * Removed "Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>"
> ---
>  drivers/char/tpm/tpm_crb.c | 33 ++++++++-------------------------
>  1 file changed, 8 insertions(+), 25 deletions(-)
> 

Reviewed-by: Jerry Snitselaar <jsnitsel@redhat.com>

> diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
> index 65ff4d2fbe8d..ea085b14ab7c 100644
> --- a/drivers/char/tpm/tpm_crb.c
> +++ b/drivers/char/tpm/tpm_crb.c
> @@ -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);
>  
> -- 
> 2.39.2
> 

Re: [PATCH v3] tpm: Enable hwrng only for Pluton on AMD CPUs

[Jerry Snitselaar]

> On Aug 29, 2023, at 12:03 PM, Jerry Snitselaar <jsnitsel@redhat.com> wrote:
> 
> On Wed, Aug 23, 2023 at 02:15:10AM +0300, Jarkko Sakkinen wrote:
>> 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.
>> 
>> Cc: stable@vger.kernel.org
>> Fixes: 554b841d4703 ("tpm: Disable RNG for all AMD fTPMs")
>> Reported-by: Todd Brandt <todd.e.brandt@intel.com>
>> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217804
>> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
>> ---
>> v3:
>> * Forgot to amend config flags.
>> v2:
>> * CONFIG_X86
>> * Removed "Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>"
>> * Removed "Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>"
>> ---
>> drivers/char/tpm/tpm_crb.c | 33 ++++++++-------------------------
>> 1 file changed, 8 insertions(+), 25 deletions(-)
>> 
> 
> Reviewed-by: Jerry Snitselaar <jsnitsel@redhat.com>


It looks like the Fedora folks are getting more reports of the issue.

Re: [PATCH v3] tpm: Enable hwrng only for Pluton on AMD CPUs

[Jarkko Sakkinen]

On Wed Aug 30, 2023 at 9:25 PM EEST, Jerry Snitselaar wrote:
>
>
> > On Aug 29, 2023, at 12:03 PM, Jerry Snitselaar <jsnitsel@redhat.com> wrote:
> > 
> > On Wed, Aug 23, 2023 at 02:15:10AM +0300, Jarkko Sakkinen wrote:
> >> 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.
> >> 
> >> Cc: stable@vger.kernel.org
> >> Fixes: 554b841d4703 ("tpm: Disable RNG for all AMD fTPMs")
> >> Reported-by: Todd Brandt <todd.e.brandt@intel.com>
> >> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217804
> >> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> >> ---
> >> v3:
> >> * Forgot to amend config flags.
> >> v2:
> >> * CONFIG_X86
> >> * Removed "Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>"
> >> * Removed "Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>"
> >> ---
> >> drivers/char/tpm/tpm_crb.c | 33 ++++++++-------------------------
> >> 1 file changed, 8 insertions(+), 25 deletions(-)
> >> 
> > 
> > Reviewed-by: Jerry Snitselaar <jsnitsel@redhat.com>
>
>
> It looks like the Fedora folks are getting more reports of the issue.

https://lore.kernel.org/linux-integrity/20230904202512.29825-1-jarkko@kernel.org/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

[PATCH v4] tpm: Enable hwrng only for Pluton on AMD CPUs

[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 on Lenovo Legion Y540 laptop.

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.

Cc: stable@vger.kernel.org
Fixes: 554b841d4703 ("tpm: Disable RNG for all AMD fTPMs")
Reported-by: Todd Brandt <todd.e.brandt@intel.com>
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217804
Reported-by: Patrick Steinhardt <ps@pks.im>
Reported-by: Raymond Jay Golo <rjgolo@gmail.com>
Reported-by: Ronan Pigott <ronan@rjp.ie>
Reviewed-by: Jerry Snitselaar <jsnitsel@redhat.com>
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
v4:
* Report the victim: Lenovo Legion Y540
* Add reported-by's back due popular request.
v3:
* Forgot to amend config flags.
v2:
* CONFIG_X86
* Removed "Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>"
* Removed "Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>"
---
 drivers/char/tpm/tpm_crb.c | 33 ++++++++-------------------------
 1 file changed, 8 insertions(+), 25 deletions(-)

diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index 65ff4d2fbe8d..ea085b14ab7c 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -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);
 
-- 
2.39.2