[PATCH V3 2/2] cmd: tpm: add a subcommand device

[Philippe REYNES]

Hi,

Sorry, I forgot to report the Reviewed-by of Miquel Raynal.


> Objet: [PATCH V3 2/2] cmd: tpm: add a subcommand device

> The command tpm (and tpm2) search the tpm and use it.
> On sandbox, there are two tpm (tpm 1.x and tpm 2.0).
> So the command tpm and tpm2 are always executed with
> the first tpm (tpm 1.x), and the command tpm2 always
> fails.
> 
> This add a subcommand device to command tpm and
> command tpm2. Then the command tpm and tpm2 use
> the device selected with the subcommand device.
> 
> To be compatible with previous behaviour, if the
> subcommand device is not used before a tpm (or tpm2)
> command, the device 0 is selected.
> 
> Signed-off-by: Philippe Reynes <philippe.reynes at softathome.com>

Reviewed-by: Miquel Raynal <miquel.raynal at bootlin.com>

> ---
> cmd/tpm-common.c | 77 +++++++++++++++++++++++++++++++++++++++++++++++++---
> cmd/tpm-user-utils.h | 1 +
> cmd/tpm-v1.c | 3 ++
> cmd/tpm-v2.c | 3 ++
> 4 files changed, 80 insertions(+), 4 deletions(-)
> 
> Changelog:
> v3:
> - no change
> v2:
> - use helper for tpm device (idea from Miquel)
> - don't set tpm in the look (feedback from Miquel)
> - add a comment to explain the backward code (feedback from Miquel)
> 
> diff --git a/cmd/tpm-common.c b/cmd/tpm-common.c
> index 38900fb..42882b1 100644
> --- a/cmd/tpm-common.c
> +++ b/cmd/tpm-common.c
> @@ -12,6 +12,8 @@
> #include <tpm-common.h>
> #include "tpm-user-utils.h"
> 
> +static struct udevice *tpm_dev;
> +
> /**
> * Print a byte string in hexdecimal format, 16-bytes per line.
> *
> @@ -231,19 +233,86 @@ int type_string_write_vars(const char *type_str, u8 *data,
> return 0;
> }
> 
> +static int tpm_show_device(void)
> +{
> + struct udevice *dev;
> + char buf[80];
> + int n = 0, rc;
> +
> + for_each_tpm_device(dev) {
> + rc = tpm_get_desc(dev, buf, sizeof(buf));
> + if (rc < 0)
> + printf("device %d: can't get info\n", n);
> + else
> + printf("device %d: %s\n", n, buf);
> +
> + n++;
> + };
> +
> + return 0;
> +}
> +
> +static int tpm_set_device(unsigned long num)
> +{
> + struct udevice *dev;
> + unsigned long n = 0;
> + int rc = CMD_RET_FAILURE;
> +
> + for_each_tpm_device(dev) {
> + if (n == num) {
> + rc = 0;
> + break;
> + }
> +
> + n++;
> + }
> +
> + if (!rc)
> + tpm_dev = dev;
> +
> + return rc;
> +}
> +
> int get_tpm(struct udevice **devp)
> {
> int rc;
> 
> - rc = uclass_first_device_err(UCLASS_TPM, devp);
> - if (rc) {
> - printf("Could not find TPM (ret=%d)\n", rc);
> - return CMD_RET_FAILURE;
> + /*
> + * To keep a backward compatibility with previous code,
> + * if a tpm device is not explicitly set, we set the first one.
> + */
> + if (!tpm_dev) {
> + rc = tpm_set_device(0);
> + if (rc) {
> + printf("Couldn't set TPM 0 (rc = %d)\n", rc);
> + return CMD_RET_FAILURE;
> + }
> }
> 
> + if (devp)
> + *devp = tpm_dev;
> +
> return 0;
> }
> 
> +int do_tpm_device(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
> +{
> + unsigned long num;
> + int rc;
> +
> + if (argc == 2) {
> + num = simple_strtoul(argv[1], NULL, 10);
> +
> + rc = tpm_set_device(num);
> + if (rc)
> + printf("Couldn't set TPM %lu (rc = %d)\n", num, rc);
> + } else {
> + rc = tpm_show_device();
> + }
> +
> + return rc;
> +}
> +
> int do_tpm_info(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
> {
> struct udevice *dev;
> diff --git a/cmd/tpm-user-utils.h b/cmd/tpm-user-utils.h
> index 8ce9861..a851d9c 100644
> --- a/cmd/tpm-user-utils.h
> +++ b/cmd/tpm-user-utils.h
> @@ -17,6 +17,7 @@ int type_string_pack(const char *type_str, char * const
> values[], u8 *data);
> int type_string_write_vars(const char *type_str, u8 *data, char * const vars[]);
> int get_tpm(struct udevice **devp);
> 
> +int do_tpm_device(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[]);
> int do_tpm_init(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[]);
> int do_tpm_info(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[]);
> int do_tpm(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
> diff --git a/cmd/tpm-v1.c b/cmd/tpm-v1.c
> index 2807331..bc34e06 100644
> --- a/cmd/tpm-v1.c
> +++ b/cmd/tpm-v1.c
> @@ -645,6 +645,7 @@ TPM_COMMAND_NO_ARG(tpm_physical_enable)
> TPM_COMMAND_NO_ARG(tpm_physical_disable)
> 
> static cmd_tbl_t 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_init, "", ""),
> U_BOOT_CMD_MKENT(startup, 0, 1,
> @@ -721,6 +722,8 @@ U_BOOT_CMD(tpm, CONFIG_SYS_MAXARGS, 1, do_tpm,
> "cmd args...\n"
> " - Issue TPM command <cmd> with arguments <args...>.\n"
> "Admin Startup and State Commands:\n"
> +" device [num device]\n"
> +" - Show all devices or set the specified device\n"
> " info - Show information about the TPM\n"
> " init\n"
> " - Put TPM into a state where it waits for 'startup' command.\n"
> diff --git a/cmd/tpm-v2.c b/cmd/tpm-v2.c
> index 459a955..0cd3982 100644
> --- a/cmd/tpm-v2.c
> +++ b/cmd/tpm-v2.c
> @@ -354,6 +354,7 @@ static int do_tpm_pcr_setauthvalue(cmd_tbl_t *cmdtp, int
> flag,
> }
> 
> static cmd_tbl_t tpm2_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_init, "", ""),
> U_BOOT_CMD_MKENT(startup, 0, 1, do_tpm2_startup, "", ""),
> @@ -381,6 +382,8 @@ cmd_tbl_t *get_tpm2_commands(unsigned int *size)
> U_BOOT_CMD(tpm2, CONFIG_SYS_MAXARGS, 1, do_tpm, "Issue a TPMv2.x command",
> "<command> [<arguments>]\n"
> "\n"
> +"device [num device]\n"
> +" Show all devices or set the specified device\n"
> "info\n"
> " Show information about the TPM.\n"
> "init\n"
> --
> 2.7.4

Regards,
Philippe