* [Patch 1/1] MdePkg/IndustryStandard: Fix CXL 1.1 structure layout issues @ 2020-11-12 23:08 Michael D Kinney 2020-11-13 1:50 ` Zhiguang Liu 2020-11-16 17:41 ` [edk2-devel] " Michael D Kinney 0 siblings, 2 replies; 6+ messages in thread From: Michael D Kinney @ 2020-11-12 23:08 UTC (permalink / raw) To: devel; +Cc: Liming Gao, Zhiguang Liu, Ashraf Javeed https://bugzilla.tianocore.org/show_bug.cgi?id=3074 * Fix offset of LinkLayerControlAndStatus in the CXL_1_1_LINK_CAPABILITY_STRUCTURE structure * Fix offset of LinkLayerAckTimerControl in the CXL_1_1_LINK_CAPABILITY_STRUCTURE structure * Fix offset of LinkLayerDefeature in the CXL_1_1_LINK_CAPABILITY_STRUCTURE structure * Add CXL_11_SIZE_ASSERT() macro to verify the size of a register layout structure at compile time and use it to verify the sizes of the CXL 1.1 register structures. * Add CXL_11_OFFSET_ASSERT() macro to verify the offset of fields in a register layout structure at compiler time and use it to verify the offset of fields in CXL 1.1 register structures. Cc: Liming Gao <gaoliming@byosoft.com.cn> Cc: Zhiguang Liu <zhiguang.liu@intel.com> Cc: Ashraf Javeed <ashraf.javeed@intel.com> Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com> --- MdePkg/Include/IndustryStandard/Cxl11.h | 96 ++++++++++++++++++++++++- 1 file changed, 93 insertions(+), 3 deletions(-) diff --git a/MdePkg/Include/IndustryStandard/Cxl11.h b/MdePkg/Include/IndustryStandard/Cxl11.h index 933c1ab817e8..46cb271d3c74 100644 --- a/MdePkg/Include/IndustryStandard/Cxl11.h +++ b/MdePkg/Include/IndustryStandard/Cxl11.h @@ -32,6 +32,40 @@ SPDX-License-Identifier: BSD-2-Clause-Patent // #pragma pack(1) +/** + Macro used to verify the size of a data type at compile time and trigger a + STATIC_ASSERT() with an error message if the size of the data type does not + match the expected size. + + @param TypeName Type name of data type to verify. + @param ExpectedSize The expected size, in bytes, of the data type specified + by TypeName. +**/ +#define CXL_11_SIZE_ASSERT(TypeName, ExpectedSize) \ + STATIC_ASSERT ( \ + sizeof (TypeName) == ExpectedSize, \ + "Size of " #TypeName \ + " does not meet CXL 1.1 Specification requirements." \ + ) + +/** + Macro used to verify the offset of a field in a data type at compile time and + trigger a STATIC_ASSERT() with an error message if the offset of the field in + the data type does not match the expected offset. + + @param TypeName Type name of data type to verify. + @param FieldName Field name in the data type specified by TypeName to + verify. + @param ExpectedOffset The expected offset, in bytes, of the field specified + by TypeName and FieldName. +**/ +#define CXL_11_OFFSET_ASSERT(TypeName, FieldName, ExpectedOffset) \ + STATIC_ASSERT ( \ + OFFSET_OF (TypeName, FieldName) == ExpectedOffset, \ + "Offset of " #TypeName "." #FieldName \ + " does not meet CXL 1.1 Specification requirements." \ + ) + /// /// The PCIe DVSEC for Flex Bus Device ///@{ @@ -201,6 +235,25 @@ typedef struct { 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; + +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, Header , 0x00); +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, DesignatedVendorSpecificHeader1, 0x04); +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, DesignatedVendorSpecificHeader2, 0x08); +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, DeviceCapability , 0x0A); +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, DeviceControl , 0x0C); +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, DeviceStatus , 0x0E); +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, DeviceControl2 , 0x10); +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, DeviceStatus2 , 0x12); +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, DeviceLock , 0x14); +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, DeviceRange1SizeHigh , 0x18); +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, DeviceRange1SizeLow , 0x1C); +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, DeviceRange1BaseHigh , 0x20); +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, DeviceRange1BaseLow , 0x24); +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, DeviceRange2SizeHigh , 0x28); +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, DeviceRange2SizeLow , 0x2C); +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, DeviceRange2BaseHigh , 0x30); +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, DeviceRange2BaseLow , 0x34); +CXL_11_SIZE_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE , 0x38); ///@} /// @@ -265,6 +318,14 @@ typedef struct { 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_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_PORT, Header , 0x00); +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_PORT, DesignatedVendorSpecificHeader1, 0x04); +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_PORT, DesignatedVendorSpecificHeader2, 0x08); +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_PORT, PortCapability , 0x0A); +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_PORT, PortControl , 0x0C); +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_PORT, PortStatus , 0x0E); +CXL_11_SIZE_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_PORT , 0x10); ///@} /// @@ -423,6 +484,15 @@ typedef struct { UINT32 HeaderLog[16]; } CXL_1_1_RAS_CAPABILITY_STRUCTURE; +CXL_11_OFFSET_ASSERT (CXL_1_1_RAS_CAPABILITY_STRUCTURE, UncorrectableErrorStatus , 0x00); +CXL_11_OFFSET_ASSERT (CXL_1_1_RAS_CAPABILITY_STRUCTURE, UncorrectableErrorMask , 0x04); +CXL_11_OFFSET_ASSERT (CXL_1_1_RAS_CAPABILITY_STRUCTURE, UncorrectableErrorSeverity , 0x08); +CXL_11_OFFSET_ASSERT (CXL_1_1_RAS_CAPABILITY_STRUCTURE, CorrectableErrorStatus , 0x0C); +CXL_11_OFFSET_ASSERT (CXL_1_1_RAS_CAPABILITY_STRUCTURE, CorrectableErrorMask , 0x10); +CXL_11_OFFSET_ASSERT (CXL_1_1_RAS_CAPABILITY_STRUCTURE, ErrorCapabilitiesAndControl, 0x14); +CXL_11_OFFSET_ASSERT (CXL_1_1_RAS_CAPABILITY_STRUCTURE, HeaderLog , 0x18); +CXL_11_SIZE_ASSERT (CXL_1_1_RAS_CAPABILITY_STRUCTURE , 0x58); + typedef union { struct { UINT32 DeviceTrustLevel : 2; // bit 0..1 @@ -435,6 +505,9 @@ typedef struct { CXL_1_1_SECURITY_POLICY SecurityPolicy; } CXL_1_1_SECURITY_CAPABILITY_STRUCTURE; +CXL_11_OFFSET_ASSERT (CXL_1_1_SECURITY_CAPABILITY_STRUCTURE, SecurityPolicy, 0x0); +CXL_11_SIZE_ASSERT (CXL_1_1_SECURITY_CAPABILITY_STRUCTURE, 0x4); + typedef union { struct { UINT64 CxlLinkVersionSupported : 4; // bit 0..3 @@ -460,7 +533,7 @@ typedef union { UINT16 LlRetryBufferConsumed : 8; // bit 5..12 UINT16 Reserved : 3; // bit 13..15 } Bits; - UINT16 Uint16; + UINT64 Uint64; } CXL_LINK_LAYER_CONTROL_AND_STATUS; typedef union { @@ -501,7 +574,7 @@ typedef union { UINT32 AckForceThreshold : 8; // bit 0..7 UINT32 AckFLushRetimer : 10; // bit 8..17 } Bits; - UINT32 Uint32; + UINT64 Uint64; } CXL_LINK_LAYER_ACK_TIMER_CONTROL; typedef union { @@ -509,7 +582,7 @@ typedef union { UINT32 MdhDisable : 1; // bit 0..0 UINT32 Reserved : 31; // bit 1..31 } Bits; - UINT32 Uint32; + UINT64 Uint64; } CXL_LINK_LAYER_DEFEATURE; typedef struct { @@ -522,6 +595,15 @@ typedef struct { CXL_LINK_LAYER_DEFEATURE LinkLayerDefeature; } CXL_1_1_LINK_CAPABILITY_STRUCTURE; +CXL_11_OFFSET_ASSERT (CXL_1_1_LINK_CAPABILITY_STRUCTURE, LinkLayerCapability , 0x00); +CXL_11_OFFSET_ASSERT (CXL_1_1_LINK_CAPABILITY_STRUCTURE, LinkLayerControlStatus , 0x08); +CXL_11_OFFSET_ASSERT (CXL_1_1_LINK_CAPABILITY_STRUCTURE, LinkLayerRxCreditControl , 0x10); +CXL_11_OFFSET_ASSERT (CXL_1_1_LINK_CAPABILITY_STRUCTURE, LinkLayerRxCreditReturnStatus, 0x18); +CXL_11_OFFSET_ASSERT (CXL_1_1_LINK_CAPABILITY_STRUCTURE, LinkLayerTxCreditStatus , 0x20); +CXL_11_OFFSET_ASSERT (CXL_1_1_LINK_CAPABILITY_STRUCTURE, LinkLayerAckTimerControl , 0x28); +CXL_11_OFFSET_ASSERT (CXL_1_1_LINK_CAPABILITY_STRUCTURE, LinkLayerDefeature , 0x30); +CXL_11_SIZE_ASSERT (CXL_1_1_LINK_CAPABILITY_STRUCTURE , 0x38); + #define CXL_IO_ARBITRATION_CONTROL_OFFSET 0x180 typedef union { struct { @@ -532,6 +614,8 @@ typedef union { UINT32 Uint32; } CXL_IO_ARBITRATION_CONTROL; +CXL_11_SIZE_ASSERT (CXL_IO_ARBITRATION_CONTROL, 0x4); + #define CXL_CACHE_MEMORY_ARBITRATION_CONTROL_OFFSET 0x1C0 typedef union { struct { @@ -541,6 +625,9 @@ typedef union { } Bits; UINT32 Uint32; } CXL_CACHE_MEMORY_ARBITRATION_CONTROL; + +CXL_11_SIZE_ASSERT (CXL_CACHE_MEMORY_ARBITRATION_CONTROL, 0x4); + ///@} /// The CXL.RCRB base register definition @@ -554,6 +641,9 @@ typedef union { } Bits; UINT64 Uint64; } CXL_RCRB_BASE; + +CXL_11_SIZE_ASSERT (CXL_RCRB_BASE, 0x8); + ///@} #pragma pack() -- 2.29.2.windows.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Patch 1/1] MdePkg/IndustryStandard: Fix CXL 1.1 structure layout issues 2020-11-12 23:08 [Patch 1/1] MdePkg/IndustryStandard: Fix CXL 1.1 structure layout issues Michael D Kinney @ 2020-11-13 1:50 ` Zhiguang Liu 2020-11-13 8:36 ` Javeed, Ashraf 2020-11-16 17:41 ` [edk2-devel] " Michael D Kinney 1 sibling, 1 reply; 6+ messages in thread From: Zhiguang Liu @ 2020-11-13 1:50 UTC (permalink / raw) To: Kinney, Michael D, devel@edk2.groups.io; +Cc: Liming Gao, Javeed, Ashraf Reviewed-by: Zhiguang Liu <zhiguang.liu@intel.com> > -----Original Message----- > From: Michael D Kinney <michael.d.kinney@intel.com> > Sent: Friday, November 13, 2020 7:08 AM > To: devel@edk2.groups.io > Cc: Liming Gao <gaoliming@byosoft.com.cn>; Liu, Zhiguang > <zhiguang.liu@intel.com>; Javeed, Ashraf <ashraf.javeed@intel.com> > Subject: [Patch 1/1] MdePkg/IndustryStandard: Fix CXL 1.1 structure layout > issues > > https://bugzilla.tianocore.org/show_bug.cgi?id=3074 > > * Fix offset of LinkLayerControlAndStatus in the > CXL_1_1_LINK_CAPABILITY_STRUCTURE structure > * Fix offset of LinkLayerAckTimerControl in the > CXL_1_1_LINK_CAPABILITY_STRUCTURE structure > * Fix offset of LinkLayerDefeature in > the CXL_1_1_LINK_CAPABILITY_STRUCTURE structure > * Add CXL_11_SIZE_ASSERT() macro to verify the size of > a register layout structure at compile time and use > it to verify the sizes of the CXL 1.1 register structures. > * Add CXL_11_OFFSET_ASSERT() macro to verify the offset of > fields in a register layout structure at compiler time and > use it to verify the offset of fields in CXL 1.1 > register structures. > > Cc: Liming Gao <gaoliming@byosoft.com.cn> > Cc: Zhiguang Liu <zhiguang.liu@intel.com> > Cc: Ashraf Javeed <ashraf.javeed@intel.com> > Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com> > --- > MdePkg/Include/IndustryStandard/Cxl11.h | 96 ++++++++++++++++++++++++- > 1 file changed, 93 insertions(+), 3 deletions(-) > > diff --git a/MdePkg/Include/IndustryStandard/Cxl11.h > b/MdePkg/Include/IndustryStandard/Cxl11.h > index 933c1ab817e8..46cb271d3c74 100644 > --- a/MdePkg/Include/IndustryStandard/Cxl11.h > +++ b/MdePkg/Include/IndustryStandard/Cxl11.h > @@ -32,6 +32,40 @@ SPDX-License-Identifier: BSD-2-Clause-Patent // > #pragma pack(1) > > +/** > + Macro used to verify the size of a data type at compile time and > +trigger a > + STATIC_ASSERT() with an error message if the size of the data type > +does not > + match the expected size. > + > + @param TypeName Type name of data type to verify. > + @param ExpectedSize The expected size, in bytes, of the data type specified > + by TypeName. > +**/ > +#define CXL_11_SIZE_ASSERT(TypeName, ExpectedSize) \ > + STATIC_ASSERT ( \ > + sizeof (TypeName) == ExpectedSize, \ > + "Size of " #TypeName \ > + " does not meet CXL 1.1 Specification requirements." \ > + ) > + > +/** > + Macro used to verify the offset of a field in a data type at compile > +time and > + trigger a STATIC_ASSERT() with an error message if the offset of the > +field in > + the data type does not match the expected offset. > + > + @param TypeName Type name of data type to verify. > + @param FieldName Field name in the data type specified by TypeName > to > + verify. > + @param ExpectedOffset The expected offset, in bytes, of the field specified > + by TypeName and FieldName. > +**/ > +#define CXL_11_OFFSET_ASSERT(TypeName, FieldName, ExpectedOffset) \ > + STATIC_ASSERT ( \ > + OFFSET_OF (TypeName, FieldName) == ExpectedOffset, \ > + "Offset of " #TypeName "." #FieldName \ > + " does not meet CXL 1.1 Specification requirements." \ > + ) > + > /// > /// The PCIe DVSEC for Flex Bus Device > ///@{ > @@ -201,6 +235,25 @@ typedef struct { > 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; > + > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, > Header , 0x00); > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, > +DesignatedVendorSpecificHeader1, 0x04); CXL_11_OFFSET_ASSERT > (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, DesignatedVendorSpecificHeader2, 0x08); > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, > DeviceCapability , 0x0A); > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, > DeviceControl , 0x0C); > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, > DeviceStatus , 0x0E); > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, > DeviceControl2 , 0x10); > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, > DeviceStatus2 , 0x12); > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, > DeviceLock , 0x14); > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, > DeviceRange1SizeHigh , 0x18); > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, > DeviceRange1SizeLow , 0x1C); > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, > DeviceRange1BaseHigh , 0x20); > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, > DeviceRange1BaseLow , 0x24); > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, > DeviceRange2SizeHigh , 0x28); > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, > DeviceRange2SizeLow , 0x2C); > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, > DeviceRange2BaseHigh , 0x30); > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, > DeviceRange2BaseLow , 0x34); > +CXL_11_SIZE_ASSERT > (CXL_1_1_DVSEC_FLEX_BUS_DEVICE , 0x38); > ///@} > > /// > @@ -265,6 +318,14 @@ typedef struct { > 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_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_PORT, > Header , 0x00); > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_PORT, > +DesignatedVendorSpecificHeader1, 0x04); CXL_11_OFFSET_ASSERT > (CXL_1_1_DVSEC_FLEX_BUS_PORT, DesignatedVendorSpecificHeader2, 0x08); > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_PORT, > PortCapability , 0x0A); > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_PORT, > PortControl , 0x0C); > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_PORT, > PortStatus , 0x0E); > +CXL_11_SIZE_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_PORT , > 0x10); > ///@} > > /// > @@ -423,6 +484,15 @@ typedef struct { > UINT32 HeaderLog[16]; > } CXL_1_1_RAS_CAPABILITY_STRUCTURE; > > +CXL_11_OFFSET_ASSERT (CXL_1_1_RAS_CAPABILITY_STRUCTURE, > UncorrectableErrorStatus , 0x00); > +CXL_11_OFFSET_ASSERT (CXL_1_1_RAS_CAPABILITY_STRUCTURE, > UncorrectableErrorMask , 0x04); > +CXL_11_OFFSET_ASSERT (CXL_1_1_RAS_CAPABILITY_STRUCTURE, > UncorrectableErrorSeverity , 0x08); > +CXL_11_OFFSET_ASSERT (CXL_1_1_RAS_CAPABILITY_STRUCTURE, > CorrectableErrorStatus , 0x0C); > +CXL_11_OFFSET_ASSERT (CXL_1_1_RAS_CAPABILITY_STRUCTURE, > CorrectableErrorMask , 0x10); > +CXL_11_OFFSET_ASSERT (CXL_1_1_RAS_CAPABILITY_STRUCTURE, > ErrorCapabilitiesAndControl, 0x14); > +CXL_11_OFFSET_ASSERT (CXL_1_1_RAS_CAPABILITY_STRUCTURE, > HeaderLog , 0x18); > +CXL_11_SIZE_ASSERT > (CXL_1_1_RAS_CAPABILITY_STRUCTURE , 0x58); > + > typedef union { > struct { > UINT32 DeviceTrustLevel : 2; // bit 0..1 > @@ -435,6 +505,9 @@ typedef struct { > CXL_1_1_SECURITY_POLICY SecurityPolicy; > } CXL_1_1_SECURITY_CAPABILITY_STRUCTURE; > > +CXL_11_OFFSET_ASSERT (CXL_1_1_SECURITY_CAPABILITY_STRUCTURE, > SecurityPolicy, 0x0); > +CXL_11_SIZE_ASSERT (CXL_1_1_SECURITY_CAPABILITY_STRUCTURE, > 0x4); > + > typedef union { > struct { > UINT64 CxlLinkVersionSupported : 4; // bit 0..3 > @@ -460,7 +533,7 @@ typedef union { > UINT16 LlRetryBufferConsumed : 8; // bit 5..12 > UINT16 Reserved : 3; // bit 13..15 > } Bits; > - UINT16 Uint16; > + UINT64 Uint64; > } CXL_LINK_LAYER_CONTROL_AND_STATUS; > > typedef union { > @@ -501,7 +574,7 @@ typedef union { > UINT32 AckForceThreshold : 8; // bit 0..7 > UINT32 AckFLushRetimer : 10; // bit 8..17 > } Bits; > - UINT32 Uint32; > + UINT64 Uint64; > } CXL_LINK_LAYER_ACK_TIMER_CONTROL; > > typedef union { > @@ -509,7 +582,7 @@ typedef union { > UINT32 MdhDisable : 1; // bit 0..0 > UINT32 Reserved : 31; // bit 1..31 > } Bits; > - UINT32 Uint32; > + UINT64 Uint64; > } CXL_LINK_LAYER_DEFEATURE; > > typedef struct { > @@ -522,6 +595,15 @@ typedef struct { > CXL_LINK_LAYER_DEFEATURE LinkLayerDefeature; > } CXL_1_1_LINK_CAPABILITY_STRUCTURE; > > +CXL_11_OFFSET_ASSERT (CXL_1_1_LINK_CAPABILITY_STRUCTURE, > LinkLayerCapability , 0x00); > +CXL_11_OFFSET_ASSERT (CXL_1_1_LINK_CAPABILITY_STRUCTURE, > LinkLayerControlStatus , 0x08); > +CXL_11_OFFSET_ASSERT (CXL_1_1_LINK_CAPABILITY_STRUCTURE, > LinkLayerRxCreditControl , 0x10); > +CXL_11_OFFSET_ASSERT (CXL_1_1_LINK_CAPABILITY_STRUCTURE, > LinkLayerRxCreditReturnStatus, 0x18); > +CXL_11_OFFSET_ASSERT (CXL_1_1_LINK_CAPABILITY_STRUCTURE, > LinkLayerTxCreditStatus , 0x20); > +CXL_11_OFFSET_ASSERT (CXL_1_1_LINK_CAPABILITY_STRUCTURE, > LinkLayerAckTimerControl , 0x28); > +CXL_11_OFFSET_ASSERT (CXL_1_1_LINK_CAPABILITY_STRUCTURE, > LinkLayerDefeature , 0x30); > +CXL_11_SIZE_ASSERT > (CXL_1_1_LINK_CAPABILITY_STRUCTURE , 0x38); > + > #define CXL_IO_ARBITRATION_CONTROL_OFFSET 0x180 > typedef union { > struct { > @@ -532,6 +614,8 @@ typedef union { > UINT32 Uint32; > } CXL_IO_ARBITRATION_CONTROL; > > +CXL_11_SIZE_ASSERT (CXL_IO_ARBITRATION_CONTROL, 0x4); > + > #define CXL_CACHE_MEMORY_ARBITRATION_CONTROL_OFFSET > 0x1C0 > typedef union { > struct { > @@ -541,6 +625,9 @@ typedef union { > } Bits; > UINT32 Uint32; > } CXL_CACHE_MEMORY_ARBITRATION_CONTROL; > + > +CXL_11_SIZE_ASSERT (CXL_CACHE_MEMORY_ARBITRATION_CONTROL, 0x4); > + > ///@} > > /// The CXL.RCRB base register definition @@ -554,6 +641,9 @@ typedef > union { > } Bits; > UINT64 Uint64; > } CXL_RCRB_BASE; > + > +CXL_11_SIZE_ASSERT (CXL_RCRB_BASE, 0x8); > + > ///@} > > #pragma pack() > -- > 2.29.2.windows.2 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Patch 1/1] MdePkg/IndustryStandard: Fix CXL 1.1 structure layout issues 2020-11-13 1:50 ` Zhiguang Liu @ 2020-11-13 8:36 ` Javeed, Ashraf 0 siblings, 0 replies; 6+ messages in thread From: Javeed, Ashraf @ 2020-11-13 8:36 UTC (permalink / raw) To: Liu, Zhiguang, Kinney, Michael D, devel@edk2.groups.io; +Cc: Liming Gao Bugs fixed! Changes looks good to me. Reviewed-by: Ashraf Javeed <ashraf.javeed@intel.com> > -----Original Message----- > From: Liu, Zhiguang <zhiguang.liu@intel.com> > Sent: Friday, November 13, 2020 7:20 AM > To: Kinney, Michael D <michael.d.kinney@intel.com>; devel@edk2.groups.io > Cc: Liming Gao <gaoliming@byosoft.com.cn>; Javeed, Ashraf > <ashraf.javeed@intel.com> > Subject: RE: [Patch 1/1] MdePkg/IndustryStandard: Fix CXL 1.1 structure > layout issues > > Reviewed-by: Zhiguang Liu <zhiguang.liu@intel.com> > > > -----Original Message----- > > From: Michael D Kinney <michael.d.kinney@intel.com> > > Sent: Friday, November 13, 2020 7:08 AM > > To: devel@edk2.groups.io > > Cc: Liming Gao <gaoliming@byosoft.com.cn>; Liu, Zhiguang > > <zhiguang.liu@intel.com>; Javeed, Ashraf <ashraf.javeed@intel.com> > > Subject: [Patch 1/1] MdePkg/IndustryStandard: Fix CXL 1.1 structure > > layout issues > > > > https://bugzilla.tianocore.org/show_bug.cgi?id=3074 > > > > * Fix offset of LinkLayerControlAndStatus in the > > CXL_1_1_LINK_CAPABILITY_STRUCTURE structure > > * Fix offset of LinkLayerAckTimerControl in the > > CXL_1_1_LINK_CAPABILITY_STRUCTURE structure > > * Fix offset of LinkLayerDefeature in > > the CXL_1_1_LINK_CAPABILITY_STRUCTURE structure > > * Add CXL_11_SIZE_ASSERT() macro to verify the size of > > a register layout structure at compile time and use > > it to verify the sizes of the CXL 1.1 register structures. > > * Add CXL_11_OFFSET_ASSERT() macro to verify the offset of > > fields in a register layout structure at compiler time and > > use it to verify the offset of fields in CXL 1.1 > > register structures. > > > > Cc: Liming Gao <gaoliming@byosoft.com.cn> > > Cc: Zhiguang Liu <zhiguang.liu@intel.com> > > Cc: Ashraf Javeed <ashraf.javeed@intel.com> > > Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com> > > --- > > MdePkg/Include/IndustryStandard/Cxl11.h | 96 > > ++++++++++++++++++++++++- > > 1 file changed, 93 insertions(+), 3 deletions(-) > > > > diff --git a/MdePkg/Include/IndustryStandard/Cxl11.h > > b/MdePkg/Include/IndustryStandard/Cxl11.h > > index 933c1ab817e8..46cb271d3c74 100644 > > --- a/MdePkg/Include/IndustryStandard/Cxl11.h > > +++ b/MdePkg/Include/IndustryStandard/Cxl11.h > > @@ -32,6 +32,40 @@ SPDX-License-Identifier: BSD-2-Clause-Patent // > > #pragma pack(1) > > > > +/** > > + Macro used to verify the size of a data type at compile time and > > +trigger a > > + STATIC_ASSERT() with an error message if the size of the data type > > +does not > > + match the expected size. > > + > > + @param TypeName Type name of data type to verify. > > + @param ExpectedSize The expected size, in bytes, of the data type > specified > > + by TypeName. > > +**/ > > +#define CXL_11_SIZE_ASSERT(TypeName, ExpectedSize) \ > > + STATIC_ASSERT ( \ > > + sizeof (TypeName) == ExpectedSize, \ > > + "Size of " #TypeName \ > > + " does not meet CXL 1.1 Specification requirements." \ > > + ) > > + > > +/** > > + Macro used to verify the offset of a field in a data type at > > +compile time and > > + trigger a STATIC_ASSERT() with an error message if the offset of > > +the field in > > + the data type does not match the expected offset. > > + > > + @param TypeName Type name of data type to verify. > > + @param FieldName Field name in the data type specified by > TypeName > > to > > + verify. > > + @param ExpectedOffset The expected offset, in bytes, of the field > specified > > + by TypeName and FieldName. > > +**/ > > +#define CXL_11_OFFSET_ASSERT(TypeName, FieldName, ExpectedOffset) > \ > > + STATIC_ASSERT ( \ > > + OFFSET_OF (TypeName, FieldName) == ExpectedOffset, \ > > + "Offset of " #TypeName "." #FieldName \ > > + " does not meet CXL 1.1 Specification requirements." \ > > + ) > > + > > /// > > /// The PCIe DVSEC for Flex Bus Device ///@{ @@ -201,6 +235,25 @@ > > typedef struct { > > 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; > > + > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, > > Header , 0x00); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, > > +DesignatedVendorSpecificHeader1, 0x04); CXL_11_OFFSET_ASSERT > > (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, DesignatedVendorSpecificHeader2, > > 0x08); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, > > DeviceCapability , 0x0A); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, > > DeviceControl , 0x0C); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, > > DeviceStatus , 0x0E); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, > > DeviceControl2 , 0x10); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, > > DeviceStatus2 , 0x12); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, > > DeviceLock , 0x14); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, > > DeviceRange1SizeHigh , 0x18); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, > > DeviceRange1SizeLow , 0x1C); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, > > DeviceRange1BaseHigh , 0x20); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, > > DeviceRange1BaseLow , 0x24); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, > > DeviceRange2SizeHigh , 0x28); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, > > DeviceRange2SizeLow , 0x2C); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, > > DeviceRange2BaseHigh , 0x30); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, > > DeviceRange2BaseLow , 0x34); > > +CXL_11_SIZE_ASSERT > > (CXL_1_1_DVSEC_FLEX_BUS_DEVICE , 0x38); > > ///@} > > > > /// > > @@ -265,6 +318,14 @@ typedef struct { > > 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_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_PORT, > > Header , 0x00); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_PORT, > > +DesignatedVendorSpecificHeader1, 0x04); CXL_11_OFFSET_ASSERT > > (CXL_1_1_DVSEC_FLEX_BUS_PORT, DesignatedVendorSpecificHeader2, > 0x08); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_PORT, > > PortCapability , 0x0A); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_PORT, > > PortControl , 0x0C); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_PORT, > > PortStatus , 0x0E); > > +CXL_11_SIZE_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_PORT > , > > 0x10); > > ///@} > > > > /// > > @@ -423,6 +484,15 @@ typedef struct { > > UINT32 HeaderLog[16]; > > } CXL_1_1_RAS_CAPABILITY_STRUCTURE; > > > > +CXL_11_OFFSET_ASSERT (CXL_1_1_RAS_CAPABILITY_STRUCTURE, > > UncorrectableErrorStatus , 0x00); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_RAS_CAPABILITY_STRUCTURE, > > UncorrectableErrorMask , 0x04); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_RAS_CAPABILITY_STRUCTURE, > > UncorrectableErrorSeverity , 0x08); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_RAS_CAPABILITY_STRUCTURE, > > CorrectableErrorStatus , 0x0C); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_RAS_CAPABILITY_STRUCTURE, > > CorrectableErrorMask , 0x10); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_RAS_CAPABILITY_STRUCTURE, > > ErrorCapabilitiesAndControl, 0x14); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_RAS_CAPABILITY_STRUCTURE, > > HeaderLog , 0x18); > > +CXL_11_SIZE_ASSERT > > (CXL_1_1_RAS_CAPABILITY_STRUCTURE , 0x58); > > + > > typedef union { > > struct { > > UINT32 DeviceTrustLevel : 2; // bit 0..1 > > @@ -435,6 +505,9 @@ typedef struct { > > CXL_1_1_SECURITY_POLICY SecurityPolicy; > > } CXL_1_1_SECURITY_CAPABILITY_STRUCTURE; > > > > +CXL_11_OFFSET_ASSERT (CXL_1_1_SECURITY_CAPABILITY_STRUCTURE, > > SecurityPolicy, 0x0); > > +CXL_11_SIZE_ASSERT (CXL_1_1_SECURITY_CAPABILITY_STRUCTURE, > > 0x4); > > + > > typedef union { > > struct { > > UINT64 CxlLinkVersionSupported : 4; // bit 0..3 > > @@ -460,7 +533,7 @@ typedef union { > > UINT16 LlRetryBufferConsumed : 8; // bit 5..12 > > UINT16 Reserved : 3; // bit 13..15 > > } Bits; > > - UINT16 Uint16; > > + UINT64 Uint64; > > } CXL_LINK_LAYER_CONTROL_AND_STATUS; > > > > typedef union { > > @@ -501,7 +574,7 @@ typedef union { > > UINT32 AckForceThreshold : 8; // bit 0..7 > > UINT32 AckFLushRetimer : 10; // bit 8..17 > > } Bits; > > - UINT32 Uint32; > > + UINT64 Uint64; > > } CXL_LINK_LAYER_ACK_TIMER_CONTROL; > > > > typedef union { > > @@ -509,7 +582,7 @@ typedef union { > > UINT32 MdhDisable : 1; // bit 0..0 > > UINT32 Reserved : 31; // bit 1..31 > > } Bits; > > - UINT32 Uint32; > > + UINT64 Uint64; > > } CXL_LINK_LAYER_DEFEATURE; > > > > typedef struct { > > @@ -522,6 +595,15 @@ typedef struct { > > CXL_LINK_LAYER_DEFEATURE LinkLayerDefeature; > > } CXL_1_1_LINK_CAPABILITY_STRUCTURE; > > > > +CXL_11_OFFSET_ASSERT (CXL_1_1_LINK_CAPABILITY_STRUCTURE, > > LinkLayerCapability , 0x00); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_LINK_CAPABILITY_STRUCTURE, > > LinkLayerControlStatus , 0x08); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_LINK_CAPABILITY_STRUCTURE, > > LinkLayerRxCreditControl , 0x10); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_LINK_CAPABILITY_STRUCTURE, > > LinkLayerRxCreditReturnStatus, 0x18); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_LINK_CAPABILITY_STRUCTURE, > > LinkLayerTxCreditStatus , 0x20); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_LINK_CAPABILITY_STRUCTURE, > > LinkLayerAckTimerControl , 0x28); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_LINK_CAPABILITY_STRUCTURE, > > LinkLayerDefeature , 0x30); > > +CXL_11_SIZE_ASSERT > > (CXL_1_1_LINK_CAPABILITY_STRUCTURE , 0x38); > > + > > #define CXL_IO_ARBITRATION_CONTROL_OFFSET 0x180 > > typedef union { > > struct { > > @@ -532,6 +614,8 @@ typedef union { > > UINT32 Uint32; > > } CXL_IO_ARBITRATION_CONTROL; > > > > +CXL_11_SIZE_ASSERT (CXL_IO_ARBITRATION_CONTROL, 0x4); > > + > > #define CXL_CACHE_MEMORY_ARBITRATION_CONTROL_OFFSET > > 0x1C0 > > typedef union { > > struct { > > @@ -541,6 +625,9 @@ typedef union { > > } Bits; > > UINT32 Uint32; > > } CXL_CACHE_MEMORY_ARBITRATION_CONTROL; > > + > > +CXL_11_SIZE_ASSERT (CXL_CACHE_MEMORY_ARBITRATION_CONTROL, > 0x4); > > + > > ///@} > > > > /// The CXL.RCRB base register definition @@ -554,6 +641,9 @@ typedef > > union { > > } Bits; > > UINT64 Uint64; > > } CXL_RCRB_BASE; > > + > > +CXL_11_SIZE_ASSERT (CXL_RCRB_BASE, 0x8); > > + > > ///@} > > > > #pragma pack() > > -- > > 2.29.2.windows.2 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [edk2-devel] [Patch 1/1] MdePkg/IndustryStandard: Fix CXL 1.1 structure layout issues 2020-11-12 23:08 [Patch 1/1] MdePkg/IndustryStandard: Fix CXL 1.1 structure layout issues Michael D Kinney 2020-11-13 1:50 ` Zhiguang Liu @ 2020-11-16 17:41 ` Michael D Kinney 2020-11-16 17:43 ` Michael D Kinney 2020-11-17 0:40 ` 回复: " gaoliming 1 sibling, 2 replies; 6+ messages in thread From: Michael D Kinney @ 2020-11-16 17:41 UTC (permalink / raw) To: devel@edk2.groups.io, Kinney, Michael D, Gao, Liming Cc: Liu, Zhiguang, Javeed, Ashraf Hi Liming, This patch passed review before the soft freeze and has not received any additional feedback. This fix should be merged into edk2-stable202011. I have created a PR with this reviewed content and it passed all CI checks. https://github.com/tianocore/edk2/pull/1124 Best regards, Mike > -----Original Message----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael D Kinney > Sent: Thursday, November 12, 2020 3:08 PM > To: devel@edk2.groups.io > Cc: Liming Gao <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>; Javeed, Ashraf > <ashraf.javeed@intel.com> > Subject: [edk2-devel] [Patch 1/1] MdePkg/IndustryStandard: Fix CXL 1.1 structure layout issues > > https://bugzilla.tianocore.org/show_bug.cgi?id=3074 > > * Fix offset of LinkLayerControlAndStatus in the > CXL_1_1_LINK_CAPABILITY_STRUCTURE structure > * Fix offset of LinkLayerAckTimerControl in the > CXL_1_1_LINK_CAPABILITY_STRUCTURE structure > * Fix offset of LinkLayerDefeature in > the CXL_1_1_LINK_CAPABILITY_STRUCTURE structure > * Add CXL_11_SIZE_ASSERT() macro to verify the size of > a register layout structure at compile time and use > it to verify the sizes of the CXL 1.1 register structures. > * Add CXL_11_OFFSET_ASSERT() macro to verify the offset of > fields in a register layout structure at compiler time and > use it to verify the offset of fields in CXL 1.1 > register structures. > > Cc: Liming Gao <gaoliming@byosoft.com.cn> > Cc: Zhiguang Liu <zhiguang.liu@intel.com> > Cc: Ashraf Javeed <ashraf.javeed@intel.com> > Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com> > --- > MdePkg/Include/IndustryStandard/Cxl11.h | 96 ++++++++++++++++++++++++- > 1 file changed, 93 insertions(+), 3 deletions(-) > > diff --git a/MdePkg/Include/IndustryStandard/Cxl11.h b/MdePkg/Include/IndustryStandard/Cxl11.h > index 933c1ab817e8..46cb271d3c74 100644 > --- a/MdePkg/Include/IndustryStandard/Cxl11.h > +++ b/MdePkg/Include/IndustryStandard/Cxl11.h > @@ -32,6 +32,40 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > // > #pragma pack(1) > > +/** > + Macro used to verify the size of a data type at compile time and trigger a > + STATIC_ASSERT() with an error message if the size of the data type does not > + match the expected size. > + > + @param TypeName Type name of data type to verify. > + @param ExpectedSize The expected size, in bytes, of the data type specified > + by TypeName. > +**/ > +#define CXL_11_SIZE_ASSERT(TypeName, ExpectedSize) \ > + STATIC_ASSERT ( \ > + sizeof (TypeName) == ExpectedSize, \ > + "Size of " #TypeName \ > + " does not meet CXL 1.1 Specification requirements." \ > + ) > + > +/** > + Macro used to verify the offset of a field in a data type at compile time and > + trigger a STATIC_ASSERT() with an error message if the offset of the field in > + the data type does not match the expected offset. > + > + @param TypeName Type name of data type to verify. > + @param FieldName Field name in the data type specified by TypeName to > + verify. > + @param ExpectedOffset The expected offset, in bytes, of the field specified > + by TypeName and FieldName. > +**/ > +#define CXL_11_OFFSET_ASSERT(TypeName, FieldName, ExpectedOffset) \ > + STATIC_ASSERT ( \ > + OFFSET_OF (TypeName, FieldName) == ExpectedOffset, \ > + "Offset of " #TypeName "." #FieldName \ > + " does not meet CXL 1.1 Specification requirements." \ > + ) > + > /// > /// The PCIe DVSEC for Flex Bus Device > ///@{ > @@ -201,6 +235,25 @@ typedef struct { > 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; > + > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, Header , 0x00); > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, DesignatedVendorSpecificHeader1, 0x04); > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, DesignatedVendorSpecificHeader2, 0x08); > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, DeviceCapability , 0x0A); > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, DeviceControl , 0x0C); > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, DeviceStatus , 0x0E); > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, DeviceControl2 , 0x10); > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, DeviceStatus2 , 0x12); > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, DeviceLock , 0x14); > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, DeviceRange1SizeHigh , 0x18); > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, DeviceRange1SizeLow , 0x1C); > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, DeviceRange1BaseHigh , 0x20); > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, DeviceRange1BaseLow , 0x24); > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, DeviceRange2SizeHigh , 0x28); > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, DeviceRange2SizeLow , 0x2C); > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, DeviceRange2BaseHigh , 0x30); > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, DeviceRange2BaseLow , 0x34); > +CXL_11_SIZE_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE , 0x38); > ///@} > > /// > @@ -265,6 +318,14 @@ typedef struct { > 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_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_PORT, Header , 0x00); > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_PORT, DesignatedVendorSpecificHeader1, 0x04); > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_PORT, DesignatedVendorSpecificHeader2, 0x08); > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_PORT, PortCapability , 0x0A); > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_PORT, PortControl , 0x0C); > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_PORT, PortStatus , 0x0E); > +CXL_11_SIZE_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_PORT , 0x10); > ///@} > > /// > @@ -423,6 +484,15 @@ typedef struct { > UINT32 HeaderLog[16]; > } CXL_1_1_RAS_CAPABILITY_STRUCTURE; > > +CXL_11_OFFSET_ASSERT (CXL_1_1_RAS_CAPABILITY_STRUCTURE, UncorrectableErrorStatus , 0x00); > +CXL_11_OFFSET_ASSERT (CXL_1_1_RAS_CAPABILITY_STRUCTURE, UncorrectableErrorMask , 0x04); > +CXL_11_OFFSET_ASSERT (CXL_1_1_RAS_CAPABILITY_STRUCTURE, UncorrectableErrorSeverity , 0x08); > +CXL_11_OFFSET_ASSERT (CXL_1_1_RAS_CAPABILITY_STRUCTURE, CorrectableErrorStatus , 0x0C); > +CXL_11_OFFSET_ASSERT (CXL_1_1_RAS_CAPABILITY_STRUCTURE, CorrectableErrorMask , 0x10); > +CXL_11_OFFSET_ASSERT (CXL_1_1_RAS_CAPABILITY_STRUCTURE, ErrorCapabilitiesAndControl, 0x14); > +CXL_11_OFFSET_ASSERT (CXL_1_1_RAS_CAPABILITY_STRUCTURE, HeaderLog , 0x18); > +CXL_11_SIZE_ASSERT (CXL_1_1_RAS_CAPABILITY_STRUCTURE , 0x58); > + > typedef union { > struct { > UINT32 DeviceTrustLevel : 2; // bit 0..1 > @@ -435,6 +505,9 @@ typedef struct { > CXL_1_1_SECURITY_POLICY SecurityPolicy; > } CXL_1_1_SECURITY_CAPABILITY_STRUCTURE; > > +CXL_11_OFFSET_ASSERT (CXL_1_1_SECURITY_CAPABILITY_STRUCTURE, SecurityPolicy, 0x0); > +CXL_11_SIZE_ASSERT (CXL_1_1_SECURITY_CAPABILITY_STRUCTURE, 0x4); > + > typedef union { > struct { > UINT64 CxlLinkVersionSupported : 4; // bit 0..3 > @@ -460,7 +533,7 @@ typedef union { > UINT16 LlRetryBufferConsumed : 8; // bit 5..12 > UINT16 Reserved : 3; // bit 13..15 > } Bits; > - UINT16 Uint16; > + UINT64 Uint64; > } CXL_LINK_LAYER_CONTROL_AND_STATUS; > > typedef union { > @@ -501,7 +574,7 @@ typedef union { > UINT32 AckForceThreshold : 8; // bit 0..7 > UINT32 AckFLushRetimer : 10; // bit 8..17 > } Bits; > - UINT32 Uint32; > + UINT64 Uint64; > } CXL_LINK_LAYER_ACK_TIMER_CONTROL; > > typedef union { > @@ -509,7 +582,7 @@ typedef union { > UINT32 MdhDisable : 1; // bit 0..0 > UINT32 Reserved : 31; // bit 1..31 > } Bits; > - UINT32 Uint32; > + UINT64 Uint64; > } CXL_LINK_LAYER_DEFEATURE; > > typedef struct { > @@ -522,6 +595,15 @@ typedef struct { > CXL_LINK_LAYER_DEFEATURE LinkLayerDefeature; > } CXL_1_1_LINK_CAPABILITY_STRUCTURE; > > +CXL_11_OFFSET_ASSERT (CXL_1_1_LINK_CAPABILITY_STRUCTURE, LinkLayerCapability , 0x00); > +CXL_11_OFFSET_ASSERT (CXL_1_1_LINK_CAPABILITY_STRUCTURE, LinkLayerControlStatus , 0x08); > +CXL_11_OFFSET_ASSERT (CXL_1_1_LINK_CAPABILITY_STRUCTURE, LinkLayerRxCreditControl , 0x10); > +CXL_11_OFFSET_ASSERT (CXL_1_1_LINK_CAPABILITY_STRUCTURE, LinkLayerRxCreditReturnStatus, 0x18); > +CXL_11_OFFSET_ASSERT (CXL_1_1_LINK_CAPABILITY_STRUCTURE, LinkLayerTxCreditStatus , 0x20); > +CXL_11_OFFSET_ASSERT (CXL_1_1_LINK_CAPABILITY_STRUCTURE, LinkLayerAckTimerControl , 0x28); > +CXL_11_OFFSET_ASSERT (CXL_1_1_LINK_CAPABILITY_STRUCTURE, LinkLayerDefeature , 0x30); > +CXL_11_SIZE_ASSERT (CXL_1_1_LINK_CAPABILITY_STRUCTURE , 0x38); > + > #define CXL_IO_ARBITRATION_CONTROL_OFFSET 0x180 > typedef union { > struct { > @@ -532,6 +614,8 @@ typedef union { > UINT32 Uint32; > } CXL_IO_ARBITRATION_CONTROL; > > +CXL_11_SIZE_ASSERT (CXL_IO_ARBITRATION_CONTROL, 0x4); > + > #define CXL_CACHE_MEMORY_ARBITRATION_CONTROL_OFFSET 0x1C0 > typedef union { > struct { > @@ -541,6 +625,9 @@ typedef union { > } Bits; > UINT32 Uint32; > } CXL_CACHE_MEMORY_ARBITRATION_CONTROL; > + > +CXL_11_SIZE_ASSERT (CXL_CACHE_MEMORY_ARBITRATION_CONTROL, 0x4); > + > ///@} > > /// The CXL.RCRB base register definition > @@ -554,6 +641,9 @@ typedef union { > } Bits; > UINT64 Uint64; > } CXL_RCRB_BASE; > + > +CXL_11_SIZE_ASSERT (CXL_RCRB_BASE, 0x8); > + > ///@} > > #pragma pack() > -- > 2.29.2.windows.2 > > > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [edk2-devel] [Patch 1/1] MdePkg/IndustryStandard: Fix CXL 1.1 structure layout issues 2020-11-16 17:41 ` [edk2-devel] " Michael D Kinney @ 2020-11-16 17:43 ` Michael D Kinney 2020-11-17 0:40 ` 回复: " gaoliming 1 sibling, 0 replies; 6+ messages in thread From: Michael D Kinney @ 2020-11-16 17:43 UTC (permalink / raw) To: devel@edk2.groups.io, gaoliming, Kinney, Michael D Cc: Liu, Zhiguang, Javeed, Ashraf Correct email address. Mike > -----Original Message----- > From: Kinney, Michael D <michael.d.kinney@intel.com> > Sent: Monday, November 16, 2020 9:42 AM > To: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com> > Cc: Liu, Zhiguang <zhiguang.liu@intel.com>; Javeed, Ashraf <ashraf.javeed@intel.com> > Subject: RE: [edk2-devel] [Patch 1/1] MdePkg/IndustryStandard: Fix CXL 1.1 structure layout issues > > Hi Liming, > > This patch passed review before the soft freeze and has not received any additional feedback. > > This fix should be merged into edk2-stable202011. > > I have created a PR with this reviewed content and it passed all CI checks. > > https://github.com/tianocore/edk2/pull/1124 > > Best regards, > > Mike > > > > > -----Original Message----- > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael D Kinney > > Sent: Thursday, November 12, 2020 3:08 PM > > To: devel@edk2.groups.io > > Cc: Liming Gao <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>; Javeed, Ashraf > > <ashraf.javeed@intel.com> > > Subject: [edk2-devel] [Patch 1/1] MdePkg/IndustryStandard: Fix CXL 1.1 structure layout issues > > > > https://bugzilla.tianocore.org/show_bug.cgi?id=3074 > > > > * Fix offset of LinkLayerControlAndStatus in the > > CXL_1_1_LINK_CAPABILITY_STRUCTURE structure > > * Fix offset of LinkLayerAckTimerControl in the > > CXL_1_1_LINK_CAPABILITY_STRUCTURE structure > > * Fix offset of LinkLayerDefeature in > > the CXL_1_1_LINK_CAPABILITY_STRUCTURE structure > > * Add CXL_11_SIZE_ASSERT() macro to verify the size of > > a register layout structure at compile time and use > > it to verify the sizes of the CXL 1.1 register structures. > > * Add CXL_11_OFFSET_ASSERT() macro to verify the offset of > > fields in a register layout structure at compiler time and > > use it to verify the offset of fields in CXL 1.1 > > register structures. > > > > Cc: Liming Gao <gaoliming@byosoft.com.cn> > > Cc: Zhiguang Liu <zhiguang.liu@intel.com> > > Cc: Ashraf Javeed <ashraf.javeed@intel.com> > > Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com> > > --- > > MdePkg/Include/IndustryStandard/Cxl11.h | 96 ++++++++++++++++++++++++- > > 1 file changed, 93 insertions(+), 3 deletions(-) > > > > diff --git a/MdePkg/Include/IndustryStandard/Cxl11.h b/MdePkg/Include/IndustryStandard/Cxl11.h > > index 933c1ab817e8..46cb271d3c74 100644 > > --- a/MdePkg/Include/IndustryStandard/Cxl11.h > > +++ b/MdePkg/Include/IndustryStandard/Cxl11.h > > @@ -32,6 +32,40 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > > // > > #pragma pack(1) > > > > +/** > > + Macro used to verify the size of a data type at compile time and trigger a > > + STATIC_ASSERT() with an error message if the size of the data type does not > > + match the expected size. > > + > > + @param TypeName Type name of data type to verify. > > + @param ExpectedSize The expected size, in bytes, of the data type specified > > + by TypeName. > > +**/ > > +#define CXL_11_SIZE_ASSERT(TypeName, ExpectedSize) \ > > + STATIC_ASSERT ( \ > > + sizeof (TypeName) == ExpectedSize, \ > > + "Size of " #TypeName \ > > + " does not meet CXL 1.1 Specification requirements." \ > > + ) > > + > > +/** > > + Macro used to verify the offset of a field in a data type at compile time and > > + trigger a STATIC_ASSERT() with an error message if the offset of the field in > > + the data type does not match the expected offset. > > + > > + @param TypeName Type name of data type to verify. > > + @param FieldName Field name in the data type specified by TypeName to > > + verify. > > + @param ExpectedOffset The expected offset, in bytes, of the field specified > > + by TypeName and FieldName. > > +**/ > > +#define CXL_11_OFFSET_ASSERT(TypeName, FieldName, ExpectedOffset) \ > > + STATIC_ASSERT ( \ > > + OFFSET_OF (TypeName, FieldName) == ExpectedOffset, \ > > + "Offset of " #TypeName "." #FieldName \ > > + " does not meet CXL 1.1 Specification requirements." \ > > + ) > > + > > /// > > /// The PCIe DVSEC for Flex Bus Device > > ///@{ > > @@ -201,6 +235,25 @@ typedef struct { > > 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; > > + > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, Header , 0x00); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, DesignatedVendorSpecificHeader1, 0x04); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, DesignatedVendorSpecificHeader2, 0x08); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, DeviceCapability , 0x0A); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, DeviceControl , 0x0C); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, DeviceStatus , 0x0E); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, DeviceControl2 , 0x10); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, DeviceStatus2 , 0x12); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, DeviceLock , 0x14); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, DeviceRange1SizeHigh , 0x18); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, DeviceRange1SizeLow , 0x1C); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, DeviceRange1BaseHigh , 0x20); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, DeviceRange1BaseLow , 0x24); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, DeviceRange2SizeHigh , 0x28); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, DeviceRange2SizeLow , 0x2C); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, DeviceRange2BaseHigh , 0x30); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, DeviceRange2BaseLow , 0x34); > > +CXL_11_SIZE_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE , 0x38); > > ///@} > > > > /// > > @@ -265,6 +318,14 @@ typedef struct { > > 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_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_PORT, Header , 0x00); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_PORT, DesignatedVendorSpecificHeader1, 0x04); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_PORT, DesignatedVendorSpecificHeader2, 0x08); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_PORT, PortCapability , 0x0A); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_PORT, PortControl , 0x0C); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_PORT, PortStatus , 0x0E); > > +CXL_11_SIZE_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_PORT , 0x10); > > ///@} > > > > /// > > @@ -423,6 +484,15 @@ typedef struct { > > UINT32 HeaderLog[16]; > > } CXL_1_1_RAS_CAPABILITY_STRUCTURE; > > > > +CXL_11_OFFSET_ASSERT (CXL_1_1_RAS_CAPABILITY_STRUCTURE, UncorrectableErrorStatus , 0x00); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_RAS_CAPABILITY_STRUCTURE, UncorrectableErrorMask , 0x04); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_RAS_CAPABILITY_STRUCTURE, UncorrectableErrorSeverity , 0x08); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_RAS_CAPABILITY_STRUCTURE, CorrectableErrorStatus , 0x0C); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_RAS_CAPABILITY_STRUCTURE, CorrectableErrorMask , 0x10); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_RAS_CAPABILITY_STRUCTURE, ErrorCapabilitiesAndControl, 0x14); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_RAS_CAPABILITY_STRUCTURE, HeaderLog , 0x18); > > +CXL_11_SIZE_ASSERT (CXL_1_1_RAS_CAPABILITY_STRUCTURE , 0x58); > > + > > typedef union { > > struct { > > UINT32 DeviceTrustLevel : 2; // bit 0..1 > > @@ -435,6 +505,9 @@ typedef struct { > > CXL_1_1_SECURITY_POLICY SecurityPolicy; > > } CXL_1_1_SECURITY_CAPABILITY_STRUCTURE; > > > > +CXL_11_OFFSET_ASSERT (CXL_1_1_SECURITY_CAPABILITY_STRUCTURE, SecurityPolicy, 0x0); > > +CXL_11_SIZE_ASSERT (CXL_1_1_SECURITY_CAPABILITY_STRUCTURE, 0x4); > > + > > typedef union { > > struct { > > UINT64 CxlLinkVersionSupported : 4; // bit 0..3 > > @@ -460,7 +533,7 @@ typedef union { > > UINT16 LlRetryBufferConsumed : 8; // bit 5..12 > > UINT16 Reserved : 3; // bit 13..15 > > } Bits; > > - UINT16 Uint16; > > + UINT64 Uint64; > > } CXL_LINK_LAYER_CONTROL_AND_STATUS; > > > > typedef union { > > @@ -501,7 +574,7 @@ typedef union { > > UINT32 AckForceThreshold : 8; // bit 0..7 > > UINT32 AckFLushRetimer : 10; // bit 8..17 > > } Bits; > > - UINT32 Uint32; > > + UINT64 Uint64; > > } CXL_LINK_LAYER_ACK_TIMER_CONTROL; > > > > typedef union { > > @@ -509,7 +582,7 @@ typedef union { > > UINT32 MdhDisable : 1; // bit 0..0 > > UINT32 Reserved : 31; // bit 1..31 > > } Bits; > > - UINT32 Uint32; > > + UINT64 Uint64; > > } CXL_LINK_LAYER_DEFEATURE; > > > > typedef struct { > > @@ -522,6 +595,15 @@ typedef struct { > > CXL_LINK_LAYER_DEFEATURE LinkLayerDefeature; > > } CXL_1_1_LINK_CAPABILITY_STRUCTURE; > > > > +CXL_11_OFFSET_ASSERT (CXL_1_1_LINK_CAPABILITY_STRUCTURE, LinkLayerCapability , 0x00); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_LINK_CAPABILITY_STRUCTURE, LinkLayerControlStatus , 0x08); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_LINK_CAPABILITY_STRUCTURE, LinkLayerRxCreditControl , 0x10); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_LINK_CAPABILITY_STRUCTURE, LinkLayerRxCreditReturnStatus, 0x18); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_LINK_CAPABILITY_STRUCTURE, LinkLayerTxCreditStatus , 0x20); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_LINK_CAPABILITY_STRUCTURE, LinkLayerAckTimerControl , 0x28); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_LINK_CAPABILITY_STRUCTURE, LinkLayerDefeature , 0x30); > > +CXL_11_SIZE_ASSERT (CXL_1_1_LINK_CAPABILITY_STRUCTURE , 0x38); > > + > > #define CXL_IO_ARBITRATION_CONTROL_OFFSET 0x180 > > typedef union { > > struct { > > @@ -532,6 +614,8 @@ typedef union { > > UINT32 Uint32; > > } CXL_IO_ARBITRATION_CONTROL; > > > > +CXL_11_SIZE_ASSERT (CXL_IO_ARBITRATION_CONTROL, 0x4); > > + > > #define CXL_CACHE_MEMORY_ARBITRATION_CONTROL_OFFSET 0x1C0 > > typedef union { > > struct { > > @@ -541,6 +625,9 @@ typedef union { > > } Bits; > > UINT32 Uint32; > > } CXL_CACHE_MEMORY_ARBITRATION_CONTROL; > > + > > +CXL_11_SIZE_ASSERT (CXL_CACHE_MEMORY_ARBITRATION_CONTROL, 0x4); > > + > > ///@} > > > > /// The CXL.RCRB base register definition > > @@ -554,6 +641,9 @@ typedef union { > > } Bits; > > UINT64 Uint64; > > } CXL_RCRB_BASE; > > + > > +CXL_11_SIZE_ASSERT (CXL_RCRB_BASE, 0x8); > > + > > ///@} > > > > #pragma pack() > > -- > > 2.29.2.windows.2 > > > > > > > > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* 回复: [edk2-devel] [Patch 1/1] MdePkg/IndustryStandard: Fix CXL 1.1 structure layout issues 2020-11-16 17:41 ` [edk2-devel] " Michael D Kinney 2020-11-16 17:43 ` Michael D Kinney @ 2020-11-17 0:40 ` gaoliming 1 sibling, 0 replies; 6+ messages in thread From: gaoliming @ 2020-11-17 0:40 UTC (permalink / raw) To: devel, michael.d.kinney, 'Gao, Liming' Cc: 'Liu, Zhiguang', 'Javeed, Ashraf' Mike: This is a bug fix. It is OK to merge it for this stable tag. Thanks Liming > -----邮件原件----- > 发件人: bounce+27952+67611+4905953+8761045@groups.io > <bounce+27952+67611+4905953+8761045@groups.io> 代表 Michael D > Kinney > 发送时间: 2020年11月17日 1:42 > 收件人: devel@edk2.groups.io; Kinney, Michael D > <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com> > 抄送: Liu, Zhiguang <zhiguang.liu@intel.com>; Javeed, Ashraf > <ashraf.javeed@intel.com> > 主题: Re: [edk2-devel] [Patch 1/1] MdePkg/IndustryStandard: Fix CXL 1.1 > structure layout issues > > Hi Liming, > > This patch passed review before the soft freeze and has not received any > additional feedback. > > This fix should be merged into edk2-stable202011. > > I have created a PR with this reviewed content and it passed all CI checks. > > https://github.com/tianocore/edk2/pull/1124 > > Best regards, > > Mike > > > > > -----Original Message----- > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael > D Kinney > > Sent: Thursday, November 12, 2020 3:08 PM > > To: devel@edk2.groups.io > > Cc: Liming Gao <gaoliming@byosoft.com.cn>; Liu, Zhiguang > <zhiguang.liu@intel.com>; Javeed, Ashraf > > <ashraf.javeed@intel.com> > > Subject: [edk2-devel] [Patch 1/1] MdePkg/IndustryStandard: Fix CXL 1.1 > structure layout issues > > > > https://bugzilla.tianocore.org/show_bug.cgi?id=3074 > > > > * Fix offset of LinkLayerControlAndStatus in the > > CXL_1_1_LINK_CAPABILITY_STRUCTURE structure > > * Fix offset of LinkLayerAckTimerControl in the > > CXL_1_1_LINK_CAPABILITY_STRUCTURE structure > > * Fix offset of LinkLayerDefeature in > > the CXL_1_1_LINK_CAPABILITY_STRUCTURE structure > > * Add CXL_11_SIZE_ASSERT() macro to verify the size of > > a register layout structure at compile time and use > > it to verify the sizes of the CXL 1.1 register structures. > > * Add CXL_11_OFFSET_ASSERT() macro to verify the offset of > > fields in a register layout structure at compiler time and > > use it to verify the offset of fields in CXL 1.1 > > register structures. > > > > Cc: Liming Gao <gaoliming@byosoft.com.cn> > > Cc: Zhiguang Liu <zhiguang.liu@intel.com> > > Cc: Ashraf Javeed <ashraf.javeed@intel.com> > > Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com> > > --- > > MdePkg/Include/IndustryStandard/Cxl11.h | 96 > ++++++++++++++++++++++++- > > 1 file changed, 93 insertions(+), 3 deletions(-) > > > > diff --git a/MdePkg/Include/IndustryStandard/Cxl11.h > b/MdePkg/Include/IndustryStandard/Cxl11.h > > index 933c1ab817e8..46cb271d3c74 100644 > > --- a/MdePkg/Include/IndustryStandard/Cxl11.h > > +++ b/MdePkg/Include/IndustryStandard/Cxl11.h > > @@ -32,6 +32,40 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > > // > > #pragma pack(1) > > > > +/** > > + Macro used to verify the size of a data type at compile time and trigger > a > > + STATIC_ASSERT() with an error message if the size of the data type does > not > > + match the expected size. > > + > > + @param TypeName Type name of data type to verify. > > + @param ExpectedSize The expected size, in bytes, of the data type > specified > > + by TypeName. > > +**/ > > +#define CXL_11_SIZE_ASSERT(TypeName, ExpectedSize) \ > > + STATIC_ASSERT ( \ > > + sizeof (TypeName) == ExpectedSize, \ > > + "Size of " #TypeName \ > > + " does not meet CXL 1.1 Specification requirements." \ > > + ) > > + > > +/** > > + Macro used to verify the offset of a field in a data type at compile time > and > > + trigger a STATIC_ASSERT() with an error message if the offset of the field > in > > + the data type does not match the expected offset. > > + > > + @param TypeName Type name of data type to verify. > > + @param FieldName Field name in the data type specified by > TypeName to > > + verify. > > + @param ExpectedOffset The expected offset, in bytes, of the field > specified > > + by TypeName and FieldName. > > +**/ > > +#define CXL_11_OFFSET_ASSERT(TypeName, FieldName, ExpectedOffset) > \ > > + STATIC_ASSERT > ( \ > > + OFFSET_OF (TypeName, FieldName) == ExpectedOffset, > \ > > + "Offset of " #TypeName "." #FieldName > \ > > + " does not meet CXL 1.1 Specification requirements." \ > > + ) > > + > > /// > > /// The PCIe DVSEC for Flex Bus Device > > ///@{ > > @@ -201,6 +235,25 @@ typedef struct { > > 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; > > + > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, > Header , 0x00); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, > DesignatedVendorSpecificHeader1, 0x04); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, > DesignatedVendorSpecificHeader2, 0x08); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, > DeviceCapability , 0x0A); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, > DeviceControl , 0x0C); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, > DeviceStatus , 0x0E); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, > DeviceControl2 , 0x10); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, > DeviceStatus2 , 0x12); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, > DeviceLock , 0x14); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, > DeviceRange1SizeHigh , 0x18); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, > DeviceRange1SizeLow , 0x1C); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, > DeviceRange1BaseHigh , 0x20); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, > DeviceRange1BaseLow , 0x24); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, > DeviceRange2SizeHigh , 0x28); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, > DeviceRange2SizeLow , 0x2C); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, > DeviceRange2BaseHigh , 0x30); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_DEVICE, > DeviceRange2BaseLow , 0x34); > > +CXL_11_SIZE_ASSERT > (CXL_1_1_DVSEC_FLEX_BUS_DEVICE > , 0x38); > > ///@} > > > > /// > > @@ -265,6 +318,14 @@ typedef struct { > > 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_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_PORT, > Header , 0x00); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_PORT, > DesignatedVendorSpecificHeader1, 0x04); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_PORT, > DesignatedVendorSpecificHeader2, 0x08); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_PORT, > PortCapability , 0x0A); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_PORT, > PortControl , 0x0C); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_DVSEC_FLEX_BUS_PORT, > PortStatus , 0x0E); > > +CXL_11_SIZE_ASSERT > (CXL_1_1_DVSEC_FLEX_BUS_PORT , > 0x10); > > ///@} > > > > /// > > @@ -423,6 +484,15 @@ typedef struct { > > UINT32 > HeaderLog[16]; > > } CXL_1_1_RAS_CAPABILITY_STRUCTURE; > > > > +CXL_11_OFFSET_ASSERT (CXL_1_1_RAS_CAPABILITY_STRUCTURE, > UncorrectableErrorStatus , 0x00); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_RAS_CAPABILITY_STRUCTURE, > UncorrectableErrorMask , 0x04); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_RAS_CAPABILITY_STRUCTURE, > UncorrectableErrorSeverity , 0x08); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_RAS_CAPABILITY_STRUCTURE, > CorrectableErrorStatus , 0x0C); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_RAS_CAPABILITY_STRUCTURE, > CorrectableErrorMask , 0x10); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_RAS_CAPABILITY_STRUCTURE, > ErrorCapabilitiesAndControl, 0x14); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_RAS_CAPABILITY_STRUCTURE, > HeaderLog , 0x18); > > +CXL_11_SIZE_ASSERT > (CXL_1_1_RAS_CAPABILITY_STRUCTURE , > 0x58); > > + > > typedef union { > > struct { > > UINT32 > DeviceTrustLevel : 2; // bit > 0..1 > > @@ -435,6 +505,9 @@ typedef struct { > > CXL_1_1_SECURITY_POLICY > SecurityPolicy; > > } CXL_1_1_SECURITY_CAPABILITY_STRUCTURE; > > > > +CXL_11_OFFSET_ASSERT (CXL_1_1_SECURITY_CAPABILITY_STRUCTURE, > SecurityPolicy, 0x0); > > +CXL_11_SIZE_ASSERT (CXL_1_1_SECURITY_CAPABILITY_STRUCTURE, > 0x4); > > + > > typedef union { > > struct { > > UINT64 > CxlLinkVersionSupported : 4; // bit 0..3 > > @@ -460,7 +533,7 @@ typedef union { > > UINT16 > LlRetryBufferConsumed : 8; // bit > 5..12 > > UINT16 > Reserved : 3; // bit > 13..15 > > } Bits; > > - UINT16 > Uint16; > > + UINT64 > Uint64; > > } CXL_LINK_LAYER_CONTROL_AND_STATUS; > > > > typedef union { > > @@ -501,7 +574,7 @@ typedef union { > > UINT32 > AckForceThreshold : 8; // bit > 0..7 > > UINT32 > AckFLushRetimer : 10; // bit > 8..17 > > } Bits; > > - UINT32 > Uint32; > > + UINT64 > Uint64; > > } CXL_LINK_LAYER_ACK_TIMER_CONTROL; > > > > typedef union { > > @@ -509,7 +582,7 @@ typedef union { > > UINT32 > MdhDisable : 1; // bit > 0..0 > > UINT32 > Reserved : 31; // bit > 1..31 > > } Bits; > > - UINT32 > Uint32; > > + UINT64 > Uint64; > > } CXL_LINK_LAYER_DEFEATURE; > > > > typedef struct { > > @@ -522,6 +595,15 @@ typedef struct { > > CXL_LINK_LAYER_DEFEATURE > LinkLayerDefeature; > > } CXL_1_1_LINK_CAPABILITY_STRUCTURE; > > > > +CXL_11_OFFSET_ASSERT (CXL_1_1_LINK_CAPABILITY_STRUCTURE, > LinkLayerCapability , 0x00); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_LINK_CAPABILITY_STRUCTURE, > LinkLayerControlStatus , 0x08); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_LINK_CAPABILITY_STRUCTURE, > LinkLayerRxCreditControl , 0x10); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_LINK_CAPABILITY_STRUCTURE, > LinkLayerRxCreditReturnStatus, 0x18); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_LINK_CAPABILITY_STRUCTURE, > LinkLayerTxCreditStatus , 0x20); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_LINK_CAPABILITY_STRUCTURE, > LinkLayerAckTimerControl , 0x28); > > +CXL_11_OFFSET_ASSERT (CXL_1_1_LINK_CAPABILITY_STRUCTURE, > LinkLayerDefeature , 0x30); > > +CXL_11_SIZE_ASSERT > (CXL_1_1_LINK_CAPABILITY_STRUCTURE > , 0x38); > > + > > #define CXL_IO_ARBITRATION_CONTROL_OFFSET > 0x180 > > typedef union { > > struct { > > @@ -532,6 +614,8 @@ typedef union { > > UINT32 > Uint32; > > } CXL_IO_ARBITRATION_CONTROL; > > > > +CXL_11_SIZE_ASSERT (CXL_IO_ARBITRATION_CONTROL, 0x4); > > + > > #define CXL_CACHE_MEMORY_ARBITRATION_CONTROL_OFFSET > 0x1C0 > > typedef union { > > struct { > > @@ -541,6 +625,9 @@ typedef union { > > } Bits; > > UINT32 > Uint32; > > } CXL_CACHE_MEMORY_ARBITRATION_CONTROL; > > + > > +CXL_11_SIZE_ASSERT (CXL_CACHE_MEMORY_ARBITRATION_CONTROL, > 0x4); > > + > > ///@} > > > > /// The CXL.RCRB base register definition > > @@ -554,6 +641,9 @@ typedef union { > > } Bits; > > UINT64 > Uint64; > > } CXL_RCRB_BASE; > > + > > +CXL_11_SIZE_ASSERT (CXL_RCRB_BASE, 0x8); > > + > > ///@} > > > > #pragma pack() > > -- > > 2.29.2.windows.2 > > > > > > > > > > > > > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-11-17 0:40 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-11-12 23:08 [Patch 1/1] MdePkg/IndustryStandard: Fix CXL 1.1 structure layout issues Michael D Kinney 2020-11-13 1:50 ` Zhiguang Liu 2020-11-13 8:36 ` Javeed, Ashraf 2020-11-16 17:41 ` [edk2-devel] " Michael D Kinney 2020-11-16 17:43 ` Michael D Kinney 2020-11-17 0:40 ` 回复: " gaoliming
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox