diff options
author | Alex Williamson <alex.williamson@redhat.com> | 2014-06-19 01:41:15 +0000 |
---|---|---|
committer | niruiyu <niruiyu@6f19259b-4bc3-4df7-8a09-765794883524> | 2014-06-19 01:41:15 +0000 |
commit | b0bc24aff5205e71aebbb7a15872974bb131d0f3 (patch) | |
tree | 8715af6dbca849b76dd3774ca531d7f81f698b72 | |
parent | 183ecff5668f46073548a7d10291b41d35052b12 (diff) | |
download | edk2-b0bc24aff5205e71aebbb7a15872974bb131d0f3.tar.gz edk2-b0bc24aff5205e71aebbb7a15872974bb131d0f3.tar.bz2 edk2-b0bc24aff5205e71aebbb7a15872974bb131d0f3.zip |
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 <alex.williamson@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@15567 6f19259b-4bc3-4df7-8a09-765794883524
-rw-r--r-- | MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c | 34 |
1 files changed, 9 insertions, 25 deletions
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c index 8c31831ad9..26c9a33854 100644 --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c @@ -1,7 +1,7 @@ /** @file
PCI emumeration support functions implementation for PCI Bus module.
-Copyright (c) 2006 - 2012, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved.<BR>
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
@@ -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;
|