| Message ID | 20230530061843.248629-1-ilias.apalodimas@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | tpm: Make 'tpm init' to call tpm_auto_start() | expand |
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@linaro.org/ > 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@linaro.org/ > > 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
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):
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@linaro.org/ 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(-) -- 2.39.2