[PATCH v2] tpm: Don't make vendor check required for probe

[PATCH v2] tpm: Don't make vendor check required for probe

[Mario Limonciello]

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.

As this isn't crucial for anything but AMD fTPM and AMD fTPM works, check
the chip vendor and if it's not AMD don't run the checks.

Cc: stable@vger.kernel.org
Fixes: 554b841d4703 ("tpm: Disable RNG for all AMD fTPMs")
Reported-by: Todd Brandt <todd.e.brandt@intel.com>
Reported-by: Patrick Steinhardt <ps@pks.im>
Reported-by: Ronan Pigott <ronan@rjp.ie>
Reported-by: Raymond Jay Golo <rjgolo@gmail.com>
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217804
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
v1->v2:
 * Check x86 vendor for AMD
---
 drivers/char/tpm/tpm_crb.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index 9eb1a18590123..7faf670201ccc 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -465,8 +465,12 @@ static bool crb_req_canceled(struct tpm_chip *chip, u8 status)
 
 static int crb_check_flags(struct tpm_chip *chip)
 {
+	int ret = 0;
+#ifdef CONFIG_X86
 	u32 val;
-	int ret;
+
+	if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD)
+		return ret;
 
 	ret = crb_request_locality(chip, 0);
 	if (ret)
@@ -481,6 +485,7 @@ static int crb_check_flags(struct tpm_chip *chip)
 
 release:
 	crb_relinquish_locality(chip, 0);
+#endif
 
 	return ret;
 }
-- 
2.34.1

Re: [PATCH v2] tpm: Don't make vendor check required for probe

[Ronan Pigott]

Hi Mario,

Thanks for the patch. I applied v2 to linux v6.4.11 and it resolves the
issue on my workstation with the intel ADL fTPM.

Tested-By: Ronan Pigott <ronan@rjp.ie>

Cheers,
Ronan

Re: [PATCH v2] tpm: Don't make vendor check required for probe

[Raymond Jay Golo]

> 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.
> 
> As this isn't crucial for anything but AMD fTPM and AMD fTPM works, check
> the chip vendor and if it's not AMD don't run the checks.
> 
> Cc: stable@vger.kernel.org
> Fixes: 554b841d4703 ("tpm: Disable RNG for all AMD fTPMs")
> Reported-by: Todd Brandt <todd.e.brandt@intel.com>
> Reported-by: Patrick Steinhardt <ps@pks.im>
> Reported-by: Ronan Pigott <ronan@rjp.ie>
> Reported-by: Raymond Jay Golo <rjgolo@gmail.com>
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217804
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> v1->v2:
>  * Check x86 vendor for AMD
> ---
>  drivers/char/tpm/tpm_crb.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
> index 9eb1a18590123..7faf670201ccc 100644
> --- a/drivers/char/tpm/tpm_crb.c
> +++ b/drivers/char/tpm/tpm_crb.c
> @@ -465,8 +465,12 @@ static bool crb_req_canceled(struct tpm_chip *chip, u8 status)
>  
>  static int crb_check_flags(struct tpm_chip *chip)
>  {
> +	int ret = 0;
> +#ifdef CONFIG_X86
>  	u32 val;
> -	int ret;
> +
> +	if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD)
> +		return ret;
>  
>  	ret = crb_request_locality(chip, 0);
>  	if (ret)
> @@ -481,6 +485,7 @@ static int crb_check_flags(struct tpm_chip *chip)
>  
>  release:
>  	crb_relinquish_locality(chip, 0);
> +#endif
>  
>  	return ret;
>  }
> -- 
> 2.34.1

Fixes problem on my machine.

Tested-by: Raymond Jay Golo <rjgolo@gmail.com>

Re: [PATCH v2] tpm: Don't make vendor check required for probe

[Jarkko Sakkinen]

