From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: None (no SPF record) identity=mailfrom; client-ip=15.241.140.73; helo=g4t3427.houston.hpe.com; envelope-from=sunnywang@hpe.com; receiver=edk2-devel@lists.01.org Received: from g4t3427.houston.hpe.com (g4t3427.houston.hpe.com [15.241.140.73]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id B809120954CCE for ; Mon, 26 Feb 2018 18:41:18 -0800 (PST) Received: from G1W8108.americas.hpqcorp.net (g1w8108.austin.hp.com [16.193.72.60]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by g4t3427.houston.hpe.com (Postfix) with ESMTPS id 1BE9A72; Tue, 27 Feb 2018 02:47:23 +0000 (UTC) Received: from G9W8456.americas.hpqcorp.net (2002:10d8:a15f::10d8:a15f) by G1W8108.americas.hpqcorp.net (2002:10c1:483c::10c1:483c) with Microsoft SMTP Server (TLS) id 15.0.1178.4; Tue, 27 Feb 2018 02:47:22 +0000 Received: from NAM03-DM3-obe.outbound.protection.outlook.com (15.241.52.12) by G9W8456.americas.hpqcorp.net (16.216.161.95) with Microsoft SMTP Server (TLS) id 15.0.1178.4 via Frontend Transport; Tue, 27 Feb 2018 02:47:22 +0000 Received: from DF4PR8401MB0650.NAMPRD84.PROD.OUTLOOK.COM (10.169.84.148) by DF4PR8401MB1275.NAMPRD84.PROD.OUTLOOK.COM (10.169.93.135) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id 15.20.527.15; Tue, 27 Feb 2018 02:47:21 +0000 Received: from DF4PR8401MB0650.NAMPRD84.PROD.OUTLOOK.COM ([fe80::1952:1670:248c:6c]) by DF4PR8401MB0650.NAMPRD84.PROD.OUTLOOK.COM ([fe80::1952:1670:248c:6c%17]) with mapi id 15.20.0527.021; Tue, 27 Feb 2018 02:47:19 +0000 From: "Wang, Sunny (HPS SW)" To: Guo Heyi CC: "edk2-devel@lists.01.org" , Ruiyu Ni , Eric Dong , Star Zeng , "Wang, Sunny (HPS SW)" Thread-Topic: [edk2] [PATCH 1/1] MdeModulePkg/UefiBootManagerLib: limit recursive call depth Thread-Index: AQHTrtwg4UZA42133U6oveLDHmPTPKO2XqbQgAAvJYCAAPdWoA== Date: Tue, 27 Feb 2018 02:47:19 +0000 Message-ID: References: <1519633779-130687-1-git-send-email-heyi.guo@linaro.org> <20180226113427.GA113703@SZX1000114654> In-Reply-To: <20180226113427.GA113703@SZX1000114654> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=sunnywang@hpe.com; x-originating-ip: [16.242.247.135] x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; DF4PR8401MB1275; 7:oSFsVlDVLPU37Xd7hLh1mVtwBxA3wHP/AtN6+9X4uHuWFW4L9KUMB7GOiQrO5DgtLVuwcfKVF3napN7phP1uznb+/TbvdXdJzrPvPNnzlvCeeJ5bVQDPHStHrob4ZQe/Rwd5Y3KhRRLod69WSeJTfuF5uImr21nv+vMvF+vT5STZXsC9ieZg450IMk5uSozsiWeyzOkR4NkvJvE6VrHVliUcxEoMzLzTfBlcCjyuDb+gKt4wbr1UX6WPS+hHdRxa x-ms-exchange-antispam-srfa-diagnostics: SSOS;SSOR; x-forefront-antispam-report: SFV:SKI; SCL:-1; SFV:NSPM; SFS:(10019020)(39860400002)(39380400002)(346002)(396003)(366004)(376002)(199004)(189003)(13464003)(26005)(478600001)(99286004)(5250100002)(186003)(106356001)(66066001)(3660700001)(76176011)(7696005)(229853002)(6306002)(55016002)(9686003)(54906003)(6436002)(53936002)(4326008)(3280700002)(102836004)(316002)(86362001)(2906002)(6506007)(53546011)(59450400001)(14454004)(33656002)(68736007)(7736002)(25786009)(305945005)(74316002)(2950100002)(105586002)(6116002)(6916009)(6246003)(5660300001)(2900100001)(97736004)(8936002)(966005)(81156014)(81166006)(8676002)(3846002); DIR:OUT; SFP:1102; SCL:1; SRVR:DF4PR8401MB1275; H:DF4PR8401MB0650.NAMPRD84.PROD.OUTLOOK.COM; FPR:; SPF:None; PTR:InfoNoRecords; A:1; MX:1; LANG:en; x-ms-office365-filtering-ht: Tenant x-ms-office365-filtering-correlation-id: 3fc3b3d8-1d8c-47e5-3ae8-08d57d8c6bb9 x-microsoft-antispam: UriScan:(222181515654134); BCL:0; PCL:0; RULEID:(7020095)(4652020)(8989060)(48565401081)(4534165)(4627221)(201703031133081)(201702281549075)(8990040)(5600026)(4604075)(3008032)(2017052603307)(7153060)(7193020); SRVR:DF4PR8401MB1275; x-ms-traffictypediagnostic: DF4PR8401MB1275: x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(227479698468861)(162533806227266)(222181515654134)(228905959029699); x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(8211001083)(6040501)(2401047)(8121501046)(5005006)(3002001)(3231220)(944501161)(52105095)(93006095)(93001095)(10201501046)(6055026)(6041288)(20161123558120)(20161123564045)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(20161123562045)(20161123560045)(6072148)(201708071742011); SRVR:DF4PR8401MB1275; BCL:0; PCL:0; RULEID:; SRVR:DF4PR8401MB1275; x-forefront-prvs: 05961EBAFC received-spf: None (protection.outlook.com: hpe.com does not designate permitted sender hosts) x-microsoft-antispam-message-info: K+DMVuvZvh1tAbJlLLXT75lx5INNV/qWAhN+AVmcPal/4+UgOteD2lHsCMI1quAti0ByZR2WlFjLZguH3LpdQMcLAdADNF578OALQr/pYGuAP3m+ygTT+gIj9MNN7VlIuC7G0rbUXLDRRju6j7S0LA+5HhqDy/rZLRAeKQSfaZE= spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM MIME-Version: 1.0 X-MS-Exchange-CrossTenant-Network-Message-Id: 3fc3b3d8-1d8c-47e5-3ae8-08d57d8c6bb9 X-MS-Exchange-CrossTenant-originalarrivaltime: 27 Feb 2018 02:47:19.8363 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 105b2061-b669-4b31-92ac-24d304d195dc X-MS-Exchange-Transport-CrossTenantHeadersStamped: DF4PR8401MB1275 X-OriginatorOrg: hpe.com 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: Tue, 27 Feb 2018 02:41:19 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Hi Heyi,=20 Thanks for looking into my suggestion. You already mentioned the value I th= ought. :) The value is to get better boot performance by setting the MAX_RE= CONNECT_REPAIR to a smaller number. 10 times reconnect may be suitable for = consumer products like laptop. However, it may be not suitable for server. = For example, user installs a problematic NIC 4-port card and its OPROM prod= uced 4 driver handles to manage 4 ports. For this case, 10 times reconnect = may take much longer time. =20 If you think it's still not worth adding a PCD for this or have other conce= rn about adding a PCD, I'm fine with dropping this suggestion. =20 Regards, Sunny Wang -----Original Message----- From: Guo Heyi [mailto:heyi.guo@linaro.org]=20 Sent: Monday, February 26, 2018 7:34 PM To: Wang, Sunny (HPS SW) Cc: Heyi Guo ; edk2-devel@lists.01.org; Ruiyu Ni ; Eric Dong ; Star Zeng Subject: Re: [edk2] [PATCH 1/1] MdeModulePkg/UefiBootManagerLib: limit recu= rsive call depth Importance: High Hi Sunny, I didn't consider it as a value necessary for platform override, for the re= try count should only have some impact on boot performance and it only happ= ens 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.=20 > Is it better to use a PCD instead of a define for MAX_RECONNECT_REPAIR? S= o that we can easily override it in our platform dsc file. >=20 > Regards, > Sunny Wang >=20 > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of=20 > Heyi Guo > Sent: Monday, February 26, 2018 4:30 PM > To: edk2-devel@lists.01.org > Cc: Ruiyu Ni ; Heyi Guo ;=20 > Eric Dong ; Star Zeng > Subject: [edk2] [PATCH 1/1] MdeModulePkg/UefiBootManagerLib: limit=20 > recursive call depth >=20 > Function BmRepairAllControllers may recursively call itself if some drive= r health protocol returns EfiDriverHealthStatusReconnectRequired. > However, driver health protocol of some buggy third party driver may alwa= ys return such status even after one and another reconnect. The endless ite= ration will cause stack overflow and then system exception, and it may be n= ot easy to find that the exception is actually caused by stack overflow. >=20 > So we limit the number of reconnect retry to 10 to improve code robustnes= s. >=20 > 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=20 > ++++++++++++++++- > 2 files changed, 22 insertions(+), 1 deletion(-) >=20 > diff --git a/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h=20 > 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 ("PlatformRec= overy####") > extern CHAR16 *mBmLoadOptionName[]; > =20 > +// > +// Maximum number of reconnect retry to repair controller; it is to=20 > +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=20 > 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" > }; > =20 > +// > +// Counter of reconnect retry to repair controller; it is to limit=20 > +the // number of recursive call of BmRepairAllControllers. > +// > +STATIC UINTN mReconnectRepairCount; > + > /** > Return the controller name. > =20 > @@ -549,7 +555,16 @@ BmRepairAllControllers ( > =20 > =20 > 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 =3D 0; > + } > } > =20 > DEBUG_CODE ( > -- > 2.7.4 >=20 > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel