Re: [U-Boot] [PATCH v2 05/19] tpm: prepare support for TPMv2 commands

Hi Miquel,

On 29 March 2018 at 15:43, Miquel Raynal miquel.raynal@bootlin.com wrote:

Later choice between v1 and v2 compliant functions will be handled by a global value in lib/tpm.c that will be accessible through set/get accessors from lib/cmd.c.

This global value is set during the initialization phase if the TPM2 handle is given to the init command.

Signed-off-by: Miquel Raynal miquel.raynal@bootlin.com

cmd/tpm.c | 37 +++++++++++++++++++++----- include/tpm.h | 20 ++++++++++++++ lib/tpm.c | 83 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 131 insertions(+), 9 deletions(-)

diff --git a/cmd/tpm.c b/cmd/tpm.c index f456396d75..1d32028b64 100644 --- a/cmd/tpm.c +++ b/cmd/tpm.c @@ -89,12 +89,16 @@ static void *parse_byte_string(char *bytes, uint8_t *data, size_t *count_ptr) */ static int report_return_code(int return_code) {

•   if (return_code) {
  

•           printf("Error: %d\n", return_code);
  

•           return CMD_RET_FAILURE;
  

•   } else {
  

•   if (!return_code)
            return CMD_RET_SUCCESS;
  

•   }
  

•   if (return_code == -EOPNOTSUPP)
  

•           printf("TPMv%d error: unavailable command with this spec\n",
  

•                  tpm_get_specification());
  

•   else
  

•           printf("TPM error: %d\n", return_code);
  

•   return CMD_RET_FAILURE;
  

}

/** @@ -427,6 +431,24 @@ static int do_tpm_get_capability(cmd_tbl_t *cmdtp, int flag, return report_return_code(rc); }

+static int do_tpm_init(cmd_tbl_t *cmdtp, int flag,

•                  int argc, char * const argv[])
  

+{

•   if (argc > 2)
  

•           return CMD_RET_USAGE;
  

•   if (argc == 2) {
  

•           if (!strcasecmp("TPM1", argv[1]))
  

•                   tpm_set_specification(1);
  

•           else if (!strcasecmp("TPM2", argv[1]))
  

•                   tpm_set_specification(2);
  

•           else
  

•                   return CMD_RET_USAGE;
  

•   }
  

I don't like the idea of setting a global before calling tpm_init(). Can we instead make it an arg to tpm_init()?

Also, instead of 1 and 2, can you create an enum?

•   return report_return_code(tpm_init());
  

+}

#define TPM_COMMAND_NO_ARG(cmd) \ static int do_##cmd(cmd_tbl_t *cmdtp, int flag, \ int argc, char * const argv[]) \ @@ -436,7 +458,6 @@ static int do_##cmd(cmd_tbl_t *cmdtp, int flag, \ return report_return_code(cmd()); \ }

-TPM_COMMAND_NO_ARG(tpm_init) TPM_COMMAND_NO_ARG(tpm_self_test_full) TPM_COMMAND_NO_ARG(tpm_continue_self_test) TPM_COMMAND_NO_ARG(tpm_force_clear) @@ -902,8 +923,10 @@ U_BOOT_CMD(tpm, CONFIG_SYS_MAXARGS, 1, do_tpm, " - Issue TPM command <cmd> with arguments <args...>.\n" "Admin Startup and State Commands:\n" " info - Show information about the TPM\n" -" init\n" +" init [<type>]\n" " - Put TPM into a state where it waits for 'startup' command.\n" +" <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" diff --git a/include/tpm.h b/include/tpm.h index 760d94865c..0ec3428ea4 100644 --- a/include/tpm.h +++ b/include/tpm.h @@ -30,6 +30,11 @@ enum tpm_startup_type { TPM_ST_DEACTIVATED = 0x0003, };

+enum tpm2_startup_types {

Please add a comment as to what this is for.

•   TPM2_SU_CLEAR   = 0x0000,
  

•   TPM2_SU_STATE   = 0x0001,
  

+};

enum tpm_physical_presence { TPM_PHYSICAL_PRESENCE_HW_DISABLE = 0x0200, TPM_PHYSICAL_PRESENCE_CMD_DISABLE = 0x0100, @@ -417,6 +422,21 @@ int tpm_xfer(struct udevice *dev, const uint8_t *sendbuf, size_t send_size, */ int tpm_init(void);

+/**

• • Assign a value to the global is_nfcv2 boolean to discriminate in the lib
• • between the available specifications.
• • @version: 1 or 2, depending on the supported specification
• */

+int tpm_set_specification(int version);

+/**

• • Return the current value of the specification.
• • @return: 1 or 2, depending on the supported specification
• */

+int tpm_get_specification(void);

/**

• Issue a TPM_Startup command.

diff --git a/lib/tpm.c b/lib/tpm.c index 99556b1cf6..38b76b4961 100644 --- a/lib/tpm.c +++ b/lib/tpm.c @@ -15,6 +15,9 @@ /* Internal error of TPM command library */ #define TPM_LIB_ERROR ((uint32_t)~0u)

+/* Global boolean to discriminate TPMv2.x from TPMv1.x functions */ +static bool is_tpmv2;

Can this go in the TPM uclass as uclass-private data? See struct uclass member 'priv'.

/* Useful constants */ enum { COMMAND_BUFFER_SIZE = 256, @@ -262,6 +265,26 @@ static uint32_t tpm_sendrecv_command(const void *command, return tpm_return_code(response); }

+int tpm_set_specification(int version) +{

•   if (version == 1) {
  

•           debug("TPM complies to the v1.x specification\n");
  

•           is_tpmv2 = false;
  

•   } else if (version == 2) {
  

•           debug("TPM complies to the v2.x specification\n");
  

•           is_tpmv2 = true;
  

•   } else {
  

•           return -EINVAL;
  

•   }
  

•   return 0;
  

+}

+int tpm_get_specification(void) +{

•   return is_tpmv2 ? 2 : 1;
  

+}

int tpm_init(void) { int err; @@ -338,6 +361,9 @@ uint32_t tpm_nv_define_space(uint32_t index, uint32_t perm, uint32_t size) const size_t size_offset = 77; uint8_t buf[COMMAND_BUFFER_SIZE];

•   if (is_tpmv2)
  

•           return -EOPNOTSUPP;
  

•   if (pack_byte_string(buf, sizeof(buf), "sddd",
                            0, command, sizeof(command),
                            index_offset, index,
  

@@ -362,6 +388,9 @@ uint32_t tpm_nv_read_value(uint32_t index, void *data, uint32_t count) uint32_t data_size; uint32_t err;

•   if (is_tpmv2)
  

•           return -EOPNOTSUPP;
  

This return code should be mentioned in the header file for these functions.

[...]

Regards, Simon