Re: [PATCH edk2-platforms 34/41] Silicon/NXP: Implement PciSegmentLib to support multiple RCs

[Leif Lindholm <leif.lindholm@linaro.org>]

On Wed, Nov 28, 2018 at 08:31:48PM +0530, Meenakshi Aggarwal wrote:
> From: Vabhav <vabhav.sharma@nxp.com>
> 
> Multiple root complex support is not provided by standard library
> PciLib/PciExpressLib/PciSegmentLib, Reimplementing it and provide
> function for reading/writing into PCIe configuration Space.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Vabhav <vabhav.sharma@nxp.com>
> Signed-off-by: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>
> ---
>  Silicon/NXP/Include/NxpPcie.h                      | 146 +++++
>  Silicon/NXP/Library/PciSegmentLib/PciSegmentLib.c  | 611 +++++++++++++++++++++
>  .../NXP/Library/PciSegmentLib/PciSegmentLib.inf    |  41 ++
>  3 files changed, 798 insertions(+)
>  create mode 100644 Silicon/NXP/Include/NxpPcie.h
>  create mode 100644 Silicon/NXP/Library/PciSegmentLib/PciSegmentLib.c
>  create mode 100644 Silicon/NXP/Library/PciSegmentLib/PciSegmentLib.inf
> 
> diff --git a/Silicon/NXP/Include/NxpPcie.h b/Silicon/NXP/Include/NxpPcie.h
> new file mode 100644
> index 0000000..a0beefe
> --- /dev/null
> +++ b/Silicon/NXP/Include/NxpPcie.h
> @@ -0,0 +1,146 @@
> +/** @file
> +  PCI memory configuration for NXP
> +
> +  Copyright 2018 NXP
> +
> +  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 __NXP_PCIE_H__
> +#define __NXP_PCIE_H__
> +
> +// Segment 0
> +#define PCI_SEG0_NUM              0
> +#define PCI_SEG0_MMIO32_MIN       0x40000000
> +#define PCI_SEG0_MMIO32_MAX       0x4fffffff
> +#define PCI_SEG0_MMIO64_MIN       PCI_SEG0_MMIO_MEMBASE + \
> +                                  SEG_MEM_SIZE + \
> +                                  MEM64_BASE
> +#define PCI_SEG0_MMIO64_MAX       PCI_SEG0_MMIO64_MIN + MEM64_LIMIT
> +#define PCI_SEG0_MMIO_MEMBASE     FixedPcdGet64 (PcdPciExp1BaseAddr)
> +#define PCI_SEG0_DBI_BASE         0x03400000
> +#define PCI_SEG0_MMIO_OFFSET      0x0
> +#define PCI_SEG0_PORTIO_MEMBASE   PCI_SEG0_MMIO_MEMBASE + SEG_IO_SIZE
> +#define PCI_SEG0_PORTIO_OFFSET    0x0
> +
> +// Segment 1
> +#define PCI_SEG1_NUM              1
> +#define PCI_SEG1_MMIO32_MIN       0x40000000
> +#define PCI_SEG1_MMIO32_MAX       0x4fffffff
> +#define PCI_SEG1_MMIO64_MIN       PCI_SEG1_MMIO_MEMBASE + \
> +                                  SEG_MEM_SIZE + \
> +                                  MEM64_BASE
> +#define PCI_SEG1_MMIO64_MAX       PCI_SEG1_MMIO64_MIN + MEM64_LIMIT
> +#define PCI_SEG1_MMIO_MEMBASE     FixedPcdGet64 (PcdPciExp2BaseAddr)
> +#define PCI_SEG1_DBI_BASE         0x03500000
> +#define PCI_SEG1_MMIO_OFFSET      0x10000000
> +#define PCI_SEG1_PORTIO_MEMBASE   PCI_SEG1_MMIO_MEMBASE + SEG_IO_SIZE
> +#define PCI_SEG1_PORTIO_OFFSET    0x10000
> +
> +// Segment 2
> +#define PCI_SEG2_NUM              2
> +#define PCI_SEG2_MMIO32_MIN       0x40000000
> +#define PCI_SEG2_MMIO32_MAX       0x4fffffff
> +#define PCI_SEG2_MMIO64_MIN       PCI_SEG2_MMIO_MEMBASE + \
> +                                  SEG_MEM_SIZE + \
> +                                  MEM64_BASE
> +#define PCI_SEG2_MMIO64_MAX       PCI_SEG2_MMIO64_MIN + MEM64_LIMIT
> +#define PCI_SEG2_MMIO_MEMBASE     FixedPcdGet64 (PcdPciExp3BaseAddr)
> +#define PCI_SEG2_DBI_BASE         0x03600000
> +#define PCI_SEG2_MMIO_OFFSET      0x20000000
> +#define PCI_SEG2_PORTIO_MEMBASE   PCI_SEG2_MMIO_MEMBASE + SEG_IO_SIZE
> +#define PCI_SEG2_PORTIO_OFFSET    0x20000
> +
> +// Segment 3
> +#define PCI_SEG3_NUM              3
> +#define PCI_SEG3_MMIO32_MIN       0x40000000
> +#define PCI_SEG3_MMIO32_MAX       0x4fffffff
> +#define PCI_SEG3_MMIO64_MIN       PCI_SEG3_MMIO_MEMBASE + \
> +                                  SEG_MEM_SIZE + \
> +                                  MEM64_BASE
> +#define PCI_SEG3_MMIO64_MAX       PCI_SEG3_MMIO64_MIN + MEM64_LIMIT
> +#define PCI_SEG3_MMIO_MEMBASE     FixedPcdGet64 (PcdPciExp4BaseAddr)
> +#define PCI_SEG3_DBI_BASE         0x03700000
> +#define PCI_SEG3_MMIO_OFFSET      0x30000000
> +#define PCI_SEG3_PORTIO_MEMBASE   PCI_SEG3_MMIO_MEMBASE + SEG_IO_SIZE
> +#define PCI_SEG3_PORTIO_OFFSET    0x30000
> +
> +// Segment configuration
> +#define PCI_SEG_BUSNUM_MIN        0x0
> +#define PCI_SEG_BUSNUM_MAX        0xff
> +#define PCI_SEG_PORTIO_MIN        0x0
> +#define PCI_SEG_PORTIO_MAX        0xffff
> +#define PCI_SEG_MMIO32_MIN        0x40000000
> +#define PCI_SEG_MMIO32_MAX        0x4fffffff
> +#define PCI_SEG_MMIO32_DIFF       0x10000000
> +#define PCI_SEG_MMIO64_MAX_DIFF   0x3fffffff
> +#define SEG_CFG_SIZE              0x00001000
> +#define SEG_CFG_BUS               0x00000000
> +#define SEG_MEM_SIZE              0x40000000
> +#define SEG_MEM_LIMIT             0x7fffffff
> +#define SEG_MEM_BUS               0x40000000
> +#define SEG_IO_SIZE               0x00010000
> +#define SEG_IO_BUS                0x00000000
> +#define PCI_SEG_PORTIO_LIMIT      (NUM_PCIE_CONTROLLER * SEG_IO_SIZE) + \
> +                                  PCI_SEG_PORTIO_MAX
> +#define PCI_BASE_DIFF             0x800000000
> +#define PCI_DBI_SIZE_DIFF         0x100000
> +#define PCI_SEG0_PHY_CFG0_BASE    PCI_SEG0_MMIO_MEMBASE
> +#define PCI_SEG0_PHY_CFG1_BASE    PCI_SEG0_PHY_CFG0_BASE + SEG_CFG_SIZE
> +#define PCI_SEG0_PHY_MEM_BASE     PCI_SEG0_MMIO64_MIN - MEM64_BASE
> +#define PCI_SEG0_PHY_MEM64_BASE   PCI_SEG0_MMIO64_MIN
> +#define PCI_SEG0_PHY_IO_BASE      PCI_SEG0_MMIO_MEMBASE + SEG_IO_SIZE
> +#define MEM64_BASE                0xC0000000  // MMIO64 starts at 4GB offset
> +#define MEM64_LIMIT               0x1FFFFFFFF
> +#define SEG_MEM64_BASE            0x100000000
> +
> +// iATU configuration
> +#define IATU_VIEWPORT_OFF                            0x900
> +#define IATU_VIEWPORT_OUTBOUND                       0
> +
> +#define IATU_REGION_CTRL_1_OFF_OUTBOUND_0            0x904
> +#define IATU_REGION_CTRL_1_OFF_OUTBOUND_0_TYPE_MEM   0x0
> +#define IATU_REGION_CTRL_1_OFF_OUTBOUND_0_TYPE_IO    0x2
> +#define IATU_REGION_CTRL_1_OFF_OUTBOUND_0_TYPE_CFG0  0x4
> +#define IATU_REGION_CTRL_1_OFF_OUTBOUND_0_TYPE_CFG1  0x5
> +
> +#define IATU_REGION_CTRL_2_OFF_OUTBOUND_0            0x908
> +#define IATU_REGION_CTRL_2_OFF_OUTBOUND_0_REGION_EN  BIT31
> +
> +#define IATU_LWR_BASE_ADDR_OFF_OUTBOUND_0            0x90C
> +#define IATU_UPPER_BASE_ADDR_OFF_OUTBOUND_0          0x910
> +#define IATU_LIMIT_ADDR_OFF_OUTBOUND_0               0x914
> +#define IATU_LWR_TARGET_ADDR_OFF_OUTBOUND_0          0x918
> +#define IATU_UPPER_TARGET_ADDR_OFF_OUTBOUND_0        0x91C
> +
> +#define IATU_REGION_INDEX0                           0x0
> +#define IATU_REGION_INDEX1                           0x1
> +#define IATU_REGION_INDEX2                           0x2
> +#define IATU_REGION_INDEX3                           0x3
> +#define IATU_REGION_INDEX4                           0x4
> +
> +// PCIe Controller configuration
> +#define NUM_PCIE_CONTROLLER  FixedPcdGet32 (PcdNumPciController)
> +#define PCI_LUT_DBG          FixedPcdGet32 (PcdPcieLutDbg)
> +#define PCI_LUT_BASE         FixedPcdGet32 (PcdPcieLutBase)
> +#define LTSSM_STATE_MASK     0x3f
> +#define LTSSM_PCIE_L0        0x11
> +#define PCI_LINK_CAP         0x7c
> +#define PCI_LINK_SPEED_MASK  0xf
> +#define PCI_CLASS_BRIDGE_PCI 0x6040010
> +#define PCI_CLASS_DEVICE     0x8
> +#define PCI_DBI_RO_WR_EN     0x8bc
> +#define PCI_BASE_ADDRESS_0   0x10
> +
> +VOID GetSerdesProtocolMaps (UINT64 *);
> +
> +BOOLEAN IsSerDesLaneProtocolConfigured (UINT64, UINT16);
> +
> +#endif
> diff --git a/Silicon/NXP/Library/PciSegmentLib/PciSegmentLib.c b/Silicon/NXP/Library/PciSegmentLib/PciSegmentLib.c
> new file mode 100644
> index 0000000..3a3e24a
> --- /dev/null
> +++ b/Silicon/NXP/Library/PciSegmentLib/PciSegmentLib.c
> @@ -0,0 +1,611 @@
> +/** @file
> +  PCI Segment Library for NXP SoCs with multiple RCs
> +
> +  Copyright 2018 NXP
> +
> +  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 <Base.h>
> +#include <Library/PciSegmentLib.h>
> +#include <Library/BaseLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/IoLib.h>
> +#include <Library/PcdLib.h>
> +#include <NxpPcie.h>
> +
> +typedef enum {
> +  PciCfgWidthUint8      = 0,
> +  PciCfgWidthUint16,
> +  PciCfgWidthUint32,
> +  PciCfgWidthMax
> +} PCI_CFG_WIDTH;

Please move to local include file.

> +
> +/**
> +  Assert the validity of a PCI Segment address.
> +  A valid PCI Segment address should not contain 1's in bits 28..31 and 48..63

Why?
Because of the memory map in the SoC?
If so, please list those requirements rather than just which bits are affected.

> +
> +  @param  A The address to validate.
> +  @param  M Additional bits to assert to be zero.
> +
> +**/
> +#define ASSERT_INVALID_PCI_SEGMENT_ADDRESS(A,M) \
> +  ASSERT (((A) & (0xffff0000f0000000ULL | (M))) == 0)

