[coreboot-gerrit] Change in coreboot[master]: tpm/tspi: clean up tpm_setup function flow

[Philipp Deppenwiese (Code Review)]

Philipp Deppenwiese has submitted this change and it was merged. ( https://review.coreboot.org/29026 )

Change subject: tpm/tspi: clean up tpm_setup function flow
......................................................................

tpm/tspi: clean up tpm_setup function flow

Introduce two helper functions for more readable code.
Use epilogue function instead of goto for error handling.

BUG=None
TEST=None

Change-Id: Ibea44880683a301e82ee2ba049003c36fcb44eba
Signed-off-by: Joel Kitching <kitching at google.com>
Reviewed-on: https://review.coreboot.org/29026
Tested-by: build bot (Jenkins) <no-reply at coreboot.org>
Reviewed-by: Aaron Durbin <adurbin at chromium.org>
Reviewed-by: Furquan Shaikh <furquan at google.com>
Reviewed-by: Philipp Deppenwiese <zaolin.daisuki at gmail.com>
---
M src/security/tpm/tspi/tspi.c
1 file changed, 46 insertions(+), 38 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Aaron Durbin: Looks good to me, approved
  Philipp Deppenwiese: Looks good to me, approved
  Furquan Shaikh: Looks good to me, approved



diff --git a/src/security/tpm/tspi/tspi.c b/src/security/tpm/tspi/tspi.c
index c1779e6..d9cade9 100644
--- a/src/security/tpm/tspi/tspi.c
+++ b/src/security/tpm/tspi/tspi.c
@@ -64,6 +64,43 @@
 }
 #endif
 
+static uint32_t tpm_setup_s3_helper(void)
+{
+	uint32_t result;
+
+	result = tlcl_resume();
+	switch (result) {
+	case TPM_SUCCESS:
+		break;
+
+	case TPM_E_INVALID_POSTINIT:
+		/*
+		 * We're on a platform where the TPM maintains power
+		 * in S3, so it's already initialized.
+		 */
+		printk(BIOS_INFO, "TPM: Already initialized.\n");
+		result = TPM_SUCCESS;
+		break;
+
+	default:
+		printk(BIOS_ERR, "TPM: Resume failed (%#x).\n", result);
+		break;
+
+	}
+
+	return result;
+}
+
+static uint32_t tpm_setup_epilogue(uint32_t result)
+{
+	if (result != TPM_SUCCESS)
+		post_code(POST_TPM_FAILURE);
+	else
+		printk(BIOS_INFO, "TPM: setup succeeded\n");
+
+	return result;
+}
+
 /*
  * tpm_setup starts the TPM and establishes the root of trust for the
  * anti-rollback mechanism.  SetupTPM can fail for three reasons.  1 A bug. 2 a
@@ -91,37 +128,19 @@
 	result = tlcl_lib_init();
 	if (result != TPM_SUCCESS) {
 		printk(BIOS_ERR, "TPM: Can't initialize.\n");
-		goto out;
+		return tpm_setup_epilogue(result);
 	}
 
 	/* Handle special init for S3 resume path */
 	if (s3flag) {
-		result = tlcl_resume();
-		switch (result) {
-		case TPM_SUCCESS:
-			break;
-
-		case TPM_E_INVALID_POSTINIT:
-			/*
-			 * We're on a platform where the TPM maintains power
-			 * in S3, so it's already initialized.
-			 */
-			printk(BIOS_INFO, "TPM: Already initialized.\n");
-			result = TPM_SUCCESS;
-			break;
-
-		default:
-			printk(BIOS_ERR, "TPM: Resume failed (%#x).\n", result);
-			break;
-
-		}
-		goto out;
+		printk(BIOS_INFO, "TPM: Handle S3 resume.\n");
+		return tpm_setup_epilogue(tpm_setup_s3_helper());
 	}
 
 	result = tlcl_startup();
 	if (result != TPM_SUCCESS) {
 		printk(BIOS_ERR, "TPM: Can't run startup command.\n");
-		goto out;
+		return tpm_setup_epilogue(result);
 	}
 
 	result = tlcl_assert_physical_presence();
@@ -133,33 +152,22 @@
 		 */
 		result = tlcl_physical_presence_cmd_enable();
 		if (result != TPM_SUCCESS) {
-			printk(
-				BIOS_ERR,
-				"TPM: Can't enable physical presence command.\n");
-			goto out;
+			printk(BIOS_ERR, "TPM: Can't enable physical presence command.\n");
+			return tpm_setup_epilogue(result);
 		}
 
 		result = tlcl_assert_physical_presence();
 		if (result != TPM_SUCCESS) {
-			printk(BIOS_ERR,
-			       "TPM: Can't assert physical presence.\n");
-			goto out;
+			printk(BIOS_ERR, "TPM: Can't assert physical presence.\n");
+			return tpm_setup_epilogue(result);
 		}
 	}
 
 #if IS_ENABLED(CONFIG_TPM1)
 	result = tpm1_invoke_state_machine();
-	if (result != TPM_SUCCESS)
-		return result;
 #endif
 
-out:
-	if (result != TPM_SUCCESS)
-		post_code(POST_TPM_FAILURE);
-	else
-		printk(BIOS_INFO, "TPM: setup succeeded\n");
-
-	return result;
+	return tpm_setup_epilogue(result);
 }
 
 uint32_t tpm_clear_and_reenable(void)

-- 
To view, visit https://review.coreboot.org/29026
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ibea44880683a301e82ee2ba049003c36fcb44eba
Gerrit-Change-Number: 29026
Gerrit-PatchSet: 6
Gerrit-Owner: Joel Kitching <kitching at google.com>
Gerrit-Reviewer: Aaron Durbin <adurbin at chromium.org>
Gerrit-Reviewer: Furquan Shaikh <furquan at google.com>
Gerrit-Reviewer: Joel Kitching <kitching at google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter at users.sourceforge.net>
Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki at gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply at coreboot.org>
Gerrit-CC: Patrick Rudolph <siro at das-labor.org>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.coreboot.org/pipermail/coreboot-gerrit/attachments/20181018/55303d40/attachment.html>