public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH V4 0/2] CXL Specification Registers
@ 2020-07-24 18:26 Javeed, Ashraf
  2020-07-24 18:26 ` [PATCH V4 1/2] MdePkg/Include/IndustryStandard: CXL 1.1 Registers Javeed, Ashraf
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Javeed, Ashraf @ 2020-07-24 18:26 UTC (permalink / raw)
  To: devel; +Cc: Michael D Kinney, Liming Gao

These 2 patches introduces the Compute Express Link (CXL) Specificition
defined registers.
The Cxl11.h has the actual register definitions of the CXL Specification
Revision 1.1; and the Cxl.h is the main header file to include all
versions of the CXL register definitions.

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2611

Signed-off-by: Ashraf Javeed <ashraf.javeed@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
--

Ashraf Javeed (2):
  MdePkg/Include/IndustryStandard: CXL 1.1 Registers
  MdePkg/Include/IndustryStandard: Main CXL header

 MdePkg/Include/IndustryStandard/Cxl.h   |  22 ++++++++++++++++++++++
 MdePkg/Include/IndustryStandard/Cxl11.h | 569 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 MdePkg/Include/IndustryStandard/Pci.h   |   6 ++----
 3 files changed, 593 insertions(+), 4 deletions(-)
 create mode 100644 MdePkg/Include/IndustryStandard/Cxl.h
 create mode 100644 MdePkg/Include/IndustryStandard/Cxl11.h

-- 
2.21.0.windows.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH V4 1/2] MdePkg/Include/IndustryStandard: CXL 1.1 Registers
  2020-07-24 18:26 [PATCH V4 0/2] CXL Specification Registers Javeed, Ashraf
@ 2020-07-24 18:26 ` Javeed, Ashraf
  2020-07-27 14:24   ` [edk2-devel] [PATCH V4 1/2] MdePkg/Include/IndustryStandard: " Leif Lindholm
  2020-07-24 18:26 ` [PATCH V4 2/2] MdePkg/Include/IndustryStandard: Main CXL header Javeed, Ashraf
  2020-07-27  2:35 ` [PATCH V4 0/2] CXL Specification Registers Liming Gao
  2 siblings, 1 reply; 13+ messages in thread
From: Javeed, Ashraf @ 2020-07-24 18:26 UTC (permalink / raw)
  To: devel; +Cc: Michael D Kinney, Liming Gao

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.javeed@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@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_
+
+#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
+
+//
+// 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)
+
+///
+/// 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
@@ -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
-- 
2.21.0.windows.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH V4 2/2] MdePkg/Include/IndustryStandard: Main CXL header
  2020-07-24 18:26 [PATCH V4 0/2] CXL Specification Registers Javeed, Ashraf
  2020-07-24 18:26 ` [PATCH V4 1/2] MdePkg/Include/IndustryStandard: CXL 1.1 Registers Javeed, Ashraf
@ 2020-07-24 18:26 ` Javeed, Ashraf
  2020-07-27  2:35 ` [PATCH V4 0/2] CXL Specification Registers Liming Gao
  2 siblings, 0 replies; 13+ messages in thread
From: Javeed, Ashraf @ 2020-07-24 18:26 UTC (permalink / raw)
  To: devel; +Cc: Michael D Kinney, Liming Gao

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2611

Introducing the Cxl.h as the main header file to support all versions
of Compute Express Link Specification register definitions.

Signed-off-by: Ashraf Javeed <ashraf.javeed@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
--

V4: fix code style

V3: Copyright date fix

V2: Indentation and double declaration fix, copyright date update
---
 MdePkg/Include/IndustryStandard/Cxl.h | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/MdePkg/Include/IndustryStandard/Cxl.h b/MdePkg/Include/IndustryStandard/Cxl.h
