The following call tree exposes a bug in the lifetime (ie. too late
creation) of PciIoDevice->DevicePath. The bug can be triggered when
PciBusDxe is built into OVMF, with qemu/KVM device assignment of
a PCI-express device on the default 440FX machine type. OVMF
correctly discovers that the device is PCIe and begins probing
extended configuration space for the device. The root bridge
has no way to access extended config space and correctly errors,
sending us into the error reporting chain seen below. It's
possible that this error path could also be reproduced on physical
hardware when a PCI-to-PCIe bridge is present.
GatherDeviceInfo() | GatherPpbInfo() | GatherP2CInfo()
[MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c]
CreatePciIoDevice()
[MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c]
AllocateZeroPool()
LocateCapabilityRegBlock()
[MdeModulePkg/Bus/Pci/PciBusDxe/PciCommand.c]
PciIoDevice->IsPciExp = TRUE
LocatePciExpressCapabilityRegBlock()
[MdeModulePkg/Bus/Pci/PciBusDxe/PciCommand.c]
PciIoConfigRead() via funcptr
[MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c]
RootBridgeIoPciRead() via funcptr
[PcAtChipsetPkg/PciHostBridgeDxe/PciRootBridgeIo.c]
FAILS
REPORT_STATUS_CODE_WITH_DEVICE_PATH()
[MdePkg/Include/Library/ReportStatusCodeLib.h]
ReportStatusCodeWithDevicePath()
[MdeModulePkg/Library/DxeReportStatusCodeLib/ReportStatusCodeLib.c]
ASSERT (DevicePath != NULL) <--+
CreatePciDevicePath() |
[MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c]
sets PciIoDevice->DevicePath ----------+
In English:
- CreatePciIoDevice() allocates a zeroed out PCI_IO_DEVICE structure.
- PciIoConfigRead() tries to access the (extended) config space, and
fails.
- PciIoConfigRead() wants to report a status code (read error) for the
device path.
- Unfortuantely, PciIoDevice->DevicePath is still NULL at that point.
- The ASSERT() in ReportStatusCodeWithDevicePath() fires.
Fix it by moving CreatePciDevicePath() into CreatePciIoDevice(),
allowing PciIoDevice->DevicePath to be initialized before we
begin probing the device capabilities:
GatherDeviceInfo() | GatherPpbInfo() | GatherP2CInfo()
[MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c]
CreatePciIoDevice()
[MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c]
AllocateZeroPool()
CreatePciDevicePath()
[MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c]
sets PciIoDevice->DevicePath -----------+
LocateCapabilityRegBlock() |
[MdeModulePkg/Bus/Pci/PciBusDxe/PciCommand.c]
PciIoDevice->IsPciExp = TRUE |
LocatePciExpressCapabilityRegBlock() |
[MdeModulePkg/Bus/Pci/PciBusDxe/PciCommand.c]
PciIoConfigRead() via funcptr |
[MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c]
RootBridgeIoPciRead() via funcptr |
[PcAtChipsetPkg/PciHostBridgeDxe/PciRootBridgeIo.c]
FAILS |
REPORT_STATUS_CODE_WITH_DEVICE_PATH() |
[MdePkg/Include/Library/ReportStatusCodeLib.h]
ReportStatusCodeWithDevicePath() |
[MdeModulePkg/Library/DxeReportStatusCodeLib/ReportStatusCodeLib.c]
ASSERT (DevicePath != NULL) <-----+
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Alex Williamson <[email protected]>
Reviewed-by: Laszlo Ersek <[email protected]>
---
.../Bus/Pci/PciBusDxe/PciEnumeratorSupport.c | 32 +++++---------------
1 file changed, 8 insertions(+), 24 deletions(-)
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
index 8c31831..f5060fc 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
@@ -393,14 +393,6 @@ GatherDeviceInfo (
}
//
- // Create a device path for this PCI device and store it into its private
data
- //
- CreatePciDevicePath (
- Bridge->DevicePath,
- PciIoDevice
- );
-
- //
// If it is a full enumeration, disconnect the device in advance
//
if (gFullEnumeration) {
@@ -475,14 +467,6 @@ GatherPpbInfo (
return NULL;
}
- //
- // Create a device path for this PCI device and store it into its private
data
- //
- CreatePciDevicePath (
- Bridge->DevicePath,
- PciIoDevice
- );
-
if (gFullEnumeration) {
PCI_DISABLE_COMMAND_REGISTER (PciIoDevice, EFI_PCI_COMMAND_BITS_OWNED);
@@ -634,14 +618,6 @@ GatherP2CInfo (
return NULL;
}
- //
- // Create a device path for this PCI device and store it into its private
data
- //
- CreatePciDevicePath (
- Bridge->DevicePath,
- PciIoDevice
- );
-
if (gFullEnumeration) {
PCI_DISABLE_COMMAND_REGISTER (PciIoDevice, EFI_PCI_COMMAND_BITS_OWNED);
@@ -2029,6 +2005,14 @@ CreatePciIoDevice (
PciIo = &PciIoDevice->PciIo;
//
+ // Create a device path for this PCI device and store it into its private
data
+ //
+ CreatePciDevicePath (
+ Bridge->DevicePath,
+ PciIoDevice
+ );
+
+ //
// Detect if PCI Express Device
//
PciIoDevice->PciExpressCapabilityOffset = 0;
------------------------------------------------------------------------------
HPCC Systems Open Source Big Data Platform from LexisNexis Risk Solutions
Find What Matters Most in Your Big Data with HPCC Systems
Open Source. Fast. Scalable. Simple. Ideal for Dirty Data.
Leverages Graph Analysis for Fast Processing & Easy Data Exploration
http://p.sf.net/sfu/hpccsystems
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/edk2-devel
