Ard, I agree with your patch, except the PCD name. Can you rename it to PcdPciDegradeResourceForOptionRom?
Mike, What's your opinion? Thanks/Ray > -----Original Message----- > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] > Sent: Monday, September 19, 2016 4:36 PM > To: edk2-devel@lists.01.org; Gao, Liming <liming....@intel.com>; Zeng, Star > <star.z...@intel.com>; Tian, Feng <feng.t...@intel.com>; Ni, Ruiyu > <ruiyu...@intel.com>; leif.lindh...@linaro.org; af...@apple.com; Kinney, > Michael D <michael.d.kin...@intel.com> > Cc: ler...@redhat.com; Yao, Jiewen <jiewen....@intel.com>; Ard > Biesheuvel <ard.biesheu...@linaro.org> > Subject: [PATCH v2] MdeModulePkg/PciBusDxe: make OPROM BAR > degradation configurable > > 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 <ard.biesheu...@linaro.org> > --- > v2: followed the suggestion of Andrew Fish to introduce a new feature flag > PCD that controls the PCI BAR degradation behavior. > > 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|TR > UE|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|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 true; > otherwise 32-bit DxeCore > -- > 2.7.4 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