On Mon Aug 21, 2023 at 5:02 PM EEST, Mario Limonciello 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.
>
> As this isn't crucial for anything but AMD fTPM and AMD fTPM works, check
> the chip vendor and if it's not AMD don't run the checks.
>
> Cc: stable@vger.kernel.org
> Fixes: 554b841d4703 ("tpm: Disable RNG for all AMD fTPMs")
> Reported-by: Todd Brandt <todd.e.brandt@intel.com>
> Reported-by: Patrick Steinhardt <ps@pks.im>
> Reported-by: Ronan Pigott <ronan@rjp.ie>
> Reported-by: Raymond Jay Golo <rjgolo@gmail.com>
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217804
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> v1->v2:
>  * Check x86 vendor for AMD
> ---
>  drivers/char/tpm/tpm_crb.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
> index 9eb1a18590123..7faf670201ccc 100644
> --- a/drivers/char/tpm/tpm_crb.c
> +++ b/drivers/char/tpm/tpm_crb.c
> @@ -465,8 +465,12 @@ static bool crb_req_canceled(struct tpm_chip *chip, u8 status)
>  
>  static int crb_check_flags(struct tpm_chip *chip)
>  {
> +	int ret = 0;
> +#ifdef CONFIG_X86
>  	u32 val;
> -	int ret;
> +
> +	if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD)
> +		return ret;
>  
>  	ret = crb_request_locality(chip, 0);
>  	if (ret)
> @@ -481,6 +485,7 @@ static int crb_check_flags(struct tpm_chip *chip)
>  
>  release:
>  	crb_relinquish_locality(chip, 0);
> +#endif

Looks much better but the main question here is that is this combination
possible:

1. AMD CPU
2. Non-AMD fTPM (i.e. manufacturer property differs)

BR, Jarkko

Re: [PATCH v2] tpm: Don't make vendor check required for probe

[Mario Limonciello]

On 8/22/2023 08:22, Jarkko Sakkinen wrote:
> On Mon Aug 21, 2023 at 5:02 PM EEST, Mario Limonciello 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.
>>
>> As this isn't crucial for anything but AMD fTPM and AMD fTPM works, check
>> the chip vendor and if it's not AMD don't run the checks.
>>
>> Cc: stable@vger.kernel.org
>> Fixes: 554b841d4703 ("tpm: Disable RNG for all AMD fTPMs")
>> Reported-by: Todd Brandt <todd.e.brandt@intel.com>
>> Reported-by: Patrick Steinhardt <ps@pks.im>
>> Reported-by: Ronan Pigott <ronan@rjp.ie>
>> Reported-by: Raymond Jay Golo <rjgolo@gmail.com>
>> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217804
>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>> ---
>> v1->v2:
>>   * Check x86 vendor for AMD
>> ---
>>   drivers/char/tpm/tpm_crb.c | 7 ++++++-
>>   1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
>> index 9eb1a18590123..7faf670201ccc 100644
>> --- a/drivers/char/tpm/tpm_crb.c
>> +++ b/drivers/char/tpm/tpm_crb.c
>> @@ -465,8 +465,12 @@ static bool crb_req_canceled(struct tpm_chip *chip, u8 status)
>>   
>>   static int crb_check_flags(struct tpm_chip *chip)
>>   {
>> +	int ret = 0;
>> +#ifdef CONFIG_X86
>>   	u32 val;
>> -	int ret;
>> +
>> +	if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD)
>> +		return ret;
>>   
>>   	ret = crb_request_locality(chip, 0);
>>   	if (ret)
>> @@ -481,6 +485,7 @@ static int crb_check_flags(struct tpm_chip *chip)
>>   
>>   release:
>>   	crb_relinquish_locality(chip, 0);
>> +#endif
> 
> Looks much better but the main question here is that is this combination
> possible:
> 
> 1. AMD CPU
> 2. Non-AMD fTPM (i.e. manufacturer property differs)
> 
> BR, Jarkko

Yes that combination is possible.

Pluton TPM uses the tpm_crb driver.

Re: [PATCH v2] tpm: Don't make vendor check required for probe

[Jarkko Sakkinen]

