Attention is currently required from: Kapil Porwal, Pranava Y N, Sowmya Aralguppe.
Subrata Banik has posted comments on this change by Sowmya Aralguppe. ( https://review.coreboot.org/c/coreboot/+/85531?usp=email )
Change subject: soc/intel/pantherlake: Remove hardcoded value for child nodes ......................................................................
Patch Set 4:
(1 comment)
File src/soc/intel/pantherlake/crashlog.c:
https://review.coreboot.org/c/coreboot/+/85531/comment/19f0ec3c_f603ec0e?usp... : PS4, Line 319: cpu_cl_disc_tab.header.fields.count = CRASHLOG_NODES_COUNT;
We hardcoded the child nodes to 2 to match previous x32 implementation. Summary : MTL has 5 crash nodes for different dielets in the North only.
PUNIT
SOCN
DMU
Compute Die
Graphics Die (GCD)
Adamantine Die (ADM) – e RAM (This is not part of the MTL as of now)
But the crash log discovery for Graphics die (5th child node ) had incorrect offset and buffer size for storing crash record. I did not debug this further. This might be because bios is not configuring discovery table for graphics Die properly or it has completely disabled crashlog for Graphics die. Since we wanted the bert file to match the previous implementation x32 (which was iterating only 2 times due to the bug in the pointer implementation which went away when we converted to x64),we read only PUNIT and SOCN crashlog into SRAM even though DMU and compute Die were having valid crash record. Coming back : The count should be read from the discovery table always and should not be hardcoded. For PTL the count is just 1 which is at the offset 0x50 and the record buffer is 0x12ec dwords. In the south PCD handled by PMC and does not belong to CPU crashlog handled by PUNIT
I'm not sure why you want to bring the 32-bit vs 64-bit implementation into this discussion. The real problem is how many nodes to parse when looking for a crash. My experience has shown that two nodes are sufficient, even though other nodes might contain additional information. If you're not sure that enabling all nodes will work seamlessly for all platforms, you shouldn't enable this feature dynamically. As we've seen in the past, that can lead to breakage. I don't see how you think you can avoid introducing the same problem this time. My suggestion is to keep only two nodes for existing platforms and use PTL to experiment on all five nodes. If you wish, you can use a static Kconfig to control this feature.