Lefi: Thanks for your comments. > -----Original Message----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Leif Lindholm > Sent: Monday, July 27, 2020 10:25 PM > To: devel@edk2.groups.io; Javeed, Ashraf <ashraf.jav...@intel.com> > Cc: Kinney, Michael D <michael.d.kin...@intel.com>; Gao, Liming > <liming....@intel.com> > Subject: Re: [edk2-devel] [PATCH V4 1/2] MdePkg/Include/IndustryStandard: CXL > 1.1 Registers > > Hi Ashraf, but also Mike, Liming. > > I just saw this patch go in and have some comments. > > On Fri, Jul 24, 2020 at 23:56:12 +0530, Javeed, Ashraf wrote: > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2611 > > > > Register definitions from chapter 7 of Compute Express Link > > Specification Revision 1.1 are ported into the new Cxl11.h. > > The CXL Flex Bus registers are based on the PCIe Extended Capability > > DVSEC structure header, led to the inclusion of upgraded Pci.h. > > > > Signed-off-by: Ashraf Javeed <ashraf.jav...@intel.com> > > Cc: Michael D Kinney <michael.d.kin...@intel.com> > > Cc: Liming Gao <liming....@intel.com> > > -- > > > > V4: fix code style > > > > V3: Copyright date fix > > > > V2: Indentation and double declaration fix, copyright date update > > --- > > MdePkg/Include/IndustryStandard/Cxl11.h | 569 > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > +++++++++ > > MdePkg/Include/IndustryStandard/Pci.h | 6 ++---- > > 2 files changed, 571 insertions(+), 4 deletions(-) > > > > diff --git a/MdePkg/Include/IndustryStandard/Cxl11.h > > b/MdePkg/Include/IndustryStandard/Cxl11.h > > new file mode 100644 > > index 0000000000..933c1ab817 > > --- /dev/null > > +++ b/MdePkg/Include/IndustryStandard/Cxl11.h > > @@ -0,0 +1,569 @@ > > +/** @file > > + CXL 1.1 Register definitions > > + > > + This file contains the register definitions based on the Compute Express > > Link > > + (CXL) Specification Revision 1.1. > > + > > +Copyright (c) 2020, Intel Corporation. All rights reserved.<BR> > > +SPDX-License-Identifier: BSD-2-Clause-Patent > > + > > +**/ > > + > > +#ifndef _CXL11_H_ > > +#define _CXL11_H_ > > We should not be adding macros with a leading _ - these are intended > for toolchain use.
This style is align to other header file. This is the file header macro to make sure the header file be included more than once. > > > + > > +#include <IndustryStandard/Pci.h> > > +// > > +// DVSEC Vendor ID > > +// Compute Express Link Specification Revision: 1.1 - Chapter 7.1.1 - > > Table 58 > > +// (subject to change as per CXL assigned Vendor ID) > > +// > > +#define INTEL_CXL_DVSEC_VENDOR_ID > > 0x8086 > > This is Cxl11.h - surely this should be CXL_DVSEC_VENDOR_ID_INTEL? Ashraf: is it defined in spec? > > > + > > +// > > +// CXL Flex Bus Device default device and function number > > +// Compute Express Link Specification Revision: 1.1 - Chapter 7.1.1 > > +// > > +#define CXL_DEV_DEV 0 > > +#define CXL_DEV_FUNC 0 > > + > > +// > > +// Ensure proper structure formats > > +// > > +#pragma pack(1) > > And this pragma has no function whatsoever with regards to any of the > register definition structs below. It would be much better if the > structs requiring packing (_DEVICE, _PORT, ...) were grouped together > and only those were given this treatment. > > #pragma pack(1) is *not* a safe default. > I know pack(1) is for the compact structure layout. > > + > > +/// > > +/// The PCIe DVSEC for Flex Bus Device > > +///@{ > > +typedef union { > > + struct { > > + UINT16 CacheCapable : 1; // > > bit 0 > > + UINT16 IoCapable : 1; // > > bit 1 > > + UINT16 MemCapable : 1; // > > bit 2 > > + UINT16 MemHwInitMode : 1; // > > bit 3 > > + UINT16 HdmCount : 2; // > > bit 4..5 > > + UINT16 Reserved1 : 8; // > > bit 6..13 > > + UINT16 ViralCapable : 1; // > > bit 14 > > + UINT16 Reserved2 : 1; // > > bit 15 > > + } Bits; > > + UINT16 Uint16; > > +} CXL_DVSEC_FLEX_BUS_DEVICE_CAPABILITY; > > + > > +typedef union { > > + struct { > > + UINT16 CacheEnable : 1; // > > bit 0 > > + UINT16 IoEnable : 1; // > > bit 1 > > + UINT16 MemEnable : 1; // > > bit 2 > > + UINT16 CacheSfCoverage : 5; // > > bit 3..7 > > + UINT16 CacheSfGranularity : 3; // > > bit 8..10 > > + UINT16 CacheCleanEviction : 1; // > > bit 11 > > + UINT16 Reserved1 : 2; // > > bit 12..13 > > + UINT16 ViralEnable : 1; // > > bit 14 > > + UINT16 Reserved2 : 1; // > > bit 15 > > + } Bits; > > + UINT16 Uint16; > > +} CXL_DVSEC_FLEX_BUS_DEVICE_CONTROL; > > + > > +typedef union { > > + struct { > > + UINT16 Reserved1 : 14; // > > bit 0..13 > > + UINT16 ViralStatus : 1; // > > bit 14 > > + UINT16 Reserved2 : 1; // > > bit 15 > > + } Bits; > > + UINT16 Uint16; > > +} CXL_DVSEC_FLEX_BUS_DEVICE_STATUS; > > + > > +typedef union { > > + struct { > > + UINT16 Reserved1 : 1; // > > bit 0 > > + UINT16 Reserved2 : 1; // > > bit 1 > > + UINT16 Reserved3 : 1; // > > bit 2 > > + UINT16 Reserved4 : 13; // > > bit 3..15 > > + } Bits; > > + UINT16 Uint16; > > +} CXL_1_1_DVSEC_FLEX_BUS_DEVICE_CONTROL2; > > + > > +typedef union { > > + struct { > > + UINT16 Reserved1 : 1; // > > bit 0 > > + UINT16 Reserved2 : 1; // > > bit 1 > > + UINT16 Reserved3 : 14; // > > bit 2..15 > > + } Bits; > > + UINT16 Uint16; > > +} CXL_1_1_DVSEC_FLEX_BUS_DEVICE_STATUS2; > > + > > +typedef union { > > + struct { > > + UINT16 ConfigLock : 1; // > > bit 0 > > + UINT16 Reserved1 : 15; // > > bit 1..15 > > + } Bits; > > + UINT16 Uint16; > > +} CXL_DVSEC_FLEX_BUS_DEVICE_LOCK; > > + > > +typedef union { > > + struct { > > + UINT32 MemorySizeHigh : 32; // > > bit 0..31 > > + } Bits; > > + UINT32 Uint32; > > +} CXL_DVSEC_FLEX_BUS_DEVICE_RANGE1_SIZE_HIGH; > > + > > +typedef union { > > + struct { > > + UINT32 MemoryInfoValid : 1; // > > bit 0 > > + UINT32 MemoryActive : 1; // > > bit 1 > > + UINT32 MediaType : 3; // > > bit 2..4 > > + UINT32 MemoryClass : 3; // > > bit 5..7 > > + UINT32 DesiredInterleave : 3; // > > bit 8..10 > > + UINT32 Reserved : 17; // > > bit 11..27 > > + UINT32 MemorySizeLow : 4; // > > bit 28..31 > > + } Bits; > > + UINT32 Uint32; > > +} CXL_DVSEC_FLEX_BUS_DEVICE_RANGE1_SIZE_LOW; > > + > > +typedef union { > > + struct { > > + UINT32 MemoryBaseHigh : 32; // > > bit 0..31 > > + } Bits; > > + UINT32 Uint32; > > +} CXL_DVSEC_FLEX_BUS_DEVICE_RANGE1_BASE_HIGH; > > + > > +typedef union { > > + struct { > > + UINT32 Reserved : 28; // > > bit 0..27 > > + UINT32 MemoryBaseLow : 4; // > > bit 28..31 > > + } Bits; > > + UINT32 Uint32; > > +} CXL_DVSEC_FLEX_BUS_DEVICE_RANGE1_BASE_LOW; > > + > > + > > +typedef union { > > + struct { > > + UINT32 MemorySizeHigh : 32; // > > bit 0..31 > > + } Bits; > > + UINT32 Uint32; > > +} CXL_DVSEC_FLEX_BUS_DEVICE_RANGE2_SIZE_HIGH; > > + > > +typedef union { > > + struct { > > + UINT32 MemoryInfoValid : 1; // > > bit 0 > > + UINT32 MemoryActive : 1; // > > bit 1 > > + UINT32 MediaType : 3; // > > bit 2..4 > > + UINT32 MemoryClass : 3; // > > bit 5..7 > > + UINT32 DesiredInterleave : 3; // > > bit 8..10 > > + UINT32 Reserved : 17; // > > bit 11..27 > > + UINT32 MemorySizeLow : 4; // > > bit 28..31 > > + } Bits; > > + UINT32 Uint32; > > +} CXL_DVSEC_FLEX_BUS_DEVICE_RANGE2_SIZE_LOW; > > + > > +typedef union { > > + struct { > > + UINT32 MemoryBaseHigh : 32; // > > bit 0..31 > > + } Bits; > > + UINT32 Uint32; > > +} CXL_DVSEC_FLEX_BUS_DEVICE_RANGE2_BASE_HIGH; > > + > > +typedef union { > > + struct { > > + UINT32 Reserved : 28; // > > bit 0..27 > > + UINT32 MemoryBaseLow : 4; // > > bit 28..31 > > + } Bits; > > + UINT32 Uint32; > > +} CXL_DVSEC_FLEX_BUS_DEVICE_RANGE2_BASE_LOW; > > + > > +// > > +// Flex Bus Device DVSEC ID > > +// Compute Express Link Specification Revision: 1.1 - Chapter 7.1.1, Table > > 58 > > +// > > +#define FLEX_BUS_DEVICE_DVSEC_ID 0 > > + > > +// > > +// PCIe DVSEC for Flex Bus Device > > +// Compute Express Link Specification Revision: 1.1 - Chapter 7.1.1, > > Figure 95 > > +// > > +typedef struct { > > + PCI_EXPRESS_EXTENDED_CAPABILITIES_HEADER Header; > > // offset 0 > > + PCI_EXPRESS_DESIGNATED_VENDOR_SPECIFIC_HEADER_1 > > DesignatedVendorSpecificHeader1; // offset 4 > > + PCI_EXPRESS_DESIGNATED_VENDOR_SPECIFIC_HEADER_2 > > DesignatedVendorSpecificHeader2; // offset 8 > > + CXL_DVSEC_FLEX_BUS_DEVICE_CAPABILITY > > DeviceCapability; // offset 10 > > + CXL_DVSEC_FLEX_BUS_DEVICE_CONTROL > > DeviceControl; // offset 12 > > + CXL_DVSEC_FLEX_BUS_DEVICE_STATUS > > DeviceStatus; // offset 14 > > + CXL_1_1_DVSEC_FLEX_BUS_DEVICE_CONTROL2 > > DeviceControl2; // offset 16 > > + CXL_1_1_DVSEC_FLEX_BUS_DEVICE_STATUS2 > > DeviceStatus2; // offset 18 > > + CXL_DVSEC_FLEX_BUS_DEVICE_LOCK > > DeviceLock; // offset 20 > > + UINT16 Reserved; > > // offset 22 > > + CXL_DVSEC_FLEX_BUS_DEVICE_RANGE1_SIZE_HIGH > > DeviceRange1SizeHigh; // offset 24 > > + CXL_DVSEC_FLEX_BUS_DEVICE_RANGE1_SIZE_LOW > > DeviceRange1SizeLow; // offset 28 > > + CXL_DVSEC_FLEX_BUS_DEVICE_RANGE1_BASE_HIGH > > DeviceRange1BaseHigh; // offset 32 > > + CXL_DVSEC_FLEX_BUS_DEVICE_RANGE1_BASE_LOW > > DeviceRange1BaseLow; // offset 36 > > + CXL_DVSEC_FLEX_BUS_DEVICE_RANGE2_SIZE_HIGH > > DeviceRange2SizeHigh; // offset 40 > > + CXL_DVSEC_FLEX_BUS_DEVICE_RANGE2_SIZE_LOW > > DeviceRange2SizeLow; // offset 44 > > + CXL_DVSEC_FLEX_BUS_DEVICE_RANGE2_BASE_HIGH > > DeviceRange2BaseHigh; // offset 48 > > + CXL_DVSEC_FLEX_BUS_DEVICE_RANGE2_BASE_LOW > > DeviceRange2BaseLow; // offset 52 > > +} CXL_1_1_DVSEC_FLEX_BUS_DEVICE; > > +///@} > > + > > +/// > > +/// PCIe DVSEC for FLex Bus Port > > +///@{ > > +typedef union { > > + struct { > > + UINT16 CacheCapable : 1; // > > bit 0 > > + UINT16 IoCapable : 1; // > > bit 1 > > + UINT16 MemCapable : 1; // > > bit 2 > > + UINT16 Reserved : 13; // > > bit 3..15 > > + } Bits; > > + UINT16 Uint16; > > +} CXL_1_1_DVSEC_FLEX_BUS_PORT_CAPABILITY; > > + > > +typedef union { > > + struct { > > + UINT16 CacheEnable : 1; // > > bit 0 > > + UINT16 IoEnable : 1; // > > bit 1 > > + UINT16 MemEnable : 1; // > > bit 2 > > + UINT16 CxlSyncBypassEnable : 1; // > > bit 3 > > + UINT16 DriftBufferEnable : 1; // > > bit 4 > > + UINT16 Reserved : 3; // > > bit 5..7 > > + UINT16 Retimer1Present : 1; // > > bit 8 > > + UINT16 Retimer2Present : 1; // > > bit 9 > > + UINT16 Reserved2 : 6; // > > bit 10..15 > > + } Bits; > > + UINT16 Uint16; > > +} CXL_1_1_DVSEC_FLEX_BUS_PORT_CONTROL; > > + > > +typedef union { > > + struct { > > + UINT16 CacheEnable : 1; // > > bit 0 > > + UINT16 IoEnable : 1; // > > bit 1 > > + UINT16 MemEnable : 1; // > > bit 2 > > + UINT16 CxlSyncBypassEnable : 1; // > > bit 3 > > + UINT16 DriftBufferEnable : 1; // > > bit 4 > > + UINT16 Reserved : 3; // > > bit 5..7 > > + UINT16 CxlCorrectableProtocolIdFramingError : 1; // > > bit 8 > > + UINT16 CxlUncorrectableProtocolIdFramingError : 1; // > > bit 9 > > + UINT16 CxlUnexpectedProtocolIdDropped : 1; // > > bit 10 > > + UINT16 Reserved2 : 5; // > > bit 11..15 > > + } Bits; > > + UINT16 Uint16; > > +} CXL_1_1_DVSEC_FLEX_BUS_PORT_STATUS; > > + > > +// > > +// Flex Bus Port DVSEC ID > > +// Compute Express Link Specification Revision: 1.1 - Chapter 7.2.1.3, > > Table 62 > > +// > > +#define FLEX_BUS_PORT_DVSEC_ID 7 > > + > > +// > > +// PCIe DVSEC for Flex Bus Port > > +// Compute Express Link Specification Revision: 1.1 - Chapter 7.2.1.3, > > Figure 99 > > +// > > +typedef struct { > > + PCI_EXPRESS_EXTENDED_CAPABILITIES_HEADER Header; > > // offset 0 > > + PCI_EXPRESS_DESIGNATED_VENDOR_SPECIFIC_HEADER_1 > > DesignatedVendorSpecificHeader1; // offset 4 > > + PCI_EXPRESS_DESIGNATED_VENDOR_SPECIFIC_HEADER_2 > > DesignatedVendorSpecificHeader2; // offset 8 > > + CXL_1_1_DVSEC_FLEX_BUS_PORT_CAPABILITY > > PortCapability; // offset 10 > > + CXL_1_1_DVSEC_FLEX_BUS_PORT_CONTROL > > PortControl; // offset 12 > > + CXL_1_1_DVSEC_FLEX_BUS_PORT_STATUS > > PortStatus; // offset 14 > > +} CXL_1_1_DVSEC_FLEX_BUS_PORT; > > +///@} > > + > > +/// > > +/// CXL 1.1 Upstream and Downstream Port Subsystem Component registers > > +/// > > + > > +/// The CXL.Cache and CXL.Memory Architectural register definitions > > +/// Based on chapter 7.2.2 of Compute Express Link Specification Revision: > > 1.1 > > +///@{ > > + > > +#define CXL_CAPABILITY_HEADER_OFFSET 0 > > +typedef union { > > + struct { > > + UINT32 CxlCapabilityId : 16; // > > bit 0..15 > > + UINT32 CxlCapabilityVersion : 4; // > > bit 16..19 > > + UINT32 CxlCacheMemVersion : 4; // > > bit 20..23 > > + UINT32 ArraySize : 8; // > > bit 24..31 > > + } Bits; > > + UINT32 Uint32; > > +} CXL_CAPABILITY_HEADER; > > + > > +#define CXL_RAS_CAPABILITY_HEADER_OFFSET 4 > > +typedef union { > > + struct { > > + UINT32 CxlCapabilityId : 16; // > > bit 0..15 > > + UINT32 CxlCapabilityVersion : 4; // > > bit 16..19 > > + UINT32 CxlRasCapabilityPointer : 12; // > > bit 20..31 > > + } Bits; > > + UINT32 Uint32; > > +} CXL_RAS_CAPABILITY_HEADER; > > + > > +#define CXL_SECURITY_CAPABILITY_HEADER_OFFSET 8 > > +typedef union { > > + struct { > > + UINT32 CxlCapabilityId : 16; // > > bit 0..15 > > + UINT32 CxlCapabilityVersion : 4; // > > bit 16..19 > > + UINT32 CxlSecurityCapabilityPointer : 12; // > > bit 20..31 > > + } Bits; > > + UINT32 Uint32; > > +} CXL_SECURITY_CAPABILITY_HEADER; > > + > > +#define CXL_LINK_CAPABILITY_HEADER_OFFSET 0xC > > +typedef union { > > + struct { > > + UINT32 CxlCapabilityId : 16; // > > bit 0..15 > > + UINT32 CxlCapabilityVersion : 4; // > > bit 16..19 > > + UINT32 CxlLinkCapabilityPointer : 12; // > > bit 20..31 > > + } Bits; > > + UINT32 Uint32; > > +} CXL_LINK_CAPABILITY_HEADER; > > + > > +typedef union { > > + struct { > > + UINT32 CacheDataParity : 1; // > > bit 0..0 > > + UINT32 CacheAddressParity : 1; // > > bit 1..1 > > + UINT32 CacheByteEnableParity : 1; // > > bit 2..2 > > + UINT32 CacheDataEcc : 1; // > > bit 3..3 > > + UINT32 MemDataParity : 1; // > > bit 4..4 > > + UINT32 MemAddressParity : 1; // > > bit 5..5 > > + UINT32 MemByteEnableParity : 1; // > > bit 6..6 > > + UINT32 MemDataEcc : 1; // > > bit 7..7 > > + UINT32 ReInitThreshold : 1; // > > bit 8..8 > > + UINT32 RsvdEncodingViolation : 1; // > > bit 9..9 > > + UINT32 PoisonReceived : 1; // > > bit 10..10 > > + UINT32 ReceiverOverflow : 1; // > > bit 11..11 > > + UINT32 Reserved : 20; // > > bit 12..31 > > + } Bits; > > + UINT32 Uint32; > > +} CXL_1_1_UNCORRECTABLE_ERROR_STATUS; > > + > > +typedef union { > > + struct { > > + UINT32 CacheDataParityMask : 1; // > > bit 0..0 > > + UINT32 CacheAddressParityMask : 1; // > > bit 1..1 > > + UINT32 CacheByteEnableParityMask : 1; // > > bit 2..2 > > + UINT32 CacheDataEccMask : 1; // > > bit 3..3 > > + UINT32 MemDataParityMask : 1; // > > bit 4..4 > > + UINT32 MemAddressParityMask : 1; // > > bit 5..5 > > + UINT32 MemByteEnableParityMask : 1; // > > bit 6..6 > > + UINT32 MemDataEccMask : 1; // > > bit 7..7 > > + UINT32 ReInitThresholdMask : 1; // > > bit 8..8 > > + UINT32 RsvdEncodingViolationMask : 1; // > > bit 9..9 > > + UINT32 PoisonReceivedMask : 1; // > > bit 10..10 > > + UINT32 ReceiverOverflowMask : 1; // > > bit 11..11 > > + UINT32 Reserved : 20; // > > bit 12..31 > > + } Bits; > > + UINT32 Uint32; > > +} CXL_1_1_UNCORRECTABLE_ERROR_MASK; > > + > > +typedef union { > > + struct { > > + UINT32 CacheDataParitySeverity : 1; // > > bit 0..0 > > + UINT32 CacheAddressParitySeverity : 1; // > > bit 1..1 > > + UINT32 CacheByteEnableParitySeverity : 1; // > > bit 2..2 > > + UINT32 CacheDataEccSeverity : 1; // > > bit 3..3 > > + UINT32 MemDataParitySeverity : 1; // > > bit 4..4 > > + UINT32 MemAddressParitySeverity : 1; // > > bit 5..5 > > + UINT32 MemByteEnableParitySeverity : 1; // > > bit 6..6 > > + UINT32 MemDataEccSeverity : 1; // > > bit 7..7 > > + UINT32 ReInitThresholdSeverity : 1; // > > bit 8..8 > > + UINT32 RsvdEncodingViolationSeverity : 1; // > > bit 9..9 > > + UINT32 PoisonReceivedSeverity : 1; // > > bit 10..10 > > + UINT32 ReceiverOverflowSeverity : 1; // > > bit 11..11 > > + UINT32 Reserved : 20; // > > bit 12..31 > > + } Bits; > > + UINT32 Uint32; > > +} CXL_1_1_UNCORRECTABLE_ERROR_SEVERITY; > > + > > +typedef union { > > + struct { > > + UINT32 CacheDataEcc : 1; // > > bit 0..0 > > + UINT32 MemoryDataEcc : 1; // > > bit 1..1 > > + UINT32 CrcThreshold : 1; // > > bit 2..2 > > + UINT32 RetryThreshold : 1; // > > bit 3..3 > > + UINT32 CachePoisonReceived : 1; // > > bit 4..4 > > + UINT32 MemoryPoisonReceived : 1; // > > bit 5..5 > > + UINT32 PhysicalLayerError : 1; // > > bit 6..6 > > + UINT32 Reserved : 25; // > > bit 7..31 > > + } Bits; > > + UINT32 Uint32; > > +} CXL_CORRECTABLE_ERROR_STATUS; > > + > > +typedef union { > > + struct { > > + UINT32 CacheDataEccMask : 1; // > > bit 0..0 > > + UINT32 MemoryDataEccMask : 1; // > > bit 1..1 > > + UINT32 CrcThresholdMask : 1; // > > bit 2..2 > > + UINT32 RetryThresholdMask : 1; // > > bit 3..3 > > + UINT32 CachePoisonReceivedMask : 1; // > > bit 4..4 > > + UINT32 MemoryPoisonReceivedMask : 1; // > > bit 5..5 > > + UINT32 PhysicalLayerErrorMask : 1; // > > bit 6..6 > > + UINT32 Reserved : 25; // > > bit 7..31 > > + } Bits; > > + UINT32 Uint32; > > +} CXL_CORRECTABLE_ERROR_MASK; > > + > > +typedef union { > > + struct { > > + UINT32 FirstErrorPointer : 4; // > > bit 0..3 > > + UINT32 Reserved1 : 5; // > > bit 4..8 > > + UINT32 MultipleHeaderRecordingCapability : 1; // > > bit 9..9 > > + UINT32 Reserved2 : 3; // > > bit 10..12 > > + UINT32 PoisonEnabled : 1; // > > bit 13..13 > > + UINT32 Reserved3 : 18; // > > bit 14..31 > > + } Bits; > > + UINT32 Uint32; > > +} CXL_ERROR_CAPABILITIES_AND_CONTROL; > > + > > +typedef struct { > > + CXL_1_1_UNCORRECTABLE_ERROR_STATUS > > UncorrectableErrorStatus; > > + CXL_1_1_UNCORRECTABLE_ERROR_MASK > > UncorrectableErrorMask; > > + CXL_1_1_UNCORRECTABLE_ERROR_SEVERITY > > UncorrectableErrorSeverity; > > + CXL_CORRECTABLE_ERROR_STATUS > > CorrectableErrorStatus; > > + CXL_CORRECTABLE_ERROR_MASK > > CorrectableErrorMask; > > + CXL_ERROR_CAPABILITIES_AND_CONTROL > > ErrorCapabilitiesAndControl; > > + UINT32 > > HeaderLog[16]; > > +} CXL_1_1_RAS_CAPABILITY_STRUCTURE; > > + > > +typedef union { > > + struct { > > + UINT32 DeviceTrustLevel : 2; // > > bit 0..1 > > + UINT32 Reserved : 30; // > > bit 2..31 > > + } Bits; > > + UINT32 Uint32; > > +} CXL_1_1_SECURITY_POLICY; > > + > > +typedef struct { > > + CXL_1_1_SECURITY_POLICY > > SecurityPolicy; > > +} CXL_1_1_SECURITY_CAPABILITY_STRUCTURE; > > + > > +typedef union { > > + struct { > > + UINT64 CxlLinkVersionSupported : 4; // > > bit 0..3 > > + UINT64 CxlLinkVersionReceived : 4; // > > bit 4..7 > > + UINT64 LlrWrapValueSupported : 8; // > > bit 8..15 > > + UINT64 LlrWrapValueReceived : 8; // > > bit 16..23 > > + UINT64 NumRetryReceived : 5; // > > bit 24..28 > > + UINT64 NumPhyReinitReceived : 5; // > > bit 29..33 > > + UINT64 WrPtrReceived : 8; // > > bit 34..41 > > + UINT64 EchoEseqReceived : 8; // > > bit 42..49 > > + UINT64 NumFreeBufReceived : 8; // > > bit 50..57 > > + UINT64 Reserved : 6; // > > bit 58..63 > > + } Bits; > > + UINT64 Uint64; > > +} CXL_LINK_LAYER_CAPABILITY; > > + > > +typedef union { > > + struct { > > + UINT16 LlReset : 1; // > > bit 0..0 > > + UINT16 LlInitStall : 1; // > > bit 1..1 > > + UINT16 LlCrdStall : 1; // > > bit 2..2 > > + UINT16 InitState : 2; // > > bit 3..4 > > + UINT16 LlRetryBufferConsumed : 8; // > > bit 5..12 > > + UINT16 Reserved : 3; // > > bit 13..15 > > + } Bits; > > + UINT16 Uint16; > > +} CXL_LINK_LAYER_CONTROL_AND_STATUS; > > + > > +typedef union { > > + struct { > > + UINT64 CacheReqCredits : 10; // > > bit 0..9 > > + UINT64 CacheRspCredits : 10; // > > bit 10..19 > > + UINT64 CacheDataCredits : 10; // > > bit 20..29 > > + UINT64 MemReqRspCredits : 10; // > > bit 30..39 > > + UINT64 MemDataCredits : 10; // > > bit 40..49 > > + } Bits; > > + UINT64 Uint64; > > +} CXL_LINK_LAYER_RX_CREDIT_CONTROL; > > + > > +typedef union { > > + struct { > > + UINT64 CacheReqCredits : 10; // > > bit 0..9 > > + UINT64 CacheRspCredits : 10; // > > bit 10..19 > > + UINT64 CacheDataCredits : 10; // > > bit 20..29 > > + UINT64 MemReqRspCredits : 10; // > > bit 30..39 > > + UINT64 MemDataCredits : 10; // > > bit 40..49 > > + } Bits; > > + UINT64 Uint64; > > +} CXL_LINK_LAYER_RX_CREDIT_RETURN_STATUS; > > + > > +typedef union { > > + struct { > > + UINT64 CacheReqCredits : 10; // > > bit 0..9 > > + UINT64 CacheRspCredits : 10; // > > bit 10..19 > > + UINT64 CacheDataCredits : 10; // > > bit 20..29 > > + UINT64 MemReqRspCredits : 10; // > > bit 30..39 > > + UINT64 MemDataCredits : 10; // > > bit 40..49 > > + } Bits; > > + UINT64 Uint64; > > +} CXL_LINK_LAYER_TX_CREDIT_STATUS; > > + > > +typedef union { > > + struct { > > + UINT32 AckForceThreshold : 8; // > > bit 0..7 > > + UINT32 AckFLushRetimer : 10; // > > bit 8..17 > > + } Bits; > > + UINT32 Uint32; > > +} CXL_LINK_LAYER_ACK_TIMER_CONTROL; > > + > > +typedef union { > > + struct { > > + UINT32 MdhDisable : 1; // > > bit 0..0 > > + UINT32 Reserved : 31; // > > bit 1..31 > > + } Bits; > > + UINT32 Uint32; > > +} CXL_LINK_LAYER_DEFEATURE; > > + > > +typedef struct { > > + CXL_LINK_LAYER_CAPABILITY > > LinkLayerCapability; > > + CXL_LINK_LAYER_CONTROL_AND_STATUS > > LinkLayerControlStatus; > > + CXL_LINK_LAYER_RX_CREDIT_CONTROL > > LinkLayerRxCreditControl; > > + CXL_LINK_LAYER_RX_CREDIT_RETURN_STATUS > > LinkLayerRxCreditReturnStatus; > > + CXL_LINK_LAYER_TX_CREDIT_STATUS > > LinkLayerTxCreditStatus; > > + CXL_LINK_LAYER_ACK_TIMER_CONTROL > > LinkLayerAckTimerControl; > > + CXL_LINK_LAYER_DEFEATURE > > LinkLayerDefeature; > > +} CXL_1_1_LINK_CAPABILITY_STRUCTURE; > > + > > +#define CXL_IO_ARBITRATION_CONTROL_OFFSET 0x180 > > +typedef union { > > + struct { > > + UINT32 Reserved1 : 4; // > > bit 0..3 > > + UINT32 WeightedRoundRobinArbitrationWeight : 4; // > > bit 4..7 > > + UINT32 Reserved2 : 24; // > > bit 8..31 > > + } Bits; > > + UINT32 Uint32; > > +} CXL_IO_ARBITRATION_CONTROL; > > + > > +#define CXL_CACHE_MEMORY_ARBITRATION_CONTROL_OFFSET 0x1C0 > > +typedef union { > > + struct { > > + UINT32 Reserved1 : 4; // > > bit 0..3 > > + UINT32 WeightedRoundRobinArbitrationWeight : 4; // > > bit 4..7 > > + UINT32 Reserved2 : 24; // > > bit 8..31 > > + } Bits; > > + UINT32 Uint32; > > +} CXL_CACHE_MEMORY_ARBITRATION_CONTROL; > > +///@} > > + > > +/// The CXL.RCRB base register definition > > +/// Based on chapter 7.3 of Compute Express Link Specification Revision: > > 1.1 > > +///@{ > > +typedef union { > > + struct { > > + UINT64 RcrbEnable : 1; // > > bit 0..0 > > + UINT64 Reserved : 12; // > > bit 1..12 > > + UINT64 RcrbBaseAddress : 51; // > > bit 13..63 > > + } Bits; > > + UINT64 Uint64; > > +} CXL_RCRB_BASE; > > +///@} > > + > > +#pragma pack() > > + > > +// > > +// CXL Downstream / Upstream Port RCRB space register offsets > > +// Compute Express Link Specification Revision: 1.1 - Chapter 7.2.1.1 - > > Figure 97 > > +// > > +#define CXL_PORT_RCRB_MEMBAR0_LOW_OFFSET > > 0x010 > > +#define CXL_PORT_RCRB_MEMBAR0_HIGH_OFFSET > > 0x014 > > +#define CXL_PORT_RCRB_EXTENDED_CAPABILITY_BASE_OFFSET > > 0x100 > > + > > +#endif > > diff --git a/MdePkg/Include/IndustryStandard/Pci.h > > b/MdePkg/Include/IndustryStandard/Pci.h > > index 8ed96b992a..42c00ac762 100644 > > --- a/MdePkg/Include/IndustryStandard/Pci.h > > +++ b/MdePkg/Include/IndustryStandard/Pci.h > > But here is the thing that really disappoints me getting through > review - this change is completely, fundamentally unrelated to the > rest of this patch, and not even mentioned in the commit message. This > should have been a separate patch preceding this one. > > > @@ -1,7 +1,7 @@ > > /** @file > > Support for the latest PCI standard. > > > > -Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR> > > +Copyright (c) 2006 - 2020, Intel Corporation. All rights reserved.<BR> > > SPDX-License-Identifier: BSD-2-Clause-Patent > > > > **/ > > @@ -9,9 +9,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > > #ifndef _PCI_H_ > > #define _PCI_H_ > > > > -#include <IndustryStandard/Pci30.h> > > -#include <IndustryStandard/PciExpress21.h> > > -#include <IndustryStandard/PciExpress30.h> > > +#include <IndustryStandard/PciExpress50.h> > > #include <IndustryStandard/PciCodeId.h> > > > > #endif > > Now, one final comment - and this is more of a project feature > suggestion: > Industry standard headers is something fairly special, even in > comparison with the rest of MdePkg. *I* would certainly like to ensure > I don't miss changes or additions to them. > Could we set up a dedicated group of reviewers for this folder only? > This need not affect the actual maintainership of MdePkg, just ensure > more eyeballs (or screen readers, braille terminals, ...) hit updates > here? > > i.e. something like the below to Maintainers.txt: > > F: MdePkg/Include/IndustryStandard/ > R: Leif ... > R: ... > R: ... > This is a good suggestion. IndustryStandard needs more feedback. Can you send the patch to update Maintainers.txt? Thanks Liming > / > Leif > > > -- > > 2.21.0.windows.1 > > > > > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#63303): https://edk2.groups.io/g/devel/message/63303 Mute This Topic: https://groups.io/mt/75823529/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-