On Tue Aug 22, 2023 at 5:05 PM EEST, Mario Limonciello wrote:
> On 8/22/2023 08:22, Jarkko Sakkinen wrote:
> > On Mon Aug 21, 2023 at 5:02 PM EEST, Mario Limonciello 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.
> >>
> >> As this isn't crucial for anything but AMD fTPM and AMD fTPM works, check
> >> the chip vendor and if it's not AMD don't run the checks.
> >>
> >> Cc: stable@vger.kernel.org
> >> Fixes: 554b841d4703 ("tpm: Disable RNG for all AMD fTPMs")
> >> Reported-by: Todd Brandt <todd.e.brandt@intel.com>
> >> Reported-by: Patrick Steinhardt <ps@pks.im>
> >> Reported-by: Ronan Pigott <ronan@rjp.ie>
> >> Reported-by: Raymond Jay Golo <rjgolo@gmail.com>
> >> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217804
> >> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> >> ---
> >> v1->v2:
> >>   * Check x86 vendor for AMD
> >> ---
> >>   drivers/char/tpm/tpm_crb.c | 7 ++++++-
> >>   1 file changed, 6 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
> >> index 9eb1a18590123..7faf670201ccc 100644
> >> --- a/drivers/char/tpm/tpm_crb.c
> >> +++ b/drivers/char/tpm/tpm_crb.c
> >> @@ -465,8 +465,12 @@ static bool crb_req_canceled(struct tpm_chip *chip, u8 status)
> >>   
> >>   static int crb_check_flags(struct tpm_chip *chip)
> >>   {
> >> +	int ret = 0;
> >> +#ifdef CONFIG_X86
> >>   	u32 val;
> >> -	int ret;
> >> +
> >> +	if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD)
> >> +		return ret;
> >>   
> >>   	ret = crb_request_locality(chip, 0);
> >>   	if (ret)
> >> @@ -481,6 +485,7 @@ static int crb_check_flags(struct tpm_chip *chip)
> >>   
> >>   release:
> >>   	crb_relinquish_locality(chip, 0);
> >> +#endif
> > 
> > Looks much better but the main question here is that is this combination
> > possible:
> > 
> > 1. AMD CPU
> > 2. Non-AMD fTPM (i.e. manufacturer property differs)
> > 
> > BR, Jarkko
>
> Yes that combination is possible.
>
> Pluton TPM uses the tpm_crb driver.

Then I guess we'll go with this for now. Thanks for the effort.

Tested-by: Jarkko Sakkinen <jarkko@kernel.org> # QEMU + swtpm
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

I'm planning to send a pull request right after this with the fix so it
will land to v6.6-rc1 or v6.6-rc2:
https://lore.kernel.org/linux-integrity/20230817201935.31399-1-jarkko@kernel.org/

BR, Jarkko

Re: [PATCH v2] tpm: Don't make vendor check required for probe

[Jerry Snitselaar]

