[PATCH v2 1/1] tpm: clear state post probing

[Heinrich Schuchardt]

On 11/17/21 20:30, Simon Glass wrote:
> Hi Heinrich,
> 
> On Wed, 17 Nov 2021 at 12:28, Heinrich Schuchardt
> <heinrich.schuchardt at canonical.com> wrote:
>>
>>
>>
>> On 11/17/21 20:22, Ilias Apalodimas wrote:
>>> Hi Simon,
>>>
>>>
>>> On Wed, 17 Nov 2021 at 19:20, Simon Glass <sjg at chromium.org> wrote:
>>>>
>>>> Hi,
>>>>
>>>> (replying to both of you)
>>>>
>>>> On Wed, 17 Nov 2021 at 01:18, Ilias Apalodimas
>>>> <ilias.apalodimas at linaro.org> wrote:
>>>>>
>>>>> Hi Simon,
>>>>>
>>>>>
>>>>> On Wed, 17 Nov 2021 at 04:48, Simon Glass <sjg at chromium.org> wrote:
>>>>>>
>>>>>> Hi Heinrich,
>>>>>>
>>>>>> On Tue, 16 Nov 2021 at 04:08, Ilias Apalodimas
>>>>>> <ilias.apalodimas at linaro.org> wrote:
>>>>>>>
>>>>>>> On Mon, Nov 15, 2021 at 08:30:06PM +0100, Heinrich Schuchardt wrote:
>>>>>>>> Before we can start measuring the TPM must be cleared. Do this in the
>>>>>>>> post_probe() method of the uclass.
>>>>>>>>
>>>>>>>> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt at canonical.com>
>>>>>>>> ---
>>>>>>>> v2:
>>>>>>>>         tpm_startup2() is not available on all boards.
>>>>>>>>         tpm_startup() takes care of translating the call.
>>>>>>>> ---
>>>>>>>>    drivers/tpm/tpm-uclass.c | 13 +++++++++++++
>>>>>>>>    1 file changed, 13 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/drivers/tpm/tpm-uclass.c b/drivers/tpm/tpm-uclass.c
>>>>>>>> index f67fe1019b..abd9ce35e8 100644
>>>>>>>> --- a/drivers/tpm/tpm-uclass.c
>>>>>>>> +++ b/drivers/tpm/tpm-uclass.c
>>>>>>>> @@ -11,6 +11,7 @@
>>>>>>>>    #include <log.h>
>>>>>>>>    #include <linux/delay.h>
>>>>>>>>    #include <linux/unaligned/be_byteshift.h>
>>>>>>>> +#include <tpm_api.h>
>>>>>>>>    #include <tpm-v1.h>
>>>>>>>>    #include <tpm-v2.h>
>>>>>>>>    #include "tpm_internal.h"
>>>>>>>> @@ -136,6 +137,17 @@ int tpm_xfer(struct udevice *dev, const uint8_t *sendbuf, size_t send_size,
>>>>>>>>         return 0;
>>>>>>>>    }
>>>>>>>>
>>>>>>>> +static int dm_tpm_post_probe(struct udevice *dev)
>>>>>>
>>>>>> Please drop the dm_
>>>>>>
>>>>>>>> +{
>>>>>>>> +     /*
>>>>>>>> +      * Clearing the TPM state is only possible once after a hard reset.
>>>>>>>> +      * As we do not know if the TPM has been cleared by a prior boot stage
>>>>>>>> +      * ignore the return value here.
>>>>>>>> +      */
>>>>>>>> +     tpm_startup(dev, TPM_ST_CLEAR);
>>>>>>
>>>>>> blank line before final return
>>>>>>
>>>>>>>> +     return 0;
>>>>>>>> +}
>>>>>>
>>>>>> This should only happen once and if the TPM is set up in SPL then this
>>>>>> seems to cause a failure if done again.
>>>>>>
>>>>>
>>>>> Not really. If you run the tpm_startup twice and the TPM is already
>>>>> initialized you'll get TPM2_RC_INITIALIZE back. That's an 'error' you
>>>>> can easily check against and decide.
>>>>>
>>>>>> Is there another way we can deal with this? Could the TPM user decide
>>>>>> whether it needs to be set?
>>>>>
>>>>> Why? Part of the TPM init is making it usable.  We are trying to move
>>>>> away from having to add something in the command line to make the
>>>>> device usable.
>>>>
>>>> For a start, we should not start up the TPM until we need it. That
>>>> goes against the U-Boot lazy-init approach. We already have a command
>>>> to do it, as well as a TPM-API thing, so let's use that.
>>
>> The TPM is started exactly when we need it. When the user executes
>> bootefi the TPM is probed if the TCG2 protocol is enabled because we
>> need to measure binaries. Probing always has initialized TPMs. With this
>> patch we only additionally clear it.
>>
>> This patch does not contradict lazy initialization.
> 
> I think it does, because you are calling a TPM operation in the
> probe() method. Apart from the double-init problem you are creating, I
> may not want to start the TPM at this point.
> 

A TPM is nothing that is 'started'. It is a coprocessor that is running 
once you switch on the board.

If the TPM is probed, you have executed a command that makes use of the 
TPM. Why should it not be cleared?

Best regards

Heinrich