From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4001:c0b::22f; helo=mail-it0-x22f.google.com; envelope-from=heyi.guo@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-it0-x22f.google.com (mail-it0-x22f.google.com [IPv6:2607:f8b0:4001:c0b::22f]) (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 B0F4C209574E5 for ; Mon, 26 Feb 2018 03:28:27 -0800 (PST) Received: by mail-it0-x22f.google.com with SMTP id a75so10431278itd.0 for ; Mon, 26 Feb 2018 03:34:32 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:date:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=2QL97J9aLjirvxM9HCZf9isG7cwx4LkCwOF0NK7HmOs=; b=PTqqZ6/peaWl/OH+Vufe0U965DFOqXzWlUru9tvsEQ2lliBICpRynq8o1ZUBX15YgT g1Ut1KGkYr7pZbuSuxZYz60QigLr29Ps8iz3m/00QeLp5Y3IjgMCG+Ycjr5wfVFhoy07 nmTu0nw1ntbiGqTEk+8OO8n17GTiSE/NmxdBs= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:date:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=2QL97J9aLjirvxM9HCZf9isG7cwx4LkCwOF0NK7HmOs=; b=KexyzYMZFMLZrWti49O7XnLldcsQxan0ksq2UChL5ZhzJYRa/DkgB8NSMyGC3qMJbK 6kz4iS6qzeUz2v3YWB2KKtBtt8G0QhkP3SxdSYxkKIhvwPSxzRZVad34/8sq5yDQ9v5d qm330AOkXEOdmM0RggPkxnUMU1/hae/96M1dWCCUTkZa7XCpoy3i9GjySasRP62vo2QQ 0qC2ehXU9NaJ7cVOKE5jgrlrwu5vHTiW7XhcT4iQOC4nl+ugCVdjm0sO4epavUyNpSC7 W7KosN56oNv+w2pUDpidxzWUZt6O0hKZhguVnDiW5mYHmOvAaLEdU3Iy2Z0xTO+/RHez 4tpw== X-Gm-Message-State: APf1xPC7KPO8cEWo53yW+4/qpYQnEL6hFi92EOfrHV4qWPnmDt3qVm78 2V7ru4GzgzBbIcD/+uSyjwGClg== X-Google-Smtp-Source: AH8x224Z+Yc0BnjdmSAmRmUHjY1Bdm4qfMHtOFmEABEdQq+l5Cx32Qt6hiVFh5Ce9iuR5thxcvcfqg== X-Received: by 10.36.131.3 with SMTP id d3mr12034551ite.149.1519644872307; Mon, 26 Feb 2018 03:34:32 -0800 (PST) Received: from SZX1000114654 ([104.237.91.49]) by smtp.gmail.com with ESMTPSA id b123sm5370121iti.22.2018.02.26.03.34.29 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 26 Feb 2018 03:34:31 -0800 (PST) From: Guo Heyi X-Google-Original-From: Guo Heyi Date: Mon, 26 Feb 2018 19:34:27 +0800 To: "Wang, Sunny (HPS SW)" Cc: Heyi Guo , "edk2-devel@lists.01.org" , Ruiyu Ni , Eric Dong , Star Zeng Message-ID: <20180226113427.GA113703@SZX1000114654> References: <1519633779-130687-1-git-send-email-heyi.guo@linaro.org> MIME-Version: 1.0 In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) Subject: Re: [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: Mon, 26 Feb 2018 11:28:29 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Hi Sunny, I didn't consider it as a value necessary for platform override, for the retry count should only have some impact on boot performance and it only happens when there is something wrong. May I know what value you will use for your platform and why? Thanks and regards, Gary (Heyi Guo) On Mon, Feb 26, 2018 at 08:56:50AM +0000, Wang, Sunny (HPS SW) wrote: > Hi Heyi, > Just a suggestion. > Is it better to use a PCD instead of a define for MAX_RECONNECT_REPAIR? So that we can easily override it in our platform dsc file. > > Regards, > Sunny Wang > > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Heyi Guo > Sent: Monday, February 26, 2018 4:30 PM > To: edk2-devel@lists.01.org > Cc: Ruiyu Ni ; Heyi Guo ; Eric Dong ; Star Zeng > Subject: [edk2] [PATCH 1/1] MdeModulePkg/UefiBootManagerLib: limit recursive call depth > > 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. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Heyi Guo > Cc: Star Zeng > Cc: Eric Dong > Cc: Ruiyu Ni > --- > MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h | 6 ++++++ > MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c | 17 ++++++++++++++++- > 2 files changed, 22 insertions(+), 1 deletion(-) > > diff --git a/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h b/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h > index 25a1d522fe84..9aa86b096525 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. > diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c b/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c > index ddcee8b0676f..30d70f32af84 100644 > --- a/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c > +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c > @@ -26,6 +26,12 @@ GLOBAL_REMOVE_IF_UNREFERENCED > L"Reboot Required" > }; > > +// > +// Counter of reconnect retry to repair controller; it is to limit the > +// number of recursive call of BmRepairAllControllers. > +// > +STATIC UINTN mReconnectRepairCount; > + > /** > Return the controller name. > > @@ -549,7 +555,16 @@ BmRepairAllControllers ( > > > if (ReconnectRequired) { > - BmRepairAllControllers (); > + if (mReconnectRepairCount < MAX_RECONNECT_REPAIR) { > + mReconnectRepairCount++; > + BmRepairAllControllers (); > + } else { > + DEBUG ((DEBUG_ERROR, "[%a:%d] Repair failed after %d retries.\n", > + __FUNCTION__, __LINE__, mReconnectRepairCount)); > + // Reset counter so that it will not affect calling > + // BmRepairAllControllers() somewhere else > + mReconnectRepairCount = 0; > + } > } > > DEBUG_CODE ( > -- > 2.7.4 > > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel