On 08/17/22 17:11, Ard Biesheuvel wrote: > Add a new library that can be incorporated into any driver built from > source, and which permits loading of the driver to be inhibited based on > the value of a QEMU fw_cfg boolean variable. This will be used in a > subsequent patch to allow dispatch of the IPv6 and IPv6 network protocol
(1) Still a typo? Did you mean "IPv4 and IPv6"? > driver to be controlled from the QEMU command line. > > This approach is based on the notion that all UEFI and DXE drivers share > a single UefiDriverEntryPoint implementation, which we can easily swap > out at build time with one that will abort execution based on the value > of some QEMU fw_cfg variable. > > Signed-off-by: Ard Biesheuvel <a...@kernel.org> > --- > > OvmfPkg/Library/UefiDriverEntryPointFwCfgOverrideLib/UefiDriverEntryPointFwCfgOverrideLib.c > | 147 ++++++++++++++++++++ > > OvmfPkg/Library/UefiDriverEntryPointFwCfgOverrideLib/UefiDriverEntryPointFwCfgOverrideLib.inf > | 57 ++++++++ > OvmfPkg/OvmfPkg.dec > | 4 + > 3 files changed, 208 insertions(+) > > diff --git > a/OvmfPkg/Library/UefiDriverEntryPointFwCfgOverrideLib/UefiDriverEntryPointFwCfgOverrideLib.c > > b/OvmfPkg/Library/UefiDriverEntryPointFwCfgOverrideLib/UefiDriverEntryPointFwCfgOverrideLib.c > new file mode 100644 > index 000000000000..6eaf0cfd16ad > --- /dev/null > +++ > b/OvmfPkg/Library/UefiDriverEntryPointFwCfgOverrideLib/UefiDriverEntryPointFwCfgOverrideLib.c > @@ -0,0 +1,147 @@ > +/** @file > + Entry point to a EFI/DXE driver. This version is specific to QEMU, and ties > + dispatch of the driver in question on the value of a QEMU fw_cfg boolean > + variable which is referenced by name via a fixed pointer PCD. > + > +Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR> > +Copyright (c) 2022, Google LLC. All rights reserved.<BR> > +SPDX-License-Identifier: BSD-2-Clause-Patent > + > +**/ > + > +#include <Uefi.h> > + > +#include <Protocol/LoadedImage.h> > + > +#include <Library/BaseLib.h> > +#include <Library/DebugLib.h> > +#include <Library/QemuFwCfgSimpleParserLib.h> > +#include <Library/UefiBootServicesTableLib.h> > +#include <Library/UefiDriverEntryPoint.h> > + > +/** > + Unloads an image from memory. > + > + This function is a callback that a driver registers to do cleanup > + when the UnloadImage boot service function is called. > + > + @param ImageHandle The handle to the image to unload. > + > + @return Status returned by all unload(). > + > +**/ > +STATIC > +EFI_STATUS > +EFIAPI > +_DriverUnloadHandler ( > + EFI_HANDLE ImageHandle > + ) > +{ > + EFI_STATUS Status; > + > + // > + // If an UnloadImage() handler is specified, then call it > + // > + Status = ProcessModuleUnloadList (ImageHandle); > + > + // > + // If the driver specific unload handler does not return an error, then > call > + // all of the library destructors. If the unload handler returned an > error, > + // then the driver can not be unloaded, and the library destructors should > + // not be called > + // > + if (!EFI_ERROR (Status)) { > + ProcessLibraryDestructorList (ImageHandle, gST); > + } > + > + // > + // Return the status from the driver specific unload handler > + // > + return Status; > +} > + > +/** > + The entry point of PE/COFF Image for a DXE Driver, DXE Runtime Driver, or > + UEFI Driver. > + > + @param ImageHandle The image handle of the DXE Driver, DXE > + Runtime Driver, or UEFI Driver. > + @param SystemTable A pointer to the EFI System Table. > + > + @retval EFI_SUCCESS The DXE Driver, DXE Runtime Driver, or > + UEFI Driver exited normally. > + @retval EFI_INCOMPATIBLE_VERSION _gUefiDriverRevision is greater than > + SystemTable->Hdr.Revision. > + @retval Other Return value from > + ProcessModuleEntryPointList(). > + > +**/ > +EFI_STATUS > +EFIAPI > +_ModuleEntryPoint ( > + IN EFI_HANDLE ImageHandle, > + IN EFI_SYSTEM_TABLE *SystemTable > + ) > +{ > + EFI_STATUS Status; > + EFI_LOADED_IMAGE_PROTOCOL *LoadedImage; > + RETURN_STATUS RetStatus; > + BOOLEAN Enabled; > + > + if (_gUefiDriverRevision != 0) { > + // > + // Make sure that the EFI/UEFI spec revision of the platform is >= > EFI/UEFI > + // spec revision of the driver > + // > + if (SystemTable->Hdr.Revision < _gUefiDriverRevision) { > + return EFI_INCOMPATIBLE_VERSION; > + } > + } > + > + // > + // Call constructor for all libraries > + // > + ProcessLibraryConstructorList (ImageHandle, SystemTable); > + > + // > + // Install unload handler... > + // > + if (_gDriverUnloadImageCount != 0) { > + Status = gBS->HandleProtocol ( > + ImageHandle, > + &gEfiLoadedImageProtocolGuid, > + (VOID **)&LoadedImage > + ); > + ASSERT_EFI_ERROR (Status); > + LoadedImage->Unload = _DriverUnloadHandler; > + } > + > + RetStatus = QemuFwCfgParseBool ( > + FixedPcdGetPtr (PcdEntryPointOverrideFwCfgVarName), > + &Enabled); > + if (!RETURN_ERROR (RetStatus) && !Enabled) { > + // > + // The QEMU fw_cfg variable tells us not to load this image. So abort. > + // > + Status = EFI_ABORTED; > + } else { > + // > + // Call the driver entry point > + // > + Status = ProcessModuleEntryPointList (ImageHandle, SystemTable); > + } Right, I think this is the way to do it -- we've constructed all the lib instances, so now we can rely on PCDs and QemuFwCfg. I'm OK with this version, just one idea: in order to avoid the code duplication (and to benefit from future improvements to the original entry point lib in MdePkg), we could introduce a new hook API to the original -- forwarding the ImageHandle and SystemTable parameters to it --, provide a Null instance implementation for the API, for the general case, in MdePkg, and provide an fw_cfg-based implementation for OVMF / ArmVirt. I think it's worth a try; if Michael, Liming and Zhiguang don't like the bit of additional complexity in MdePkg, you can still merge this version. If you don't want to patch MdePkg though, I won't insist, of course. Reviewed-by: Laszlo Ersek <ler...@redhat.com> Laszlo > + > + // > + // If all of the drivers returned errors, or we if are aborting, then > invoke > + // all of the library destructors > + // > + if (EFI_ERROR (Status)) { > + ProcessLibraryDestructorList (ImageHandle, SystemTable); > + } > + > + // > + // Return the cumulative return status code from all of the driver entry > + // points > + // > + return Status; > +} > diff --git > a/OvmfPkg/Library/UefiDriverEntryPointFwCfgOverrideLib/UefiDriverEntryPointFwCfgOverrideLib.inf > > b/OvmfPkg/Library/UefiDriverEntryPointFwCfgOverrideLib/UefiDriverEntryPointFwCfgOverrideLib.inf > new file mode 100644 > index 000000000000..263e00ceef66 > --- /dev/null > +++ > b/OvmfPkg/Library/UefiDriverEntryPointFwCfgOverrideLib/UefiDriverEntryPointFwCfgOverrideLib.inf > @@ -0,0 +1,57 @@ > +## @file > +# Entry point to a EFI/DXE driver. This version is specific to QEMU, and > ties > +# dispatch of the driver in question on the value of a QEMU fw_cfg boolean > +# variable which is referenced by name via a fixed pointer PCD. > +# > +# Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.<BR> > +# Copyright (c) 2022, Google LLC. All rights reserved.<BR> > +# > +# SPDX-License-Identifier: BSD-2-Clause-Patent > +# > +# > +## > + > +[Defines] > + INF_VERSION = 1.29 > + BASE_NAME = UefiDriverEntryPointFwCfgOverrideLib > + FILE_GUID = 73349b79-f148-43b8-b24e-9098a6f3e1db > + MODULE_TYPE = UEFI_DRIVER > + VERSION_STRING = 1.0 > + LIBRARY_CLASS = UefiDriverEntryPoint|DXE_DRIVER > DXE_RUNTIME_DRIVER UEFI_DRIVER > + > +[Sources] > + UefiDriverEntryPointFwCfgOverrideLib.c > + > +[Packages] > + MdePkg/MdePkg.dec > + OvmfPkg/OvmfPkg.dec > + > +[LibraryClasses] > + BaseLib > + DebugLib > + QemuFwCfgSimpleParserLib > + UefiBootServicesTableLib > + > +[Protocols] > + gEfiLoadedImageProtocolGuid ## SOMETIMES_CONSUMES > + > +[FixedPcd] > + gUefiOvmfPkgTokenSpaceGuid.PcdEntryPointOverrideFwCfgVarName > + > +# > +# For UEFI drivers, these architectural protocols defined in PI 1.0 spec need > +# to be appended and merged to the final dependency section. > +# > +[Depex.common.UEFI_DRIVER] > + gEfiBdsArchProtocolGuid AND > + gEfiCpuArchProtocolGuid AND > + gEfiMetronomeArchProtocolGuid AND > + gEfiMonotonicCounterArchProtocolGuid AND > + gEfiRealTimeClockArchProtocolGuid AND > + gEfiResetArchProtocolGuid AND > + gEfiRuntimeArchProtocolGuid AND > + gEfiSecurityArchProtocolGuid AND > + gEfiTimerArchProtocolGuid AND > + gEfiVariableWriteArchProtocolGuid AND > + gEfiVariableArchProtocolGuid AND > + gEfiWatchdogTimerArchProtocolGuid > diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec > index 5af76a540529..9816aa41377d 100644 > --- a/OvmfPkg/OvmfPkg.dec > +++ b/OvmfPkg/OvmfPkg.dec > @@ -399,6 +399,10 @@ [PcdsFixedAtBuild] > ## The Tdx accept page size. 0x1000(4k),0x200000(2M) > gUefiOvmfPkgTokenSpaceGuid.PcdTdxAcceptPageSize|0x200000|UINT32|0x65 > > + ## The QEMU fw_cfg variable that UefiDriverEntryPointFwCfgOverrideLib will > + # check to decide whether to abort dispatch of the driver it is linked > into. > + gUefiOvmfPkgTokenSpaceGuid.PcdEntryPointOverrideFwCfgVarName|""|VOID*|0x68 > + > [PcdsDynamic, PcdsDynamicEx] > gUefiOvmfPkgTokenSpaceGuid.PcdEmuVariableEvent|0|UINT64|2 > gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashVariablesEnable|FALSE|BOOLEAN|0x10 > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#92541): https://edk2.groups.io/g/devel/message/92541 Mute This Topic: https://groups.io/mt/93083118/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-