[edk2-devel] [PATCH 1/2] OvmfPkg: Introduce NULL class library to inhibit driver load
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
driver to be controlled from the QEMU command line.
Signed-off-by: Ard Biesheuvel
---
OvmfPkg/Library/DriverLoadInhibitorLib/DriverLoadInhibitorLib.c | 30
OvmfPkg/Library/DriverLoadInhibitorLib/DriverLoadInhibitorLib.inf | 28
++
OvmfPkg/OvmfPkg.dec | 4 +++
3 files changed, 62 insertions(+)
diff --git a/OvmfPkg/Library/DriverLoadInhibitorLib/DriverLoadInhibitorLib.c
b/OvmfPkg/Library/DriverLoadInhibitorLib/DriverLoadInhibitorLib.c
new file mode 100644
index ..dc8544bc38be
--- /dev/null
+++ b/OvmfPkg/Library/DriverLoadInhibitorLib/DriverLoadInhibitorLib.c
@@ -0,0 +1,30 @@
+// @file
+// Copyright (c) 2022, Google LLC. All rights reserved.
+// SPDX-License-Identifier: BSD-2-Clause-Patent
+//
+
+#include
+
+#include
+#include
+
+STATIC CHAR16 mExitData[] = L"Driver dispatch inhibited by QEMU fw_cfg
variable.";
+
+EFI_STATUS
+EFIAPI
+DriverLoadInhibitorLibConstructor (
+ IN EFI_HANDLEHandle,
+ IN EFI_SYSTEM_TABLE *SystemTable
+ )
+{
+ RETURN_STATUS Status;
+ BOOLEAN Enabled;
+
+ Status = QemuFwCfgParseBool (FixedPcdGetPtr (PcdDriverInhibitorFwCfgVarName),
+ &Enabled);
+ if (!RETURN_ERROR (Status) && !Enabled) {
+return gBS->Exit (Handle, EFI_REQUEST_UNLOAD_IMAGE, sizeof mExitData,
+ mExitData);
+ }
+ return EFI_SUCCESS;
+}
diff --git a/OvmfPkg/Library/DriverLoadInhibitorLib/DriverLoadInhibitorLib.inf
b/OvmfPkg/Library/DriverLoadInhibitorLib/DriverLoadInhibitorLib.inf
new file mode 100644
index ..ed521d12d335
--- /dev/null
+++ b/OvmfPkg/Library/DriverLoadInhibitorLib/DriverLoadInhibitorLib.inf
@@ -0,0 +1,28 @@
+## @file
+# Copyright (c) 2022, Google LLC. All rights reserved.
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+ INF_VERSION= 1.29
+ BASE_NAME = DriverLoadInhibitorLib
+ FILE_GUID = af4c2c0b-f7ed-4d61-ad97-5953982c3531
+ MODULE_TYPE= DXE_DRIVER
+ VERSION_STRING = 1.0
+ LIBRARY_CLASS = NULL
+ CONSTRUCTOR= DriverLoadInhibitorLibConstructor
+
+[Sources]
+ DriverLoadInhibitorLib.c
+
+[LibraryClasses]
+ QemuFwCfgSimpleParserLib
+ UefiBootServicesTableLib
+
+[Packages]
+ MdePkg/MdePkg.dec
+ OvmfPkg/OvmfPkg.dec
+
+[FixedPcd]
+ gUefiOvmfPkgTokenSpaceGuid.PcdDriverInhibitorFwCfgVarName
diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
index 5af76a540529..e9a22cab088c 100644
--- a/OvmfPkg/OvmfPkg.dec
+++ b/OvmfPkg/OvmfPkg.dec
@@ -399,6 +399,10 @@ [PcdsFixedAtBuild]
## The Tdx accept page size. 0x1000(4k),0x20(2M)
gUefiOvmfPkgTokenSpaceGuid.PcdTdxAcceptPageSize|0x20|UINT32|0x65
+ ## The QEMU fw_cfg variable that DriverLoadInhibitorLib will check to
+ # decide whether to abort dispatch of the driver it is linked into.
+ gUefiOvmfPkgTokenSpaceGuid.PcdDriverInhibitorFwCfgVarName|""|VOID*|0x68
+
[PcdsDynamic, PcdsDynamicEx]
gUefiOvmfPkgTokenSpaceGuid.PcdEmuVariableEvent|0|UINT64|2
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashVariablesEnable|FALSE|BOOLEAN|0x10
--
2.35.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#92421): https://edk2.groups.io/g/devel/message/92421
Mute This Topic: https://groups.io/mt/93032846/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 1/2] OvmfPkg: Introduce NULL class library to inhibit driver load
On 08/15/22 11:40, 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) typo? (should be "IPv4 and IPv6" I think)
> driver to be controlled from the QEMU command line.
>
> Signed-off-by: Ard Biesheuvel
> ---
> OvmfPkg/Library/DriverLoadInhibitorLib/DriverLoadInhibitorLib.c | 30
>
> OvmfPkg/Library/DriverLoadInhibitorLib/DriverLoadInhibitorLib.inf | 28
> ++
> OvmfPkg/OvmfPkg.dec | 4 +++
> 3 files changed, 62 insertions(+)
>
> diff --git a/OvmfPkg/Library/DriverLoadInhibitorLib/DriverLoadInhibitorLib.c
> b/OvmfPkg/Library/DriverLoadInhibitorLib/DriverLoadInhibitorLib.c
> new file mode 100644
> index ..dc8544bc38be
> --- /dev/null
> +++ b/OvmfPkg/Library/DriverLoadInhibitorLib/DriverLoadInhibitorLib.c
> @@ -0,0 +1,30 @@
> +// @file
> +// Copyright (c) 2022, Google LLC. All rights reserved.
> +// SPDX-License-Identifier: BSD-2-Clause-Patent
> +//
> +
> +#include
> +
> +#include
> +#include
> +
> +STATIC CHAR16 mExitData[] = L"Driver dispatch inhibited by QEMU fw_cfg
> variable.";
> +
> +EFI_STATUS
> +EFIAPI
> +DriverLoadInhibitorLibConstructor (
> + IN EFI_HANDLEHandle,
> + IN EFI_SYSTEM_TABLE *SystemTable
> + )
> +{
> + RETURN_STATUS Status;
> + BOOLEAN Enabled;
> +
> + Status = QemuFwCfgParseBool (FixedPcdGetPtr
> (PcdDriverInhibitorFwCfgVarName),
> + &Enabled);
> + if (!RETURN_ERROR (Status) && !Enabled) {
> +return gBS->Exit (Handle, EFI_REQUEST_UNLOAD_IMAGE, sizeof mExitData,
> + mExitData);
(2) Per UEFI spec, ExitData should be allocated with
gBS->AllocatePool().
(3) EFI_REQUEST_UNLOAD_IMAGE is from the PI spec; while not wrong, I
think it's strange to use here. EFI_ABORTED or something similar from
the UEFI spec would be a better fit IMO.
(4) And then, the big problem:
I agree that returning an error from the constructor would not be
beneficial, as it would cause an assertion to fail in the
ProcessLibraryConstructorList() function, in the generated "AutoGen.c"
file.
However, calling gBS->Exit() from a constructor seems unsafe to me, with
regard to library destructors.
Now, in the current case (considering patch#2), this unsafety is not
visible. That's because:
(quoting ProcessLibraryConstructorList() and
ProcessLibraryDestructorList() from
"Build/OvmfX64/NOOPT_GCC5/X64/NetworkPkg/Ip4Dxe/Ip4Dxe/DEBUG/AutoGen.c",
from an earlier build on my machine anyway):
> VOID
> EFIAPI
> ProcessLibraryConstructorList (
> IN EFI_HANDLEImageHandle,
> IN EFI_SYSTEM_TABLE *SystemTable
> )
> {
> EFI_STATUS Status;
>
> Status = PlatformDebugLibIoPortConstructor ();
> ASSERT_RETURN_ERROR (Status);
>
> Status = UefiBootServicesTableLibConstructor (ImageHandle, SystemTable);
> ASSERT_EFI_ERROR (Status);
>
> Status = DevicePathLibConstructor (ImageHandle, SystemTable);
> ASSERT_EFI_ERROR (Status);
>
> Status = UefiRuntimeServicesTableLibConstructor (ImageHandle, SystemTable);
> ASSERT_EFI_ERROR (Status);
>
> Status = UefiLibConstructor (ImageHandle, SystemTable);
> ASSERT_EFI_ERROR (Status);
>
> Status = UefiHiiServicesLibConstructor (ImageHandle, SystemTable);
> ASSERT_EFI_ERROR (Status);
>
> Status = DpcLibConstructor (ImageHandle, SystemTable);
> ASSERT_EFI_ERROR (Status);
>
> }
>
>
>
> VOID
> EFIAPI
> ProcessLibraryDestructorList (
> IN EFI_HANDLEImageHandle,
> IN EFI_SYSTEM_TABLE *SystemTable
> )
> {
>
> }
there is no library destruction to speak of -- none of the used library
instances have resources they need to release at destruction time.
However, the general case looks problematic. The new library constructor
call would be inserted *somewhere* in ProcessLibraryConstructorList() --
the insertion point is likely "mostly unspecified", as no library
instance depends on DriverLoadInhibitorLib, and DriverLoadInhibitorLib
seems to depend on relatively few lib classes too. Therefore, in theory
anyway, the new lib constructor could call gBS->Exit() somewhere in the
middle of ProcessLibraryConstructorList(), with only some of the library
constructors having been executed.
Then the questions are
- does gBS->Exit() call ProcessLibraryDestructorList() or not?
- if it does not, that could lead to memory leaks.
- If it does though, is ProcessLibraryDestructorList() smart enough to
call only those destructors whose constructors have previously run?
- If not, it could call destructors on never-constructed data.
Unfortunately, this looks really tough to figure out; testing it (with
some actual library destructors) could be easier.
FWIW, there are two call sites for ProcessLibraryDestructorList() (for
UEFI/DXE dr
Re: [edk2-devel] [PATCH 1/2] OvmfPkg: Introduce NULL class library to inhibit driver load
On 8/16/22 07:30, Laszlo Ersek wrote:
On 08/15/22 11:40, 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) typo? (should be "IPv4 and IPv6" I think)
driver to be controlled from the QEMU command line.
Signed-off-by: Ard Biesheuvel
---
OvmfPkg/Library/DriverLoadInhibitorLib/DriverLoadInhibitorLib.c | 30
OvmfPkg/Library/DriverLoadInhibitorLib/DriverLoadInhibitorLib.inf | 28
++
OvmfPkg/OvmfPkg.dec | 4 +++
3 files changed, 62 insertions(+)
diff --git a/OvmfPkg/Library/DriverLoadInhibitorLib/DriverLoadInhibitorLib.c
b/OvmfPkg/Library/DriverLoadInhibitorLib/DriverLoadInhibitorLib.c
new file mode 100644
index ..dc8544bc38be
--- /dev/null
+++ b/OvmfPkg/Library/DriverLoadInhibitorLib/DriverLoadInhibitorLib.c
@@ -0,0 +1,30 @@
+// @file
+// Copyright (c) 2022, Google LLC. All rights reserved.
+// SPDX-License-Identifier: BSD-2-Clause-Patent
+//
+
+#include
+
+#include
+#include
+
+STATIC CHAR16 mExitData[] = L"Driver dispatch inhibited by QEMU fw_cfg
variable.";
+
+EFI_STATUS
+EFIAPI
+DriverLoadInhibitorLibConstructor (
+ IN EFI_HANDLEHandle,
+ IN EFI_SYSTEM_TABLE *SystemTable
+ )
+{
+ RETURN_STATUS Status;
+ BOOLEAN Enabled;
+
+ Status = QemuFwCfgParseBool (FixedPcdGetPtr (PcdDriverInhibitorFwCfgVarName),
+ &Enabled);
+ if (!RETURN_ERROR (Status) && !Enabled) {
+return gBS->Exit (Handle, EFI_REQUEST_UNLOAD_IMAGE, sizeof mExitData,
+ mExitData);
(2) Per UEFI spec, ExitData should be allocated with
gBS->AllocatePool().
(3) EFI_REQUEST_UNLOAD_IMAGE is from the PI spec; while not wrong, I
think it's strange to use here. EFI_ABORTED or something similar from
the UEFI spec would be a better fit IMO.
(4) And then, the big problem:
I agree that returning an error from the constructor would not be
beneficial, as it would cause an assertion to fail in the
ProcessLibraryConstructorList() function, in the generated "AutoGen.c"
file.
However, calling gBS->Exit() from a constructor seems unsafe to me, with
regard to library destructors.
Now, in the current case (considering patch#2), this unsafety is not
visible. That's because:
(quoting ProcessLibraryConstructorList() and
ProcessLibraryDestructorList() from
"Build/OvmfX64/NOOPT_GCC5/X64/NetworkPkg/Ip4Dxe/Ip4Dxe/DEBUG/AutoGen.c",
from an earlier build on my machine anyway):
VOID
EFIAPI
ProcessLibraryConstructorList (
IN EFI_HANDLEImageHandle,
IN EFI_SYSTEM_TABLE *SystemTable
)
{
EFI_STATUS Status;
Status = PlatformDebugLibIoPortConstructor ();
ASSERT_RETURN_ERROR (Status);
Status = UefiBootServicesTableLibConstructor (ImageHandle, SystemTable);
ASSERT_EFI_ERROR (Status);
Status = DevicePathLibConstructor (ImageHandle, SystemTable);
ASSERT_EFI_ERROR (Status);
Status = UefiRuntimeServicesTableLibConstructor (ImageHandle, SystemTable);
ASSERT_EFI_ERROR (Status);
Status = UefiLibConstructor (ImageHandle, SystemTable);
ASSERT_EFI_ERROR (Status);
Status = UefiHiiServicesLibConstructor (ImageHandle, SystemTable);
ASSERT_EFI_ERROR (Status);
Status = DpcLibConstructor (ImageHandle, SystemTable);
ASSERT_EFI_ERROR (Status);
}
VOID
EFIAPI
ProcessLibraryDestructorList (
IN EFI_HANDLEImageHandle,
IN EFI_SYSTEM_TABLE *SystemTable
)
{
}
there is no library destruction to speak of -- none of the used library
instances have resources they need to release at destruction time.
However, the general case looks problematic. The new library constructor
call would be inserted *somewhere* in ProcessLibraryConstructorList() --
the insertion point is likely "mostly unspecified", as no library
instance depends on DriverLoadInhibitorLib, and DriverLoadInhibitorLib
seems to depend on relatively few lib classes too. Therefore, in theory
anyway, the new lib constructor could call gBS->Exit() somewhere in the
middle of ProcessLibraryConstructorList(), with only some of the library
constructors having been executed.
Then the questions are
- does gBS->Exit() call ProcessLibraryDestructorList() or not?
- if it does not, that could lead to memory leaks.
- If it does though, is ProcessLibraryDestructorList() smart enough to
call only those destructors whose constructors have previously run?
- If not, it could call destructors on never-constructed data.
Unfortunately, this looks really tough to figure out; testing it (with
some actual library destructors) could be easier.
FWIW, there are two call sites for ProcessLibraryDestructorList() (for
UEFI/DXE drivers anyway); both in
"MdePkg/Library/UefiDriverEntryPoint/DriverEntryPoint.c":
- One is inside th
Re: [edk2-devel] [PATCH 1/2] OvmfPkg: Introduce NULL class library to inhibit driver load
On Tue, 16 Aug 2022 at 23:10, Brian J. Johnson wrote:
>
> On 8/16/22 07:30, Laszlo Ersek wrote:
> > On 08/15/22 11:40, 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) typo? (should be "IPv4 and IPv6" I think)
> >
> >> driver to be controlled from the QEMU command line.
> >>
> >> Signed-off-by: Ard Biesheuvel
> >> ---
> >> OvmfPkg/Library/DriverLoadInhibitorLib/DriverLoadInhibitorLib.c | 30
> >>
> >> OvmfPkg/Library/DriverLoadInhibitorLib/DriverLoadInhibitorLib.inf | 28
> >> ++
> >> OvmfPkg/OvmfPkg.dec | 4
> >> +++
> >> 3 files changed, 62 insertions(+)
> >>
> >> diff --git
> >> a/OvmfPkg/Library/DriverLoadInhibitorLib/DriverLoadInhibitorLib.c
> >> b/OvmfPkg/Library/DriverLoadInhibitorLib/DriverLoadInhibitorLib.c
> >> new file mode 100644
> >> index ..dc8544bc38be
> >> --- /dev/null
> >> +++ b/OvmfPkg/Library/DriverLoadInhibitorLib/DriverLoadInhibitorLib.c
> >> @@ -0,0 +1,30 @@
> >> +// @file
> >> +// Copyright (c) 2022, Google LLC. All rights reserved.
> >> +// SPDX-License-Identifier: BSD-2-Clause-Patent
> >> +//
> >> +
> >> +#include
> >> +
> >> +#include
> >> +#include
> >> +
> >> +STATIC CHAR16 mExitData[] = L"Driver dispatch inhibited by QEMU fw_cfg
> >> variable.";
> >> +
> >> +EFI_STATUS
> >> +EFIAPI
> >> +DriverLoadInhibitorLibConstructor (
> >> + IN EFI_HANDLEHandle,
> >> + IN EFI_SYSTEM_TABLE *SystemTable
> >> + )
> >> +{
> >> + RETURN_STATUS Status;
> >> + BOOLEAN Enabled;
> >> +
> >> + Status = QemuFwCfgParseBool (FixedPcdGetPtr
> >> (PcdDriverInhibitorFwCfgVarName),
> >> + &Enabled);
> >> + if (!RETURN_ERROR (Status) && !Enabled) {
> >> +return gBS->Exit (Handle, EFI_REQUEST_UNLOAD_IMAGE, sizeof mExitData,
> >> + mExitData);
> >
> > (2) Per UEFI spec, ExitData should be allocated with
> > gBS->AllocatePool().
> >
> > (3) EFI_REQUEST_UNLOAD_IMAGE is from the PI spec; while not wrong, I
> > think it's strange to use here. EFI_ABORTED or something similar from
> > the UEFI spec would be a better fit IMO.
> >
> > (4) And then, the big problem:
> >
> > I agree that returning an error from the constructor would not be
> > beneficial, as it would cause an assertion to fail in the
> > ProcessLibraryConstructorList() function, in the generated "AutoGen.c"
> > file.
> >
> > However, calling gBS->Exit() from a constructor seems unsafe to me, with
> > regard to library destructors.
> >
> > Now, in the current case (considering patch#2), this unsafety is not
> > visible. That's because:
> >
> > (quoting ProcessLibraryConstructorList() and
> > ProcessLibraryDestructorList() from
> > "Build/OvmfX64/NOOPT_GCC5/X64/NetworkPkg/Ip4Dxe/Ip4Dxe/DEBUG/AutoGen.c",
> > from an earlier build on my machine anyway):
> >
> >> VOID
> >> EFIAPI
> >> ProcessLibraryConstructorList (
> >>IN EFI_HANDLEImageHandle,
> >>IN EFI_SYSTEM_TABLE *SystemTable
> >>)
> >> {
> >>EFI_STATUS Status;
> >>
> >>Status = PlatformDebugLibIoPortConstructor ();
> >>ASSERT_RETURN_ERROR (Status);
> >>
> >>Status = UefiBootServicesTableLibConstructor (ImageHandle, SystemTable);
> >>ASSERT_EFI_ERROR (Status);
> >>
> >>Status = DevicePathLibConstructor (ImageHandle, SystemTable);
> >>ASSERT_EFI_ERROR (Status);
> >>
> >>Status = UefiRuntimeServicesTableLibConstructor (ImageHandle,
> >> SystemTable);
> >>ASSERT_EFI_ERROR (Status);
> >>
> >>Status = UefiLibConstructor (ImageHandle, SystemTable);
> >>ASSERT_EFI_ERROR (Status);
> >>
> >>Status = UefiHiiServicesLibConstructor (ImageHandle, SystemTable);
> >>ASSERT_EFI_ERROR (Status);
> >>
> >>Status = DpcLibConstructor (ImageHandle, SystemTable);
> >>ASSERT_EFI_ERROR (Status);
> >>
> >> }
> >>
> >>
> >>
> >> VOID
> >> EFIAPI
> >> ProcessLibraryDestructorList (
> >>IN EFI_HANDLEImageHandle,
> >>IN EFI_SYSTEM_TABLE *SystemTable
> >>)
> >> {
> >>
> >> }
> >
> > there is no library destruction to speak of -- none of the used library
> > instances have resources they need to release at destruction time.
> >
> > However, the general case looks problematic. The new library constructor
> > call would be inserted *somewhere* in ProcessLibraryConstructorList() --
> > the insertion point is likely "mostly unspecified", as no library
> > instance depends on DriverLoadInhibitorLib, and DriverLoadInhibitorLib
> > seems to depend on relatively few lib classes too. Therefore, in theory
> > anyway, the new lib constructor could call gBS->Exit() somewhere in the
> > middle of ProcessLibraryConstructorList(), with only some of the library
> > constructors having
Re: [edk2-devel] [PATCH 1/2] OvmfPkg: Introduce NULL class library to inhibit driver load
On 08/16/22 23:08, Brian J. Johnson wrote: > On 8/16/22 07:30, Laszlo Ersek wrote: >> On physical machines, I've seen firmware options for disabling the IP >> stack entirely; I wonder how those firmwares do it... > > I don't know how any physical machine handles that particular option. > But one approach would be to add a GUID to the depex of the module you > want to control, and install it only when you want the module to be > dispatched. That's pretty straightforward, although it does result in > "Driver %g was discovered but not loaded!!" messages from > CoreDisplayDiscoveredNotDispatched() if sufficient debugging is enabled. Indeed, thanks for the reminder! ArmVirtPkg and OvmfPkg uses this pattern with several core drivers: PlatformHasIoMmuLib: - depex: gEdkiiIoMmuProtocolGuid OR gIoMmuAbsentProtocolGuid - hooked into: PciHostBridgeDxe PlatformHasAcpiLib: - depex: gEdkiiPlatformHasAcpiGuid - hooked into: AcpiTableDxe NvVarStoreFormattedLib: - depex: gEdkiiNvVarStoreFormattedGuid - hooked into: VariableRuntimeDxe >From these, the first and third examples are not full matches, as the depexes injected via PlatformHasIoMmuLib and NvVarStoreFormattedLib are always supposed to be satisfied *eventually* -- the drivers that the dependencies are injected into are always required; the dependencies ensure some platform specific ordering requirements. But PlatformHasAcpiLib seems like a 100% match; it *is* meant to block AcpiTableDxe indefinitely, if the platform chooses so (dynamically). Thanks! Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#92507): https://edk2.groups.io/g/devel/message/92507 Mute This Topic: https://groups.io/mt/93032846/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 1/2] OvmfPkg: Introduce NULL class library to inhibit driver load
On 08/17/22 10:39, Ard Biesheuvel wrote: > Agree with all of the above. At this point, I think the only way to do > this properly is to create an alternative UefiDriverEntrypoint library > with the fw_cfg check folded into it. This is easy to do and addresses > all the concerns raised here (as it can force the driver entry point > function to return any value at any point) but the code duplication is > unfortunate. Ah, that's very creative. I didn't think of duplicating and customizing the entry point library! > On Tue, 16 Aug 2022 at 23:10, Brian J. Johnson > wrote: >> I don't know how any physical machine handles that particular option. >> But one approach would be to add a GUID to the depex of the module >> you want to control, and install it only when you want the module to >> be dispatched. That's pretty straightforward, although it does >> result in "Driver %g was discovered but not loaded!!" messages from >> CoreDisplayDiscoveredNotDispatched() if sufficient debugging is >> enabled. >> > > I think the diagnostic is fine. But I don't think adding DEPEXes to > UEFI drivers (as opposed to DXE drivers) is proper, or even supported. > It would also require the drivers in other packages to be updated. Sigh, I'm totally getting rusty on all this (which is quite expected). You are right about the difference between DXE and UEFI drivers wrt. depexes. :/ > What i i like about the current approach is that the library can tie > any driver or app dispatch to any fw_cfg variable in QEMU (provided > that its build is directed by the .dsc in question). Switching to an > alternate UefiDriverEntrypoint implementation would limit this to > drivers, but this is fine for the purpose at hand. Just don't forget that the number of fw_cfg slots in QEMU is limited. It is exposed as an experimental property (x-file-slots) of the fw_cfg_io and fw_cfg_mem on-board devices. It matters for migration, and so it is versioned. It used to be 0x10, for the 2.8 and earlier machine types, and has been 0x20 for later machine types. See QEMU commit a5b3ebfd23bc. IOW, while the mechanism in edk2 could scale "indefinitely", QEMU will in effect limit the number of fw_cfg files that can be used for this purpose too. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#92511): https://edk2.groups.io/g/devel/message/92511 Mute This Topic: https://groups.io/mt/93032846/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 1/2] OvmfPkg: Introduce NULL class library to inhibit driver load
On Wed, 17 Aug 2022 at 11:22, Laszlo Ersek wrote: > > On 08/17/22 10:39, Ard Biesheuvel wrote: > > > Agree with all of the above. At this point, I think the only way to do > > this properly is to create an alternative UefiDriverEntrypoint library > > with the fw_cfg check folded into it. This is easy to do and addresses > > all the concerns raised here (as it can force the driver entry point > > function to return any value at any point) but the code duplication is > > unfortunate. > > Ah, that's very creative. I didn't think of duplicating and customizing > the entry point library! > Yeah, I'll spin a v2 based on this idea. > > On Tue, 16 Aug 2022 at 23:10, Brian J. Johnson > > wrote: > >> I don't know how any physical machine handles that particular option. > >> But one approach would be to add a GUID to the depex of the module > >> you want to control, and install it only when you want the module to > >> be dispatched. That's pretty straightforward, although it does > >> result in "Driver %g was discovered but not loaded!!" messages from > >> CoreDisplayDiscoveredNotDispatched() if sufficient debugging is > >> enabled. > >> > > > > I think the diagnostic is fine. But I don't think adding DEPEXes to > > UEFI drivers (as opposed to DXE drivers) is proper, or even supported. > > It would also require the drivers in other packages to be updated. > > Sigh, I'm totally getting rusty on all this (which is quite expected). > You are right about the difference between DXE and UEFI drivers wrt. > depexes. :/ > > > What i i like about the current approach is that the library can tie > > any driver or app dispatch to any fw_cfg variable in QEMU (provided > > that its build is directed by the .dsc in question). Switching to an > > alternate UefiDriverEntrypoint implementation would limit this to > > drivers, but this is fine for the purpose at hand. > > Just don't forget that the number of fw_cfg slots in QEMU is limited. It > is exposed as an experimental property (x-file-slots) of the fw_cfg_io > and fw_cfg_mem on-board devices. It matters for migration, and so it is > versioned. It used to be 0x10, for the 2.8 and earlier machine types, > and has been 0x20 for later machine types. See QEMU commit a5b3ebfd23bc. > > IOW, while the mechanism in edk2 could scale "indefinitely", QEMU will > in effect limit the number of fw_cfg files that can be used for this > purpose too. > Right, that is good to know. But I don't think the point here is to make each individual driver controllable from the command line. The goal here is really to make this work in a way that is generic (i.e., not specific to networking) that doesn't require changes to the other packages. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#92520): https://edk2.groups.io/g/devel/message/92520 Mute This Topic: https://groups.io/mt/93032846/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
