Regards, Ray
>-----Original Message----- >From: Laszlo Ersek [mailto:ler...@redhat.com] >Sent: Tuesday, May 3, 2016 1:19 AM >To: Ni, Ruiyu <ruiyu...@intel.com>; edk2-de...@ml01.01.org >Subject: Re: [edk2] [Patch v4 00/23] Use MdeModulePkg/BDS in OVMF platform > >On 04/29/16 09:00, Ruiyu Ni wrote: >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? > I agree that expanding a network boot option device path is costly, most of the time. The opening target boot option behavior you noticed is necessary because when there are multiple results when expanding a short form device path, the logic chooses the result which contains the boot option. For example, for a USB short form device path, it could be expanded to two full device paths when there are two usb storage plugged in. BDS core can choose the 2nd USB storage when the first one doesn't contain BOOTX64.EFI. I agree your proposal to expose a API like *ExpandDevicePath() and the opening target boot option behavior could be performed only when there are multiple results when expanding a short form device path. But the behavior could still be performed sometimes. I like your second proposal to only call *GetLoadOptionBuffer() for short-form device paths. We do have ways to identify the short-form device paths. The logic is defined in UEFI Spec and implemented in *GetLoadOptionBuffer(). Do you agree that I duplicate the short-form device paths identification logic in QemuNewBootOrderLib? >Thanks! >Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
