On 2016-03-04 06:46:32, Laszlo Ersek wrote:
> This library is a trivial unification of the following two PciLib
> instances (and the result is easily diffable against each):
> - MdePkg/Library/BasePciLibCf8
> - MdePkg/Library/BasePciLibPciExpress
>
Mike, what would you think about a MdePkg/Library/BasePciLibPcieCf8
lib instance that selected the access mode by a PCD? Is this too
platform specific for MdePkg?
Laszlo, if Mike doesn't think this is reasonable for MdePkg, then how
about the lib name I suggested above instead? In other words make it a
PCIe vs. CF8 switch, and nothing to do with 440FX vs. Q35.
-Jordan
> The PCI config access method is determined in the constructor function,
> from the dynamic PCD "PcdOvmfHostBridgePciDevId" that is set by
> PlatformPei.
>
> The library instance is usable in DXE phase or later modules: the PciLib
> instances being unified have no firmware phase / client module type
> restrictions, and here the only PCD access is made in the constructor
> function. That is, even before a given client executable's entry point is
> invoked.
>
> The library instance depends on PlatformPei both for setting the PCD
> mentioned above, and also for enabling MMCONFIG on Q35. PEI and earlier
> phase modules are not expected to need extended config access even on Q35.
>
> Cc: Gabriel Somlo <so...@cmu.edu>
> Cc: Jordan Justen <jordan.l.jus...@intel.com>
> Cc: Marcel Apfelbaum <mar...@redhat.com>
> Cc: Michał Zegan <webczat_...@poczta.onet.pl>
> Cc: Ruiyu Ni <ruiyu...@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek <ler...@redhat.com>
> ---
> OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
> | 47 ++++++
> {MdePkg/Library/BasePciLibCf8 =>
> OvmfPkg/Library/DxePciLibI440FxQ35}/PciLib.c | 161 +++++++++++++++-----
> 2 files changed, 173 insertions(+), 35 deletions(-)
>
> diff --git a/OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
> b/OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
> new file mode 100644
> index 000000000000..2bd10cc23282
> --- /dev/null
> +++ b/OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
> @@ -0,0 +1,47 @@
> +## @file
> +# An instance of the PCI Library that is based on both the PCI CF8 Library
> and
> +# the PCI Express Library.
> +#
> +# This PciLib instance caches the OVMF platform type (I440FX vs. Q35) in
> +# its entry point function, then delegates function calls to one of the
> +# PciCf8Lib or PciExpressLib "backends" as appropriate.
> +#
> +# Copyright (C) 2016, Red Hat, Inc.
> +#
> +# Copyright (c) 2007 - 2014, Intel Corporation. All rights reserved.<BR>
> +#
> +# This program and the accompanying materials are licensed and made
> available
> +# under the terms and conditions of the BSD License which accompanies this
> +# distribution. The full text of the license may be found at
> +# http://opensource.org/licenses/bsd-license.php.
> +#
> +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR
> +# IMPLIED.
> +##
> +
> +[Defines]
> + INF_VERSION = 0x00010005
> + BASE_NAME = DxePciLibI440FxQ35
> + FILE_GUID = 5360bff6-3911-4495-ae3c-b02ff004b585
> + MODULE_TYPE = BASE
> + VERSION_STRING = 1.0
> + LIBRARY_CLASS = PciLib|DXE_DRIVER DXE_RUNTIME_DRIVER
> SMM_CORE DXE_SMM_DRIVER UEFI_DRIVER UEFI_APPLICATION
> + CONSTRUCTOR = InitializeConfigAccessMethod
> +
> +# VALID_ARCHITECTURES = IA32 X64
> +
> +[Sources]
> + PciLib.c
> +
> +[Packages]
> + MdePkg/MdePkg.dec
> + OvmfPkg/OvmfPkg.dec
> +
> +[LibraryClasses]
> + PcdLib
> + PciCf8Lib
> + PciExpressLib
> +
> +[Pcd]
> + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId
> diff --git a/MdePkg/Library/BasePciLibCf8/PciLib.c
> b/OvmfPkg/Library/DxePciLibI440FxQ35/PciLib.c
> similarity index 85%
> copy from MdePkg/Library/BasePciLibCf8/PciLib.c
> copy to OvmfPkg/Library/DxePciLibI440FxQ35/PciLib.c
> index f5f21475d8bd..6c1a272973af 100644
> --- a/MdePkg/Library/BasePciLibCf8/PciLib.c
> +++ b/OvmfPkg/Library/DxePciLibI440FxQ35/PciLib.c
> @@ -1,6 +1,14 @@
> /** @file
> - PCI Library functions that use I/O ports 0xCF8 and 0xCFC to perform
> - PCI Configuration cycles. Layers on top of one PCI CF8 Library instance.
> + PCI Library functions that use
> + (a) I/O ports 0xCF8 and 0xCFC to perform PCI Configuration cycles, layering
> + on top of one PCI CF8 Library instance; or
> + (b) PCI Library functions that use the 256 MB PCI Express MMIO window to
> + perform PCI Configuration cycles, layering on PCI Express Library.
> +
> + The decision is made in the entry point function, based on the OVMF
> platform
> + type, and then adhered to during the lifetime of the client module.
> +
> + Copyright (C) 2016, Red Hat, Inc.
>
> Copyright (c) 2006 - 2012, Intel Corporation. All rights reserved.<BR>
> This program and the accompanying materials
> @@ -16,8 +24,25 @@
>
> #include <Base.h>
>
> +#include <IndustryStandard/Q35MchIch9.h>
> +
> #include <Library/PciLib.h>
> #include <Library/PciCf8Lib.h>
> +#include <Library/PciExpressLib.h>
> +#include <Library/PcdLib.h>
> +
> +STATIC BOOLEAN mRunningOnQ35;
> +
> +RETURN_STATUS
> +EFIAPI
> +InitializeConfigAccessMethod (
> + VOID
> + )
> +{
> + mRunningOnQ35 = (PcdGet16 (PcdOvmfHostBridgePciDevId) ==
> + INTEL_Q35_MCH_DEVICE_ID);
> + return RETURN_SUCCESS;
> +}
>
> /**
> Registers a PCI device so PCI configuration registers may be accessed
> after
> @@ -46,7 +71,9 @@ PciRegisterForRuntimeAccess (
> IN UINTN Address
> )
> {
> - return PciCf8RegisterForRuntimeAccess (Address);
> + return mRunningOnQ35 ?
> + PciExpressRegisterForRuntimeAccess (Address) :
> + PciCf8RegisterForRuntimeAccess (Address);
> }
>
> /**
> @@ -70,7 +97,9 @@ PciRead8 (
> IN UINTN Address
> )
> {
> - return PciCf8Read8 (Address);
> + return mRunningOnQ35 ?
> + PciExpressRead8 (Address) :
> + PciCf8Read8 (Address);
> }
>
> /**
> @@ -96,7 +125,9 @@ PciWrite8 (
> IN UINT8 Value
> )
> {
> - return PciCf8Write8 (Address, Value);
> + return mRunningOnQ35 ?
> + PciExpressWrite8 (Address, Value) :
> + PciCf8Write8 (Address, Value);
> }
>
> /**
> @@ -126,7 +157,9 @@ PciOr8 (
> IN UINT8 OrData
> )
> {
> - return PciCf8Or8 (Address, OrData);
> + return mRunningOnQ35 ?
> + PciExpressOr8 (Address, OrData) :
> + PciCf8Or8 (Address, OrData);
> }
>
> /**
> @@ -156,7 +189,9 @@ PciAnd8 (
> IN UINT8 AndData
> )
> {
> - return PciCf8And8 (Address, AndData);
> + return mRunningOnQ35 ?
> + PciExpressAnd8 (Address, AndData) :
> + PciCf8And8 (Address, AndData);
> }
>
> /**
> @@ -189,7 +224,9 @@ PciAndThenOr8 (
> IN UINT8 OrData
> )
> {
> - return PciCf8AndThenOr8 (Address, AndData, OrData);
> + return mRunningOnQ35 ?
> + PciExpressAndThenOr8 (Address, AndData, OrData) :
> + PciCf8AndThenOr8 (Address, AndData, OrData);
> }
>
> /**
> @@ -221,7 +258,9 @@ PciBitFieldRead8 (
> IN UINTN EndBit
> )
> {
> - return PciCf8BitFieldRead8 (Address, StartBit, EndBit);
> + return mRunningOnQ35 ?
> + PciExpressBitFieldRead8 (Address, StartBit, EndBit) :
> + PciCf8BitFieldRead8 (Address, StartBit, EndBit);
> }
>
> /**
> @@ -257,7 +296,9 @@ PciBitFieldWrite8 (
> IN UINT8 Value
> )
> {
> - return PciCf8BitFieldWrite8 (Address, StartBit, EndBit, Value);
> + return mRunningOnQ35 ?
> + PciExpressBitFieldWrite8 (Address, StartBit, EndBit, Value) :
> + PciCf8BitFieldWrite8 (Address, StartBit, EndBit, Value);
> }
>
> /**
> @@ -296,7 +337,9 @@ PciBitFieldOr8 (
> IN UINT8 OrData
> )
> {
> - return PciCf8BitFieldOr8 (Address, StartBit, EndBit, OrData);
> + return mRunningOnQ35 ?
> + PciExpressBitFieldOr8 (Address, StartBit, EndBit, OrData) :
> + PciCf8BitFieldOr8 (Address, StartBit, EndBit, OrData);
> }
>
> /**
> @@ -335,7 +378,9 @@ PciBitFieldAnd8 (
> IN UINT8 AndData
> )
> {
> - return PciCf8BitFieldAnd8 (Address, StartBit, EndBit, AndData);
> + return mRunningOnQ35 ?
> + PciExpressBitFieldAnd8 (Address, StartBit, EndBit, AndData) :
> + PciCf8BitFieldAnd8 (Address, StartBit, EndBit, AndData);
> }
>
> /**
> @@ -379,7 +424,9 @@ PciBitFieldAndThenOr8 (
> IN UINT8 OrData
> )
> {
> - return PciCf8BitFieldAndThenOr8 (Address, StartBit, EndBit, AndData,
> OrData);
> + return mRunningOnQ35 ?
> + PciExpressBitFieldAndThenOr8 (Address, StartBit, EndBit, AndData,
> OrData) :
> + PciCf8BitFieldAndThenOr8 (Address, StartBit, EndBit, AndData,
> OrData);
> }
>
> /**
> @@ -404,7 +451,9 @@ PciRead16 (
> IN UINTN Address
> )
> {
> - return PciCf8Read16 (Address);
> + return mRunningOnQ35 ?
> + PciExpressRead16 (Address) :
> + PciCf8Read16 (Address);
> }
>
> /**
> @@ -431,7 +480,9 @@ PciWrite16 (
> IN UINT16 Value
> )
> {
> - return PciCf8Write16 (Address, Value);
> + return mRunningOnQ35 ?
> + PciExpressWrite16 (Address, Value) :
> + PciCf8Write16 (Address, Value);
> }
>
> /**
> @@ -462,7 +513,9 @@ PciOr16 (
> IN UINT16 OrData
> )
> {
> - return PciCf8Or16 (Address, OrData);
> + return mRunningOnQ35 ?
> + PciExpressOr16 (Address, OrData) :
> + PciCf8Or16 (Address, OrData);
> }
>
> /**
> @@ -493,7 +546,9 @@ PciAnd16 (
> IN UINT16 AndData
> )
> {
> - return PciCf8And16 (Address, AndData);
> + return mRunningOnQ35 ?
> + PciExpressAnd16 (Address, AndData) :
> + PciCf8And16 (Address, AndData);
> }
>
> /**
> @@ -527,7 +582,9 @@ PciAndThenOr16 (
> IN UINT16 OrData
> )
> {
> - return PciCf8AndThenOr16 (Address, AndData, OrData);
> + return mRunningOnQ35 ?
> + PciExpressAndThenOr16 (Address, AndData, OrData) :
> + PciCf8AndThenOr16 (Address, AndData, OrData);
> }
>
> /**
> @@ -560,7 +617,9 @@ PciBitFieldRead16 (
> IN UINTN EndBit
> )
> {
> - return PciCf8BitFieldRead16 (Address, StartBit, EndBit);
> + return mRunningOnQ35 ?
> + PciExpressBitFieldRead16 (Address, StartBit, EndBit) :
> + PciCf8BitFieldRead16 (Address, StartBit, EndBit);
> }
>
> /**
> @@ -597,7 +656,9 @@ PciBitFieldWrite16 (
> IN UINT16 Value
> )
> {
> - return PciCf8BitFieldWrite16 (Address, StartBit, EndBit, Value);
> + return mRunningOnQ35 ?
> + PciExpressBitFieldWrite16 (Address, StartBit, EndBit, Value) :
> + PciCf8BitFieldWrite16 (Address, StartBit, EndBit, Value);
> }
>
> /**
> @@ -637,7 +698,9 @@ PciBitFieldOr16 (
> IN UINT16 OrData
> )
> {
> - return PciCf8BitFieldOr16 (Address, StartBit, EndBit, OrData);
> + return mRunningOnQ35 ?
> + PciExpressBitFieldOr16 (Address, StartBit, EndBit, OrData) :
> + PciCf8BitFieldOr16 (Address, StartBit, EndBit, OrData);
> }
>
> /**
> @@ -677,7 +740,9 @@ PciBitFieldAnd16 (
> IN UINT16 AndData
> )
> {
> - return PciCf8BitFieldAnd16 (Address, StartBit, EndBit, AndData);
> + return mRunningOnQ35 ?
> + PciExpressBitFieldAnd16 (Address, StartBit, EndBit, AndData) :
> + PciCf8BitFieldAnd16 (Address, StartBit, EndBit, AndData);
> }
>
> /**
> @@ -722,7 +787,9 @@ PciBitFieldAndThenOr16 (
> IN UINT16 OrData
> )
> {
> - return PciCf8BitFieldAndThenOr16 (Address, StartBit, EndBit, AndData,
> OrData);
> + return mRunningOnQ35 ?
> + PciExpressBitFieldAndThenOr16 (Address, StartBit, EndBit, AndData,
> OrData) :
> + PciCf8BitFieldAndThenOr16 (Address, StartBit, EndBit, AndData,
> OrData);
> }
>
> /**
> @@ -747,7 +814,9 @@ PciRead32 (
> IN UINTN Address
> )
> {
> - return PciCf8Read32 (Address);
> + return mRunningOnQ35 ?
> + PciExpressRead32 (Address) :
> + PciCf8Read32 (Address);
> }
>
> /**
> @@ -774,7 +843,9 @@ PciWrite32 (
> IN UINT32 Value
> )
> {
> - return PciCf8Write32 (Address, Value);
> + return mRunningOnQ35 ?
> + PciExpressWrite32 (Address, Value) :
> + PciCf8Write32 (Address, Value);
> }
>
> /**
> @@ -805,7 +876,9 @@ PciOr32 (
> IN UINT32 OrData
> )
> {
> - return PciCf8Or32 (Address, OrData);
> + return mRunningOnQ35 ?
> + PciExpressOr32 (Address, OrData) :
> + PciCf8Or32 (Address, OrData);
> }
>
> /**
> @@ -836,7 +909,9 @@ PciAnd32 (
> IN UINT32 AndData
> )
> {
> - return PciCf8And32 (Address, AndData);
> + return mRunningOnQ35 ?
> + PciExpressAnd32 (Address, AndData) :
> + PciCf8And32 (Address, AndData);
> }
>
> /**
> @@ -870,7 +945,9 @@ PciAndThenOr32 (
> IN UINT32 OrData
> )
> {
> - return PciCf8AndThenOr32 (Address, AndData, OrData);
> + return mRunningOnQ35 ?
> + PciExpressAndThenOr32 (Address, AndData, OrData) :
> + PciCf8AndThenOr32 (Address, AndData, OrData);
> }
>
> /**
> @@ -903,7 +980,9 @@ PciBitFieldRead32 (
> IN UINTN EndBit
> )
> {
> - return PciCf8BitFieldRead32 (Address, StartBit, EndBit);
> + return mRunningOnQ35 ?
> + PciExpressBitFieldRead32 (Address, StartBit, EndBit) :
> + PciCf8BitFieldRead32 (Address, StartBit, EndBit);
> }
>
> /**
> @@ -940,7 +1019,9 @@ PciBitFieldWrite32 (
> IN UINT32 Value
> )
> {
> - return PciCf8BitFieldWrite32 (Address, StartBit, EndBit, Value);
> + return mRunningOnQ35 ?
> + PciExpressBitFieldWrite32 (Address, StartBit, EndBit, Value) :
> + PciCf8BitFieldWrite32 (Address, StartBit, EndBit, Value);
> }
>
> /**
> @@ -980,7 +1061,9 @@ PciBitFieldOr32 (
> IN UINT32 OrData
> )
> {
> - return PciCf8BitFieldOr32 (Address, StartBit, EndBit, OrData);
> + return mRunningOnQ35 ?
> + PciExpressBitFieldOr32 (Address, StartBit, EndBit, OrData) :
> + PciCf8BitFieldOr32 (Address, StartBit, EndBit, OrData);
> }
>
> /**
> @@ -1020,7 +1103,9 @@ PciBitFieldAnd32 (
> IN UINT32 AndData
> )
> {
> - return PciCf8BitFieldAnd32 (Address, StartBit, EndBit, AndData);
> + return mRunningOnQ35 ?
> + PciExpressBitFieldAnd32 (Address, StartBit, EndBit, AndData) :
> + PciCf8BitFieldAnd32 (Address, StartBit, EndBit, AndData);
> }
>
> /**
> @@ -1065,7 +1150,9 @@ PciBitFieldAndThenOr32 (
> IN UINT32 OrData
> )
> {
> - return PciCf8BitFieldAndThenOr32 (Address, StartBit, EndBit, AndData,
> OrData);
> + return mRunningOnQ35 ?
> + PciExpressBitFieldAndThenOr32 (Address, StartBit, EndBit, AndData,
> OrData) :
> + PciCf8BitFieldAndThenOr32 (Address, StartBit, EndBit, AndData,
> OrData);
> }
>
> /**
> @@ -1099,7 +1186,9 @@ PciReadBuffer (
> OUT VOID *Buffer
> )
> {
> - return PciCf8ReadBuffer (StartAddress, Size, Buffer);
> + return mRunningOnQ35 ?
> + PciExpressReadBuffer (StartAddress, Size, Buffer) :
> + PciCf8ReadBuffer (StartAddress, Size, Buffer);
> }
>
> /**
> @@ -1134,5 +1223,7 @@ PciWriteBuffer (
> IN VOID *Buffer
> )
> {
> - return PciCf8WriteBuffer (StartAddress, Size, Buffer);
> + return mRunningOnQ35 ?
> + PciExpressWriteBuffer (StartAddress, Size, Buffer) :
> + PciCf8WriteBuffer (StartAddress, Size, Buffer);
> }
> --
> 1.8.3.1
>
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
