From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-it0-x22b.google.com (mail-it0-x22b.google.com [IPv6:2607:f8b0:4001:c0b::22b]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 34AE31A20B7 for ; Fri, 23 Sep 2016 01:01:03 -0700 (PDT) Received: by mail-it0-x22b.google.com with SMTP id n143so8332970ita.1 for ; Fri, 23 Sep 2016 01:01:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=ClFH7YOGq27AcxmFmHOQh+b1d0TPUpSk8m4rulbV9+U=; b=iYqcUOesit8Dr71Dgw9zJVZdSW5x2+i9weaWq1xxnd6c6efTo9dM7maNwaJyPP+Njy xwuFIn/D4zzUpLDTmpxD6LRaCox86C6UwRcEY5evjR2rE9XjFjjFHz45Ya1smoDI0tcK wpwYKMSDhw3HFqHoqwTNR84+7PCeScsyrkmF4= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=ClFH7YOGq27AcxmFmHOQh+b1d0TPUpSk8m4rulbV9+U=; b=P+hZpqoHuVEU3tIEpcvLl4OQDXrmZT9hTHYa++zHVxwHxZ9qHtXVyO/RM0PNyWAZwD O32adAKQ6HA+hMfid2qpjb2Q2SnhID1T3DkrPPwDgExEIGGdbf/MXywOGgZHdQqf09u5 AbLRie6tXsR1a+LcIbqFoOIy+nsvKmYwlUYgj37U5ysGjX0HMiTcBX3P3icqlMfuf6KY nix8G/d7pu38TyOE+Rsem9tGO4P4N5W4G4z4RDDdBm3pF/WzqEMXA6DScCkneUG0hhpU YNHpl7X1G2LpeK/ftGS1ljY65f5DawaqpKj2o2Nzxi0tuVmAV6n/wdHBj3DrOYf5VMxz vnEg== X-Gm-Message-State: AA6/9RnuOqWqWWElRGjfLLTPMqljF5cdNyzSwTSrugb2UqSU1T917tdA+ololRUbWIS0vRZZ8UokysDgt9K8aNY+ X-Received: by 10.36.129.213 with SMTP id q204mr1882351itd.38.1474617662417; Fri, 23 Sep 2016 01:01:02 -0700 (PDT) MIME-Version: 1.0 Received: by 10.36.204.195 with HTTP; Fri, 23 Sep 2016 01:01:01 -0700 (PDT) In-Reply-To: <1474274173-8432-1-git-send-email-ard.biesheuvel@linaro.org> References: <1474274173-8432-1-git-send-email-ard.biesheuvel@linaro.org> From: Ard Biesheuvel Date: Fri, 23 Sep 2016 09:01:01 +0100 Message-ID: To: "edk2-devel@lists.01.org" , "Gao, Liming" , "Zeng, Star" , "Tian, Feng" , Ruiyu Ni , Leif Lindholm , "afish@apple.com" , "Kinney, Michael D" Cc: Laszlo Ersek , "Yao, Jiewen" , Ard Biesheuvel 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:01:03 -0000 Content-Type: text/plain; charset=UTF-8 On 19 September 2016 at 09:36, Ard Biesheuvel wrote: > The 'universal' PCI bus driver in MdeModulePkg contains a quirk to > degrade 64-bit PCI MMIO BARs to 32-bit in the presence of an option > ROM on the same PCI controller. > > 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. > > So instead, make the quirk configurable, by introducing a feature flag PCD > which defaults to TRUE only for X64. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ard Biesheuvel > --- > v2: followed the suggestion of Andrew Fish to introduce a new feature flag > PCD that controls the PCI BAR degradation behavior. > Ping? Any comments? > 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(-) > > 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 > > [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; > > - // > - // 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 = Bridge->ChildList.ForwardLink; > - while (ChildDeviceLink != NULL && ChildDeviceLink != &Bridge->ChildList) { > - PciIoDevice = PCI_IO_DEVICE_FROM_LINK (ChildDeviceLink); > - if (PciIoDevice->RomSize != 0) { > - if (!IsListEmpty (&Mem64Node->ChildList)) { > - ChildNodeLink = Mem64Node->ChildList.ForwardLink; > - while (ChildNodeLink != &Mem64Node->ChildList) { > - ResourceNode = RESOURCE_NODE_FROM_LINK (ChildNodeLink); > - NextChildNodeLink = ChildNodeLink->ForwardLink; > - > - if ((ResourceNode->PciDev == PciIoDevice) && > - (ResourceNode->Virtual || !PciIoDevice->PciBar[ResourceNode->Bar].BarTypeFixed) > - ) { > - RemoveEntryList (ChildNodeLink); > - InsertResourceNode (Mem32Node, ResourceNode); > + if (FeaturePcdGet (PcdPciDegradeBarsForOptionRom)) { > + // > + // 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 = Bridge->ChildList.ForwardLink; > + while (ChildDeviceLink != NULL && ChildDeviceLink != &Bridge->ChildList) { > + PciIoDevice = PCI_IO_DEVICE_FROM_LINK (ChildDeviceLink); > + if (PciIoDevice->RomSize != 0) { > + if (!IsListEmpty (&Mem64Node->ChildList)) { > + ChildNodeLink = Mem64Node->ChildList.ForwardLink; > + while (ChildNodeLink != &Mem64Node->ChildList) { > + ResourceNode = RESOURCE_NODE_FROM_LINK (ChildNodeLink); > + NextChildNodeLink = ChildNodeLink->ForwardLink; > + > + if ((ResourceNode->PciDev == PciIoDevice) && > + (ResourceNode->Virtual || !PciIoDevice->PciBar[ResourceNode->Bar].BarTypeFixed) > + ) { > + RemoveEntryList (ChildNodeLink); > + InsertResourceNode (Mem32Node, ResourceNode); > + } > + ChildNodeLink = NextChildNodeLink; > } > - ChildNodeLink = NextChildNodeLink; > - } > - } > + } > > - if (!IsListEmpty (&PMem64Node->ChildList)) { > - ChildNodeLink = PMem64Node->ChildList.ForwardLink; > - while (ChildNodeLink != &PMem64Node->ChildList) { > - ResourceNode = RESOURCE_NODE_FROM_LINK (ChildNodeLink); > - NextChildNodeLink = ChildNodeLink->ForwardLink; > - > - if ((ResourceNode->PciDev == PciIoDevice) && > - (ResourceNode->Virtual || !PciIoDevice->PciBar[ResourceNode->Bar].BarTypeFixed) > - ) { > - RemoveEntryList (ChildNodeLink); > - InsertResourceNode (PMem32Node, ResourceNode); > + if (!IsListEmpty (&PMem64Node->ChildList)) { > + ChildNodeLink = PMem64Node->ChildList.ForwardLink; > + while (ChildNodeLink != &PMem64Node->ChildList) { > + ResourceNode = RESOURCE_NODE_FROM_LINK (ChildNodeLink); > + NextChildNodeLink = ChildNodeLink->ForwardLink; > + > + if ((ResourceNode->PciDev == PciIoDevice) && > + (ResourceNode->Virtual || !PciIoDevice->PciBar[ResourceNode->Bar].BarTypeFixed) > + ) { > + RemoveEntryList (ChildNodeLink); > + InsertResourceNode (PMem32Node, ResourceNode); > + } > + ChildNodeLink = NextChildNodeLink; > } > - ChildNodeLink = NextChildNodeLink; > - } > - } > + } > > + } > + ChildDeviceLink = ChildDeviceLink->ForwardLink; > } > - ChildDeviceLink = ChildDeviceLink->ForwardLink; > } > > // > 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 > gEfiMdeModulePkgTokenSpaceGuid.PcdPs2MouseExtendedVerification|TRUE|BOOLEAN|0x00010075 > > +[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 if it has an option ROM > + # FALSE - PCI MMIO BARs of a device may be located above 4 GB even if it has an option ROM > + # @Prompt Degrade 64-bit PCI MMIO BARs for legacy BIOS option ROMs > + gEfiMdeModulePkgTokenSpaceGuid.PcdPciDegradeBarsForOptionRom|TRUE|BOOLEAN|0x0001003a > + > +[PcdsFeatureFlag.IA32, PcdsFeatureFlag.ARM, PcdsFeatureFlag.AARCH64] > + gEfiMdeModulePkgTokenSpaceGuid.PcdPciDegradeBarsForOptionRom|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 true; otherwise 32-bit DxeCore > -- > 2.7.4 >