Hi Simon, On Fri, 30 Mar 2018 06:42:32 +0800, Simon Glass <s...@chromium.org> wrote:
> Hi Miquel, > > On 29 March 2018 at 15:43, Miquel Raynal <miquel.ray...@bootlin.com> wrote: > > Add support for the TPM2_Clear command. > > > > Change the command file and the help accordingly. > > > > Signed-off-by: Miquel Raynal <miquel.ray...@bootlin.com> > > --- > > cmd/tpm.c | 29 ++++++++++++++++++++++++++--- > > cmd/tpm_test.c | 6 +++--- > > include/tpm.h | 21 +++++++++++++++++---- > > lib/tpm.c | 42 ++++++++++++++++++++++++++++++++++++++---- > > 4 files changed, 84 insertions(+), 14 deletions(-) > > > > diff --git a/cmd/tpm.c b/cmd/tpm.c > > index fc9ef9d4a3..32921e1a70 100644 > > --- a/cmd/tpm.c > > +++ b/cmd/tpm.c > > @@ -454,6 +454,29 @@ static int do_tpm_init(cmd_tbl_t *cmdtp, int flag, > > return report_return_code(tpm_init()); > > } > > > > +static int do_tpm_force_clear(cmd_tbl_t *cmdtp, int flag, > > + int argc, char * const argv[]) > > +{ > > + u32 handle = 0; > > + const char *pw = (argc < 3) ? NULL : argv[2]; > > + const ssize_t pw_sz = pw ? strlen(pw) : 0; > > + > > + if (argc < 2 || argc > 3) > > + return CMD_RET_USAGE; > > + > > + if (pw_sz > TPM2_DIGEST_LENGTH) > > + return -EINVAL; > > + > > + if (!strcasecmp("TPM2_RH_LOCKOUT", argv[1])) > > + handle = TPM2_RH_LOCKOUT; > > + else if (!strcasecmp("TPM2_RH_PLATFORM", argv[1])) > > + handle = TPM2_RH_PLATFORM; > > + else > > + return CMD_RET_USAGE; > > + > > + return report_return_code(tpm_force_clear(handle, pw, pw_sz)); > > +} > > + > > #define TPM_COMMAND_NO_ARG(cmd) \ > > static int do_##cmd(cmd_tbl_t *cmdtp, int flag, \ > > int argc, char * const argv[]) \ > > @@ -465,7 +488,6 @@ static int do_##cmd(cmd_tbl_t *cmdtp, int flag, > > \ > > > > TPM_COMMAND_NO_ARG(tpm_self_test_full) > > TPM_COMMAND_NO_ARG(tpm_continue_self_test) > > -TPM_COMMAND_NO_ARG(tpm_force_clear) > > TPM_COMMAND_NO_ARG(tpm_physical_enable) > > TPM_COMMAND_NO_ARG(tpm_physical_disable) > > > > @@ -951,8 +973,9 @@ U_BOOT_CMD(tpm, CONFIG_SYS_MAXARGS, 1, do_tpm, > > " physical_set_deactivated 0|1\n" > > " - Set deactivated flag.\n" > > "Admin Ownership Commands:\n" > > -" force_clear\n" > > -" - Issue TPM_ForceClear command.\n" > > +" force_clear [<type>]\n" > > +" - Issue TPM_[Force]Clear command, with <type> one of (TPMv2 only):\n" > > +" * TPM2_RH_LOCKOUT, TPM2_RH_PLATFORM.\n" > > " tsc_physical_presence flags\n" > > " - Set TPM device's Physical Presence flags to <flags>.\n" > > "The Capability Commands:\n" > > diff --git a/cmd/tpm_test.c b/cmd/tpm_test.c > > index 37ad2ff33d..da40dbc423 100644 > > --- a/cmd/tpm_test.c > > +++ b/cmd/tpm_test.c > > @@ -176,7 +176,7 @@ static int test_fast_enable(void) > > TPM_CHECK(tpm_get_flags(&disable, &deactivated, NULL)); > > printf("\tdisable is %d, deactivated is %d\n", disable, > > deactivated); > > for (i = 0; i < 2; i++) { > > - TPM_CHECK(tpm_force_clear()); > > + TPM_CHECK(tpm_force_clear(0, NULL, 0)); > > TPM_CHECK(tpm_get_flags(&disable, &deactivated, NULL)); > > printf("\tdisable is %d, deactivated is %d\n", disable, > > deactivated); > > @@ -458,7 +458,7 @@ static int test_write_limit(void) > > TPM_CHECK(TlclStartupIfNeeded()); > > TPM_CHECK(tpm_self_test_full()); > > TPM_CHECK(tpm_tsc_physical_presence(PRESENCE)); > > - TPM_CHECK(tpm_force_clear()); > > + TPM_CHECK(tpm_force_clear(0, NULL, 0)); > > TPM_CHECK(tpm_physical_enable()); > > TPM_CHECK(tpm_physical_set_deactivated(0)); > > > > @@ -477,7 +477,7 @@ static int test_write_limit(void) > > } > > > > /* Reset write count */ > > - TPM_CHECK(tpm_force_clear()); > > + TPM_CHECK(tpm_force_clear(0, NULL, 0)); > > TPM_CHECK(tpm_physical_enable()); > > TPM_CHECK(tpm_physical_set_deactivated(0)); > > > > diff --git a/include/tpm.h b/include/tpm.h > > index 38d7cb899d..2f17166662 100644 > > --- a/include/tpm.h > > +++ b/include/tpm.h > > @@ -43,13 +43,23 @@ enum tpm_startup_type { > > }; > > > > enum tpm2_startup_types { > > - TPM2_SU_CLEAR = 0x0000, > > - TPM2_SU_STATE = 0x0001, > > + TPM2_SU_CLEAR = 0x0000, > > + TPM2_SU_STATE = 0x0001, > > +}; > > + > > +enum tpm2_handles { > > Please add comment to enum Fine, I will document all of them. Just for you to know, these are values extracted from the (publicly available) specification, I did not change any of them. > > > + TPM2_RH_OWNER = 0x40000001, > > + TPM2_RS_PW = 0x40000009, > > + TPM2_RH_LOCKOUT = 0x4000000A, > > + TPM2_RH_ENDORSEMENT = 0x4000000B, > > + TPM2_RH_PLATFORM = 0x4000000C, > > }; > > > > enum tpm2_command_codes { > > TPM2_CC_STARTUP = 0x0144, > > TPM2_CC_SELF_TEST = 0x0143, > > + TPM2_CC_CLEAR = 0x0126, > > + TPM2_CC_CLEARCONTROL = 0x0127, > > TPM2_CC_GET_CAPABILITY = 0x017A, > > TPM2_CC_PCR_READ = 0x017E, > > TPM2_CC_PCR_EXTEND = 0x0182, > > @@ -567,11 +577,14 @@ uint32_t tpm_tsc_physical_presence(uint16_t presence); > > uint32_t tpm_read_pubek(void *data, size_t count); > > > > /** > > - * Issue a TPM_ForceClear command. > > + * Issue a TPM_ForceClear or a TPM2_Clear command. > > * > > + * @param handle Handle > > + * @param pw Password > > + * @param pw_sz Length of the password > > * @return return code of the operation > > */ > > -uint32_t tpm_force_clear(void); > > +int tpm_force_clear(u32 handle, const char *pw, const ssize_t pw_sz); > > > > /** > > * Issue a TPM_PhysicalEnable command. > > diff --git a/lib/tpm.c b/lib/tpm.c > > index 3cc2888267..9a46ac09e6 100644 > > --- a/lib/tpm.c > > +++ b/lib/tpm.c > > @@ -608,13 +608,47 @@ uint32_t tpm_read_pubek(void *data, size_t count) > > return 0; > > } > > > > -uint32_t tpm_force_clear(void) > > +int tpm_force_clear(u32 handle, const char *pw, const ssize_t pw_sz) > > { > > - const uint8_t command[10] = { > > - 0x0, 0xc1, 0x0, 0x0, 0x0, 0xa, 0x0, 0x0, 0x0, 0x5d, > > + const u8 command_v1[10] = { > > + U16_TO_ARRAY(0xc1), > > + U32_TO_ARRAY(10), > > + U32_TO_ARRAY(0x5d), > > }; > > + u8 command_v2[COMMAND_BUFFER_SIZE] = { > > + U16_TO_ARRAY(TPM2_ST_SESSIONS), /* TAG */ > > + U32_TO_ARRAY(27 + pw_sz), /* Length */ > > + U32_TO_ARRAY(TPM2_CC_CLEAR), /* Command code */ > > Here we have both v1 and v2 commands. Perhaps we should have a Kconfig > option to enable v2 support? Or do you think it is not a big addition > in terms of code side? It is a big addition in terms of code side. > > One way to do this would be to have an inline function which can tell > if you are using v2: > > static inline bool tpm_v2(void) { > return IS_ENABLED(CONFIG_TPM_V2) && further things... > } > > static inline bool tpm_v1(void) { > return IS_ENABLED(CONFIG_TPM_V1) && further things... > } I like this way of choosing one specification or the other, I could make them mutually exclusive. It would prevent the need for a global variable (or an additional field in uclass). Would it be fine to actually rename the file (with a 'tpm1' suffix) and create a new one specific for TPMv2 commands ? Only one file would be compiled/linked depending on the configuration, avoiding the possibility to call a v1 command from a v2 chip and vice versa. At first I thought a lot of code would be shared but I don't think we would add so much by having one function per revision now. > > > > > - return tpm_sendrecv_command(command, NULL, NULL); > > + /* HANDLE */ > > + U32_TO_ARRAY(handle), /* TPM resource handle */ > > If we really need these, perhaps tpm_u32() is a better name that > U32_TO_ARRAY() ? > > > + > > + /* AUTH_SESSION */ > > + U32_TO_ARRAY(9 + pw_sz), /* Authorization size */ > > + U32_TO_ARRAY(TPM2_RS_PW), /* Session handle */ > > + U16_TO_ARRAY(0), /* Size of <nonce> */ > > + /* <nonce> (if any) */ > > + 0, /* Attributes: > > Cont/Excl/Rst */ > > + U16_TO_ARRAY(pw_sz), /* Size of <hmac/password> > > */ > > + /* STRING(pw) <hmac/password> (if any) > > */ > > + }; > > + unsigned int offset = 27; > > + int ret; > > + > > + if (!is_tpmv2) > > + tpm_sendrecv_command(command_v1, NULL, NULL); > > + > > + /* > > + * Fill the command structure starting from the first buffer: > > + * - the password (if any) > > + */ > > + ret = pack_byte_string(command_v2, sizeof(command_v2), "s", > > + offset, pw, pw_sz); > > + offset += pw_sz; > > + if (ret) > > + return TPM_LIB_ERROR; > > + > > + return tpm_sendrecv_command(command_v2, NULL, NULL); > > } > > > > uint32_t tpm_physical_enable(void) > > -- > > 2.14.1 > > > > Regards, > Simon Thanks, Miquèl -- Miquel Raynal, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot