[edk2-devel] [PATCH 13/37] OvmfPkg/IncompatiblePciDeviceSupportDxe: ignore CSM presence

["Laszlo Ersek" <lersek@redhat.com>]

The UEFI protocol database cannot contain gEfiLegacyBiosProtocolGuid any
longer, after excluding LegacyBiosDxe from the OVMF platforms. Therefore,
instruct PciBusDxe from IncompatiblePciDeviceSupportDxe to allocate 64-bit
BARs above 4 GB regardless of a CSM.

Regression test: in commit 855743f71774 ("OvmfPkg: prevent 64-bit MMIO BAR
degradation if there is no CSM", 2016-05-25), where we introduced
IncompatiblePciDeviceSupportDxe, we said, "By default, the PCI Bus driver
considers an option ROM reason enough for allocating the 64-bit MMIO BARs
in 32-bit address space". Therefore it suffices to verify the 64-bit BARs
of a device for which QEMU provides an option ROM. The simplest case is
the virtio-net-pci device. And indeed, with this patch applied, the log
contains:

> PciBus: Discovered PCI @ [04|00|00]  [VID = 0x1AF4, DID = 0x1041]
>    BAR[1]: Type =  Mem32; Alignment = 0xFFF;    Length = 0x1000;        Offset = 0x14
>    BAR[4]: Type = PMem64; Alignment = 0x3FFF;   Length = 0x4000;        Offset = 0x20

This portion shows that Bus|Device|Function 04|00|00 is a (modern)
virito-net-pci device [VID = 0x1AF4, DID = 0x1041].

> PciBus: Resource Map for Bridge [00|01|03]
> Type =  Mem32; Base = 0x81200000;       Length = 0x200000;      Alignment = 0x1FFFFF
>    Base = Padding;      Length = 0x200000;      Alignment = 0x1FFFFF
>    Base = 0x81200000;   Length = 0x1000;        Alignment = 0xFFF;      Owner = PCI [04|00|00:14]
> Type =  Mem32; Base = 0x81A43000;       Length = 0x1000;        Alignment = 0xFFF
> Type = PMem64; Base = 0x800200000;      Length = 0x100000;      Alignment = 0xFFFFF
>    Base = 0x800200000;  Length = 0x4000;        Alignment = 0x3FFF;     Owner = PCI [04|00|00:20]

This quote shows that 04|00|00 has a BAR at 0x8_0020_0000.

(It also shows that the device is behind a bridge (PCIe root port) whose
own BDF is 00|01|03.)

> [Security] 3rd party image[7CEEB418] can be loaded after EndOfDxe: PciRoot(0x0)/Pci(0x1,0x3)/Pci(0x0,0x0)/Offset(0x10E00,0x273FF).
> None of Tcg2Protocol/CcMeasurementProtocol is installed.
> InstallProtocolInterface: [EfiLoadedImageProtocol] 7D2E5140
> Loading driver at 0x0007CA9F000 EntryPoint=0x0007CAA5447 1af41000.efi
> InstallProtocolInterface: [EfiLoadedImageDevicePathProtocol] 7D5B2198

And this part finally shows that the iPXE option ROM for the device
(1af41000.efi) was detected and is loaded. (Same PCIe root port, and PCIe
root ports can only host a single device.)

Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=4588
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf |   5 +-
 OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.c   | 141 +-------------------
 2 files changed, 6 insertions(+), 140 deletions(-)

diff --git a/OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf b/OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf
index ad38128fcbf9..b92662d0bb36 100644
--- a/OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf
+++ b/OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf
@@ -1,7 +1,7 @@
 ## @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.
+# MMIO BARs above 4 GB, regardless of option ROM availability, conserving 32-bit
+# MMIO aperture for 32-bit BARs.
 #
 # Copyright (C) 2016, Red Hat, Inc.
 #
@@ -33,7 +33,6 @@ [LibraryClasses]
 
 [Protocols]
   gEfiIncompatiblePciDeviceSupportProtocolGuid ## SOMETIMES_PRODUCES
-  gEfiLegacyBiosProtocolGuid                   ## NOTIFY
 
 [Pcd]
   gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Size                ## CONSUMES
diff --git a/OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.c b/OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.c
index 3092a174bc51..434cdca84b23 100644
--- a/OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.c
+++ b/OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.c
@@ -1,7 +1,7 @@
 /** @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.
+  MMIO BARs above 4 GB, regardless of option ROM availability, conserving 32-bit
+  MMIO aperture for 32-bit BARs.
 
   Copyright (C) 2016, Red Hat, Inc.
   Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
@@ -21,12 +21,6 @@
 #include <Library/CcProbeLib.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.
@@ -111,50 +105,6 @@ STATIC CONST EFI_ACPI_END_TAG_DESCRIPTOR  mEndDesc = {
   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.
@@ -228,7 +178,6 @@ CheckDevice (
   OUT VOID                                          **Configuration
   )
 {
-  mCheckDeviceCalled = TRUE;
   UINTN  Length;
   UINT8  *Ptr;
 
@@ -243,17 +192,8 @@ CheckDevice (
   // 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;
-  }
-
+  // address space. But, we don't support legacy option ROMs at all, thus this
+  // problem cannot arise.
   //
   // This member function is mis-specified actually: it is supposed to allocate
   // memory, but as specified, it could not return an error status. Thankfully,
@@ -317,8 +257,6 @@ DriverInitialize (
   )
 {
   EFI_STATUS  Status;
-  EFI_EVENT   Event;
-  VOID        *Registration;
 
   //
   // If there is no 64-bit PCI MMIO aperture, then 64-bit MMIO BARs have to be
@@ -328,63 +266,6 @@ DriverInitialize (
     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,
@@ -392,19 +273,5 @@ DriverInitialize (
                                                      &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;
 }



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111084): https://edk2.groups.io/g/devel/message/111084
Mute This Topic: https://groups.io/mt/102518651/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-