On 05/04/16 07:58, Fu Siyuan wrote:
> V2:
> Remove unnecessary ZeroMem and free load option.
>
> This patch updates the HTTP boot driver to use the API in UefiBootManagerLib
> to
> create new load option, to avoid duplicate code.
>
> Cc: Ye Ting <[email protected]>
> Cc: Wu Jiaxin <[email protected]>
> Cc: Ni Ruiyu <[email protected]>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Fu Siyuan <[email protected]>
> ---
> NetworkPkg/HttpBootDxe/HttpBootConfig.c | 136
> ++++++--------------------------
> NetworkPkg/HttpBootDxe/HttpBootDxe.inf | 1 +
> NetworkPkg/NetworkPkg.dsc | 8 ++
> 3 files changed, 33 insertions(+), 112 deletions(-)
This patch breaks the OvmfPkg build, with the following options:
-D USE_OLD_BDS -D HTTP_ENABLE
The reason is that the new UefiBootManagerLib dependency is not resolved
in the OvmfPkg DSC files with USE_OLD_BDS.
... Hm, actually, I think this build breakage is indicative of a deeper
change. Namely, requiring UefiBootManagerLib in HttpBootDxe means that
HttpBootDxe is no longer compatible with the IntelFrameworkModulePkg BDS.
Is that intentional?
If so, I'm not opposing it, of course, but it means that the usefulness
of USE_OLD_BDS in OvmfPkg is decreasing rapidly -- much faster than I
expected. We should probably remove USE_OLD_BDS from OvmfPkg in a week
or two.
Ultimately, I think it's not necessary to fix up this patch.
Thanks
Laszlo
> diff --git a/NetworkPkg/HttpBootDxe/HttpBootConfig.c
> b/NetworkPkg/HttpBootDxe/HttpBootConfig.c
> index 00e4782..2ca38b5 100644
> --- a/NetworkPkg/HttpBootDxe/HttpBootConfig.c
> +++ b/NetworkPkg/HttpBootDxe/HttpBootConfig.c
> @@ -13,6 +13,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR IMPLIED.
> **/
>
> #include "HttpBootDxe.h"
> +#include <Library/UefiBootManagerLib.h>
>
> CHAR16 mHttpBootConfigStorageName[] = L"HTTP_BOOT_CONFIG_IFR_NVDATA";
>
> @@ -36,31 +37,18 @@ HttpBootAddBootOption (
> IN CHAR16 *Uri
> )
> {
> - EFI_DEV_PATH *Node;
> - EFI_DEVICE_PATH_PROTOCOL *TmpDevicePath;
> - EFI_DEVICE_PATH_PROTOCOL *NewDevicePath;
> - UINTN Length;
> - CHAR8 AsciiUri[URI_STR_MAX_SIZE];
> - CHAR16 *CurrentOrder;
> - EFI_STATUS Status;
> - UINTN OrderCount;
> - UINTN TargetLocation;
> - BOOLEAN Found;
> - UINT8 *TempByteBuffer;
> - UINT8 *TempByteStart;
> - UINTN DescSize;
> - UINTN FilePathSize;
> - CHAR16 OptionStr[10];
> - UINT16 *NewOrder;
> - UINTN Index;
> -
> - NewOrder = NULL;
> - TempByteStart = NULL;
> + EFI_DEV_PATH *Node;
> + EFI_DEVICE_PATH_PROTOCOL *TmpDevicePath;
> + EFI_DEVICE_PATH_PROTOCOL *NewDevicePath;
> + UINTN Length;
> + CHAR8 AsciiUri[URI_STR_MAX_SIZE];
> + EFI_STATUS Status;
> + UINTN Index;
> + EFI_BOOT_MANAGER_LOAD_OPTION NewOption;
> +
> NewDevicePath = NULL;
> - NewOrder = NULL;
> Node = NULL;
> TmpDevicePath = NULL;
> - CurrentOrder = NULL;
>
> if (StrLen (Description) == 0) {
> return EFI_INVALID_PARAMETER;
> @@ -137,105 +125,29 @@ HttpBootAddBootOption (
> }
>
> //
> - // Get current "BootOrder" variable and find a free target.
> - //
> - Length = 0;
> - Status = GetVariable2 (
> - L"BootOrder",
> - &gEfiGlobalVariableGuid,
> - (VOID **)&CurrentOrder,
> - &Length
> - );
> - if (EFI_ERROR (Status) && Status != EFI_NOT_FOUND) {
> - goto ON_EXIT;
> - }
> - OrderCount = Length / sizeof (UINT16);
> - Found = FALSE;
> - for (TargetLocation=0; TargetLocation < 0xFFFF; TargetLocation++) {
> - Found = TRUE;
> - for (Index = 0; Index < OrderCount; Index++) {
> - if (CurrentOrder[Index] == TargetLocation) {
> - Found = FALSE;
> - break;
> - }
> - }
> - if (Found) {
> - break;
> - }
> - }
> -
> - if (TargetLocation == 0xFFFF) {
> - DEBUG ((EFI_D_ERROR, "Could not find unused target index.\n"));
> - Status = EFI_OUT_OF_RESOURCES;
> - goto ON_EXIT;
> - } else {
> - DEBUG ((EFI_D_INFO, "TargetIndex = %04x.\n", TargetLocation));
> - }
> -
> - //
> - // Construct and set the "Boot####" variable
> + // Add a new load option.
> //
> - DescSize = StrSize(Description);
> - FilePathSize = GetDevicePathSize (NewDevicePath);
> - TempByteBuffer = AllocateZeroPool(sizeof(EFI_LOAD_OPTION) + DescSize +
> FilePathSize);
> - if (TempByteBuffer == NULL) {
> - Status = EFI_OUT_OF_RESOURCES;
> - goto ON_EXIT;
> - }
> -
> - TempByteStart = TempByteBuffer;
> - *((UINT32 *) TempByteBuffer) = LOAD_OPTION_ACTIVE; // Attributes
> - TempByteBuffer += sizeof (UINT32);
> -
> - *((UINT16 *) TempByteBuffer) = (UINT16)FilePathSize; //
> FilePathListLength
> - TempByteBuffer += sizeof (UINT16);
> -
> - CopyMem (TempByteBuffer, Description, DescSize);
> - TempByteBuffer += DescSize;
> - CopyMem (TempByteBuffer, NewDevicePath, FilePathSize);
> -
> - UnicodeSPrint (OptionStr, sizeof(OptionStr), L"%s%04x", L"Boot",
> TargetLocation);
> - Status = gRT->SetVariable (
> - OptionStr,
> - &gEfiGlobalVariableGuid,
> - EFI_VARIABLE_NON_VOLATILE |
> EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS,
> - sizeof(UINT32) + sizeof(UINT16) + DescSize + FilePathSize,
> - TempByteStart
> - );
> + Status = EfiBootManagerInitializeLoadOption (
> + &NewOption,
> + LoadOptionNumberUnassigned,
> + LoadOptionTypeBoot,
> + LOAD_OPTION_ACTIVE,
> + Description,
> + NewDevicePath,
> + NULL,
> + 0
> + );
> if (EFI_ERROR (Status)) {
> goto ON_EXIT;
> }
>
> - //
> - // Insert into the order list and set "BootOrder" variable
> - //
> - NewOrder = AllocateZeroPool ((OrderCount + 1) * sizeof (UINT16));
> - if (NewOrder == NULL) {
> - Status = EFI_OUT_OF_RESOURCES;
> - goto ON_EXIT;
> + Status = EfiBootManagerAddLoadOptionVariable (&NewOption, (UINTN) -1);
> + if (EFI_ERROR (Status)) {
> + EfiBootManagerFreeLoadOption (&NewOption);
> }
> - CopyMem(NewOrder, CurrentOrder, OrderCount * sizeof(UINT16));
> - NewOrder[OrderCount] = (UINT16) TargetLocation;
> - Status = gRT->SetVariable (
> - L"BootOrder",
> - &gEfiGlobalVariableGuid,
> - EFI_VARIABLE_NON_VOLATILE |
> EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS,
> - ((OrderCount + 1) * sizeof (UINT16)),
> - NewOrder
> - );
> -
>
> ON_EXIT:
>
> - if (CurrentOrder != NULL) {
> - FreePool (CurrentOrder);
> - }
> - if (NewOrder != NULL) {
> - FreePool (NewOrder);
> - }
> - if (TempByteStart != NULL) {
> - FreePool (TempByteStart);
> - }
> if (NewDevicePath != NULL) {
> FreePool (NewDevicePath);
> }
> diff --git a/NetworkPkg/HttpBootDxe/HttpBootDxe.inf
> b/NetworkPkg/HttpBootDxe/HttpBootDxe.inf
> index d3df5f7..e6ce864 100644
> --- a/NetworkPkg/HttpBootDxe/HttpBootDxe.inf
> +++ b/NetworkPkg/HttpBootDxe/HttpBootDxe.inf
> @@ -61,6 +61,7 @@
> HiiLib
> PrintLib
> UefiHiiServicesLib
> + UefiBootManagerLib
>
> [Protocols]
> ## TO_START
> diff --git a/NetworkPkg/NetworkPkg.dsc b/NetworkPkg/NetworkPkg.dsc
> index 0695dc1..3c809d5 100644
> --- a/NetworkPkg/NetworkPkg.dsc
> +++ b/NetworkPkg/NetworkPkg.dsc
> @@ -39,6 +39,12 @@
> UefiLib|MdePkg/Library/UefiLib/UefiLib.inf
>
> UefiRuntimeServicesTableLib|MdePkg/Library/UefiRuntimeServicesTableLib/UefiRuntimeServicesTableLib.inf
>
> UefiHiiServicesLib|MdeModulePkg/Library/UefiHiiServicesLib/UefiHiiServicesLib.inf
> +
> UefiBootManagerLib|MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
> +
> TimerLib|MdePkg/Library/BaseTimerLibNullTemplate/BaseTimerLibNullTemplate.inf
> +
> PerformanceLib|MdePkg/Library/BasePerformanceLibNull/BasePerformanceLibNull.inf
> +
> PeCoffGetEntryPointLib|MdePkg/Library/BasePeCoffGetEntryPointLib/BasePeCoffGetEntryPointLib.inf
> + DxeServicesLib|MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
> +
> DxeServicesTableLib|MdePkg/Library/DxeServicesTableLib/DxeServicesTableLib.inf
>
> DpcLib|MdeModulePkg/Library/DxeDpcLib/DxeDpcLib.inf
> NetLib|MdeModulePkg/Library/DxeNetLib/DxeNetLib.inf
> @@ -54,6 +60,8 @@
> SortLib|MdeModulePkg/Library/UefiSortLib/UefiSortLib.inf
>
> [LibraryClasses.common.UEFI_DRIVER]
> + HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
> +
> ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf
> DebugLib|MdePkg/Library/UefiDebugLibConOut/UefiDebugLibConOut.inf
>
> [LibraryClasses.common.UEFI_APPLICATION]
>
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel
