* [PATCH 0/7] MdePkg, OvmfPkg, ArmVirtPkg: add and use PCI(E) Capabilities Library
@ 2018-05-04 21:36 Laszlo Ersek
2018-05-04 21:36 ` [PATCH 1/7] MdePkg: introduce PciCapLib Laszlo Ersek
` (7 more replies)
0 siblings, 8 replies; 11+ messages in thread
From: Laszlo Ersek @ 2018-05-04 21:36 UTC (permalink / raw)
To: edk2-devel-01; +Cc: Ard Biesheuvel, Jordan Justen, Liming Gao, Michael D Kinney
Repo: https://github.com/lersek/edk2.git
Branch: pci_cap
In message [1], Jordan suggested a general "capabilities helper lib that
could iterate & read the PCI capability structs". Patch #1 introduces
that library (class and central BASE instance) to MdePkg; the library
class is called PciCapLib.
[1] http://mid.mail-archive.com/150696619010.2454.9266470245604846575@jljusten-skl
PciCapLib offers APIs to parse capabilities lists, and to locate,
describe, read and write capabilities.
The library class targets a wide range of client module types; for
example, platform PEIMs, platform DXE drivers, and UEFI drivers that
follow the UEFI Driver Model. For this reason, the library class is
designed with PCI config access abstracted away, even beyond the PPI /
Protocol level, since those cannot cross the PEI / DXE boundary.
This (minimal) config space access method abstraction is called
PCI_CAP_DEV. It is *conceptually* a protocol. Client modules plug
PCI_CAP_DEV instances into PciCapLib for config space access, similarly
to how DiskIo and BlockIo instances are plugged into filesystem drivers.
Similarly to the filesystem driver example, PciCapLib is an example for
the "strategy pattern".
Patches #2 and #3 introduce small "factory" libraries that help client
modules produce PCI_CAP_DEV instances:
- PciCapPciSegmentLib -- incl. class and central BASE instance -- from
patch #2 produces PCI_CAP_DEV instances on top of PciSegmentLib, using
the "Segment:Bus:Device.Function" notation. This is meant for platform
modules (PEIMs and DXE drivers), which generally have a working
PciSegmentLib instance available.
- PciCapPciIoLib -- incl. class and central UEFI instance -- from patch
#3 lets client modules produce PCI_CAP_DEV instances on top of
EFI_PCI_IO_PROTOCOL instances. This is meant for UEFI drivers.
Patches #4 and #5 resolve the three new lib classes to their respective
central instances in the OvmfPkg and ArmVirtPkg DSC files.
Patch #6 converts OvmfPkg/PciHotPlugInitDxe to the new APIs, from manual
capability list parsing. Because PciHotPlugInitDxe is a platform DXE
driver, this conversion uses PciCapPciSegmentLib and PciCapLib.
Patch #7 converts OvmfPkg/Virtio10Dxe to the new APIs, from manual
capability list parsing. Because Virtio10Dxe is a UEFI driver, this
conversion uses PciCapPciIoLib and PciCapLib.
----[ jump to the patches now, and return if you have questions: I
have answers, but you might find them too verbose in advance ]----
Q1: If PCI config access needs to be abstracted from PciCapLib, then why
don't we provide multiple instances of PciCapLib? In other words,
why the PCI_CAP_DEV "protocol" rather than multiple instances of
PciCapLib?
A1: The point of lib classes and instances is that the class provides a
common interface, and the instances provide different
implementations. We have the opposite use case here (hence the
"strategy pattern"): the implementation of the higher level
algorithm is common, and the objects that plug into it differ at the
*interface* level ("Segment:Bus:Device.Function" notation versus
PciIo protocol).
None of those notations are suitable for inclusion in the PciCapLib
*class*; a more abstract interface is needed for identifying the PCI
device to the library, for parsing of capabilities lists.
Q2: OK, let's say we can't (and shouldn't) include both notations in the
PciCapLib class; can we settle on one of them, rather than
PCI_CAP_DEV?
A2: No, we can't. PciIo is obviously unusable in PEIMs (and in quite a
few platform DXE drivers too).
It takes a bit longer to explain why the other choice, i.e. adopting
the "Segment:Bus:Device.Function" notation for the PciCapLib
interface, is not good either. It has to do with the portability of
UEFI drivers that bind PciIo protocol instances:
It is indeed easy enough to call PciIo->GetLocation() and plug the
output values into a theoretical "Segment:Bus:Device.Function"
interface. However, about the only two things that could consume
such location identifiers and actually *implement* config space
access would be:
(a) the EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL.Pci.Read/Write member
functions, where the member functions themselves take the
"Bus:Device.Function" part, and the "Segment" part must be used
for selecting the EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL instance
itself, through the SegmentNumber member;
(b) the PciSegmentLib interfaces.
Building a PciCapLib instance that uses (a) into a UEFI driver is a
bad idea, because there are platforms that provide PciIo instances
-- hence the driver can generally run on them -- but don't provide
PciRootBridgeIo instances.
Building a PciCapLib instance that uses (b) into a UEFI driver is
worse. The PciSegmentLib interfaces are not capable of returning
errors. For example, if a device appears as PCI Express (implying
its extended config space exists, potentially containing extended
capabilities), but the underlying platform does not support extended
config space access (for example because it only supports
0xCF8/0xCFC, and not ECAM), then peeking at the extended config
space of the device may invoke undefined behavior within
PciSegmentLib, and the PciCapLib instance will be none the wiser.
(In fact, for the same reason, even PciCapPciSegmentLib has to
extend the "Segment:Bus:Device.Function" notation slightly; so that
such issues are caught *before* the call is made to PciSegmentLib.)
Q3: Wait, I thought it wasn't possible for a PCI Express device to
appear on a platform that lacked ECAM.
A3: Is that a question? :) Yes, it is plenty possible [2]. Same way as
garbage / looped config spaces are [3].
[2] https://github.com/tianocore/edk2/commit/014b472053ae3
[3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=02d578e5edd9
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Thanks,
Laszlo
Laszlo Ersek (7):
MdePkg: introduce PciCapLib
MdePkg: introduce PciCapPciSegmentLib
MdePkg: introduce PciCapPciIoLib
OvmfPkg: resolve PciCapLib, PciCapPciSegmentLib, PciCapPciIoLib
ArmVirtPkg: resolve PciCapLib, PciCapPciSegmentLib, PciCapPciIoLib
OvmfPkg/PciHotPlugInitDxe: convert to PciCapLib
OvmfPkg/Virtio10Dxe: convert to PciCapLib
ArmVirtPkg/ArmVirt.dsc.inc | 3 +
MdePkg/Include/Library/PciCapLib.h | 429 +++++++++
MdePkg/Include/Library/PciCapPciIoLib.h | 58 ++
MdePkg/Include/Library/PciCapPciSegmentLib.h | 82 ++
MdePkg/Library/BasePciCapLib/BasePciCapLib.c | 1007 ++++++++++++++++++++
MdePkg/Library/BasePciCapLib/BasePciCapLib.h | 60 ++
MdePkg/Library/BasePciCapLib/BasePciCapLib.inf | 37 +
MdePkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.c | 226 +++++
MdePkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.h | 47 +
MdePkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf | 34 +
MdePkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.c | 243 +++++
MdePkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.h | 44 +
MdePkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf | 35 +
MdePkg/MdePkg.dec | 14 +
MdePkg/MdePkg.dsc | 3 +
OvmfPkg/Include/IndustryStandard/Virtio10.h | 7 +-
OvmfPkg/OvmfPkgIa32.dsc | 3 +
OvmfPkg/OvmfPkgIa32X64.dsc | 3 +
OvmfPkg/OvmfPkgX64.dsc | 3 +
OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c | 267 ++----
OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf | 5 +
OvmfPkg/Virtio10Dxe/Virtio10.c | 135 +--
OvmfPkg/Virtio10Dxe/Virtio10.inf | 2 +
23 files changed, 2485 insertions(+), 262 deletions(-)
create mode 100644 MdePkg/Include/Library/PciCapLib.h
create mode 100644 MdePkg/Include/Library/PciCapPciIoLib.h
create mode 100644 MdePkg/Include/Library/PciCapPciSegmentLib.h
create mode 100644 MdePkg/Library/BasePciCapLib/BasePciCapLib.c
create mode 100644 MdePkg/Library/BasePciCapLib/BasePciCapLib.h
create mode 100644 MdePkg/Library/BasePciCapLib/BasePciCapLib.inf
create mode 100644 MdePkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.c
create mode 100644 MdePkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.h
create mode 100644 MdePkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
create mode 100644 MdePkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.c
create mode 100644 MdePkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.h
create mode 100644 MdePkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
--
2.14.1.3.gb7cf6e02401b
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/7] MdePkg: introduce PciCapLib
2018-05-04 21:36 [PATCH 0/7] MdePkg, OvmfPkg, ArmVirtPkg: add and use PCI(E) Capabilities Library Laszlo Ersek
@ 2018-05-04 21:36 ` Laszlo Ersek
2018-05-04 21:36 ` [PATCH 2/7] MdePkg: introduce PciCapPciSegmentLib Laszlo Ersek
` (6 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Laszlo Ersek @ 2018-05-04 21:36 UTC (permalink / raw)
To: edk2-devel-01; +Cc: Ard Biesheuvel, Jordan Justen, Liming Gao, Michael D Kinney
Add a library class, and a BASE lib instance, to work more easily with PCI
capabilities in PCI config space. Functions are provided to parse
capabilities lists, and to locate, describe, read and write capabilities.
PCI config space access is abstracted away.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Suggested-by: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
MdePkg/MdePkg.dec | 4 +
MdePkg/MdePkg.dsc | 1 +
MdePkg/Library/BasePciCapLib/BasePciCapLib.inf | 37 +
MdePkg/Include/Library/PciCapLib.h | 429 +++++++++
MdePkg/Library/BasePciCapLib/BasePciCapLib.h | 60 ++
MdePkg/Library/BasePciCapLib/BasePciCapLib.c | 1007 ++++++++++++++++++++
6 files changed, 1538 insertions(+)
diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index 0e64f22f4a74..e961d64ac89a 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -151,6 +151,10 @@ [LibraryClasses]
## @libraryclass Provides services to access PCI Configuration Space using the I/O ports 0xCF8 and 0xCFC.
PciCf8Lib|Include/Library/PciCf8Lib.h
+ ## @libraryclass Provides services to work with PCI capabilities in PCI
+ ## config space.
+ PciCapLib|Include/Library/PciCapLib.h
+
## @libraryclass Provides library services to get and set Platform Configuration Database entries.
PcdLib|Include/Library/PcdLib.h
diff --git a/MdePkg/MdePkg.dsc b/MdePkg/MdePkg.dsc
index 60efd722e9d7..9ec86b95ed4e 100644
--- a/MdePkg/MdePkg.dsc
+++ b/MdePkg/MdePkg.dsc
@@ -70,6 +70,7 @@ [Components]
MdePkg/Library/BasePciLibPciExpress/BasePciLibPciExpress.inf
MdePkg/Library/BasePciSegmentLibPci/BasePciSegmentLibPci.inf
MdePkg/Library/BasePciSegmentInfoLibNull/BasePciSegmentInfoLibNull.inf
+ MdePkg/Library/BasePciCapLib/BasePciCapLib.inf
MdePkg/Library/PciSegmentLibSegmentInfo/BasePciSegmentLibSegmentInfo.inf
MdePkg/Library/PciSegmentLibSegmentInfo/DxeRuntimePciSegmentLibSegmentInfo.inf
MdePkg/Library/BaseS3PciSegmentLib/BaseS3PciSegmentLib.inf
diff --git a/MdePkg/Library/BasePciCapLib/BasePciCapLib.inf b/MdePkg/Library/BasePciCapLib/BasePciCapLib.inf
new file mode 100644
index 000000000000..9979d32042ed
--- /dev/null
+++ b/MdePkg/Library/BasePciCapLib/BasePciCapLib.inf
@@ -0,0 +1,37 @@
+## @file
+# Work with PCI capabilities in PCI config space.
+#
+# Provides functions to parse capabilities lists, and to locate, describe, read
+# and write capabilities. PCI config space access is abstracted away.
+#
+# Copyright (C) 2018, Red Hat, Inc.
+#
+# 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 = 1.27
+ BASE_NAME = BasePciCapLib
+ FILE_GUID = 6957540D-F7B5-4D5B-BEE4-FC14114DCD3C
+ MODULE_TYPE = BASE
+ VERSION_STRING = 1.0
+ LIBRARY_CLASS = PciCapLib
+
+[Sources]
+ BasePciCapLib.h
+ BasePciCapLib.c
+
+[Packages]
+ MdePkg/MdePkg.dec
+
+[LibraryClasses]
+ BaseMemoryLib
+ DebugLib
+ MemoryAllocationLib
+ OrderedCollectionLib
diff --git a/MdePkg/Include/Library/PciCapLib.h b/MdePkg/Include/Library/PciCapLib.h
new file mode 100644
index 000000000000..22a1ad624bd3
--- /dev/null
+++ b/MdePkg/Include/Library/PciCapLib.h
@@ -0,0 +1,429 @@
+/** @file
+ Library class to work with PCI capabilities in PCI config space.
+
+ Provides functions to parse capabilities lists, and to locate, describe, read
+ and write capabilities. PCI config space access is abstracted away.
+
+ Copyright (C) 2018, Red Hat, Inc.
+
+ 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.
+**/
+
+#ifndef __PCI_CAP_LIB_H__
+#define __PCI_CAP_LIB_H__
+
+#include <Uefi/UefiBaseType.h>
+
+//
+// Base structure for representing a PCI device -- down to the PCI function
+// level -- for the purposes of this library class. This is a forward
+// declaration that is completed below. Concrete implementations are supposed
+// to inherit and extend this type.
+//
+typedef struct PCI_CAP_DEV PCI_CAP_DEV;
+
+/**
+ Read the config space of a given PCI device (both normal and extended).
+
+ PCI_CAP_DEV_READ_CONFIG performs as few config space accesses as possible
+ (without attempting 64-bit wide accesses).
+
+ PCI_CAP_DEV_READ_CONFIG returns an unspecified error if accessing Size bytes
+ from SourceOffset exceeds the config space limit of the PCI device. Fewer
+ than Size bytes may have been read in this case.
+
+ @param[in] PciDevice Implementation-specific unique representation
+ of the PCI device in the PCI hierarchy.
+
+ @param[in] SourceOffset Source offset in the config space of the PCI
+ device to start reading from.
+
+ @param[out] DestinationBuffer Buffer to store the read data to.
+
+ @param[in] Size The number of bytes to transfer.
+
+ @retval RETURN_SUCCESS Size bytes have been transferred from config space to
+ DestinationBuffer.
+
+ @return Unspecified error codes. Fewer than Size bytes may
+ have been read.
+**/
+typedef
+RETURN_STATUS
+(EFIAPI *PCI_CAP_DEV_READ_CONFIG) (
+ IN PCI_CAP_DEV *PciDevice,
+ IN UINT16 SourceOffset,
+ OUT VOID *DestinationBuffer,
+ IN UINT16 Size
+ );
+
+/**
+ Write the config space of a given PCI device (both normal and extended).
+
+ PCI_CAP_DEV_WRITE_CONFIG performs as few config space accesses as possible
+ (without attempting 64-bit wide accesses).
+
+ PCI_CAP_DEV_WRITE_CONFIG returns an unspecified error if accessing Size bytes
+ at DestinationOffset exceeds the config space limit of the PCI device. Fewer
+ than Size bytes may have been written in this case.
+
+ @param[in] PciDevice Implementation-specific unique representation
+ of the PCI device in the PCI hierarchy.
+
+ @param[in] DestinationOffset Destination offset in the config space of the
+ PCI device to start writing at.
+
+ @param[in] SourceBuffer Buffer to read the data to be stored from.
+
+ @param[in] Size The number of bytes to transfer.
+
+ @retval RETURN_SUCCESS Size bytes have been transferred from SourceBuffer to
+ config space.
+
+ @return Unspecified error codes. Fewer than Size bytes may
+ have been written.
+**/
+typedef
+RETURN_STATUS
+(EFIAPI *PCI_CAP_DEV_WRITE_CONFIG) (
+ IN PCI_CAP_DEV *PciDevice,
+ IN UINT16 DestinationOffset,
+ IN VOID *SourceBuffer,
+ IN UINT16 Size
+ );
+
+//
+// Complete the PCI_CAP_DEV type here. The base abstraction only requires
+// config space accessors.
+//
+struct PCI_CAP_DEV {
+ PCI_CAP_DEV_READ_CONFIG ReadConfig;
+ PCI_CAP_DEV_WRITE_CONFIG WriteConfig;
+};
+
+//
+// Opaque data structure representing parsed PCI Capabilities Lists.
+//
+typedef struct PCI_CAP_LIST PCI_CAP_LIST;
+
+//
+// Opaque data structure representing a PCI Capability in a parsed Capability
+// List.
+//
+typedef struct PCI_CAP PCI_CAP;
+
+//
+// Distinguishes whether a Capability ID is 8-bit wide and interpreted in
+// normal config space, or 16-bit wide and interpreted in extended config
+// space. Capability ID definitions are relative to domain.
+//
+typedef enum {
+ PciCapNormal,
+ PciCapExtended
+} PCI_CAP_DOMAIN;
+
+//
+// Public data structure that PciCapGetInfo() fills in about a PCI_CAP object.
+//
+typedef struct {
+ PCI_CAP_DOMAIN Domain;
+ UINT16 CapId;
+ //
+ // The capability identified by Domain and CapId may have multiple instances
+ // in config space. NumInstances provides the total count of occurrences of
+ // the capability. It is always positive.
+ //
+ UINT16 NumInstances;
+ //
+ // Instance is the serial number, in capabilities list traversal order (not
+ // necessarily config space offset order), of the one capability instance
+ // that PciCapGetInfo() is reporting about. Instance is always smaller than
+ // NumInstances.
+ //
+ UINT16 Instance;
+ //
+ // The offset in config space at which the capability header of the
+ // capability instance starts.
+ //
+ UINT16 Offset;
+ //
+ // The deduced maximum size of the capability instance, including the
+ // capability header. This hint is an upper bound, calculated -- without
+ // regard to the internal structure of the capability -- from (a) the next
+ // lowest offset in configuration space that is known to be used by another
+ // capability, and (b) from the end of the config space identified by Domain,
+ // whichever is lower.
+ //
+ UINT16 MaxSizeHint;
+ //
+ // The version number of the capability instance. Always zero when Domain is
+ // PciCapNormal.
+ //
+ UINT8 Version;
+} PCI_CAP_INFO;
+
+
+/**
+ Parse the capabilities lists (both normal and extended, as applicable) of a
+ PCI device.
+
+ If the PCI device has no capabilities, that per se will not fail
+ PciCapListInit(); an empty capabilities list will be represented.
+
+ If the PCI device is found to be PCI Express, then an attempt will be made to
+ parse the extended capabilities list as well. If the first extended config
+ space access -- via PciDevice->ReadConfig() with SourceOffset=0x100 and
+ Size=4 -- fails, that per se will not fail PciCapListInit(); the device will
+ be assumed to have no extended capabilities.
+
+ @param[in] PciDevice Implementation-specific unique representation of the
+ PCI device in the PCI hierarchy.
+
+ @param[out] CapList Opaque data structure that holds an in-memory
+ representation of the parsed capabilities lists of
+ PciDevice.
+
+ @retval RETURN_SUCCESS The capabilities lists have been parsed from
+ config space.
+
+ @retval RETURN_OUT_OF_RESOURCES Memory allocation failed.
+
+ @retval RETURN_DEVICE_ERROR A loop or some other kind of invalid pointer
+ was detected in the capabilities lists of
+ PciDevice.
+
+ @return Error codes propagated from
+ PciDevice->ReadConfig().
+**/
+RETURN_STATUS
+EFIAPI
+PciCapListInit (
+ IN PCI_CAP_DEV *PciDevice,
+ OUT PCI_CAP_LIST **CapList
+ );
+
+
+/**
+ Free the resources used by CapList.
+
+ @param[in] CapList The PCI_CAP_LIST object to free, originally produced by
+ PciCapListInit().
+**/
+VOID
+EFIAPI
+PciCapListUninit (
+ IN PCI_CAP_LIST *CapList
+ );
+
+
+/**
+ Locate a capability instance in the parsed capabilities lists.
+
+ @param[in] CapList The PCI_CAP_LIST object produced by PciCapListInit().
+
+ @param[in] Domain Distinguishes whether CapId is 8-bit wide and
+ interpreted in normal config space, or 16-bit wide and
+ interpreted in extended config space. Capability ID
+ definitions are relative to domain.
+
+ @param[in] CapId Capability identifier to look up.
+
+ @param[in] Instance Domain and CapId may identify a multi-instance
+ capability. When Instance is zero, the first instance of
+ the capability is located (in list traversal order --
+ which may not mean increasing config space offset
+ order). Higher Instance values locate subsequent
+ instances of the same capability (in list traversal
+ order).
+
+ @param[out] Cap The capability instance that matches the search
+ criteria. Cap is owned by CapList and becomes invalid
+ when CapList is freed with PciCapListUninit().
+ PciCapListFindCap() may be called with Cap set to NULL,
+ in order to test the existence of a specific capability
+ instance.
+
+ @retval RETURN_SUCCESS The capability instance identified by (Domain,
+ CapId, Instance) has been found.
+
+ @retval RETURN_NOT_FOUND The requested (Domain, CapId, Instance) capability
+ instance does not exist.
+**/
+RETURN_STATUS
+EFIAPI
+PciCapListFindCap (
+ IN PCI_CAP_LIST *CapList,
+ IN PCI_CAP_DOMAIN Domain,
+ IN UINT16 CapId,
+ IN UINT16 Instance,
+ OUT PCI_CAP **Cap OPTIONAL
+ );
+
+
+/**
+ Locate the first instance of the capability given by (Domain, CapId) such
+ that the instance's Version is greater than or equal to MinVersion.
+
+ This is a convenience function that may save client code calls to
+ PciCapListFindCap() and PciCapGetInfo().
+
+ @param[in] CapList The PCI_CAP_LIST object produced by PciCapListInit().
+
+ @param[in] Domain Distinguishes whether CapId is 8-bit wide and
+ interpreted in normal config space, or 16-bit wide and
+ interpreted in extended config space. Capability ID
+ definitions are relative to domain.
+
+ @param[in] CapId Capability identifier to look up.
+
+ @param[in] MinVersion The minimum version that the capability instance is
+ required to have. Note that all capability instances
+ in Domain=PciCapNormal have Version=0.
+
+ @param[out] Cap The first capability instance that matches the search
+ criteria. Cap is owned by CapList and becomes invalid
+ when CapList is freed with PciCapListUninit().
+ PciCapListFindCapVersion() may be called with Cap set
+ to NULL, in order just to test whether the search
+ criteria are satisfiable.
+
+ @retval RETURN_SUCCESS The first capability instance matching (Domain,
+ CapId, MinVersion) has been located.
+
+ @retval RETURN_NOT_FOUND No capability instance matches (Domain, CapId,
+ MinVersion).
+**/
+RETURN_STATUS
+EFIAPI
+PciCapListFindCapVersion (
+ IN PCI_CAP_LIST *CapList,
+ IN PCI_CAP_DOMAIN Domain,
+ IN UINT16 CapId,
+ IN UINT8 MinVersion,
+ OUT PCI_CAP **Cap OPTIONAL
+ );
+
+
+/**
+ Get information about a PCI Capability instance.
+
+ @param[in] Cap The capability instance to get info about, located with
+ PciCapListFindCap*().
+
+ @param[out] Info A PCI_CAP_INFO structure that describes the properties of
+ Cap.
+
+ @retval RETURN_SUCCESS Fields of Info have been set.
+
+ @return Unspecified error codes, if filling in Info failed
+ for some reason.
+**/
+RETURN_STATUS
+EFIAPI
+PciCapGetInfo (
+ IN PCI_CAP *Cap,
+ OUT PCI_CAP_INFO *Info
+ );
+
+
+/**
+ Read a slice of a capability instance.
+
+ The function performs as few config space accesses as possible (without
+ attempting 64-bit wide accesses). PciCapRead() performs bounds checking on
+ SourceOffsetInCap and Size, and only invokes PciDevice->ReadConfig() if the
+ requested transfer falls within Cap.
+
+ @param[in] PciDevice Implementation-specific unique representation
+ of the PCI device in the PCI hierarchy.
+
+ @param[in] Cap The capability instance to read, located with
+ PciCapListFindCap*().
+
+ @param[in] SourceOffsetInCap Source offset relative to the capability
+ header to start reading from. A zero value
+ refers to the first byte of the capability
+ header.
+
+ @param[out] DestinationBuffer Buffer to store the read data to.
+
+ @param[in] Size The number of bytes to transfer.
+
+ @retval RETURN_SUCCESS Size bytes have been transferred from Cap to
+ DestinationBuffer.
+
+ @retval RETURN_BAD_BUFFER_SIZE Reading Size bytes starting from
+ SourceOffsetInCap would not (entirely) be
+ contained within Cap, as suggested by
+ PCI_CAP_INFO.MaxSizeHint. No bytes have been
+ read.
+
+ @return Error codes propagated from
+ PciDevice->ReadConfig(). Fewer than Size
+ bytes may have been read.
+**/
+RETURN_STATUS
+EFIAPI
+PciCapRead (
+ IN PCI_CAP_DEV *PciDevice,
+ IN PCI_CAP *Cap,
+ IN UINT16 SourceOffsetInCap,
+ OUT VOID *DestinationBuffer,
+ IN UINT16 Size
+ );
+
+
+/**
+ Write a slice of a capability instance.
+
+ The function performs as few config space accesses as possible (without
+ attempting 64-bit wide accesses). PciCapWrite() performs bounds checking on
+ DestinationOffsetInCap and Size, and only invokes PciDevice->WriteConfig() if
+ the requested transfer falls within Cap.
+
+ @param[in] PciDevice Implementation-specific unique
+ representation of the PCI device in the
+ PCI hierarchy.
+
+ @param[in] Cap The capability instance to write, located
+ with PciCapListFindCap*().
+
+ @param[in] DestinationOffsetInCap Destination offset relative to the
+ capability header to start writing at. A
+ zero value refers to the first byte of the
+ capability header.
+
+ @param[in] SourceBuffer Buffer to read the data to be stored from.
+
+ @param[in] Size The number of bytes to transfer.
+
+ @retval RETURN_SUCCESS Size bytes have been transferred from
+ SourceBuffer to Cap.
+
+ @retval RETURN_BAD_BUFFER_SIZE Writing Size bytes starting at
+ DestinationOffsetInCap would not (entirely)
+ be contained within Cap, as suggested by
+ PCI_CAP_INFO.MaxSizeHint. No bytes have been
+ written.
+
+ @return Error codes propagated from
+ PciDevice->WriteConfig(). Fewer than Size
+ bytes may have been written.
+**/
+RETURN_STATUS
+EFIAPI
+PciCapWrite (
+ IN PCI_CAP_DEV *PciDevice,
+ IN PCI_CAP *Cap,
+ IN UINT16 DestinationOffsetInCap,
+ IN VOID *SourceBuffer,
+ IN UINT16 Size
+ );
+
+#endif // __PCI_CAP_LIB_H__
diff --git a/MdePkg/Library/BasePciCapLib/BasePciCapLib.h b/MdePkg/Library/BasePciCapLib/BasePciCapLib.h
new file mode 100644
index 000000000000..e631745834d9
--- /dev/null
+++ b/MdePkg/Library/BasePciCapLib/BasePciCapLib.h
@@ -0,0 +1,60 @@
+/** @file
+ Work with PCI capabilities in PCI config space -- internal type definitions.
+
+ Copyright (C) 2018, Red Hat, Inc.
+
+ 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.
+**/
+
+#ifndef __BASE_PCI_CAP_LIB_H__
+#define __BASE_PCI_CAP_LIB_H__
+
+#include <Library/OrderedCollectionLib.h>
+
+#include <Library/PciCapLib.h>
+
+//
+// Structure that uniquely identifies a capability instance and serves as key
+// for insertion and lookup.
+//
+typedef struct {
+ PCI_CAP_DOMAIN Domain;
+ UINT16 CapId;
+ UINT16 Instance;
+} PCI_CAP_KEY;
+
+//
+// In Instance==0 PCI_CAP objects, store NumInstances directly. In Instance>0
+// PCI_CAP objects, link Instance#0 of the same (Domain, CapId). This way
+// NumInstances needs maintenance in one object only, per (Domain, CapId) pair.
+//
+typedef union {
+ UINT16 NumInstances;
+ PCI_CAP *InstanceZero;
+} PCI_CAP_NUM_INSTANCES;
+
+//
+// Complete the incomplete PCI_CAP structure here.
+//
+struct PCI_CAP {
+ PCI_CAP_KEY Key;
+ PCI_CAP_NUM_INSTANCES NumInstancesUnion;
+ UINT16 Offset;
+ UINT16 MaxSizeHint;
+ UINT8 Version;
+};
+
+//
+// Complete the incomplete PCI_CAP_LIST structure here.
+//
+struct PCI_CAP_LIST {
+ ORDERED_COLLECTION *Capabilities;
+};
+
+#endif // __BASE_PCI_CAP_LIB_H__
diff --git a/MdePkg/Library/BasePciCapLib/BasePciCapLib.c b/MdePkg/Library/BasePciCapLib/BasePciCapLib.c
new file mode 100644
index 000000000000..6789359f0a54
--- /dev/null
+++ b/MdePkg/Library/BasePciCapLib/BasePciCapLib.c
@@ -0,0 +1,1007 @@
+/** @file
+ Work with PCI capabilities in PCI config space.
+
+ Provides functions to parse capabilities lists, and to locate, describe, read
+ and write capabilities. PCI config space access is abstracted away.
+
+ Copyright (C) 2018, Red Hat, Inc.
+
+ 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.
+**/
+
+#include <IndustryStandard/PciExpress21.h>
+
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/MemoryAllocationLib.h>
+
+#include "BasePciCapLib.h"
+
+
+/**
+ Compare a standalone PCI_CAP_KEY against a PCI_CAP containing an embedded
+ PCI_CAP_KEY.
+
+ @param[in] PciCapKey Pointer to the bare PCI_CAP_KEY.
+
+ @param[in] PciCap Pointer to the PCI_CAP with the embedded PCI_CAP_KEY.
+
+ @retval <0 If PciCapKey compares less than PciCap->Key.
+
+ @retval 0 If PciCapKey compares equal to PciCap->Key.
+
+ @retval >0 If PciCapKey compares greater than PciCap->Key.
+**/
+STATIC
+INTN
+EFIAPI
+ComparePciCapKey (
+ IN CONST VOID *PciCapKey,
+ IN CONST VOID *PciCap
+ )
+{
+ CONST PCI_CAP_KEY *Key1;
+ CONST PCI_CAP_KEY *Key2;
+
+ Key1 = PciCapKey;
+ Key2 = &((CONST PCI_CAP *)PciCap)->Key;
+
+ if (Key1->Domain < Key2->Domain) {
+ return -1;
+ }
+ if (Key1->Domain > Key2->Domain) {
+ return 1;
+ }
+ if (Key1->CapId < Key2->CapId) {
+ return -1;
+ }
+ if (Key1->CapId > Key2->CapId) {
+ return 1;
+ }
+ if (Key1->Instance < Key2->Instance) {
+ return -1;
+ }
+ if (Key1->Instance > Key2->Instance) {
+ return 1;
+ }
+ return 0;
+}
+
+
+/**
+ Compare two PCI_CAP objects based on PCI_CAP.Key.
+
+ @param[in] PciCap1 Pointer to the first PCI_CAP.
+
+ @param[in] PciCap2 Pointer to the second PCI_CAP.
+
+ @retval <0 If PciCap1 compares less than PciCap2.
+
+ @retval 0 If PciCap1 compares equal to PciCap2.
+
+ @retval >0 If PciCap1 compares greater than PciCap2.
+**/
+STATIC
+INTN
+EFIAPI
+ComparePciCap (
+ IN CONST VOID *PciCap1,
+ IN CONST VOID *PciCap2
+ )
+{
+ CONST PCI_CAP_KEY *PciCap1Key;
+
+ PciCap1Key = &((CONST PCI_CAP *)PciCap1)->Key;
+ return ComparePciCapKey (PciCap1Key, PciCap2);
+}
+
+
+/**
+ Compare the standalone UINT16 config space offset of a capability header
+ against a PCI_CAP containing an embedded Offset.
+
+ @param[in] CapHdrOffset Pointer to the bare UINT16 config space offset.
+
+ @param[in] PciCap Pointer to the PCI_CAP with the embedded Offset.
+
+ @retval <0 If CapHdrOffset compares less than PciCap->Offset.
+
+ @retval 0 If CapHdrOffset compares equal to PciCap->Offset.
+
+ @retval >0 If CapHdrOffset compares greater than PciCap->Offset.
+**/
+STATIC
+INTN
+EFIAPI
+ComparePciCapOffsetKey (
+ IN CONST VOID *CapHdrOffset,
+ IN CONST VOID *PciCap
+ )
+{
+ UINT16 Offset1;
+ UINT16 Offset2;
+
+ Offset1 = *(CONST UINT16 *)CapHdrOffset;
+ Offset2 = ((CONST PCI_CAP *)PciCap)->Offset;
+ //
+ // Note: both Offset1 and Offset2 are promoted to INT32 below, and the
+ // subtraction takes place between INT32 values.
+ //
+ return Offset1 - Offset2;
+}
+
+
+/**
+ Compare two PCI_CAP objects based on PCI_CAP.Offset.
+
+ @param[in] PciCap1 Pointer to the first PCI_CAP.
+
+ @param[in] PciCap2 Pointer to the second PCI_CAP.
+
+ @retval <0 If PciCap1 compares less than PciCap2.
+
+ @retval 0 If PciCap1 compares equal to PciCap2.
+
+ @retval >0 If PciCap1 compares greater than PciCap2.
+**/
+STATIC
+INTN
+EFIAPI
+ComparePciCapOffset (
+ IN CONST VOID *PciCap1,
+ IN CONST VOID *PciCap2
+ )
+{
+ UINT16 Offset1;
+ UINT16 Offset2;
+
+ Offset1 = ((CONST PCI_CAP *)PciCap1)->Offset;
+ Offset2 = ((CONST PCI_CAP *)PciCap2)->Offset;
+ //
+ // Note: both Offset1 and Offset2 are promoted to INT32 below, and the
+ // subtraction takes place between INT32 values.
+ //
+ return Offset1 - Offset2;
+}
+
+
+/**
+ Insert a new instance of the PCI capability given by (Domain, CapId) in
+ CapList.
+
+ @param[in,out] CapList The PCI_CAP_LIST into which the new PCI_CAP
+ should be inserted. CapList will own the new
+ PCI_CAP structure.
+
+ @param[in,out] CapHdrOffsets Link the new PCI_CAP structure into the
+ (non-owning) CapHdrOffsets collection as well.
+ CapHdrOffsets orders the PCI_CAP structures
+ based on the PCI_CAP.Offset member, and enables
+ the calculation of PCI_CAP.MaxSizeHint.
+
+ @param[in] Domain Whether the capability is normal or extended.
+
+ @param[in] CapId Capability ID (specific to Domain).
+
+ @param[in] Offset Config space offset at which the standard
+ header of the capability starts. The caller is
+ responsible for ensuring that Offset be DWORD
+ aligned. The caller is also responsible for
+ ensuring that Offset be within the config space
+ identified by Domain.
+
+ @param[in] Version The version number of the capability. The
+ caller is responsible for passing 0 as Version
+ if Domain is PciCapNormal.
+
+ @retval RETURN_SUCCESS Insertion successful.
+
+ @retval RETURN_OUT_OF_RESOURCES Memory allocation failed.
+
+ @retval RETURN_DEVICE_ERROR A PCI_CAP with Offset is already linked by
+ CapHdrOffsets. This indicates a loop in the
+ capabilities list being parsed.
+**/
+STATIC
+RETURN_STATUS
+InsertPciCap (
+ IN OUT PCI_CAP_LIST *CapList,
+ IN OUT ORDERED_COLLECTION *CapHdrOffsets,
+ IN PCI_CAP_DOMAIN Domain,
+ IN UINT16 CapId,
+ IN UINT16 Offset,
+ IN UINT8 Version
+ )
+{
+ PCI_CAP *PciCap;
+ RETURN_STATUS Status;
+ ORDERED_COLLECTION_ENTRY *PciCapEntry;
+ PCI_CAP *InstanceZero;
+
+ ASSERT ((Offset & 0x3) == 0);
+ ASSERT (Offset < (Domain == PciCapNormal ?
+ PCI_MAX_CONFIG_OFFSET : PCI_EXP_MAX_CONFIG_OFFSET));
+ ASSERT (Domain == PciCapExtended || Version == 0);
+
+ //
+ // Set InstanceZero to suppress incorrect compiler/analyzer warnings.
+ //
+ InstanceZero = NULL;
+
+ //
+ // Allocate PciCap, and populate it assuming it is the first occurrence of
+ // (Domain, CapId). Note that PciCap->MaxSizeHint is not assigned the final
+ // value just yet.
+ //
+ PciCap = AllocatePool (sizeof *PciCap);
+ if (PciCap == NULL) {
+ return RETURN_OUT_OF_RESOURCES;
+ }
+ PciCap->Key.Domain = Domain;
+ PciCap->Key.CapId = CapId;
+ PciCap->Key.Instance = 0;
+ PciCap->NumInstancesUnion.NumInstances = 1;
+ PciCap->Offset = Offset;
+ PciCap->MaxSizeHint = 0;
+ PciCap->Version = Version;
+
+ //
+ // Add PciCap to CapList.
+ //
+ Status = OrderedCollectionInsert (CapList->Capabilities, &PciCapEntry,
+ PciCap);
+ if (RETURN_ERROR (Status)) {
+ if (Status == RETURN_OUT_OF_RESOURCES) {
+ goto FreePciCap;
+ }
+ ASSERT (Status == RETURN_ALREADY_STARTED);
+ //
+ // PciCap is not the first instance of (Domain, CapId). Add it as a new
+ // instance, taking the current instance count from Instance#0. Note that
+ // we don't bump the instance count maintained in Instance#0 just yet, to
+ // keep rollback on errors simple.
+ //
+ InstanceZero = OrderedCollectionUserStruct (PciCapEntry);
+ PciCap->Key.Instance = InstanceZero->NumInstancesUnion.NumInstances;
+ PciCap->NumInstancesUnion.InstanceZero = InstanceZero;
+
+ ASSERT (PciCap->Key.Instance > 0);
+ Status = OrderedCollectionInsert (CapList->Capabilities, &PciCapEntry,
+ PciCap);
+ if (Status == RETURN_OUT_OF_RESOURCES) {
+ goto FreePciCap;
+ }
+ }
+ //
+ // At this point, PciCap has been inserted in CapList->Capabilities, either
+ // with Instance==0 or with Instance>0. PciCapEntry is the iterator that
+ // links PciCap.
+ //
+ ASSERT_RETURN_ERROR (Status);
+
+ //
+ // Link PciCap into CapHdrOffsets too, to order it globally based on config
+ // space offset. Note that partial overlaps between capability headers is not
+ // possible: Offset is DWORD aligned, normal capability headers are 16-bit
+ // wide, and extended capability headers are 32-bit wide. Therefore any two
+ // capability headers either are distinct or start at the same offset
+ // (implying a loop in the respective capabilities list).
+ //
+ Status = OrderedCollectionInsert (CapHdrOffsets, NULL, PciCap);
+ if (RETURN_ERROR (Status)) {
+ if (Status == RETURN_ALREADY_STARTED) {
+ //
+ // Loop found; map return status accordingly.
+ //
+ Status = RETURN_DEVICE_ERROR;
+ }
+ goto DeletePciCapFromCapList;
+ }
+
+ //
+ // Now we can bump the instance count maintained in Instance#0, if PciCap is
+ // not the first instance of (Domain, CapId).
+ //
+ if (PciCap->Key.Instance > 0) {
+ InstanceZero->NumInstancesUnion.NumInstances++;
+ }
+ return RETURN_SUCCESS;
+
+DeletePciCapFromCapList:
+ OrderedCollectionDelete (CapList->Capabilities, PciCapEntry, NULL);
+
+FreePciCap:
+ FreePool (PciCap);
+
+ return Status;
+}
+
+
+/**
+ Calculate the MaxSizeHint member for a PCI_CAP object.
+
+ CalculatePciCapMaxSizeHint() may only be called once all capability instances
+ have been successfully processed by InsertPciCap().
+
+ @param[in,out] PciCap The PCI_CAP object for which to calculate the
+ MaxSizeHint member. The caller is responsible for
+ passing a PCI_CAP object that has been created by a
+ successful invocation of InsertPciCap().
+
+ @param[in] NextPciCap If NextPciCap is NULL, then the caller is responsible
+ for PciCap to represent the capability instance with
+ the highest header offset in all config space. If
+ NextPciCap is not NULL, then the caller is responsible
+ for (a) having created NextPciCap with a successful
+ invocation of InsertPciCap(), and (b) NextPciCap being
+ the direct successor of PciCap in config space offset
+ order, as ordered by ComparePciCapOffset().
+**/
+STATIC
+VOID
+CalculatePciCapMaxSizeHint (
+ IN OUT PCI_CAP *PciCap,
+ IN PCI_CAP *NextPciCap OPTIONAL
+ )
+{
+ UINT16 ConfigSpaceSize;
+
+ ConfigSpaceSize = (PciCap->Key.Domain == PciCapNormal ?
+ PCI_MAX_CONFIG_OFFSET : PCI_EXP_MAX_CONFIG_OFFSET);
+ //
+ // The following is guaranteed by the interface contract on
+ // CalculatePciCapMaxSizeHint().
+ //
+ ASSERT (NextPciCap == NULL || PciCap->Offset < NextPciCap->Offset);
+ //
+ // The following is guaranteed by the interface contract on InsertPciCap().
+ //
+ ASSERT (PciCap->Offset < ConfigSpaceSize);
+ //
+ // Thus we can safely subtract PciCap->Offset from either of
+ // - ConfigSpaceSize
+ // - and NextPciCap->Offset (if NextPciCap is not NULL).
+ //
+ // PciCap extends from PciCap->Offset to NextPciCap->Offset (if any), except
+ // it cannot cross config space boundary.
+ //
+ if (NextPciCap == NULL || NextPciCap->Offset >= ConfigSpaceSize) {
+ PciCap->MaxSizeHint = ConfigSpaceSize - PciCap->Offset;
+ return;
+ }
+ PciCap->MaxSizeHint = NextPciCap->Offset - PciCap->Offset;
+}
+
+
+/**
+ Debug dump a PCI_CAP_LIST object at the DEBUG_VERBOSE level.
+
+ @param[in] CapList The PCI_CAP_LIST object to dump.
+**/
+STATIC
+VOID
+EFIAPI
+DebugDumpPciCapList (
+ IN PCI_CAP_LIST *CapList
+ )
+{
+ DEBUG_CODE_BEGIN ();
+ ORDERED_COLLECTION_ENTRY *PciCapEntry;
+
+ for (PciCapEntry = OrderedCollectionMin (CapList->Capabilities);
+ PciCapEntry != NULL;
+ PciCapEntry = OrderedCollectionNext (PciCapEntry)) {
+ PCI_CAP *PciCap;
+ RETURN_STATUS Status;
+ PCI_CAP_INFO Info;
+
+ PciCap = OrderedCollectionUserStruct (PciCapEntry);
+ Status = PciCapGetInfo (PciCap, &Info);
+ //
+ // PciCapGetInfo() cannot fail in this library instance.
+ //
+ ASSERT_RETURN_ERROR (Status);
+
+ DEBUG ((DEBUG_VERBOSE,
+ "%a:%a: %a 0x%04x %03u/%03u v0x%x @0x%03x+0x%03x\n", gEfiCallerBaseName,
+ __FUNCTION__, (Info.Domain == PciCapNormal ? "Norm" : "Extd"),
+ Info.CapId, Info.Instance, Info.NumInstances, Info.Version, Info.Offset,
+ Info.MaxSizeHint));
+ }
+ DEBUG_CODE_END ();
+}
+
+
+/**
+ Empty a collection of PCI_CAP structures, optionally releasing the referenced
+ PCI_CAP structures themselves. Release the collection at last.
+
+ @param[in,out] PciCapCollection The collection to empty and release.
+
+ @param[in] FreePciCap TRUE if the PCI_CAP structures linked by
+ PciCapCollection should be released. When
+ FALSE, the caller is responsible for
+ retaining at least one reference to each
+ PCI_CAP structure originally linked by
+ PciCapCollection.
+**/
+STATIC
+VOID
+EmptyAndUninitPciCapCollection (
+ IN OUT ORDERED_COLLECTION *PciCapCollection,
+ IN BOOLEAN FreePciCap
+ )
+{
+ ORDERED_COLLECTION_ENTRY *PciCapEntry;
+ ORDERED_COLLECTION_ENTRY *NextEntry;
+
+ for (PciCapEntry = OrderedCollectionMin (PciCapCollection);
+ PciCapEntry != NULL;
+ PciCapEntry = NextEntry) {
+ PCI_CAP *PciCap;
+
+ NextEntry = OrderedCollectionNext (PciCapEntry);
+ OrderedCollectionDelete (PciCapCollection, PciCapEntry, (VOID **)&PciCap);
+ if (FreePciCap) {
+ FreePool (PciCap);
+ }
+ }
+ OrderedCollectionUninit (PciCapCollection);
+}
+
+
+/**
+ Parse the capabilities lists (both normal and extended, as applicable) of a
+ PCI device.
+
+ If the PCI device has no capabilities, that per se will not fail
+ PciCapListInit(); an empty capabilities list will be represented.
+
+ If the PCI device is found to be PCI Express, then an attempt will be made to
+ parse the extended capabilities list as well. If the first extended config
+ space access -- via PciDevice->ReadConfig() with SourceOffset=0x100 and
+ Size=4 -- fails, that per se will not fail PciCapListInit(); the device will
+ be assumed to have no extended capabilities.
+
+ @param[in] PciDevice Implementation-specific unique representation of the
+ PCI device in the PCI hierarchy.
+
+ @param[out] CapList Opaque data structure that holds an in-memory
+ representation of the parsed capabilities lists of
+ PciDevice.
+
+ @retval RETURN_SUCCESS The capabilities lists have been parsed from
+ config space.
+
+ @retval RETURN_OUT_OF_RESOURCES Memory allocation failed.
+
+ @retval RETURN_DEVICE_ERROR A loop or some other kind of invalid pointer
+ was detected in the capabilities lists of
+ PciDevice.
+
+ @return Error codes propagated from
+ PciDevice->ReadConfig().
+**/
+RETURN_STATUS
+EFIAPI
+PciCapListInit (
+ IN PCI_CAP_DEV *PciDevice,
+ OUT PCI_CAP_LIST **CapList
+ )
+{
+ PCI_CAP_LIST *OutCapList;
+ RETURN_STATUS Status;
+ ORDERED_COLLECTION *CapHdrOffsets;
+ UINT16 PciStatusReg;
+ BOOLEAN DeviceIsExpress;
+ ORDERED_COLLECTION_ENTRY *OffsetEntry;
+
+ //
+ // Allocate the output structure.
+ //
+ OutCapList = AllocatePool (sizeof *OutCapList);
+ if (OutCapList == NULL) {
+ return RETURN_OUT_OF_RESOURCES;
+ }
+ //
+ // The OutCapList->Capabilities collection owns the PCI_CAP structures and
+ // orders them based on PCI_CAP.Key.
+ //
+ OutCapList->Capabilities = OrderedCollectionInit (ComparePciCap,
+ ComparePciCapKey);
+ if (OutCapList->Capabilities == NULL) {
+ Status = RETURN_OUT_OF_RESOURCES;
+ goto FreeOutCapList;
+ }
+
+ //
+ // The (temporary) CapHdrOffsets collection only references PCI_CAP
+ // structures, and orders them based on PCI_CAP.Offset.
+ //
+ CapHdrOffsets = OrderedCollectionInit (ComparePciCapOffset,
+ ComparePciCapOffsetKey);
+ if (CapHdrOffsets == NULL) {
+ Status = RETURN_OUT_OF_RESOURCES;
+ goto FreeCapabilities;
+ }
+
+ //
+ // Whether the device is PCI Express depends on the normal capability with
+ // identifier EFI_PCI_CAPABILITY_ID_PCIEXP.
+ //
+ DeviceIsExpress = FALSE;
+
+ //
+ // Check whether a normal capabilities list is present. If there's none,
+ // that's not an error; we'll just return OutCapList->Capabilities empty.
+ //
+ Status = PciDevice->ReadConfig (PciDevice, PCI_PRIMARY_STATUS_OFFSET,
+ &PciStatusReg, sizeof PciStatusReg);
+ if (RETURN_ERROR (Status)) {
+ goto FreeCapHdrOffsets;
+ }
+ if ((PciStatusReg & EFI_PCI_STATUS_CAPABILITY) != 0) {
+ UINT8 NormalCapHdrOffset;
+
+ //
+ // Fetch the start offset of the normal capabilities list.
+ //
+ Status = PciDevice->ReadConfig (PciDevice, PCI_CAPBILITY_POINTER_OFFSET,
+ &NormalCapHdrOffset, sizeof NormalCapHdrOffset);
+ if (RETURN_ERROR (Status)) {
+ goto FreeCapHdrOffsets;
+ }
+
+ //
+ // Traverse the normal capabilities list.
+ //
+ NormalCapHdrOffset &= 0xFC;
+ while (NormalCapHdrOffset > 0) {
+ EFI_PCI_CAPABILITY_HDR NormalCapHdr;
+
+ Status = PciDevice->ReadConfig (PciDevice, NormalCapHdrOffset,
+ &NormalCapHdr, sizeof NormalCapHdr);
+ if (RETURN_ERROR (Status)) {
+ goto FreeCapHdrOffsets;
+ }
+
+ Status = InsertPciCap (OutCapList, CapHdrOffsets, PciCapNormal,
+ NormalCapHdr.CapabilityID, NormalCapHdrOffset, 0);
+ if (RETURN_ERROR (Status)) {
+ goto FreeCapHdrOffsets;
+ }
+
+ if (NormalCapHdr.CapabilityID == EFI_PCI_CAPABILITY_ID_PCIEXP) {
+ DeviceIsExpress = TRUE;
+ }
+ NormalCapHdrOffset = NormalCapHdr.NextItemPtr & 0xFC;
+ }
+ }
+
+ //
+ // If the device has been found PCI Express, attempt to traverse the extended
+ // capabilities list. It starts right after the normal config space.
+ //
+ if (DeviceIsExpress) {
+ UINT16 ExtendedCapHdrOffset;
+
+ ExtendedCapHdrOffset = PCI_MAX_CONFIG_OFFSET;
+ while (ExtendedCapHdrOffset > 0) {
+ PCI_EXPRESS_EXTENDED_CAPABILITIES_HEADER ExtendedCapHdr;
+
+ Status = PciDevice->ReadConfig (PciDevice, ExtendedCapHdrOffset,
+ &ExtendedCapHdr, sizeof ExtendedCapHdr);
+ //
+ // If the first extended config space access fails, assume the device has
+ // no extended capabilities. If the first extended config space access
+ // succeeds but we read an "all bits zero" extended capability header,
+ // that means (by spec) the device has no extended capabilities.
+ //
+ if (ExtendedCapHdrOffset == PCI_MAX_CONFIG_OFFSET &&
+ (RETURN_ERROR (Status) ||
+ IsZeroBuffer (&ExtendedCapHdr, sizeof ExtendedCapHdr))) {
+ break;
+ }
+ if (RETURN_ERROR (Status)) {
+ goto FreeCapHdrOffsets;
+ }
+
+ Status = InsertPciCap (OutCapList, CapHdrOffsets, PciCapExtended,
+ ExtendedCapHdr.CapabilityId, ExtendedCapHdrOffset,
+ ExtendedCapHdr.CapabilityVersion);
+ if (RETURN_ERROR (Status)) {
+ goto FreeCapHdrOffsets;
+ }
+
+ ExtendedCapHdrOffset = ExtendedCapHdr.NextCapabilityOffset & 0xFFC;
+ if (ExtendedCapHdrOffset > 0 &&
+ ExtendedCapHdrOffset < PCI_MAX_CONFIG_OFFSET) {
+ //
+ // Invalid capability pointer.
+ //
+ Status = RETURN_DEVICE_ERROR;
+ goto FreeCapHdrOffsets;
+ }
+ }
+ }
+
+ //
+ // Both capabilities lists have been parsed; compute the PCI_CAP.MaxSizeHint
+ // members if at least one capability has been found. In parallel, evacuate
+ // the CapHdrOffsets collection.
+ //
+ // At first, set OffsetEntry to the iterator of the PCI_CAP object with the
+ // lowest Offset (if such exists).
+ //
+ OffsetEntry = OrderedCollectionMin (CapHdrOffsets);
+ if (OffsetEntry != NULL) {
+ ORDERED_COLLECTION_ENTRY *NextOffsetEntry;
+ PCI_CAP *PciCap;
+
+ //
+ // Initialize NextOffsetEntry to the iterator of the PCI_CAP object with
+ // the second lowest Offset (if such exists).
+ //
+ NextOffsetEntry = OrderedCollectionNext (OffsetEntry);
+ //
+ // Calculate MaxSizeHint for all PCI_CAP objects except the one with the
+ // highest Offset.
+ //
+ while (NextOffsetEntry != NULL) {
+ PCI_CAP *NextPciCap;
+
+ OrderedCollectionDelete (CapHdrOffsets, OffsetEntry, (VOID **)&PciCap);
+ NextPciCap = OrderedCollectionUserStruct (NextOffsetEntry);
+ CalculatePciCapMaxSizeHint (PciCap, NextPciCap);
+
+ OffsetEntry = NextOffsetEntry;
+ NextOffsetEntry = OrderedCollectionNext (OffsetEntry);
+ }
+ //
+ // Calculate MaxSizeHint for the PCI_CAP object with the highest Offset.
+ //
+ OrderedCollectionDelete (CapHdrOffsets, OffsetEntry, (VOID **)&PciCap);
+ CalculatePciCapMaxSizeHint (PciCap, NULL);
+ }
+ ASSERT (OrderedCollectionIsEmpty (CapHdrOffsets));
+ OrderedCollectionUninit (CapHdrOffsets);
+
+ DebugDumpPciCapList (OutCapList);
+ *CapList = OutCapList;
+ return RETURN_SUCCESS;
+
+FreeCapHdrOffsets:
+ EmptyAndUninitPciCapCollection (CapHdrOffsets, FALSE);
+
+FreeCapabilities:
+ EmptyAndUninitPciCapCollection (OutCapList->Capabilities, TRUE);
+
+FreeOutCapList:
+ FreePool (OutCapList);
+
+ ASSERT (RETURN_ERROR (Status));
+ DEBUG ((DEBUG_ERROR, "%a:%a: %r\n", gEfiCallerBaseName, __FUNCTION__,
+ Status));
+ return Status;
+}
+
+
+/**
+ Free the resources used by CapList.
+
+ @param[in] CapList The PCI_CAP_LIST object to free, originally produced by
+ PciCapListInit().
+**/
+VOID
+EFIAPI
+PciCapListUninit (
+ IN PCI_CAP_LIST *CapList
+ )
+{
+ EmptyAndUninitPciCapCollection (CapList->Capabilities, TRUE);
+ FreePool (CapList);
+}
+
+
+/**
+ Locate a capability instance in the parsed capabilities lists.
+
+ @param[in] CapList The PCI_CAP_LIST object produced by PciCapListInit().
+
+ @param[in] Domain Distinguishes whether CapId is 8-bit wide and
+ interpreted in normal config space, or 16-bit wide and
+ interpreted in extended config space. Capability ID
+ definitions are relative to domain.
+
+ @param[in] CapId Capability identifier to look up.
+
+ @param[in] Instance Domain and CapId may identify a multi-instance
+ capability. When Instance is zero, the first instance of
+ the capability is located (in list traversal order --
+ which may not mean increasing config space offset
+ order). Higher Instance values locate subsequent
+ instances of the same capability (in list traversal
+ order).
+
+ @param[out] Cap The capability instance that matches the search
+ criteria. Cap is owned by CapList and becomes invalid
+ when CapList is freed with PciCapListUninit().
+ PciCapListFindCap() may be called with Cap set to NULL,
+ in order to test the existence of a specific capability
+ instance.
+
+ @retval RETURN_SUCCESS The capability instance identified by (Domain,
+ CapId, Instance) has been found.
+
+ @retval RETURN_NOT_FOUND The requested (Domain, CapId, Instance) capability
+ instance does not exist.
+**/
+RETURN_STATUS
+EFIAPI
+PciCapListFindCap (
+ IN PCI_CAP_LIST *CapList,
+ IN PCI_CAP_DOMAIN Domain,
+ IN UINT16 CapId,
+ IN UINT16 Instance,
+ OUT PCI_CAP **Cap OPTIONAL
+ )
+{
+ PCI_CAP_KEY Key;
+ ORDERED_COLLECTION_ENTRY *PciCapEntry;
+
+ Key.Domain = Domain;
+ Key.CapId = CapId;
+ Key.Instance = Instance;
+
+ PciCapEntry = OrderedCollectionFind (CapList->Capabilities, &Key);
+ if (PciCapEntry == NULL) {
+ return RETURN_NOT_FOUND;
+ }
+ if (Cap != NULL) {
+ *Cap = OrderedCollectionUserStruct (PciCapEntry);
+ }
+ return RETURN_SUCCESS;
+}
+
+
+/**
+ Locate the first instance of the capability given by (Domain, CapId) such
+ that the instance's Version is greater than or equal to MinVersion.
+
+ This is a convenience function that may save client code calls to
+ PciCapListFindCap() and PciCapGetInfo().
+
+ @param[in] CapList The PCI_CAP_LIST object produced by PciCapListInit().
+
+ @param[in] Domain Distinguishes whether CapId is 8-bit wide and
+ interpreted in normal config space, or 16-bit wide and
+ interpreted in extended config space. Capability ID
+ definitions are relative to domain.
+
+ @param[in] CapId Capability identifier to look up.
+
+ @param[in] MinVersion The minimum version that the capability instance is
+ required to have. Note that all capability instances
+ in Domain=PciCapNormal have Version=0.
+
+ @param[out] Cap The first capability instance that matches the search
+ criteria. Cap is owned by CapList and becomes invalid
+ when CapList is freed with PciCapListUninit().
+ PciCapListFindCapVersion() may be called with Cap set
+ to NULL, in order just to test whether the search
+ criteria are satisfiable.
+
+ @retval RETURN_SUCCESS The first capability instance matching (Domain,
+ CapId, MinVersion) has been located.
+
+ @retval RETURN_NOT_FOUND No capability instance matches (Domain, CapId,
+ MinVersion).
+**/
+RETURN_STATUS
+EFIAPI
+PciCapListFindCapVersion (
+ IN PCI_CAP_LIST *CapList,
+ IN PCI_CAP_DOMAIN Domain,
+ IN UINT16 CapId,
+ IN UINT8 MinVersion,
+ OUT PCI_CAP **Cap OPTIONAL
+ )
+{
+ PCI_CAP_KEY Key;
+ ORDERED_COLLECTION_ENTRY *PciCapEntry;
+
+ //
+ // Start the version checks at Instance#0 of (Domain, CapId).
+ //
+ Key.Domain = Domain;
+ Key.CapId = CapId;
+ Key.Instance = 0;
+
+ for (PciCapEntry = OrderedCollectionFind (CapList->Capabilities, &Key);
+ PciCapEntry != NULL;
+ PciCapEntry = OrderedCollectionNext (PciCapEntry)) {
+ PCI_CAP *PciCap;
+
+ PciCap = OrderedCollectionUserStruct (PciCapEntry);
+ //
+ // PCI_CAP.Key ordering keeps instances of the same (Domain, CapId)
+ // adjacent to each other, so stop searching if either Domain or CapId
+ // changes.
+ //
+ if (PciCap->Key.Domain != Domain || PciCap->Key.CapId != CapId) {
+ break;
+ }
+ if (PciCap->Version >= MinVersion) {
+ //
+ // Match found.
+ //
+ if (Cap != NULL) {
+ *Cap = PciCap;
+ }
+ return RETURN_SUCCESS;
+ }
+ }
+ return RETURN_NOT_FOUND;
+}
+
+
+/**
+ Get information about a PCI Capability instance.
+
+ @param[in] Cap The capability instance to get info about, located with
+ PciCapListFindCap*().
+
+ @param[out] Info A PCI_CAP_INFO structure that describes the properties of
+ Cap.
+
+ @retval RETURN_SUCCESS Fields of Info have been set.
+
+ @return Unspecified error codes, if filling in Info failed
+ for some reason.
+**/
+RETURN_STATUS
+EFIAPI
+PciCapGetInfo (
+ IN PCI_CAP *Cap,
+ OUT PCI_CAP_INFO *Info
+ )
+{
+ PCI_CAP *InstanceZero;
+
+ InstanceZero = (Cap->Key.Instance == 0 ? Cap :
+ Cap->NumInstancesUnion.InstanceZero);
+
+ Info->Domain = Cap->Key.Domain;
+ Info->CapId = Cap->Key.CapId;
+ Info->NumInstances = InstanceZero->NumInstancesUnion.NumInstances;
+ Info->Instance = Cap->Key.Instance;
+ Info->Offset = Cap->Offset;
+ Info->MaxSizeHint = Cap->MaxSizeHint;
+ Info->Version = Cap->Version;
+
+ return RETURN_SUCCESS;
+}
+
+
+/**
+ Read a slice of a capability instance.
+
+ The function performs as few config space accesses as possible (without
+ attempting 64-bit wide accesses). PciCapRead() performs bounds checking on
+ SourceOffsetInCap and Size, and only invokes PciDevice->ReadConfig() if the
+ requested transfer falls within Cap.
+
+ @param[in] PciDevice Implementation-specific unique representation
+ of the PCI device in the PCI hierarchy.
+
+ @param[in] Cap The capability instance to read, located with
+ PciCapListFindCap*().
+
+ @param[in] SourceOffsetInCap Source offset relative to the capability
+ header to start reading from. A zero value
+ refers to the first byte of the capability
+ header.
+
+ @param[out] DestinationBuffer Buffer to store the read data to.
+
+ @param[in] Size The number of bytes to transfer.
+
+ @retval RETURN_SUCCESS Size bytes have been transferred from Cap to
+ DestinationBuffer.
+
+ @retval RETURN_BAD_BUFFER_SIZE Reading Size bytes starting from
+ SourceOffsetInCap would not (entirely) be
+ contained within Cap, as suggested by
+ PCI_CAP_INFO.MaxSizeHint. No bytes have been
+ read.
+
+ @return Error codes propagated from
+ PciDevice->ReadConfig(). Fewer than Size
+ bytes may have been read.
+**/
+RETURN_STATUS
+EFIAPI
+PciCapRead (
+ IN PCI_CAP_DEV *PciDevice,
+ IN PCI_CAP *Cap,
+ IN UINT16 SourceOffsetInCap,
+ OUT VOID *DestinationBuffer,
+ IN UINT16 Size
+ )
+{
+ //
+ // Note: all UINT16 values are promoted to INT32 below, and addition and
+ // comparison take place between INT32 values.
+ //
+ if (SourceOffsetInCap + Size > Cap->MaxSizeHint) {
+ return RETURN_BAD_BUFFER_SIZE;
+ }
+ return PciDevice->ReadConfig (PciDevice, Cap->Offset + SourceOffsetInCap,
+ DestinationBuffer, Size);
+}
+
+
+/**
+ Write a slice of a capability instance.
+
+ The function performs as few config space accesses as possible (without
+ attempting 64-bit wide accesses). PciCapWrite() performs bounds checking on
+ DestinationOffsetInCap and Size, and only invokes PciDevice->WriteConfig() if
+ the requested transfer falls within Cap.
+
+ @param[in] PciDevice Implementation-specific unique
+ representation of the PCI device in the
+ PCI hierarchy.
+
+ @param[in] Cap The capability instance to write, located
+ with PciCapListFindCap*().
+
+ @param[in] DestinationOffsetInCap Destination offset relative to the
+ capability header to start writing at. A
+ zero value refers to the first byte of the
+ capability header.
+
+ @param[in] SourceBuffer Buffer to read the data to be stored from.
+
+ @param[in] Size The number of bytes to transfer.
+
+ @retval RETURN_SUCCESS Size bytes have been transferred from
+ SourceBuffer to Cap.
+
+ @retval RETURN_BAD_BUFFER_SIZE Writing Size bytes starting at
+ DestinationOffsetInCap would not (entirely)
+ be contained within Cap, as suggested by
+ PCI_CAP_INFO.MaxSizeHint. No bytes have been
+ written.
+
+ @return Error codes propagated from
+ PciDevice->WriteConfig(). Fewer than Size
+ bytes may have been written.
+**/
+RETURN_STATUS
+EFIAPI
+PciCapWrite (
+ IN PCI_CAP_DEV *PciDevice,
+ IN PCI_CAP *Cap,
+ IN UINT16 DestinationOffsetInCap,
+ IN VOID *SourceBuffer,
+ IN UINT16 Size
+ )
+{
+ //
+ // Note: all UINT16 values are promoted to INT32 below, and addition and
+ // comparison take place between INT32 values.
+ //
+ if (DestinationOffsetInCap + Size > Cap->MaxSizeHint) {
+ return RETURN_BAD_BUFFER_SIZE;
+ }
+ return PciDevice->WriteConfig (PciDevice,
+ Cap->Offset + DestinationOffsetInCap, SourceBuffer,
+ Size);
+}
--
2.14.1.3.gb7cf6e02401b
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/7] MdePkg: introduce PciCapPciSegmentLib
2018-05-04 21:36 [PATCH 0/7] MdePkg, OvmfPkg, ArmVirtPkg: add and use PCI(E) Capabilities Library Laszlo Ersek
2018-05-04 21:36 ` [PATCH 1/7] MdePkg: introduce PciCapLib Laszlo Ersek
@ 2018-05-04 21:36 ` Laszlo Ersek
2018-05-04 21:36 ` [PATCH 3/7] MdePkg: introduce PciCapPciIoLib Laszlo Ersek
` (5 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Laszlo Ersek @ 2018-05-04 21:36 UTC (permalink / raw)
To: edk2-devel-01; +Cc: Ard Biesheuvel, Jordan Justen, Liming Gao, Michael D Kinney
Add a library class, and a BASE lib instance, that are layered on top of
PciCapLib, and allow clients to plug a PciSegmentLib backend into
PciCapLib, for config space access.
(Side note:
The "MaxDomain" parameter is provided because, in practice, platforms
exist where a PCI Express device may show up on a root bridge such that
the root bridge doesn't support access to extended config space. Earlier
the same issue was handled for MdeModulePkg/PciHostBridgeDxe in commit
014b472053ae3. However, that solution does not apply to the PciSegmentLib
class, because:
(1) The config space accessor functions of the PciSegmentLib class, such
as PciSegmentReadBuffer(), have no way of informing the caller whether
access to extended config space actually succeeds.
(For example, in the UefiPciSegmentLibPciRootBridgeIo instace, which
could in theory benefit from commit 014b472053ae3, the
EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL.Pci.Read() status code is explicitly
ignored, because there's no way for the lib instance to propagate it
to the PciSegmentLib caller. If the
EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL.Pci.Read() call fails, then
DxePciSegmentLibPciRootBridgeIoReadWorker() returns Data with
indeterminate value.)
(2) There is no *general* way for any firmware platform to provide, or
use, a PciSegmentLib instance in which access to extended config space
always succeeds.
In brief, on a platform where config space may be limited to 256 bytes,
access to extended config space through PciSegmentLib may invoke undefined
behavior; therefore PciCapPciSegmentLib must give platforms a way to
prevent such access.)
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
MdePkg/MdePkg.dec | 5 +
MdePkg/MdePkg.dsc | 1 +
MdePkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf | 34 +++
MdePkg/Include/Library/PciCapPciSegmentLib.h | 82 +++++++
MdePkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.h | 47 ++++
MdePkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.c | 226 ++++++++++++++++++++
6 files changed, 395 insertions(+)
diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index e961d64ac89a..acda885146ab 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -155,6 +155,11 @@ [LibraryClasses]
## config space.
PciCapLib|Include/Library/PciCapLib.h
+ ## @libraryclass Layered on top of PciCapLib, allows clients to plug a
+ ## PciSegmentLib backend into PciCapLib, for config space
+ ## access.
+ PciCapPciSegmentLib|Include/Library/PciCapPciSegmentLib.h
+
## @libraryclass Provides library services to get and set Platform Configuration Database entries.
PcdLib|Include/Library/PcdLib.h
diff --git a/MdePkg/MdePkg.dsc b/MdePkg/MdePkg.dsc
index 9ec86b95ed4e..f655ec827080 100644
--- a/MdePkg/MdePkg.dsc
+++ b/MdePkg/MdePkg.dsc
@@ -71,6 +71,7 @@ [Components]
MdePkg/Library/BasePciSegmentLibPci/BasePciSegmentLibPci.inf
MdePkg/Library/BasePciSegmentInfoLibNull/BasePciSegmentInfoLibNull.inf
MdePkg/Library/BasePciCapLib/BasePciCapLib.inf
+ MdePkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
MdePkg/Library/PciSegmentLibSegmentInfo/BasePciSegmentLibSegmentInfo.inf
MdePkg/Library/PciSegmentLibSegmentInfo/DxeRuntimePciSegmentLibSegmentInfo.inf
MdePkg/Library/BaseS3PciSegmentLib/BaseS3PciSegmentLib.inf
diff --git a/MdePkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf b/MdePkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
new file mode 100644
index 000000000000..a9ac3877c768
--- /dev/null
+++ b/MdePkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
@@ -0,0 +1,34 @@
+## @file
+# Plug a PciSegmentLib backend into PciCapLib, for config space access.
+#
+# Copyright (C) 2018, Red Hat, Inc.
+#
+# 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 = 1.27
+ BASE_NAME = BasePciCapPciSegmentLib
+ FILE_GUID = ED011855-AA31-43B9-ACC0-BF45A05C5985
+ MODULE_TYPE = BASE
+ VERSION_STRING = 1.0
+ LIBRARY_CLASS = PciCapPciSegmentLib
+
+[Sources]
+ BasePciCapPciSegmentLib.h
+ BasePciCapPciSegmentLib.c
+
+[Packages]
+ MdePkg/MdePkg.dec
+
+[LibraryClasses]
+ BaseLib
+ DebugLib
+ MemoryAllocationLib
+ PciSegmentLib
diff --git a/MdePkg/Include/Library/PciCapPciSegmentLib.h b/MdePkg/Include/Library/PciCapPciSegmentLib.h
new file mode 100644
index 000000000000..6b6930288d16
--- /dev/null
+++ b/MdePkg/Include/Library/PciCapPciSegmentLib.h
@@ -0,0 +1,82 @@
+/** @file
+ Library class layered on top of PciCapLib that allows clients to plug a
+ PciSegmentLib backend into PciCapLib, for config space access.
+
+ Copyright (C) 2018, Red Hat, Inc.
+
+ 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.
+**/
+
+#ifndef __PCI_CAP_PCI_SEGMENT_LIB_H__
+#define __PCI_CAP_PCI_SEGMENT_LIB_H__
+
+#include <Library/PciCapLib.h>
+
+
+/**
+ Create a PCI_CAP_DEV object from the PCI Segment:Bus:Device.Function
+ quadruplet. The config space accessors are based upon PciSegmentLib.
+
+ @param[in] MaxDomain If MaxDomain is PciCapExtended, then
+ PciDevice->ReadConfig() and PciDevice->WriteConfig()
+ will delegate extended config space accesses too to
+ PciSegmentReadBuffer() and PciSegmentWriteBuffer(),
+ respectively. Otherwise, PciDevice->ReadConfig() and
+ PciDevice->WriteConfig() will reject accesses to
+ extended config space with RETURN_UNSUPPORTED, without
+ calling PciSegmentReadBuffer() or
+ PciSegmentWriteBuffer(). By setting MaxDomain to
+ PciCapNormal, the platform can prevent undefined
+ PciSegmentLib behavior when the PCI root bridge under
+ the PCI device at Segment:Bus:Device.Function doesn't
+ support extended config space.
+
+ @param[in] Segment 16-bit wide segment number.
+
+ @param[in] Bus 8-bit wide bus number.
+
+ @param[in] Device 5-bit wide device number.
+
+ @param[in] Function 3-bit wide function number.
+
+ @param[out] PciDevice The PCI_CAP_DEV object constructed as described above.
+ PciDevice can be passed to the PciCapLib APIs.
+
+ @retval RETURN_SUCCESS PciDevice has been constructed and output.
+
+ @retval RETURN_INVALID_PARAMETER Device or Function does not fit in the
+ permitted number of bits.
+
+ @retval RETURN_OUT_OF_RESOURCES Memory allocation failed.
+**/
+RETURN_STATUS
+EFIAPI
+PciCapPciSegmentDeviceInit (
+ IN PCI_CAP_DOMAIN MaxDomain,
+ IN UINT16 Segment,
+ IN UINT8 Bus,
+ IN UINT8 Device,
+ IN UINT8 Function,
+ OUT PCI_CAP_DEV **PciDevice
+ );
+
+
+/**
+ Free the resources used by PciDevice.
+
+ @param[in] PciDevice The PCI_CAP_DEV object to free, originally produced by
+ PciCapPciSegmentDeviceInit().
+**/
+VOID
+EFIAPI
+PciCapPciSegmentDeviceUninit (
+ IN PCI_CAP_DEV *PciDevice
+ );
+
+#endif // __PCI_CAP_PCI_SEGMENT_LIB_H__
diff --git a/MdePkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.h b/MdePkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.h
new file mode 100644
index 000000000000..3ce15fe0fb57
--- /dev/null
+++ b/MdePkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.h
@@ -0,0 +1,47 @@
+/** @file
+ Plug a PciSegmentLib backend into PciCapLib, for config space access --
+ internal macro and type definitions.
+
+ Copyright (C) 2018, Red Hat, Inc.
+
+ 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.
+**/
+
+#ifndef __BASE_PCI_CAP_PCI_SEGMENT_LIB_H__
+#define __BASE_PCI_CAP_PCI_SEGMENT_LIB_H__
+
+#include <Library/DebugLib.h>
+
+#include <Library/PciCapPciSegmentLib.h>
+
+#define SEGMENT_DEV_SIG SIGNATURE_64 ('P', 'C', 'P', 'S', 'G', 'M', 'N', 'T')
+
+typedef struct {
+ //
+ // Signature identifying the derived class.
+ //
+ UINT64 Signature;
+ //
+ // Members added by the derived class, specific to the use of PciSegmentLib.
+ //
+ PCI_CAP_DOMAIN MaxDomain;
+ UINT16 SegmentNr;
+ UINT8 BusNr;
+ UINT8 DeviceNr;
+ UINT8 FunctionNr;
+ //
+ // Base class.
+ //
+ PCI_CAP_DEV BaseDevice;
+} SEGMENT_DEV;
+
+#define SEGMENT_DEV_FROM_PCI_CAP_DEV(PciDevice) \
+ CR (PciDevice, SEGMENT_DEV, BaseDevice, SEGMENT_DEV_SIG)
+
+#endif // __BASE_PCI_CAP_PCI_SEGMENT_LIB_H__
diff --git a/MdePkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.c b/MdePkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.c
new file mode 100644
index 000000000000..57eb6b625b56
--- /dev/null
+++ b/MdePkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.c
@@ -0,0 +1,226 @@
+/** @file
+ Plug a PciSegmentLib backend into PciCapLib, for config space access.
+
+ Copyright (C) 2018, Red Hat, Inc.
+
+ 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.
+**/
+
+#include <IndustryStandard/Pci23.h>
+
+#include <Library/BaseLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/PciSegmentLib.h>
+
+#include "BasePciCapPciSegmentLib.h"
+
+
+/**
+ Read the config space of a given PCI device (both normal and extended).
+
+ SegmentDevReadConfig() performs as few config space accesses as possible
+ (without attempting 64-bit wide accesses).
+
+ @param[in] PciDevice Implementation-specific unique representation
+ of the PCI device in the PCI hierarchy.
+
+ @param[in] SourceOffset Source offset in the config space of the PCI
+ device to start reading from.
+
+ @param[out] DestinationBuffer Buffer to store the read data to.
+
+ @param[in] Size The number of bytes to transfer.
+
+ @retval RETURN_SUCCESS Size bytes have been transferred from config
+ space to DestinationBuffer.
+
+ @retval RETURN_UNSUPPORTED Accessing Size bytes from SourceOffset exceeds
+ the config space limit of the PCI device.
+ Although PCI_CAP_DEV_READ_CONFIG allows reading
+ fewer than Size bytes in this case,
+ SegmentDevReadConfig() will read none.
+**/
+STATIC
+RETURN_STATUS
+EFIAPI
+SegmentDevReadConfig (
+ IN PCI_CAP_DEV *PciDevice,
+ IN UINT16 SourceOffset,
+ OUT VOID *DestinationBuffer,
+ IN UINT16 Size
+ )
+{
+ SEGMENT_DEV *SegmentDev;
+ UINT16 ConfigSpaceSize;
+ UINT64 SourceAddress;
+
+ SegmentDev = SEGMENT_DEV_FROM_PCI_CAP_DEV (PciDevice);
+ ConfigSpaceSize = (SegmentDev->MaxDomain == PciCapNormal ?
+ PCI_MAX_CONFIG_OFFSET : PCI_EXP_MAX_CONFIG_OFFSET);
+ //
+ // Note that all UINT16 variables below are promoted to INT32, and the
+ // addition and the comparison is carried out in INT32.
+ //
+ if (SourceOffset + Size > ConfigSpaceSize) {
+ return RETURN_UNSUPPORTED;
+ }
+ SourceAddress = PCI_SEGMENT_LIB_ADDRESS (SegmentDev->SegmentNr,
+ SegmentDev->BusNr, SegmentDev->DeviceNr,
+ SegmentDev->FunctionNr, SourceOffset);
+ PciSegmentReadBuffer (SourceAddress, Size, DestinationBuffer);
+ return RETURN_SUCCESS;
+}
+
+
+/**
+ Write the config space of a given PCI device (both normal and extended).
+
+ SegmentDevWriteConfig() performs as few config space accesses as possible
+ (without attempting 64-bit wide accesses).
+
+ @param[in] PciDevice Implementation-specific unique representation
+ of the PCI device in the PCI hierarchy.
+
+ @param[in] DestinationOffset Destination offset in the config space of the
+ PCI device to start writing at.
+
+ @param[in] SourceBuffer Buffer to read the data to be stored from.
+
+ @param[in] Size The number of bytes to transfer.
+
+ @retval RETURN_SUCCESS Size bytes have been transferred from
+ SourceBuffer to config space.
+
+ @retval RETURN_UNSUPPORTED Accessing Size bytes at DestinationOffset exceeds
+ the config space limit of the PCI device.
+ Although PCI_CAP_DEV_WRITE_CONFIG allows writing
+ fewer than Size bytes in this case,
+ SegmentDevWriteConfig() will write none.
+**/
+STATIC
+RETURN_STATUS
+EFIAPI
+SegmentDevWriteConfig (
+ IN PCI_CAP_DEV *PciDevice,
+ IN UINT16 DestinationOffset,
+ IN VOID *SourceBuffer,
+ IN UINT16 Size
+ )
+{
+ SEGMENT_DEV *SegmentDev;
+ UINT16 ConfigSpaceSize;
+ UINT64 DestinationAddress;
+
+ SegmentDev = SEGMENT_DEV_FROM_PCI_CAP_DEV (PciDevice);
+ ConfigSpaceSize = (SegmentDev->MaxDomain == PciCapNormal ?
+ PCI_MAX_CONFIG_OFFSET : PCI_EXP_MAX_CONFIG_OFFSET);
+ //
+ // Note that all UINT16 variables below are promoted to INT32, and the
+ // addition and the comparison is carried out in INT32.
+ //
+ if (DestinationOffset + Size > ConfigSpaceSize) {
+ return RETURN_UNSUPPORTED;
+ }
+ DestinationAddress = PCI_SEGMENT_LIB_ADDRESS (SegmentDev->SegmentNr,
+ SegmentDev->BusNr, SegmentDev->DeviceNr,
+ SegmentDev->FunctionNr, DestinationOffset);
+ PciSegmentWriteBuffer (DestinationAddress, Size, SourceBuffer);
+ return RETURN_SUCCESS;
+}
+
+
+/**
+ Create a PCI_CAP_DEV object from the PCI Segment:Bus:Device.Function
+ quadruplet. The config space accessors are based upon PciSegmentLib.
+
+ @param[in] MaxDomain If MaxDomain is PciCapExtended, then
+ PciDevice->ReadConfig() and PciDevice->WriteConfig()
+ will delegate extended config space accesses too to
+ PciSegmentReadBuffer() and PciSegmentWriteBuffer(),
+ respectively. Otherwise, PciDevice->ReadConfig() and
+ PciDevice->WriteConfig() will reject accesses to
+ extended config space with RETURN_UNSUPPORTED, without
+ calling PciSegmentReadBuffer() or
+ PciSegmentWriteBuffer(). By setting MaxDomain to
+ PciCapNormal, the platform can prevent undefined
+ PciSegmentLib behavior when the PCI root bridge under
+ the PCI device at Segment:Bus:Device.Function doesn't
+ support extended config space.
+
+ @param[in] Segment 16-bit wide segment number.
+
+ @param[in] Bus 8-bit wide bus number.
+
+ @param[in] Device 5-bit wide device number.
+
+ @param[in] Function 3-bit wide function number.
+
+ @param[out] PciDevice The PCI_CAP_DEV object constructed as described above.
+ PciDevice can be passed to the PciCapLib APIs.
+
+ @retval RETURN_SUCCESS PciDevice has been constructed and output.
+
+ @retval RETURN_INVALID_PARAMETER Device or Function does not fit in the
+ permitted number of bits.
+
+ @retval RETURN_OUT_OF_RESOURCES Memory allocation failed.
+**/
+RETURN_STATUS
+EFIAPI
+PciCapPciSegmentDeviceInit (
+ IN PCI_CAP_DOMAIN MaxDomain,
+ IN UINT16 Segment,
+ IN UINT8 Bus,
+ IN UINT8 Device,
+ IN UINT8 Function,
+ OUT PCI_CAP_DEV **PciDevice
+ )
+{
+ SEGMENT_DEV *SegmentDev;
+
+ if (Device > PCI_MAX_DEVICE || Function > PCI_MAX_FUNC) {
+ return RETURN_INVALID_PARAMETER;
+ }
+
+ SegmentDev = AllocatePool (sizeof *SegmentDev);
+ if (SegmentDev == NULL) {
+ return RETURN_OUT_OF_RESOURCES;
+ }
+
+ SegmentDev->Signature = SEGMENT_DEV_SIG;
+ SegmentDev->MaxDomain = MaxDomain;
+ SegmentDev->SegmentNr = Segment;
+ SegmentDev->BusNr = Bus;
+ SegmentDev->DeviceNr = Device;
+ SegmentDev->FunctionNr = Function;
+ SegmentDev->BaseDevice.ReadConfig = SegmentDevReadConfig;
+ SegmentDev->BaseDevice.WriteConfig = SegmentDevWriteConfig;
+
+ *PciDevice = &SegmentDev->BaseDevice;
+ return RETURN_SUCCESS;
+}
+
+
+/**
+ Free the resources used by PciDevice.
+
+ @param[in] PciDevice The PCI_CAP_DEV object to free, originally produced by
+ PciCapPciSegmentDeviceInit().
+**/
+VOID
+EFIAPI
+PciCapPciSegmentDeviceUninit (
+ IN PCI_CAP_DEV *PciDevice
+ )
+{
+ SEGMENT_DEV *SegmentDev;
+
+ SegmentDev = SEGMENT_DEV_FROM_PCI_CAP_DEV (PciDevice);
+ FreePool (SegmentDev);
+}
--
2.14.1.3.gb7cf6e02401b
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/7] MdePkg: introduce PciCapPciIoLib
2018-05-04 21:36 [PATCH 0/7] MdePkg, OvmfPkg, ArmVirtPkg: add and use PCI(E) Capabilities Library Laszlo Ersek
2018-05-04 21:36 ` [PATCH 1/7] MdePkg: introduce PciCapLib Laszlo Ersek
2018-05-04 21:36 ` [PATCH 2/7] MdePkg: introduce PciCapPciSegmentLib Laszlo Ersek
@ 2018-05-04 21:36 ` Laszlo Ersek
2018-05-04 21:36 ` [PATCH 4/7] OvmfPkg: resolve PciCapLib, PciCapPciSegmentLib, PciCapPciIoLib Laszlo Ersek
` (4 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Laszlo Ersek @ 2018-05-04 21:36 UTC (permalink / raw)
To: edk2-devel-01; +Cc: Ard Biesheuvel, Jordan Justen, Liming Gao, Michael D Kinney
Add a library class, and a UEFI_DRIVER lib instance, that are layered on
top of PciCapLib, and allow clients to plug an EFI_PCI_IO_PROTOCOL backend
into PciCapLib, for config space access.
(Side note:
Although the UEFI spec says that EFI_PCI_IO_PROTOCOL_CONFIG() returns
EFI_UNSUPPORTED if "[t]he address range specified by Offset, Width, and
Count is not valid for the PCI configuration header of the PCI
controller", this patch doesn't directly document the EFI_UNSUPPORTED
error code, for ProtoDevTransferConfig() and its callers
ProtoDevReadConfig() and ProtoDevWriteConfig(). Instead, the patch refers
to "unspecified error codes". The reason is that in edk2, the
PciIoConfigRead() and PciIoConfigWrite() functions [1] can also return
EFI_INVALID_PARAMETER for the above situation.
Namely, PciIoConfigRead() and PciIoConfigWrite() first call
PciIoVerifyConfigAccess(), which indeed produces the standard
EFI_UNSUPPORTED error code, if the device's config space is exceeded.
However, if PciIoVerifyConfigAccess() passes, and we reach
RootBridgeIoPciRead() and RootBridgeIoPciWrite() [2], then
RootBridgeIoCheckParameter() can still fail, e.g. if the root bridge
doesn't support extended config space (see commit 014b472053ae3).
For all kinds of Limit violations in IO, MMIO, and config space,
RootBridgeIoCheckParameter() returns EFI_INVALID_PARAMETER, not
EFI_UNSUPPORTED. That error code is then propagated up to, and out of,
PciIoConfigRead() and PciIoConfigWrite().
[1] MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
[2] MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
)
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
MdePkg/MdePkg.dec | 5 +
MdePkg/MdePkg.dsc | 1 +
MdePkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf | 35 +++
MdePkg/Include/Library/PciCapPciIoLib.h | 58 +++++
MdePkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.h | 44 ++++
MdePkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.c | 243 ++++++++++++++++++++
6 files changed, 386 insertions(+)
diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index acda885146ab..0d41b3bb3449 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -160,6 +160,11 @@ [LibraryClasses]
## access.
PciCapPciSegmentLib|Include/Library/PciCapPciSegmentLib.h
+ ## @libraryclass Layered on top of PciCapLib, allows clients to plug an
+ ## EFI_PCI_IO_PROTOCOL backend into PciCapLib, for config
+ ## space access.
+ PciCapPciIoLib|Include/Library/PciCapPciIoLib.h
+
## @libraryclass Provides library services to get and set Platform Configuration Database entries.
PcdLib|Include/Library/PcdLib.h
diff --git a/MdePkg/MdePkg.dsc b/MdePkg/MdePkg.dsc
index f655ec827080..c18477d11f49 100644
--- a/MdePkg/MdePkg.dsc
+++ b/MdePkg/MdePkg.dsc
@@ -136,6 +136,7 @@ [Components]
MdePkg/Library/UefiUsbLib/UefiUsbLib.inf
MdePkg/Library/UefiPciLibPciRootBridgeIo/UefiPciLibPciRootBridgeIo.inf
MdePkg/Library/UefiPciSegmentLibPciRootBridgeIo/UefiPciSegmentLibPciRootBridgeIo.inf
+ MdePkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
MdePkg/Library/SmmLibNull/SmmLibNull.inf
MdePkg/Library/BaseExtractGuidedSectionLib/BaseExtractGuidedSectionLib.inf
diff --git a/MdePkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf b/MdePkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
new file mode 100644
index 000000000000..239a3f4aa8c7
--- /dev/null
+++ b/MdePkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
@@ -0,0 +1,35 @@
+## @file
+# Plug an EFI_PCI_IO_PROTOCOL backend into PciCapLib, for config space access.
+#
+# Copyright (C) 2018, Red Hat, Inc.
+#
+# 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 = 1.27
+ BASE_NAME = UefiPciCapPciIoLib
+ FILE_GUID = 4102F4FE-DA10-4F0F-AC18-4982ED506154
+ MODULE_TYPE = UEFI_DRIVER
+ VERSION_STRING = 1.0
+ LIBRARY_CLASS = PciCapPciIoLib
+
+[Sources]
+ UefiPciCapPciIoLib.h
+ UefiPciCapPciIoLib.c
+
+[Packages]
+ MdePkg/MdePkg.dec
+
+[LibraryClasses]
+ DebugLib
+ MemoryAllocationLib
+
+[Protocols]
+ gEfiPciIoProtocolGuid ## CONSUMES
diff --git a/MdePkg/Include/Library/PciCapPciIoLib.h b/MdePkg/Include/Library/PciCapPciIoLib.h
new file mode 100644
index 000000000000..553715fd5080
--- /dev/null
+++ b/MdePkg/Include/Library/PciCapPciIoLib.h
@@ -0,0 +1,58 @@
+/** @file
+ Library class layered on top of PciCapLib that allows clients to plug an
+ EFI_PCI_IO_PROTOCOL backend into PciCapLib, for config space access.
+
+ Copyright (C) 2018, Red Hat, Inc.
+
+ 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.
+**/
+
+#ifndef __PCI_CAP_PCI_IO_LIB_H__
+#define __PCI_CAP_PCI_IO_LIB_H__
+
+#include <Protocol/PciIo.h>
+
+#include <Library/PciCapLib.h>
+
+
+/**
+ Create a PCI_CAP_DEV object from an EFI_PCI_IO_PROTOCOL instance. The config
+ space accessors are based upon EFI_PCI_IO_PROTOCOL.Pci.Read() and
+ EFI_PCI_IO_PROTOCOL.Pci.Write().
+
+ @param[in] PciIo EFI_PCI_IO_PROTOCOL representation of the PCI device.
+
+ @param[out] PciDevice The PCI_CAP_DEV object constructed as described above.
+ PciDevice can be passed to the PciCapLib APIs.
+
+ @retval EFI_SUCCESS PciDevice has been constructed and output.
+
+ @retval EFI_OUT_OF_RESOURCES Memory allocation failed.
+**/
+EFI_STATUS
+EFIAPI
+PciCapPciIoDeviceInit (
+ IN EFI_PCI_IO_PROTOCOL *PciIo,
+ OUT PCI_CAP_DEV **PciDevice
+ );
+
+
+/**
+ Free the resources used by PciDevice.
+
+ @param[in] PciDevice The PCI_CAP_DEV object to free, originally produced by
+ PciCapPciIoDeviceInit().
+**/
+VOID
+EFIAPI
+PciCapPciIoDeviceUninit (
+ IN PCI_CAP_DEV *PciDevice
+ );
+
+#endif // __PCI_CAP_PCI_IO_LIB_H__
diff --git a/MdePkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.h b/MdePkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.h
new file mode 100644
index 000000000000..a94f65e5a4e3
--- /dev/null
+++ b/MdePkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.h
@@ -0,0 +1,44 @@
+/** @file
+ Plug an EFI_PCI_IO_PROTOCOL backend into PciCapLib, for config space access
+ -- internal macro and type definitions.
+
+ Copyright (C) 2018, Red Hat, Inc.
+
+ 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.
+**/
+
+#ifndef __UEFI_PCI_CAP_PCI_IO_LIB_H__
+#define __UEFI_PCI_CAP_PCI_IO_LIB_H__
+
+#include <Library/DebugLib.h>
+
+#include <Library/PciCapPciIoLib.h>
+
+#define PROTO_DEV_SIG SIGNATURE_64 ('P', 'C', 'P', 'I', 'O', 'P', 'R', 'T')
+
+typedef struct {
+ //
+ // Signature identifying the derived class.
+ //
+ UINT64 Signature;
+ //
+ // Members added by the derived class, specific to the use of
+ // EFI_PCI_IO_PROTOCOL.
+ //
+ EFI_PCI_IO_PROTOCOL *PciIo;
+ //
+ // Base class.
+ //
+ PCI_CAP_DEV BaseDevice;
+} PROTO_DEV;
+
+#define PROTO_DEV_FROM_PCI_CAP_DEV(PciDevice) \
+ CR (PciDevice, PROTO_DEV, BaseDevice, PROTO_DEV_SIG)
+
+#endif // __UEFI_PCI_CAP_PCI_IO_LIB_H__
diff --git a/MdePkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.c b/MdePkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.c
new file mode 100644
index 000000000000..84369e4dc3a8
--- /dev/null
+++ b/MdePkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.c
@@ -0,0 +1,243 @@
+/** @file
+ Plug an EFI_PCI_IO_PROTOCOL backend into PciCapLib, for config space access.
+
+ Copyright (C) 2018, Red Hat, Inc.
+
+ 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.
+**/
+
+#include <Library/MemoryAllocationLib.h>
+
+#include "UefiPciCapPciIoLib.h"
+
+
+/**
+ Transfer bytes between the config space of a given PCI device and a memory
+ buffer.
+
+ ProtoDevTransferConfig() performs as few config space accesses as possible
+ (without attempting 64-bit wide accesses).
+
+ @param[in] PciIo The EFI_PCI_IO_PROTOCOL representation of the
+ PCI device.
+
+ @param[in] TransferFunction The EFI_PCI_IO_PROTOCOL_CONFIG function that
+ implements the transfer. The direction of the
+ transfer is inherent to TransferFunction.
+ TransferFunction() is required to return an
+ unspecified error if any sub-transfer within
+ Size bytes from ConfigOffset exceeds the config
+ space limit of the PCI device.
+
+ @param[in] ConfigOffset The offset in the config space of the PCI device
+ at which the transfer should commence.
+
+ @param[in,out] Buffer The memory buffer where the transfer should
+ occur.
+
+ @param[in] Size The number of bytes to transfer.
+
+ @retval EFI_SUCCESS Size bytes have been transferred between config space
+ and Buffer.
+
+ @return Error codes propagated from TransferFunction(). Fewer
+ than Size bytes may have been transferred.
+**/
+STATIC
+EFI_STATUS
+ProtoDevTransferConfig (
+ IN EFI_PCI_IO_PROTOCOL *PciIo,
+ IN EFI_PCI_IO_PROTOCOL_CONFIG TransferFunction,
+ IN UINT16 ConfigOffset,
+ IN OUT UINT8 *Buffer,
+ IN UINT16 Size
+ )
+{
+ while (Size > 0) {
+ EFI_PCI_IO_PROTOCOL_WIDTH Width;
+ UINT16 Count;
+ EFI_STATUS Status;
+ UINT16 Progress;
+
+ //
+ // Pick the largest access size that is allowed by the remaining transfer
+ // Size and by the alignment of ConfigOffset.
+ //
+ // When the largest access size is available, transfer as many bytes as
+ // possible in one iteration of the loop. Otherwise, transfer only one
+ // unit, to improve the alignment.
+ //
+ if (Size >= BIT2 && (ConfigOffset & (BIT2 - 1)) == 0) {
+ Width = EfiPciIoWidthUint32;
+ Count = Size >> Width;
+ } else if (Size >= BIT1 && (ConfigOffset & (BIT1 - 1)) == 0) {
+ Width = EfiPciIoWidthUint16;
+ Count = 1;
+ } else {
+ Width = EfiPciIoWidthUint8;
+ Count = 1;
+ }
+ Status = TransferFunction (PciIo, Width, ConfigOffset, Count, Buffer);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+ Progress = Count << Width;
+ ConfigOffset += Progress;
+ Buffer += Progress;
+ Size -= Progress;
+ }
+ return EFI_SUCCESS;
+}
+
+
+/**
+ Read the config space of a given PCI device (both normal and extended).
+
+ ProtoDevReadConfig() performs as few config space accesses as possible
+ (without attempting 64-bit wide accesses).
+
+ ProtoDevReadConfig() returns an unspecified error if accessing Size bytes
+ from SourceOffset exceeds the config space limit of the PCI device. Fewer
+ than Size bytes may have been read in this case.
+
+ @param[in] PciDevice Implementation-specific unique representation
+ of the PCI device in the PCI hierarchy.
+
+ @param[in] SourceOffset Source offset in the config space of the PCI
+ device to start reading from.
+
+ @param[out] DestinationBuffer Buffer to store the read data to.
+
+ @param[in] Size The number of bytes to transfer.
+
+ @retval RETURN_SUCCESS Size bytes have been transferred from config space to
+ DestinationBuffer.
+
+ @return Error codes propagated from
+ EFI_PCI_IO_PROTOCOL.Pci.Read(). Fewer than Size bytes
+ may have been read.
+**/
+STATIC
+RETURN_STATUS
+EFIAPI
+ProtoDevReadConfig (
+ IN PCI_CAP_DEV *PciDevice,
+ IN UINT16 SourceOffset,
+ OUT VOID *DestinationBuffer,
+ IN UINT16 Size
+ )
+{
+ PROTO_DEV *ProtoDev;
+
+ ProtoDev = PROTO_DEV_FROM_PCI_CAP_DEV (PciDevice);
+ return ProtoDevTransferConfig (ProtoDev->PciIo, ProtoDev->PciIo->Pci.Read,
+ SourceOffset, DestinationBuffer, Size);
+}
+
+
+/**
+ Write the config space of a given PCI device (both normal and extended).
+
+ ProtoDevWriteConfig() performs as few config space accesses as possible
+ (without attempting 64-bit wide accesses).
+
+ ProtoDevWriteConfig() returns an unspecified error if accessing Size bytes at
+ DestinationOffset exceeds the config space limit of the PCI device. Fewer
+ than Size bytes may have been written in this case.
+
+ @param[in] PciDevice Implementation-specific unique representation
+ of the PCI device in the PCI hierarchy.
+
+ @param[in] DestinationOffset Destination offset in the config space of the
+ PCI device to start writing at.
+
+ @param[in] SourceBuffer Buffer to read the data to be stored from.
+
+ @param[in] Size The number of bytes to transfer.
+
+ @retval RETURN_SUCCESS Size bytes have been transferred from SourceBuffer to
+ config space.
+
+ @return Error codes propagated from
+ EFI_PCI_IO_PROTOCOL.Pci.Write(). Fewer than Size
+ bytes may have been written.
+**/
+STATIC
+RETURN_STATUS
+EFIAPI
+ProtoDevWriteConfig (
+ IN PCI_CAP_DEV *PciDevice,
+ IN UINT16 DestinationOffset,
+ IN VOID *SourceBuffer,
+ IN UINT16 Size
+ )
+{
+ PROTO_DEV *ProtoDev;
+
+ ProtoDev = PROTO_DEV_FROM_PCI_CAP_DEV (PciDevice);
+ return ProtoDevTransferConfig (ProtoDev->PciIo, ProtoDev->PciIo->Pci.Write,
+ DestinationOffset, SourceBuffer, Size);
+}
+
+
+/**
+ Create a PCI_CAP_DEV object from an EFI_PCI_IO_PROTOCOL instance. The config
+ space accessors are based upon EFI_PCI_IO_PROTOCOL.Pci.Read() and
+ EFI_PCI_IO_PROTOCOL.Pci.Write().
+
+ @param[in] PciIo EFI_PCI_IO_PROTOCOL representation of the PCI device.
+
+ @param[out] PciDevice The PCI_CAP_DEV object constructed as described above.
+ PciDevice can be passed to the PciCapLib APIs.
+
+ @retval EFI_SUCCESS PciDevice has been constructed and output.
+
+ @retval EFI_OUT_OF_RESOURCES Memory allocation failed.
+**/
+EFI_STATUS
+EFIAPI
+PciCapPciIoDeviceInit (
+ IN EFI_PCI_IO_PROTOCOL *PciIo,
+ OUT PCI_CAP_DEV **PciDevice
+ )
+{
+ PROTO_DEV *ProtoDev;
+
+ ProtoDev = AllocatePool (sizeof *ProtoDev);
+ if (ProtoDev == NULL) {
+ return EFI_OUT_OF_RESOURCES;
+ }
+
+ ProtoDev->Signature = PROTO_DEV_SIG;
+ ProtoDev->PciIo = PciIo;
+ ProtoDev->BaseDevice.ReadConfig = ProtoDevReadConfig;
+ ProtoDev->BaseDevice.WriteConfig = ProtoDevWriteConfig;
+
+ *PciDevice = &ProtoDev->BaseDevice;
+ return EFI_SUCCESS;
+}
+
+
+/**
+ Free the resources used by PciDevice.
+
+ @param[in] PciDevice The PCI_CAP_DEV object to free, originally produced by
+ PciCapPciIoDeviceInit().
+**/
+VOID
+EFIAPI
+PciCapPciIoDeviceUninit (
+ IN PCI_CAP_DEV *PciDevice
+ )
+{
+ PROTO_DEV *ProtoDev;
+
+ ProtoDev = PROTO_DEV_FROM_PCI_CAP_DEV (PciDevice);
+ FreePool (ProtoDev);
+}
--
2.14.1.3.gb7cf6e02401b
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 4/7] OvmfPkg: resolve PciCapLib, PciCapPciSegmentLib, PciCapPciIoLib
2018-05-04 21:36 [PATCH 0/7] MdePkg, OvmfPkg, ArmVirtPkg: add and use PCI(E) Capabilities Library Laszlo Ersek
` (2 preceding siblings ...)
2018-05-04 21:36 ` [PATCH 3/7] MdePkg: introduce PciCapPciIoLib Laszlo Ersek
@ 2018-05-04 21:36 ` Laszlo Ersek
2018-05-04 21:36 ` [PATCH 5/7] ArmVirtPkg: " Laszlo Ersek
` (3 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Laszlo Ersek @ 2018-05-04 21:36 UTC (permalink / raw)
To: edk2-devel-01; +Cc: Ard Biesheuvel, Jordan Justen
Resolve the PciCapLib, PciCapPciSegmentLib, and PciCapPciIoLib classes to
their single respective instances under MdePkg. Later patches will use
those lib classes in OvmfPkg drivers.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
OvmfPkg/OvmfPkgIa32.dsc | 3 +++
OvmfPkg/OvmfPkgIa32X64.dsc | 3 +++
OvmfPkg/OvmfPkgX64.dsc | 3 +++
3 files changed, 9 insertions(+)
diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 2d6c4c4615b6..3a78e25bb529 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -127,6 +127,9 @@ [LibraryClasses]
PciExpressLib|MdePkg/Library/BasePciExpressLib/BasePciExpressLib.inf
PciLib|MdePkg/Library/BasePciLibCf8/BasePciLibCf8.inf
PciSegmentLib|MdePkg/Library/BasePciSegmentLibPci/BasePciSegmentLibPci.inf
+ PciCapLib|MdePkg/Library/BasePciCapLib/BasePciCapLib.inf
+ PciCapPciSegmentLib|MdePkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
+ PciCapPciIoLib|MdePkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index 43158c5f0627..51cad3959d45 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -132,6 +132,9 @@ [LibraryClasses]
PciExpressLib|MdePkg/Library/BasePciExpressLib/BasePciExpressLib.inf
PciLib|MdePkg/Library/BasePciLibCf8/BasePciLibCf8.inf
PciSegmentLib|MdePkg/Library/BasePciSegmentLibPci/BasePciSegmentLibPci.inf
+ PciCapLib|MdePkg/Library/BasePciCapLib/BasePciCapLib.inf
+ PciCapPciSegmentLib|MdePkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
+ PciCapPciIoLib|MdePkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index d1fdf7c307c2..7dd960770f9d 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -132,6 +132,9 @@ [LibraryClasses]
PciExpressLib|MdePkg/Library/BasePciExpressLib/BasePciExpressLib.inf
PciLib|MdePkg/Library/BasePciLibCf8/BasePciLibCf8.inf
PciSegmentLib|MdePkg/Library/BasePciSegmentLibPci/BasePciSegmentLibPci.inf
+ PciCapLib|MdePkg/Library/BasePciCapLib/BasePciCapLib.inf
+ PciCapPciSegmentLib|MdePkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
+ PciCapPciIoLib|MdePkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
--
2.14.1.3.gb7cf6e02401b
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 5/7] ArmVirtPkg: resolve PciCapLib, PciCapPciSegmentLib, PciCapPciIoLib
2018-05-04 21:36 [PATCH 0/7] MdePkg, OvmfPkg, ArmVirtPkg: add and use PCI(E) Capabilities Library Laszlo Ersek
` (3 preceding siblings ...)
2018-05-04 21:36 ` [PATCH 4/7] OvmfPkg: resolve PciCapLib, PciCapPciSegmentLib, PciCapPciIoLib Laszlo Ersek
@ 2018-05-04 21:36 ` Laszlo Ersek
2018-05-04 21:36 ` [PATCH 6/7] OvmfPkg/PciHotPlugInitDxe: convert to PciCapLib Laszlo Ersek
` (2 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Laszlo Ersek @ 2018-05-04 21:36 UTC (permalink / raw)
To: edk2-devel-01; +Cc: Ard Biesheuvel
Resolve the PciCapLib, PciCapPciSegmentLib, and PciCapPciIoLib classes to
their single respective instances under MdePkg. Later patches will use
those lib classes in OvmfPkg drivers, some of which are included in
ArmVirt platforms.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
ArmVirtPkg/ArmVirt.dsc.inc | 3 +++
1 file changed, 3 insertions(+)
diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
index 35bccb3dc1f4..5f622ce41710 100644
--- a/ArmVirtPkg/ArmVirt.dsc.inc
+++ b/ArmVirtPkg/ArmVirt.dsc.inc
@@ -131,6 +131,9 @@ [LibraryClasses.common]
# PCI Libraries
PciLib|MdePkg/Library/BasePciLibPciExpress/BasePciLibPciExpress.inf
PciExpressLib|ArmVirtPkg/Library/BaseCachingPciExpressLib/BaseCachingPciExpressLib.inf
+ PciCapLib|MdePkg/Library/BasePciCapLib/BasePciCapLib.inf
+ PciCapPciSegmentLib|MdePkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
+ PciCapPciIoLib|MdePkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
# USB Libraries
UefiUsbLib|MdePkg/Library/UefiUsbLib/UefiUsbLib.inf
--
2.14.1.3.gb7cf6e02401b
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 6/7] OvmfPkg/PciHotPlugInitDxe: convert to PciCapLib
2018-05-04 21:36 [PATCH 0/7] MdePkg, OvmfPkg, ArmVirtPkg: add and use PCI(E) Capabilities Library Laszlo Ersek
` (4 preceding siblings ...)
2018-05-04 21:36 ` [PATCH 5/7] ArmVirtPkg: " Laszlo Ersek
@ 2018-05-04 21:36 ` Laszlo Ersek
2018-05-04 21:36 ` [PATCH 7/7] OvmfPkg/Virtio10Dxe: " Laszlo Ersek
2018-05-14 13:15 ` [PATCH 0/7] MdePkg, OvmfPkg, ArmVirtPkg: add and use PCI(E) Capabilities Library Laszlo Ersek
7 siblings, 0 replies; 11+ messages in thread
From: Laszlo Ersek @ 2018-05-04 21:36 UTC (permalink / raw)
To: edk2-devel-01; +Cc: Ard Biesheuvel, Jordan Justen
Replace the manual capability list parsing in OvmfPkg/PciHotPlugInitDxe
with PciCapLib and PciCapPciSegmentLib API calls.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf | 5 +
OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c | 267 +++++++-------------
2 files changed, 102 insertions(+), 170 deletions(-)
diff --git a/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf b/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
index 38043986eb67..cc2b60d44263 100644
--- a/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
+++ b/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
@@ -35,6 +35,8 @@ [LibraryClasses]
DebugLib
DevicePathLib
MemoryAllocationLib
+ PciCapLib
+ PciCapPciSegmentLib
PciLib
UefiBootServicesTableLib
UefiDriverEntryPoint
@@ -42,5 +44,8 @@ [LibraryClasses]
[Protocols]
gEfiPciHotPlugInitProtocolGuid ## ALWAYS_PRODUCES
+[Pcd]
+ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId ## CONSUMES
+
[Depex]
TRUE
diff --git a/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c b/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c
index 177e1a62120d..3449796878ef 100644
--- a/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c
+++ b/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c
@@ -14,6 +14,7 @@
**/
#include <IndustryStandard/Acpi10.h>
+#include <IndustryStandard/Q35MchIch9.h>
#include <IndustryStandard/QemuPciBridgeCapabilities.h>
#include <Library/BaseLib.h>
@@ -21,12 +22,20 @@
#include <Library/DebugLib.h>
#include <Library/DevicePathLib.h>
#include <Library/MemoryAllocationLib.h>
+#include <Library/PciCapLib.h>
+#include <Library/PciCapPciSegmentLib.h>
#include <Library/PciLib.h>
#include <Library/UefiBootServicesTableLib.h>
#include <Protocol/PciHotPlugInit.h>
#include <Protocol/PciRootBridgeIo.h>
+//
+// TRUE if the PCI platform supports extended config space, FALSE otherwise.
+//
+STATIC BOOLEAN mPciExtConfSpaceSupported;
+
+
//
// The protocol interface this driver produces.
//
@@ -248,91 +257,11 @@ HighBitSetRoundUp64 (
}
-/**
- Read a slice from conventional PCI config space at the given offset, then
- advance the offset.
-
- @param[in] PciAddress The address of the PCI Device -- Bus, Device, Function
- -- in UEFI (not PciLib) encoding.
-
- @param[in,out] Offset On input, the offset in conventional PCI config space
- to start reading from. On output, the offset of the
- first byte that was not read.
-
- @param[in] Size The number of bytes to read.
-
- @param[out] Buffer On output, the bytes read from PCI config space are
- stored in this object.
-**/
-STATIC
-VOID
-ReadConfigSpace (
- IN CONST EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_PCI_ADDRESS *PciAddress,
- IN OUT UINT8 *Offset,
- IN UINT8 Size,
- OUT VOID *Buffer
- )
-{
- PciReadBuffer (
- PCI_LIB_ADDRESS (
- PciAddress->Bus,
- PciAddress->Device,
- PciAddress->Function,
- *Offset
- ),
- Size,
- Buffer
- );
- *Offset += Size;
-}
-
-
-/**
- Convenience wrapper macro for ReadConfigSpace().
-
- Given the following conditions:
-
- - HeaderField is the first field in the structure pointed-to by Struct,
-
- - Struct->HeaderField has been populated from the conventional PCI config
- space of the PCI device identified by PciAddress,
-
- - *Offset points one past HeaderField in the conventional PCI config space of
- the PCI device identified by PciAddress,
-
- populate the rest of *Struct from conventional PCI config space, starting at
- *Offset. Finally, increment *Offset so that it point one past *Struct.
-
- @param[in] PciAddress The address of the PCI Device -- Bus, Device, Function
- -- in UEFI (not PciLib) encoding. Type: pointer to
- CONST EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_PCI_ADDRESS.
-
- @param[in,out] Offset On input, the offset in conventional PCI config space
- to start reading from; one past Struct->HeaderField.
- On output, the offset of the first byte that was not
- read; one past *Struct. Type: pointer to UINT8.
-
- @param[out] Struct The structure to complete. Type: pointer to structure
- object.
-
- @param[in] HeaderField The name of the first field in *Struct, after which
- *Struct should be populated. Type: structure member
- identifier.
-**/
-#define COMPLETE_CONFIG_SPACE_STRUCT(PciAddress, Offset, Struct, HeaderField) \
- ReadConfigSpace ( \
- (PciAddress), \
- (Offset), \
- (UINT8)(sizeof *(Struct) - sizeof ((Struct)->HeaderField)), \
- &((Struct)->HeaderField) + 1 \
- )
-
-
/**
Look up the QEMU-specific Resource Reservation capability in the conventional
config space of a Hotplug Controller (that is, PCI Bridge).
- This function performs as few config space reads as possible.
+ On error, the contents of ReservationHint are indeterminate.
@param[in] HpcPciAddress The address of the PCI Bridge -- Bus, Device,
Function -- in UEFI (not PciLib) encoding.
@@ -343,8 +272,9 @@ ReadConfigSpace (
@retval EFI_SUCCESS The capability has been found, ReservationHint has
been populated.
- @retval EFI_NOT_FOUND The capability is missing. The contents of
- ReservationHint are now indeterminate.
+ @retval EFI_NOT_FOUND The capability is missing.
+
+ @return Error codes from PciCapPciSegmentLib and PciCapLib.
**/
STATIC
EFI_STATUS
@@ -353,10 +283,12 @@ QueryReservationHint (
OUT QEMU_PCI_BRIDGE_CAPABILITY_RESOURCE_RESERVATION *ReservationHint
)
{
- UINT16 PciVendorId;
- UINT16 PciStatus;
- UINT8 PciCapPtr;
- UINT8 Offset;
+ UINT16 PciVendorId;
+ EFI_STATUS Status;
+ PCI_CAP_DEV *PciDevice;
+ PCI_CAP_LIST *CapList;
+ UINT16 VendorInstance;
+ PCI_CAP *VendorCap;
//
// Check the vendor identifier.
@@ -374,108 +306,101 @@ QueryReservationHint (
}
//
- // Check the Capabilities List bit in the PCI Status Register.
+ // Parse the capabilities lists.
//
- PciStatus = PciRead16 (
- PCI_LIB_ADDRESS (
- HpcPciAddress->Bus,
- HpcPciAddress->Device,
- HpcPciAddress->Function,
- PCI_PRIMARY_STATUS_OFFSET
- )
- );
- if ((PciStatus & EFI_PCI_STATUS_CAPABILITY) == 0) {
- return EFI_NOT_FOUND;
+ Status = PciCapPciSegmentDeviceInit (
+ mPciExtConfSpaceSupported ? PciCapExtended : PciCapNormal,
+ 0, // Segment
+ HpcPciAddress->Bus,
+ HpcPciAddress->Device,
+ HpcPciAddress->Function,
+ &PciDevice
+ );
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+ Status = PciCapListInit (PciDevice, &CapList);
+ if (EFI_ERROR (Status)) {
+ goto UninitPciDevice;
}
//
- // Fetch the start of the Capabilities List.
- //
- PciCapPtr = PciRead8 (
- PCI_LIB_ADDRESS (
- HpcPciAddress->Bus,
- HpcPciAddress->Device,
- HpcPciAddress->Function,
- PCI_CAPBILITY_POINTER_OFFSET
- )
- );
-
- //
- // Scan the Capabilities List until we find the terminator element, or the
- // Resource Reservation capability.
+ // Scan the vendor capability instances for the Resource Reservation
+ // capability.
//
- for (Offset = PciCapPtr & 0xFC;
- Offset > 0;
- Offset = ReservationHint->BridgeHdr.VendorHdr.Hdr.NextItemPtr & 0xFC) {
- BOOLEAN EnoughRoom;
-
- //
- // Check if the Resource Reservation capability would fit into config space
- // at this offset.
- //
- EnoughRoom = (BOOLEAN)(
- Offset <= PCI_MAX_CONFIG_OFFSET - sizeof *ReservationHint
- );
+ VendorInstance = 0;
+ for (;;) {
+ UINT8 VendorLength;
+ UINT8 BridgeCapType;
- //
- // Read the standard capability header so we can check the capability ID
- // (if necessary) and advance to the next capability.
- //
- ReadConfigSpace (
- HpcPciAddress,
- &Offset,
- (UINT8)sizeof ReservationHint->BridgeHdr.VendorHdr.Hdr,
- &ReservationHint->BridgeHdr.VendorHdr.Hdr
- );
- if (!EnoughRoom ||
- (ReservationHint->BridgeHdr.VendorHdr.Hdr.CapabilityID !=
- EFI_PCI_CAPABILITY_ID_VENDOR)) {
- continue;
+ Status = PciCapListFindCap (
+ CapList,
+ PciCapNormal,
+ EFI_PCI_CAPABILITY_ID_VENDOR,
+ VendorInstance++,
+ &VendorCap
+ );
+ if (EFI_ERROR (Status)) {
+ goto UninitCapList;
}
//
- // Read the rest of the vendor capability header so we can check the
- // capability length.
+ // Check the vendor capability length.
//
- COMPLETE_CONFIG_SPACE_STRUCT (
- HpcPciAddress,
- &Offset,
- &ReservationHint->BridgeHdr.VendorHdr,
- Hdr
- );
- if (ReservationHint->BridgeHdr.VendorHdr.Length !=
- sizeof *ReservationHint) {
+ Status = PciCapRead (
+ PciDevice,
+ VendorCap,
+ OFFSET_OF (EFI_PCI_CAPABILITY_VENDOR_HDR, Length),
+ &VendorLength,
+ sizeof VendorLength
+ );
+ if (EFI_ERROR (Status)) {
+ goto UninitCapList;
+ }
+ if (VendorLength != sizeof *ReservationHint) {
continue;
}
//
- // Read the rest of the QEMU bridge capability header so we can check the
- // capability type.
+ // Check the vendor bridge capability type.
//
- COMPLETE_CONFIG_SPACE_STRUCT (
- HpcPciAddress,
- &Offset,
- &ReservationHint->BridgeHdr,
- VendorHdr
- );
- if (ReservationHint->BridgeHdr.Type !=
+ Status = PciCapRead (
+ PciDevice,
+ VendorCap,
+ OFFSET_OF (QEMU_PCI_BRIDGE_CAPABILITY_HDR, Type),
+ &BridgeCapType,
+ sizeof BridgeCapType
+ );
+ if (EFI_ERROR (Status)) {
+ goto UninitCapList;
+ }
+ if (BridgeCapType ==
QEMU_PCI_BRIDGE_CAPABILITY_TYPE_RESOURCE_RESERVATION) {
- continue;
+ //
+ // We have a match.
+ //
+ break;
}
-
- //
- // Read the body of the reservation hint.
- //
- COMPLETE_CONFIG_SPACE_STRUCT (
- HpcPciAddress,
- &Offset,
- ReservationHint,
- BridgeHdr
- );
- return EFI_SUCCESS;
}
- return EFI_NOT_FOUND;
+ //
+ // Populate ReservationHint.
+ //
+ Status = PciCapRead (
+ PciDevice,
+ VendorCap,
+ 0, // SourceOffsetInCap
+ ReservationHint,
+ sizeof *ReservationHint
+ );
+
+UninitCapList:
+ PciCapListUninit (CapList);
+
+UninitPciDevice:
+ PciCapPciSegmentDeviceUninit (PciDevice);
+
+ return Status;
}
@@ -870,6 +795,8 @@ DriverInitialize (
{
EFI_STATUS Status;
+ mPciExtConfSpaceSupported = (PcdGet16 (PcdOvmfHostBridgePciDevId) ==
+ INTEL_Q35_MCH_DEVICE_ID);
mPciHotPlugInit.GetRootHpcList = GetRootHpcList;
mPciHotPlugInit.InitializeRootHpc = InitializeRootHpc;
mPciHotPlugInit.GetResourcePadding = GetResourcePadding;
--
2.14.1.3.gb7cf6e02401b
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 7/7] OvmfPkg/Virtio10Dxe: convert to PciCapLib
2018-05-04 21:36 [PATCH 0/7] MdePkg, OvmfPkg, ArmVirtPkg: add and use PCI(E) Capabilities Library Laszlo Ersek
` (5 preceding siblings ...)
2018-05-04 21:36 ` [PATCH 6/7] OvmfPkg/PciHotPlugInitDxe: convert to PciCapLib Laszlo Ersek
@ 2018-05-04 21:36 ` Laszlo Ersek
2018-05-14 13:15 ` [PATCH 0/7] MdePkg, OvmfPkg, ArmVirtPkg: add and use PCI(E) Capabilities Library Laszlo Ersek
7 siblings, 0 replies; 11+ messages in thread
From: Laszlo Ersek @ 2018-05-04 21:36 UTC (permalink / raw)
To: edk2-devel-01; +Cc: Ard Biesheuvel, Jordan Justen
Replace the manual capability list parsing in OvmfPkg/Virtio10Dxe with
PciCapLib and PciCapPciIoLib API calls.
The VIRTIO_PCI_CAP_LINK structure type is now superfluous. (Well, it
always has been; we should have used EFI_PCI_CAPABILITY_HDR.)
Also, EFI_PCI_CAPABILITY_VENDOR_HDR is now included at the front of
VIRTIO_PCI_CAP. No driver other than Virtio10Dxe relies on VIRTIO_PCI_CAP.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
OvmfPkg/Virtio10Dxe/Virtio10.inf | 2 +
OvmfPkg/Include/IndustryStandard/Virtio10.h | 7 +-
OvmfPkg/Virtio10Dxe/Virtio10.c | 135 +++++++-------------
3 files changed, 52 insertions(+), 92 deletions(-)
diff --git a/OvmfPkg/Virtio10Dxe/Virtio10.inf b/OvmfPkg/Virtio10Dxe/Virtio10.inf
index c4ef15d94bfc..db0cb1189a29 100644
--- a/OvmfPkg/Virtio10Dxe/Virtio10.inf
+++ b/OvmfPkg/Virtio10Dxe/Virtio10.inf
@@ -32,6 +32,8 @@ [LibraryClasses]
BaseMemoryLib
DebugLib
MemoryAllocationLib
+ PciCapLib
+ PciCapPciIoLib
UefiBootServicesTableLib
UefiDriverEntryPoint
UefiLib
diff --git a/OvmfPkg/Include/IndustryStandard/Virtio10.h b/OvmfPkg/Include/IndustryStandard/Virtio10.h
index c5efb5cfcb8a..7d51aa36b326 100644
--- a/OvmfPkg/Include/IndustryStandard/Virtio10.h
+++ b/OvmfPkg/Include/IndustryStandard/Virtio10.h
@@ -16,6 +16,7 @@
#ifndef _VIRTIO_1_0_H_
#define _VIRTIO_1_0_H_
+#include <IndustryStandard/Pci23.h>
#include <IndustryStandard/Virtio095.h>
//
@@ -29,11 +30,7 @@
//
#pragma pack (1)
typedef struct {
- UINT8 CapId; // Capability identifier (generic)
- UINT8 CapNext; // Link to next capability (generic)
-} VIRTIO_PCI_CAP_LINK;
-
-typedef struct {
+ EFI_PCI_CAPABILITY_VENDOR_HDR VendorHdr;
UINT8 ConfigType; // Identifies the specific VirtIo 1.0 config structure
UINT8 Bar; // The BAR that contains the structure
UINT8 Padding[3];
diff --git a/OvmfPkg/Virtio10Dxe/Virtio10.c b/OvmfPkg/Virtio10Dxe/Virtio10.c
index e9b50b6e437b..9ebb72c76bfd 100644
--- a/OvmfPkg/Virtio10Dxe/Virtio10.c
+++ b/OvmfPkg/Virtio10Dxe/Virtio10.c
@@ -21,6 +21,8 @@
#include <Library/BaseMemoryLib.h>
#include <Library/DebugLib.h>
#include <Library/MemoryAllocationLib.h>
+#include <Library/PciCapLib.h>
+#include <Library/PciCapPciIoLib.h>
#include <Library/UefiBootServicesTableLib.h>
#include <Library/UefiLib.h>
@@ -184,48 +186,6 @@ GetBarType (
}
-/**
- Read a slice from PCI config space at the given offset, then advance the
- offset.
-
- @param [in] PciIo The EFI_PCI_IO_PROTOCOL instance that represents the
- device.
-
- @param [in,out] Offset On input, the offset in PCI config space to start
- reading from. On output, the offset of the first byte
- that was not read. On error, Offset is not modified.
-
- @param [in] Size The number of bytes to read.
-
- @param [out] Buffer On output, the bytes read from PCI config space are
- stored in this object.
-
- @retval EFI_SUCCESS Size bytes have been transferred from PCI config space
- (from Offset) to Buffer, and Offset has been incremented
- by Size.
-
- @return Error codes from PciIo->Pci.Read().
-**/
-STATIC
-EFI_STATUS
-ReadConfigSpace (
- IN EFI_PCI_IO_PROTOCOL *PciIo,
- IN OUT UINT32 *Offset,
- IN UINTN Size,
- OUT VOID *Buffer
- )
-{
- EFI_STATUS Status;
-
- Status = PciIo->Pci.Read (PciIo, EfiPciIoWidthUint8, *Offset, Size, Buffer);
- if (EFI_ERROR (Status)) {
- return Status;
- }
- *Offset += (UINT32)Size;
- return EFI_SUCCESS;
-}
-
-
/*
Traverse the PCI capabilities list of a virtio-1.0 device, and capture the
locations of the interesting virtio-1.0 register blocks.
@@ -239,57 +199,51 @@ ReadConfigSpace (
will have been updated from the PCI
capabilities found.
- @param[in] CapabilityPtr The offset of the first capability in PCI
- config space, taken from the standard PCI
- device header.
-
@retval EFI_SUCCESS Traversal successful.
- @return Error codes from the ReadConfigSpace() and GetBarType()
- helper functions.
+ @return Error codes from PciCapPciIoLib, PciCapLib, and the
+ GetBarType() helper function.
*/
STATIC
EFI_STATUS
ParseCapabilities (
- IN OUT VIRTIO_1_0_DEV *Device,
- IN UINT8 CapabilityPtr
+ IN OUT VIRTIO_1_0_DEV *Device
)
{
- UINT32 Offset;
- VIRTIO_PCI_CAP_LINK CapLink;
+ EFI_STATUS Status;
+ PCI_CAP_DEV *PciDevice;
+ PCI_CAP_LIST *CapList;
+ UINT16 VendorInstance;
+ PCI_CAP *VendorCap;
- for (Offset = CapabilityPtr & 0xFC;
- Offset > 0;
- Offset = CapLink.CapNext & 0xFC
- ) {
- EFI_STATUS Status;
+ Status = PciCapPciIoDeviceInit (Device->PciIo, &PciDevice);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+ Status = PciCapListInit (PciDevice, &CapList);
+ if (EFI_ERROR (Status)) {
+ goto UninitPciDevice;
+ }
+
+ for (VendorInstance = 0;
+ !EFI_ERROR (PciCapListFindCap (CapList, PciCapNormal,
+ EFI_PCI_CAPABILITY_ID_VENDOR, VendorInstance,
+ &VendorCap));
+ VendorInstance++) {
UINT8 CapLen;
VIRTIO_PCI_CAP VirtIoCap;
VIRTIO_1_0_CONFIG *ParsedConfig;
- //
- // Read capability identifier and link to next capability.
- //
- Status = ReadConfigSpace (Device->PciIo, &Offset, sizeof CapLink,
- &CapLink);
- if (EFI_ERROR (Status)) {
- return Status;
- }
- if (CapLink.CapId != 0x09) {
- //
- // Not a vendor-specific capability, move to the next one.
- //
- continue;
- }
-
//
// Big enough to accommodate a VIRTIO_PCI_CAP structure?
//
- Status = ReadConfigSpace (Device->PciIo, &Offset, sizeof CapLen, &CapLen);
+ Status = PciCapRead (PciDevice, VendorCap,
+ OFFSET_OF (EFI_PCI_CAPABILITY_VENDOR_HDR, Length), &CapLen,
+ sizeof CapLen);
if (EFI_ERROR (Status)) {
- return Status;
+ goto UninitCapList;
}
- if (CapLen < sizeof CapLink + sizeof CapLen + sizeof VirtIoCap) {
+ if (CapLen < sizeof VirtIoCap) {
//
// Too small, move to next.
//
@@ -299,11 +253,11 @@ ParseCapabilities (
//
// Read interesting part of capability.
//
- Status = ReadConfigSpace (Device->PciIo, &Offset, sizeof VirtIoCap,
- &VirtIoCap);
+ Status = PciCapRead (PciDevice, VendorCap, 0, &VirtIoCap, sizeof VirtIoCap);
if (EFI_ERROR (Status)) {
- return Status;
+ goto UninitCapList;
}
+
switch (VirtIoCap.ConfigType) {
case VIRTIO_PCI_CAP_COMMON_CFG:
ParsedConfig = &Device->CommonConfig;
@@ -326,7 +280,7 @@ ParseCapabilities (
//
Status = GetBarType (Device->PciIo, VirtIoCap.Bar, &ParsedConfig->BarType);
if (EFI_ERROR (Status)) {
- return Status;
+ goto UninitCapList;
}
ParsedConfig->Bar = VirtIoCap.Bar;
ParsedConfig->Offset = VirtIoCap.Offset;
@@ -337,19 +291,18 @@ ParseCapabilities (
// This capability has an additional field called NotifyOffsetMultiplier;
// parse it too.
//
- if (CapLen < sizeof CapLink + sizeof CapLen + sizeof VirtIoCap +
- sizeof Device->NotifyOffsetMultiplier) {
+ if (CapLen < sizeof VirtIoCap + sizeof Device->NotifyOffsetMultiplier) {
//
// Too small, move to next.
//
continue;
}
- Status = ReadConfigSpace (Device->PciIo, &Offset,
- sizeof Device->NotifyOffsetMultiplier,
- &Device->NotifyOffsetMultiplier);
+ Status = PciCapRead (PciDevice, VendorCap, sizeof VirtIoCap,
+ &Device->NotifyOffsetMultiplier,
+ sizeof Device->NotifyOffsetMultiplier);
if (EFI_ERROR (Status)) {
- return Status;
+ goto UninitCapList;
}
}
@@ -359,7 +312,15 @@ ParseCapabilities (
ParsedConfig->Exists = TRUE;
}
- return EFI_SUCCESS;
+ ASSERT_EFI_ERROR (Status);
+
+UninitCapList:
+ PciCapListUninit (CapList);
+
+UninitPciDevice:
+ PciCapPciIoDeviceUninit (PciDevice);
+
+ return Status;
}
@@ -1015,7 +976,7 @@ Virtio10BindingStart (
Device->VirtIo.SubSystemDeviceId = Pci.Hdr.DeviceId - 0x1040;
- Status = ParseCapabilities (Device, Pci.Device.CapabilityPtr);
+ Status = ParseCapabilities (Device);
if (EFI_ERROR (Status)) {
goto ClosePciIo;
}
--
2.14.1.3.gb7cf6e02401b
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 0/7] MdePkg, OvmfPkg, ArmVirtPkg: add and use PCI(E) Capabilities Library
2018-05-04 21:36 [PATCH 0/7] MdePkg, OvmfPkg, ArmVirtPkg: add and use PCI(E) Capabilities Library Laszlo Ersek
` (6 preceding siblings ...)
2018-05-04 21:36 ` [PATCH 7/7] OvmfPkg/Virtio10Dxe: " Laszlo Ersek
@ 2018-05-14 13:15 ` Laszlo Ersek
2018-05-14 15:06 ` Gao, Liming
7 siblings, 1 reply; 11+ messages in thread
From: Laszlo Ersek @ 2018-05-14 13:15 UTC (permalink / raw)
To: edk2-devel-01; +Cc: Michael D Kinney, Jordan Justen, Liming Gao, Ard Biesheuvel
ping
On 05/04/18 23:36, Laszlo Ersek wrote:
> Repo: https://github.com/lersek/edk2.git
> Branch: pci_cap
>
> In message [1], Jordan suggested a general "capabilities helper lib that
> could iterate & read the PCI capability structs". Patch #1 introduces
> that library (class and central BASE instance) to MdePkg; the library
> class is called PciCapLib.
>
> [1] http://mid.mail-archive.com/150696619010.2454.9266470245604846575@jljusten-skl
>
> PciCapLib offers APIs to parse capabilities lists, and to locate,
> describe, read and write capabilities.
>
> The library class targets a wide range of client module types; for
> example, platform PEIMs, platform DXE drivers, and UEFI drivers that
> follow the UEFI Driver Model. For this reason, the library class is
> designed with PCI config access abstracted away, even beyond the PPI /
> Protocol level, since those cannot cross the PEI / DXE boundary.
>
> This (minimal) config space access method abstraction is called
> PCI_CAP_DEV. It is *conceptually* a protocol. Client modules plug
> PCI_CAP_DEV instances into PciCapLib for config space access, similarly
> to how DiskIo and BlockIo instances are plugged into filesystem drivers.
> Similarly to the filesystem driver example, PciCapLib is an example for
> the "strategy pattern".
>
> Patches #2 and #3 introduce small "factory" libraries that help client
> modules produce PCI_CAP_DEV instances:
>
> - PciCapPciSegmentLib -- incl. class and central BASE instance -- from
> patch #2 produces PCI_CAP_DEV instances on top of PciSegmentLib, using
> the "Segment:Bus:Device.Function" notation. This is meant for platform
> modules (PEIMs and DXE drivers), which generally have a working
> PciSegmentLib instance available.
>
> - PciCapPciIoLib -- incl. class and central UEFI instance -- from patch
> #3 lets client modules produce PCI_CAP_DEV instances on top of
> EFI_PCI_IO_PROTOCOL instances. This is meant for UEFI drivers.
>
> Patches #4 and #5 resolve the three new lib classes to their respective
> central instances in the OvmfPkg and ArmVirtPkg DSC files.
>
> Patch #6 converts OvmfPkg/PciHotPlugInitDxe to the new APIs, from manual
> capability list parsing. Because PciHotPlugInitDxe is a platform DXE
> driver, this conversion uses PciCapPciSegmentLib and PciCapLib.
>
> Patch #7 converts OvmfPkg/Virtio10Dxe to the new APIs, from manual
> capability list parsing. Because Virtio10Dxe is a UEFI driver, this
> conversion uses PciCapPciIoLib and PciCapLib.
>
> ----[ jump to the patches now, and return if you have questions: I
> have answers, but you might find them too verbose in advance ]----
>
> Q1: If PCI config access needs to be abstracted from PciCapLib, then why
> don't we provide multiple instances of PciCapLib? In other words,
> why the PCI_CAP_DEV "protocol" rather than multiple instances of
> PciCapLib?
>
> A1: The point of lib classes and instances is that the class provides a
> common interface, and the instances provide different
> implementations. We have the opposite use case here (hence the
> "strategy pattern"): the implementation of the higher level
> algorithm is common, and the objects that plug into it differ at the
> *interface* level ("Segment:Bus:Device.Function" notation versus
> PciIo protocol).
>
> None of those notations are suitable for inclusion in the PciCapLib
> *class*; a more abstract interface is needed for identifying the PCI
> device to the library, for parsing of capabilities lists.
>
> Q2: OK, let's say we can't (and shouldn't) include both notations in the
> PciCapLib class; can we settle on one of them, rather than
> PCI_CAP_DEV?
>
> A2: No, we can't. PciIo is obviously unusable in PEIMs (and in quite a
> few platform DXE drivers too).
>
> It takes a bit longer to explain why the other choice, i.e. adopting
> the "Segment:Bus:Device.Function" notation for the PciCapLib
> interface, is not good either. It has to do with the portability of
> UEFI drivers that bind PciIo protocol instances:
>
> It is indeed easy enough to call PciIo->GetLocation() and plug the
> output values into a theoretical "Segment:Bus:Device.Function"
> interface. However, about the only two things that could consume
> such location identifiers and actually *implement* config space
> access would be:
>
> (a) the EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL.Pci.Read/Write member
> functions, where the member functions themselves take the
> "Bus:Device.Function" part, and the "Segment" part must be used
> for selecting the EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL instance
> itself, through the SegmentNumber member;
>
> (b) the PciSegmentLib interfaces.
>
> Building a PciCapLib instance that uses (a) into a UEFI driver is a
> bad idea, because there are platforms that provide PciIo instances
> -- hence the driver can generally run on them -- but don't provide
> PciRootBridgeIo instances.
>
> Building a PciCapLib instance that uses (b) into a UEFI driver is
> worse. The PciSegmentLib interfaces are not capable of returning
> errors. For example, if a device appears as PCI Express (implying
> its extended config space exists, potentially containing extended
> capabilities), but the underlying platform does not support extended
> config space access (for example because it only supports
> 0xCF8/0xCFC, and not ECAM), then peeking at the extended config
> space of the device may invoke undefined behavior within
> PciSegmentLib, and the PciCapLib instance will be none the wiser.
>
> (In fact, for the same reason, even PciCapPciSegmentLib has to
> extend the "Segment:Bus:Device.Function" notation slightly; so that
> such issues are caught *before* the call is made to PciSegmentLib.)
>
> Q3: Wait, I thought it wasn't possible for a PCI Express device to
> appear on a platform that lacked ECAM.
>
> A3: Is that a question? :) Yes, it is plenty possible [2]. Same way as
> garbage / looped config spaces are [3].
>
> [2] https://github.com/tianocore/edk2/commit/014b472053ae3
> [3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=02d578e5edd9
>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>
> Thanks,
> Laszlo
>
> Laszlo Ersek (7):
> MdePkg: introduce PciCapLib
> MdePkg: introduce PciCapPciSegmentLib
> MdePkg: introduce PciCapPciIoLib
> OvmfPkg: resolve PciCapLib, PciCapPciSegmentLib, PciCapPciIoLib
> ArmVirtPkg: resolve PciCapLib, PciCapPciSegmentLib, PciCapPciIoLib
> OvmfPkg/PciHotPlugInitDxe: convert to PciCapLib
> OvmfPkg/Virtio10Dxe: convert to PciCapLib
>
> ArmVirtPkg/ArmVirt.dsc.inc | 3 +
> MdePkg/Include/Library/PciCapLib.h | 429 +++++++++
> MdePkg/Include/Library/PciCapPciIoLib.h | 58 ++
> MdePkg/Include/Library/PciCapPciSegmentLib.h | 82 ++
> MdePkg/Library/BasePciCapLib/BasePciCapLib.c | 1007 ++++++++++++++++++++
> MdePkg/Library/BasePciCapLib/BasePciCapLib.h | 60 ++
> MdePkg/Library/BasePciCapLib/BasePciCapLib.inf | 37 +
> MdePkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.c | 226 +++++
> MdePkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.h | 47 +
> MdePkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf | 34 +
> MdePkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.c | 243 +++++
> MdePkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.h | 44 +
> MdePkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf | 35 +
> MdePkg/MdePkg.dec | 14 +
> MdePkg/MdePkg.dsc | 3 +
> OvmfPkg/Include/IndustryStandard/Virtio10.h | 7 +-
> OvmfPkg/OvmfPkgIa32.dsc | 3 +
> OvmfPkg/OvmfPkgIa32X64.dsc | 3 +
> OvmfPkg/OvmfPkgX64.dsc | 3 +
> OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c | 267 ++----
> OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf | 5 +
> OvmfPkg/Virtio10Dxe/Virtio10.c | 135 +--
> OvmfPkg/Virtio10Dxe/Virtio10.inf | 2 +
> 23 files changed, 2485 insertions(+), 262 deletions(-)
> create mode 100644 MdePkg/Include/Library/PciCapLib.h
> create mode 100644 MdePkg/Include/Library/PciCapPciIoLib.h
> create mode 100644 MdePkg/Include/Library/PciCapPciSegmentLib.h
> create mode 100644 MdePkg/Library/BasePciCapLib/BasePciCapLib.c
> create mode 100644 MdePkg/Library/BasePciCapLib/BasePciCapLib.h
> create mode 100644 MdePkg/Library/BasePciCapLib/BasePciCapLib.inf
> create mode 100644 MdePkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.c
> create mode 100644 MdePkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.h
> create mode 100644 MdePkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
> create mode 100644 MdePkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.c
> create mode 100644 MdePkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.h
> create mode 100644 MdePkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/7] MdePkg, OvmfPkg, ArmVirtPkg: add and use PCI(E) Capabilities Library
2018-05-14 13:15 ` [PATCH 0/7] MdePkg, OvmfPkg, ArmVirtPkg: add and use PCI(E) Capabilities Library Laszlo Ersek
@ 2018-05-14 15:06 ` Gao, Liming
2018-05-14 17:19 ` Laszlo Ersek
0 siblings, 1 reply; 11+ messages in thread
From: Gao, Liming @ 2018-05-14 15:06 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel-01
Cc: Kinney, Michael D, Justen, Jordan L, Ard Biesheuvel
Laszlo:
I have not fully understood its goal and design. Could you give me one more week to review it?
Thanks
Liming
> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Monday, May 14, 2018 9:16 PM
> To: edk2-devel-01 <edk2-devel@lists.01.org>
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>; Gao, Liming
> <liming.gao@intel.com>; Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Subject: Re: [edk2] [PATCH 0/7] MdePkg, OvmfPkg, ArmVirtPkg: add and use PCI(E) Capabilities Library
>
> ping
>
> On 05/04/18 23:36, Laszlo Ersek wrote:
> > Repo: https://github.com/lersek/edk2.git
> > Branch: pci_cap
> >
> > In message [1], Jordan suggested a general "capabilities helper lib that
> > could iterate & read the PCI capability structs". Patch #1 introduces
> > that library (class and central BASE instance) to MdePkg; the library
> > class is called PciCapLib.
> >
> > [1] http://mid.mail-archive.com/150696619010.2454.9266470245604846575@jljusten-skl
> >
> > PciCapLib offers APIs to parse capabilities lists, and to locate,
> > describe, read and write capabilities.
> >
> > The library class targets a wide range of client module types; for
> > example, platform PEIMs, platform DXE drivers, and UEFI drivers that
> > follow the UEFI Driver Model. For this reason, the library class is
> > designed with PCI config access abstracted away, even beyond the PPI /
> > Protocol level, since those cannot cross the PEI / DXE boundary.
> >
> > This (minimal) config space access method abstraction is called
> > PCI_CAP_DEV. It is *conceptually* a protocol. Client modules plug
> > PCI_CAP_DEV instances into PciCapLib for config space access, similarly
> > to how DiskIo and BlockIo instances are plugged into filesystem drivers.
> > Similarly to the filesystem driver example, PciCapLib is an example for
> > the "strategy pattern".
> >
> > Patches #2 and #3 introduce small "factory" libraries that help client
> > modules produce PCI_CAP_DEV instances:
> >
> > - PciCapPciSegmentLib -- incl. class and central BASE instance -- from
> > patch #2 produces PCI_CAP_DEV instances on top of PciSegmentLib, using
> > the "Segment:Bus:Device.Function" notation. This is meant for platform
> > modules (PEIMs and DXE drivers), which generally have a working
> > PciSegmentLib instance available.
> >
> > - PciCapPciIoLib -- incl. class and central UEFI instance -- from patch
> > #3 lets client modules produce PCI_CAP_DEV instances on top of
> > EFI_PCI_IO_PROTOCOL instances. This is meant for UEFI drivers.
> >
> > Patches #4 and #5 resolve the three new lib classes to their respective
> > central instances in the OvmfPkg and ArmVirtPkg DSC files.
> >
> > Patch #6 converts OvmfPkg/PciHotPlugInitDxe to the new APIs, from manual
> > capability list parsing. Because PciHotPlugInitDxe is a platform DXE
> > driver, this conversion uses PciCapPciSegmentLib and PciCapLib.
> >
> > Patch #7 converts OvmfPkg/Virtio10Dxe to the new APIs, from manual
> > capability list parsing. Because Virtio10Dxe is a UEFI driver, this
> > conversion uses PciCapPciIoLib and PciCapLib.
> >
> > ----[ jump to the patches now, and return if you have questions: I
> > have answers, but you might find them too verbose in advance ]----
> >
> > Q1: If PCI config access needs to be abstracted from PciCapLib, then why
> > don't we provide multiple instances of PciCapLib? In other words,
> > why the PCI_CAP_DEV "protocol" rather than multiple instances of
> > PciCapLib?
> >
> > A1: The point of lib classes and instances is that the class provides a
> > common interface, and the instances provide different
> > implementations. We have the opposite use case here (hence the
> > "strategy pattern"): the implementation of the higher level
> > algorithm is common, and the objects that plug into it differ at the
> > *interface* level ("Segment:Bus:Device.Function" notation versus
> > PciIo protocol).
> >
> > None of those notations are suitable for inclusion in the PciCapLib
> > *class*; a more abstract interface is needed for identifying the PCI
> > device to the library, for parsing of capabilities lists.
> >
> > Q2: OK, let's say we can't (and shouldn't) include both notations in the
> > PciCapLib class; can we settle on one of them, rather than
> > PCI_CAP_DEV?
> >
> > A2: No, we can't. PciIo is obviously unusable in PEIMs (and in quite a
> > few platform DXE drivers too).
> >
> > It takes a bit longer to explain why the other choice, i.e. adopting
> > the "Segment:Bus:Device.Function" notation for the PciCapLib
> > interface, is not good either. It has to do with the portability of
> > UEFI drivers that bind PciIo protocol instances:
> >
> > It is indeed easy enough to call PciIo->GetLocation() and plug the
> > output values into a theoretical "Segment:Bus:Device.Function"
> > interface. However, about the only two things that could consume
> > such location identifiers and actually *implement* config space
> > access would be:
> >
> > (a) the EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL.Pci.Read/Write member
> > functions, where the member functions themselves take the
> > "Bus:Device.Function" part, and the "Segment" part must be used
> > for selecting the EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL instance
> > itself, through the SegmentNumber member;
> >
> > (b) the PciSegmentLib interfaces.
> >
> > Building a PciCapLib instance that uses (a) into a UEFI driver is a
> > bad idea, because there are platforms that provide PciIo instances
> > -- hence the driver can generally run on them -- but don't provide
> > PciRootBridgeIo instances.
> >
> > Building a PciCapLib instance that uses (b) into a UEFI driver is
> > worse. The PciSegmentLib interfaces are not capable of returning
> > errors. For example, if a device appears as PCI Express (implying
> > its extended config space exists, potentially containing extended
> > capabilities), but the underlying platform does not support extended
> > config space access (for example because it only supports
> > 0xCF8/0xCFC, and not ECAM), then peeking at the extended config
> > space of the device may invoke undefined behavior within
> > PciSegmentLib, and the PciCapLib instance will be none the wiser.
> >
> > (In fact, for the same reason, even PciCapPciSegmentLib has to
> > extend the "Segment:Bus:Device.Function" notation slightly; so that
> > such issues are caught *before* the call is made to PciSegmentLib.)
> >
> > Q3: Wait, I thought it wasn't possible for a PCI Express device to
> > appear on a platform that lacked ECAM.
> >
> > A3: Is that a question? :) Yes, it is plenty possible [2]. Same way as
> > garbage / looped config spaces are [3].
> >
> > [2] https://github.com/tianocore/edk2/commit/014b472053ae3
> > [3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=02d578e5edd9
> >
> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > Cc: Jordan Justen <jordan.l.justen@intel.com>
> > Cc: Liming Gao <liming.gao@intel.com>
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> >
> > Thanks,
> > Laszlo
> >
> > Laszlo Ersek (7):
> > MdePkg: introduce PciCapLib
> > MdePkg: introduce PciCapPciSegmentLib
> > MdePkg: introduce PciCapPciIoLib
> > OvmfPkg: resolve PciCapLib, PciCapPciSegmentLib, PciCapPciIoLib
> > ArmVirtPkg: resolve PciCapLib, PciCapPciSegmentLib, PciCapPciIoLib
> > OvmfPkg/PciHotPlugInitDxe: convert to PciCapLib
> > OvmfPkg/Virtio10Dxe: convert to PciCapLib
> >
> > ArmVirtPkg/ArmVirt.dsc.inc | 3 +
> > MdePkg/Include/Library/PciCapLib.h | 429 +++++++++
> > MdePkg/Include/Library/PciCapPciIoLib.h | 58 ++
> > MdePkg/Include/Library/PciCapPciSegmentLib.h | 82 ++
> > MdePkg/Library/BasePciCapLib/BasePciCapLib.c | 1007 ++++++++++++++++++++
> > MdePkg/Library/BasePciCapLib/BasePciCapLib.h | 60 ++
> > MdePkg/Library/BasePciCapLib/BasePciCapLib.inf | 37 +
> > MdePkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.c | 226 +++++
> > MdePkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.h | 47 +
> > MdePkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf | 34 +
> > MdePkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.c | 243 +++++
> > MdePkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.h | 44 +
> > MdePkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf | 35 +
> > MdePkg/MdePkg.dec | 14 +
> > MdePkg/MdePkg.dsc | 3 +
> > OvmfPkg/Include/IndustryStandard/Virtio10.h | 7 +-
> > OvmfPkg/OvmfPkgIa32.dsc | 3 +
> > OvmfPkg/OvmfPkgIa32X64.dsc | 3 +
> > OvmfPkg/OvmfPkgX64.dsc | 3 +
> > OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c | 267 ++----
> > OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf | 5 +
> > OvmfPkg/Virtio10Dxe/Virtio10.c | 135 +--
> > OvmfPkg/Virtio10Dxe/Virtio10.inf | 2 +
> > 23 files changed, 2485 insertions(+), 262 deletions(-)
> > create mode 100644 MdePkg/Include/Library/PciCapLib.h
> > create mode 100644 MdePkg/Include/Library/PciCapPciIoLib.h
> > create mode 100644 MdePkg/Include/Library/PciCapPciSegmentLib.h
> > create mode 100644 MdePkg/Library/BasePciCapLib/BasePciCapLib.c
> > create mode 100644 MdePkg/Library/BasePciCapLib/BasePciCapLib.h
> > create mode 100644 MdePkg/Library/BasePciCapLib/BasePciCapLib.inf
> > create mode 100644 MdePkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.c
> > create mode 100644 MdePkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.h
> > create mode 100644 MdePkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
> > create mode 100644 MdePkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.c
> > create mode 100644 MdePkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.h
> > create mode 100644 MdePkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
> >
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/7] MdePkg, OvmfPkg, ArmVirtPkg: add and use PCI(E) Capabilities Library
2018-05-14 15:06 ` Gao, Liming
@ 2018-05-14 17:19 ` Laszlo Ersek
0 siblings, 0 replies; 11+ messages in thread
From: Laszlo Ersek @ 2018-05-14 17:19 UTC (permalink / raw)
To: Gao, Liming, edk2-devel-01
Cc: Kinney, Michael D, Justen, Jordan L, Ard Biesheuvel
On 05/14/18 17:06, Gao, Liming wrote:
> Laszlo:
> I have not fully understood its goal and design. Could you give me one more week to review it?
Sure! Thank you.
Laszlo
>> -----Original Message-----
>> From: Laszlo Ersek [mailto:lersek@redhat.com]
>> Sent: Monday, May 14, 2018 9:16 PM
>> To: edk2-devel-01 <edk2-devel@lists.01.org>
>> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>; Gao, Liming
>> <liming.gao@intel.com>; Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> Subject: Re: [edk2] [PATCH 0/7] MdePkg, OvmfPkg, ArmVirtPkg: add and use PCI(E) Capabilities Library
>>
>> ping
>>
>> On 05/04/18 23:36, Laszlo Ersek wrote:
>>> Repo: https://github.com/lersek/edk2.git
>>> Branch: pci_cap
>>>
>>> In message [1], Jordan suggested a general "capabilities helper lib that
>>> could iterate & read the PCI capability structs". Patch #1 introduces
>>> that library (class and central BASE instance) to MdePkg; the library
>>> class is called PciCapLib.
>>>
>>> [1] http://mid.mail-archive.com/150696619010.2454.9266470245604846575@jljusten-skl
>>>
>>> PciCapLib offers APIs to parse capabilities lists, and to locate,
>>> describe, read and write capabilities.
>>>
>>> The library class targets a wide range of client module types; for
>>> example, platform PEIMs, platform DXE drivers, and UEFI drivers that
>>> follow the UEFI Driver Model. For this reason, the library class is
>>> designed with PCI config access abstracted away, even beyond the PPI /
>>> Protocol level, since those cannot cross the PEI / DXE boundary.
>>>
>>> This (minimal) config space access method abstraction is called
>>> PCI_CAP_DEV. It is *conceptually* a protocol. Client modules plug
>>> PCI_CAP_DEV instances into PciCapLib for config space access, similarly
>>> to how DiskIo and BlockIo instances are plugged into filesystem drivers.
>>> Similarly to the filesystem driver example, PciCapLib is an example for
>>> the "strategy pattern".
>>>
>>> Patches #2 and #3 introduce small "factory" libraries that help client
>>> modules produce PCI_CAP_DEV instances:
>>>
>>> - PciCapPciSegmentLib -- incl. class and central BASE instance -- from
>>> patch #2 produces PCI_CAP_DEV instances on top of PciSegmentLib, using
>>> the "Segment:Bus:Device.Function" notation. This is meant for platform
>>> modules (PEIMs and DXE drivers), which generally have a working
>>> PciSegmentLib instance available.
>>>
>>> - PciCapPciIoLib -- incl. class and central UEFI instance -- from patch
>>> #3 lets client modules produce PCI_CAP_DEV instances on top of
>>> EFI_PCI_IO_PROTOCOL instances. This is meant for UEFI drivers.
>>>
>>> Patches #4 and #5 resolve the three new lib classes to their respective
>>> central instances in the OvmfPkg and ArmVirtPkg DSC files.
>>>
>>> Patch #6 converts OvmfPkg/PciHotPlugInitDxe to the new APIs, from manual
>>> capability list parsing. Because PciHotPlugInitDxe is a platform DXE
>>> driver, this conversion uses PciCapPciSegmentLib and PciCapLib.
>>>
>>> Patch #7 converts OvmfPkg/Virtio10Dxe to the new APIs, from manual
>>> capability list parsing. Because Virtio10Dxe is a UEFI driver, this
>>> conversion uses PciCapPciIoLib and PciCapLib.
>>>
>>> ----[ jump to the patches now, and return if you have questions: I
>>> have answers, but you might find them too verbose in advance ]----
>>>
>>> Q1: If PCI config access needs to be abstracted from PciCapLib, then why
>>> don't we provide multiple instances of PciCapLib? In other words,
>>> why the PCI_CAP_DEV "protocol" rather than multiple instances of
>>> PciCapLib?
>>>
>>> A1: The point of lib classes and instances is that the class provides a
>>> common interface, and the instances provide different
>>> implementations. We have the opposite use case here (hence the
>>> "strategy pattern"): the implementation of the higher level
>>> algorithm is common, and the objects that plug into it differ at the
>>> *interface* level ("Segment:Bus:Device.Function" notation versus
>>> PciIo protocol).
>>>
>>> None of those notations are suitable for inclusion in the PciCapLib
>>> *class*; a more abstract interface is needed for identifying the PCI
>>> device to the library, for parsing of capabilities lists.
>>>
>>> Q2: OK, let's say we can't (and shouldn't) include both notations in the
>>> PciCapLib class; can we settle on one of them, rather than
>>> PCI_CAP_DEV?
>>>
>>> A2: No, we can't. PciIo is obviously unusable in PEIMs (and in quite a
>>> few platform DXE drivers too).
>>>
>>> It takes a bit longer to explain why the other choice, i.e. adopting
>>> the "Segment:Bus:Device.Function" notation for the PciCapLib
>>> interface, is not good either. It has to do with the portability of
>>> UEFI drivers that bind PciIo protocol instances:
>>>
>>> It is indeed easy enough to call PciIo->GetLocation() and plug the
>>> output values into a theoretical "Segment:Bus:Device.Function"
>>> interface. However, about the only two things that could consume
>>> such location identifiers and actually *implement* config space
>>> access would be:
>>>
>>> (a) the EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL.Pci.Read/Write member
>>> functions, where the member functions themselves take the
>>> "Bus:Device.Function" part, and the "Segment" part must be used
>>> for selecting the EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL instance
>>> itself, through the SegmentNumber member;
>>>
>>> (b) the PciSegmentLib interfaces.
>>>
>>> Building a PciCapLib instance that uses (a) into a UEFI driver is a
>>> bad idea, because there are platforms that provide PciIo instances
>>> -- hence the driver can generally run on them -- but don't provide
>>> PciRootBridgeIo instances.
>>>
>>> Building a PciCapLib instance that uses (b) into a UEFI driver is
>>> worse. The PciSegmentLib interfaces are not capable of returning
>>> errors. For example, if a device appears as PCI Express (implying
>>> its extended config space exists, potentially containing extended
>>> capabilities), but the underlying platform does not support extended
>>> config space access (for example because it only supports
>>> 0xCF8/0xCFC, and not ECAM), then peeking at the extended config
>>> space of the device may invoke undefined behavior within
>>> PciSegmentLib, and the PciCapLib instance will be none the wiser.
>>>
>>> (In fact, for the same reason, even PciCapPciSegmentLib has to
>>> extend the "Segment:Bus:Device.Function" notation slightly; so that
>>> such issues are caught *before* the call is made to PciSegmentLib.)
>>>
>>> Q3: Wait, I thought it wasn't possible for a PCI Express device to
>>> appear on a platform that lacked ECAM.
>>>
>>> A3: Is that a question? :) Yes, it is plenty possible [2]. Same way as
>>> garbage / looped config spaces are [3].
>>>
>>> [2] https://github.com/tianocore/edk2/commit/014b472053ae3
>>> [3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=02d578e5edd9
>>>
>>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>>> Cc: Liming Gao <liming.gao@intel.com>
>>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>>>
>>> Thanks,
>>> Laszlo
>>>
>>> Laszlo Ersek (7):
>>> MdePkg: introduce PciCapLib
>>> MdePkg: introduce PciCapPciSegmentLib
>>> MdePkg: introduce PciCapPciIoLib
>>> OvmfPkg: resolve PciCapLib, PciCapPciSegmentLib, PciCapPciIoLib
>>> ArmVirtPkg: resolve PciCapLib, PciCapPciSegmentLib, PciCapPciIoLib
>>> OvmfPkg/PciHotPlugInitDxe: convert to PciCapLib
>>> OvmfPkg/Virtio10Dxe: convert to PciCapLib
>>>
>>> ArmVirtPkg/ArmVirt.dsc.inc | 3 +
>>> MdePkg/Include/Library/PciCapLib.h | 429 +++++++++
>>> MdePkg/Include/Library/PciCapPciIoLib.h | 58 ++
>>> MdePkg/Include/Library/PciCapPciSegmentLib.h | 82 ++
>>> MdePkg/Library/BasePciCapLib/BasePciCapLib.c | 1007 ++++++++++++++++++++
>>> MdePkg/Library/BasePciCapLib/BasePciCapLib.h | 60 ++
>>> MdePkg/Library/BasePciCapLib/BasePciCapLib.inf | 37 +
>>> MdePkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.c | 226 +++++
>>> MdePkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.h | 47 +
>>> MdePkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf | 34 +
>>> MdePkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.c | 243 +++++
>>> MdePkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.h | 44 +
>>> MdePkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf | 35 +
>>> MdePkg/MdePkg.dec | 14 +
>>> MdePkg/MdePkg.dsc | 3 +
>>> OvmfPkg/Include/IndustryStandard/Virtio10.h | 7 +-
>>> OvmfPkg/OvmfPkgIa32.dsc | 3 +
>>> OvmfPkg/OvmfPkgIa32X64.dsc | 3 +
>>> OvmfPkg/OvmfPkgX64.dsc | 3 +
>>> OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c | 267 ++----
>>> OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf | 5 +
>>> OvmfPkg/Virtio10Dxe/Virtio10.c | 135 +--
>>> OvmfPkg/Virtio10Dxe/Virtio10.inf | 2 +
>>> 23 files changed, 2485 insertions(+), 262 deletions(-)
>>> create mode 100644 MdePkg/Include/Library/PciCapLib.h
>>> create mode 100644 MdePkg/Include/Library/PciCapPciIoLib.h
>>> create mode 100644 MdePkg/Include/Library/PciCapPciSegmentLib.h
>>> create mode 100644 MdePkg/Library/BasePciCapLib/BasePciCapLib.c
>>> create mode 100644 MdePkg/Library/BasePciCapLib/BasePciCapLib.h
>>> create mode 100644 MdePkg/Library/BasePciCapLib/BasePciCapLib.inf
>>> create mode 100644 MdePkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.c
>>> create mode 100644 MdePkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.h
>>> create mode 100644 MdePkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
>>> create mode 100644 MdePkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.c
>>> create mode 100644 MdePkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.h
>>> create mode 100644 MdePkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
>>>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2018-05-14 17:19 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-05-04 21:36 [PATCH 0/7] MdePkg, OvmfPkg, ArmVirtPkg: add and use PCI(E) Capabilities Library Laszlo Ersek
2018-05-04 21:36 ` [PATCH 1/7] MdePkg: introduce PciCapLib Laszlo Ersek
2018-05-04 21:36 ` [PATCH 2/7] MdePkg: introduce PciCapPciSegmentLib Laszlo Ersek
2018-05-04 21:36 ` [PATCH 3/7] MdePkg: introduce PciCapPciIoLib Laszlo Ersek
2018-05-04 21:36 ` [PATCH 4/7] OvmfPkg: resolve PciCapLib, PciCapPciSegmentLib, PciCapPciIoLib Laszlo Ersek
2018-05-04 21:36 ` [PATCH 5/7] ArmVirtPkg: " Laszlo Ersek
2018-05-04 21:36 ` [PATCH 6/7] OvmfPkg/PciHotPlugInitDxe: convert to PciCapLib Laszlo Ersek
2018-05-04 21:36 ` [PATCH 7/7] OvmfPkg/Virtio10Dxe: " Laszlo Ersek
2018-05-14 13:15 ` [PATCH 0/7] MdePkg, OvmfPkg, ArmVirtPkg: add and use PCI(E) Capabilities Library Laszlo Ersek
2018-05-14 15:06 ` Gao, Liming
2018-05-14 17:19 ` Laszlo Ersek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox