From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) (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 83ED91A1E03 for ; Sun, 25 Sep 2016 23:34:45 -0700 (PDT) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga105.jf.intel.com with ESMTP; 25 Sep 2016 23:34:45 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.30,398,1470726000"; d="scan'208";a="765942467" Received: from fmsmsx104.amr.corp.intel.com ([10.18.124.202]) by FMSMGA003.fm.intel.com with ESMTP; 25 Sep 2016 23:34:45 -0700 Received: from fmsmsx124.amr.corp.intel.com (10.18.125.39) by fmsmsx104.amr.corp.intel.com (10.18.124.202) with Microsoft SMTP Server (TLS) id 14.3.248.2; Sun, 25 Sep 2016 23:34:44 -0700 Received: from shsmsx103.ccr.corp.intel.com (10.239.4.69) by fmsmsx124.amr.corp.intel.com (10.18.125.39) with Microsoft SMTP Server (TLS) id 14.3.248.2; Sun, 25 Sep 2016 23:34:44 -0700 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.101]) by SHSMSX103.ccr.corp.intel.com ([169.254.4.234]) with mapi id 14.03.0248.002; Mon, 26 Sep 2016 14:34:42 +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 v3] MdeModulePkg/PciBusDxe: make OPROM BAR degradation configurable Thread-Index: AQHSF78JerYcpey/vUW+ppR4wNMsG6CLUJmQ Date: Mon, 26 Sep 2016 06:34:41 +0000 Message-ID: <734D49CCEBEEF84792F5B80ED585239D58E08C36@SHSMSX104.ccr.corp.intel.com> References: <1474871228-27465-1-git-send-email-ard.biesheuvel@linaro.org> In-Reply-To: <1474871228-27465-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 v3] 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: Mon, 26 Sep 2016 06:34:45 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Reviewed-by: Ruiyu Ni > -----Original Message----- > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > Sent: Monday, September 26, 2016 2:27 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 v3] 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 > 'PcdPciDegradeResourceForOptionRom' which defaults to TRUE only for X64. >=20 > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ard Biesheuvel > --- > v3: rename feature flag PCD to 'PcdPciDegradeResourceForOptionRom' at > the > request of Ray > 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 | 7 +- > MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c | 76 > ++++++++++---------- > MdeModulePkg/MdeModulePkg.dec | 12 ++++ > 3 files changed, 55 insertions(+), 40 deletions(-) >=20 > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf > index 330ccc8cbffc..a3ab11fd8d93 100644 > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf > @@ -97,9 +97,10 @@ [Protocols] > gEfiLoadFile2ProtocolGuid ## SOMETIMES_PRODUCES >=20 > [FeaturePcd] > - gEfiMdeModulePkgTokenSpaceGuid.PcdPciBusHotplugDeviceSupport ## > CONSUMES > - gEfiMdeModulePkgTokenSpaceGuid.PcdPciBridgeIoAlignmentProbe ## > CONSUMES > - gEfiMdeModulePkgTokenSpaceGuid.PcdUnalignedPciIoEnable ## > CONSUMES > + gEfiMdeModulePkgTokenSpaceGuid.PcdPciBusHotplugDeviceSupport > ## CONSUMES > + gEfiMdeModulePkgTokenSpaceGuid.PcdPciBridgeIoAlignmentProbe ## > CONSUMES > + gEfiMdeModulePkgTokenSpaceGuid.PcdUnalignedPciIoEnable ## > CONSUMES > + > gEfiMdeModulePkgTokenSpaceGuid.PcdPciDegradeResourceForOptionRom > ## > + 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..e93134613b48 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 (PcdPciDegradeResourceForOptionRom)) { > + // > + // 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..270088c2e6a3 > 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.PcdPciDegradeResourceForOptionRo > m|TRUE|B > +OOLEAN|0x0001003a > + > +[PcdsFeatureFlag.IA32, PcdsFeatureFlag.ARM, PcdsFeatureFlag.AARCH64] > + > +gEfiMdeModulePkgTokenSpaceGuid.PcdPciDegradeResourceForOptionRo > m|FALSE| > +BOOLEAN|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