From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:400e:c00::243; helo=mail-pf0-x243.google.com; envelope-from=heyi.guo@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-pf0-x243.google.com (mail-pf0-x243.google.com [IPv6:2607:f8b0:400e:c00::243]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id A8E88223522AB for ; Wed, 28 Feb 2018 18:33:55 -0800 (PST) Received: by mail-pf0-x243.google.com with SMTP id 68so1861771pfx.3 for ; Wed, 28 Feb 2018 18:40:04 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id; bh=fZ2vUEhJeGYu+b1gZDK94D82O6zxkcqkTobKP9Ei32k=; b=OlQMsDbjxnW19k6D6x1rESx2B2EZ02r/eTvBsOYakSy6mAFqRb3U1ziDodcnc/fvbt TQUj9xK1u46F4zos2BkZS8uZUhjqV3fdWiQj4dQndOI75PB1dcOkA31ZaPT1TKHXtIAz yVSeLXnu1sdJ+k7AlWiyaC/NqGUNDXW5/9ks8= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=fZ2vUEhJeGYu+b1gZDK94D82O6zxkcqkTobKP9Ei32k=; b=BByMTAEhqYwMMmlo/QSpurrhx85VPL5Xk+nW6tZt29KSWOpz686TtWQtPR09pd5hNf gxCWXYqAhC6yZRJp5l0YQtpXQE9V6D5kKUrVyPjWMzM5Ye69bZn2mHnEGJoDV2AEJK1+ V+qzvVOLGS9Z0LoTza5FHlGRv/VH7Sd2qPn6zW7r/PCVlfq7e2/NoxoU2ok+hNXfuiuA AcAZRnDfH+H6DDV2dN8HxnjvoKw5+TK3iFRzwpDAMI65yCsOWYZsuodu2Jmdpencbj9d esJHQ80abhA5qInr5S15EHpMTtrI/1+5X6RLnYg1tPIFGNu8AY5Jq0uIjkBN9xZbrmt1 jvGA== X-Gm-Message-State: APf1xPBWcEwFsHVjDYpM0Vnxm2Xl1TxML4cgqqebFbz5+JcCPpGqddV5 Bz+s5R8qrSylwfmlIaUsSFSOI6U/TbU= X-Google-Smtp-Source: AG47ELuKtTZh1rKZ6rDj4eZyuWjt7155w8OHmWxhPEfEGOdGqcCtAdWt3rsFlZkdYoV3NJZnIbR2KQ== X-Received: by 10.98.71.3 with SMTP id u3mr288200pfa.219.1519872003450; Wed, 28 Feb 2018 18:40:03 -0800 (PST) Received: from localhost.localdomain ([45.56.152.115]) by smtp.gmail.com with ESMTPSA id l26sm6015849pfj.112.2018.02.28.18.40.01 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Wed, 28 Feb 2018 18:40:02 -0800 (PST) From: Heyi Guo To: edk2-devel@lists.01.org Cc: Heyi Guo , Star Zeng , Eric Dong , Ruiyu Ni , Laszlo Ersek Date: Thu, 1 Mar 2018 10:39:32 +0800 Message-Id: <1519871972-39281-1-git-send-email-heyi.guo@linaro.org> X-Mailer: git-send-email 2.7.4 Subject: [PATCH 1/1] MdeModulePkg/UefiBootManagerLib: limit recursive call depth X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 01 Mar 2018 02:33:57 -0000 Function BmRepairAllControllers may recursively call itself if some driver health protocol returns EfiDriverHealthStatusReconnectRequired. However, driver health protocol of some buggy third party driver may always return such status even after one and another reconnect. The endless iteration will cause stack overflow and then system exception, and it may be not easy to find that the exception is actually caused by stack overflow. So we limit the number of reconnect retry to 10 to improve code robustness, and DEBUG_CODE is moved ahead before recursive repair to track the repair result. We also remove a duplicated declaration of BmRepairAllControllers() in InternalBm.h in this patch, for it is only a trivial change. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Heyi Guo Cc: Star Zeng Cc: Eric Dong Cc: Ruiyu Ni Cc: Laszlo Ersek --- Notes: v2 - Use argument instead of global variable to record the recursive count [Ray] - Move DEBUG_CODE before recursively calling BmRepairAllControllers() to track the change of each reconnect [Ray] - Remove a duplicated declaration of BmRepairAllControllers() in InternalBm.h. MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h | 19 ++++++++++--------- MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 2 +- MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c | 18 +++++++++++++----- 3 files changed, 24 insertions(+), 15 deletions(-) diff --git a/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h b/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h index 25a1d522fe84..21ecd8584d24 100644 --- a/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h +++ b/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h @@ -108,6 +108,12 @@ CHAR16 * #define BM_OPTION_NAME_LEN sizeof ("PlatformRecovery####") extern CHAR16 *mBmLoadOptionName[]; +// +// Maximum number of reconnect retry to repair controller; it is to limit the +// number of recursive call of BmRepairAllControllers. +// +#define MAX_RECONNECT_REPAIR 10 + /** Visitor function to be called by BmForEachVariable for each variable in variable storage. @@ -145,10 +151,13 @@ typedef struct { /** Repair all the controllers according to the Driver Health status queried. + + @param ReconnectRepairCount To record the number of recursive call of + this function itself. **/ VOID BmRepairAllControllers ( - VOID + UINTN ReconnectRepairCount ); #define BM_HOTKEY_SIGNATURE SIGNATURE_32 ('b', 'm', 'h', 'k') @@ -328,14 +337,6 @@ BmDelPartMatchInstance ( ); /** - Repair all the controllers according to the Driver Health status queried. -**/ -VOID -BmRepairAllControllers ( - VOID - ); - -/** Print the device path info. @param DevicePath The device path need to print. diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c index ce19ae400660..b842d5824aed 100644 --- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c @@ -1767,7 +1767,7 @@ EfiBootManagerBoot ( // // 4. Repair system through DriverHealth protocol // - BmRepairAllControllers (); + BmRepairAllControllers (0); } PERF_START_EX (gImageHandle, "BdsAttempt", NULL, 0, (UINT32) OptionNumber); diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c b/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c index ddcee8b0676f..db2f859ae73d 100644 --- a/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c @@ -423,10 +423,13 @@ EfiBootManagerFreeDriverHealthInfo ( /** Repair all the controllers according to the Driver Health status queried. + + @param ReconnectRepairCount To record the number of recursive call of + this function itself. **/ VOID BmRepairAllControllers ( - VOID + UINTN ReconnectRepairCount ) { EFI_STATUS Status; @@ -548,10 +551,6 @@ BmRepairAllControllers ( EfiBootManagerFreeDriverHealthInfo (DriverHealthInfo, Count); - if (ReconnectRequired) { - BmRepairAllControllers (); - } - DEBUG_CODE ( CHAR16 *ControllerName; @@ -576,6 +575,15 @@ BmRepairAllControllers ( EfiBootManagerFreeDriverHealthInfo (DriverHealthInfo, Count); ); + if (ReconnectRequired) { + if (ReconnectRepairCount < MAX_RECONNECT_REPAIR) { + BmRepairAllControllers (ReconnectRepairCount + 1); + } else { + DEBUG ((DEBUG_ERROR, "[%a:%d] Repair failed after %d retries.\n", + __FUNCTION__, __LINE__, ReconnectRepairCount)); + } + } + if (RebootRequired) { DEBUG ((EFI_D_INFO, "[BDS] One of the Driver Health instances requires rebooting.\n")); gRT->ResetSystem (EfiResetWarm, EFI_SUCCESS, 0, NULL); -- 2.7.4