Re: [PATCH] tpm: Start the tpm2 before running a self test.

[Angga]

On 23/11/2023 1:34 am, Stefan Berger wrote:
>
>
> On 11/22/23 01:55, Hermin Anggawijaya wrote:
>> Before sending a command to attempt the self test, the TPM
>> may need to be started, otherwise the self test returns
>> TPM2_RC_INITIALIZE value causing a log as follows:
>> "tpm tpm0: A TPM error (256) occurred attempting the self test".
>>
>> Signed-off-by: Hermin Anggawijaya 
>> <hermin.anggawijaya@xxxxxxxxxxxxxxxxxxx>
>> ---
>>   drivers/char/tpm/tpm2-cmd.c | 8 ++++----
>>   1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
>> index 93545be190a5..0530f3b5f86a 100644
>> --- a/drivers/char/tpm/tpm2-cmd.c
>> +++ b/drivers/char/tpm/tpm2-cmd.c
>> @@ -737,15 +737,15 @@ int tpm2_auto_startup(struct tpm_chip *chip)
>>       if (rc)
>>           goto out;
>>   +    rc = tpm2_startup(chip);
>> +    if (rc && rc != TPM2_RC_INITIALIZE)
>> +        goto out;
>> +
>
> Most platforms should have firmware initialize the TPM 2 these days. 
> Therefore, a selftest should work and in case it doesn't work you fall 
> back to the tpm2_startup below and if you get an error message in the 
> log you at least know that you firmware is not up-to-date.
>
>>       rc = tpm2_do_selftest(chip);
>>       if (rc && rc != TPM2_RC_INITIALIZE)
>>           goto out;
>>         if (rc == TPM2_RC_INITIALIZE) {
>> -        rc = tpm2_startup(chip);
>> -        if (rc)
>> -            goto out;
>> -
>>           rc = tpm2_do_selftest(chip);
>>           if (rc)
>>               goto out;

Hello Stefan

Thank you for your comments.

Unfortunately our platforms (custom hardware design) are the ones which 
do not initialize/start the TPM2 from boot loader yet, and because of 
that the
self test in tpm2_auto_startup always produce a log error message on the 
platform start up.

While I understand your point about the log being useful for "pointing 
out not up-to-date firmware", but it might also generate unnecessary support
queries from some users on such platforms ? And maybe the kernel being 
able to deal with TPM being started more than once is better ?

If wanted, I have the second version of the patch which consist of code 
changes as in v1, plus ability for tpm2_transmit_cmd to handle multiple
attempts to start up the TPM silently, for example, once by the firmware 
and another by the kernel during tpm2 auto-startup.

Kind regards

Hermin Anggawijaya