On Tue, Aug 22, 2023 at 05:56:03PM +0300, Jarkko Sakkinen wrote:
> On Tue Aug 22, 2023 at 5:05 PM EEST, Mario Limonciello wrote:
> > On 8/22/2023 08:22, Jarkko Sakkinen wrote:
> > > On Mon Aug 21, 2023 at 5:02 PM EEST, Mario Limonciello 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.
> > >>
> > >> As this isn't crucial for anything but AMD fTPM and AMD fTPM works, check
> > >> the chip vendor and if it's not AMD don't run the checks.
> > >>
> > >> Cc: stable@vger.kernel.org
> > >> Fixes: 554b841d4703 ("tpm: Disable RNG for all AMD fTPMs")
> > >> Reported-by: Todd Brandt <todd.e.brandt@intel.com>
> > >> Reported-by: Patrick Steinhardt <ps@pks.im>
> > >> Reported-by: Ronan Pigott <ronan@rjp.ie>
> > >> Reported-by: Raymond Jay Golo <rjgolo@gmail.com>
> > >> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217804
> > >> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> > >> ---
> > >> v1->v2:
> > >>   * Check x86 vendor for AMD
> > >> ---
> > >>   drivers/char/tpm/tpm_crb.c | 7 ++++++-
> > >>   1 file changed, 6 insertions(+), 1 deletion(-)
> > >>
> > >> diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
> > >> index 9eb1a18590123..7faf670201ccc 100644
> > >> --- a/drivers/char/tpm/tpm_crb.c
> > >> +++ b/drivers/char/tpm/tpm_crb.c
> > >> @@ -465,8 +465,12 @@ static bool crb_req_canceled(struct tpm_chip *chip, u8 status)
> > >>   
> > >>   static int crb_check_flags(struct tpm_chip *chip)
> > >>   {
> > >> +	int ret = 0;
> > >> +#ifdef CONFIG_X86
> > >>   	u32 val;
> > >> -	int ret;
> > >> +
> > >> +	if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD)
> > >> +		return ret;
> > >>   
> > >>   	ret = crb_request_locality(chip, 0);
> > >>   	if (ret)
> > >> @@ -481,6 +485,7 @@ static int crb_check_flags(struct tpm_chip *chip)
> > >>   
> > >>   release:
> > >>   	crb_relinquish_locality(chip, 0);
> > >> +#endif
> > > 
> > > Looks much better but the main question here is that is this combination
> > > possible:
> > > 
> > > 1. AMD CPU
> > > 2. Non-AMD fTPM (i.e. manufacturer property differs)
> > > 
> > > BR, Jarkko
> >
> > Yes that combination is possible.
> >
> > Pluton TPM uses the tpm_crb driver.
> 
> Then I guess we'll go with this for now. Thanks for the effort.
> 
> Tested-by: Jarkko Sakkinen <jarkko@kernel.org> # QEMU + swtpm
> Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> 
> I'm planning to send a pull request right after this with the fix so it
> will land to v6.6-rc1 or v6.6-rc2:
> https://lore.kernel.org/linux-integrity/20230817201935.31399-1-jarkko@kernel.org/
> 
> BR, Jarkko


Super minor nit that isn't this patch in particular so don't hold this
up, but it seems like the function name for the earlier attempt to
solve this issue that mentioned amd and ftpm was a clearer description
of what was happening than crb_check_flags.

Regards,
Jerry

Re: [PATCH v2] tpm: Don't make vendor check required for probe

[Jarkko Sakkinen]

On Tue Aug 22, 2023 at 10:50 PM EEST, Jerry Snitselaar wrote:
> On Tue, Aug 22, 2023 at 05:56:03PM +0300, Jarkko Sakkinen wrote:
> > On Tue Aug 22, 2023 at 5:05 PM EEST, Mario Limonciello wrote:
> > > On 8/22/2023 08:22, Jarkko Sakkinen wrote:
> > > > On Mon Aug 21, 2023 at 5:02 PM EEST, Mario Limonciello 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.
> > > >>
> > > >> As this isn't crucial for anything but AMD fTPM and AMD fTPM works, check
> > > >> the chip vendor and if it's not AMD don't run the checks.
> > > >>
> > > >> Cc: stable@vger.kernel.org
> > > >> Fixes: 554b841d4703 ("tpm: Disable RNG for all AMD fTPMs")
> > > >> Reported-by: Todd Brandt <todd.e.brandt@intel.com>
> > > >> Reported-by: Patrick Steinhardt <ps@pks.im>
> > > >> Reported-by: Ronan Pigott <ronan@rjp.ie>
> > > >> Reported-by: Raymond Jay Golo <rjgolo@gmail.com>
> > > >> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217804
> > > >> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> > > >> ---
> > > >> v1->v2:
> > > >>   * Check x86 vendor for AMD
> > > >> ---
> > > >>   drivers/char/tpm/tpm_crb.c | 7 ++++++-
> > > >>   1 file changed, 6 insertions(+), 1 deletion(-)
> > > >>
> > > >> diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
> > > >> index 9eb1a18590123..7faf670201ccc 100644
> > > >> --- a/drivers/char/tpm/tpm_crb.c
> > > >> +++ b/drivers/char/tpm/tpm_crb.c
> > > >> @@ -465,8 +465,12 @@ static bool crb_req_canceled(struct tpm_chip *chip, u8 status)
> > > >>   
> > > >>   static int crb_check_flags(struct tpm_chip *chip)
> > > >>   {
> > > >> +	int ret = 0;

Oops I missed this. This adds unnecessary clutter to the diff.
> > > >> +#ifdef CONFIG_X86 u32 val;
> > > >> -	int ret;
> > > >> +
> > > >> +	if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD)
> > > >> +		return ret;

