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 7DF7A1A20B4 for ; Fri, 23 Sep 2016 01:37:21 -0700 (PDT) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga103.fm.intel.com with ESMTP; 23 Sep 2016 01:37:21 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.30,381,1470726000"; d="scan'208";a="1044576549" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by fmsmga001.fm.intel.com with ESMTP; 23 Sep 2016 01:37:21 -0700 Received: from fmsmsx114.amr.corp.intel.com (10.18.116.8) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.248.2; Fri, 23 Sep 2016 01:37:20 -0700 Received: from shsmsx151.ccr.corp.intel.com (10.239.6.50) by FMSMSX114.amr.corp.intel.com (10.18.116.8) with Microsoft SMTP Server (TLS) id 14.3.248.2; Fri, 23 Sep 2016 01:37:20 -0700 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.101]) by SHSMSX151.ccr.corp.intel.com ([169.254.3.96]) with mapi id 14.03.0248.002; Fri, 23 Sep 2016 16:37:17 +0800 From: "Ni, Ruiyu" To: Ard Biesheuvel , "edk2-devel@lists.01.org" , "Gao, Liming" , "Zeng, Star" , "Tian, Feng" , "leif.lindholm@linaro.org" , "afish@apple.com" , "Kinney, Michael D" CC: "lersek@redhat.com" , "Yao, Jiewen" Thread-Topic: [PATCH v2] MdeModulePkg/PciBusDxe: make OPROM BAR degradation configurable Thread-Index: AQHSElD1merMdPM6jkOqdMR3eUM+OaCGxdSg Date: Fri, 23 Sep 2016 08:37:16 +0000 Message-ID: <734D49CCEBEEF84792F5B80ED585239D58E06E87@SHSMSX104.ccr.corp.intel.com> References: <1474274173-8432-1-git-send-email-ard.biesheuvel@linaro.org> In-Reply-To: <1474274173-8432-1-git-send-email-ard.biesheuvel@linaro.org> Accept-Language: en-US, zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH v2] MdeModulePkg/PciBusDxe: make OPROM BAR degradation configurable 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: Fri, 23 Sep 2016 08:37:21 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Ard, I agree with your patch, except the PCD name. Can you rename it to PcdPciDe= gradeResourceForOptionRom? Mike, What's your opinion? Thanks/Ray > -----Original Message----- > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > Sent: Monday, September 19, 2016 4:36 PM > To: edk2-devel@lists.01.org; Gao, Liming ; Zeng, St= ar > ; Tian, Feng ; Ni, Ruiyu > ; leif.lindholm@linaro.org; afish@apple.com; Kinney, > Michael D > Cc: lersek@redhat.com; Yao, Jiewen ; Ard > Biesheuvel > Subject: [PATCH v2] MdeModulePkg/PciBusDxe: make OPROM BAR > degradation configurable >=20 > The 'universal' PCI bus driver in MdeModulePkg contains a quirk to degrad= e > 64-bit PCI MMIO BARs to 32-bit in the presence of an option ROM on the > same PCI controller. >=20 > This quirk is highly specific to not just the X64 architecture in general= , but to > the PC platform in particular, given that only X64 platforms that require > legacy PC BIOS compatibility require it. However, making the quirk > dependent on the presence of the legacy BIOS protocol met with resistance= , > due to the fact that it introduces a dependency on the > IntelFrameworkModulePkg package. >=20 > So instead, make the quirk configurable, by introducing a feature flag PC= D > which defaults to TRUE only for X64. >=20 > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ard Biesheuvel > --- > v2: followed the suggestion of Andrew Fish to introduce a new feature fla= g > PCD that controls the PCI BAR degradation behavior. >=20 > MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf | 1 + > MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c | 76 > ++++++++++---------- > MdeModulePkg/MdeModulePkg.dec | 12 ++++ > 3 files changed, 52 insertions(+), 37 deletions(-) >=20 > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf > index 330ccc8cbffc..fc49f3d9412c 100644 > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf > @@ -100,6 +100,7 @@ [FeaturePcd] > gEfiMdeModulePkgTokenSpaceGuid.PcdPciBusHotplugDeviceSupport ## > CONSUMES > gEfiMdeModulePkgTokenSpaceGuid.PcdPciBridgeIoAlignmentProbe ## > CONSUMES > gEfiMdeModulePkgTokenSpaceGuid.PcdUnalignedPciIoEnable ## > CONSUMES > + gEfiMdeModulePkgTokenSpaceGuid.PcdPciDegradeBarsForOptionRom > ## > + CONSUMES >=20 > [Pcd] > gEfiMdeModulePkgTokenSpaceGuid.PcdSrIovSystemPageSize ## > SOMETIMES_CONSUMES > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c > index b0632d53b82b..37dc03e90358 100644 > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c > @@ -1058,48 +1058,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 6= 4-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[ResourceNod= e- > >Bar].BarTypeFixed) > - ) { > - RemoveEntryList (ChildNodeLink); > - InsertResourceNode (Mem32Node, ResourceNode); > + if (FeaturePcdGet (PcdPciDegradeBarsForOptionRom)) { > + // > + // 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->Ch= ildList) > { > + 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[ResourceN= ode- > >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[ResourceNod= e- > >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[ResourceN= ode- > >Bar].BarTypeFixed) > + ) { > + RemoveEntryList (ChildNodeLink); > + InsertResourceNode (PMem32Node, ResourceNode); > + } > + ChildNodeLink =3D NextChildNodeLink; > } > - ChildNodeLink =3D NextChildNodeLink; > - } > - } > + } >=20 > + } > + ChildDeviceLink =3D ChildDeviceLink->ForwardLink; > } > - ChildDeviceLink =3D ChildDeviceLink->ForwardLink; > } >=20 > // > diff --git a/MdeModulePkg/MdeModulePkg.dec > b/MdeModulePkg/MdeModulePkg.dec index 8d90f169b26e..d8cb9c119598 > 100644 > --- a/MdeModulePkg/MdeModulePkg.dec > +++ b/MdeModulePkg/MdeModulePkg.dec > @@ -749,6 +749,18 @@ [PcdsFeatureFlag] > # @Prompt Turn on PS2 Mouse Extended Verification >=20 > gEfiMdeModulePkgTokenSpaceGuid.PcdPs2MouseExtendedVerification|TR > UE|BOOLEAN|0x00010075 >=20 > +[PcdsFeatureFlag.X64] > + ## Indicates whether 64-bit PCI MMIO BARs should degrade to 32-bit in > +the presence of an option ROM > + # On X64 platforms, Option ROMs may contain code that executes in > +the context of a legacy BIOS (CSM), > + # which requires that all PCI MMIO BARs are located below 4 GB > + # TRUE - All PCI MMIO BARs of a device will be located below 4 GB i= f it has > an option ROM > + # FALSE - PCI MMIO BARs of a device may be located above 4 GB even i= f it > has an option ROM > + # @Prompt Degrade 64-bit PCI MMIO BARs for legacy BIOS option ROMs > + > +gEfiMdeModulePkgTokenSpaceGuid.PcdPciDegradeBarsForOptionRom|TR > UE|BOOLE > +AN|0x0001003a > + > +[PcdsFeatureFlag.IA32, PcdsFeatureFlag.ARM, PcdsFeatureFlag.AARCH64] > + > +gEfiMdeModulePkgTokenSpaceGuid.PcdPciDegradeBarsForOptionRom|FA > LSE|BOOL > +EAN|0x0001003a > + > [PcdsFeatureFlag.IA32, PcdsFeatureFlag.X64] > ## Indicates if DxeIpl should switch to long mode to enter DXE phase. > # It is assumed that 64-bit DxeCore is built in firmware if it is tru= e; > otherwise 32-bit DxeCore > -- > 2.7.4