Re: [edk2-devel] [RFC EDK2 PATCH v4 1/1] MdePkg/IndustryStandard: add definitions for CXL CEDT

["Jonathan Cameron via groups.io" <jonathan.cameron=huawei.com@groups.io>]

On Tue, 26 Nov 2024 11:01:59 +0800
Yuquan Wang <wangyuquan1236@phytium.com.cn> wrote:

> This adds #defines and struct typedefs for the various structure
> types in the CXL Early Discovery Table (CEDT).
> 
> Signed-off-by: Yuquan Wang <wangyuquan1236@phytium.com.cn>
Hi.

One errata applies. See below.

Otherwise,  I've checked these against the r3.2 specification and
they look correct.  Some suggestions on places where comments might
make that easier to establish when it comes to maximum sizing.
Otherwise, with the errata fixed.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>


> diff --git a/MdePkg/Include/IndustryStandard/Cxl30.h b/MdePkg/Include/IndustryStandard/Cxl30.h
> index 7a9a6d69405d..63e56aea8254 100644
> --- a/MdePkg/Include/IndustryStandard/Cxl30.h
> +++ b/MdePkg/Include/IndustryStandard/Cxl30.h
> @@ -45,6 +45,13 @@
>  #define CXL_HDM_6_WAY_INTERLEAVING   0x9
>  #define CXL_HDM_12_WAY_INTERLEAVING  0xA
>  
> +///
> +/// "CEDT" CXL Early Discovery Table
> +///
> +#define CEDT_TYPE_CFMWS    0x1
> +#define CEDT_TYPE_CXIMS    0x2
> +#define CEDT_TYPE_RDPAS    0x3
> +
>  //
>  // Ensure proper structure formats
>  //
> @@ -311,6 +318,45 @@ typedef struct {
>    CXL_3_0_CXL_TIMEOUT_AND_ISOLATION_STATUS        TimeoutAndIsolationStatus;
>  } CXL_3_0_CXL_TIMEOUT_AND_ISOLATION_CAPABILITY_STRUCTURE;
>  
> +///
> +/// Definition for CXL Fixed Memory Window Structure (CFMWS)
> +///
> +typedef struct {
> +  CEDT_STRUCTURE                       Header;
> +  UINT32                               Reserved;
> +  UINT64                               BaseHpa;
> +  UINT64                               WindowSize;
> +  UINT8                                InterleaveMembers;
Spec naming now at least is Encode Number of Interleave Ways.
This name looses quite a bit of that.  I'd use a longer name,
perhaps EncodedInterleaveWays


> +  UINT8                                InterleaveArithmetic;
> +  UINT16                               Reserved1;
> +  UINT32                               Granularity;
> +  UINT16                               Restrictions;
> +  UINT16                               QtgId;
> +  UINT32                               TargetList[16];

Perhaps a comment on why 16 given it depends on decoded version
of EncodedInterleaveWays.  16 is the current maximum.

> +} CXL_FIXED_MEMORY_WINDOW_STRUCTURE;
> +
> +///
> +/// Definition for CXL XOR Interleave Math Structure (CXIMS)
> +///
> +typedef struct {
> +  CEDT_STRUCTURE                       Header;
> +  UINT16                               Reserved;
> +  UINT8                                HBIG;
> +  UINT8                                NIB;
> +  UINT64                               XORMAPLIST[4];

Why 4?  Number of entries depends on NIB. Add a comment if this
is the effective maximum to refer to specific part of spec that says that.


> +} CXL_XOR_INTERLEAVE_MATH_STRUCTURE;
> +
> +///
> +/// Definition for RCEC Downstream Port Association Structure (RDPAS)
> +///
> +typedef struct {
> +  CEDT_STRUCTURE                       Header;
> +  UINT16                               SegmentNumber;
> +  UINT16                               Bdf;
> +  UINT8                                ProtocolType;
> +  UINT64                               BaseAddress;

Always check the latest spec, which as of last week is 3.2 or
any errata released in the meantime.

There is an errata on this.  Key is that in the 3.1 and before
the fields don't fit.  Offset of RCEC BDF is 06h, length 2 bytes, yet
protocol type is in 07h so overlaps.

Fix was to move protocol type later.

> +} RCEC_DOWNSTREAM_PORT_ASSOCIATION_STRUCTURE;
> +
>  #pragma pack()
>  
>  #endif



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#120845): https://edk2.groups.io/g/devel/message/120845
Mute This Topic: https://groups.io/mt/109784487/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-