Ray,
On 10/13/15 04:34, Ni, Ruiyu wrote:
> Sunny,
> Yes it's acceptable. Currently the implementation of *FindLoadOption
> does an exactly match. If other consumers of this API need to just match
> partially, they need to provide their own version instead of using this
> exposed
> one.
>
> Laszlo,
> Sorry I missed your comments. I am thinking about whether exposing
> the *GetLoadOptionBuffer() breaks the orthogonality because we already
> have EfiBootManagerProcessLoadOption() API. The latter one not only
> loads the buffer but also treats the buffer as a PECOFF buffer to start it.
The functionality that OVMF depends on is to expand a relative ("short form")
device path to a full / absolute one. OVMF needs this in order to match and
filter the completed device path against various patterns it downloads and
translates from QEMU. It is very possible that after the expansion, the subject
boot option will ultimately *not* be loaded / launched.
> The *GetLoadOptionBuffer() seems to do part of the job.
And that is exactly the part that OVMF needs. :)
Let me quote the leading comments on the functions:
> /**
> Process (load and execute) the load option.
>
> @param LoadOption Pointer to the load option.
>
> @retval EFI_INVALID_PARAMETER The load option type is invalid,
> or the load option file path doesn't point
> to a valid file.
> @retval EFI_UNSUPPORTED The load option type is of
> LoadOptionTypeBoot.
> @retval EFI_SUCCESS The load option is inactive, or successfully
> loaded and executed.
> **/
> EFI_STATUS
> EFIAPI
> EfiBootManagerProcessLoadOption (
> IN EFI_BOOT_MANAGER_LOAD_OPTION *LoadOption
> );
This is too much for OVMF. We wouldn't like to load or execute the boot option.
> /**
> Get the load option by its device path.
>
> @param FilePath The device path pointing to a load option.
> It could be a short-form device path.
> @param FullPath Return the full device path of the load option after
> short-form device path expanding.
> Caller is responsible to free it.
> @param FileSize Return the load option size.
>
> @return The load option buffer. Caller is responsible to free the memory.
> **/
> VOID *
> BmGetLoadOptionBuffer (
> IN EFI_DEVICE_PATH_PROTOCOL *FilePath,
> OUT EFI_DEVICE_PATH_PROTOCOL **FullPath,
> OUT UINTN *FileSize
> );
And this is exactly what we'd need -- put in a short-form device path, and
retrieve the full (expanded) device path. Nothing more.
> Anyway I think your request also makes sense. Just want to know if
> there is other better idea to not break the orthogonality.
I agree that BmGetLoadOptionBuffer() does a *little* more than what OVMF needs:
it returns a buffer (and its size) that OVMF would ignore (i.e., just free).
If there was a public function that did nothing beyond device path expansion,
from short-form to full-form, then in OVMF we'd like to call that. Without such
a dedicated function however, BmGetLoadOptionBuffer() is the best match.
EfiBootManagerProcessLoadOption() definitely does too much; we have to filter
the full-form device paths (and possibly modify BootOrder) before actual boot
option processing (ie. loading and execution) can commence.
Thanks!
Laszlo
>
> Thanks,
> Ray
>
> -----Original Message-----
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Monday, October 12, 2015 8:11 PM
> To: Wang, Sunny (HPS SW) <sunnyw...@hpe.com>; Ni, Ruiyu <ruiyu...@intel.com>
> Cc: edk2-devel@lists.01.org <edk2-de...@ml01.01.org>
> Subject: Re: [edk2] [PATCH] MdeModulePkg: Make the BmFindLoadOption function
> public
>
> On 10/12/15 09:45, Wang, Sunny (HPS SW) wrote:
>> Hi Ruiyu,
>>
>> Thanks for the quick response.
>> It is because of that we need to do same thing as what
>> BmFindLoadOption function did in our PlatformBootManagerLib to
>> check/find specific boot option from the boot options array which is
>> got from EfiBootManagerGetLoadOptions function. I also think other
>> developers may have the same need. Therefore, it would be better to
>> make BmFindLoadOption public to reduce the maintenance effort caused
>> by duplicating the function into other BDS libraries like
>> PlatformBootManagerLib. Is it reasonable and acceptable? If so, I
>> will rename the BmFindLoadOption function to
>> EfiBootManagerFindLoadOption and resend the patch.
>
> On a similar note: in the message
>
> http://thread.gmane.org/gmane.comp.bios.edk2.devel/759/focus=1153
>
> point (10), I requested that the function BmGetLoadOptionBuffer() be
> made public.
>
> Looks like there are now at least two Bm*() functions that see outside
> demand! :)
>
> Thanks
> Laszlo
>
>>
>> Regards,
>> Sunny Wang
>>
>> -----Original Message-----
>> From: Ni, Ruiyu [mailto:ruiyu...@intel.com]
>> Sent: Monday, October 12, 2015 3:29 PM
>> To: Wang, Sunny (HPS SW); edk2-devel@lists.01.org
>> Subject: RE: [edk2] [PATCH] MdeModulePkg: Make the BmFindLoadOption function
>> public
>> Importance: High
>>
>> Sunny,
>> Why do you want to expose this function?
>> Bm* needs to change to EfiBootManager* if it's public.
>>
>> Thanks,
>> Ray
>>
>> -----Original Message-----
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Sunny
>> Wang
>> Sent: Monday, October 12, 2015 3:15 PM
>> To: edk2-devel@lists.01.org
>> Subject: [edk2] [PATCH] MdeModulePkg: Make the BmFindLoadOption function
>> public
>>
>> Make the BmFindLoadOption function public
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>>
>> Signed-off-by: Sunny Wang <sunnyw...@hpe.com>
>> ---
>> MdeModulePkg/Include/Library/UefiBootManagerLib.h | 21 +++++++++++++++++++++
>> 1 file changed, 21 insertions(+)
>>
>> diff --git a/MdeModulePkg/Include/Library/UefiBootManagerLib.h
>> b/MdeModulePkg/Include/Library/UefiBootManagerLib.h
>> index 5538d90..e86b589 100644
>> --- a/MdeModulePkg/Include/Library/UefiBootManagerLib.h
>> +++ b/MdeModulePkg/Include/Library/UefiBootManagerLib.h
>> @@ -2,6 +2,7 @@
>> Provide Boot Manager related library APIs.
>>
>> Copyright (c) 2011 - 2015, Intel Corporation. All rights reserved.<BR>
>> +(C) Copyright 2015 Hewlett Packard Enterprise Development LP<BR>
>> This program and the accompanying materials are licensed and made
>> available under the terms and conditions of the BSD License which
>> accompanies this distribution. The full text of the license may be found at
>> @@ -222,6 +223,26 @@ EfiBootManagerSortLoadOptionVariable (
>> IN SORT_COMPARE CompareFunction
>> );
>>
>> +/**
>> + Return the index of the load option in the load option array.
>> +
>> + The function consider two load options are equal when the
>> + OptionType, Attributes, Description, FilePath and OptionalData are equal.
>> +
>> + @param Key Pointer to the load option to be found.
>> + @param Array Pointer to the array of load options to be found.
>> + @param Count Number of entries in the Array.
>> +
>> + @retval -1 Key wasn't found in the Array.
>> + @retval 0 ~ Count-1 The index of the Key in the Array.
>> +**/
>> +INTN
>> +BmFindLoadOption (
>> + IN CONST EFI_BOOT_MANAGER_LOAD_OPTION *Key,
>> + IN CONST EFI_BOOT_MANAGER_LOAD_OPTION *Array,
>> + IN UINTN Count
>> + );
>> +
>> //
>> // Boot Manager hot key library functions.
>> //
>> --
>> 2.5.0.windows.1
>>
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
>>
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
