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

[Heinrich Schuchardt]

On 11/17/21 03:48, Simon Glass 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_

sure

> 
>>> +{
>>> +     /*
>>> +      * 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

ok

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

It does not create a failure. The conforming TPM just ignores the 
TPM_ST_CLEAR and returns an error code.

> 
> Is there another way we can deal with this? Could the TPM user decide
> whether it needs to be set?
> 
> For the approach you have here, I think the best option might be to
> add a property to the devicetree. That way the prior stage can control
> it.

The idea of a prior stage adding something U-Boot specific sucks.

Best regards

Heinrich

> 
>>> +
>>>   UCLASS_DRIVER(tpm) = {
>>>        .id             = UCLASS_TPM,
>>>        .name           = "tpm",
>>> @@ -143,5 +155,6 @@ UCLASS_DRIVER(tpm) = {
>>>   #if CONFIG_IS_ENABLED(OF_REAL)
>>>        .post_bind      = dm_scan_fdt_dev,
>>>   #endif
>>> +     .post_probe     = dm_tpm_post_probe,
>>>        .per_device_auto        = sizeof(struct tpm_chip_priv),
>>>   };
>>> --
>>> 2.32.0
>>>
>>
>> Reviewed-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> 
> Regards,
> Simon
>