No reason to use variable here. Should be just "return 0". It also
documents what is going on. Now this gives impression as the "ret"
could change.

I dropped the current version from my -next. I did not notice the
change in declarations earlier, sorry.

> > > >>   
> > > >>   	ret = crb_request_locality(chip, 0);
> > > >>   	if (ret)
> > > >> @@ -481,6 +485,7 @@ static int crb_check_flags(struct tpm_chip *chip)
> > > >>   
> > > >>   release:
> > > >>   	crb_relinquish_locality(chip, 0);
> > > >> +#endif
> > > > 
> > > > Looks much better but the main question here is that is this combination
> > > > possible:
> > > > 
> > > > 1. AMD CPU
> > > > 2. Non-AMD fTPM (i.e. manufacturer property differs)
> > > > 
> > > > BR, Jarkko
> > >
> > > Yes that combination is possible.
> > >
> > > Pluton TPM uses the tpm_crb driver.
> > 
> > Then I guess we'll go with this for now. Thanks for the effort.
> > 
> > Tested-by: Jarkko Sakkinen <jarkko@kernel.org> # QEMU + swtpm
> > Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> > 
> > I'm planning to send a pull request right after this with the fix so it
> > will land to v6.6-rc1 or v6.6-rc2:
> > https://lore.kernel.org/linux-integrity/20230817201935.31399-1-jarkko@kernel.org/
> > 
> > BR, Jarkko
>
>
> Super minor nit that isn't this patch in particular so don't hold this
> up, but it seems like the function name for the earlier attempt to
> solve this issue that mentioned amd and ftpm was a clearer description
> of what was happening than crb_check_flags.

I posted an alternative: https://lkml.org/lkml/2023/8/22/1188

BR, Jarkko

Re: [PATCH v2] tpm: Don't make vendor check required for probe

[Jarkko Sakkinen]

