[PATCH 1/2 v3] tpm: add a function that performs selftest + startup

[Simon Glass]

Hi Ilias,

On Thu, 26 Jan 2023 at 01:18, Ilias Apalodimas
<ilias.apalodimas at linaro.org> wrote:
>
> As described in [0] if a command requires use of an untested algorithm
> or functional module, the TPM performs the test and then completes the
> command actions.
>
> Since we don't check for TPM_RC_NEEDS_TEST (which is the return code of
> the TPM in that case) and even if we would, it would complicate our TPM
> code for no apparent reason,  add a wrapper function that performs both
> the selftest and the startup sequence of the TPM.
>
> It's worth noting that this is implemented on TPMv2.0.  The code for
> 1.2 would look similar,  but I don't have a device available to test.
>
> [0]
> https://trustedcomputinggroup.org/wp-content/uploads/TPM-Rev-2.0-Part-1-Architecture-01.07-2014-03-13.pdf
> §12.3 Self-test modes
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> ---
> Changes since v2:
> - add tpm_init() to auto start
>
> Changes since v1:
> - Remove a superfluous if statement
> - Move function comments to the header file
>  include/tpm-v2.h  | 19 +++++++++++++++++++
>  include/tpm_api.h |  8 ++++++++
>  lib/tpm-v2.c      | 24 ++++++++++++++++++++++++
>  lib/tpm_api.c     |  8 ++++++++
>  4 files changed, 59 insertions(+)

I think this is a good idea, but it should be implemented at the API
level. Please see below.

>
> diff --git a/include/tpm-v2.h b/include/tpm-v2.h
> index 737e57551d73..1c644f0048f6 100644
> --- a/include/tpm-v2.h
> +++ b/include/tpm-v2.h
> @@ -688,4 +688,23 @@ u32 tpm2_report_state(struct udevice *dev, uint vendor_cmd, uint vendor_subcmd,
>  u32 tpm2_enable_nvcommits(struct udevice *dev, uint vendor_cmd,
>                           uint vendor_subcmd);
>
> +/**
> + * tpm2_auto_start() - start up the TPM and perform selftests.
> + *                     If a testable function has not been tested and is
> + *                     requested the TPM2  will return TPM_RC_NEEDS_TEST.
> + *
> + *
> + *

drop extra lines

> + * @param dev          TPM device
> + * Return: TPM2_RC_TESTING, if TPM2 self-test has been received and the tests are

80 cols

> + *         not complete.
> + *         TPM2_RC_SUCCESS, if testing of all functions is complete without
> + *         functional failures.
> + *         TPM2_RC_FAILURE, if any test failed.
> + *         TPM2_RC_INITIALIZE, if the TPM has not gone through the Startup
> + *         sequence
> +
> + */
> +u32 tpm2_auto_start(struct udevice *dev);
> +
>  #endif /* __TPM_V2_H */
> diff --git a/include/tpm_api.h b/include/tpm_api.h
> index 8979d9d6df7e..022a8bbaeca6 100644
> --- a/include/tpm_api.h
> +++ b/include/tpm_api.h
> @@ -331,4 +331,12 @@ static inline bool tpm_is_v2(struct udevice *dev)
>         return IS_ENABLED(CONFIG_TPM_V2) && tpm_get_version(dev) == TPM_V2;
>  }
>
> +/**
> + * tpm_auto_start() - start up the TPM and perform selftests
> + *
> + * @param dev          TPM device
> + * Return: return code of the operation (0 = success)
> + */
> +u32 tpm_auto_start(struct udevice *dev);
> +
>  #endif /* __TPM_API_H */
> diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c
> index 697b982e079f..2141d58632ff 100644
> --- a/lib/tpm-v2.c
> +++ b/lib/tpm-v2.c
> @@ -44,6 +44,30 @@ u32 tpm2_self_test(struct udevice *dev, enum tpm2_yes_no full_test)
>         return tpm_sendrecv_command(dev, command_v2, NULL, NULL);
>  }
>
> +u32 tpm2_auto_start(struct udevice *dev)
> +{
> +       u32 rc;
> +
> +       /*
> +        * the tpm_init() will return -EBUSY if the init has already happened
> +        * The selftest and startup code can run multiple times with no side effects

80 cols

> +        */
> +       rc = tpm_init(dev);
> +       if (rc && rc != -EBUSY)

Does that work, with rc being unsigned?

> +               return rc;
> +       rc = tpm2_self_test(dev, TPMI_YES);

Can we call tpm_self_test_full() ? If not, please update the API as needed.

> +
> +       if (rc == TPM2_RC_INITIALIZE) {
> +               rc = tpm2_startup(dev, TPM2_SU_CLEAR);

Should call tpm_startup()

> +               if (rc)
> +                       return rc;
> +
> +               rc = tpm2_self_test(dev, TPMI_YES);

Again, tpm_self_test_full(). We are trying to provide a TPM API that
covers v1 and v2, to the extent possible.

> +       }
> +
> +       return rc;
> +}
> +

Also please add a test for this to test/dm/tpm.c

>  u32 tpm2_clear(struct udevice *dev, u32 handle, const char *pw,
>                const ssize_t pw_sz)
>  {
> diff --git a/lib/tpm_api.c b/lib/tpm_api.c
> index 7e8df8795ef3..5b2c11a277cc 100644
> --- a/lib/tpm_api.c
> +++ b/lib/tpm_api.c
> @@ -35,6 +35,14 @@ u32 tpm_startup(struct udevice *dev, enum tpm_startup_type mode)
>         }
>  }
>
> +u32 tpm_auto_start(struct udevice *dev)
> +{
> +       if (tpm_is_v2(dev))
> +               return tpm2_auto_start(dev);

Hopefully all the code from above can move here.

> +
> +       return -ENOSYS;
> +}
> +
>  u32 tpm_resume(struct udevice *dev)
>  {
>         if (tpm_is_v1(dev))
> --
> 2.38.1
>

Regards,
Simon