From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) (using TLSv1 with cipher CAMELLIA256-SHA (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 451471A1E0A for ; Mon, 12 Sep 2016 06:15:39 -0700 (PDT) Received: from orsmga001.jf.intel.com ([10.7.209.18]) by fmsmga103.fm.intel.com with ESMTP; 12 Sep 2016 06:15:40 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.30,322,1470726000"; d="scan'208";a="1028737606" Received: from fmsmsx107.amr.corp.intel.com ([10.18.124.205]) by orsmga001.jf.intel.com with ESMTP; 12 Sep 2016 06:15:38 -0700 Received: from fmsmsx158.amr.corp.intel.com (10.18.116.75) by fmsmsx107.amr.corp.intel.com (10.18.124.205) with Microsoft SMTP Server (TLS) id 14.3.248.2; Mon, 12 Sep 2016 06:15:38 -0700 Received: from shsmsx104.ccr.corp.intel.com (10.239.4.70) by fmsmsx158.amr.corp.intel.com (10.18.116.75) with Microsoft SMTP Server (TLS) id 14.3.248.2; Mon, 12 Sep 2016 06:15:38 -0700 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.109]) by SHSMSX104.ccr.corp.intel.com ([169.254.5.174]) with mapi id 14.03.0248.002; Mon, 12 Sep 2016 21:15:35 +0800 From: "Yao, Jiewen" To: Ard Biesheuvel , "edk2-devel@lists.01.org" , "Gao, Liming" , "Zeng, Star" , "Tian, Feng" , "Ni, Ruiyu" CC: "lersek@redhat.com" , "leif.lindholm@linaro.org" Thread-Topic: [edk2] [PATCH] MdeModulePkg/PciBusDxe: make BAR degradation dependent on OPROM presence Thread-Index: AQHSDPZ/y6jTdnKmYEGKXsvlAZYRSaB11R8w Date: Mon, 12 Sep 2016 13:15:35 +0000 Message-ID: <74D8A39837DF1E4DA445A8C0B3885C50385FCEB6@shsmsx102.ccr.corp.intel.com> References: <1473685561-1418-1-git-send-email-ard.biesheuvel@linaro.org> In-Reply-To: <1473685561-1418-1-git-send-email-ard.biesheuvel@linaro.org> Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH] MdeModulePkg/PciBusDxe: make BAR degradation dependent on OPROM presence X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 12 Sep 2016 13:15:39 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable HI Ard We should not let MdeModulePkg depend on IntelFrameworkPkg. You patch violates the dependency rule. I suggest we figure out other solution to resolve the problem. Thank you Yao Jiewen > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of > Ard Biesheuvel > Sent: Monday, September 12, 2016 9:06 PM > To: edk2-devel@lists.01.org; Gao, Liming ; Zeng, > Star ; Tian, Feng ; Ni, Ruiyu > > Cc: lersek@redhat.com; leif.lindholm@linaro.org; Ard Biesheuvel > > Subject: [edk2] [PATCH] MdeModulePkg/PciBusDxe: make BAR degradation > dependent on OPROM presence >=20 > The practice of unconditionally degrading 64-bit PCI MMIO BARs to 32-bit > if the device in question happens to have an option ROM is based on > platform constraints, not architectural constraints, and really only make= s > sense on Intel platforms that contain a CSM implementation. >=20 > So let's copy the OVMF code that checks for the presence of the legacy > BIOS protocol (&gEfiLegacyBiosProtocolGuid), and only perform the BAR > degradation if this protocol is installed. >=20 > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ard Biesheuvel > --- > MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c | 42 > ++++++++++ > MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h | 1 + > MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf | 2 + > MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c | 83 > ++++++++++---------- > 4 files changed, 88 insertions(+), 40 deletions(-) >=20 > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c > index a463bea80f3d..857f3e11b6bd 100644 > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c > @@ -49,6 +49,39 @@ GLOBAL_REMOVE_IF_UNREFERENCED > EFI_PCI_HOTPLUG_REQUEST_PROTOCOL mPciHotPlugReques > }; >=20 > /** > + Legacy BIOS installed callback > + > + @param[in] Event Event whose notification function is being > invoked. > + @param[in] Context Pointer to the notification function's context. > + > +**/ > +STATIC > +VOID > +EFIAPI > +LegacyBiosInstalledCallBack ( > + IN EFI_EVENT Event, > + IN VOID *Context > + ) > +{ > + EFI_STATUS Status; > + EFI_LEGACY_BIOS_PROTOCOL *LegacyBios; > + > + Status =3D gBS->LocateProtocol (&gEfiLegacyBiosProtocolGuid, > + NULL /* Registration */, (VOID **)&LegacyBios); > + if (EFI_ERROR (Status)) { > + return; > + } > + > + mLegacyBiosInstalled =3D TRUE; > + > + // > + // Close the event and deregister this callback. > + // > + Status =3D gBS->CloseEvent (Event); > + ASSERT_EFI_ERROR (Status); > +} > + > +/** > The Entry Point for PCI Bus module. The user code starts with this > function. >=20 > Installs driver module protocols and. Creates virtual device handles f= or > ConIn, > @@ -72,6 +105,7 @@ PciBusEntryPoint ( > { > EFI_STATUS Status; > EFI_HANDLE Handle; > + VOID *Registration; >=20 > // > // Initializes PCI devices pool > @@ -91,6 +125,14 @@ PciBusEntryPoint ( > ); > ASSERT_EFI_ERROR (Status); >=20 > + EfiCreateProtocolNotifyEvent ( > + &gEfiLegacyBiosProtocolGuid, > + TPL_CALLBACK, > + LegacyBiosInstalledCallBack, > + NULL, > + &Registration > + ); > + > if (FeaturePcdGet (PcdPciBusHotplugDeviceSupport)) { > // > // If Hot Plug is supported, install EFI PCI Hot Plug Request protoc= ol. > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h > index b12d7ec5032f..2bf5695476a1 100644 > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h > @@ -321,6 +321,7 @@ extern EFI_PCI_PLATFORM_PROTOCOL > *gPciPlatformProtocol; > extern EFI_PCI_OVERRIDE_PROTOCOL > *gPciOverrideProtocol; > extern BOOLEAN > mReserveIsaAliases; > extern BOOLEAN > mReserveVgaAliases; > +extern BOOLEAN > mLegacyBiosInstalled; >=20 > /** > Macro that checks whether device is a GFX device. > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf > index 330ccc8cbffc..b843ccc49934 100644 > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf > @@ -66,6 +66,7 @@ [Sources] > [Packages] > MdePkg/MdePkg.dec > MdeModulePkg/MdeModulePkg.dec > + IntelFrameworkPkg/IntelFrameworkPkg.dec >=20 > [LibraryClasses] > PcdLib > @@ -95,6 +96,7 @@ [Protocols] > gEfiPciRootBridgeIoProtocolGuid ## TO_START > gEfiIncompatiblePciDeviceSupportProtocolGuid ## > SOMETIMES_CONSUMES > gEfiLoadFile2ProtocolGuid ## > SOMETIMES_PRODUCES > + gEfiLegacyBiosProtocolGuid ## > SOMETIMES_CONSUMES >=20 > [FeaturePcd] > gEfiMdeModulePkgTokenSpaceGuid.PcdPciBusHotplugDeviceSupport > ## CONSUMES > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c > index b0632d53b82b..6637625b210d 100644 > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c > @@ -17,9 +17,10 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF > ANY KIND, EITHER EXPRESS OR IMPLIED. > // > // The default policy for the PCI bus driver is NOT to reserve I/O range= s for > both ISA aliases and VGA aliases. > // > -BOOLEAN mReserveIsaAliases =3D FALSE; > -BOOLEAN mReserveVgaAliases =3D FALSE; > -BOOLEAN mPolicyDetermined =3D FALSE; > +BOOLEAN mReserveIsaAliases =3D FALSE; > +BOOLEAN mReserveVgaAliases =3D FALSE; > +BOOLEAN mPolicyDetermined =3D FALSE; > +BOOLEAN mLegacyBiosInstalled =3D FALSE; >=20 > /** > The function is used to skip VGA range. > @@ -1058,48 +1059,50 @@ DegradeResource ( > LIST_ENTRY *NextChildNodeLink; > PCI_RESOURCE_NODE *ResourceNode; >=20 > - // > - // If any child device has both option ROM and 64-bit BAR, degrade its > PMEM64/MEM64 > - // requests in case that if a legacy option ROM image can not access > 64-bit resources. > - // > - ChildDeviceLink =3D Bridge->ChildList.ForwardLink; > - while (ChildDeviceLink !=3D NULL && ChildDeviceLink !=3D &Bridge->Chil= dList) > { > - PciIoDevice =3D PCI_IO_DEVICE_FROM_LINK (ChildDeviceLink); > - if (PciIoDevice->RomSize !=3D 0) { > - if (!IsListEmpty (&Mem64Node->ChildList)) { > - ChildNodeLink =3D Mem64Node->ChildList.ForwardLink; > - while (ChildNodeLink !=3D &Mem64Node->ChildList) { > - ResourceNode =3D RESOURCE_NODE_FROM_LINK > (ChildNodeLink); > - NextChildNodeLink =3D ChildNodeLink->ForwardLink; > - > - if ((ResourceNode->PciDev =3D=3D PciIoDevice) && > - (ResourceNode->Virtual > || !PciIoDevice->PciBar[ResourceNode->Bar].BarTypeFixed) > - ) { > - RemoveEntryList (ChildNodeLink); > - InsertResourceNode (Mem32Node, ResourceNode); > + if (mLegacyBiosInstalled) { > + // > + // If any child device has both option ROM and 64-bit BAR, degrade i= ts > PMEM64/MEM64 > + // requests in case that if a legacy option ROM image can not access > 64-bit resources. > + // > + ChildDeviceLink =3D Bridge->ChildList.ForwardLink; > + while (ChildDeviceLink !=3D NULL && ChildDeviceLink !=3D > &Bridge->ChildList) { > + PciIoDevice =3D PCI_IO_DEVICE_FROM_LINK (ChildDeviceLink); > + if (PciIoDevice->RomSize !=3D 0) { > + if (!IsListEmpty (&Mem64Node->ChildList)) { > + ChildNodeLink =3D Mem64Node->ChildList.ForwardLink; > + while (ChildNodeLink !=3D &Mem64Node->ChildList) { > + ResourceNode =3D RESOURCE_NODE_FROM_LINK > (ChildNodeLink); > + NextChildNodeLink =3D ChildNodeLink->ForwardLink; > + > + if ((ResourceNode->PciDev =3D=3D PciIoDevice) && > + (ResourceNode->Virtual > || !PciIoDevice->PciBar[ResourceNode->Bar].BarTypeFixed) > + ) { > + RemoveEntryList (ChildNodeLink); > + InsertResourceNode (Mem32Node, ResourceNode); > + } > + ChildNodeLink =3D NextChildNodeLink; > } > - ChildNodeLink =3D NextChildNodeLink; > - } > - } > + } >=20 > - if (!IsListEmpty (&PMem64Node->ChildList)) { > - ChildNodeLink =3D PMem64Node->ChildList.ForwardLink; > - while (ChildNodeLink !=3D &PMem64Node->ChildList) { > - ResourceNode =3D RESOURCE_NODE_FROM_LINK > (ChildNodeLink); > - NextChildNodeLink =3D ChildNodeLink->ForwardLink; > - > - if ((ResourceNode->PciDev =3D=3D PciIoDevice) && > - (ResourceNode->Virtual > || !PciIoDevice->PciBar[ResourceNode->Bar].BarTypeFixed) > - ) { > - RemoveEntryList (ChildNodeLink); > - InsertResourceNode (PMem32Node, ResourceNode); > + if (!IsListEmpty (&PMem64Node->ChildList)) { > + ChildNodeLink =3D PMem64Node->ChildList.ForwardLink; > + while (ChildNodeLink !=3D &PMem64Node->ChildList) { > + ResourceNode =3D RESOURCE_NODE_FROM_LINK > (ChildNodeLink); > + NextChildNodeLink =3D ChildNodeLink->ForwardLink; > + > + if ((ResourceNode->PciDev =3D=3D PciIoDevice) && > + (ResourceNode->Virtual > || !PciIoDevice->PciBar[ResourceNode->Bar].BarTypeFixed) > + ) { > + RemoveEntryList (ChildNodeLink); > + InsertResourceNode (PMem32Node, ResourceNode); > + } > + ChildNodeLink =3D NextChildNodeLink; > } > - ChildNodeLink =3D NextChildNodeLink; > - } > - } > + } >=20 > + } > + ChildDeviceLink =3D ChildDeviceLink->ForwardLink; > } > - ChildDeviceLink =3D ChildDeviceLink->ForwardLink; > } >=20 > // > -- > 2.7.4 >=20 > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel