From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.88; helo=mga01.intel.com; envelope-from=ruiyu.ni@intel.com; receiver=edk2-devel@lists.01.org Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id ED98821CEB149 for ; Thu, 26 Oct 2017 23:50:58 -0700 (PDT) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 26 Oct 2017 23:54:45 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.44,303,1505804400"; d="scan'208";a="1030015311" Received: from ray-dev.ccr.corp.intel.com ([10.239.9.7]) by orsmga003.jf.intel.com with ESMTP; 26 Oct 2017 23:54:44 -0700 From: Ruiyu Ni To: edk2-devel@lists.01.org Date: Fri, 27 Oct 2017 14:54:38 +0800 Message-Id: <20171027065438.284440-4-ruiyu.ni@intel.com> X-Mailer: git-send-email 2.12.2.windows.2 In-Reply-To: <20171027065438.284440-1-ruiyu.ni@intel.com> References: <20171027065438.284440-1-ruiyu.ni@intel.com> Subject: [PATCH 3/3] MdeModulePkg/PciBus: Fix bug that doesn't produce BusOverride X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 27 Oct 2017 06:50:59 -0000 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 Reviewed-by: Star Zeng --- 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.
+# Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.
# # 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.
+Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.
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.
+Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.
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