[PATCH] tpm: Add TPM2_GetTestResult command support

[Simon Glass]

Hi Julia,

On Wed, 5 Jul 2023 at 00:55, Julia Daxenberger
<julia.daxenberger at infineon.com> wrote:
>
> Hi Simon,
>
> On Mon, 3 Jul 2023 at 15:31, Simon Glass <sjg at chromium.org> wrote:
> >
> > Hi Julia,
> >
> > On Mon, 3 Jul 2023 at 14:03, Julia Daxenberger
> > <julia.daxenberger at 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.daxenberger at 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 <sjg at 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.raynal at 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)
> > > +
> > > + at 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