I agree. Reviewed-by: Ruiyu Ni <[email protected]>
>-----Original Message----- >From: Laszlo Ersek [mailto:[email protected]] >Sent: Thursday, May 19, 2016 3:38 PM >To: Ni, Ruiyu <[email protected]>; edk2-devel-01 <[email protected]> >Cc: Justen, Jordan L <[email protected]> >Subject: Re: [edk2] [PATCH] OvmfPkg: prevent 64-bit MMIO BAR degradation if >there is no CSM > >On 05/19/16 04:02, Ni, Ruiyu wrote: >> Laszlo, >> All are good except: can you use EfiCreateProtocolNotifyEvent() >> exposed by UefiLib? > >I am aware of that function, but I dislike it. The reason I dislike it >is that (in my opinion) its internal error handling is inferior. For the >return values of gBS->CreateEvent() and gBS->RegisterProtocolNotify(), >is only uses ASSERT_EFI_ERROR(), and to the return value of >gBS->SignalEvent(), it doesn't apply any check. > >I think the error handling in my code below is superior. CreateEvent() >can legitimately fail with EFI_OUT_OF_RESOURCES. So can >RegisterProtocolNotify(). > >I wondered before how the error handling could be fixed in >EfiCreateProtocolNotifyEvent(). One problem I see is that it returns an >EFI_EVENT object, not an EFI_STATUS one. > >Given that EFI_EVENT is just a typedef for VOID*, we could return a NULL >if something fails. However, in the library class header file, such a >return value is not permitted: > > @return The notification event that was created. > >This means that the ASSERT_EFI_ERROR() macro invocations in the function >body actually conform to the specification: namely, >EfiCreateProtocolNotifyEvent() is never supposed to fail (and I guess >all of its callers rely on this). > >Too bad that this function specification doesn't actually reflect >reality. (See the above EFI_OUT_OF_RESOURCES failure modes for >CreateEvent() and RegisterProtocolNotify().) > >Which is, I guess, my ultimate reason for never using this utility >function, and always open-coding CreateEvent() + >RegisterProtocolNotify() + SignalEvent() with proper error checking >(using an assert for SignalEvent(), which really can't fail, according >to the UEFI spec). > >Thanks >Laszlo > >>> -----Original Message----- >>> From: edk2-devel [mailto:[email protected]] On Behalf Of >>> Laszlo Ersek >>> Sent: Thursday, May 19, 2016 6:13 AM >>> To: edk2-devel-01 <[email protected]> >>> Cc: Ni, Ruiyu <[email protected]>; Justen, Jordan L >>> <[email protected]> >>> Subject: [edk2] [PATCH] OvmfPkg: prevent 64-bit MMIO BAR degradation if >>> there is no CSM >>> >>> According to edk2 commit >>> >>> "MdeModulePkg/PciBus: do not improperly degrade resource" >>> >>> and to the EFI_INCOMPATIBLE_PCI_DEVICE_SUPPORT_PROTOCOL definition in the >>> Platform Init 1.4a specification, a platform can provide such a protocol >>> in order to influence the PCI resource allocation performed by the PCI Bus >>> driver. >>> >>> In particular it is possible instruct the PCI Bus driver, with a >>> "wildcard" hint, to allocate the 64-bit MMIO BARs of a device in 64-bit >>> address space, regardless of whether the device features an option ROM. >>> >>> (By default, the PCI Bus driver considers an option ROM reason enough for >>> allocating the 64-bit MMIO BARs in 32-bit address space. It cannot know if >>> BDS will launch a legacy boot option, and under legacy boot, a legacy BIOS >>> binary from a combined option ROM could be dispatched, and fail to access >>> MMIO BARs in 64-bit address space.) >>> >>> In platform code we can ascertain whether a CSM is present or not. If not, >>> then legacy BIOS binaries in option ROMs can't be dispatched, hence the >>> BAR degradation is detrimental, and we should prevent it. This is expected >>> to conserve the 32-bit address space for 32-bit MMIO BARs. >>> >>> The driver added in this patch could be simplified based on the following >>> facts: >>> >>> - In the Ia32 build, the 64-bit MMIO aperture is always zero-size, hence >>> the driver will exit immediately. Therefore the driver could be omitted >>> from the Ia32 build. >>> >>> - In the Ia32X64 and X64 builds, the driver could be omitted if CSM_ENABLE >>> was defined (because in that case the degradation would be justified). >>> On the other hand, if CSM_ENABLE was undefined, then the driver could be >>> included, and it could provide the hint unconditionally (without looking >>> for the Legacy BIOS protocol). >>> >>> These short-cuts are not taken because they would increase the differences >>> between the OVMF DSC/FDF files. If we can manage without extreme >>> complexity, we should use dynamic logic (vs. build time configuration), >>> plus keep conditional compilation to a minimum. >>> >>> Cc: Jordan Justen <[email protected]> >>> Cc: Ruiyu Ni <[email protected]> >>> Contributed-under: TianoCore Contribution Agreement 1.0 >>> Signed-off-by: Laszlo Ersek <[email protected]> >>> --- >>> >>> Notes: >>> This patch will make a difference only when Ray's patch at >>> <http://thread.gmane.org/gmane.comp.bios.edk2.devel/12290/focus=12292> >>> is committed. >>> >>> OvmfPkg/OvmfPkgIa32.dsc | >>> 1 + >>> OvmfPkg/OvmfPkgIa32X64.dsc | >>> 1 + >>> OvmfPkg/OvmfPkgX64.dsc | >>> 1 + >>> OvmfPkg/OvmfPkgIa32.fdf | >>> 1 + >>> OvmfPkg/OvmfPkgIa32X64.fdf | >>> 1 + >>> OvmfPkg/OvmfPkgX64.fdf | >>> 1 + >>> OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf | >>> 50 +++ >>> OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.c | >>> 353 ++++++++++++++++++++ >>> 8 files changed, 409 insertions(+) >>> >>> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc >>> index 45569f2875b5..1becb65cd878 100644 >>> --- a/OvmfPkg/OvmfPkgIa32.dsc >>> +++ b/OvmfPkg/OvmfPkgIa32.dsc >>> @@ -552,6 +552,7 @@ [Components] >>> UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf >>> UefiCpuPkg/CpuDxe/CpuDxe.inf >>> PcAtChipsetPkg/8254TimerDxe/8254Timer.inf >>> + OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf >>> MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf { >>> <LibraryClasses> >>> PciHostBridgeLib|OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf >>> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc >>> index 05ff1c914455..3d838cdcb125 100644 >>> --- a/OvmfPkg/OvmfPkgIa32X64.dsc >>> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc >>> @@ -561,6 +561,7 @@ [Components.X64] >>> UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf >>> UefiCpuPkg/CpuDxe/CpuDxe.inf >>> PcAtChipsetPkg/8254TimerDxe/8254Timer.inf >>> + OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf >>> MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf { >>> <LibraryClasses> >>> PciHostBridgeLib|OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf >>> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc >>> index 54d4413f11f6..16d58bad7865 100644 >>> --- a/OvmfPkg/OvmfPkgX64.dsc >>> +++ b/OvmfPkg/OvmfPkgX64.dsc >>> @@ -559,6 +559,7 @@ [Components] >>> UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf >>> UefiCpuPkg/CpuDxe/CpuDxe.inf >>> PcAtChipsetPkg/8254TimerDxe/8254Timer.inf >>> + OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf >>> MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf { >>> <LibraryClasses> >>> PciHostBridgeLib|OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf >>> diff --git a/OvmfPkg/OvmfPkgIa32.fdf b/OvmfPkg/OvmfPkgIa32.fdf >>> index 594e84fd713b..8ab1633f832a 100644 >>> --- a/OvmfPkg/OvmfPkgIa32.fdf >>> +++ b/OvmfPkg/OvmfPkgIa32.fdf >>> @@ -207,6 +207,7 @@ [FV.DXEFV] >>> INF UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf >>> INF UefiCpuPkg/CpuDxe/CpuDxe.inf >>> INF PcAtChipsetPkg/8254TimerDxe/8254Timer.inf >>> +INF >>> OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf >>> INF MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf >>> INF MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf >>> INF PcAtChipsetPkg/KbcResetDxe/Reset.inf >>> diff --git a/OvmfPkg/OvmfPkgIa32X64.fdf b/OvmfPkg/OvmfPkgIa32X64.fdf >>> index 36a8f7e279c3..85fead07e957 100644 >>> --- a/OvmfPkg/OvmfPkgIa32X64.fdf >>> +++ b/OvmfPkg/OvmfPkgIa32X64.fdf >>> @@ -207,6 +207,7 @@ [FV.DXEFV] >>> INF UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf >>> INF UefiCpuPkg/CpuDxe/CpuDxe.inf >>> INF PcAtChipsetPkg/8254TimerDxe/8254Timer.inf >>> +INF >>> OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf >>> INF MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf >>> INF MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf >>> INF PcAtChipsetPkg/KbcResetDxe/Reset.inf >>> diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf >>> index 4b1fa944ad1b..dd5ea30662d6 100644 >>> --- a/OvmfPkg/OvmfPkgX64.fdf >>> +++ b/OvmfPkg/OvmfPkgX64.fdf >>> @@ -207,6 +207,7 @@ [FV.DXEFV] >>> INF UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf >>> INF UefiCpuPkg/CpuDxe/CpuDxe.inf >>> INF PcAtChipsetPkg/8254TimerDxe/8254Timer.inf >>> +INF >>> OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf >>> INF MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf >>> INF MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf >>> INF PcAtChipsetPkg/KbcResetDxe/Reset.inf >>> diff --git >>> a/OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf >>> b/OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf >>> new file mode 100644 >>> index 000000000000..883e45c17436 >>> --- /dev/null >>> +++ >>> b/OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf >>> @@ -0,0 +1,50 @@ >>> +## @file >>> +# A simple DXE_DRIVER that causes the PCI Bus UEFI_DRIVER to allocate >>> 64-bit >>> +# MMIO BARs above 4 GB, regardless of option ROM availability (as long as >>> a CSM >>> +# is not present), conserving 32-bit MMIO aperture for 32-bit BARs. >>> +# >>> +# Copyright (C) 2016, Red Hat, Inc. >>> +# >>> +# This program and the accompanying materials are licensed and made >>> available >>> +# under the terms and conditions of the BSD License which accompanies this >>> +# distribution. The full text of the license may be found at >>> +# http://opensource.org/licenses/bsd-license.php >>> +# >>> +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, >>> WITHOUT >>> +# WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. >>> +## >>> + >>> +[Defines] >>> + INF_VERSION = 0x00010005 >>> + BASE_NAME = IncompatiblePciDeviceSupportDxe >>> + FILE_GUID = F6697AC4-A776-4EE1-B643-1FEFF2B615BB >>> + MODULE_TYPE = DXE_DRIVER >>> + VERSION_STRING = 1.0 >>> + ENTRY_POINT = DriverInitialize >>> + >>> +[Sources] >>> + IncompatiblePciDeviceSupport.c >>> + >>> +[Packages] >>> + IntelFrameworkPkg/IntelFrameworkPkg.dec >>> + MdeModulePkg/MdeModulePkg.dec >>> + MdePkg/MdePkg.dec >>> + OvmfPkg/OvmfPkg.dec >>> + >>> +[LibraryClasses] >>> + DebugLib >>> + MemoryAllocationLib >>> + PcdLib >>> + UefiBootServicesTableLib >>> + UefiDriverEntryPoint >>> + >>> +[Protocols] >>> + gEfiIncompatiblePciDeviceSupportProtocolGuid ## SOMETIMES_PRODUCES >>> + gEfiLegacyBiosProtocolGuid ## NOTIFY >>> + >>> +[Pcd] >>> + gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration ## CONSUMES >>> + gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Size ## CONSUMES >>> + >>> +[Depex] >>> + TRUE >>> diff --git >>> a/OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.c >>> b/OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.c >>> new file mode 100644 >>> index 000000000000..b6ff12842c21 >>> --- /dev/null >>> +++ b/OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.c >>> @@ -0,0 +1,353 @@ >>> +/** @file >>> + A simple DXE_DRIVER that causes the PCI Bus UEFI_DRIVER to allocate >>> 64-bit >>> + MMIO BARs above 4 GB, regardless of option ROM availability (as long as >>> a CSM >>> + is not present), conserving 32-bit MMIO aperture for 32-bit BARs. >>> + >>> + Copyright (C) 2016, Red Hat, Inc. >>> + >>> + This program and the accompanying materials are licensed and made >>> available >>> + under the terms and conditions of the BSD License which accompanies this >>> + distribution. The full text of the license may be found at >>> + http://opensource.org/licenses/bsd-license.php >>> + >>> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, >>> WITHOUT >>> + WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. >>> +**/ >>> + >>> +#include <IndustryStandard/Acpi10.h> >>> +#include <IndustryStandard/Pci22.h> >>> + >>> +#include <Library/DebugLib.h> >>> +#include <Library/MemoryAllocationLib.h> >>> +#include <Library/PcdLib.h> >>> +#include <Library/UefiBootServicesTableLib.h> >>> + >>> +#include <Protocol/IncompatiblePciDeviceSupport.h> >>> +#include <Protocol/LegacyBios.h> >>> + >>> +// >>> +// The Legacy BIOS protocol has been located. >>> +// >>> +STATIC BOOLEAN mLegacyBiosInstalled; >>> + >>> +// >>> +// The protocol interface this driver produces. >>> +// >>> +STATIC EFI_INCOMPATIBLE_PCI_DEVICE_SUPPORT_PROTOCOL >>> + >>> mIncompatiblePciDeviceSupport; >>> + >>> +// >>> +// Configuration template for the CheckDevice() protocol member function. >>> +// >>> +// Refer to Table 20 "ACPI 2.0 & 3.0 QWORD Address Space Descriptor Usage" >>> in >>> +// the Platform Init 1.4a Spec, Volume 5. >>> +// >>> +// This structure is interpreted by the UpdatePciInfo() function in the >>> edk2 >>> +// PCI Bus UEFI_DRIVER. >>> +// >>> +#pragma pack (1) >>> +typedef struct { >>> + EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR AddressSpaceDesc; >>> + EFI_ACPI_END_TAG_DESCRIPTOR EndDesc; >>> +} MMIO64_PREFERENCE; >>> +#pragma pack () >>> + >>> +STATIC CONST MMIO64_PREFERENCE mConfiguration = { >>> + // >>> + // AddressSpaceDesc >>> + // >>> + { >>> + ACPI_ADDRESS_SPACE_DESCRIPTOR, // Desc >>> + (UINT16)( // Len >>> + sizeof (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR) - >>> + OFFSET_OF ( >>> + EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR, >>> + ResType >>> + ) >>> + ), >>> + ACPI_ADDRESS_SPACE_TYPE_MEM, // ResType >>> + PCI_ACPI_UNUSED, // GenFlag >>> + PCI_ACPI_UNUSED, // SpecificFlag >>> + 64, // AddrSpaceGranularity: >>> + // aperture selection >>> hint >>> + // for BAR allocation >>> + PCI_ACPI_UNUSED, // AddrRangeMin >>> + PCI_BAR_OLD_ALIGN, // AddrRangeMax: >>> + // no special >>> alignment >>> + // for affected BARs >>> + PCI_BAR_ALL, // >>> AddrTranslationOffset: >>> + // hint covers all >>> + // eligible BARs >>> + PCI_BAR_NOCHANGE // AddrLen: >>> + // use probed BAR size >>> + }, >>> + // >>> + // EndDesc >>> + // >>> + { >>> + ACPI_END_TAG_DESCRIPTOR, // Desc >>> + 0 // Checksum: to be >>> ignored >>> + } >>> +}; >>> + >>> +// >>> +// The CheckDevice() member function has been called. >>> +// >>> +STATIC BOOLEAN mCheckDeviceCalled; >>> + >>> + >>> +/** >>> + Notification callback for Legacy BIOS protocol installation. >>> + >>> + @param[in] Event Event whose notification function is being invoked. >>> + >>> + @param[in] Context The pointer to the notification function's context, >>> which >>> + is implementation-dependent. >>> +**/ >>> +STATIC >>> +VOID >>> +EFIAPI >>> +LegacyBiosInstalled ( >>> + IN EFI_EVENT Event, >>> + IN VOID *Context >>> + ) >>> +{ >>> + EFI_STATUS Status; >>> + EFI_LEGACY_BIOS_PROTOCOL *LegacyBios; >>> + >>> + ASSERT (!mCheckDeviceCalled); >>> + >>> + Status = gBS->LocateProtocol (&gEfiLegacyBiosProtocolGuid, >>> + NULL /* Registration */, (VOID **)&LegacyBios); >>> + if (EFI_ERROR (Status)) { >>> + return; >>> + } >>> + >>> + mLegacyBiosInstalled = TRUE; >>> + >>> + // >>> + // Close the event and deregister this callback. >>> + // >>> + Status = gBS->CloseEvent (Event); >>> + ASSERT_EFI_ERROR (Status); >>> +} >>> + >>> + >>> +/** >>> + Returns a list of ACPI resource descriptors that detail the special >>> resource >>> + configuration requirements for an incompatible PCI device. >>> + >>> + Prior to bus enumeration, the PCI bus driver will look for the presence >>> of >>> + the EFI_INCOMPATIBLE_PCI_DEVICE_SUPPORT_PROTOCOL. Only one instance of >>> this >>> + protocol can be present in the system. For each PCI device that the PCI >>> bus >>> + driver discovers, the PCI bus driver calls this function with the >>> device's >>> + vendor ID, device ID, revision ID, subsystem vendor ID, and subsystem >>> device >>> + ID. If the VendorId, DeviceId, RevisionId, SubsystemVendorId, or >>> + SubsystemDeviceId value is set to (UINTN)-1, that field will be ignored. >>> The >>> + ID values that are not (UINTN)-1 will be used to identify the current >>> device. >>> + >>> + This function will only return EFI_SUCCESS. However, if the device is an >>> + incompatible PCI device, a list of ACPI resource descriptors will be >>> returned >>> + in Configuration. Otherwise, NULL will be returned in Configuration >>> instead. >>> + The PCI bus driver does not need to allocate memory for Configuration. >>> + However, it is the PCI bus driver's responsibility to free it. The PCI >>> bus >>> + driver then can configure this device with the information that is >>> derived >>> + from this list of resource nodes, rather than the result of BAR probing. >>> + >>> + Only the following two resource descriptor types from the ACPI >>> Specification >>> + may be used to describe the incompatible PCI device resource >>> requirements: >>> + - QWORD Address Space Descriptor (ACPI 2.0, section 6.4.3.5.1; also ACPI >>> 3.0) >>> + - End Tag (ACPI 2.0, section 6.4.2.8; also ACPI 3.0) >>> + >>> + The QWORD Address Space Descriptor can describe memory, I/O, and bus >>> number >>> + ranges for dynamic or fixed resources. The configuration of a PCI root >>> bridge >>> + is described with one or more QWORD Address Space Descriptors, followed >>> by an >>> + End Tag. See the ACPI Specification for details on the field values. >>> + >>> + @param[in] This Pointer to the >>> + >>> EFI_INCOMPATIBLE_PCI_DEVICE_SUPPORT_PROTOCOL >>> + instance. >>> + >>> + @param[in] VendorId A unique ID to identify the manufacturer >>> of >>> + the PCI device. See the Conventional PCI >>> + Specification 3.0 for details. >>> + >>> + @param[in] DeviceId A unique ID to identify the particular >>> PCI >>> + device. See the Conventional PCI >>> + Specification 3.0 for details. >>> + >>> + @param[in] RevisionId A PCI device-specific revision >>> identifier. >>> + See the Conventional PCI Specification >>> 3.0 >>> + for details. >>> + >>> + @param[in] SubsystemVendorId Specifies the subsystem vendor ID. See >>> the >>> + Conventional PCI Specification 3.0 for >>> + details. >>> + >>> + @param[in] SubsystemDeviceId Specifies the subsystem device ID. See >>> the >>> + Conventional PCI Specification 3.0 for >>> + details. >>> + >>> + @param[out] Configuration A list of ACPI resource descriptors that >>> + detail the configuration requirement. >>> + >>> + @retval EFI_SUCCESS The function always returns EFI_SUCCESS. >>> +**/ >>> +STATIC >>> +EFI_STATUS >>> +EFIAPI >>> +CheckDevice ( >>> + IN EFI_INCOMPATIBLE_PCI_DEVICE_SUPPORT_PROTOCOL *This, >>> + IN UINTN VendorId, >>> + IN UINTN DeviceId, >>> + IN UINTN RevisionId, >>> + IN UINTN SubsystemVendorId, >>> + IN UINTN SubsystemDeviceId, >>> + OUT VOID **Configuration >>> + ) >>> +{ >>> + mCheckDeviceCalled = TRUE; >>> + >>> + // >>> + // Unlike the general description of this protocol member suggests, >>> there is >>> + // nothing incompatible about the PCI devices that we'll match here. >>> We'll >>> + // match all PCI devices, and generate exactly one QWORD Address Space >>> + // Descriptor for each. That descriptor will instruct the PCI Bus >>> UEFI_DRIVER >>> + // not to degrade 64-bit MMIO BARs for the device, even if a PCI option >>> ROM >>> + // BAR is present on the device. >>> + // >>> + // The concern captured in the PCI Bus UEFI_DRIVER is that a legacy BIOS >>> boot >>> + // (via a CSM) could dispatch a legacy option ROM on the device, which >>> might >>> + // have trouble with MMIO BARs that have been allocated outside of the >>> 32-bit >>> + // address space. But, if we don't support legacy option ROMs at all, >>> then >>> + // this problem cannot arise. >>> + // >>> + if (mLegacyBiosInstalled) { >>> + // >>> + // Don't interfere with resource degradation. >>> + // >>> + *Configuration = NULL; >>> + return EFI_SUCCESS; >>> + } >>> + >>> + // >>> + // This member function is mis-specified actually: it is supposed to >>> allocate >>> + // memory, but as specified, it could not return an error status. >>> Thankfully, >>> + // the edk2 PCI Bus UEFI_DRIVER actually handles error codes; see the >>> + // UpdatePciInfo() function. >>> + // >>> + *Configuration = AllocateCopyPool (sizeof mConfiguration, >>> &mConfiguration); >>> + if (*Configuration == NULL) { >>> + DEBUG ((EFI_D_WARN, >>> + "%a: 64-bit MMIO BARs may be degraded for PCI 0x%04x:0x%04x (rev >>> %d)\n", >>> + __FUNCTION__, (UINT32)VendorId, (UINT32)DeviceId, >>> (UINT8)RevisionId)); >>> + return EFI_OUT_OF_RESOURCES; >>> + } >>> + return EFI_SUCCESS; >>> +} >>> + >>> + >>> +/** >>> + Entry point for this driver. >>> + >>> + @param[in] ImageHandle Image handle of this driver. >>> + @param[in] SystemTable Pointer to SystemTable. >>> + >>> + @retval EFI_SUCESS Driver has loaded successfully. >>> + @retval EFI_UNSUPPORTED PCI resource allocation has been disabled. >>> + @retval EFI_UNSUPPORTED There is no 64-bit PCI MMIO aperture. >>> + @return Error codes from lower level functions. >>> + >>> +**/ >>> +EFI_STATUS >>> +EFIAPI >>> +DriverInitialize ( >>> + IN EFI_HANDLE ImageHandle, >>> + IN EFI_SYSTEM_TABLE *SystemTable >>> + ) >>> +{ >>> + EFI_STATUS Status; >>> + EFI_EVENT Event; >>> + VOID *Registration; >>> + >>> + // >>> + // If the PCI Bus driver is not supposed to allocate resources, then it >>> makes >>> + // no sense to install a protocol that influences the resource >>> allocation. >>> + // >>> + // Similarly, if there is no 64-bit PCI MMIO aperture, then 64-bit MMIO >>> BARs >>> + // have to be allocated under 4 GB unconditionally. >>> + // >>> + if (PcdGetBool (PcdPciDisableBusEnumeration) || >>> + PcdGet64 (PcdPciMmio64Size) == 0) { >>> + return EFI_UNSUPPORTED; >>> + } >>> + >>> + // >>> + // Otherwise, create a protocol notify to see if a CSM is present. (With >>> the >>> + // CSM absent, the PCI Bus driver won't have to worry about allocating >>> 64-bit >>> + // MMIO BARs in the 32-bit MMIO aperture, for the sake of a legacy BIOS.) >>> + // >>> + // If the Legacy BIOS Protocol is present at the time of this driver >>> starting >>> + // up, we can mark immediately that the PCI Bus driver should perform the >>> + // usual 64-bit MMIO BAR degradation. >>> + // >>> + // Otherwise, if the Legacy BIOS Protocol is absent at startup, it may be >>> + // installed later. However, if it doesn't show up until the first >>> + // EFI_INCOMPATIBLE_PCI_DEVICE_SUPPORT_PROTOCOL.CheckDevice() call from >>> the >>> + // PCI Bus driver, then it never will: >>> + // >>> + // 1. The following drivers are dispatched in some unspecified order: >>> + // - PCI Host Bridge DXE_DRIVER, >>> + // - PCI Bus UEFI_DRIVER, >>> + // - this DXE_DRIVER, >>> + // - Legacy BIOS DXE_DRIVER. >>> + // >>> + // 2. The DXE_CORE enters BDS. >>> + // >>> + // 3. The platform BDS connects the PCI Root Bridge IO instances >>> (produced by >>> + // the PCI Host Bridge DXE_DRIVER). >>> + // >>> + // 4. The PCI Bus UEFI_DRIVER enumerates resources and calls into this >>> + // DXE_DRIVER (CheckDevice()). >>> + // >>> + // 5. This driver remembers if EFI_LEGACY_BIOS_PROTOCOL has been >>> installed >>> + // sometime during step 1 (produced by the Legacy BIOS DXE_DRIVER). >>> + // >>> + // For breaking this order, the Legacy BIOS DXE_DRIVER would have to >>> install >>> + // its protocol after the firmware enters BDS, which cannot happen. >>> + // >>> + Status = gBS->CreateEvent (EVT_NOTIFY_SIGNAL, TPL_CALLBACK, >>> + LegacyBiosInstalled, NULL /* Context */, &Event); >>> + if (EFI_ERROR (Status)) { >>> + return Status; >>> + } >>> + >>> + Status = gBS->RegisterProtocolNotify (&gEfiLegacyBiosProtocolGuid, Event, >>> + &Registration); >>> + if (EFI_ERROR (Status)) { >>> + goto CloseEvent; >>> + } >>> + >>> + Status = gBS->SignalEvent (Event); >>> + ASSERT_EFI_ERROR (Status); >>> + >>> + mIncompatiblePciDeviceSupport.CheckDevice = CheckDevice; >>> + Status = gBS->InstallMultipleProtocolInterfaces (&ImageHandle, >>> + &gEfiIncompatiblePciDeviceSupportProtocolGuid, >>> + &mIncompatiblePciDeviceSupport, NULL); >>> + if (EFI_ERROR (Status)) { >>> + goto CloseEvent; >>> + } >>> + >>> + return EFI_SUCCESS; >>> + >>> +CloseEvent: >>> + if (!mLegacyBiosInstalled) { >>> + EFI_STATUS CloseStatus; >>> + >>> + CloseStatus = gBS->CloseEvent (Event); >>> + ASSERT_EFI_ERROR (CloseStatus); >>> + } >>> + >>> + return Status; >>> +} >>> -- >>> 1.8.3.1 >>> >>> _______________________________________________ >>> edk2-devel mailing list >>> [email protected] >>> https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
