On 05/19/16 10:42, Ni, Ruiyu wrote:
> I agree.
> 
> Reviewed-by: Ruiyu Ni <ruiyu...@intel.com>

Thanks Ray!

I'll await Jordan's feedback too.

Cheers!
Laszlo

>>-----Original Message-----
>>From: Laszlo Ersek [mailto:ler...@redhat.com]
>>Sent: Thursday, May 19, 2016 3:38 PM
>>To: Ni, Ruiyu <ruiyu...@intel.com>; edk2-devel-01 <edk2-de...@ml01.01.org>
>>Cc: Justen, Jordan L <jordan.l.jus...@intel.com>
>>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:edk2-devel-boun...@lists.01.org] On Behalf Of 
>>>> Laszlo Ersek
>>>> Sent: Thursday, May 19, 2016 6:13 AM
>>>> To: edk2-devel-01 <edk2-de...@ml01.01.org>
>>>> Cc: Ni, Ruiyu <ruiyu...@intel.com>; Justen, Jordan L 
>>>> <jordan.l.jus...@intel.com>
>>>> 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 <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:
>>>>    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
>>>> edk2-devel@lists.01.org
>>>> https://lists.01.org/mailman/listinfo/edk2-devel
> 

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to