new file mode 100644
index 0000000000..632aa146d0
--- /dev/null
+++ b/MdePkg/Include/IndustryStandard/Cxl.h
@@ -0,0 +1,22 @@
+/** @file
+  Support for the latest CXL standard
+
+  The main header to reference all versions of CXL Base specification registers
+  from the MDE
+
+Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef _CXL_MAIN_H_
+#define _CXL_MAIN_H_
+
+#include <IndustryStandard/Cxl11.h>
+//
+// CXL assigned new Vendor ID
+//
+#define CXL_DVSEC_VENDOR_ID                                             0x1E98
+
+#endif
+
-- 
2.21.0.windows.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH V4 0/2] CXL Specification Registers
  2020-07-24 18:26 [PATCH V4 0/2] CXL Specification Registers Javeed, Ashraf
  2020-07-24 18:26 ` [PATCH V4 1/2] MdePkg/Include/IndustryStandard: CXL 1.1 Registers Javeed, Ashraf
  2020-07-24 18:26 ` [PATCH V4 2/2] MdePkg/Include/IndustryStandard: Main CXL header Javeed, Ashraf
@ 2020-07-27  2:35 ` Liming Gao
  2020-07-27  3:43   ` Liming Gao
  2 siblings, 1 reply; 13+ messages in thread
From: Liming Gao @ 2020-07-27  2:35 UTC (permalink / raw)
  To: Javeed, Ashraf, devel@edk2.groups.io; +Cc: Kinney, Michael D

This version is good to me. Reviewed-by: Liming Gao <liming.gao@intel.com>

-----Original Message-----
From: Javeed, Ashraf <ashraf.javeed@intel.com> 
Sent: 2020年7月25日 2:26
To: devel@edk2.groups.io
Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>
Subject: [PATCH V4 0/2] CXL Specification Registers

These 2 patches introduces the Compute Express Link (CXL) Specificition defined registers.
The Cxl11.h has the actual register definitions of the CXL Specification Revision 1.1; and the Cxl.h is the main header file to include all versions of the CXL register definitions.

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2611

Signed-off-by: Ashraf Javeed <ashraf.javeed@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
--

Ashraf Javeed (2):
  MdePkg/Include/IndustryStandard: CXL 1.1 Registers
  MdePkg/Include/IndustryStandard: Main CXL header

 MdePkg/Include/IndustryStandard/Cxl.h   |  22 ++++++++++++++++++++++
 MdePkg/Include/IndustryStandard/Cxl11.h | 569 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 MdePkg/Include/IndustryStandard/Pci.h   |   6 ++----
 3 files changed, 593 insertions(+), 4 deletions(-)  create mode 100644 MdePkg/Include/IndustryStandard/Cxl.h
 create mode 100644 MdePkg/Include/IndustryStandard/Cxl11.h

--
2.21.0.windows.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH V4 0/2] CXL Specification Registers
  2020-07-27  2:35 ` [PATCH V4 0/2] CXL Specification Registers Liming Gao
@ 2020-07-27  3:43   ` Liming Gao
  0 siblings, 0 replies; 13+ messages in thread
From: Liming Gao @ 2020-07-27  3:43 UTC (permalink / raw)
  To: Javeed, Ashraf, devel@edk2.groups.io; +Cc: Kinney, Michael D

https://github.com/tianocore/edk2/pull/828 for this change, be merged. 

-----Original Message-----
From: Gao, Liming 
Sent: 2020年7月27日 10:36
To: Javeed, Ashraf <ashraf.javeed@intel.com>; devel@edk2.groups.io
Cc: Kinney, Michael D <michael.d.kinney@intel.com>
Subject: RE: [PATCH V4 0/2] CXL Specification Registers

This version is good to me. Reviewed-by: Liming Gao <liming.gao@intel.com>

-----Original Message-----
From: Javeed, Ashraf <ashraf.javeed@intel.com> 
Sent: 2020年7月25日 2:26
To: devel@edk2.groups.io
Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>
Subject: [PATCH V4 0/2] CXL Specification Registers

These 2 patches introduces the Compute Express Link (CXL) Specificition defined registers.
The Cxl11.h has the actual register definitions of the CXL Specification Revision 1.1; and the Cxl.h is the main header file to include all versions of the CXL register definitions.

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2611

Signed-off-by: Ashraf Javeed <ashraf.javeed@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
--

Ashraf Javeed (2):
  MdePkg/Include/IndustryStandard: CXL 1.1 Registers
  MdePkg/Include/IndustryStandard: Main CXL header

 MdePkg/Include/IndustryStandard/Cxl.h   |  22 ++++++++++++++++++++++
 MdePkg/Include/IndustryStandard/Cxl11.h | 569 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 MdePkg/Include/IndustryStandard/Pci.h   |   6 ++----
 3 files changed, 593 insertions(+), 4 deletions(-)  create mode 100644 MdePkg/Include/IndustryStandard/Cxl.h
 create mode 100644 MdePkg/Include/IndustryStandard/Cxl11.h

--
2.21.0.windows.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [edk2-devel] [PATCH V4 1/2] MdePkg/Include/IndustryStandard: CXL 1.1 Registers
  2020-07-24 18:26 ` [PATCH V4 1/2] MdePkg/Include/IndustryStandard: CXL 1.1 Registers Javeed, Ashraf
@ 2020-07-27 14:24   ` Leif Lindholm
  2020-07-27 14:55     ` Liming Gao
  0 siblings, 1 reply; 13+ messages in thread
From: Leif Lindholm @ 2020-07-27 14:24 UTC (permalink / raw)
  To: devel, ashraf.javeed; +Cc: Michael D Kinney, Liming Gao

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.javeed@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@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.

> +
> +#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?

> +
> +//
> +// 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.

> +
> +///
> +/// 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: ...

/
    Leif

> -- 
> 2.21.0.windows.1
> 
> 
> 
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [edk2-devel] [PATCH V4 1/2] MdePkg/Include/IndustryStandard: CXL 1.1 Registers
  2020-07-27 14:24   ` [edk2-devel] [PATCH V4 1/2] MdePkg/Include/IndustryStandard: " Leif Lindholm
@ 2020-07-27 14:55     ` Liming Gao
  2020-07-27 16:14       ` Leif Lindholm
  2020-07-28  6:46       ` Javeed, Ashraf
  0 siblings, 2 replies; 13+ messages in thread
From: Liming Gao @ 2020-07-27 14:55 UTC (permalink / raw)
  To: devel@edk2.groups.io, leif@nuviainc.com, Javeed, Ashraf
  Cc: Kinney, Michael D, Gao, Liming

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.javeed@intel.com>
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@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.javeed@intel.com>
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > Cc: Liming Gao <liming.gao@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
> >
> >
> >
> >
> 
> 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [edk2-devel] [PATCH V4 1/2] MdePkg/Include/IndustryStandard: CXL 1.1 Registers
  2020-07-27 14:55     ` Liming Gao
@ 2020-07-27 16:14       ` Leif Lindholm
  2020-07-28  2:07         ` Liming Gao
  2020-07-28  6:46       ` Javeed, Ashraf
  1 sibling, 1 reply; 13+ messages in thread
From: Leif Lindholm @ 2020-07-27 16:14 UTC (permalink / raw)
  To: Gao, Liming; +Cc: devel@edk2.groups.io, Javeed, Ashraf, Kinney, Michael D

On Mon, Jul 27, 2020 at 14:55:03 +0000, Gao, Liming wrote:
> > > 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.

Yes. The other headers should also be changed, but in the meantime it
would be good to stop adding more incorrect ones.
https://edk2-docs.gitbook.io/edk-ii-c-coding-standards-specification/5_source_files/53_include_files#5-3-5-all-include-file-contents-must-be-protected-by-a-include-guard

> > > +
> > > +//
> > > +// 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. 

Yes. And it should be used when structs need to be compacted.
All of the bitfield structs are single-variable structs, so the
packing has no effect on them, other than setting the struct alignment
requirements to 1 byte, which will not be correct (or efficient) on all
architectures.

> > 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?

Sure, I can do that. Any thoughts on others to add than me?

/
    Leif

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [edk2-devel] [PATCH V4 1/2] MdePkg/Include/IndustryStandard: CXL 1.1 Registers
  2020-07-27 16:14       ` Leif Lindholm
@ 2020-07-28  2:07         ` Liming Gao
  2020-07-28  2:22           ` Michael D Kinney
  2020-07-28  2:36           ` Ni, Ray
  0 siblings, 2 replies; 13+ messages in thread
From: Liming Gao @ 2020-07-28  2:07 UTC (permalink / raw)
  To: Leif Lindholm, Javeed, Ashraf, Laszlo Ersek, Sean Brogan
  Cc: devel@edk2.groups.io, Kinney, Michael D

Leif:

-----Original Message-----
From: Leif Lindholm <leif@nuviainc.com> 
Sent: 2020年7月28日 0:14
To: Gao, Liming <liming.gao@intel.com>
Cc: devel@edk2.groups.io; Javeed, Ashraf <ashraf.javeed@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: Re: [edk2-devel] [PATCH V4 1/2] MdePkg/Include/IndustryStandard: CXL 1.1 Registers

On Mon, Jul 27, 2020 at 14:55:03 +0000, Gao, Liming wrote:
> > > 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.

Yes. The other headers should also be changed, but in the meantime it would be good to stop adding more incorrect ones.
https://edk2-docs.gitbook.io/edk-ii-c-coding-standards-specification/5_source_files/53_include_files#5-3-5-all-include-file-contents-must-be-protected-by-a-include-guard

[Liming] Thank for your point. I miss this one, too. Now, most cases don't follow this rule. So, there is no good example for the reference. 
I agree the rule to apply the strict check for new adding file. I will check whether ECC has this check point. 

> > > +
> > > +//
> > > +// 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. 

Yes. And it should be used when structs need to be compacted.
All of the bitfield structs are single-variable structs, so the packing has no effect on them, other than setting the struct alignment requirements to 1 byte, which will not be correct (or efficient) on all architectures.

[Liming] Yes. There is no effect for bitfield structure. This header file still includes some structure, such as CXL_1_1_DVSEC_FLEX_BUS_DEVICE. They may have the compact alignment requirement. 
@Javeed, Ashraf, can you conform it?

> > 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?

Sure, I can do that. Any thoughts on others to add than me?

[Liming] Thanks. Laszlo or Sean may be added if they are also interested in IndustryStandard header file. 

Thanks
Liming
/
    Leif

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [edk2-devel] [PATCH V4 1/2] MdePkg/Include/IndustryStandard: CXL 1.1 Registers
  2020-07-28  2:07         ` Liming Gao
@ 2020-07-28  2:22           ` Michael D Kinney
  2020-07-28  2:36           ` Ni, Ray
  1 sibling, 0 replies; 13+ messages in thread
From: Michael D Kinney @ 2020-07-28  2:22 UTC (permalink / raw)
  To: Gao, Liming, Leif Lindholm, Javeed, Ashraf, Laszlo Ersek,
	Sean Brogan, Kinney, Michael D
  Cc: devel@edk2.groups.io

Liming,

Pragma once is an alternative to include guards.  If we want to 
remove the use of the include guard macros that start with '_',
we can consider using pragma once everywhere.

    https://en.wikipedia.org/wiki/Pragma_once

If other include files have packed structure requirements,
then they should do it in their own files.  No include file
should depend on #pragma pack from another include file.

Mike

> -----Original Message-----
> From: Gao, Liming <liming.gao@intel.com>
> Sent: Monday, July 27, 2020 7:07 PM
> To: Leif Lindholm <leif@nuviainc.com>; Javeed, Ashraf
> <ashraf.javeed@intel.com>; Laszlo Ersek
> <lersek@redhat.com>; Sean Brogan
> <sean.brogan@microsoft.com>
> Cc: devel@edk2.groups.io; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Subject: RE: [edk2-devel] [PATCH V4 1/2]
> MdePkg/Include/IndustryStandard: CXL 1.1 Registers
> 
> Leif:
> 
> -----Original Message-----
> From: Leif Lindholm <leif@nuviainc.com>
> Sent: 2020年7月28日 0:14
> To: Gao, Liming <liming.gao@intel.com>
> Cc: devel@edk2.groups.io; Javeed, Ashraf
> <ashraf.javeed@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Subject: Re: [edk2-devel] [PATCH V4 1/2]
> MdePkg/Include/IndustryStandard: CXL 1.1 Registers
> 
> On Mon, Jul 27, 2020 at 14:55:03 +0000, Gao, Liming
> wrote:
> > > > 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.
> 
> Yes. The other headers should also be changed, but in
> the meantime it would be good to stop adding more
> incorrect ones.
> https://edk2-docs.gitbook.io/edk-ii-c-coding-standards-
> specification/5_source_files/53_include_files#5-3-5-
> all-include-file-contents-must-be-protected-by-a-
> include-guard
> 
> [Liming] Thank for your point. I miss this one, too.
> Now, most cases don't follow this rule. So, there is no
> good example for the reference.
> I agree the rule to apply the strict check for new
> adding file. I will check whether ECC has this check
> point.
> 
> > > > +
> > > > +//
> > > > +// 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.
> 
> Yes. And it should be used when structs need to be
> compacted.
> All of the bitfield structs are single-variable
> structs, so the packing has no effect on them, other
> than setting the struct alignment requirements to 1
> byte, which will not be correct (or efficient) on all
> architectures.
> 
> [Liming] Yes. There is no effect for bitfield
> structure. This header file still includes some
> structure, such as CXL_1_1_DVSEC_FLEX_BUS_DEVICE. They
> may have the compact alignment requirement.
> @Javeed, Ashraf, can you conform it?
> 
> > > 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?
> 
> Sure, I can do that. Any thoughts on others to add than
> me?
> 
> [Liming] Thanks. Laszlo or Sean may be added if they
> are also interested in IndustryStandard header file.
> 
> Thanks
> Liming
> /
>     Leif

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [edk2-devel] [PATCH V4 1/2] MdePkg/Include/IndustryStandard: CXL 1.1 Registers
  2020-07-28  2:07         ` Liming Gao
  2020-07-28  2:22           ` Michael D Kinney
@ 2020-07-28  2:36           ` Ni, Ray
  2020-07-28  6:18             ` Javeed, Ashraf
  1 sibling, 1 reply; 13+ messages in thread
From: Ni, Ray @ 2020-07-28  2:36 UTC (permalink / raw)
  To: devel@edk2.groups.io, Gao, Liming, Leif Lindholm, Javeed, Ashraf,
	Laszlo Ersek, Sean Brogan
  Cc: Kinney, Michael D, Gough, Robert

I am not sure if Robert (Cced) has the interest to be the reviewer of the MdePkg/Include/IndustryStandard directory.

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Liming Gao
> Sent: Tuesday, July 28, 2020 10:07 AM
> To: Leif Lindholm <leif@nuviainc.com>; Javeed, Ashraf <ashraf.javeed@intel.com>; Laszlo Ersek <lersek@redhat.com>;
> Sean Brogan <sean.brogan@microsoft.com>
> Cc: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: Re: [edk2-devel] [PATCH V4 1/2] MdePkg/Include/IndustryStandard: CXL 1.1 Registers
> 
> Leif:
> 
> -----Original Message-----
> From: Leif Lindholm <leif@nuviainc.com>
> Sent: 2020年7月28日 0:14
> To: Gao, Liming <liming.gao@intel.com>
> Cc: devel@edk2.groups.io; Javeed, Ashraf <ashraf.javeed@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: Re: [edk2-devel] [PATCH V4 1/2] MdePkg/Include/IndustryStandard: CXL 1.1 Registers
> 
> On Mon, Jul 27, 2020 at 14:55:03 +0000, Gao, Liming wrote:
> > > > 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.
> 
> Yes. The other headers should also be changed, but in the meantime it would be good to stop adding more incorrect ones.
> https://edk2-docs.gitbook.io/edk-ii-c-coding-standards-specification/5_source_files/53_include_files#5-3-5-all-include-file-
> contents-must-be-protected-by-a-include-guard
> 
> [Liming] Thank for your point. I miss this one, too. Now, most cases don't follow this rule. So, there is no good example for
> the reference.
> I agree the rule to apply the strict check for new adding file. I will check whether ECC has this check point.
> 
> > > > +
> > > > +//
> > > > +// 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.
> 
> Yes. And it should be used when structs need to be compacted.
> All of the bitfield structs are single-variable structs, so the packing has no effect on them, other than setting the struct
> alignment requirements to 1 byte, which will not be correct (or efficient) on all architectures.
> 
> [Liming] Yes. There is no effect for bitfield structure. This header file still includes some structure, such as
> CXL_1_1_DVSEC_FLEX_BUS_DEVICE. They may have the compact alignment requirement.
> @Javeed, Ashraf, can you conform it?
> 
> > > 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?
> 
> Sure, I can do that. Any thoughts on others to add than me?
> 
> [Liming] Thanks. Laszlo or Sean may be added if they are also interested in IndustryStandard header file.
> 
> Thanks
> Liming
> /
>     Leif
> 
> 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [edk2-devel] [PATCH V4 1/2] MdePkg/Include/IndustryStandard: CXL 1.1 Registers
  2020-07-28  2:36           ` Ni, Ray
@ 2020-07-28  6:18             ` Javeed, Ashraf
  0 siblings, 0 replies; 13+ messages in thread
From: Javeed, Ashraf @ 2020-07-28  6:18 UTC (permalink / raw)
  To: Ni, Ray, devel@edk2.groups.io, Gao, Liming, Leif Lindholm,
	Laszlo Ersek, Sean Brogan
  Cc: Kinney, Michael D, Gough, Robert

Leif, Liming,
My comments inline below.
Thanks
Ashraf

> -----Original Message-----
> From: Ni, Ray <ray.ni@intel.com>
> Sent: Tuesday, July 28, 2020 8:06 AM
> To: devel@edk2.groups.io; Gao, Liming <liming.gao@intel.com>; Leif Lindholm
> <leif@nuviainc.com>; Javeed, Ashraf <ashraf.javeed@intel.com>; Laszlo Ersek
> <lersek@redhat.com>; Sean Brogan <sean.brogan@microsoft.com>
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gough, Robert
> <robert.gough@intel.com>
> Subject: RE: [edk2-devel] [PATCH V4 1/2] MdePkg/Include/IndustryStandard:
> CXL 1.1 Registers
> 
> I am not sure if Robert (Cced) has the interest to be the reviewer of the
> MdePkg/Include/IndustryStandard directory.
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Liming
> > Gao
> > Sent: Tuesday, July 28, 2020 10:07 AM
> > To: Leif Lindholm <leif@nuviainc.com>; Javeed, Ashraf
> > <ashraf.javeed@intel.com>; Laszlo Ersek <lersek@redhat.com>; Sean
> > Brogan <sean.brogan@microsoft.com>
> > Cc: devel@edk2.groups.io; Kinney, Michael D
> > <michael.d.kinney@intel.com>
> > Subject: Re: [edk2-devel] [PATCH V4 1/2]
> > MdePkg/Include/IndustryStandard: CXL 1.1 Registers
> >
> > Leif:
> >
> > -----Original Message-----
> > From: Leif Lindholm <leif@nuviainc.com>
> > Sent: 2020年7月28日 0:14
> > To: Gao, Liming <liming.gao@intel.com>
> > Cc: devel@edk2.groups.io; Javeed, Ashraf <ashraf.javeed@intel.com>;
> > Kinney, Michael D <michael.d.kinney@intel.com>
> > Subject: Re: [edk2-devel] [PATCH V4 1/2]
> > MdePkg/Include/IndustryStandard: CXL 1.1 Registers
> >
> > On Mon, Jul 27, 2020 at 14:55:03 +0000, Gao, Liming wrote:
> > > > > 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.
> >
> > Yes. The other headers should also be changed, but in the meantime it would
> be good to stop adding more incorrect ones.
> > https://edk2-docs.gitbook.io/edk-ii-c-coding-standards-specification/5
> > _source_files/53_include_files#5-3-5-all-include-file-
> > contents-must-be-protected-by-a-include-guard
> >
> > [Liming] Thank for your point. I miss this one, too. Now, most cases
> > don't follow this rule. So, there is no good example for the reference.
> > I agree the rule to apply the strict check for new adding file. I will check
> whether ECC has this check point.
> >
Can make the change to remove the _ in the beginning of the macro guards when I am adding for CXL 2.0 specification

> > > > > +
> > > > > +//
> > > > > +// 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.
> >
> > Yes. And it should be used when structs need to be compacted.
> > All of the bitfield structs are single-variable structs, so the
> > packing has no effect on them, other than setting the struct alignment
> requirements to 1 byte, which will not be correct (or efficient) on all
> architectures.
> >
> > [Liming] Yes. There is no effect for bitfield structure. This header
> > file still includes some structure, such as CXL_1_1_DVSEC_FLEX_BUS_DEVICE.
> They may have the compact alignment requirement.
> > @Javeed, Ashraf, can you conform it?
> >
Yes, all the grouped CXL registers that are listed below are packed to fixed offsets, hence I thought it is better to have pack(1) in the beginning and pack() in the end to avoid any effect of global compiler settings...
-CXL_1_1_DVSEC_FLEX_BUS_DEVICE,
-CXL_1_1_DVSEC_FLEX_BUS_PORT,
-CXL_1_1_RAS_CAPABILITY_STRUCTURE,
-CXL_1_1_LINK_CAPABILITY_STRUCTURE
Is the preference is to add the packing instruction to each group independently?

> > > > 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?
> >
> > Sure, I can do that. Any thoughts on others to add than me?
> >
> > [Liming] Thanks. Laszlo or Sean may be added if they are also interested in
> IndustryStandard header file.
> >
> > Thanks
> > Liming
> > /
> >     Leif
> >
> > 
> 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [edk2-devel] [PATCH V4 1/2] MdePkg/Include/IndustryStandard: CXL 1.1 Registers
  2020-07-27 14:55     ` Liming Gao
  2020-07-27 16:14       ` Leif Lindholm
@ 2020-07-28  6:46       ` Javeed, Ashraf
  1 sibling, 0 replies; 13+ messages in thread
From: Javeed, Ashraf @ 2020-07-28  6:46 UTC (permalink / raw)
  To: Gao, Liming, devel@edk2.groups.io, leif@nuviainc.com; +Cc: Kinney, Michael D

My response inline.
Regards
Ashraf

> -----Original Message-----
> From: Gao, Liming <liming.gao@intel.com>
> Sent: Monday, July 27, 2020 8:25 PM
> To: devel@edk2.groups.io; leif@nuviainc.com; Javeed, Ashraf
> <ashraf.javeed@intel.com>
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> <liming.gao@intel.com>
> Subject: RE: [edk2-devel] [PATCH V4 1/2] MdePkg/Include/IndustryStandard:
> CXL 1.1 Registers
> 
> 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.javeed@intel.com>
> > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> > <liming.gao@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.javeed@intel.com>
> > > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > > Cc: Liming Gao <liming.gao@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?
> 
Yes, the Intel Vendor ID is defined in CXL 1.1 Specification, section 7.1.1.

> >
> > > +
> > > +//
> > > +// 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
> > >
> > >
> > >
> > >
> >
> > 


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2020-07-28  6:46 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-07-24 18:26 [PATCH V4 0/2] CXL Specification Registers Javeed, Ashraf
2020-07-24 18:26 ` [PATCH V4 1/2] MdePkg/Include/IndustryStandard: CXL 1.1 Registers Javeed, Ashraf
2020-07-27 14:24   ` [edk2-devel] [PATCH V4 1/2] MdePkg/Include/IndustryStandard: " Leif Lindholm
2020-07-27 14:55     ` Liming Gao
2020-07-27 16:14       ` Leif Lindholm
2020-07-28  2:07         ` Liming Gao
2020-07-28  2:22           ` Michael D Kinney
2020-07-28  2:36           ` Ni, Ray
2020-07-28  6:18             ` Javeed, Ashraf
2020-07-28  6:46       ` Javeed, Ashraf
2020-07-24 18:26 ` [PATCH V4 2/2] MdePkg/Include/IndustryStandard: Main CXL header Javeed, Ashraf
2020-07-27  2:35 ` [PATCH V4 0/2] CXL Specification Registers Liming Gao
2020-07-27  3:43   ` Liming Gao

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox