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

[Ilias Apalodimas]

On Wed, 10 May 2023 at 17:32, Simon Glass <sjg at chromium.org> wrote:
>
> Hi Ilias,
>
> On Wed, 10 May 2023 at 01:44, Ilias Apalodimas
> <ilias.apalodimas at 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.   We recently added tpm_auto_start() though,
> > which automates the initialization process -- On top of that calling
> > tpm_init() on selftests is a bit problematic,  since calling it twice
> > will return -EBUSY the second time although there is no actual problem
> > with the TPM or the software stack.
> >
> > So let's wire up the 'tpm init' command and call tpm_auto_start() which
> > leaves the device in an operational state.
> >
> > 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 keep
> > repeating the pattern of calling
> > - tpm_init
> > - tpm_startup
> > - tpm_self_test_full or tpm_continue_self_test
> >
> > So 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 at linaro.org>
> > ---
> >  cmd/tpm-common.c | 3 ++-
> >  test/dm/tpm.c    | 9 +++++----
> >  2 files changed, 7 insertions(+), 5 deletions(-)
>
> We've been through this before. I do understand that EFI just does
> everything in U-Boot proper, but it is better for previous phases to
> set up the TPM, e.g. VPL, as we discussed on irc. In that case we
> cannot init the TPM twice.

Why can't we? Nothing bad happens to the device and the auto start
function takes that into account and doesn't run tpm2_startup() twice
if it's already initialized.

>
> I think what you want is a new 'tpm autostart' command, or something
> like that? You already have the tpm_auto_start() function so you can
> call that as needed.

I don't like having many confusing ways of starting the TPM.  To me
'init' means, initialize the device so I can use it.  Our code right
now needs 4 extra commands to happen which is confusing at best.  Do
you have any measurements that running auto start twice adds
substantial overhead?  Not to mention that tpm_init() returns 2
different error codes even if no errors are there.  Half oof our code
just ignores the return code of tpm_init due to that.  So my plan is
to get rid of it eventually and only have one sane way of starting the
device

Thanks
/Ilias
>
> Regards,
> Simon