vboot: don't unset VB2_NV_DISPLAY_REQUEST in EC sync

VB2_NV_DISPLAY_REQUEST disabling code has been relocated to
VbBootNormal.  Remove from EC sync code.

Remove the vb2_shared_data argument from
check_reboot_for_display.

Avoid calling ec_sync_check_aux_fw after phase 1 if already
certain that a reboot is required.

BUG=b:124141368, chromium:948592, chromium:967298
TEST=make clean && make runtests
BRANCH=none

Change-Id: Ia5472aceb0b2a415f24dd76d26179632009d07cb
Signed-off-by: Joel Kitching <kitching@google.com>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/1634453
Commit-Queue: Jason Clinton <jclinton@chromium.org>
Tested-by: Jason Clinton <jclinton@chromium.org>
Reviewed-by: Julius Werner <jwerner@chromium.org>

3 files changed, 9 insertions, 42 deletions

diff --git a/firmware/lib/ec_sync.c b/firmware/lib/ec_sync.c
index bcd10e71..b6729985 100644
--- a/firmware/lib/ec_sync.c
+++ b/firmware/lib/ec_sync.c
@@ -254,9 +254,6 @@ static VbError_t sync_one_ec(struct vb2_context *ctx, int devidx)
 		 * TODO: Switch slot and proceed if EC is still in RO. */
 		if (is_rw_ab) {
 			VB2_DEBUG("Rebooting to jump to new EC-RW\n");
-			/* Since we're rebooting, ec_sync_all won't be given a
-			 * chance to clear the flag. Need to clear it here. */
-			vb2_nv_set(ctx, VB2_NV_DISPLAY_REQUEST, 0);
 			return VBERROR_EC_REBOOT_TO_SWITCH_RW;
 		}
 	}
diff --git a/firmware/lib/ec_sync_all.c b/firmware/lib/ec_sync_all.c
index b2506779..7e6cbf34 100644
--- a/firmware/lib/ec_sync_all.c
+++ b/firmware/lib/ec_sync_all.c
@@ -17,30 +17,9 @@
 #include "vboot_display.h"
 #include "vboot_kernel.h"
 
