[coreboot-gerrit] Change in coreboot[master]: cr50: add unmarshaling of vendor commands and process 'enabl...

Vadim Bendebury (Code Review) gerrit at coreboot.org
Thu Mar 23 23:50:02 CET 2017


Vadim Bendebury has submitted this change and it was merged. ( https://review.coreboot.org/18945 )

Change subject: cr50: add unmarshaling of vendor commands and process 'enable_update'
......................................................................


cr50: add unmarshaling of vendor commands and process 'enable_update'

The upcoming Cr50 firmware changes will require the AP to enable the
previously downloaded Cr50 firmware update(s).

A new vendor command (TPM2_CR50_SUB_CMD_TURN_UPDATE_ON) is used for
that. The command accepts one parameter - a timeout value in range of
0 to 1000 ms.

When processing the command the Cr50 checks if the alternative RO or
RW image(s) need to be enabled, and if so - enables them and returns
to the host the number of enabled headers.

If the vendor command requested a non-zero timeout, the Cr50 starts
a timer to trigger system reboot after the requested timeout expires.

The host acts on the number of enabled headers - if the number is
nonzero, the host prepares the device to be reset and waits for the
Cr50 to reboot the device after timeout expires.

This patch also adds more formal vendor command
marshaling/unmarshaling to make future additions easier.

BRANCH=gru,reef
BUG=b:35580805
TEST=with the actual user of this code in the next patch verified that
     the cr50 update is enabled as expected.

Change-Id: Ic76d384d637c0eeaad206e0a8242cbb8e2b19b37
Signed-off-by: Vadim Bendebury <vbendeb at chromium.org>
Reviewed-on: https://review.coreboot.org/18945
Reviewed-by: Paul Menzel <paulepanter at users.sourceforge.net>
Tested-by: build bot (Jenkins)
Reviewed-by: Aaron Durbin <adurbin at chromium.org>
---
M src/include/tpm_lite/tlcl.h
M src/lib/tpm2_marshaling.c
M src/lib/tpm2_tlcl.c
M src/lib/tpm2_tlcl_structures.h
4 files changed, 65 insertions(+), 10 deletions(-)

Approvals:
  Aaron Durbin: Looks good to me, approved
  Paul Menzel: Looks good to me, but someone else must approve
  build bot (Jenkins): Verified



diff --git a/src/include/tpm_lite/tlcl.h b/src/include/tpm_lite/tlcl.h
index c8e68d2..8dd5d80 100644
--- a/src/include/tpm_lite/tlcl.h
+++ b/src/include/tpm_lite/tlcl.h
@@ -162,4 +162,16 @@
  */
 uint32_t tlcl_cr50_enable_nvcommits(void);
 
+/**
+ * CR50 specific tpm command to restore header(s) of the dormant RO/RW
+ * image(s) and in case there indeed was a dormant image, trigger reboot after
+ * the timeout milliseconds. Note that timeout of zero means "NO REBOOT", not
+ * "IMMEDIATE REBOOT".
+ *
+ * Return value indicates success or failure of accessing the TPM; in case of
+ * success the number of restored headers is saved in num_restored_headers.
+ */
+uint32_t tlcl_cr50_enable_update(uint16_t timeout_ms,
+				 uint8_t *num_restored_headers);
+
 #endif  /* TPM_LITE_TLCL_H_ */
diff --git a/src/lib/tpm2_marshaling.c b/src/lib/tpm2_marshaling.c
index 6d4d622..52dac9c 100644
--- a/src/lib/tpm2_marshaling.c
+++ b/src/lib/tpm2_marshaling.c
@@ -394,19 +394,15 @@
 			size_t *buffer_space)
 {
 	uint16_t *sub_command;
-
-	/* Ensure at least the sub command can fit in the body and there's
-	   valid pointer. Could be reading past the buffer... */
-	if (command_body == NULL || *buffer_space < sizeof(uint16_t)) {
-		*buffer_space = 0;
-		return;
-	}
-
 	sub_command = command_body;
 
 	switch (*sub_command) {
 	case TPM2_CR50_SUB_CMD_NVMEM_ENABLE_COMMITS:
 		marshal_u16(buffer, *sub_command, buffer_space);
+		break;
+	case TPM2_CR50_SUB_CMD_TURN_UPDATE_ON:
+		marshal_u16(buffer, sub_command[0], buffer_space);
+		marshal_u16(buffer, sub_command[1], buffer_space);
 		break;
 	default:
 		/* Unsupported subcommand. */
@@ -596,6 +592,25 @@
 	*size = 0;
 }
 
+static void unmarshal_vendor_command(void **buffer, int *size,
+				     struct vendor_command_response *vcr)
+{
+	vcr->vc_subcommand = unmarshal_u16(buffer, size);
+
+	switch (vcr->vc_subcommand) {
+	case TPM2_CR50_SUB_CMD_NVMEM_ENABLE_COMMITS:
+		break;
+	case TPM2_CR50_SUB_CMD_TURN_UPDATE_ON:
+		vcr->num_restored_headers = unmarshal_u8(buffer, size);
+		break;
+	default:
+		printk(BIOS_ERR,
+		       "%s:%d - unsupported vendor command %#04x!\n",
+		       __func__, __LINE__, vcr->vc_subcommand);
+		break;
+	}
+}
+
 struct tpm2_response *tpm_unmarshal_response(TPM_CC command,
 					     void *response_body,
 					     size_t in_size)
@@ -648,8 +663,8 @@
 		break;
 
 	case TPM2_CR50_VENDOR_COMMAND:
-		/* Assume no other data returned for the time being. */
-		cr_size = 0;
+		unmarshal_vendor_command(&response_body, &cr_size,
+					 &tpm2_resp->vcr);
 		break;
 
 	default:
diff --git a/src/lib/tpm2_tlcl.c b/src/lib/tpm2_tlcl.c
index 967612a..1118afb 100644
--- a/src/lib/tpm2_tlcl.c
+++ b/src/lib/tpm2_tlcl.c
@@ -406,3 +406,22 @@
 	}
 	return TPM_SUCCESS;
 }
