[PATCH] tpm: Add TPM2_GetTestResult command support

[Simon Glass]

Hi Julia,

On Wed, 2 Aug 2023 at 02:28, Julia Daxenberger
<julia.daxenberger at infineon.com> wrote:
>
> Hi Simon,
>
> On Thu, 6 Jul 2023 at 09:57, Simon Glass <sjg at chromium.org> wrote:
> >
> > 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?
> > >
> > > 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().
>
> After looking at the code again, I agree that rebooting sandbox at the
> beginning of every test is not optimal. Currently, I do not have an
> in-depth understanding of the needed changes and their full impact.
> Taking into account the current changes to tpm functions, such as the
> new startup sequence, I would prefer to retain the restart for now.
>
> I will send a new version of the patch shortly, where I will move the
> tests relying on the restart into an additional patch, to be merged
> independently.

OK that sounds good, thank you. With that final patch, I should be
able to figure out what is going wrong, so it is not needed.

Regards,
Simon