Hi Julia, Apologies for the late response, I was on vacation.
On Mon, 3 Jul 2023 at 16:03, Julia Daxenberger <julia.daxenber...@infineon.com> wrote: > > Add TPM2_GetTestResult command support and change the command file and the > help accordingly. Add Python tests and sandbox driver functionality. > > The TPM2_GetTestResult command is performed after the TPM2_SelfTest command > and returns manufacturer-specific information regarding the results of the > self-test and an indication of the test status. What the code does is quite obvious, can you please elaborate on why we need this applied? [...] > +#define TEST_RESULT_DATA_BUFFER_SIZE 256 > + > static int do_tpm2_startup(struct cmd_tbl *cmdtp, int flag, int argc, > char *const argv[]) > { > @@ -356,6 +359,57 @@ static int do_tpm_pcr_setauthvalue(struct cmd_tbl > *cmdtp, int flag, > key, key_sz)); > } > > +static int do_tpm2_get_test_result(struct cmd_tbl *cmdtp, int flag, int argc, > + char *const argv[]) > +{ > + struct udevice *dev; > + int ret; > + u8 data[TEST_RESULT_DATA_BUFFER_SIZE]; > + size_t data_len = TEST_RESULT_DATA_BUFFER_SIZE; = sizeof(data) > + u32 test_result; > + u32 rc; > + > + ret = get_tpm(&dev); > + if (ret) > + return ret; > + > + if (argc != 1) > + return CMD_RET_USAGE; Can you move this one above the get_tpm? > + > + rc = tpm2_get_test_result(dev, data, &data_len, &test_result); > + if (rc) > + goto out; > + > + printf("Test Result:\n\t0x%08X", test_result); > + switch (test_result) { > + case 0x0000: We have enum tpm2_return_codes. Please add the missing cases there and use that instead of magic values > + printf("\tTPM2_RC_SUCCESS\n"); > + break; > + case 0x0101: > + printf("\tTPM2_RC_FAILURE\n"); > + break; > + case 0x0153: > + printf("\tTPM2_RC_NEEDS_TEST\n"); > + break; > + case 0x090A: > + printf("\tTPM2_RC_TESTING\n"); > + break; > + } > + > + if (!data_len) { > + printf("No Test Result Data available\n"); > + goto out; > + } > + > + printf("Test Result Data of Self Test:\n\t0x"); > + for (int i = 0; i < data_len; i++) > + printf("%02X", data[i]); > + printf("\n"); > + > +out: > + return report_return_code(rc); > +} > + > static struct cmd_tbl tpm2_commands[] = { > U_BOOT_CMD_MKENT(device, 0, 1, do_tpm_device, "", ""), > U_BOOT_CMD_MKENT(info, 0, 1, do_tpm_info, "", ""), > @@ -374,6 +428,8 @@ static struct cmd_tbl tpm2_commands[] = { > do_tpm_pcr_setauthpolicy, "", ""), > U_BOOT_CMD_MKENT(pcr_setauthvalue, 0, 1, > do_tpm_pcr_setauthvalue, "", ""), > + U_BOOT_CMD_MKENT(get_test_result, 0, 1, > + do_tpm2_get_test_result, "", ""), > }; > > struct cmd_tbl *get_tpm2_commands(unsigned int *size) > @@ -447,4 +503,8 @@ U_BOOT_CMD(tpm2, CONFIG_SYS_MAXARGS, 1, do_tpm, "Issue a > TPMv2.x command", > " <pcr>: index of the PCR\n" > " <key>: secret to protect the access of PCR #<pcr>\n" > " <password>: optional password of the PLATFORM hierarchy\n" > +"get_test_result\n" > +" Show manufacturer-specific information regarding the results of a\n" > +" self-test and an indication of the test status.\n" > + > ); > diff --git a/drivers/tpm/tpm2_tis_sandbox.c b/drivers/tpm/tpm2_tis_sandbox.c > index e4004cfcca..17979cd33a 100644 > --- a/drivers/tpm/tpm2_tis_sandbox.c > +++ b/drivers/tpm/tpm2_tis_sandbox.c > @@ -2,6 +2,7 @@ > /* > * Copyright (c) 2018, Bootlin > * Author: Miquel Raynal <miquel.ray...@bootlin.com> > + * Copyright (C) 2023 Infineon Technologies AG > */ > > #include <common.h> > @@ -231,6 +232,7 @@ static int sandbox_tpm2_check_session(struct udevice > *dev, u32 command, u16 tag, > case TPM2_CC_SELF_TEST: > case TPM2_CC_GET_CAPABILITY: > case TPM2_CC_PCR_READ: > + case TPM2_CC_GET_TEST_RESULT: > if (tag != TPM2_ST_NO_SESSIONS) { > printf("No session required for command 0x%x\n", > command); > @@ -364,6 +366,13 @@ static int sandbox_tpm2_check_readyness(struct udevice > *dev, int command) > if (!tpm->startup_done) > return TPM2_RC_INITIALIZE; > > + break; > + case TPM2_CC_GET_TEST_RESULT: > + if (!tpm->init_done || !tpm->startup_done) > + return TPM2_RC_INITIALIZE; > + if (!tpm->tests_done) > + return TPM2_RC_NEEDS_TEST; > + > break; > default: > /* Skip this, since the startup may have happened in SPL > @@ -458,7 +467,7 @@ static int sandbox_tpm2_xfer(struct udevice *dev, const > u8 *sendbuf, > command = get_unaligned_be32(sent); > sent += sizeof(command); > rc = sandbox_tpm2_check_readyness(dev, command); > - if (rc) { > + if (rc && rc != TPM2_RC_NEEDS_TEST) { > sandbox_tpm2_fill_buf(recv, recv_len, tag, rc); > return 0; > } > @@ -778,6 +787,42 @@ static int sandbox_tpm2_xfer(struct udevice *dev, const > u8 *sendbuf, > *recv_len = 12; > memset(recvbuf, '\0', *recv_len); > break; > + > + case TPM2_CC_GET_TEST_RESULT: { > + u32 testresult = 0; > + u8 data[10] = { 0 }; > + > + /* Check readiness */ > + testresult = sandbox_tpm2_check_readyness(dev, command); > + > + /* Write tag */ > + put_unaligned_be16(tag, recv); > + recv += sizeof(tag); > + > + /* Ignore length for now */ > + recv += sizeof(u32); > + > + /* Write return code */ > + put_unaligned_be32(rc, recv); > + recv += sizeof(rc); > + > + /* Write manufacturer-specific test result data */ > + memcpy(recv, data, sizeof(data)); > + recv += sizeof(data); > + > + /* Write test result */ > + put_unaligned_be32(testresult, recv); > + recv += sizeof(testresult); > + > + /* Add trailing \0 */ > + *recv = '\0'; > + > + /* Write response length */ > + *recv_len = recv - recvbuf; > + put_unaligned_be32(*recv_len, recvbuf + sizeof(tag)); > + > + break; > + } > default: > printf("TPM2 command %02x unknown in Sandbox\n", command); > rc = TPM2_RC_COMMAND_CODE; > diff --git a/include/tpm-v2.h b/include/tpm-v2.h > index 2b6980e441..f6ec430316 100644 > --- a/include/tpm-v2.h > +++ b/include/tpm-v2.h > @@ -3,6 +3,7 @@ > * Defines APIs and structures that allow software to interact with a > * TPM2 device > * > + * Copyright (C) 2023 Infineon Technologies AG > * Copyright (c) 2020 Linaro > * Copyright (c) 2018 Bootlin > * > @@ -276,6 +277,7 @@ enum tpm2_handles { > * @TPM2_CC_DAM_PARAMETERS: TPM2_DictionaryAttackParameters(). > * @TPM2_CC_GET_CAPABILITY: TPM2_GetCapibility(). > * @TPM2_CC_GET_RANDOM: TPM2_GetRandom(). > + * @TPM2_CC_GET_TEST_RESULT: TPM2_GetTestResult(). > * @TPM2_CC_PCR_READ: TPM2_PCR_Read(). > * @TPM2_CC_PCR_EXTEND: TPM2_PCR_Extend(). > * @TPM2_CC_PCR_SETAUTHVAL: TPM2_PCR_SetAuthValue(). > @@ -296,6 +298,7 @@ enum tpm2_command_codes { > TPM2_CC_NV_READ = 0x014E, > TPM2_CC_GET_CAPABILITY = 0x017A, > TPM2_CC_GET_RANDOM = 0x017B, > + TPM2_CC_GET_TEST_RESULT = 0x017C, > TPM2_CC_PCR_READ = 0x017E, > TPM2_CC_PCR_EXTEND = 0x0182, > TPM2_CC_PCR_SETAUTHVAL = 0x0183, > @@ -706,4 +709,24 @@ u32 tpm2_enable_nvcommits(struct udevice *dev, uint > vendor_cmd, > */ > u32 tpm2_auto_start(struct udevice *dev); > > +/** > + * Issue a TPM2_GetTestResult command. > + * > + * @dev TPM device > + * @data output buffer for manufacturer-specific test result data > + * @data_len input/output parameter > + * input: size of data buffer, output: size of test result data > + * @test_result output parameter: test result response code: > + * TPM2_RC_NEEDS_TEST, if TPM2 self-test has not been executed > and > + * a testable function has not been tested > + * TPM2_RC_TESTING, if TPM2 self-test is in progress. > + * TPM2_RC_SUCCESS, if testing of all functions is complete > without > + * functional failures. > + * TPM2_RC_FAILURE, if any test failed. > + * > + * Return: code of the operation > + */ > +u32 tpm2_get_test_result(struct udevice *dev, void *data, size_t *data_len, > + u32 *test_result); > + > #endif /* __TPM_V2_H */ > diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c > index 9ab5b46df1..a3a29048e3 100644 > --- a/lib/tpm-v2.c > +++ b/lib/tpm-v2.c > @@ -2,12 +2,14 @@ > /* > * Copyright (c) 2018 Bootlin > * Author: Miquel Raynal <miquel.ray...@bootlin.com> > + * Copyright (C) 2023 Infineon Technologies AG > */ > > #include <common.h> > #include <dm.h> > #include <tpm-common.h> > #include <tpm-v2.h> > +#include <asm/unaligned.h> > #include <linux/bitops.h> > #include "tpm-utils.h" > > @@ -742,3 +744,83 @@ u32 tpm2_enable_nvcommits(struct udevice *dev, uint > vendor_cmd, > > return 0; > } > + > +u32 tpm2_get_test_result(struct udevice *dev, void *data, > + size_t *data_len, u32 *test_result) > +{ > + const u8 command_v2[COMMAND_BUFFER_SIZE] = { > + /* header 10 bytes */ > + tpm_u16(TPM2_ST_NO_SESSIONS), /* TAG */ > + tpm_u32(10), /* Length */ > + tpm_u32(TPM2_CC_GET_TEST_RESULT), /* Command code */ > + }; > + > + int ret; > + u8 response[COMMAND_BUFFER_SIZE]; > + size_t response_len = COMMAND_BUFFER_SIZE; > + u32 response_size; /* Size of response > bytestream */ reposnse_size and response_len is a bit confusing. Please find some names that indicate the first one is the reserved size while the second is the actual > + unsigned int response_size_off; > + unsigned int data_off; > + unsigned int test_result_off; > + size_t data_buff_len; > + > + ret = tpm_sendrecv_command(dev, command_v2, response, &response_len); > + > + if (ret) { > + log_debug("Returned on: ret = tpm_sendrecv_command()\n"); > + return ret; Isn't this an error? Can we use return log_msg_ret("test", ret); or something along those lines? > + } > + > + /* > + * Get responseSize from return bytestream, switch endianness: > big->little > + * In the response buffer, the responseSize is located after: > + * tag (u16) > + */ > + response_size_off = sizeof(u16); Keep the comment and just do response + sizeof(u16) > + response_size = get_unaligned_be32(response + response_size_off); > + > + if (response_len < 10) { > + log_debug > + ("Returned on plausibility check: response_len < 10\n"); Why is this an error? > + return -EINVAL; > + } > + > + if (response_size > response_len) { > + log_debug > + ("Returned on plausibility check: response_size > > response_len\n\t" Can you remove this line and just report the size mismatch? It should be obvious why it's wrong. > + "Response Size: %d\n\tResponse Length: %ld\n", > + response_size, response_len); > + return -E2BIG; > + } > + > + /* > + * Copy the test result data to buffer, transfer data_len > + * In the response buffer, the test result data is located after: > + * tag (u16), response size (u32), response code (u32). > + */ > + data_off = sizeof(u16) + sizeof(u32) + sizeof(u32); > + data_buff_len = *data_len; > + *data_len = response_size - data_off - sizeof(u32); > + > + /* Checks, if the reserved data buffer size is insufficient */ > + if (*data_len > data_buff_len) { > + log_debug > + ("Returned on plausibility check: data_len > > data_buff_len\n\t" > + "Data Length: %ld\n\tData Buffer Length: %ld\n\t", > + *data_len, data_buff_len); > + return -E2BIG; > + } > + > + memcpy(data, &response[data_off], *data_len); > + > + /* > + * Get testResult from return bytestream, switch endianness: > big->little > + * In the response buffer, the test result is located after: > + * tag (u16), response size (u32), response code (u32), > + * test result data (*data_len). > + */ > + test_result_off = data_off + *data_len; > + *test_result = get_unaligned_be32(response + test_result_off); > + > + return 0; > +} [...] Thanks /Ilias