[PATCH] tpm: Make 'tpm init' to call tpm_auto_start()

For a TPM device to be operational we need to initialize it and perform its startup sequence. The 'tpm init' command currently calls tpm_init() which ends up calling the ->open() per-device callback and performs the initial hardware configuration as well as requesting locality 0 for the caller. There no code that currently calls tpm_init() without following up with a tpm_startup() and tpm_self_test_full() or tpm_continue_self_test().
So let's wire up the 'tpm init' command and call tpm_auto_start() which leaves the device in an operational state and adjust any defconfigs using 'tpm init'.
It's worth noting that calling tpm_init() only, doesn't allow a someone to use the TPM since the startup sequence is mandatory. We always repeat the pattern of calling - tpm_init() - tpm_startup() - tpm_self_test_full() or tpm_continue_self_test() as a result we don't expect any regression or boot delays with the current change.
While at it fix the identation of test_tpm_autostart() comments as well.
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org ---
This is a split and resend of https://lore.kernel.org/u-boot/20230510074359.2837818-9-ilias.apalodimas@lin... Since Simon some had concerns I decided to split this off the series and send it as a single patch for further discussion.
cmd/tpm-common.c | 3 ++- configs/chromebook_coral_defconfig | 2 +- test/dm/tpm.c | 9 +++++---- test/py/tests/test_tpm2.py | 9 --------- 4 files changed, 8 insertions(+), 15 deletions(-)
diff --git a/cmd/tpm-common.c b/cmd/tpm-common.c index d0c63cadf413..9b1ad0b371df 100644 --- a/cmd/tpm-common.c +++ b/cmd/tpm-common.c @@ -11,6 +11,7 @@ #include <asm/unaligned.h> #include <linux/string.h> #include <tpm-common.h> +#include <tpm_api.h> #include "tpm-user-utils.h"
static struct udevice *tpm_dev; @@ -364,7 +365,7 @@ int do_tpm_init(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) if (rc) return rc;
- return report_return_code(tpm_init(dev)); + return report_return_code(tpm_auto_start(dev)); }
int do_tpm(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) diff --git a/configs/chromebook_coral_defconfig b/configs/chromebook_coral_defconfig index f5995f22004e..8b4c1228a1fc 100644 --- a/configs/chromebook_coral_defconfig +++ b/configs/chromebook_coral_defconfig @@ -33,7 +33,7 @@ CONFIG_BOOTSTAGE_STASH=y CONFIG_USE_BOOTARGS=y CONFIG_BOOTARGS_SUBST=y CONFIG_USE_BOOTCOMMAND=y -CONFIG_BOOTCOMMAND="tpm init; tpm startup TPM2_SU_CLEAR; read mmc 0:2 100000 0 80; setexpr loader *001004f0; setexpr size *00100518; setexpr blocks $size / 200; read mmc 0:2 100000 80 $blocks; setexpr setup $loader - 1000; setexpr cmdline_ptr $loader - 2000; setexpr.s cmdline *$cmdline_ptr; setexpr cmdline gsub %U \\${uuid}; if part uuid mmc 0:2 uuid; then zboot start 100000 0 0 0 $setup cmdline; zboot load; zboot setup; zboot dump; zboot go;fi" +CONFIG_BOOTCOMMAND="tpm init; read mmc 0:2 100000 0 80; setexpr loader *001004f0; setexpr size *00100518; setexpr blocks $size / 200; read mmc 0:2 100000 80 $blocks; setexpr setup $loader - 1000; setexpr cmdline_ptr $loader - 2000; setexpr.s cmdline *$cmdline_ptr; setexpr cmdline gsub %U \\${uuid}; if part uuid mmc 0:2 uuid; then zboot start 100000 0 0 0 $setup cmdline; zboot load; zboot setup; zboot dump; zboot go;fi" CONFIG_SYS_CONSOLE_INFO_QUIET=y CONFIG_DISPLAY_BOARDINFO_LATE=y CONFIG_LAST_STAGE_INIT=y diff --git a/test/dm/tpm.c b/test/dm/tpm.c index 3defb3c3da1f..cde933ab2848 100644 --- a/test/dm/tpm.c +++ b/test/dm/tpm.c @@ -98,10 +98,11 @@ static int test_tpm_autostart(struct unit_test_state *uts,
if (reinit) ut_assertok(tpm_init(dev)); - /* - * tpm_auto_start will rerun tpm_init() if reinit, but handles the - * -EBUSY return code internally. - */ + + /* + * tpm_auto_start will rerun tpm_init() if reinit, but handles the + * -EBUSY return code internally. + */ ut_assertok(tpm_auto_start(dev));
return 0; diff --git a/test/py/tests/test_tpm2.py b/test/py/tests/test_tpm2.py index d2ad6f9e73c0..6f9b1dd89258 100644 --- a/test/py/tests/test_tpm2.py +++ b/test/py/tests/test_tpm2.py @@ -44,8 +44,6 @@ def force_init(u_boot_console, force=False): output = u_boot_console.run_command('tpm2 init') if force or not 'Error' in output: u_boot_console.run_command('echo --- start of init ---') - u_boot_console.run_command('tpm2 startup TPM2_SU_CLEAR') - u_boot_console.run_command('tpm2 self_test full') u_boot_console.run_command('tpm2 clear TPM2_RH_LOCKOUT') output = u_boot_console.run_command('echo $?') if not output.endswith('0'): @@ -90,13 +88,6 @@ def tpm2_sandbox_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 startup TPM2_SU_CLEAR') - output = u_boot_console.run_command('echo $?') - assert output.endswith('0') - - u_boot_console.run_command('tpm2 self_test full') - output = u_boot_console.run_command('echo $?') - assert output.endswith('0')
@pytest.mark.buildconfigspec('cmd_tpm_v2') def test_tpm2_sandbox_self_test_full(u_boot_console): -- 2.39.2

Hi Ilias,
On Tue, 30 May 2023 at 00:18, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
For a TPM device to be operational we need to initialize it and perform its startup sequence. The 'tpm init' command currently calls tpm_init() which ends up calling the ->open() per-device callback and performs the initial hardware configuration as well as requesting locality 0 for the caller. There no code that currently calls tpm_init() without following up with a tpm_startup() and tpm_self_test_full() or tpm_continue_self_test().
So let's wire up the 'tpm init' command and call tpm_auto_start() which leaves the device in an operational state and adjust any defconfigs using 'tpm init'.
It's worth noting that calling tpm_init() only, doesn't allow a someone to use the TPM since the startup sequence is mandatory. We always repeat the pattern of calling
- tpm_init()
- tpm_startup()
- tpm_self_test_full() or tpm_continue_self_test()
as a result we don't expect any regression or boot delays with the current change.
While at it fix the identation of test_tpm_autostart() comments as well.
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org
This is a split and resend of https://lore.kernel.org/u-boot/20230510074359.2837818-9-ilias.apalodimas@lin... Since Simon some had concerns I decided to split this off the series and send it as a single patch for further discussion.
cmd/tpm-common.c | 3 ++- configs/chromebook_coral_defconfig | 2 +- test/dm/tpm.c | 9 +++++---- test/py/tests/test_tpm2.py | 9 --------- 4 files changed, 8 insertions(+), 15 deletions(-)
We've already discussed this before. Please can you:
- Call tpm_autostart() instead - Add a new 'tpm autostart' command
That way we can keep tpm_init() as it is.
Regards, Simon

Hi Simon,
On Wed, May 31, 2023 at 09:28:04PM -0600, Simon Glass wrote:
Hi Ilias,
On Tue, 30 May 2023 at 00:18, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
For a TPM device to be operational we need to initialize it and perform its startup sequence. The 'tpm init' command currently calls tpm_init() which ends up calling the ->open() per-device callback and performs the initial hardware configuration as well as requesting locality 0 for the caller. There no code that currently calls tpm_init() without following up with a tpm_startup() and tpm_self_test_full() or tpm_continue_self_test().
So let's wire up the 'tpm init' command and call tpm_auto_start() which leaves the device in an operational state and adjust any defconfigs using 'tpm init'.
It's worth noting that calling tpm_init() only, doesn't allow a someone to use the TPM since the startup sequence is mandatory. We always repeat the pattern of calling
- tpm_init()
- tpm_startup()
- tpm_self_test_full() or tpm_continue_self_test()
as a result we don't expect any regression or boot delays with the current change.
While at it fix the identation of test_tpm_autostart() comments as well.
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org
This is a split and resend of https://lore.kernel.org/u-boot/20230510074359.2837818-9-ilias.apalodimas@lin... Since Simon some had concerns I decided to split this off the series and send it as a single patch for further discussion.
cmd/tpm-common.c | 3 ++- configs/chromebook_coral_defconfig | 2 +- test/dm/tpm.c | 9 +++++---- test/py/tests/test_tpm2.py | 9 --------- 4 files changed, 8 insertions(+), 15 deletions(-)
We've already discussed this before. Please can you:
- Call tpm_autostart() instead
- Add a new 'tpm autostart' command
That way we can keep tpm_init() as it is.
We have discussed but I am not convinced on why we should ever keep 'tpm init' as is. You mentioned the ability to not always boot with Startup(Clear) but that makes little sense to be in a u-boot command. If we ever want to resume a tpm we should do that automatically.
In any case the spec itself mentions that the _init function should put the TPM in a state were the next command *must* be a startup one, so I don't mind adding a 'tpm autostart'.
Regards /Ilias
Regards, Simon
participants (2)
-
Ilias Apalodimas
-
Simon Glass