On 2016-03-06 06:28:26, Laszlo Ersek wrote:
> On 03/05/16 00:47, Kinney, Michael D wrote:
> > Jordan,
> >
> > Given these libs are Base type, a Dynamic PCD could not be used. Only a
> > fixed
> > or patchable PCD could be used, and if you go through path, you might as
> > well
> > pick the right lib in the DSC at build time and get smaller size and better
> > performance.
>
> I'd like to discuss two things here.
>
> * First, I fully agree with you that determining the branch in this
> unified library instance *at build time* is functionally equivalent (but
> is size- and perf-wise inferior) to simply picking the right one of the
> currently available PciLib instances, at build time.
>
> I think that this library instance is special to virtualization. On the
> bare metal, you never face a situation where you don't know in advance
> whether your platform will be a PCIe system, or a PCI-only system. But,
> that's exactly what we have with QEMU (the i440fx and Q35 machine
> types), so this dynamism is needed. (In fact I find it very nice that
> the modularity of edk2 allowed me to do such a simple unification.)
>
> Another possibility for influencing the branch to take would be a GUID
> HOB. In this particular case however I only saw downsides in a HOB. On
> the source code side, it would take another GUID definition, another
> structure definition, possibly another header file. And on the
> functionality side, the only "extension" it would buy us would be the
> usability of the library instance in the DXE_CORE. (The DXE_CORE cannot
> use PCDs, but it can look at HOBs.)
>
> We couldn't use it in PEI anyway (without ugly depex trickery), because
> a PEIM using this library instance could be dispatched before the PEIM
> that produces the HOB.
>
> But, in OVMF, PEI is perfectly fine with just 0xCF8 access anyway.
>
> * Second, any given library instance has two kinds of "qualification" in
> its INF file, as far as client modules are concerned. One is
> MODULE_TYPE, and the other is the restriction list in LIBRARY_CLASS.
>
> These qualifications seem to overlap, in a (superficially) confusing manner:
>
> (a) the INF file spec says about MODULE_TYPE,
>
> For Library Modules, the MODULE_TYPE must specify the MODULE_TYPE
> of the module that will use the driver.
>
> Furthermore, in appendix G, MODULE_TYPE=BASE is explicitly allowed for
> libraries, and then BASE is documented as:
>
> Modules of this type can be ported to any execution environment.
> This module type is intended to be use by silicon module developers
> to produce source code that is not tied to any specific execution
> environment.
>
> (b) But the INF file also says
>
> Each LIBRARY_CLASS statement must provide the name of the library
> class supported, followed by the pipe "|" field separator and then
> a space " " delimited list of module types the library instances
> supports.
>
> Thus a conflict appears between
>
> MODULE_TYPE = BASE
>
> and
>
> LIBRARY_CLASS = WhateverLib|DXE_DRIVER
>
> because the former implies "any execution environment", whereas the
> second implies the DXE phase.
>
> However, this apparent conflict is resolved by the INF spec in the
> following:
>
> The MODULE_TYPE entry in the [Defines] section for a library only
> defines the module type that the build system must assume for
> building the library. It does not define the types of modules that
> a library may be linked with. Instead, the LIBRARY_CLASS entry in
> the [Defines] section optionally lists the supported module types
> that the library may be linked against.
>
> Therefore, the MODULE_TYPE for a library instance only determines
> prototype / calling convention compatibility. In my experience, this
> practically boils down to two things, and nothing more:
>
> - functions must return RETURN_STATUS vs. EFI_STATUS,
>
> - the constructor function of the library instance (if any) gets VOID
> if the MODULE_TYPE is BASE, vs. ImageHandle plus SystemTable if the
> MODULE_TYPE is DXE_DRIVER.
>
> That's it.
>
> What *really* matters is the restriction list in LIBRARY_CLASS; that's
> where the author of the library instance ensures that the instance
> doesn't get linked into a module (and firmware phase) that cannot
> provide the library instance with the necessary environment.
>
> Therefore, making a library instance MODULE_TYPE=BASE, and then
> restricting it to the DXE phase and later with LIBRARY_CLASS, is valid
> in my opinion. The library instance may not need ImageHandle and
> SystemTable for its own code at all, it is okay with returning
> RETURN_STATUS values, but it might need the environment to be DXE or later.
>
> The question then emerges, how we should *name* the library instance. In
> my experience, the name is supposed to reflect the actual client module
> / firmware phase restrictions. Which is why I named this instance DxeXXXXXX.
>
> In summary, if there is one bit of inconsistency in this patch, then
> that is the MODULE_TYPE setting. The name prefix (Dxe*) and the
> LIBRARY_CLASS restriction list are correct, they reflect my intent and
> reality.
>
> If you or Jordan wish that I flip MODULE_TYPE to DXE_DRIVER, I can
> certainly try that (all of the listed module types are able to provide
> ImageHandle and SystemTable to the constructor function). I think it's
> unnecessary (the code never directly uses ImageHandle or SystemTable),
> but if you think it would be more consistent, I can do that.
>
> So:
>
> - I can change MODULE_TYPE to DXE_DRIVER if you guys want that
> (although I think it's unnecessary),
>
> - I can also replace the "I440FxQ35" *suffix* with "PcieCf8"
> (although the actual functionality would still have to be based on
> the OVMF platform type, for which we already have a PCD in OVMF).
>
I'm fine with making this either a 'Base' or 'PeiDxe' library. Mike
appears to prefer PeiDxe.
Although I can't think of a use case besides OVMF, I would prefer to
add it to MdePkg simply because it just switches between CF8 and PCIe,
which is not specific to virtualization. (Even if OVMF is the only use
case we have right now.)
If Mike doesn't think it makes sense to add it to MdePkg, then I would
still like to design it in the manner that could make sense for
inclusion in MdePkg.
> > If we wanted to restrict the use of this new PciLib to PEIMs that run after
> > the
> > PCD PEIM and DXE/SMM drivers that run after the PCD DXE driver, then a new
> > PciLib in MdePkg is possible. A PCD can be defined for the access method
> > CF8/MMCFG. An OVMF specific platform PEIM runs early (but after PCD PEIM)
> > and detects the if the currently running platform is 440FX or Q35.
>
> I see two problems with this:
>
> - Small problem: for OVMF specifically, we'd need to set yet another
> PCD for this purpose (near the spot where we set the platform type
> PCD already.)
>
> - Large problem: although such a centralized library instance could
> express the dependency on the PCD PPI / Protocol -- simply by
> depending on the phase-matching PcdLib instance --, it could not
> easily express the dependency on some code *actually setting* the PCD
> to the right value. For platforms that made the PCD fixed or
> patchable, this would be no problems, but for a platform where the
> PCD is dynamic, this could be misleading.
>
> For OVMF specifically, it would *not* be a problem. OVMF would never
> link the library into modules that run before the DXE phase, and the
> PCD would be set in PEI. However, if another platform would like to
> link the library into PEIMs as well, *and* keep the PCD dynamic, the
> build system itself would not catch the unspecified dispatch order.
> The platform would have to resort to BEFORE or AFTER directives, or
> order the relevant PEIMs with an APRIORI file. I find that *very*
> obscure for a core library instance that is supposed to be linkable
> into anything at all.
>
> In summary, this patch indeed makes the library instance (living under
> OvmfPkg) more restrictive than it could theoretically be, but it's just
> right for OVMF, *plus* any mis-use is immediately caught at build time.
> Whereas generalizing the library into a core module would:
>
> - not benefit actual physical platforms,
>
> - place obscure, explicit ordering requirements on platforms that would
> like to set the PCD dynamically in firmware phase P, then also
> consume the PCD -- through the library instance -- in the same phase
> P.
>
> (OVMF would *happen* to be immune to this problem, because the
> setting and the consuming are in separate phases.)
>
I'm not too concerned about those issues. I think the platform
developer does have to pay special attention to when PCIe gets
initialized, but this doesn't add much to that.
I have a question for Mike: Would it be possible for OVMF to set
dynamic PCDs and/or the SKU from SEC? That might allow us to configure
PCIe in SEC and have all PEI modules use PCIe.
> > 2 options from this point:
> >
> > 1) Call LibPcdSetSku() in PcdLib to set the SKU, so Dynamic PCD values that
> > are
> > specific to 440FX or Q35 are available. OVMF DSC file is updated for
> > multiple SKUs with the SKU specific settings including the new PCD for
> > the PCI Config access method.
> >
> > 2) Use PcdSetxx() to set the PCI Config access method PCD to the
> > appropriate value.
> >
> > I would recommend (1) if there is more than one PCD values that needs to
> > be different for 440FX and Q35.
>
> I've never heard of LibPcdSetSku(). :) In retrospect it's not
> surprising, there's not one call in the open source edk2 tree to
> LibPcdSetSku().
>
> The function comments in "MdePkg/Include/Library/PcdLib.h" say:
>
> This function provides a means by which SKU support can be established
> in the PCD infrastructure.
>
> Sets the current SKU in the PCD database to the value specified by
> SkuId. SkuId is returned. If SkuId >= PCD_MAX_SKU_ID, then ASSERT().
>
> @param SkuId The SKU value that will be used when the PCD service
> retrieves and sets values associated with a PCD token.
>
> Then, looking at the protocol member function that implements this,
> DxePcdSetSku() in "MdeModulePkg/Universal/PCD/Dxe/Pcd.c":
>
> Sets the SKU value for subsequent calls to set or get PCD token
> values.
>
> SetSku() sets the SKU Id to be used for subsequent calls to set or get
> PCD values. SetSku() is normally called only once by the system.
>
> For each item (token), the database can hold a single value that
> applies to all SKUs, or multiple values, where each value is
> associated with a specific SKU Id. Items with multiple, SKU-specific
> values are called SKU enabled.
>
> The SKU Id of zero is reserved as a default. The valid SkuId range is
> 1 to 255. For tokens that are not SKU enabled, the system ignores
> any set SKU Id and works with the single value for that token. For
> SKU-enabled tokens, the system will use the SKU Id set by the last
> call to SetSku(). If no SKU Id is set or the currently set SKU Id
> isn't valid for the specified token, the system uses the default SKU
> Id. If the system attempts to use the default SKU Id and no value has
> been set for that Id, the results are unpredictable.
>
> @param[in] SkuId The SKU value that will be used when the PCD service
> will retrieve and set values associated with a PCD token.
>
> This confirms my fears, actually. I think that multi-valued PCDs
> (dependent on SKU enablement or whatever else) are a terrible idea for
> OVMF. It enables the possibility that getting a dynamic PCD on the
> opposite sides of a SetSku() call will return different values, without
> ever touching the PCD itself. It can also cause practically-fixed PCDs
> (i.e. such that are not suitable for PcdSet calls()) to change value.
>
> I find this very hard to understand, debug, or *grep* for, and it would
> introduce yet more ordering requirements ("set the SKU as early as
> possible").
>
> Hence my preference would definitely be (2), as far as I'm responsible
> for maintaining OvmfPkg. I didn't forget what I wrote just above (under
> "small problem") -- yes, using two PCDs would be ugly, but using
> SKU-enabled PCDs would be *much* worse.
>
> So here's what I would like, in decreasing order of preference:
>
> * Keep the patch, as is. (Most preferred.)
>
> * Keep the library instance under OvmfPkg, but set MODULE_TYPE to
> DXE_DRIVER, and/or replace the "I440FxQ35" suffix with "PcieCf8".
>
> * Turn the library instance into a core module. Don't restrict it to
> DXE and later. Consume a new core PCD, introducing obscure ordering
> requirements that are not enforced at build time.
>
> Set this new PCD explicitly in OVMF's PlatformPei, near the current
>
> PcdSet16 (PcdOvmfHostBridgePciDevId, mHostBridgeDevId)
>
> call (where we capture the platform type).
>
I don't see this as much different than depending on the
PcdOvmfHostBridgePciDevId.
I prefer this, or the next option, but it is probably better to push
out the SKU thing until later. So, I guess I'd prefer this option for
now. If Mike doesn't think the library makes sense for MdePkg, then
I'd still like to define it generically enough that it *could* live in
MdePkg, even if it ends up in OVMF. (I seem to waste a lot of time on
such pointless things. For example,
OvmfPkg/Include/Library/PlatformFvbLib.h)
I guess I'm fine with the library as currently defined. So, if you
would prefer that I make these changes, then I guess we can move
forward with what you currently have.
-Jordan
> * Turn the library instance into a core module. Don't restrict it to
> DXE and later. Consume a new core PCD, introducing obscure ordering
> requirements that are not enforced at build time.
>
> Set SKU-dependent DynamicDefaults for this PCD (and maybe other OVMF
> PCDs as well) in OVMF's DSC files. Call LibPcdSetSku() in
> PlatformPei, instead of the current call to
>
> PcdSet16 (PcdOvmfHostBridgePciDevId, mHostBridgeDevId).
>
> (Least preferred.)
>
> Thanks
> Laszlo
>
> >
> > Best regards,
> >
> > Mike
> >
> >> -----Original Message-----
> >> From: Justen, Jordan L
> >> Sent: Friday, March 4, 2016 12:39 PM
> >> To: Laszlo Ersek <ler...@redhat.com>; edk2-devel@lists.01.org; Kinney,
> >> Michael D
> >> <michael.d.kin...@intel.com>
> >> Cc: Gabriel Somlo <so...@cmu.edu>; Marcel Apfelbaum <mar...@redhat.com>;
> >> Michał Zegan
> >> <webczat_...@poczta.onet.pl>; Ni, Ruiyu <ruiyu...@intel.com>
> >> Subject: Re: [PATCH 3/5] OvmfPkg: add DxePciLibI440FxQ35
> >>
> >> 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
