On 05/05/16 10:08, Laszlo Ersek wrote: > On 05/05/16 07:08, Ni, Ruiyu wrote: >>> -----Original Message----- >>> From: Daniil Egranov [mailto:[email protected]] >>> Sent: Thursday, May 5, 2016 7:57 AM >>> To: Ni, Ruiyu <[email protected]>; [email protected] >>> Cc: Fan, Jeff <[email protected]> >>> Subject: Re: [edk2] [PATCH v2 2/3] IntelFrameworkModulePkg/BdsDxe: Fix for >>> the BDS boot timeout >>> >>> Hi Ray, >>> >>> Thank you for review and comments. Please see my answers below. >>> >>> Regards, >>> Daniil >>> >>> On 05/04/2016 12:04 AM, Ni, Ruiyu wrote: >>>> 3 comments below. >>>> >>>> Regards, >>>> Ray >>>> >>>>> -----Original Message----- >>>>> From: edk2-devel [mailto:[email protected]] On Behalf Of >>>>> Daniil Egranov >>>>> Sent: Wednesday, May 4, 2016 9:34 AM >>>>> To: [email protected] >>>>> Cc: Fan, Jeff <[email protected]> >>>>> 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 <[email protected]> >>>>> --- >>>>> 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? >>> The previous logic allows entry into the BDS boot menu in two cases: no >>> active boot options and all attempts to boot from the current boot >>> options fail. If there is a valid bootable device in the boot options, i >>> found no way to interrupt the boot process. In a lot of cases, the boot >>> options have to be changed before any boot attempts have been made. This >>> patch checks if boot timeout is set and uses PlatformBdsEnterFrontPage >>> to manage time and key inputs before using current boot options. >> >> You could refer to the OvmfPkg/Library/PlatformBdsLib. >> PlatformBdsEnterFrontPage() is called by PlatformBdsPolicyBehavior() >> with the timeout. > > Right, that's what I wanted to mention. It is not obvious (it was not > obvious to me either -- someone on this list advised me about the > correct solution), but the point is that platform BDS code has to call > PlatformBdsEnterFrontPage() explicitly, with the desired timeout.
Since Daniil is posting from an @arm.com email address, I'll mention ArmVirtPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.c as well. The last action of PlatformBdsPolicyBehavior() is to call PlatformBdsEnterFrontPage(). Thanks Laszlo _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
