Laszlo,
OVMF uses old style of FV device path node: MemoryMapped(...).
If you check Nt32Pkg/Nt32Pkg.fdf, there is FvNameGuid field under[FV] section.
-----
[FV.FvRecovery]
......
FvNameGuid = 6D99E806-3D38-42c2-A095-5F4300BFD7DC
----
If you add FvNameGuid field under [FV] section for OVMF, the firmware will
produce Fv(...) device path node for FV.
Which is also the recommended way to identify a FV because it won't cause
the FV boot option file path changing across boot.
You patch looks good but it assumes a FV boot option should always starts
with MemoryMapped(...) node, which is not always TRUE.
I recommend you change FDF to add FvNameGuid field to [FV] section
and then just use the following algorithm to remove stale options:
For each option:
Node = option.FilePath
Status = gBS->LocateDevicePath (&gEfiFirmwareVolume2ProtocolGuid,
&Node, &FvHandle);
If (EFI_ERROR (Status)) {
// Current one is not a FV boot option
Continue;
}
//... Use Fv->ReadFile as what you did in below patch to check.
With the above code, you don't need to check whether the first node is
a MemoryMapped(...) node and second node is a FvFile(...) node.
Thanks/Ray
> -----Original Message-----
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Tuesday, July 12, 2016 3:51 AM
> To: edk2-devel-01 <edk2-de...@ml01.01.org>
> Cc: Ard Biesheuvel <ard.biesheu...@linaro.org>; Gary Lin <g...@suse.com>;
> Justen, Jordan L <jordan.l.jus...@intel.com>; Ni, Ruiyu <ruiyu...@intel.com>
> Subject: [PATCH] OvmfPkg/PlatformBootManagerLib: remove stale FvFile
> boot options
>
> Changing the base address or the size of DXEFV, changing PcdShellFile, or
> omitting the UEFI Shell from the OVMF binary, all lead to "EFI Internal Shell"
> boot options that are technically different from each other and do 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.
>
> This functionality is not added to QemuBootOrderLib, because it is
> independent from QEMU and fw_cfg.
>
> Cc: Ard Biesheuvel <ard.biesheu...@linaro.org>
> Cc: Gary Lin <g...@suse.com>
> Cc: Jordan Justen <jordan.l.jus...@intel.com>
> Cc: Ruiyu Ni <ruiyu...@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek <ler...@redhat.com>
> ---
>
> Notes:
> Once this patch converges, I'll port it to ArmVirtPkg.
>
> OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf |
> 1 +
> OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c | 115
> ++++++++++++++++++++
> 2 files changed, 116 insertions(+)
>
> diff --git
> a/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> b/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> index ffa1288e4d1a..f3303b9f2cf3 100644
> ---
> a/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> +++
> b/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> @@ -72,6 +72,7 @@ [Protocols]
> gEfiS3SaveStateProtocolGuid # PROTOCOL
> SOMETIMES_CONSUMED
> gEfiDxeSmmReadyToLockProtocolGuid # PROTOCOL
> SOMETIMES_PRODUCED
> gEfiLoadedImageProtocolGuid # PROTOCOL
> SOMETIMES_PRODUCED
> + gEfiFirmwareVolume2ProtocolGuid # PROTOCOL
> SOMETIMES_CONSUMED
>
> [Guids]
> gEfiXenInfoGuid
> diff --git a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> index 912c5ed1ece4..727c8b4792c0 100644
> --- a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> +++ b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> @@ -15,6 +15,7 @@
> #include "BdsPlatform.h"
> #include <Guid/XenInfo.h>
> #include <Guid/RootBridgesConnectedEventGroup.h>
> +#include <Protocol/FirmwareVolume2.h>
>
>
> //
> @@ -149,6 +150,119 @@ PlatformRegisterFvBootOption (
> EfiBootManagerFreeLoadOptions (BootOptions, BootOptionCount); }
>
> +/**
> + Remove all MemoryMapped(...)/FvFile(...) boot options whose device
> +paths do
> + not resolve exactly to an FvFile in the system.
> +
> + This prevents the proliferation of UEFI Shell boot options if DXEFV's
> +base
> + address or size changes, or a different FILE_GUID binary is used as Shell.
> + EfiBootManagerFindLoadOption() used in PlatformRegisterFvBootOption()
> +only
> + avoids exact duplicates.
> +**/
> +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;
> +
> + Node1 = BootOptions[Index].FilePath;
> + if (DevicePathType (Node1) != HARDWARE_DEVICE_PATH ||
> + DevicePathSubType (Node1) != HW_MEMMAP_DP) {
> + continue;
> + }
> +
> + 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);
> + 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);
> + }
> + );
> + }
> +
> + EfiBootManagerFreeLoadOptions (BootOptions, BootOptionCount); }
> +
> VOID
> PlatformRegisterOptionsAndKeys (
> VOID
> @@ -1343,6 +1457,7 @@ Routine Description:
> 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
