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-boun...@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 <ruiyu...@intel.com>; Heyi Guo <heyi....@linaro.org>; Eric Dong > <eric.d...@intel.com>; Star Zeng <star.z...@intel.com> > 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 <heyi....@linaro.org> > Cc: Star Zeng <star.z...@intel.com> > Cc: Eric Dong <eric.d...@intel.com> > Cc: Ruiyu Ni <ruiyu...@intel.com> > --- > 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 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