-static VbError_t ec_sync_disable_display(struct vb2_context *ctx,
-					 int need_wait_screen)
+static int check_reboot_for_display(struct vb2_context *ctx)
 {
-	/*
-	 * Reboot to disable display initialization
-	 * - we displayed the EC wait screen (otherwise we may be interfering
-	 *   with some other vboot feature requesting display initialization)
-	 * - vboot requested display to be initialized on this boot
-	 * - the system is NOT in developer mode (which will also need display)
-	 */
-	if (need_wait_screen &&
-	    vb2_nv_get(ctx, VB2_NV_DISPLAY_REQUEST) &&
-	    !(vb2_get_sd(ctx)->vbsd->flags & VBSD_BOOT_DEV_SWITCH_ON)) {
-		VB2_DEBUG("Reboot to undo display initialization\n");
-		vb2_nv_set(ctx, VB2_NV_DISPLAY_REQUEST, 0);
-		return VBERROR_REBOOT_REQUIRED;
-	}
-	return VBERROR_SUCCESS;
-}
-
-static int check_reboot_for_display(struct vb2_context *ctx,
-					struct vb2_shared_data *sd)
-{
-	if (!(sd->flags & VB2_SD_FLAG_DISPLAY_AVAILABLE)) {
+	if (!(vb2_get_sd(ctx)->flags & VB2_SD_FLAG_DISPLAY_AVAILABLE)) {
 		VB2_DEBUG("Reboot to initialize display\n");
 		vb2_nv_set(ctx, VB2_NV_DISPLAY_REQUEST, 1);
 		return 1;
@@ -56,7 +35,6 @@ static void display_wait_screen(struct vb2_context *ctx, const char *fw_name)
 
 VbError_t ec_sync_all(struct vb2_context *ctx)
 {
-	struct vb2_shared_data *sd = vb2_get_sd(ctx);
 	VbAuxFwUpdateSeverity_t fw_update = VB_AUX_FW_NO_UPDATE;
 	VbError_t rv;
 
@@ -69,14 +47,14 @@ VbError_t ec_sync_all(struct vb2_context *ctx)
 		ec_sync_check_aux_fw(ctx, &fw_update);
 		/* It does -- speculatively check if we need display as well */
 		if (need_wait_screen || fw_update == VB_AUX_FW_SLOW_UPDATE)
-			check_reboot_for_display(ctx, sd);
+			check_reboot_for_display(ctx);
 		return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
 	}
 
 	/* Is EC already in RO and needs slow update? */
 	if (need_wait_screen) {
 		/* Might still need display in that case */
-		if (check_reboot_for_display(ctx, sd))
+		if (check_reboot_for_display(ctx))
 			return VBERROR_REBOOT_REQUIRED;
 		/* Display is available, so pop up the wait screen */
 		display_wait_screen(ctx, "EC FW");
@@ -94,9 +72,8 @@ VbError_t ec_sync_all(struct vb2_context *ctx)
 
 	/* If AUX FW update is slow display the wait screen */
 	if (fw_update == VB_AUX_FW_SLOW_UPDATE) {
-		need_wait_screen = 1;
 		/* Display should be available, but better check again */
-		if (check_reboot_for_display(ctx, sd))
+		if (check_reboot_for_display(ctx))
 			return VBERROR_REBOOT_REQUIRED;
 		display_wait_screen(ctx, "AUX FW");
 	}
@@ -107,13 +84,6 @@ VbError_t ec_sync_all(struct vb2_context *ctx)
 	 * disable display request if needed to prevent a second reboot.
 	 */
 	rv = ec_sync_update_aux_fw(ctx);
-	if (rv) {
-		ec_sync_disable_display(ctx, need_wait_screen);
-		return rv;
-	}
-
-	/* Reboot to disable display initialization if needed */
-	rv = ec_sync_disable_display(ctx, need_wait_screen);
 	if (rv)
 		return rv;
 
diff --git a/tests/ec_sync_tests.c b/tests/ec_sync_tests.c
index 3f8b7a52..1698e032 100644
--- a/tests/ec_sync_tests.c
+++ b/tests/ec_sync_tests.c
@@ -376,11 +376,11 @@ static void VbSoftwareSyncTest(void)
 	mock_ec_rw_hash[0]++;
 	ctx.flags |= VB2_CONTEXT_EC_SYNC_SLOW;
 	vb2_nv_set(&ctx, VB2_NV_DISPLAY_REQUEST, 1);
-	test_ssync(VBERROR_REBOOT_REQUIRED, 0,
+	test_ssync(VBERROR_SUCCESS, 0,
 		   "Slow update with display request");
 	TEST_EQ(screens_displayed[0], VB_SCREEN_WAIT, "  wait screen");
-	TEST_EQ(vb2_nv_get(&ctx, VB2_NV_DISPLAY_REQUEST), 0,
-		"  DISPLAY_REQUEST disabled");
+	TEST_EQ(vb2_nv_get(&ctx, VB2_NV_DISPLAY_REQUEST), 1,
+		"  DISPLAY_REQUEST left untouched");
 
 	ResetMocks();
 	mock_ec_rw_hash[0]++;
@@ -390,7 +390,7 @@ static void VbSoftwareSyncTest(void)
 		   "Slow update without display request (no reboot needed)");
 	TEST_EQ(screens_displayed[0], VB_SCREEN_WAIT, "  wait screen");
 	TEST_EQ(vb2_nv_get(&ctx, VB2_NV_DISPLAY_REQUEST), 0,
-		"  DISPLAY_REQUEST still disabled");
+		"  DISPLAY_REQUEST left untouched");
 
 	/* RW cases, no update */
 	ResetMocks();