Hi Simon
On Fri, Oct 27, 2023, 22:15 Simon Glass <s...@chromium.org> wrote:
> Hi Ilias,
>
> On Wed, 25 Oct 2023 at 18:23, Simon Glass <s...@chromium.org> wrote:
> >
> > Hi Ilias,
> >
> > On Wed, 25 Oct 2023 at 07:25, Ilias Apalodimas
> > <ilias.apalodi...@linaro.org> wrote:
> > >
> > > commit 789ed2784256 ("test/py: replace 'tpm2 init, startup, selftest'
> sequences")
> > > changed some of the tpm2 init sequences to 'tpm2 autostart' instead of
> > > calling 'tpm init', 'tpm startup TPM2_SU_CLEAR', 'tpm2 self_test full'.
> > >
> > > The autostart command calls the afforementioned sequence and on top of
> > > that deals with the 'tpm2 init' return codes if the tpm is already
> > > started. Since we initialize the tpm from various subsystems now,
> > > replace the last remaining instances of 'tpm2 init' with 'tpm2
> > > autostart'. Since the latter calls 'tpm2 init' anyway we will still be
> > > implicitly testing the validity of that command
> > >
> > > It's worth noting that since 'tpm2 autostart' performs the startup and
> > > self tests sequences of the tpm we could drop
> > > 'test_tpm2_sandbox_self_test_full' and 'test_tpm2_startup, but let's
> > > keep the since they test tpm commands and options
> > >
> > > Signed-off-by: Ilias Apalodimas <ilias.apalodi...@linaro.org>
> > > ---
> > > test/py/tests/test_tpm2.py | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/test/py/tests/test_tpm2.py b/test/py/tests/test_tpm2.py
> > > index 47392b87a98f..1d654cd4a23b 100644
> > > --- a/test/py/tests/test_tpm2.py
> > > +++ b/test/py/tests/test_tpm2.py
> > > @@ -61,7 +61,7 @@ def test_tpm2_init(u_boot_console):
> > > skip_test =
> u_boot_console.config.env.get('env__tpm_device_test_skip', False)
> > > if skip_test:
> > > pytest.skip('skip TPM device test')
> > > - u_boot_console.run_command('tpm2 init')
> > > + u_boot_console.run_command('tpm2 autostart')
> > > output = u_boot_console.run_command('echo $?')
> > > assert output.endswith('0')
> > >
> > > @@ -100,7 +100,7 @@ def
> test_tpm2_sandbox_self_test_full(u_boot_console):
> > > """
> > > if is_sandbox(u_boot_console):
> > > u_boot_console.restart_uboot()
> >
> > We should not need this. Can you please take a look at what reset is
> > needed in the TPM code to get the TPM back into its original state? We
> > have dm_test_pre_run() which could call a tpm_reset_for_test()
> > function, or similar.
> >
> > > - u_boot_console.run_command('tpm2 init')
> > > + u_boot_console.run_command('tpm2 autostart')
> > > output = u_boot_console.run_command('echo $?')
> > > assert output.endswith('0')
>
> Somehow I didn't see your reply come through in my email.
>
> There are two problems as I see it:
>
> tpm2_init() needs to get the 'tpm2 init' command. If that test can
> only pass on sandbox, then we should mark it as such, perhaps creating
> a different test for tpm_autostart
>
Ok. Please note that Tom pulled this along with the rest of the tpm series.
I'll revert it Monday and send a fix.
> The second chunk is definitely for sandbox only, so we should fix the
> reset_for_test stuff so that the TPM is inited before every test. That
> is what we do for the rest of the sandbox tests. If you are not sure
> or need me to dig into how to do that, let me know. But we should not
> paper over this...it has been a problem for long enough now, causing
> us to add hacks like restarting U-Boot, etc.
>
To do that we need to add a tpm_close. Should we do that or simply not fail
on EBUSY, which isn't actually an error?
Thanks
Ilias
>
> Regards,
> Simon
>
