From 30017e78bc900b1288489730ac3d53ea04802f1c Mon Sep 17 00:00:00 2001 From: Mark Salyzyn Date: Wed, 13 May 2020 12:39:12 -0700 Subject: 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 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 6dcb161fa..b2c828f34 100644 --- a/recovery_ui/screen_ui.cpp +++ b/recovery_ui/screen_ui.cpp @@ -37,6 +37,7 @@ #include #include +#include #include #include #include @@ -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; } -- cgit v1.2.3