From 30017e78bc900b1288489730ac3d53ea04802f1c Mon Sep 17 00:00:00 2001
From: Mark Salyzyn <salyzyn@google.com>
Date: Wed, 13 May 2020 12:39:12 -0700
Subject: [PATCH] recovery: fastbootd: retry opening graphics

Recovery and fastbootd hardening.

With GKI we find in certain situations the timing of the drivers
loading is delayed as compared to a monolithic kernel.  This
exasperates an existing race where during second stage init, the
graphics driver might not have completely instantiated by the time
fastboot or recovery menu ui is being set up.

To address this, we call gr_init() every 10ms until either 5 seconds
timeout or success.  If we timeout, there will be no TUI (minui), but
this is better than if we just tried once and dropped the TUI and was
effectively running headless.  5 seconds timeout is arbitrary, based
on the default in init for wait for file; but makes sense as any
longer will impact flashstation and fastboot flashall enough to be
a cause for consternation.

It should be noted that both recovery and fastbootd can still service
adb or fastboot gadgets headless.  For instance if the device tree is
misconfigured as a permanent issue that would head for the timeout.
Prefering to give up after a timeout so that device can be flashed by
fastbootd, or rebooted by recovery adbd, over the protocol is
advantageous for tool stability.

Architectural Concern:

The graphics driver, if not well written, may panic the kernel if we
try to access it too soon while it is probing.  On such devices it
will be necessary to hold off in 'on init' or 'on early-init' phases
until the graphics drivers have completely probed.  Or better yet,
fix the driver.  This problem would happen in any case occasionally
even without this adjustments, but could conceivably be amplified by
the loop trying to open the graphics device.

Signed-off-by: Mark Salyzyn <salyzyn@google.com>
Bug: 151950334
Test: make sure user space fastbootd comes up reliably for a GKI kernel
Change-Id: I1ce31a01544a58cdf7b9d657c1146bee77894e46
---
 recovery_ui/screen_ui.cpp | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/recovery_ui/screen_ui.cpp b/recovery_ui/screen_ui.cpp
index 6dcb161f..b2c828f3 100644
--- a/recovery_ui/screen_ui.cpp
+++ b/recovery_ui/screen_ui.cpp
@@ -37,6 +37,7 @@
 #include <unordered_map>
 #include <vector>
 
+#include <android-base/chrono_utils.h>
 #include <android-base/logging.h>
 #include <android-base/properties.h>
 #include <android-base/stringprintf.h>
@@ -881,10 +882,28 @@ bool ScreenRecoveryUI::LoadWipeDataMenuText() {
   return true;
 }
 
+static bool InitGraphics() {
+  // Timeout is same as init wait for file default of 5 seconds and is arbitrary
+  const unsigned timeout = 500;  // 10ms increments
+  for (auto retry = timeout; retry > 0; --retry) {
+    if (gr_init() == 0) {
+      if (retry < timeout) {
+        // Log message like init wait for file completion log for consistency.
+        LOG(WARNING) << "wait for 'graphics' took " << ((timeout - retry) * 10) << "ms";
+      }
+      return true;
+    }
+    std::this_thread::sleep_for(10ms);
+  }
+  // Log message like init wait for file timeout log for consistency.
+  LOG(ERROR) << "timeout wait for 'graphics' took " << (timeout * 10) << "ms";
+  return false;
+}
+
 bool ScreenRecoveryUI::Init(const std::string& locale) {
   RecoveryUI::Init(locale);
 
-  if (gr_init() == -1) {
+  if (!InitGraphics()) {
     return false;
   }