Please move to local include file.

> +
> +/**
> +  Function to return PCIe Physical Address(PCIe view) or Controller
> +  Address(CPU view) for different RCs
> +
> +  @param  Address Address passed from bus layer.
> +
> +  @return Return PCIe CPU or Controller address.
> +
> +**/
> +STATIC
> +UINT64
> +PciSegmentLibGetConfigBase (
> +  IN UINT64 Address
> +  )
> +{
> +
> +  UINT16 Segment;
> +
> +  //
> +  // Reading Segment number(47-32 bits) in Address
> +  //
> +  Segment = (Address >> 32);

Assumes 63-48 are zero.
Could this be a macro, which also did proper masking?

> +
> +  switch (Segment) {
> +    // Root Complex 1
> +    case PCI_SEG0_NUM:
> +      // Reading bus number(bits 20-27)
> +      if ((Address >> 20) & 1) {

No, this is reading bit 20. With a comment describing why 0000001 is
MMIO_MBASE and anything else is DBI_BASE, that's probably fine. But
the comment does not reflect the operation performed.

Could this be a macro?

> +        return PCI_SEG0_MMIO_MEMBASE;
> +      } else {
> +        // On Bus 0 RCs are connected
> +        return PCI_SEG0_DBI_BASE;
> +      }
> +    // Root Complex 2
> +    case PCI_SEG1_NUM:
> +      // Reading bus number(bits 20-27)
> +      if ((Address >> 20) & 1) {

Same comment as above.
Could use the same macro.

> +        return PCI_SEG1_MMIO_MEMBASE;
> +      } else {
> +        // On Bus 0 RCs are connected
> +        return PCI_SEG1_DBI_BASE;
> +      }
> +    // Root Complex 3
> +    case PCI_SEG2_NUM:
> +      // Reading bus number(bits 20-27)
> +      if ((Address >> 20) & 1) {

Same comment as above.
Could use the same macro.

> +        return PCI_SEG2_MMIO_MEMBASE;
> +      } else {
> +        // On Bus 0 RCs are connected
> +        return PCI_SEG2_DBI_BASE;
> +      }
> +    // Root Complex 4
> +    case PCI_SEG3_NUM:
> +      // Reading bus number(bits 20-27)
> +      if ((Address >> 20) & 1) {

Same comment as above.
Could use the same macro.

> +        return PCI_SEG3_MMIO_MEMBASE;
> +      } else {
> +        // On Bus 0 RCs are connected
> +        return PCI_SEG3_DBI_BASE;
> +      }
> +    default:
> +      return 0;

PLease use a define to show you're returning an error and not the
address 0.

> +  }
> +
> +}
> +
> +/**
> +  Internal worker function to ignore device
> +
> +  @param  Address The address that encodes BDF
> +
> +  @return TRUE to ignore the devices
> +
> +**/
> +STATIC
> +BOOLEAN
> +IgnoreDevices (
> +  IN UINT64 Address
> +  )
> +{
> +  //
> +  // ignore devices > 0 on bus 0
> +  //
> +  if ((Address & 0xff00000) == 0 && (Address & 0xf8000) != 0) {

Macros please.

> +    return TRUE;
> +  }
> +
> +  //
> +  // ignore device > 0 on bus 1
> +  //
> +  if ((Address & 0xfe00000) == 0 && (Address & 0xf8000) != 0) {

Macros please.

> +    return TRUE;
> +  }
> +
> +  return FALSE;
> +}
> +
> +/**
> +  Internal worker function to read a PCI configuration register.
> +
> +  @param  Address The address that encodes the Segment, PCI Bus, Device,
> +                  Function and Register.
> +  @param  Width   The width of data to read
> +
> +  @return The value read from the PCI configuration register.
> +
> +**/
> +STATIC
> +UINT32
> +PciSegmentLibReadWorker (
> +  IN  UINT64         Address,
> +  IN  PCI_CFG_WIDTH  Width
> +  )
> +{
> +  UINT64 Base;
> +  UINT16 Offset;
> +
> +  //
> +  // Reading Function(12-0) bits in Address
> +  //
> +  Offset = (Address & (SIZE_4KB - 1));

This would read bits 11-0.

> +
> +  Base = PciSegmentLibGetConfigBase (Address);
> +
> +  if (IgnoreDevices (Address)) {
> +    return MAX_UINT32;
> +  }
> +
> +  switch (Width) {
> +  case PciCfgWidthUint8:
> +    return MmioRead8 (Base + Offset);
> +  case PciCfgWidthUint16:
> +    return MmioRead16 (Base + Offset);
> +  case PciCfgWidthUint32:
> +    return MmioRead32 (Base + Offset);
> +  default:
> +    ASSERT (FALSE);
> +  }
> +
> +  return CHAR_NULL;

This is not a string parsing function, please choose (or create) a
better define.

> +}
> +
> +/**
> +  Internal worker function to writes a PCI configuration register.
> +
> +  @param  Address The address that encodes the Segment, PCI Bus, Device,
> +                  Function and Register.
> +  @param  Width   The width of data to write
> +  @param  Data    The value to write.
> +
> +  @return The value written to the PCI configuration register.
> +
> +**/
> +STATIC
> +UINT32
> +PciSegmentLibWriteWorker (
> +  IN  UINT64         Address,
> +  IN  PCI_CFG_WIDTH  Width,
> +  IN  UINT32         Data
> +  )
> +{
> +  UINT64 Base;
> +  UINT32 Offset;
> +
> +  //
> +  // Reading Function(12-0 bits) in Address
> +  //
> +  Offset = (Address & (SIZE_4KB - 1));

11-0.

> +
> +  Base = PciSegmentLibGetConfigBase (Address);
> +
> +  if (IgnoreDevices (Address)) {
> +    return MAX_UINT32;
> +  }
> +
> +  switch (Width) {
> +  case PciCfgWidthUint8:
> +    MmioWrite8 (Base + Offset, Data);
> +    break;
> +  case PciCfgWidthUint16:
> +    MmioWrite16 (Base + Offset, Data);
> +    break;
> +  case PciCfgWidthUint32:
> +    MmioWrite32 (Base + Offset, Data);
> +    break;
> +  default:
> +    ASSERT (FALSE);

This means we're failing silently if (somehow) we ended up attempting 
a 64-bit write. That feels less than ideal.

> +  }
> +
> +  return Data;
> +}
> +
> +/**
> +  Register a PCI device so PCI configuration registers may be accessed after
> +  SetVirtualAddressMap().
> +
> +  If any reserved bits in Address are set, then ASSERT().
> +
> +  @param  Address                  The address that encodes the PCI Bus, Device,
> +                                   Function and Register.
> +
> +  @retval RETURN_SUCCESS           The PCI device was registered for runtime access.
> +  @retval RETURN_UNSUPPORTED       An attempt was made to call this function
> +                                   after ExitBootServices().
> +  @retval RETURN_UNSUPPORTED       The resources required to access the PCI device
> +                                   at runtime could not be mapped.
> +  @retval RETURN_OUT_OF_RESOURCES  There are not enough resources available to
> +                                   complete the registration.
> +
> +**/
> +RETURN_STATUS
> +EFIAPI
> +PciSegmentRegisterForRuntimeAccess (
> +  IN UINTN  Address
> +  )
> +{
> +  ASSERT_INVALID_PCI_SEGMENT_ADDRESS (Address, 0);
> +  return RETURN_UNSUPPORTED;
> +}
> +
> +/**
> +  Reads an 8-bit PCI configuration register.
> +
> +  Reads and returns the 8-bit PCI configuration register specified by Address.
> +
> +  If any reserved bits in Address are set, then ASSERT().
> +
> +  @param  Address   The address that encodes the PCI Segment, Bus, Device, Function,
> +                    and Register.
> +
> +  @return The 8-bit PCI configuration register specified by Address.
> +
> +**/
> +UINT8
> +EFIAPI
> +PciSegmentRead8 (
> +  IN UINT64                    Address
> +  )
> +{
> +  ASSERT_INVALID_PCI_SEGMENT_ADDRESS (Address, 0);
> +
> +  return (UINT8) PciSegmentLibReadWorker (Address, PciCfgWidthUint8);

No space after (UINT8).

> +}
> +
> +/**
> +  Writes an 8-bit PCI configuration register.
> +
> +  Writes the 8-bit PCI configuration register specified by Address with the value specified by Value.
> +  Value is returned.  This function must guarantee that all PCI read and write operations are serialized.
> +
> +  If any reserved bits in Address are set, then ASSERT().
> +
> +  @param  Address     The address that encodes the PCI Segment, Bus, Device, Function, and Register.
> +  @param  Value       The value to write.
> +
> +  @return The value written to the PCI configuration register.
> +
> +**/
> +UINT8
> +EFIAPI
> +PciSegmentWrite8 (
> +  IN UINT64                    Address,
> +  IN UINT8                     Value
> +  )
> +{
> +  ASSERT_INVALID_PCI_SEGMENT_ADDRESS (Address, 0);
> +
> +  return (UINT8) PciSegmentLibWriteWorker (Address, PciCfgWidthUint8, Value);

No space after (UINT8).

> +}
> +
> +/**
> +  Reads a 16-bit PCI configuration register.
> +
> +  Reads and returns the 16-bit PCI configuration register specified by Address.
> +
> +  If any reserved bits in Address are set, then ASSERT().
> +  If Address is not aligned on a 16-bit boundary, then ASSERT().
> +
> +  @param  Address   The address that encodes the PCI Segment, Bus, Device, Function, and Register.
> +
> +  @return The 16-bit PCI configuration register specified by Address.
> +
> +**/
> +UINT16
> +EFIAPI
> +PciSegmentRead16 (
> +  IN UINT64                    Address
> +  )
> +{
> +  ASSERT_INVALID_PCI_SEGMENT_ADDRESS (Address, 1);

This demonstrates the opaqueness of the macro's "additional bits to
check". There is zero chance anyone who didn't just look at that macro
would understand the meaning of that 1.

Please create macros ALIGN_8, ALIGN_16, ALIGN_32 and use those.

> +
> +  return (UINT16) PciSegmentLibReadWorker (Address, PciCfgWidthUint16);

No space after (UINT16).
(Please address throughout.)

/
    Leif

> +}
> +
> +/**
> +  Writes a 16-bit PCI configuration register.
> +
> +  Writes the 16-bit PCI configuration register specified by Address with the
> +  value specified by Value.
> +
> +  Value is returned.
> +
> +  If any reserved bits in Address are set, then ASSERT().
> +  If Address is not aligned on a 16-bit boundary, then ASSERT().
> +
> +  @param  Address     The address that encodes the PCI Segment, Bus, Device, Function, and Register.
> +  @param  Value       The value to write.
> +
> +  @return The parameter of Value.
> +
> +**/
> +UINT16
> +EFIAPI
> +PciSegmentWrite16 (
> +  IN UINT64                    Address,
> +  IN UINT16                    Value
> +  )
> +{
> +  ASSERT_INVALID_PCI_SEGMENT_ADDRESS (Address, 1);
> +
> +  return (UINT16) PciSegmentLibWriteWorker (Address, PciCfgWidthUint16, Value);
> +}
> +
> +/**
> +  Reads a 32-bit PCI configuration register.
> +
> +  Reads and returns the 32-bit PCI configuration register specified by Address.
> +
> +  If any reserved bits in Address are set, then ASSERT().
> +  If Address is not aligned on a 32-bit boundary, then ASSERT().
> +
> +  @param  Address   The address that encodes the PCI Segment, Bus, Device, Function,
> +                    and Register.
> +
> +  @return The 32-bit PCI configuration register specified by Address.
> +
> +**/
> +UINT32
> +EFIAPI
> +PciSegmentRead32 (
> +  IN UINT64                    Address
> +  )
> +{
> +  ASSERT_INVALID_PCI_SEGMENT_ADDRESS (Address, 3);
> +
> +  return PciSegmentLibReadWorker (Address, PciCfgWidthUint32);
> +}
> +
> +/**
> +  Writes a 32-bit PCI configuration register.
> +
> +  Writes the 32-bit PCI configuration register specified by Address with the
> +  value specified by Value.
> +
> +  Value is returned.
> +
> +  If any reserved bits in Address are set, then ASSERT().
> +  If Address is not aligned on a 32-bit boundary, then ASSERT().
> +
> +  @param  Address     The address that encodes the PCI Segment, Bus, Device,
> +                      Function, and Register.
> +  @param  Value       The value to write.
> +
> +  @return The parameter of Value.
> +
> +**/
> +UINT32
> +EFIAPI
> +PciSegmentWrite32 (
> +  IN UINT64                    Address,
> +  IN UINT32                    Value
> +  )
> +{
> +  ASSERT_INVALID_PCI_SEGMENT_ADDRESS (Address, 3);
> +
> +  return PciSegmentLibWriteWorker (Address, PciCfgWidthUint32, Value);
> +}
> +
> +/**
> +  Reads a range of PCI configuration registers into a caller supplied buffer.
> +
> +  Reads the range of PCI configuration registers specified by StartAddress and
> +  Size into the buffer specified by Buffer. This function only allows the PCI
> +  configuration registers from a single PCI function to be read. Size is
> +  returned.
> +
> +  If any reserved bits in StartAddress are set, then ASSERT().
> +  If ((StartAddress & 0xFFF) + Size) > 0x1000, then ASSERT().
> +  If Size > 0 and Buffer is NULL, then ASSERT().
> +
> +  @param  StartAddress  The starting address that encodes the PCI Segment, Bus,
> +                        Device, Function and Register.
> +  @param  Size          The size in bytes of the transfer.
> +  @param  Buffer        The pointer to a buffer receiving the data read.
> +
> +  @return Size
> +
> +**/
> +UINTN
> +EFIAPI
> +PciSegmentReadBuffer (
> +  IN  UINT64                   StartAddress,
> +  IN  UINTN                    Size,
> +  OUT VOID                     *Buffer
> +  )
> +{
> +  UINTN                             ReturnValue;
> +
> +  ASSERT_INVALID_PCI_SEGMENT_ADDRESS (StartAddress, 0);
> +  // 0xFFF is used as limit for 4KB config space
> +  ASSERT (((StartAddress & (SIZE_4KB - 1)) + Size) <= SIZE_4KB);
> +
> +  if (Size == 0) {
> +    return Size;
> +  }
> +
> +  ASSERT (Buffer != NULL);
> +
> +  //
> +  // Save Size for return
> +  //
> +  ReturnValue = Size;
> +
> +  if ((StartAddress & BIT0) != 0) {
> +    //
> +    // Read a byte if StartAddress is byte aligned
> +    //
> +    *(UINT8 *)Buffer = PciSegmentRead8 (StartAddress);
> +    StartAddress += sizeof (UINT8);
> +    Size -= sizeof (UINT8);
> +    Buffer += sizeof (UINT8);
> +  }
> +
> +  if (Size >= sizeof (UINT16) && (StartAddress & BIT1) != 0) {
> +    //
> +    // Read a word if StartAddress is word aligned
> +    //
> +    WriteUnaligned16 (Buffer, PciSegmentRead16 (StartAddress));
> +    StartAddress += sizeof (UINT16);
> +    Size -= sizeof (UINT16);
> +    Buffer += sizeof (UINT16);
> +  }
> +
> +  while (Size >= sizeof (UINT32)) {
> +    //
> +    // Read as many double words as possible
> +    //
> +    WriteUnaligned32 (Buffer, PciSegmentRead32 (StartAddress));
> +    StartAddress += sizeof (UINT32);
> +    Size -= sizeof (UINT32);
> +    Buffer += sizeof (UINT32);
> +  }
> +
> +  if (Size >= sizeof (UINT16)) {
> +    //
> +    // Read the last remaining word if exist
> +    //
> +    WriteUnaligned16 (Buffer, PciSegmentRead16 (StartAddress));
> +    StartAddress += sizeof (UINT16);
> +    Size -= sizeof (UINT16);
> +    Buffer += sizeof (UINT16);
> +  }
> +
> +  if (Size >= sizeof (UINT8)) {
> +    //
> +    // Read the last remaining byte if exist
> +    //
> +    *(UINT8 *)Buffer = PciSegmentRead8 (StartAddress);
> +  }
> +
> +  return ReturnValue;
> +}
> +
> +
> +/**
> +  Copies the data in a caller supplied buffer to a specified range of PCI
> +  configuration space.
> +
> +  Writes the range of PCI configuration registers specified by StartAddress and
> +  Size from the buffer specified by Buffer. This function only allows the PCI
> +  configuration registers from a single PCI function to be written. Size is
> +  returned.
> +
> +  If any reserved bits in StartAddress are set, then ASSERT().
> +  If ((StartAddress & 0xFFF) + Size) > 0x1000, then ASSERT().
> +  If Size > 0 and Buffer is NULL, then ASSERT().
> +
> +  @param  StartAddress  The starting address that encodes the PCI Segment, Bus,
> +                        Device, Function and Register.
> +  @param  Size          The size in bytes of the transfer.
> +  @param  Buffer        The pointer to a buffer containing the data to write.
> +
> +  @return The parameter of Size.
> +
> +**/
> +UINTN
> +EFIAPI
> +PciSegmentWriteBuffer (
> +  IN UINT64                    StartAddress,
> +  IN UINTN                     Size,
> +  IN VOID                      *Buffer
> +  )
> +{
> +  UINTN                             ReturnValue;
> +
> +  ASSERT_INVALID_PCI_SEGMENT_ADDRESS (StartAddress, 0);
> +  // 0xFFF is used as limit for 4KB config space
> +  ASSERT (((StartAddress & (SIZE_4KB - 1)) + Size) <= SIZE_4KB);
> +
> +  if (Size == 0) {
> +    return Size;
> +  }
> +
> +  ASSERT (Buffer != NULL);
> +
> +  //
> +  // Save Size for return
> +  //
> +  ReturnValue = Size;
> +
> +  if ((StartAddress & BIT0) != 0) {
> +    //
> +    // Write a byte if StartAddress is byte aligned
> +    //
> +    PciSegmentWrite8 (StartAddress, *(UINT8*)Buffer);
> +    StartAddress += sizeof (UINT8);
> +    Size -= sizeof (UINT8);
> +    Buffer += sizeof (UINT8);
> +  }
> +
> +  if (Size >= sizeof (UINT16) && (StartAddress & BIT1) != 0) {
> +    //
> +    // Write a word if StartAddress is word aligned
> +    //
> +    PciSegmentWrite16 (StartAddress, ReadUnaligned16 (Buffer));
> +    StartAddress += sizeof (UINT16);
> +    Size -= sizeof (UINT16);
> +    Buffer += sizeof (UINT16);
> +  }
> +
> +  while (Size >= sizeof (UINT32)) {
> +    //
> +    // Write as many double words as possible
> +    //
> +    PciSegmentWrite32 (StartAddress, ReadUnaligned32 (Buffer));
> +    StartAddress += sizeof (UINT32);
> +    Size -= sizeof (UINT32);
> +    Buffer += sizeof (UINT32);
> +  }
> +
> +  if (Size >= sizeof (UINT16)) {
> +    //
> +    // Write the last remaining word if exist
> +    //
> +    PciSegmentWrite16 (StartAddress, ReadUnaligned16 (Buffer));
> +    StartAddress += sizeof (UINT16);
> +    Size -= sizeof (UINT16);
> +    Buffer += sizeof (UINT16);
> +  }
> +
> +  if (Size >= sizeof (UINT8)) {
> +    //
> +    // Write the last remaining byte if exist
> +    //
> +    PciSegmentWrite8 (StartAddress, *(UINT8*)Buffer);
> +  }
> +
> +  return ReturnValue;
> +}
> diff --git a/Silicon/NXP/Library/PciSegmentLib/PciSegmentLib.inf b/Silicon/NXP/Library/PciSegmentLib/PciSegmentLib.inf
> new file mode 100644
> index 0000000..1ac83d4
> --- /dev/null
> +++ b/Silicon/NXP/Library/PciSegmentLib/PciSegmentLib.inf
> @@ -0,0 +1,41 @@
> +## @file
> +#  PCI Segment Library for NXP SoCs with multiple RCs
> +#
> +#  Copyright 2018 NXP
> +#
> +#  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                    = 0x0001001A
> +  BASE_NAME                      = PciSegmentLib
> +  FILE_GUID                      = c9f59261-5a60-4a4c-82f6-1f520442e100
> +  MODULE_TYPE                    = BASE
> +  VERSION_STRING                 = 1.0
> +  LIBRARY_CLASS                  = PciSegmentLib
> +
> +[Sources]
> +  PciSegmentLib.c
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +  Silicon/NXP/NxpQoriqLs.dec
> +
> +[LibraryClasses]
> +  BaseLib
> +  DebugLib
> +  IoLib
> +  PcdLib
> +
> +[Pcd]
> +  gNxpQoriqLsTokenSpaceGuid.PcdPciExp1BaseAddr
> +  gNxpQoriqLsTokenSpaceGuid.PcdPciExp2BaseAddr
> +  gNxpQoriqLsTokenSpaceGuid.PcdPciExp3BaseAddr
> +  gNxpQoriqLsTokenSpaceGuid.PcdPciExp4BaseAddr
> -- 
> 1.9.1
>