From de5ae37bb23a67f936a9671e8a78c40f06fd5392 Mon Sep 17 00:00:00 2001 From: Laszlo Ersek Date: Thu, 13 Mar 2014 17:34:55 +0000 Subject: [PATCH] OvmfPkg: BDS: remove historic (now defunct) boot mode hack When PI can distinguish the "full config" boot mode from "assume no changes", then the following BDS logic is correct: if BootMode == BOOT_WITH_FULL_CONFIGURATION: // // connect all devices // create & append each default boot option that's missing // BdsLibConnectAll BdsLibEnumerateAllBootOption else if BootMode == BOOT_ASSUMING_NO_CONFIGURATION_CHANGES: // // just stick with current BootOrder and the Boot#### variables // referenced by it // In theory, the first branch is intended to run infrequently, and the "assume no changes" branch should run most of the time. However, some platforms can't tell these two boot modes apart. The following substitute had been introduced: // // Technically, always assume "full config", but the BootMode HOB is // actually meaningless wrt. to "full config" or "assume no changes". // ASSERT (BootMode == BOOT_WITH_FULL_CONFIGURATION); // // Key off the existence of BootOrder. Try to prepare an in-memory list // of boot options, based on BootOrder and the referenced Boot#### // variables. // Status = BdsLibBuildOptionFromVar() // // If that succeeded, we'll treat it as "assume no changes". If it // failed (*only* if it failed), we'll build default boot options, // calling it "full config": // if EFI_ERROR(Status): BdsLibConnectAll() BdsLibEnumerateAllBootOption(BootOptionList) What we have now in OVMF is a mixture of the hack, and the behavior that's theoretically correct for "full config": - We assert "full config" -- this is OK. - We call "connect all" and "enumerate all" deliberately -- this is OK too. It matches "full config" which we assert. - However, we also have the hack in place, which had been meant as an alternative. In order to clean this up, we either need to restore the hack to its original form (ie. comment out the unconditional calls again), or we ought to remove the hack altogether. The unconditional "connect all" + "enumerate all" calls are the correct approach for OVMF, because we want, in fact, to start with "full config". The QEMU boot order specification and the set of emulated devices might change "out of band", which excludes "assume no changes". In other words, removing the hack corresponds to the "real production" case that the comment hints at. Because SetBootOrderFromQemu() may change the BootOrder NvVar, we must preserve the BdsLibBuildOptionFromVar() function call, in order to refresh the in-memory list with the new boot priorities. (The last step of BdsLibEnumerateAllBootOption() is such a call too.) Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Laszlo Ersek Reviewed-by: Jordan Justen git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@15326 6f19259b-4bc3-4df7-8a09-765794883524 --- OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c | 20 +++----------------- 1 file changed, 3 insertions(+), 17 deletions(-) diff --git a/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c b/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c index ab9c93eaf8..9f953acf79 100644 --- a/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c +++ b/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c @@ -1156,25 +1156,11 @@ Returns: BdsLibEnumerateAllBootOption (BootOptionList); SetBootOrderFromQemu (BootOptionList); - - // - // Please uncomment above ConnectAll and EnumerateAll code and remove following first boot - // checking code in real production tip. // - // In BOOT_WITH_FULL_CONFIGURATION boot mode, should always connect every device - // and do enumerate all the default boot options. But in development system board, the boot mode - // cannot be BOOT_ASSUMING_NO_CONFIGURATION_CHANGES because the machine box - // is always open. So the following code only do the ConnectAll and EnumerateAll at first boot. + // The BootOrder variable may have changed, reload the in-memory list with + // it. // - Status = BdsLibBuildOptionFromVar (BootOptionList, L"BootOrder"); - if (EFI_ERROR(Status)) { - // - // If cannot find "BootOrder" variable, it may be first boot. - // Try to connect all devices and enumerate all boot options here. - // - BdsLibConnectAll (); - BdsLibEnumerateAllBootOption (BootOptionList); - } + BdsLibBuildOptionFromVar (BootOptionList, L"BootOrder"); // // To give the User a chance to enter Setup here, if user set TimeOut is 0. -- 2.39.2