From patchwork Thu Mar 1 02:39:32 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: gary guo X-Patchwork-Id: 130098 Delivered-To: patch@linaro.org Received: by 10.46.66.2 with SMTP id p2csp476631lja; Wed, 28 Feb 2018 18:40:07 -0800 (PST) X-Google-Smtp-Source: AG47ELvybXa6nHi6denFY0T/ka83w4yhmPxjeEK468eVaN9qUNk7KbBO+g1IYp7N6BYtjuqwtKVm X-Received: by 10.98.198.146 with SMTP id x18mr325245pfk.22.1519872007313; Wed, 28 Feb 2018 18:40:07 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1519872007; cv=none; d=google.com; s=arc-20160816; b=kkZl2MMCzcs+hD5edNjD5LQUePXmOEFHaGAdi4FQwyNvbVc8HbzA0bo7M7OBFOfI29 NJhLJ7E1fTq5hOdecSbv0tRwQEJxpfh4duPkST7TVvDhy2jdD3gXlAw4lBFHR7/kcmS1 aGPsRKG6L655esxKiGiQRqnqB8bc09oPB32QCKf1RrpQL5K7pEtns8EHqj3a8ZFxLF4a pD61uUSHcn3Wf25+d+ofB/xoSO341XnxvObcLTHDM/o1kk9TktZaUpnmH9j54albUS6U OFYqH5FmeXFtizBdPgQAgXQCKByNQ9UZcJRytv1FrfrPzgyHC6vTlUODZ+xIsQvTToF2 RlPA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:content-transfer-encoding:mime-version:cc :list-subscribe:list-help:list-post:list-archive:list-unsubscribe :list-id:precedence:subject:message-id:date:to:from:dkim-signature :delivered-to:arc-authentication-results; bh=ot3BzuicnqJxHBRMfmkB0Q3THcxDU+Sr2GHPTROMU+k=; b=HkFMmIChGk+tij7d50IKqCtCYnOOdYpfJamC9xBZwWS1WN3B+EV5ZVwwKrJrebsZft OMcmC8ZCtkuKPnIa85M3OiTyHN9mlgXfqXrLOq9oYPFbpXmtZyUYgn8NwxVOXQU4QFIQ DmgZgoLLbVUXDQe80hxn/lJEK9bIKtcwdSL+4b0ZDsrwYQPnIxu7ZuwAg/4KPRD9bWk1 sxeE8ZXZi3wHTTUjWmPfUi1qAz8dVioUITvvPZMzmboy8xB++ZS6+i7QVG7dN6QpobZJ bCvgC33a5a4Uc/5RuC2GMcc6xPTBLz2aER2o/mwt+bfOPIf/E1Q72tXk+D/JIsI194Gy w9sA== ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@linaro.org header.s=google header.b=OlQMsDbj; spf=pass (google.com: best guess record for domain of edk2-devel-bounces@lists.01.org designates 198.145.21.10 as permitted sender) smtp.mailfrom=edk2-devel-bounces@lists.01.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from ml01.01.org (ml01.01.org. [198.145.21.10]) by mx.google.com with ESMTPS id i189si1826014pgc.505.2018.02.28.18.40.06 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 28 Feb 2018 18:40:07 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of edk2-devel-bounces@lists.01.org designates 198.145.21.10 as permitted sender) client-ip=198.145.21.10; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@linaro.org header.s=google header.b=OlQMsDbj; spf=pass (google.com: best guess record for domain of edk2-devel-bounces@lists.01.org designates 198.145.21.10 as permitted sender) smtp.mailfrom=edk2-devel-bounces@lists.01.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: from [127.0.0.1] (localhost [IPv6:::1]) by ml01.01.org (Postfix) with ESMTP id 25092223522B5; Wed, 28 Feb 2018 18:33:58 -0800 (PST) X-Original-To: edk2-devel@lists.01.org Delivered-To: edk2-devel@lists.01.org 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-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 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: [edk2] [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: , Cc: Ruiyu Ni , Heyi Guo , Laszlo Ersek , Eric Dong , Star Zeng MIME-Version: 1.0 Errors-To: edk2-devel-bounces@lists.01.org Sender: "edk2-devel" 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(-) -- 2.7.4 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel Reviewed-by: Ruiyu Ni 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);