+
+uint32_t tlcl_cr50_enable_update(uint16_t timeout_ms,
+				 uint8_t *num_restored_headers)
+{
+	struct tpm2_response *response;
+	uint16_t command_body[] = {
+		TPM2_CR50_SUB_CMD_TURN_UPDATE_ON, timeout_ms
+	};
+
+	printk(BIOS_INFO, "Checking cr50 for pending updates\n");
+
+	response = tpm_process_command(TPM2_CR50_VENDOR_COMMAND, command_body);
+
+	if (!response || response->hdr.tpm_code)
+		return TPM_E_INTERNAL_INCONSISTENCY;
+
+	*num_restored_headers = response->vcr.num_restored_headers;
+	return TPM_SUCCESS;
+}
diff --git a/src/lib/tpm2_tlcl_structures.h b/src/lib/tpm2_tlcl_structures.h
index ec5b674..2d6164b 100644
--- a/src/lib/tpm2_tlcl_structures.h
+++ b/src/lib/tpm2_tlcl_structures.h
@@ -78,6 +78,7 @@
    knowledge of all commands. */
 #define TPM2_CR50_VENDOR_COMMAND ((TPM_CC)(TPM_CC_VENDOR_BIT_MASK | 0))
 #define  TPM2_CR50_SUB_CMD_NVMEM_ENABLE_COMMITS (21)
+#define  TPM2_CR50_SUB_CMD_TURN_UPDATE_ON (24)
 
 /* Startup values. */
 #define TPM_SU_CLEAR 0
@@ -279,6 +280,13 @@
 	TPM2B_MAX_NV_BUFFER buffer;
 };
 
+struct vendor_command_response {
+	uint16_t vc_subcommand;
+	union {
+		uint8_t num_restored_headers;
+	};
+};
+
 struct tpm2_session_attrs {
 	uint8_t continueSession : 1;
 	uint8_t auditExclusive  : 1;
@@ -311,6 +319,7 @@
 		struct get_cap_response gc;
 		struct nv_read_response nvr;
 		struct tpm2_session_header def_space;
+		struct vendor_command_response vcr;
 	};
 };
 

-- 
To view, visit https://review.coreboot.org/18945
To unsubscribe, visit https://review.coreboot.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ic76d384d637c0eeaad206e0a8242cbb8e2b19b37
Gerrit-PatchSet: 6
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Owner: Vadim Bendebury <vbendeb at chromium.org>
Gerrit-Reviewer: Aaron Durbin <adurbin at chromium.org>
Gerrit-Reviewer: Duncan Laurie <dlaurie at chromium.org>
Gerrit-Reviewer: Furquan Shaikh <furquan at google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter at users.sourceforge.net>
Gerrit-Reviewer: Vadim Bendebury <vbendeb at chromium.org>
Gerrit-Reviewer: build bot (Jenkins)



More information about the coreboot-gerrit mailing list