Hi Julia,
On Wed, 5 Jul 2023 at 00:55, Julia Daxenberger
<julia.daxenber...@infineon.com> wrote:
>
> Hi Simon,
>
> On Mon, 3 Jul 2023 at 15:31, Simon Glass <s...@chromium.org> wrote:
> >
> > Hi Julia,
> >
> > On Mon, 3 Jul 2023 at 14:03, Julia Daxenberger
> > <julia.daxenber...@infineon.com> wrote:
> > >
> > > Add TPM2_GetTestResult command support and change the command file and the
> > > help accordingly. Add Python tests and sandbox driver functionality.
> > >
> > > The TPM2_GetTestResult command is performed after the TPM2_SelfTest
> > > command
> > > and returns manufacturer-specific information regarding the results of the
> > > self-test and an indication of the test status.
> > >
> > > Signed-off-by: Julia Daxenberger <julia.daxenber...@infineon.com>
> > > ---
> > > cmd/tpm-v2.c | 60 +++++++++++++++++++++++++
> > > drivers/tpm/tpm2_tis_sandbox.c | 47 ++++++++++++++++++-
> > > include/tpm-v2.h | 23 ++++++++++
> > > lib/tpm-v2.c | 82 ++++++++++++++++++++++++++++++++++
> > > test/py/tests/test_tpm2.py | 50 +++++++++++++++++++++
> > > 5 files changed, 261 insertions(+), 1 deletion(-)
> > >
> >
> > Reviewed-by: Simon Glass <s...@chromium.org>
> >
> >
> > See below
> >
> > [..]
> >
> > > diff --git a/test/py/tests/test_tpm2.py b/test/py/tests/test_tpm2.py
> > > index d2ad6f9e73..aad1d7a55b 100644
> > > --- a/test/py/tests/test_tpm2.py
> > > +++ b/test/py/tests/test_tpm2.py
> > > @@ -1,6 +1,7 @@
> > > # SPDX-License-Identifier: GPL-2.0+
> > > # Copyright (c) 2018, Bootlin
> > > # Author: Miquel Raynal <miquel.ray...@bootlin.com>
> > > +# Copyright (C) 2023 Infineon Technologies AG
> > >
> > > import os.path
> > > import pytest
> > > @@ -316,3 +317,52 @@ def test_tpm2_cleanup(u_boot_console):
> > > """Ensure the TPM is cleared from password or test related
> > > configuration."""
> > >
> > > force_init(u_boot_console, True)
> > > +
> > > +@pytest.mark.buildconfigspec('cmd_tpm_v2')
> > > +def test_tpm2_get_test_result(u_boot_console):
> > > + """Execute a TPM_GetTestResult command.
> > > +
> > > + Ask the TPM to get the test result of the self test.
> > > + Display the Test Result and Test Result Data.
> > > +
> > > + Expected default return value of tpm2_get_test_result, if the TPM
> > > has not been initialized:
> > > + - TPM2_RC_INITIALIZE = TPM2_RC_VER1 + 0x0000 = 0x00000100.
> > > +
> > > + Expected default value for test_result:
> > > + - TPM_RC_NEEDS_TEST = 0x00000153, if tpm2 self_test has not been
> > > executed.
> > > + - TPM_RC_SUCCESS = 0x00000000, if testing is complete without
> > > functional failures.
> > > +
> > > + There is no expected default value for the test result data because
> > > it would depend on the chip
> > > + used. The test result data is therefore not tested.
> > > + """
> > > + if is_sandbox(u_boot_console):
> > > + u_boot_console.restart_uboot()
> >
> > We should get rid of this somehow. We don't want sandbox rebooting
> > inthe middle of a test. It makes debugging painful, apart from
> > anything else. What TPM state needs to be reset?
> >
> > Looking at tpm2_tis_sandbox.c it is probably the s_state variable. The
> > TPM state can be preserved across runs and is stored in the state
> > file.
> >
> > But if the state file is not being used (no -s argument) then the TPM
> > should be reset each time DM is brought back up, i.e. between every
> > test.
> >
> > So, do we even need this reset?
> >
> > Regards,
> > Simon
>
> In my understanding, shutting down and restarting U-Boot with
> restart_uboot() at the beginning of a test is beneficial, as it puts
> sandbox back into a known state for the test. This allows all tests to
> run in parallel, since no test depends on another.
Actually with sandbox, we just restart the driver model each time. You
can see this logic in test-main.c
>
> In the test_tpm2_get_test_result test specifically, the reset is needed
> to test the behaviour of the do_tpm2_get_test_result() function, when
> the tpm has not already been initialized and the TPM self_test has not
> been executed. Performing restart_uboot() is essential to test for error
> code output, as sandbox always returns "TPM2_RC_SUCCESS" when the
> restart is not performed, making the test fail.
>
> restart_uboot() is performed at the beginning of all tests in
> test_tpm2.py, either on its own, or as part of the tpm2_sandbox_init()
> command.
Yes, we should fix this. No other subsystem has this problem. I am
wondering what is actually going on? The tpm must be keeping state
outside of driver model. Perhaps we just need to call a TPM function
to clear that state, in state_reset_for_test().
Regards,
Simon
