On 04/29/16 09:00, Ruiyu Ni wrote:
> The patch serials creates a flag USE_OLD_BDS and by default the value
> of the flag is FALSE so that the new MdeModulePkg/BDS is used.
> User can define USE_OLD_BDS as TRUE to force to use IntelFrameworkModulePkg
> /BDS.
>
> The v3 adopts comments for v1 and v2 to split the big changes to
> small changes and also expose the EfiBootManagerGetLoadOptionBuffer().
>
> The v4 adopts comments for v3.
>
> Ruiyu Ni (23):
> MdeModulePkg/UefiBootManagerLib: Expose *GetLoadOptionBuffer() API
> OvmfPkg/PlatformPei: Add memory above 4GB as tested
> OvmfPkg: Duplicate QemuBootOrderLib to QemuNewBootOrderLib
> OvmfPkg/QemuNewBootOrderLib: Build with UefiBootManagerLib
> OvmfPkg: Duplicate PlatformBdsLib to PlatformBootManagerLib
> OvmfPkg/PlatformBootManagerLib: Follow PlatformBootManagerLib
> interfaces
> OvmfPkg/PlatformBootManagerLib: use
> EfiBootManagerUpdateConsoleVariable
> OvmfPkg/PlatformBootManagerLib: link to UefiBootManagerLib
> OvmfPkg/PlatformBootManagerLib: Use ConvertDevicePathToText()
> OvmfPkg/PlatformBootManagerLib: Init console vars in *BeforeConsole()
> OvmfPkg/PlatformBootManagerLib: Do not launch Boot Manager Menu
> OvmfPkg/PlatformBootManagerLib: Register boot options and hot keys
> OvmfPkg/PlatformBootManagerLib: Remove unused local functions.
> OvmfPkg/PlatformBootManagerLib: port PlatformBdsConnectSequence to
> UefiBootManagerLib
> OvmfPkg/PlatformBootManagerLib: Use
> EfiBootManagerRefreshAllBootOption()
> OvmfPkg/PlatformBootManagerLib: Remove PlatformBdsGetDriverOption()
> OvmfPkg/PlatformBootManagerLib: Use GetBootModeHob() in HobLib
> OvmfPkg/PlatformBootManagerLib: Remove unnecessary memory test
> OvmfPkg/PlatformBootManagerLib: Remove unused vars and func prototypes
> OvmfPkg/PlatformBootManagerLib: Add EnableQuietBoot & DisableQuietBoot
> OvmfPkg/PlatformBootManagerLib: Remove unused C structures definitions
> OvmfPkg: Use MdeModulePkg/BDS
> OvmfPkg/OvmfPkgIa32X64.dsc: Move PcdShellFile to
> [PcdsFixedAtBuild.X64]
>
> MdeModulePkg/Include/Library/UefiBootManagerLib.h | 23 +-
> MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 11 +-
> .../Library/UefiBootManagerLib/BmLoadOption.c | 2 +-
> .../Library/UefiBootManagerLib/InternalBm.h | 19 -
> .../Library/PlatformBootManagerLib/BdsPlatform.c | 1397
> ++++++++++++++++++++
> .../Library/PlatformBootManagerLib/BdsPlatform.h | 214 +++
> .../PlatformBootManagerLib.inf | 83 ++
> .../Library/PlatformBootManagerLib/PlatformData.c | 41 +
> .../QemuKernel.c | 0
> OvmfPkg/Library/PlatformBootManagerLib/QuietBoot.c | 671 ++++++++++
> .../ExtraRootBusMap.c | 0
> .../ExtraRootBusMap.h | 0
> .../QemuBootOrderLib.c | 139 +-
> .../QemuBootOrderLib.inf | 10 +-
> OvmfPkg/OvmfPkgIa32.dsc | 41 +-
> OvmfPkg/OvmfPkgIa32.fdf | 5 +
> OvmfPkg/OvmfPkgIa32X64.dsc | 43 +-
> OvmfPkg/OvmfPkgIa32X64.fdf | 5 +
> OvmfPkg/OvmfPkgX64.dsc | 41 +-
> OvmfPkg/OvmfPkgX64.fdf | 5 +
> OvmfPkg/PlatformPei/MemDetect.c | 4 +-
> OvmfPkg/PlatformPei/Platform.c | 29 -
> OvmfPkg/PlatformPei/Platform.h | 14 +-
> OvmfPkg/PlatformPei/Xen.c | 8 +-
> 24 files changed, 2653 insertions(+), 152 deletions(-)
> create mode 100644 OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> create mode 100644 OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.h
> create mode 100644
> OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> create mode 100644 OvmfPkg/Library/PlatformBootManagerLib/PlatformData.c
> copy OvmfPkg/Library/{PlatformBdsLib => PlatformBootManagerLib}/QemuKernel.c
> (100%)
> create mode 100644 OvmfPkg/Library/PlatformBootManagerLib/QuietBoot.c
> copy OvmfPkg/Library/{QemuBootOrderLib =>
> QemuNewBootOrderLib}/ExtraRootBusMap.c (100%)
> copy OvmfPkg/Library/{QemuBootOrderLib =>
> QemuNewBootOrderLib}/ExtraRootBusMap.h (100%)
> copy OvmfPkg/Library/{QemuBootOrderLib =>
> QemuNewBootOrderLib}/QemuBootOrderLib.c (91%)
> copy OvmfPkg/Library/{QemuBootOrderLib =>
> QemuNewBootOrderLib}/QemuBootOrderLib.inf (83%)
>
I tried to do some light testing with this v4 series.
It seems that the -D USE_OLD_BDS build compiles and works as expected,
so that's great.
However, I think I found a problem with
EfiBootManagerGetLoadOptionBuffer() -- i.e., the UefiBootManagerLib
function that is exported in patch 1.
The problem is that the function seems to actually open the file to
which the short-form device path points, in the course of the device
path expansion. This is not too bad as long as the target file resides
on a local FAT file system (the cost is some additional CPU usage).
However, when the boot option is a network device path (PXE), this
becomes extremely costly -- when OVMF completes the device path, it
results in DHCP and PXE traffic, which can take very long to time out,
even if OVMF ends up booting a different boot option.
Now, I can see two possible causes for this:
* The first possibility is that EfiBootManagerGetLoadOptionBuffer()
works differently from BdsExpandPartitionPartialDevicePathToFull() *in
general*, not just in case of network boot options.
Namely, when the devpath being expanded points to a FAT-resident file, I
can see "FSOpen" messages in the log from the FAT driver too. And this
only happens when calling EfiBootManagerGetLoadOptionBuffer(). The old
function (BdsExpandPartitionPartialDevicePathToFull()) does not do this.
Ray, can you please explain why this is a good thing? Can we turn off
the "open the target" logic in EfiBootManagerGetLoadOptionBuffer()? What
OVMF needs from this function is only the expanded device path; the
contents of the pointed-to file is irrelevant.
Can we perhaps factor out a public function from
EfiBootManagerGetLoadOptionBuffer() that does only the device path
expansion?
* The other cause of this problem could be that patch v4 04/23:
OvmfPkg/QemuNewBootOrderLib: Build with UefiBootManagerLib
removes the MEDIA_DEVICE_PATH/MEDIA_HARDDRIVE_DP condition around the
devpath expansion -- at my request even, as far as I remember. (Of
course, when I requested that EfiBootManagerGetLoadOptionBuffer() be
called unconditionally, I was not aware that it would try to open the
target file!) In other words, we now try to expand every boot option,
even if the device path is not short-form.
In the worst case, we should just restore the
MEDIA_DEVICE_PATH/MEDIA_HARDDRIVE_DP condition around
EfiBootManagerGetLoadOptionBuffer(). I was hoping we could generalize
the devpath expansion to USB disks and so on. (Not network though...)
Ray, do you have an idea for identifying short-form device paths, so we
can restrict Match() to call EfiBootManagerGetLoadOptionBuffer() only
for short-form devpaths?
Thanks!
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
