On 13 July 2016 at 18:44, Laszlo Ersek <ler...@redhat.com> wrote:
> (This patch ports OvmfPkg commit 2eb358986052 to ArmVirtPkg. That
> functionality was not added to QemuBootOrderLib, because it was (and is)
> independent from QEMU and fw_cfg.)
>
> Remove any boot options that point to binaries built into the firmware and
> have become stale due to any of the following:
> - FvMain's base address or size changed (historical -- see commit
> e191a3114f4c),
> - FvMain's FvNameGuid changed,
> - the FILE_GUID of the pointed-to binary changed,
> - the referenced binary is no longer built into the firmware.
>
> For example, multiple such "EFI Internal Shell" boot options can coexist.
> They technically differ from each other, but may not describe any built-in
> shell binary exactly. Such options can accumulate in a varstore over time,
> and while they remain generally bootable (thanks to the efforts of
> BmGetFileBufferByFvFilePath()), they look bad.
>
> Filter out any stale options.
>
> Cc: Ard Biesheuvel <ard.biesheu...@linaro.org>
> Fixes: https://github.com/tianocore/edk2/issues/107
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek <ler...@redhat.com>
> ---
> ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf | 1 +
> ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c | 132
> ++++++++++++++++++++
> 2 files changed, 133 insertions(+)
>
> diff --git
> a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> b/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> index 9c95b69cc974..bec7fabb479c 100644
> --- a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> +++ b/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> @@ -78,6 +78,7 @@ [Guids]
>
> [Protocols]
> gEfiDevicePathProtocolGuid
> + gEfiFirmwareVolume2ProtocolGuid
> gEfiGraphicsOutputProtocolGuid
> gEfiLoadedImageProtocolGuid
> gEfiPciRootBridgeIoProtocolGuid
> diff --git a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c
> b/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c
> index eaafe7ff57ea..c11196a8a59c 100644
> --- a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c
> +++ b/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c
> @@ -22,6 +22,7 @@
> #include <Library/QemuBootOrderLib.h>
> #include <Library/UefiBootManagerLib.h>
> #include <Protocol/DevicePath.h>
> +#include <Protocol/FirmwareVolume2.h>
> #include <Protocol/GraphicsOutput.h>
> #include <Protocol/LoadedImage.h>
> #include <Protocol/PciIo.h>
> @@ -387,6 +388,136 @@ PlatformRegisterFvBootOption (
> }
>
>
> +/**
> + Remove all MemoryMapped(...)/FvFile(...) and Fv(...)/FvFile(...) boot
> options
> + whose device paths do not resolve exactly to an FvFile in the system.
> +
> + This removes any boot options that point to binaries built into the
> firmware
> + and have become stale due to any of the following:
> + - FvMain's base address or size changed (historical),
> + - FvMain's FvNameGuid changed,
> + - the FILE_GUID of the pointed-to binary changed,
> + - the referenced binary is no longer built into the firmware.
> +
> + EfiBootManagerFindLoadOption() used in PlatformRegisterFvBootOption() only
> + avoids exact duplicates.
> +**/
> +STATIC
> +VOID
> +RemoveStaleFvFileOptions (
> + VOID
> + )
> +{
> + EFI_BOOT_MANAGER_LOAD_OPTION *BootOptions;
> + UINTN BootOptionCount;
> + UINTN Index;
> +
> + BootOptions = EfiBootManagerGetLoadOptions (&BootOptionCount,
> + LoadOptionTypeBoot);
> +
> + for (Index = 0; Index < BootOptionCount; ++Index) {
> + EFI_DEVICE_PATH_PROTOCOL *Node1, *Node2, *SearchNode;
> + EFI_STATUS Status;
> + EFI_HANDLE FvHandle;
> +
> + //
> + // If the device path starts with neither MemoryMapped(...) nor Fv(...),
> + // then keep the boot option.
> + //
> + Node1 = BootOptions[Index].FilePath;
> + if (!(DevicePathType (Node1) == HARDWARE_DEVICE_PATH &&
> + DevicePathSubType (Node1) == HW_MEMMAP_DP) &&
> + !(DevicePathType (Node1) == MEDIA_DEVICE_PATH &&
> + DevicePathSubType (Node1) == MEDIA_PIWG_FW_VOL_DP)) {
> + continue;
> + }
> +
> + //
> + // If the second device path node is not FvFile(...), then keep the boot
> + // option.
> + //
> + Node2 = NextDevicePathNode (Node1);
> + if (DevicePathType (Node2) != MEDIA_DEVICE_PATH ||
> + DevicePathSubType (Node2) != MEDIA_PIWG_FW_FILE_DP) {
> + continue;
> + }
> +
> + //
> + // Locate the Firmware Volume2 protocol instance that is denoted by the
> + // boot option. If this lookup fails (i.e., the boot option references a
> + // firmware volume that doesn't exist), then we'll proceed to delete the
> + // boot option.
> + //
> + SearchNode = Node1;
> + Status = gBS->LocateDevicePath (&gEfiFirmwareVolume2ProtocolGuid,
> + &SearchNode, &FvHandle);
> +
> + if (!EFI_ERROR (Status)) {
> + //
> + // The firmware volume was found; now let's see if it contains the
> FvFile
> + // identified by GUID.
> + //
> + EFI_FIRMWARE_VOLUME2_PROTOCOL *FvProtocol;
> + MEDIA_FW_VOL_FILEPATH_DEVICE_PATH *FvFileNode;
> + UINTN BufferSize;
> + EFI_FV_FILETYPE FoundType;
> + EFI_FV_FILE_ATTRIBUTES FileAttributes;
> + UINT32 AuthenticationStatus;
> +
> + Status = gBS->HandleProtocol (FvHandle,
> &gEfiFirmwareVolume2ProtocolGuid,
> + (VOID **)&FvProtocol);
> + ASSERT_EFI_ERROR (Status);
> +
> + FvFileNode = (MEDIA_FW_VOL_FILEPATH_DEVICE_PATH *)Node2;
> + //
> + // Buffer==NULL means we request metadata only: BufferSize, FoundType,
> + // FileAttributes.
> + //
> + Status = FvProtocol->ReadFile (
> + FvProtocol,
> + &FvFileNode->FvFileName, // NameGuid
> + NULL, // Buffer
> + &BufferSize,
> + &FoundType,
> + &FileAttributes,
> + &AuthenticationStatus
> + );
> + if (!EFI_ERROR (Status)) {
> + //
> + // The FvFile was found. Keep the boot option.
> + //
> + continue;
> + }
> + }
> +
> + //
> + // Delete the boot option.
> + //
> + Status = EfiBootManagerDeleteLoadOptionVariable (
> + BootOptions[Index].OptionNumber, LoadOptionTypeBoot);
So I suppose deleting by index does not reshuffle the list?
> + DEBUG_CODE (
> + CHAR16 *DevicePathString;
> +
> + DevicePathString = ConvertDevicePathToText(BootOptions[Index].FilePath,
> + FALSE, FALSE);
> + DEBUG ((
> + EFI_ERROR (Status) ? EFI_D_WARN : EFI_D_VERBOSE,
> + "%a: removing stale Boot#%04x %s: %r\n",
> + __FUNCTION__,
> + (UINT32)BootOptions[Index].OptionNumber,
> + DevicePathString == NULL ? L"<unavailable>" : DevicePathString,
> + Status
> + ));
> + if (DevicePathString != NULL) {
> + FreePool (DevicePathString);
> + }
> + );
Indentation?
> + }
> +
> + EfiBootManagerFreeLoadOptions (BootOptions, BootOptionCount);
> +}
> +
> +
> STATIC
> VOID
> PlatformRegisterOptionsAndKeys (
> @@ -560,6 +691,7 @@ PlatformBootManagerAfterConsole (
> PcdGetPtr (PcdShellFile), L"EFI Internal Shell", LOAD_OPTION_ACTIVE
> );
>
> + RemoveStaleFvFileOptions ();
> SetBootOrderFromQemu ();
> }
>
> --
> 1.8.3.1
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