On Tue Aug 22, 2023 at 11:29 PM EEST, Jarkko Sakkinen wrote:
> On Tue Aug 22, 2023 at 10:50 PM EEST, Jerry Snitselaar wrote:
> > On Tue, Aug 22, 2023 at 05:56:03PM +0300, Jarkko Sakkinen wrote:
> > > On Tue Aug 22, 2023 at 5:05 PM EEST, Mario Limonciello wrote:
> > > > On 8/22/2023 08:22, Jarkko Sakkinen wrote:
> > > > > On Mon Aug 21, 2023 at 5:02 PM EEST, Mario Limonciello 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.
> > > > >>
> > > > >> As this isn't crucial for anything but AMD fTPM and AMD fTPM works, check
> > > > >> the chip vendor and if it's not AMD don't run the checks.
> > > > >>
> > > > >> Cc: stable@vger.kernel.org
> > > > >> Fixes: 554b841d4703 ("tpm: Disable RNG for all AMD fTPMs")
> > > > >> Reported-by: Todd Brandt <todd.e.brandt@intel.com>
> > > > >> Reported-by: Patrick Steinhardt <ps@pks.im>
> > > > >> Reported-by: Ronan Pigott <ronan@rjp.ie>
> > > > >> Reported-by: Raymond Jay Golo <rjgolo@gmail.com>
> > > > >> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217804
> > > > >> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> > > > >> ---
> > > > >> v1->v2:
> > > > >>   * Check x86 vendor for AMD
> > > > >> ---
> > > > >>   drivers/char/tpm/tpm_crb.c | 7 ++++++-
> > > > >>   1 file changed, 6 insertions(+), 1 deletion(-)
> > > > >>
> > > > >> diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
> > > > >> index 9eb1a18590123..7faf670201ccc 100644
> > > > >> --- a/drivers/char/tpm/tpm_crb.c
> > > > >> +++ b/drivers/char/tpm/tpm_crb.c
> > > > >> @@ -465,8 +465,12 @@ static bool crb_req_canceled(struct tpm_chip *chip, u8 status)
> > > > >>   
> > > > >>   static int crb_check_flags(struct tpm_chip *chip)
> > > > >>   {
> > > > >> +	int ret = 0;
>
> Oops I missed this. This adds unnecessary clutter to the diff.
> > > > >> +#ifdef CONFIG_X86 u32 val;
> > > > >> -	int ret;
> > > > >> +
> > > > >> +	if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD)
> > > > >> +		return ret;
>
> No reason to use variable here. Should be just "return 0". It also
> documents what is going on. Now this gives impression as the "ret"
> could change.
>
> I dropped the current version from my -next. I did not notice the
> change in declarations earlier, sorry.
>
> > > > >>   
> > > > >>   	ret = crb_request_locality(chip, 0);
> > > > >>   	if (ret)
> > > > >> @@ -481,6 +485,7 @@ static int crb_check_flags(struct tpm_chip *chip)
> > > > >>   
> > > > >>   release:
> > > > >>   	crb_relinquish_locality(chip, 0);
> > > > >> +#endif
> > > > > 
> > > > > Looks much better but the main question here is that is this combination
> > > > > possible:
> > > > > 
> > > > > 1. AMD CPU
> > > > > 2. Non-AMD fTPM (i.e. manufacturer property differs)
> > > > > 
> > > > > BR, Jarkko
> > > >
> > > > Yes that combination is possible.
> > > >
> > > > Pluton TPM uses the tpm_crb driver.
> > > 
> > > Then I guess we'll go with this for now. Thanks for the effort.
> > > 
> > > Tested-by: Jarkko Sakkinen <jarkko@kernel.org> # QEMU + swtpm
> > > Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> > > 
> > > I'm planning to send a pull request right after this with the fix so it
> > > will land to v6.6-rc1 or v6.6-rc2:
> > > https://lore.kernel.org/linux-integrity/20230817201935.31399-1-jarkko@kernel.org/
> > > 
> > > BR, Jarkko
> >
> >
> > Super minor nit that isn't this patch in particular so don't hold this
> > up, but it seems like the function name for the earlier attempt to
> > solve this issue that mentioned amd and ftpm was a clearer description
> > of what was happening than crb_check_flags.
>
> I posted an alternative: https://lkml.org/lkml/2023/8/22/1188

Ignore my reviewed-by (I cannot review my own patches) :-)

Also should be probably v6.4+ (should check when the fix landed)?

BR, Jarkko

Re: [PATCH v2] tpm: Don't make vendor check required for probe

[Jarkko Sakkinen]

