diff mbox series

[1/2] tpm: Add 'tpm autostart' shell command

Message ID 20230601062041.524010-1-ilias.apalodimas@linaro.org
State Superseded
Headers show
Series [1/2] tpm: Add 'tpm autostart' shell command | expand

Commit Message

Ilias Apalodimas June 1, 2023, 6:20 a.m. UTC
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 add a 'tpm init' command and call tpm_auto_start() which
leaves the device in an operational state.

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()

Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
 cmd/tpm-common.c     | 16 ++++++++++++++++
 cmd/tpm-user-utils.h |  1 +
 cmd/tpm-v1.c         |  6 +++++-
 cmd/tpm-v2.c         |  6 ++++++
 4 files changed, 28 insertions(+), 1 deletion(-)

Comments

Simon Glass June 1, 2023, 9:51 p.m. UTC | #1
Hi Ilias,

On Thu, 1 Jun 2023 at 00:21, 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 add a 'tpm init' command and call tpm_auto_start() which
> leaves the device in an operational state.
>
> 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()
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
>  cmd/tpm-common.c     | 16 ++++++++++++++++
>  cmd/tpm-user-utils.h |  1 +
>  cmd/tpm-v1.c         |  6 +++++-
>  cmd/tpm-v2.c         |  6 ++++++
>  4 files changed, 28 insertions(+), 1 deletion(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

You could add a doc/ if you have time, at least for the new subcommand

Regards,
Simon
Ilias Apalodimas June 2, 2023, 7:58 a.m. UTC | #2
On Fri, 2 Jun 2023 at 00:51, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Ilias,
>
> On Thu, 1 Jun 2023 at 00:21, 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 add a 'tpm init' command and call tpm_auto_start() which
> > leaves the device in an operational state.
> >
> > 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()
> >
> > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > ---
> >  cmd/tpm-common.c     | 16 ++++++++++++++++
> >  cmd/tpm-user-utils.h |  1 +
> >  cmd/tpm-v1.c         |  6 +++++-
> >  cmd/tpm-v2.c         |  6 ++++++
> >  4 files changed, 28 insertions(+), 1 deletion(-)
>
> Reviewed-by: Simon Glass <sjg@chromium.org>
>
> You could add a doc/ if you have time, at least for the new subcommand

Yep you are right, I'll send a pull-request to Tom for -next with this
and send updates on doc/ within next week

Thanks
/Ilias
>
> Regards,
> Simon
diff mbox series

Patch

diff --git a/cmd/tpm-common.c b/cmd/tpm-common.c
index d0c63cadf413..a7dc23d85d5d 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;
@@ -367,6 +368,21 @@  int do_tpm_init(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 	return report_return_code(tpm_init(dev));
 }
 
+int do_tpm_autostart(struct cmd_tbl *cmdtp, int flag, int argc,
+		     char *const argv[])
+{
+	struct udevice *dev;
+	int rc;
+
+	if (argc != 1)
+		return CMD_RET_USAGE;
+	rc = get_tpm(&dev);
+	if (rc)
+		return rc;
+
+	return report_return_code(tpm_auto_start(dev));
+}
+
 int do_tpm(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 {
 	struct cmd_tbl *tpm_commands, *cmd;
diff --git a/cmd/tpm-user-utils.h b/cmd/tpm-user-utils.h
index de4a934aab6c..dfa11353e122 100644
--- a/cmd/tpm-user-utils.h
+++ b/cmd/tpm-user-utils.h
@@ -20,6 +20,7 @@  int get_tpm(struct udevice **devp);
 int do_tpm_device(struct cmd_tbl *cmdtp, int flag, int argc,
 		  char *const argv[]);
 int do_tpm_init(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]);
+int do_tpm_autostart(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]);
 int do_tpm_info(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]);
 int do_tpm_report_state(struct cmd_tbl *cmdtp, int flag, int argc,
 			char *const argv[]);
diff --git a/cmd/tpm-v1.c b/cmd/tpm-v1.c
index 0efb079b0a9b..3b95c950cc96 100644
--- a/cmd/tpm-v1.c
+++ b/cmd/tpm-v1.c
@@ -655,6 +655,7 @@  TPM_COMMAND_NO_ARG(tpm_physical_disable)
 static struct cmd_tbl tpm1_commands[] = {
 	U_BOOT_CMD_MKENT(device, 0, 1, do_tpm_device, "", ""),
 	U_BOOT_CMD_MKENT(info, 0, 1, do_tpm_info, "", ""),
+	U_BOOT_CMD_MKENT(init, 0, 1, do_tpm_autostart, "", ""),
 	U_BOOT_CMD_MKENT(init, 0, 1, do_tpm_init, "", ""),
 	U_BOOT_CMD_MKENT(startup, 0, 1,
 			 do_tpm_startup, "", ""),
@@ -733,9 +734,12 @@  U_BOOT_CMD(tpm, CONFIG_SYS_MAXARGS, 1, do_tpm,
 "  device [num device]\n"
 "    - Show all devices or set the specified device\n"
 "  info - Show information about the TPM\n"
+"  autostart\n"
+"    - Initalize the tpm, perform a Startup(clear) and run a full selftest\n"
+"      sequence\n"
 "  init\n"
 "    - Put TPM into a state where it waits for 'startup' command.\n"
-"  startup mode\n"
+"      startup mode\n"
 "    - Issue TPM_Starup command.  <mode> is one of TPM_ST_CLEAR,\n"
 "      TPM_ST_STATE, and TPM_ST_DEACTIVATED.\n"
 "Admin Testing Commands:\n"
diff --git a/cmd/tpm-v2.c b/cmd/tpm-v2.c
index d93b83ada934..7e479b9dfe36 100644
--- a/cmd/tpm-v2.c
+++ b/cmd/tpm-v2.c
@@ -370,6 +370,7 @@  static struct cmd_tbl tpm2_commands[] = {
 	U_BOOT_CMD_MKENT(dam_reset, 0, 1, do_tpm_dam_reset, "", ""),
 	U_BOOT_CMD_MKENT(dam_parameters, 0, 1, do_tpm_dam_parameters, "", ""),
 	U_BOOT_CMD_MKENT(change_auth, 0, 1, do_tpm_change_auth, "", ""),
+	U_BOOT_CMD_MKENT(autostart, 0, 1, do_tpm_autostart, "", ""),
 	U_BOOT_CMD_MKENT(pcr_setauthpolicy, 0, 1,
 			 do_tpm_pcr_setauthpolicy, "", ""),
 	U_BOOT_CMD_MKENT(pcr_setauthvalue, 0, 1,
@@ -392,8 +393,13 @@  U_BOOT_CMD(tpm2, CONFIG_SYS_MAXARGS, 1, do_tpm, "Issue a TPMv2.x command",
 "    Show information about the TPM.\n"
 "state\n"
 "    Show internal state from the TPM (if available)\n"
+"autostart\n"
+"    Initalize the tpm, perform a Startup(clear) and run a full selftest\n"
+"    sequence\n"
 "init\n"
 "    Initialize the software stack. Always the first command to issue.\n"
+"    'tpm startup' is the only acceptable command after a 'tpm init' has been\n"
+"    issued\n"
 "startup <mode>\n"
 "    Issue a TPM2_Startup command.\n"
 "    <mode> is one of:\n"