On Fri, Apr 20, 2018 at 06:40:27AM +0000, Vabhav Sharma wrote:
>
>
> >-----Original Message-----
> >From: Leif Lindholm [mailto:[email protected]]
> >Sent: Friday, April 20, 2018 12:57 AM
> >To: Meenakshi Aggarwal <[email protected]>
> >Cc: [email protected]; [email protected]; Udit Kumar
> ><[email protected]>; Varun Sethi <[email protected]>; Vabhav Sharma
> ><[email protected]>
> >Subject: Re: [PATCH edk2-platforms 32/39] Silicon/NXP: Implement
> >PciSegmentLib to support multiple RCs
> >
> >On Fri, Feb 16, 2018 at 02:20:28PM +0530, Meenakshi wrote:
> >> From: Vabhav <[email protected]>
> >>
> >> 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 <[email protected]>
> >> Signed-off-by: Meenakshi Aggarwal <[email protected]>
> >> ---
> >> Silicon/NXP/Include/Pcie.h | 143 +++++
> >> Silicon/NXP/Library/PciSegmentLib/PciSegmentLib.c | 604
> >+++++++++++++++++++++
> >> .../NXP/Library/PciSegmentLib/PciSegmentLib.inf | 41 ++
> >> 3 files changed, 788 insertions(+)
> >> create mode 100644 Silicon/NXP/Include/Pcie.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/Pcie.h b/Silicon/NXP/Include/Pcie.h
> >> new file mode 100644 index 0000000..a7e6f9b
> >> --- /dev/null
> >> +++ b/Silicon/NXP/Include/Pcie.h
> >> @@ -0,0 +1,143 @@
> >> +/** @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
> >https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fopensou
> >rce.org%2Flicenses%2Fbsd-
> >license.php&data=02%7C01%7Cvabhav.sharma%40nxp.com%7C081a2ebc43af4a
> >daaf8e08d5a62b9921%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6
> >36597628604269440&sdata=kt661HfkUD2E3sYswy0I4vVFTV3%2Fq%2FiM%2BiW
> >egCISP%2BU%3D&reserved=0.
> >> +
> >> + 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_H__
> >> +#define __PCI_H__
> >
> >I'm not super happy about reusing such a generic name for the include guard
> >- or
> >really even the filename. (MdePkg/Include/Pci.h has
> >_PCI_H_.)
> >
> >Could you rename this header NxpPcie.h and change the include guard to
> >_NXP_PCIE_H_?
> I see, Sure.
> >
> >> +
> >> +// Segment 0
> >> +#define PCI_SEG0_NUM 0
> >> +
> >> +#define PCI_SEG0_BUSNUM_MIN 0x0
> >> +#define PCI_SEG0_BUSNUM_MAX 0xff
> >> +
> >> +#define PCI_SEG0_PORTIO_MIN 0x0
> >> +#define PCI_SEG0_PORTIO_MAX 0xffff
> >> +
> >> +#define PCI_SEG0_MMIO32_MIN 0x40000000
> >> +#define PCI_SEG0_MMIO32_MAX 0x4fffffff
> >> +#define PCI_SEG0_MMIO64_MIN PCI_SEG0_MMIO_MEMBASE +
> >SEG_MEM_SIZE
> >> +#define PCI_SEG0_MMIO64_MAX PCI_SEG0_MMIO_MEMBASE +
> >SEG_MEM_LIMIT
> >> +#define PCI_SEG0_MMIO_MEMBASE FixedPcdGet64 (PcdPciExp1BaseAddr)
> >> +
> >> +#define PCI_SEG0_DBI_BASE 0x03400000
> >> +
> >> +// Segment 1
> >> +#define PCI_SEG1_NUM 1
> >> +
> >> +#define PCI_SEG1_BUSNUM_MIN 0x0
> >> +#define PCI_SEG1_BUSNUM_MAX 0xff
> >> +
> >> +#define PCI_SEG1_PORTIO_MIN 0x10000
> >> +#define PCI_SEG1_PORTIO_MAX 0x1ffff
> >> +
> >> +#define PCI_SEG1_MMIO32_MIN 0x50000000
> >> +#define PCI_SEG1_MMIO32_MAX 0x5fffffff
> >> +#define PCI_SEG1_MMIO64_MIN PCI_SEG1_MMIO_MEMBASE +
> >SEG_MEM_SIZE
> >> +#define PCI_SEG1_MMIO64_MAX PCI_SEG1_MMIO_MEMBASE +
> >SEG_MEM_LIMIT
> >> +#define PCI_SEG1_MMIO_MEMBASE FixedPcdGet64 (PcdPciExp2BaseAddr)
> >> +
> >> +#define PCI_SEG1_DBI_BASE 0x03500000
> >> +
> >> +// Segment 2
> >> +#define PCI_SEG2_NUM 2
> >> +
> >> +#define PCI_SEG2_BUSNUM_MIN 0x0
> >> +#define PCI_SEG2_BUSNUM_MAX 0xff
> >> +
> >> +#define PCI_SEG2_PORTIO_MIN 0x20000
> >> +#define PCI_SEG2_PORTIO_MAX 0x2ffff
> >> +
> >> +#define PCI_SEG2_MMIO32_MIN 0x60000000
> >> +#define PCI_SEG2_MMIO32_MAX 0x6fffffff
> >> +#define PCI_SEG2_MMIO64_MIN PCI_SEG2_MMIO_MEMBASE +
> >SEG_MEM_SIZE
> >> +#define PCI_SEG2_MMIO64_MAX PCI_SEG2_MMIO_MEMBASE +
> >SEG_MEM_LIMIT
> >> +#define PCI_SEG2_MMIO_MEMBASE FixedPcdGet64 (PcdPciExp3BaseAddr)
> >> +
> >> +#define PCI_SEG2_DBI_BASE 0x03600000
> >> +
> >> +// Segment 3
> >> +#define PCI_SEG3_NUM 3
> >> +
> >> +#define PCI_SEG3_BUSNUM_MIN 0x0
> >> +#define PCI_SEG3_BUSNUM_MAX 0xff
> >> +
> >> +#define PCI_SEG3_PORTIO_MIN 0x30000
> >> +#define PCI_SEG3_PORTIO_MAX 0x3ffff
> >> +
> >> +#define PCI_SEG3_MMIO32_MIN 0x70000000
> >> +#define PCI_SEG3_MMIO32_MAX 0x7fffffff
> >> +#define PCI_SEG3_MMIO64_MIN PCI_SEG3_MMIO_MEMBASE +
> >SEG_MEM_SIZE
> >> +#define PCI_SEG3_MMIO64_MAX PCI_SEG3_MMIO_MEMBASE +
> >SEG_MEM_LIMIT
> >> +#define PCI_SEG3_MMIO_MEMBASE FixedPcdGet64 (PcdPciExp4BaseAddr)
> >> +
> >> +#define PCI_SEG3_DBI_BASE 0x03700000
> >> +
> >> +// Segment configuration
> >> +#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_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
> >> +#define PCI_SEG0_PHY_IO_BASE PCI_SEG0_MMIO_MEMBASE +
> >SEG_IO_SIZE
> >> +
> >> +// 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
> >> +
> >> +// 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..acb614d
> >> --- /dev/null
> >> +++ b/Silicon/NXP/Library/PciSegmentLib/PciSegmentLib.c
> >> @@ -0,0 +1,604 @@
> >> +/** @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
> >> +
> >https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fopensou
> >rce.org%2Flicenses%2Fbsd-
> >license.php&data=02%7C01%7Cvabhav.sharma%40nxp.com%7C081a2ebc43af4a
> >daaf8e08d5a62b9921%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6
> >36597628604269440&sdata=kt661HfkUD2E3sYswy0I4vVFTV3%2Fq%2FiM%2BiW
> >egCISP%2BU%3D&reserved=0.
> >> +
> >> + 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 <Pcie.h>
> >> +
> >> +typedef enum {
> >> + PciCfgWidthUint8 = 0,
> >> + PciCfgWidthUint16,
> >> + PciCfgWidthUint32,
> >> + PciCfgWidthMax
> >> +} PCI_CFG_WIDTH;
> >> +
> >> +/**
> >> + 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
> >> +
> >> + @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)
> >> +
> >> +/**
> >> + Function to return PCIe Physical Address(PCIe view) or Controller
> >> + Address(CPU view) for different RCs
> >> +
> >> + @param Address Address passed from bus layer.
> >> + @param Segment Segment number for Root Complex.
> >> +
> >> + @return Return PCIe CPU or Controller address.
> >> +
> >> +**/
> >> +STATIC
> >> +UINT64
> >> +PciSegmentLibGetConfigBase (
> >> + IN UINT64 Address,
> >> + IN UINT16 Segment
> >> + )
> >> +{
> >> +
> >> + switch (Segment) {
> >> + // Root Complex 1
> >> + case PCI_SEG0_NUM:
> >> + // Reading bus number(bits 20-27)
> >> + if ((Address >> 20) & 1) {
> >> + 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) {
> >> + 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) {
> >> + 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) {
> >> + return PCI_SEG3_MMIO_MEMBASE;
> >> + } else {
> >> + // On Bus 0 RCs are connected
> >> + return PCI_SEG3_DBI_BASE;
> >> + }
> >> + default:
> >> + return 0;
> >> + }
> >> +
> >> +}
> >> +
> >> +/**
> >> + 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;
> >> + UINT16 Segment;
> >> +
> >> + //
> >> + // Reading Segment number(47-32) bits in Address // Segment =
> >> + (Address >> 32); // // Reading Function(12-0) bits in Address //
> >> + Offset = (Address & 0xfff );
> >> +
> >> + Base = PciSegmentLibGetConfigBase (Address, Segment);
> >> +
> >> + //
> >> + // ignore devices > 0 on bus 0
> >> + //
> >> + if ((Address & 0xff00000) == 0 && (Address & 0xf8000) != 0) {
> >> + return MAX_UINT32;
> >> + }
> >> +
> >> + //
> >> + // ignore device > 0 on bus 1
> >> + //
> >> + if ((Address & 0xfe00000) == 0 && (Address & 0xf8000) != 0) {
> >> + return MAX_UINT32;
> >> + }
> >> +
> >> + switch (Width) {
> >> + case PciCfgWidthUint8:
> >> + return MmioRead8 (Base + (UINT8)Offset); case PciCfgWidthUint16:
Sorry - I missed thie before but spotted it while scrolling through
the reply: This cast of the offset is not only unnecessary, it is a
bug - offset is masked with 0xfff above, meaning it carries 12 bits of
value. Casting it to UINT8 discards the top 4.
(And because an explicit cast amounts to the same as telling the
compiler "I know better than you", this does not even result in a
warning.)
Anyway, please drop this and the subsequent two casts.
> >> + return MmioRead16 (Base + (UINT16)Offset); case
> >> + PciCfgWidthUint32:
> >> + return MmioRead32 (Base + (UINT32)Offset);
> >> + default:
> >> + ASSERT (FALSE);
> >> + }
> >> +
> >> + return CHAR_NULL;
> >> +}
> >> +
> >> +/**
> >> + 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;
> >> + UINT16 Segment;
> >> +
> >> + //
> >> + // Reading Segment number(47-32 bits) in Address Segment =
> >> + (Address >> 32); // // Reading Function(12-0 bits) in Address //
> >> + Offset = (Address & 0xfff );
> >
> >Spurious space after 0xfff.
> Alright, Thanks.
> Same applies to PciSegmentLibReadWorker()
> >
> >Could we have some macros and #defines instead of live-coded values in this
> >function?
>
> Sure, I assume Similar changes in PciSegmentLibReadWorker() required.
Err, yes please.
Umm, actually - the base functionality of those two functions are
identical (which is why I must have missed the ReadWorker with a
double page-down). Could that be turned into a helper function that
calculates addresses and offsets, called by readworker and writeworker
separately?
> >> +
> >> + Base = PciSegmentLibGetConfigBase (Address, Segment);
> >> +
> >> + //
> >> + // ignore devices > 0 on bus 0
> >> + //
> >> + if ((Address & 0xff00000) == 0 && (Address & 0xf8000) != 0) {
> >> + return Data;
> >> + }
> >> +
> >> + //
> >> + // ignore device > 0 on bus 1
> >> + //
> >> + if ((Address & 0xfe00000) == 0 && (Address & 0xf8000) != 0) {
> >> + return MAX_UINT32;
> >> + }
> >> +
> >> + switch (Width) {
> >> + case PciCfgWidthUint8:
> >> + MmioWrite8 (Base + (UINT8)Offset, Data);
(Same problem with the casts here.)
> >> + break;
> >> + case PciCfgWidthUint16:
> >> + MmioWrite16 (Base + (UINT16)Offset, Data);
> >> + break;
> >> + case PciCfgWidthUint32:
> >> + MmioWrite32 (Base + (UINT16)Offset, Data);
/
Leif
> >> + break;
> >> + default:
> >> + ASSERT (FALSE);
> >> + }
> >> +
> >> + 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);
> >> +}
> >> +
> >> +/**
> >> + 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); }
> >> +
> >> +/**
> >> + 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);
> >> +
> >> + return (UINT16) PciSegmentLibReadWorker (Address,
> >> +PciCfgWidthUint16); }
> >> +
> >> +/**
> >> + 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 & 0xFFF)
> >> + + 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
> >> + //
> >> + *(volatile UINT8 *)Buffer = PciSegmentRead8 (StartAddress);
> >
> >Why volatile on Buffer?
> I took reference from Socionext, Not required .
> PCIe region has device attribute.
> >
> >> + StartAddress += sizeof (UINT8);
> >> + Size -= sizeof (UINT8);
> >> + Buffer = (UINT8*)Buffer + BIT0;
> >
> >sizeof (UINT8) instead of BIT0?
> >
> >And this is a VOID *, so can just write.
> > Buffer += sizeof (UINT8);
> >
> Yes,Sure.
> >> + }
> >> +
> >> + 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 = (UINT16*)Buffer + BIT0;
> >
> >That is a very confusing use of pointer arithmetic.
> > Buffer += sizeof (UINT16);
> >
> Agree, alright.
> >> + }
> >> +
> >> + while (Size >= sizeof (UINT32)) {
> >> + //
> >> + // Read as many double words as possible
> >> + //
> >> + WriteUnaligned32 (Buffer, PciSegmentRead32 (StartAddress));
> >> + StartAddress += sizeof (UINT32);
> >> + Size -= sizeof (UINT32);
> >> + Buffer = (UINT32*)Buffer + BIT0;
> >
> > Buffer += sizeof (UINT32);
> Ok
> >
> >> + }
> >> +
> >> + if (Size >= sizeof (UINT16)) {
> >> + //
> >> + // Read the last remaining word if exist
> >> + //
> >> + WriteUnaligned16 (Buffer, PciSegmentRead16 (StartAddress));
> >> + StartAddress += sizeof (UINT16);
> >> + Size -= sizeof (UINT16);
> >> + Buffer = (UINT16*)Buffer + BIT0;
> >
> > Buffer += sizeof (UINT16);
> Ok
> >
> >> + }
> >> +
> >> + if (Size >= sizeof (UINT8)) {
> >> + //
> >> + // Read the last remaining byte if exist
> >> + //
> >> + *(volatile UINT8 *)Buffer = PciSegmentRead8 (StartAddress);
> >
> >There is that scary volatile again.
> Not required.
> >
> >> + }
> >> +
> >> + 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 & 0xFFF)
> >> + + Size) <= SIZE_4KB);
> >
> >Can you use (SIZE_4KB - 1) instead of 0xFFF?
> Yes,Sure.
> >
> >> +
> >> + 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 = (UINT8*)Buffer + BIT0;
> >
> >Same comments for Buffer pointer update as previous function, throughout.
> Ok
> >
> >/
> > Leif
> >
> >> + }
> >> +
> >> + 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 = (UINT16*)Buffer + BIT0;
> >> + }
> >> +
> >> + while (Size >= sizeof (UINT32)) {
> >> + //
> >> + // Write as many double words as possible
> >> + //
> >> + PciSegmentWrite32 (StartAddress, ReadUnaligned32 (Buffer));
> >> + StartAddress += sizeof (UINT32);
> >> + Size -= sizeof (UINT32);
> >> + Buffer = (UINT32*)Buffer + BIT0;
> >> + }
> >> +
> >> + if (Size >= sizeof (UINT16)) {
> >> + //
> >> + // Write the last remaining word if exist
> >> + //
> >> + PciSegmentWrite16 (StartAddress, ReadUnaligned16 (Buffer));
> >> + StartAddress += sizeof (UINT16);
> >> + Size -= sizeof (UINT16);
> >> + Buffer = (UINT16*)Buffer + BIT0;
> >> + }
> >> +
> >> + 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 #
> >https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fopensou
> >rce.org%2Flicenses%2Fbsd-
> >license.php&data=02%7C01%7Cvabhav.sharma%40nxp.com%7C081a2ebc43af4a
> >daaf8e08d5a62b9921%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6
> >36597628604269440&sdata=kt661HfkUD2E3sYswy0I4vVFTV3%2Fq%2FiM%2BiW
> >egCISP%2BU%3D&reserved=0.
> >> +# 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
> >>
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel
