Change in coreboot[master]: soc/intel/xeon_sp/cpx: Add DIMM definition in SystemMemoryMapHob
Johnny Lin has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/44847 )
Change subject: soc/intel/xeon_sp/cpx: Add DIMM definition in SystemMemoryMapHob ......................................................................
soc/intel/xeon_sp/cpx: Add DIMM definition in SystemMemoryMapHob
Most of them are needed for SMBIOS type 17 creation.
Tested=With FSP WW34 added with SPDRegVen DIMM member, verified the printed hob values match with FSP hob data.
Change-Id: I02f4600f1be39e2576d7c84a5a6b6672ebb7034b Signed-off-by: Johnny Lin johnny_lin@wiwynn.com --- M src/vendorcode/intel/fsp/fsp2_0/cooperlake_sp/hob_memmap.h 1 file changed, 48 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/47/44847/1
diff --git a/src/vendorcode/intel/fsp/fsp2_0/cooperlake_sp/hob_memmap.h b/src/vendorcode/intel/fsp/fsp2_0/cooperlake_sp/hob_memmap.h index f85c4ff..50ead77 100644 --- a/src/vendorcode/intel/fsp/fsp2_0/cooperlake_sp/hob_memmap.h +++ b/src/vendorcode/intel/fsp/fsp2_0/cooperlake_sp/hob_memmap.h @@ -58,6 +58,9 @@ #define MEM_TYPE_RESERVED (1 << 8) #define MEM_ADDR_64MB_SHIFT_BITS 26
+#define NGN_MAX_SERIALNUMBER_STRLEN 4 +#define NGN_MAX_PARTNUMBER_STRLEN 20 +#define NGN_FW_VER_LEN 4 // // System Memory Map HOB information // @@ -85,6 +88,46 @@ UINT32 ElementSize; } SYSTEM_MEMORY_MAP_ELEMENT;
+typedef struct DimmDevice { + UINT8 reserved1[2]; + UINT8 DcpmmPresent; + UINT8 reserved2[1]; + UINT8 NumRanks; + UINT8 reserved3[1]; + UINT8 actKeyByte2; + UINT8 reserved4[4]; + UINT16 nonVolCap; + UINT16 DimmSize; + UINT8 reserved5[4]; + UINT16 SPDMMfgId; // Module Mfg Id from SPD + UINT16 VendorID; + UINT16 DeviceID; + UINT8 reserved6[22]; + UINT8 serialNumber[NGN_MAX_SERIALNUMBER_STRLEN]; // Serial Number + UINT8 PartNumber[NGN_MAX_PARTNUMBER_STRLEN]; // Part Number + UINT8 FirmwareVersionStr[NGN_FW_VER_LEN]; // Used to update the SMBIOS TYPE 17 + UINT8 reserved7[23]; + UINT16 SubsystemVendorID; + UINT16 SubsystemDeviceID; + UINT8 reserved8[4]; + UINT8 DimmSku; // Dimm SKU info + UINT8 reserved9[3]; + INT32 commonTck; + UINT8 EnergyType; + UINT8 reserved10[1]; + UINT16 SPDRegVen; // Register Vendor ID in SPD +} MEMMAP_DIMM_DEVICE_INFO_STRUCT; + +struct ChannelDevice { + UINT8 reserved1[15]; + MEMMAP_DIMM_DEVICE_INFO_STRUCT DimmInfo[MAX_IMC]; +}; + +typedef struct socket { + UINT8 reserved1[1110]; + struct ChannelDevice ChannelInfo[MAX_CH]; +} MEMMAP_SOCKET; + /* NOTE - Reserved sizes need to be calibrated if any of the above #define values change */ typedef struct SystemMemoryMapHob { UINT8 reserved1[61]; @@ -101,16 +144,17 @@ UINT8 NumChPerMC; UINT8 numberEntries; // Number of Memory Map Elements SYSTEM_MEMORY_MAP_ELEMENT Element[(MAX_SOCKET * MAX_DRAM_CLUSTERS * MAX_SAD_RULES) + MAX_FPGA_REMOTE_SAD_RULES]; - - UINT8 reserved3[24551]; + UINT8 reserved3[2212]; + MEMMAP_SOCKET Socket[MAX_SOCKET]; + UINT8 reserved4[1603];
UINT16 BiosFisVersion; // Firmware Interface Specification version currently supported by BIOS
- UINT8 reserved4[24]; + UINT8 reserved5[24];
UINT32 MmiohBase; // MMIOH base in 64MB granularity
- UINT8 reserved5[2]; + UINT8 reserved6[2];
} SYSTEM_MEMORY_MAP_HOB;
Johnny Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44847 )
Change subject: soc/intel/xeon_sp/cpx: Add DIMM definition in SystemMemoryMapHob ......................................................................
Patch Set 1:
This change is ready for review.
Johnny Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44847 )
Change subject: soc/intel/xeon_sp/cpx: Add DIMM definition in SystemMemoryMapHob ......................................................................
Patch Set 1: Code-Review-1
Can be reviewed first but needs to wait for newer FSP release to be merged.
Johnny Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44847 )
Change subject: vendorcode/intel/fsp/fsp2_0/cpx_sp: Add DIMM definition in SystemMemoryMapHob ......................................................................
Patch Set 3:
This change is ready for review.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44847 )
Change subject: vendorcode/intel/fsp/fsp2_0/cpx_sp: Add DIMM definition in SystemMemoryMapHob ......................................................................
Patch Set 3: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/44847/3/src/vendorcode/intel/fsp/fs... File src/vendorcode/intel/fsp/fsp2_0/cooperlake_sp/hob_memmap.h:
https://review.coreboot.org/c/coreboot/+/44847/3/src/vendorcode/intel/fsp/fs... PS3, Line 61: NGN What does NGN mean?
Hello build bot (Jenkins), Anjaneya "Reddy" Chagam, Jonathan Zhang, Jingle Hsu, Angel Pons, Morgan Jang, Bryant Ou, Patrick Rudolph, Nathaniel L Desimone, Tim Chu,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44847
to look at the new patch set (#4).
Change subject: vendorcode/intel/fsp/fsp2_0/cpx_sp: Add DIMM definition in SystemMemoryMapHob ......................................................................
vendorcode/intel/fsp/fsp2_0/cpx_sp: Add DIMM definition in SystemMemoryMapHob
Most of them are needed for SMBIOS type 17 creation.
Tested=With FSP WW36 verified the printed hob values match with FSP hob data.
Change-Id: I02f4600f1be39e2576d7c84a5a6b6672ebb7034b Signed-off-by: Johnny Lin johnny_lin@wiwynn.com --- M src/vendorcode/intel/fsp/fsp2_0/cooperlake_sp/hob_memmap.h 1 file changed, 48 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/47/44847/4
Johnny Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44847 )
Change subject: vendorcode/intel/fsp/fsp2_0/cpx_sp: Add DIMM definition in SystemMemoryMapHob ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44847/3/src/vendorcode/intel/fsp/fs... File src/vendorcode/intel/fsp/fsp2_0/cooperlake_sp/hob_memmap.h:
https://review.coreboot.org/c/coreboot/+/44847/3/src/vendorcode/intel/fsp/fs... PS3, Line 61: NGN
What does NGN mean?
Next Generation Non volatile memory.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44847 )
Change subject: vendorcode/intel/fsp/fsp2_0/cpx_sp: Add DIMM definition in SystemMemoryMapHob ......................................................................
Patch Set 4: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/44847/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44847/4//COMMIT_MSG@7 PS4, Line 7: vendorcode nit: shorten `vendorcode` to `vc`
Hello build bot (Jenkins), Anjaneya "Reddy" Chagam, Jonathan Zhang, Jingle Hsu, Angel Pons, Morgan Jang, Bryant Ou, Patrick Rudolph, Nathaniel L Desimone, Tim Chu,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44847
to look at the new patch set (#5).
Change subject: vc/intel/fsp/fsp2_0/cpx_sp: Add DIMM definition in SystemMemoryMapHob ......................................................................
vc/intel/fsp/fsp2_0/cpx_sp: Add DIMM definition in SystemMemoryMapHob
Most of them are needed for SMBIOS type 17 creation.
Tested=With FSP WW36 verified the printed hob values match with FSP hob data.
Change-Id: I02f4600f1be39e2576d7c84a5a6b6672ebb7034b Signed-off-by: Johnny Lin johnny_lin@wiwynn.com --- M src/vendorcode/intel/fsp/fsp2_0/cooperlake_sp/hob_memmap.h 1 file changed, 48 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/47/44847/5
Johnny Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44847 )
Change subject: vc/intel/fsp/fsp2_0/cpx_sp: Add DIMM definition in SystemMemoryMapHob ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44847/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44847/4//COMMIT_MSG@7 PS4, Line 7: vendorcode
nit: shorten `vendorcode` to `vc`
Done
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/44847 )
Change subject: vc/intel/fsp/fsp2_0/cpx_sp: Add DIMM definition in SystemMemoryMapHob ......................................................................
vc/intel/fsp/fsp2_0/cpx_sp: Add DIMM definition in SystemMemoryMapHob
Most of them are needed for SMBIOS type 17 creation.
Tested=With FSP WW36 verified the printed hob values match with FSP hob data.
Change-Id: I02f4600f1be39e2576d7c84a5a6b6672ebb7034b Signed-off-by: Johnny Lin johnny_lin@wiwynn.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/44847 Reviewed-by: Angel Pons th3fanbus@gmail.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/vendorcode/intel/fsp/fsp2_0/cooperlake_sp/hob_memmap.h 1 file changed, 48 insertions(+), 4 deletions(-)
Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved
diff --git a/src/vendorcode/intel/fsp/fsp2_0/cooperlake_sp/hob_memmap.h b/src/vendorcode/intel/fsp/fsp2_0/cooperlake_sp/hob_memmap.h index f85c4ff..50ead77 100644 --- a/src/vendorcode/intel/fsp/fsp2_0/cooperlake_sp/hob_memmap.h +++ b/src/vendorcode/intel/fsp/fsp2_0/cooperlake_sp/hob_memmap.h @@ -58,6 +58,9 @@ #define MEM_TYPE_RESERVED (1 << 8) #define MEM_ADDR_64MB_SHIFT_BITS 26
+#define NGN_MAX_SERIALNUMBER_STRLEN 4 +#define NGN_MAX_PARTNUMBER_STRLEN 20 +#define NGN_FW_VER_LEN 4 // // System Memory Map HOB information // @@ -85,6 +88,46 @@ UINT32 ElementSize; } SYSTEM_MEMORY_MAP_ELEMENT;
+typedef struct DimmDevice { + UINT8 reserved1[2]; + UINT8 DcpmmPresent; + UINT8 reserved2[1]; + UINT8 NumRanks; + UINT8 reserved3[1]; + UINT8 actKeyByte2; + UINT8 reserved4[4]; + UINT16 nonVolCap; + UINT16 DimmSize; + UINT8 reserved5[4]; + UINT16 SPDMMfgId; // Module Mfg Id from SPD + UINT16 VendorID; + UINT16 DeviceID; + UINT8 reserved6[22]; + UINT8 serialNumber[NGN_MAX_SERIALNUMBER_STRLEN]; // Serial Number + UINT8 PartNumber[NGN_MAX_PARTNUMBER_STRLEN]; // Part Number + UINT8 FirmwareVersionStr[NGN_FW_VER_LEN]; // Used to update the SMBIOS TYPE 17 + UINT8 reserved7[23]; + UINT16 SubsystemVendorID; + UINT16 SubsystemDeviceID; + UINT8 reserved8[4]; + UINT8 DimmSku; // Dimm SKU info + UINT8 reserved9[3]; + INT32 commonTck; + UINT8 EnergyType; + UINT8 reserved10[1]; + UINT16 SPDRegVen; // Register Vendor ID in SPD +} MEMMAP_DIMM_DEVICE_INFO_STRUCT; + +struct ChannelDevice { + UINT8 reserved1[15]; + MEMMAP_DIMM_DEVICE_INFO_STRUCT DimmInfo[MAX_IMC]; +}; + +typedef struct socket { + UINT8 reserved1[1110]; + struct ChannelDevice ChannelInfo[MAX_CH]; +} MEMMAP_SOCKET; + /* NOTE - Reserved sizes need to be calibrated if any of the above #define values change */ typedef struct SystemMemoryMapHob { UINT8 reserved1[61]; @@ -101,16 +144,17 @@ UINT8 NumChPerMC; UINT8 numberEntries; // Number of Memory Map Elements SYSTEM_MEMORY_MAP_ELEMENT Element[(MAX_SOCKET * MAX_DRAM_CLUSTERS * MAX_SAD_RULES) + MAX_FPGA_REMOTE_SAD_RULES]; - - UINT8 reserved3[24551]; + UINT8 reserved3[2212]; + MEMMAP_SOCKET Socket[MAX_SOCKET]; + UINT8 reserved4[1603];
UINT16 BiosFisVersion; // Firmware Interface Specification version currently supported by BIOS
- UINT8 reserved4[24]; + UINT8 reserved5[24];
UINT32 MmiohBase; // MMIOH base in 64MB granularity
- UINT8 reserved5[2]; + UINT8 reserved6[2];
} SYSTEM_MEMORY_MAP_HOB;
9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44847 )
Change subject: vc/intel/fsp/fsp2_0/cpx_sp: Add DIMM definition in SystemMemoryMapHob ......................................................................
Patch Set 6:
Automatic boot test returned (PASS/FAIL/TOTAL): 7/1/8 "QEMU x86 q35/ich9" (x86_32) using payload TianoCore : SUCCESS : https://lava.9esec.io/r/18268 "QEMU x86 q35/ich9" (x86_32) using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/18267 "QEMU x86 i440fx/piix4" (x86_64) using payload SeaBIOS : FAIL : https://lava.9esec.io/r/18266 "QEMU x86 i440fx/piix4" (x86_32) using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/18265 "QEMU AArch64" using payload LinuxBoot_u-root_kexec : SUCCESS : https://lava.9esec.io/r/18264 "HP Z220 SFF Workstation" (x86_32) using payload LinuxBoot_BusyBox_kexec : SUCCESS : https://lava.9esec.io/r/18271 "HP Compaq 8200 Elite SFF PC" (x86_32) using payload TianoCore : SUCCESS : https://lava.9esec.io/r/18270 "HP Compaq 8200 Elite SFF PC" (x86_32) using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/18269
Please note: This test is under development and might not be accurate at all!
-
9elements QA (Code Review) -
Angel Pons (Code Review)
-
Johnny Lin (Code Review)
-
Patrick Georgi (Code Review)