Re: [PATCH] tpm: Make 'tpm init' to call tpm_auto_start()

[Ilias Apalodimas <ilias.apalodimas@linaro.org>]

Hi Simon,

On Wed, May 31, 2023 at 09:28:04PM -0600, Simon Glass wrote:
> Hi Ilias,
>
> On Tue, 30 May 2023 at 00:18, Ilias Apalodimas
> <ilias.apalodimas@linaro.org> wrote:
> >
> > For a TPM device to be operational we need to initialize it and
> > perform its startup sequence.  The 'tpm init' command currently calls
> > tpm_init() which ends up calling the ->open() per-device callback and
> > performs the initial hardware configuration as well as requesting
> > locality 0 for the caller.  There no code that currently calls
> > tpm_init() without following up with a tpm_startup() and tpm_self_test_full()
> > or tpm_continue_self_test().
> >
> > So let's wire up the 'tpm init' command and call tpm_auto_start() which
> > leaves the device in an operational state and adjust any defconfigs
> > using 'tpm init'.
> >
> > It's worth noting that calling tpm_init() only, doesn't allow a someone
> > to use the TPM since the startup sequence is mandatory. We always
> > repeat the pattern of calling
> > - tpm_init()
> > - tpm_startup()
> > - tpm_self_test_full() or tpm_continue_self_test()
> > as a result we don't expect any regression or boot delays with the current
> > change.
> >
> > While at it fix the identation of test_tpm_autostart() comments as well.
> >
> > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > ---
> >
> > This is a split and resend of https://lore.kernel.org/u-boot/20230510074359.2837818-9-ilias.apalodimas@linaro.org/
> > Since Simon some had concerns I decided to split this off the series and send it
> > as a single patch for further discussion.
> >
> >  cmd/tpm-common.c                   | 3 ++-
> >  configs/chromebook_coral_defconfig | 2 +-
> >  test/dm/tpm.c                      | 9 +++++----
> >  test/py/tests/test_tpm2.py         | 9 ---------
> >  4 files changed, 8 insertions(+), 15 deletions(-)
>
> We've already discussed this before. Please can you:
>
> - Call tpm_autostart() instead
> - Add a new 'tpm autostart' command
>
> That way we can keep tpm_init() as it is.
>

We have discussed but I am not convinced on why we should ever keep
'tpm init' as is.  You mentioned the ability to not always boot with
Startup(Clear) but that makes little sense to be in a u-boot command.
If we ever want to resume a tpm we should do that automatically.

In any case the spec itself mentions that the _init function should put the
TPM in a state were the next command *must* be a startup one,  so I don't
mind adding a 'tpm autostart'.

Regards
/Ilias

> Regards,
> Simon