3 comments below. Regards, Ray
>-----Original Message----- >From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Daniil >Egranov >Sent: Wednesday, May 4, 2016 9:34 AM >To: edk2-devel@lists.01.org >Cc: Fan, Jeff <jeff....@intel.com> >Subject: [edk2] [PATCH v2 2/3] IntelFrameworkModulePkg/BdsDxe: Fix for the BDS >boot timeout > >The patch loads timeout value from the "Timeout" global variable and passes >it to PlatformBdsEnterFrontPage(), which handles delay and key input. >The PcdPlatformBootTimeOut is only used at the BDS entry point and updates >the "Timeout" value. This will allow the modification of the timeout value >through the BDS menu and overwrite it if PcdPlatformBootTimeOut has been >set. > >Contributed-under: TianoCore Contribution Agreement 1.0 >Signed-off-by: Daniil Egranov <daniil.egra...@arm.com> >--- > IntelFrameworkModulePkg/Universal/BdsDxe/BdsEntry.c | 18 +++++++++++++++--- > .../Universal/BdsDxe/BootMaint/BootMaint.c | 17 ++++++++++++++++- > .../Universal/BdsDxe/BootMaint/UpdatePage.c | 13 +++++++------ > 3 files changed, 38 insertions(+), 10 deletions(-) > >diff --git a/IntelFrameworkModulePkg/Universal/BdsDxe/BdsEntry.c >b/IntelFrameworkModulePkg/Universal/BdsDxe/BdsEntry.c >index ae7ad21..1d80fca 100644 >--- a/IntelFrameworkModulePkg/Universal/BdsDxe/BdsEntry.c >+++ b/IntelFrameworkModulePkg/Universal/BdsDxe/BdsEntry.c >@@ -123,6 +123,7 @@ BdsBootDeviceSelect ( > BOOLEAN BootNextExist; > LIST_ENTRY *LinkBootNext; > EFI_EVENT ConnectConInEvent; >+ UINTN Size; > > // > // Got the latest boot option >@@ -214,6 +215,18 @@ BdsBootDeviceSelect ( > if (Link == NULL) { > return ; > } >+ >+ //Read boot timeout variable >+ Status = gRT->GetVariable (L"Timeout", >+ &gEfiGlobalVariableGuid, >+ NULL, >+ &Size, >+ (VOID *) &Timeout); >+ >+ if (EFI_ERROR (Status)) { >+ Timeout = 0xFFFF; >+ } >+ > // > // Here we make the boot in a loop, every boot success will > // return to the front page >@@ -222,7 +235,7 @@ BdsBootDeviceSelect ( > // > // Check the boot option list first > // >- if (Link == &BootLists) { >+ if (Link == &BootLists || Timeout != 0xFFFF) { > // > // When LazyConIn enabled, signal connect ConIn event before enter UI > // >@@ -238,12 +251,11 @@ BdsBootDeviceSelect ( > // one is success to boot, then we back here to allow user > // add new active boot option > // >- Timeout = 0xffff; > PlatformBdsEnterFrontPage (Timeout, FALSE); >+ Timeout = 0xffff; 1. The old code is to enter front page unconditionally and immediately while you changed the behavior to wait for Timeout seconds before entering front page. Why? > InitializeListHead (&BootLists); > BdsLibBuildOptionFromVar (&BootLists, L"BootOrder"); > Link = BootLists.ForwardLink; >- continue; > } > // > // Get the boot option from the link list >diff --git a/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/BootMaint.c >b/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/BootMaint.c >index d4b4475..cc2d656 100644 >--- a/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/BootMaint.c >+++ b/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/BootMaint.c >@@ -111,6 +111,9 @@ InitializeBmmConfig ( > BM_MENU_ENTRY *NewMenuEntry; > BM_LOAD_CONTEXT *NewLoadContext; > UINT16 Index; >+ UINT16 BootTimeOut; >+ EFI_STATUS Status; >+ UINTN Size; > > ASSERT (CallbackData != NULL); > >@@ -128,7 +131,19 @@ InitializeBmmConfig ( > } > } > >- CallbackData->BmmFakeNvData.BootTimeOut = PcdGet16 (PcdPlatformBootTimeOut); >+ //Read boot timeout variable. If PcdPlatformBootTimeOut has been set, >+ //the Timeout variable will be initialized as part of the BDS startup >procedure >+ Status = gRT->GetVariable ( L"Timeout", >+ &gEfiGlobalVariableGuid, >+ NULL, >+ &Size, >+ (VOID *) &BootTimeOut); >+ >+ if (EFI_ERROR (Status)) { >+ BootTimeOut = 0; >+ } >+ >+ CallbackData->BmmFakeNvData.BootTimeOut = BootTimeOut; 2. With the platform DSC maps the PcdPlatformBootTimeout to L"Timeout" variable correctly, the above change is not necessary. right? > > // > // Initialize data which located in Boot Options Menu >diff --git a/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/UpdatePage.c >b/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/UpdatePage.c >index b13ed11..c872766 100644 >--- a/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/UpdatePage.c >+++ b/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/UpdatePage.c >@@ -722,18 +722,21 @@ UpdateTimeOutPage ( > IN BMM_CALLBACK_DATA *CallbackData > ) > { >- UINT16 BootTimeOut; > VOID *DefaultOpCodeHandle; > > CallbackData->BmmAskSaveOrNot = TRUE; > > UpdatePageStart (CallbackData); > >- BootTimeOut = PcdGet16 (PcdPlatformBootTimeOut); >- > DefaultOpCodeHandle = HiiAllocateOpCodeHandle (); > ASSERT (DefaultOpCodeHandle != NULL); >- HiiCreateDefaultOpCode (DefaultOpCodeHandle, >EFI_HII_DEFAULT_CLASS_STANDARD, EFI_IFR_TYPE_NUM_SIZE_16, >BootTimeOut); >+ >+ HiiCreateDefaultOpCode ( >+ DefaultOpCodeHandle, >+ EFI_HII_DEFAULT_CLASS_STANDARD, >+ EFI_IFR_TYPE_NUM_SIZE_16, >+ CallbackData->BmmFakeNvData.BootTimeOut >+ ); 3. I agree it's not necessary to re-get the timeout value from PCD again. So in summary I think only #3 is necessary. > > HiiCreateNumericOpCode ( > mStartOpCodeHandle, >@@ -752,8 +755,6 @@ UpdateTimeOutPage ( > > HiiFreeOpCodeHandle (DefaultOpCodeHandle); > >- //CallbackData->BmmFakeNvData.BootTimeOut = BootTimeOut; >- > UpdatePageEnd (CallbackData); > } > >-- >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
