[U-Boot] [PATCH v2 09/19] tpm: add TPM2_Startup command support

[Simon Glass]

Hi Miquel,

On 29 March 2018 at 15:43, Miquel Raynal <miquel.raynal at bootlin.com> wrote:
> Add support for the TPM2_Startup command.
>
> Change the command file and the help accordingly.
>
> Signed-off-by: Miquel Raynal <miquel.raynal at bootlin.com>
> ---
>  cmd/tpm.c     |  9 +++++++--
>  include/tpm.h | 26 +++++++++++++++++++++++++-
>  lib/tpm.c     | 35 +++++++++++++++++++++++++----------
>  3 files changed, 57 insertions(+), 13 deletions(-)
>
> diff --git a/cmd/tpm.c b/cmd/tpm.c
> index 3e2bb3b118..fc9ef9d4a3 100644
> --- a/cmd/tpm.c
> +++ b/cmd/tpm.c
> @@ -255,6 +255,10 @@ static int do_tpm_startup(cmd_tbl_t *cmdtp, int flag,
>                 mode = TPM_ST_STATE;
>         } else if (!strcasecmp("TPM_ST_DEACTIVATED", argv[1])) {
>                 mode = TPM_ST_DEACTIVATED;
> +       } else if (!strcasecmp("TPM2_SU_CLEAR", argv[1])) {
> +               mode = TPM2_SU_CLEAR;
> +       } else if (!strcasecmp("TPM2_SU_STATE", argv[1])) {
> +               mode = TPM2_SU_STATE;
>         } else {
>                 printf("Couldn't recognize mode string: %s\n", argv[1]);
>                 return CMD_RET_FAILURE;
> @@ -929,8 +933,9 @@ U_BOOT_CMD(tpm, CONFIG_SYS_MAXARGS, 1, do_tpm,
>  "      <type> is one of TPM1 (default) or TPM2. This choice impacts the way\n"
>  "      other functions will behave.\n"
>  "  startup mode\n"
> -"    - Issue TPM_Starup command.  <mode> is one of TPM_ST_CLEAR,\n"
> -"      TPM_ST_STATE, and TPM_ST_DEACTIVATED.\n"
> +"    - Issue TPM_Startup command. <mode> is one of:\n"
> +"          * TPM_ST_CLEAR, TPM_ST_STATE, TPM_ST_DEACTIVATED for TPMv1.x.\n"
> +"          * TPM2_SU_CLEAR, TPM2_SU_STATE, for TPMv2.x.\n"
>  "Admin Testing Commands:\n"
>  "  self_test_full\n"
>  "    - Test all of the TPM capabilities.\n"
> diff --git a/include/tpm.h b/include/tpm.h
> index 1a60ef5b36..ba71bac885 100644
> --- a/include/tpm.h
> +++ b/include/tpm.h
> @@ -26,6 +26,11 @@ enum tpm_duration {
>         TPM_DURATION_COUNT,
>  };
>
> +enum tpm2_structures {

Please add comments for these.

> +       TPM2_ST_NO_SESSIONS     = 0x8001,
> +       TPM2_ST_SESSIONS        = 0x8002,
> +};
> +
>  enum tpm_startup_type {
>         TPM_ST_CLEAR            = 0x0001,
>         TPM_ST_STATE            = 0x0002,
> @@ -37,6 +42,25 @@ enum tpm2_startup_types {
>         TPM2_SU_STATE   = 0x0001,
>  };
>
> +enum tpm2_command_codes {
> +       TPM2_CC_STARTUP         = 0x0144,
> +       TPM2_CC_SELF_TEST       = 0x0143,
> +       TPM2_CC_GET_CAPABILITY  = 0x017A,
> +       TPM2_CC_PCR_READ        = 0x017E,
> +       TPM2_CC_PCR_EXTEND      = 0x0182,
> +};
> +
> +enum tpm2_return_codes {
> +       TPM2_RC_SUCCESS         = 0x0000,
> +       TPM2_RC_HASH            = 0x0083, /* RC_FMT1 */
> +       TPM2_RC_HANDLE          = 0x008B,
> +       TPM2_RC_INITIALIZE      = 0x0100, /* RC_VER1 */
> +       TPM2_RC_DISABLED        = 0x0120,
> +       TPM2_RC_TESTING         = 0x090A, /* RC_WARN */
> +       TPM2_RC_REFERENCE_H0    = 0x0910,
> +       TPM2_RC_LOCKOUT         = 0x0921,
> +};
> +
>  enum tpm_physical_presence {
>         TPM_PHYSICAL_PRESENCE_HW_DISABLE        = 0x0200,
>         TPM_PHYSICAL_PRESENCE_CMD_DISABLE       = 0x0100,
> @@ -445,7 +469,7 @@ int tpm_get_specification(void);
>   * @param mode         TPM startup mode
>   * @return return code of the operation
>   */
> -uint32_t tpm_startup(enum tpm_startup_type mode);
> +int tpm_startup(enum tpm_startup_type mode);

How come the change to an int? It's fine, I just want to understand it.

>
>  /**
>   * Issue a TPM_SelfTestFull command.
> diff --git a/lib/tpm.c b/lib/tpm.c
> index c0fbba86ff..0c57ef8dc7 100644
> --- a/lib/tpm.c
> +++ b/lib/tpm.c
> @@ -310,20 +310,35 @@ int tpm_init(void)
>         return tpm_open(dev);
>  }
>
> -uint32_t tpm_startup(enum tpm_startup_type mode)
> +int tpm_startup(enum tpm_startup_type mode)
>  {
> -       const uint8_t command[12] = {
> -               0x0, 0xc1, 0x0, 0x0, 0x0, 0xc, 0x0, 0x0, 0x0, 0x99, 0x0, 0x0,
> +       const u8 command_v1[12] = {
> +               U16_TO_ARRAY(0xc1),
> +               U32_TO_ARRAY(12),
> +               U32_TO_ARRAY(0x99),
> +               U16_TO_ARRAY(mode),
>         };
> -       const size_t mode_offset = 10;
> -       uint8_t buf[COMMAND_BUFFER_SIZE];
> +       const u8 command_v2[12] = {
> +               U16_TO_ARRAY(TPM2_ST_NO_SESSIONS),
> +               U32_TO_ARRAY(12),
> +               U32_TO_ARRAY(TPM2_CC_STARTUP),
> +               U16_TO_ARRAY(mode),
> +       };
> +       int ret;
> +
> +       if (!is_tpmv2)
> +               return tpm_sendrecv_command(command_v1, NULL, NULL);
> +
> +       ret = tpm_sendrecv_command(command_v2, NULL, NULL);

This doesn't seem better (than the pack_byte thing) to me. What is the benefit?

>
> -       if (pack_byte_string(buf, sizeof(buf), "sw",
> -                               0, command, sizeof(command),
> -                               mode_offset, mode))
> -               return TPM_LIB_ERROR;
> +       /*
> +        * Note TPMv2: STARTUP command will return RC_SUCCESS the first time,
> +        * but will return RC_INITIALIZE otherwise.
> +        */
> +       if (ret && ret != TPM2_RC_INITIALIZE)
> +               return ret;
>
> -       return tpm_sendrecv_command(buf, NULL, NULL);
> +       return 0;
>  }
>
>  uint32_t tpm_self_test_full(void)
> --
> 2.14.1
>

Regards,
Simon