On Tue Aug 22, 2023 at 11:32 PM EEST, Jarkko Sakkinen wrote:
> On Tue Aug 22, 2023 at 11:29 PM EEST, Jarkko Sakkinen wrote:
> > On Tue Aug 22, 2023 at 10:50 PM EEST, Jerry Snitselaar wrote:
> > > On Tue, Aug 22, 2023 at 05:56:03PM +0300, Jarkko Sakkinen wrote:
> > > > On Tue Aug 22, 2023 at 5:05 PM EEST, Mario Limonciello wrote:
> > > > > On 8/22/2023 08:22, Jarkko Sakkinen wrote:
> > > > > > On Mon Aug 21, 2023 at 5:02 PM EEST, Mario Limonciello 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.
> > > > > >>
> > > > > >> As this isn't crucial for anything but AMD fTPM and AMD fTPM works, check
> > > > > >> the chip vendor and if it's not AMD don't run the checks.
> > > > > >>
> > > > > >> Cc: stable@vger.kernel.org
> > > > > >> Fixes: 554b841d4703 ("tpm: Disable RNG for all AMD fTPMs")
> > > > > >> Reported-by: Todd Brandt <todd.e.brandt@intel.com>
> > > > > >> Reported-by: Patrick Steinhardt <ps@pks.im>
> > > > > >> Reported-by: Ronan Pigott <ronan@rjp.ie>
> > > > > >> Reported-by: Raymond Jay Golo <rjgolo@gmail.com>
> > > > > >> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217804
> > > > > >> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> > > > > >> ---
> > > > > >> v1->v2:
> > > > > >>   * Check x86 vendor for AMD
> > > > > >> ---
> > > > > >>   drivers/char/tpm/tpm_crb.c | 7 ++++++-
> > > > > >>   1 file changed, 6 insertions(+), 1 deletion(-)
> > > > > >>
> > > > > >> diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
> > > > > >> index 9eb1a18590123..7faf670201ccc 100644
> > > > > >> --- a/drivers/char/tpm/tpm_crb.c
> > > > > >> +++ b/drivers/char/tpm/tpm_crb.c
> > > > > >> @@ -465,8 +465,12 @@ static bool crb_req_canceled(struct tpm_chip *chip, u8 status)
> > > > > >>   
> > > > > >>   static int crb_check_flags(struct tpm_chip *chip)
> > > > > >>   {
> > > > > >> +	int ret = 0;
> >
> > Oops I missed this. This adds unnecessary clutter to the diff.
> > > > > >> +#ifdef CONFIG_X86 u32 val;
> > > > > >> -	int ret;
> > > > > >> +
> > > > > >> +	if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD)
> > > > > >> +		return ret;
> >
> > No reason to use variable here. Should be just "return 0". It also
> > documents what is going on. Now this gives impression as the "ret"
> > could change.
> >
> > I dropped the current version from my -next. I did not notice the
> > change in declarations earlier, sorry.
> >
> > > > > >>   
> > > > > >>   	ret = crb_request_locality(chip, 0);
> > > > > >>   	if (ret)
> > > > > >> @@ -481,6 +485,7 @@ static int crb_check_flags(struct tpm_chip *chip)
> > > > > >>   
> > > > > >>   release:
> > > > > >>   	crb_relinquish_locality(chip, 0);
> > > > > >> +#endif
> > > > > > 
> > > > > > Looks much better but the main question here is that is this combination
> > > > > > possible:
> > > > > > 
> > > > > > 1. AMD CPU
> > > > > > 2. Non-AMD fTPM (i.e. manufacturer property differs)
> > > > > > 
> > > > > > BR, Jarkko
> > > > >
> > > > > Yes that combination is possible.
> > > > >
> > > > > Pluton TPM uses the tpm_crb driver.
> > > > 
> > > > Then I guess we'll go with this for now. Thanks for the effort.
> > > > 
> > > > Tested-by: Jarkko Sakkinen <jarkko@kernel.org> # QEMU + swtpm
> > > > Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> > > > 
> > > > I'm planning to send a pull request right after this with the fix so it
> > > > will land to v6.6-rc1 or v6.6-rc2:
> > > > https://lore.kernel.org/linux-integrity/20230817201935.31399-1-jarkko@kernel.org/
> > > > 
> > > > BR, Jarkko
> > >
> > >
> > > Super minor nit that isn't this patch in particular so don't hold this
> > > up, but it seems like the function name for the earlier attempt to
> > > solve this issue that mentioned amd and ftpm was a clearer description
> > > of what was happening than crb_check_flags.
> >
> > I posted an alternative: https://lkml.org/lkml/2023/8/22/1188
>
> Ignore my reviewed-by (I cannot review my own patches) :-)
>
> Also should be probably v6.4+ (should check when the fix landed)?

v2: https://lkml.org/lkml/2023/8/22/1197

BR, Jarkko