* [PATCH 1/3] MdeModulePkg/PciBus: Refine EFI_PCI_ROM_IMAGE_MAPPING
2017-10-27 6:54 [PATCH 0/3] MdeModulePkg/PciBus: Fix bug that doesn't produce BusOverride Ruiyu Ni
@ 2017-10-27 6:54 ` Ruiyu Ni
2017-10-27 6:54 ` [PATCH 2/3] MdeModulePkg/PciBus: Don't create entry when recording ImageHandle Ruiyu Ni
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Ruiyu Ni @ 2017-10-27 6:54 UTC (permalink / raw)
To: edk2-devel; +Cc: Star Zeng
The patch doesn't impact real functionality.
It only renames EFI_PCI_ROM_IMAGE_MAPPING to PCI_ROM_IMAGE,
and changes prototype of PciRomAddImageMapping so that
no explicit type cast is needed when calling this function.
It also removes unused field RomBase from PCI_IO_DEVICE structure.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
---
MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h | 7 +--
MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c | 4 +-
.../Bus/Pci/PciBusDxe/PciOptionRomSupport.c | 4 +-
MdeModulePkg/Bus/Pci/PciBusDxe/PciRomTable.c | 55 ++++++++++------------
MdeModulePkg/Bus/Pci/PciBusDxe/PciRomTable.h | 10 ++--
5 files changed, 35 insertions(+), 45 deletions(-)
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
index 3bcc134d0b..55eb3a5a80 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
@@ -1,7 +1,7 @@
/** @file
Header files and data structures needed by PCI Bus module.
-Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2017, 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
@@ -240,11 +240,6 @@ struct _PCI_IO_DEVICE {
UINT64 RomSize;
//
- // The OptionRom Size
- //
- UINT64 RomBase;
-
- //
// TRUE if all OpROM (in device or in platform specific position) have been processed
//
BOOLEAN AllOpRomProcessed;
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c
index 359b9ded6d..97bb971a59 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c
@@ -282,7 +282,7 @@ RegisterPciDevice (
PciIoDevice->BusNumber,
PciIoDevice->DeviceNumber,
PciIoDevice->FunctionNumber,
- (UINT64) (UINTN) PciIoDevice->PciIo.RomImage,
+ PciIoDevice->PciIo.RomImage,
PciIoDevice->PciIo.RomSize
);
}
@@ -308,7 +308,7 @@ RegisterPciDevice (
PciIoDevice->BusNumber,
PciIoDevice->DeviceNumber,
PciIoDevice->FunctionNumber,
- (UINT64) (UINTN) PciIoDevice->PciIo.RomImage,
+ PciIoDevice->PciIo.RomImage,
PciIoDevice->PciIo.RomSize
);
}
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c
index 3713c07844..4382d79c2d 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c
@@ -551,7 +551,7 @@ LoadOpRomImage (
PciDevice->BusNumber,
PciDevice->DeviceNumber,
PciDevice->FunctionNumber,
- (UINT64) (UINTN) PciDevice->PciIo.RomImage,
+ PciDevice->PciIo.RomImage,
PciDevice->PciIo.RomSize
);
@@ -766,7 +766,7 @@ ProcessOpRomImage (
PciDevice->BusNumber,
PciDevice->DeviceNumber,
PciDevice->FunctionNumber,
- (UINT64) (UINTN) PciDevice->PciIo.RomImage,
+ PciDevice->PciIo.RomImage,
PciDevice->PciIo.RomSize
);
RetStatus = EFI_SUCCESS;
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciRomTable.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciRomTable.c
index f48c3a0c59..0eef41739c 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciRomTable.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciRomTable.c
@@ -1,7 +1,7 @@
/** @file
Set up ROM Table for PCI Bus module.
-Copyright (c) 2006 - 2009, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2017, 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
@@ -23,13 +23,13 @@ typedef struct {
UINT8 Bus;
UINT8 Dev;
UINT8 Func;
- UINT64 RomAddress;
- UINT64 RomLength;
-} EFI_PCI_ROM_IMAGE_MAPPING;
+ VOID *RomImage;
+ UINT64 RomSize;
+} PCI_ROM_IMAGE;
-UINTN mNumberOfPciRomImages = 0;
-UINTN mMaxNumberOfPciRomImages = 0;
-EFI_PCI_ROM_IMAGE_MAPPING *mRomImageTable = NULL;
+UINTN mNumberOfPciRomImages = 0;
+UINTN mMaxNumberOfPciRomImages = 0;
+PCI_ROM_IMAGE *mRomImageTable = NULL;
/**
Add the Rom Image to internal database for later PCI light enumeration.
@@ -39,9 +39,8 @@ EFI_PCI_ROM_IMAGE_MAPPING *mRomImageTable = NULL;
@param Bus Bus NO of PCI space.
@param Dev Dev NO of PCI space.
@param Func Func NO of PCI space.
- @param RomAddress Base address of OptionRom.
- @param RomLength Length of rom image.
-
+ @param RomImage Option Rom buffer.
+ @param RomSize Size of Option Rom buffer.
**/
VOID
PciRomAddImageMapping (
@@ -50,29 +49,25 @@ PciRomAddImageMapping (
IN UINT8 Bus,
IN UINT8 Dev,
IN UINT8 Func,
- IN UINT64 RomAddress,
- IN UINT64 RomLength
+ IN VOID *RomImage,
+ IN UINT64 RomSize
)
{
- EFI_PCI_ROM_IMAGE_MAPPING *TempMapping;
-
- if (mNumberOfPciRomImages >= mMaxNumberOfPciRomImages) {
+ PCI_ROM_IMAGE *NewTable;
- mMaxNumberOfPciRomImages += 0x20;
+ if (mNumberOfPciRomImages == mMaxNumberOfPciRomImages) {
- TempMapping = NULL;
- TempMapping = AllocatePool (mMaxNumberOfPciRomImages * sizeof (EFI_PCI_ROM_IMAGE_MAPPING));
- if (TempMapping == NULL) {
+ NewTable = ReallocatePool (
+ mMaxNumberOfPciRomImages * sizeof (PCI_ROM_IMAGE),
+ (mMaxNumberOfPciRomImages + 0x20) * sizeof (PCI_ROM_IMAGE),
+ mRomImageTable
+ );
+ if (NewTable == NULL) {
return ;
}
- CopyMem (TempMapping, mRomImageTable, mNumberOfPciRomImages * sizeof (EFI_PCI_ROM_IMAGE_MAPPING));
-
- if (mRomImageTable != NULL) {
- FreePool (mRomImageTable);
- }
-
- mRomImageTable = TempMapping;
+ mRomImageTable = NewTable;
+ mMaxNumberOfPciRomImages += 0x20;
}
mRomImageTable[mNumberOfPciRomImages].ImageHandle = ImageHandle;
@@ -80,8 +75,8 @@ PciRomAddImageMapping (
mRomImageTable[mNumberOfPciRomImages].Bus = Bus;
mRomImageTable[mNumberOfPciRomImages].Dev = Dev;
mRomImageTable[mNumberOfPciRomImages].Func = Func;
- mRomImageTable[mNumberOfPciRomImages].RomAddress = RomAddress;
- mRomImageTable[mNumberOfPciRomImages].RomLength = RomLength;
+ mRomImageTable[mNumberOfPciRomImages].RomImage = RomImage;
+ mRomImageTable[mNumberOfPciRomImages].RomSize = RomSize;
mNumberOfPciRomImages++;
}
@@ -116,8 +111,8 @@ PciRomGetImageMapping (
if (mRomImageTable[Index].ImageHandle != NULL) {
AddDriver (PciIoDevice, mRomImageTable[Index].ImageHandle);
} else {
- PciIoDevice->PciIo.RomImage = (VOID *) (UINTN) mRomImageTable[Index].RomAddress;
- PciIoDevice->PciIo.RomSize = (UINTN) mRomImageTable[Index].RomLength;
+ PciIoDevice->PciIo.RomImage = mRomImageTable[Index].RomImage;
+ PciIoDevice->PciIo.RomSize = mRomImageTable[Index].RomSize;
}
}
}
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciRomTable.h b/MdeModulePkg/Bus/Pci/PciBusDxe/PciRomTable.h
index d443f83336..a7a1ed4ce3 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciRomTable.h
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciRomTable.h
@@ -1,7 +1,7 @@
/** @file
Set up ROM Table for PCI Bus module.
-Copyright (c) 2006 - 2009, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2017, 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
@@ -23,8 +23,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
@param Bus Bus NO of PCI space.
@param Dev Dev NO of PCI space.
@param Func Func NO of PCI space.
- @param RomAddress Base address of OptionRom.
- @param RomLength Length of rom image.
+ @param RomImage Option ROM buffer.
+ @param RomSize Size of Option ROM buffer.
**/
VOID
@@ -34,8 +34,8 @@ PciRomAddImageMapping (
IN UINT8 Bus,
IN UINT8 Dev,
IN UINT8 Func,
- IN UINT64 RomAddress,
- IN UINT64 RomLength
+ IN VOID *RomImage,
+ IN UINT64 RomSize
);
/**
--
2.12.2.windows.2
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 2/3] MdeModulePkg/PciBus: Don't create entry when recording ImageHandle
2017-10-27 6:54 [PATCH 0/3] MdeModulePkg/PciBus: Fix bug that doesn't produce BusOverride Ruiyu Ni
2017-10-27 6:54 ` [PATCH 1/3] MdeModulePkg/PciBus: Refine EFI_PCI_ROM_IMAGE_MAPPING Ruiyu Ni
@ 2017-10-27 6:54 ` Ruiyu Ni
2017-10-27 6:54 ` [PATCH 3/3] MdeModulePkg/PciBus: Fix bug that doesn't produce BusOverride Ruiyu Ni
2017-10-30 8:42 ` [PATCH 0/3] " Zeng, Star
3 siblings, 0 replies; 5+ messages in thread
From: Ruiyu Ni @ 2017-10-27 6:54 UTC (permalink / raw)
To: edk2-devel; +Cc: Star Zeng
The patch shouldn't impact functionality.
Original code records the ImageHandle of Option ROM by creating a
new entry. It's not necessary.
The patch updates the ImageHandle in the old entry.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
---
MdeModulePkg/Bus/Pci/PciBusDxe/PciRomTable.c | 72 ++++++++++++++++++----------
1 file changed, 46 insertions(+), 26 deletions(-)
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciRomTable.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciRomTable.c
index 0eef41739c..fc6f579293 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciRomTable.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciRomTable.c
@@ -53,31 +53,54 @@ PciRomAddImageMapping (
IN UINT64 RomSize
)
{
+ UINTN Index;
PCI_ROM_IMAGE *NewTable;
- if (mNumberOfPciRomImages == mMaxNumberOfPciRomImages) {
-
- NewTable = ReallocatePool (
- mMaxNumberOfPciRomImages * sizeof (PCI_ROM_IMAGE),
- (mMaxNumberOfPciRomImages + 0x20) * sizeof (PCI_ROM_IMAGE),
- mRomImageTable
- );
- if (NewTable == NULL) {
- return ;
+ for (Index = 0; Index < mNumberOfPciRomImages; Index++) {
+ if (mRomImageTable[Index].Seg == Seg &&
+ mRomImageTable[Index].Bus == Bus &&
+ mRomImageTable[Index].Dev == Dev &&
+ mRomImageTable[Index].Func == Func) {
+ //
+ // Expect once RomImage and RomSize are recorded, they will be passed in
+ // later when updating ImageHandle
+ //
+ ASSERT ((mRomImageTable[Index].RomImage == NULL) || (RomImage == mRomImageTable[Index].RomImage));
+ ASSERT ((mRomImageTable[Index].RomSize == 0 ) || (RomSize == mRomImageTable[Index].RomSize ));
+ break;
}
+ }
- mRomImageTable = NewTable;
- mMaxNumberOfPciRomImages += 0x20;
+ if (Index == mNumberOfPciRomImages) {
+ //
+ // Rom Image Table buffer needs to grow.
+ //
+ if (mNumberOfPciRomImages == mMaxNumberOfPciRomImages) {
+ NewTable = ReallocatePool (
+ mMaxNumberOfPciRomImages * sizeof (PCI_ROM_IMAGE),
+ (mMaxNumberOfPciRomImages + 0x20) * sizeof (PCI_ROM_IMAGE),
+ mRomImageTable
+ );
+ if (NewTable == NULL) {
+ return ;
+ }
+
+ mRomImageTable = NewTable;
+ mMaxNumberOfPciRomImages += 0x20;
+ }
+ //
+ // Record the new PCI device
+ //
+ mRomImageTable[Index].Seg = Seg;
+ mRomImageTable[Index].Bus = Bus;
+ mRomImageTable[Index].Dev = Dev;
+ mRomImageTable[Index].Func = Func;
+ mNumberOfPciRomImages++;
}
- mRomImageTable[mNumberOfPciRomImages].ImageHandle = ImageHandle;
- mRomImageTable[mNumberOfPciRomImages].Seg = Seg;
- mRomImageTable[mNumberOfPciRomImages].Bus = Bus;
- mRomImageTable[mNumberOfPciRomImages].Dev = Dev;
- mRomImageTable[mNumberOfPciRomImages].Func = Func;
- mRomImageTable[mNumberOfPciRomImages].RomImage = RomImage;
- mRomImageTable[mNumberOfPciRomImages].RomSize = RomSize;
- mNumberOfPciRomImages++;
+ mRomImageTable[Index].ImageHandle = ImageHandle;
+ mRomImageTable[Index].RomImage = RomImage;
+ mRomImageTable[Index].RomSize = RomSize;
}
/**
@@ -96,26 +119,23 @@ PciRomGetImageMapping (
{
EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL *PciRootBridgeIo;
UINTN Index;
- BOOLEAN Found;
PciRootBridgeIo = PciIoDevice->PciRootBridgeIo;
- Found = FALSE;
for (Index = 0; Index < mNumberOfPciRomImages; Index++) {
if (mRomImageTable[Index].Seg == PciRootBridgeIo->SegmentNumber &&
mRomImageTable[Index].Bus == PciIoDevice->BusNumber &&
mRomImageTable[Index].Dev == PciIoDevice->DeviceNumber &&
mRomImageTable[Index].Func == PciIoDevice->FunctionNumber ) {
- Found = TRUE;
if (mRomImageTable[Index].ImageHandle != NULL) {
AddDriver (PciIoDevice, mRomImageTable[Index].ImageHandle);
- } else {
- PciIoDevice->PciIo.RomImage = mRomImageTable[Index].RomImage;
- PciIoDevice->PciIo.RomSize = mRomImageTable[Index].RomSize;
}
+ PciIoDevice->PciIo.RomImage = mRomImageTable[Index].RomImage;
+ PciIoDevice->PciIo.RomSize = mRomImageTable[Index].RomSize;
+ return TRUE;
}
}
- return Found;
+ return FALSE;
}
--
2.12.2.windows.2
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 3/3] MdeModulePkg/PciBus: Fix bug that doesn't produce BusOverride
2017-10-27 6:54 [PATCH 0/3] MdeModulePkg/PciBus: Fix bug that doesn't produce BusOverride Ruiyu Ni
2017-10-27 6:54 ` [PATCH 1/3] MdeModulePkg/PciBus: Refine EFI_PCI_ROM_IMAGE_MAPPING Ruiyu Ni
2017-10-27 6:54 ` [PATCH 2/3] MdeModulePkg/PciBus: Don't create entry when recording ImageHandle Ruiyu Ni
@ 2017-10-27 6:54 ` Ruiyu Ni
2017-10-30 8:42 ` [PATCH 0/3] " Zeng, Star
3 siblings, 0 replies; 5+ messages in thread
From: Ruiyu Ni @ 2017-10-27 6:54 UTC (permalink / raw)
To: edk2-devel
It's a regression of below commit:
SHA-1: 8be37a5cee700777ca8e8e8a34cc2225b21931a7
* MdeModulePkg/SecurityStubDxe: Defer 3rd party image before EndOfDxe
When PciBus driver fails to load the Option ROM, it doesn't produce
BusOverride protocol. It was a correct behavior before the above
commit. But due to the above commit, BusOverride protocol never is
produced by PciBus driver.
The patch fixes this issue using the following solution:
1. PciBus records the image device path when LoadImage fails.
2. Override.GetDriver() tries to look for the image handle using
the stored image device path.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
---
MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf | 3 +-
MdeModulePkg/Bus/Pci/PciBusDxe/PciDriverOverride.c | 147 ++++++++++++++-------
MdeModulePkg/Bus/Pci/PciBusDxe/PciDriverOverride.h | 17 ++-
.../Bus/Pci/PciBusDxe/PciOptionRomSupport.c | 17 ++-
MdeModulePkg/Bus/Pci/PciBusDxe/PciRomTable.c | 2 +-
5 files changed, 124 insertions(+), 62 deletions(-)
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
index 5da094f582..97608bfcf2 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
@@ -2,7 +2,7 @@
# The PCI bus driver will probe all PCI devices and allocate MMIO and IO space for these devices.
# Please use PCD feature flag PcdPciBusHotplugDeviceSupport to enable hot plug supporting.
#
-# Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2006 - 2017, 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
@@ -96,6 +96,7 @@ [Protocols]
gEfiIncompatiblePciDeviceSupportProtocolGuid ## SOMETIMES_CONSUMES
gEfiLoadFile2ProtocolGuid ## SOMETIMES_PRODUCES
gEdkiiIoMmuProtocolGuid ## SOMETIMES_CONSUMES
+ gEfiLoadedImageDevicePathProtocolGuid ## CONSUMES
[FeaturePcd]
gEfiMdeModulePkgTokenSpaceGuid.PcdPciBusHotplugDeviceSupport ## CONSUMES
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciDriverOverride.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciDriverOverride.c
index 97f45e42d0..e8fae17e8b 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciDriverOverride.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciDriverOverride.c
@@ -1,7 +1,7 @@
/** @file
Functions implementation for Bus Specific Driver Override protoocl.
-Copyright (c) 2006 - 2009, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2017, 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
@@ -28,6 +28,56 @@ InitializePciDriverOverrideInstance (
PciIoDevice->PciDriverOverride.GetDriver = GetDriver;
}
+/**
+ Find the image handle whose path equals to ImagePath.
+
+ @param ImagePath Image path.
+
+ @return Image handle.
+**/
+EFI_HANDLE
+LocateImageHandle (
+ IN EFI_DEVICE_PATH_PROTOCOL *ImagePath
+ )
+{
+ EFI_STATUS Status;
+ EFI_HANDLE *Handles;
+ UINTN Index;
+ UINTN HandleNum;
+ EFI_DEVICE_PATH_PROTOCOL *DevicePath;
+ UINTN ImagePathSize;
+ EFI_HANDLE ImageHandle;
+
+ Status = gBS->LocateHandleBuffer (
+ ByProtocol,
+ &gEfiLoadedImageDevicePathProtocolGuid,
+ NULL,
+ &HandleNum,
+ &Handles
+ );
+ if (EFI_ERROR (Status)) {
+ return NULL;
+ }
+
+ ImageHandle = NULL;
+ ImagePathSize = GetDevicePathSize (ImagePath);
+
+ for (Index = 0; Index < HandleNum; Index++) {
+ Status = gBS->HandleProtocol (Handles[Index], &gEfiLoadedImageDevicePathProtocolGuid, (VOID **) &DevicePath);
+ if (EFI_ERROR (Status)) {
+ continue;
+ }
+ if ((ImagePathSize == GetDevicePathSize (DevicePath)) &&
+ (CompareMem (ImagePath, DevicePath, ImagePathSize) == 0)
+ ) {
+ ImageHandle = Handles[Index];
+ break;
+ }
+ }
+
+ FreePool (Handles);
+ return ImageHandle;
+}
/**
Uses a bus specific algorithm to retrieve a driver image handle for a controller.
@@ -53,49 +103,60 @@ GetDriver (
)
{
PCI_IO_DEVICE *PciIoDevice;
- LIST_ENTRY *CurrentLink;
- PCI_DRIVER_OVERRIDE_LIST *Node;
+ LIST_ENTRY *Link;
+ PCI_DRIVER_OVERRIDE_LIST *Override;
+ BOOLEAN ReturnNext;
+ Override = NULL;
PciIoDevice = PCI_IO_DEVICE_FROM_PCI_DRIVER_OVERRIDE_THIS (This);
+ ReturnNext = (BOOLEAN) (*DriverImageHandle == NULL);
+ for ( Link = GetFirstNode (&PciIoDevice->OptionRomDriverList)
+ ; !IsNull (&PciIoDevice->OptionRomDriverList, Link)
+ ; Link = GetNextNode (&PciIoDevice->OptionRomDriverList, Link)
+ ) {
- CurrentLink = PciIoDevice->OptionRomDriverList.ForwardLink;
-
- while (CurrentLink != NULL && CurrentLink != &PciIoDevice->OptionRomDriverList) {
-
- Node = DRIVER_OVERRIDE_FROM_LINK (CurrentLink);
-
- if (*DriverImageHandle == NULL) {
+ Override = DRIVER_OVERRIDE_FROM_LINK (Link);
- *DriverImageHandle = Node->DriverImageHandle;
- return EFI_SUCCESS;
- }
-
- if (*DriverImageHandle == Node->DriverImageHandle) {
-
- if (CurrentLink->ForwardLink == &PciIoDevice->OptionRomDriverList ||
- CurrentLink->ForwardLink == NULL) {
- return EFI_NOT_FOUND;
+ if (ReturnNext) {
+ if (Override->DriverImageHandle == NULL) {
+ Override->DriverImageHandle = LocateImageHandle (Override->DriverImagePath);
}
- //
- // Get next node
- //
- Node = DRIVER_OVERRIDE_FROM_LINK (CurrentLink->ForwardLink);
- *DriverImageHandle = Node->DriverImageHandle;
- return EFI_SUCCESS;
+ if (Override->DriverImageHandle == NULL) {
+ //
+ // The Option ROM identified by Override->DriverImagePath is not loaded.
+ //
+ continue;
+ } else {
+ *DriverImageHandle = Override->DriverImageHandle;
+ return EFI_SUCCESS;
+ }
}
- CurrentLink = CurrentLink->ForwardLink;
+ if (*DriverImageHandle == Override->DriverImageHandle) {
+ ReturnNext = TRUE;
+ }
}
- return EFI_INVALID_PARAMETER;
+ ASSERT (IsNull (&PciIoDevice->OptionRomDriverList, Link));
+ //
+ // ReturnNext indicates a handle match happens.
+ // If all nodes are checked without handle match happening,
+ // the DriverImageHandle should be a invalid handle.
+ //
+ if (ReturnNext) {
+ return EFI_NOT_FOUND;
+ } else {
+ return EFI_INVALID_PARAMETER;
+ }
}
/**
Add an overriding driver image.
@param PciIoDevice Instance of PciIo device.
- @param DriverImageHandle new added driver image.
+ @param DriverImageHandle Image handle of newly added driver image.
+ @param DriverImagePath Device path of newly added driver image.
@retval EFI_SUCCESS Successfully added driver.
@retval EFI_OUT_OF_RESOURCES No memory resource for new driver instance.
@@ -104,40 +165,30 @@ GetDriver (
**/
EFI_STATUS
AddDriver (
- IN PCI_IO_DEVICE *PciIoDevice,
- IN EFI_HANDLE DriverImageHandle
+ IN PCI_IO_DEVICE *PciIoDevice,
+ IN EFI_HANDLE DriverImageHandle,
+ IN EFI_DEVICE_PATH_PROTOCOL *DriverImagePath
)
{
- EFI_STATUS Status;
- EFI_LOADED_IMAGE_PROTOCOL *LoadedImage;
- PE_COFF_LOADER_IMAGE_CONTEXT ImageContext;
PCI_DRIVER_OVERRIDE_LIST *Node;
- Status = gBS->HandleProtocol (DriverImageHandle, &gEfiLoadedImageProtocolGuid, (VOID **) &LoadedImage);
- if (EFI_ERROR (Status)) {
- return Status;
- }
+ //
+ // Caller should pass in either Image Handle or Image Path, but not both.
+ //
+ ASSERT ((DriverImageHandle == NULL) || (DriverImagePath == NULL));
- Node = AllocatePool (sizeof (PCI_DRIVER_OVERRIDE_LIST));
+ Node = AllocateZeroPool (sizeof (PCI_DRIVER_OVERRIDE_LIST));
if (Node == NULL) {
return EFI_OUT_OF_RESOURCES;
}
Node->Signature = DRIVER_OVERRIDE_SIGNATURE;
Node->DriverImageHandle = DriverImageHandle;
+ Node->DriverImagePath = DuplicateDevicePath (DriverImagePath);
- InsertTailList (&PciIoDevice->OptionRomDriverList, &(Node->Link));
+ InsertTailList (&PciIoDevice->OptionRomDriverList, &Node->Link);
PciIoDevice->BusOverride = TRUE;
-
- ImageContext.Handle = LoadedImage->ImageBase;
- ImageContext.ImageRead = PeCoffLoaderImageReadFromMemory;
-
- //
- // Get information about the image
- //
- PeCoffLoaderGetImageInfo (&ImageContext);
-
return EFI_SUCCESS;
}
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciDriverOverride.h b/MdeModulePkg/Bus/Pci/PciBusDxe/PciDriverOverride.h
index bf8efff8f1..f0679c51ec 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciDriverOverride.h
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciDriverOverride.h
@@ -1,7 +1,7 @@
/** @file
Functions declaration for Bus Specific Driver Override protoocl.
-Copyright (c) 2006 - 2009, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2017, 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
@@ -22,9 +22,10 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
// PCI driver override driver image list
//
typedef struct {
- UINT32 Signature;
- LIST_ENTRY Link;
- EFI_HANDLE DriverImageHandle;
+ UINT32 Signature;
+ LIST_ENTRY Link;
+ EFI_HANDLE DriverImageHandle;
+ EFI_DEVICE_PATH_PROTOCOL *DriverImagePath;
} PCI_DRIVER_OVERRIDE_LIST;
@@ -46,7 +47,8 @@ InitializePciDriverOverrideInstance (
Add an overriding driver image.
@param PciIoDevice Instance of PciIo device.
- @param DriverImageHandle new added driver image.
+ @param DriverImageHandle Image handle of newly added driver image.
+ @param DriverImagePath Device path of newly added driver image.
@retval EFI_SUCCESS Successfully added driver.
@retval EFI_OUT_OF_RESOURCES No memory resource for new driver instance.
@@ -55,8 +57,9 @@ InitializePciDriverOverrideInstance (
**/
EFI_STATUS
AddDriver (
- IN PCI_IO_DEVICE *PciIoDevice,
- IN EFI_HANDLE DriverImageHandle
+ IN PCI_IO_DEVICE *PciIoDevice,
+ IN EFI_HANDLE DriverImageHandle,
+ IN EFI_DEVICE_PATH_PROTOCOL *DriverImagePath
);
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c
index 4382d79c2d..d390bb655a 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c
@@ -753,13 +753,19 @@ ProcessOpRomImage (
BufferSize,
&ImageHandle
);
-
- FreePool (PciOptionRomImageDevicePath);
-
- if (!EFI_ERROR (Status)) {
+ if (EFI_ERROR (Status)) {
+ //
+ // Record the Option ROM Image device path when LoadImage fails.
+ // PciOverride.GetDriver() will try to look for the Image Handle using the device path later.
+ //
+ AddDriver (PciDevice, NULL, PciOptionRomImageDevicePath);
+ } else {
Status = gBS->StartImage (ImageHandle, NULL, NULL);
if (!EFI_ERROR (Status)) {
- AddDriver (PciDevice, ImageHandle);
+ //
+ // Record the Option ROM Image Handle
+ //
+ AddDriver (PciDevice, ImageHandle, NULL);
PciRomAddImageMapping (
ImageHandle,
PciDevice->PciRootBridgeIo->SegmentNumber,
@@ -772,6 +778,7 @@ ProcessOpRomImage (
RetStatus = EFI_SUCCESS;
}
}
+ FreePool (PciOptionRomImageDevicePath);
NextImage:
RomBarOffset += ImageSize;
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciRomTable.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciRomTable.c
index fc6f579293..aa7cfec232 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciRomTable.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciRomTable.c
@@ -129,7 +129,7 @@ PciRomGetImageMapping (
mRomImageTable[Index].Func == PciIoDevice->FunctionNumber ) {
if (mRomImageTable[Index].ImageHandle != NULL) {
- AddDriver (PciIoDevice, mRomImageTable[Index].ImageHandle);
+ AddDriver (PciIoDevice, mRomImageTable[Index].ImageHandle, NULL);
}
PciIoDevice->PciIo.RomImage = mRomImageTable[Index].RomImage;
PciIoDevice->PciIo.RomSize = mRomImageTable[Index].RomSize;
--
2.12.2.windows.2
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH 0/3] MdeModulePkg/PciBus: Fix bug that doesn't produce BusOverride
2017-10-27 6:54 [PATCH 0/3] MdeModulePkg/PciBus: Fix bug that doesn't produce BusOverride Ruiyu Ni
` (2 preceding siblings ...)
2017-10-27 6:54 ` [PATCH 3/3] MdeModulePkg/PciBus: Fix bug that doesn't produce BusOverride Ruiyu Ni
@ 2017-10-30 8:42 ` Zeng, Star
3 siblings, 0 replies; 5+ messages in thread
From: Zeng, Star @ 2017-10-30 8:42 UTC (permalink / raw)
To: Ni, Ruiyu, edk2-devel@lists.01.org; +Cc: Zeng, Star
Reviewed-by: Star Zeng <star.zeng@intel.com>
-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ruiyu Ni
Sent: Friday, October 27, 2017 2:55 PM
To: edk2-devel@lists.01.org
Subject: [edk2] [PATCH 0/3] MdeModulePkg/PciBus: Fix bug that doesn't produce BusOverride
It's a regression of below commit:
SHA-1: 8be37a5cee700777ca8e8e8a34cc2225b21931a7
* MdeModulePkg/SecurityStubDxe: Defer 3rd party image before EndOfDxe
When PciBus driver fails to load the Option ROM, it doesn't produce BusOverride protocol. It was a correct behavior before the above commit. But due to the above commit, BusOverride protocol never is produced by PciBus driver.
The patch fixes this issue using the following solution:
1. PciBus records the image device path when LoadImage fails.
2. Override.GetDriver() tries to look for the image handle using
the stored image device path.
Ruiyu Ni (3):
MdeModulePkg/PciBus: Refine EFI_PCI_ROM_IMAGE_MAPPING
MdeModulePkg/PciBus: Don't create entry when recording ImageHandle
MdeModulePkg/PciBus: Fix bug that doesn't produce BusOverride
MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h | 7 +-
MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf | 3 +-
MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c | 4 +-
MdeModulePkg/Bus/Pci/PciBusDxe/PciDriverOverride.c | 147 ++++++++++++++------- MdeModulePkg/Bus/Pci/PciBusDxe/PciDriverOverride.h | 17 ++-
.../Bus/Pci/PciBusDxe/PciOptionRomSupport.c | 21 ++-
MdeModulePkg/Bus/Pci/PciBusDxe/PciRomTable.c | 99 ++++++++------
MdeModulePkg/Bus/Pci/PciBusDxe/PciRomTable.h | 10 +-
8 files changed, 190 insertions(+), 118 deletions(-)
--
2.12.2.windows.2
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 5+ messages in thread