On 09/05/13 12:18, Michael Chang wrote: > Hm..while I was testing another patch, occasionally I found that efi > boot entries were duplicating on every reboot. After several cycles I > have the following boot entries .. > > BootCurrent: 0005 > BootOrder: 0005 > Boot0000* EFI DVD/CDROM ACPI(a0341d0,0)PCI(1,1)ATAPI(1,0,0) > Boot0001* EFI Floppy ACPI(a0341d0,0)PCI(1,0)ACPI(60441d0,0) > Boot0002* EFI Floppy 1 ACPI(a0341d0,0)PCI(1,0)ACPI(60441d0,1) > Boot0003* EFI Network ACPI(a0341d0,0)PCI(3,0)MAC(525400c2a452,1) > Boot0004* EFI Internal Shell MM(b,3fa7e000,3ffbdfff) > Boot0005* opensuse > HD(1,800,4e000,1dc0cd72-7066-4f25-9d2c-1beb4da6dfcf)File(\EFI\opensuse\grubx64.efi) > Boot0006* EFI DVD/CDROM ACPI(a0341d0,0)PCI(1,1)ATAPI(1,0,0) > Boot0007* EFI Floppy ACPI(a0341d0,0)PCI(1,0)ACPI(60441d0,0) > Boot0008* EFI Floppy 1 ACPI(a0341d0,0)PCI(1,0)ACPI(60441d0,1) > Boot0009* EFI Network ACPI(a0341d0,0)PCI(3,0)MAC(525400c2a452,1) > Boot000A* EFI Internal Shell MM(b,3fa7e000,3ffbdfff) > Boot000B* EFI DVD/CDROM ACPI(a0341d0,0)PCI(1,1)ATAPI(1,0,0) > Boot000C* EFI Floppy ACPI(a0341d0,0)PCI(1,0)ACPI(60441d0,0) > Boot000D* EFI Floppy 1 ACPI(a0341d0,0)PCI(1,0)ACPI(60441d0,1) > Boot000E* EFI Network ACPI(a0341d0,0)PCI(3,0)MAC(525400c2a452,1) > Boot000F* EFI Internal Shell MM(b,3fa7e000,3ffbdfff) > Boot0010* EFI DVD/CDROM ACPI(a0341d0,0)PCI(1,1)ATAPI(1,0,0) > Boot0011* EFI Floppy ACPI(a0341d0,0)PCI(1,0)ACPI(60441d0,0) > Boot0012* EFI Floppy 1 ACPI(a0341d0,0)PCI(1,0)ACPI(60441d0,1) > Boot0013* EFI Network ACPI(a0341d0,0)PCI(3,0)MAC(525400c2a452,1) > Boot0014* EFI Internal Shell MM(b,3fa7e000,3ffbdfff) > Boot0015* EFI DVD/CDROM ACPI(a0341d0,0)PCI(1,1)ATAPI(1,0,0) > Boot0016* EFI Floppy ACPI(a0341d0,0)PCI(1,0)ACPI(60441d0,0) > Boot0017* EFI Floppy 1 ACPI(a0341d0,0)PCI(1,0)ACPI(60441d0,1) > Boot0018* EFI Network ACPI(a0341d0,0)PCI(3,0)MAC(525400c2a452,1) > Boot0019* EFI Internal Shell MM(b,3fa7e000,3ffbdfff) > Boot001A* EFI DVD/CDROM ACPI(a0341d0,0)PCI(1,1)ATAPI(1,0,0) > Boot001B* EFI Floppy ACPI(a0341d0,0)PCI(1,0)ACPI(60441d0,0) > Boot001C* EFI Floppy 1 ACPI(a0341d0,0)PCI(1,0)ACPI(60441d0,1) > Boot001D* EFI Network ACPI(a0341d0,0)PCI(3,0)MAC(525400c2a452,1) > Boot001E* EFI Internal Shell MM(b,3fa7e000,3ffbdfff)
This should not be a problem in practice I guess, since the BootOrder variable references only a few (the most recent ones) of these. > After reverting this patch (git commit 6bc7a08) the problem was gone. > Quite irony I was the one who acknowledged the test result. :/ Interesting -- I believe I had witnessed this way before Jordan's patch was committed. I don't think this is caused by Jordan's patch; it may only trigger / expose the problem for you. > Trying to go further and debugging it, a (clueless) patch attached > which's working for me to fix the problem. The puzzle work is > obviously too much for me. > > diff --git a/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c > b/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c > index ba6af2c..be1bb79 100644 > --- a/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c > +++ b/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c > @@ -1148,7 +1148,7 @@ Returns: > > DEBUG ((EFI_D_INFO, "BdsLibConnectAll\n")); > BdsLibConnectAll (); > - BdsLibEnumerateAllBootOption (BootOptionList); > + //BdsLibEnumerateAllBootOption (BootOptionList); > > SetBootOrderFromQemu (BootOptionList); > > My guess is BdsLibConnectAll () reads NvVars and then > BdsLibEnumerateAllBootOption (BootOptionList) appends even more, and > finally they were saved. This logics repeats every time and the Boot > varaibles grows .. I think we should understand what this comment means (reformatted here more narrowly): // // 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. // This seems to have been copied from "DuetPkg/Library/DuetBdsLib/BdsPlatform.c". In OvmfPkg, code preceding this comment, ie. the code subject to the uncommenting, has in fact been uncommented; in DuetPkg it is still commented out. Right now OvmfPkg only boots in BOOT_WITH_FULL_CONFIGURATION mode. (There's some related documentation in Vol1 of the PI spec, but I still don't understand the above comment.) I can imagine that BdsLibConnectAll() + BdsLibEnumerateAllBootOption() are a good choice in BOOT_WITH_FULL_CONFIGURATION mode. But, a shipped hw platform would likely almost never boot in that mode. If you look at "DuetPkg/Library/DuetBdsLib/BdsPlatform.c", that probably does the right thing for an emulated platform that always boots in BOOT_WITH_FULL_CONFIGURATION. Namely, the full enumeration is only done inside the "if" (dependent on the absence of the BootOrder variable). Your patch actually brings the code closer *back* to what is in DuetPkg. I think we need to understand why the unconditional enumeration was uncommented (enabled) in OvmfPkg. My current understanding is: - real hw platform: - BOOT_WITH_FULL_CONFIGURATION does full enumeration, unconditionally, - however BOOT_WITH_FULL_CONFIGURATION is almost never the case! - DuetPkg: - BOOT_WITH_FULL_CONFIGURATION is likely always the case, - hence make the full enumeration dependent on the BootOrder variable, - OvmfPkg: - BOOT_WITH_FULL_CONFIGURATION is *always* the case, definitely, - still allow full enumeration in that case -- why? Thanks Laszlo ------------------------------------------------------------------------------ Learn the latest--Visual Studio 2012, SharePoint 2013, SQL 2012, more! Discover the easy way to master current and previous Microsoft technologies and advance your career. Get an incredible 1,500+ hours of step-by-step tutorial videos with LearnDevNow. Subscribe today and save! http://pubads.g.doubleclick.net/gampad/clk?id=58041391&iu=/4140/ostg.clktrk _______________________________________________ edk2-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/edk2-devel
