[M] Change in coreboot[main]: soc/intel: Adapt crashlog IP to also support 64-bit

[Sowmya Aralguppe (Code Review)]

Attention is currently required from: Angel Pons, Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Jamie Ryu, Kapil Porwal, Nick Vaccaro, Paul Menzel, Pratikkumar V Prajapati, Subrata Banik, Tarun.

Sowmya Aralguppe has posted comments on this change by Sowmya Aralguppe. ( https://review.coreboot.org/c/coreboot/+/83106?usp=email )

Change subject: soc/intel: Adapt crashlog IP to also support 64-bit
......................................................................

Patch Set 22:

(2 comments)

Patchset:

PS22:
> @[e-mail]: can you please check below prints w/ and w/o your CL? […]

There were some discrepancies in the original implementation
1)      The child record count is 5 but the loop happens for only 2 times.
2)      Cpu crash log size is wrong .

PMC CrashLog size in discovery mode: 0x2208
[DEBUG]  adjusted cpu discovery table offset: 0x1f70
[DEBUG]  cpu_crashlog_discovery_table buffer count: 0x5
[DEBUG]  cpu_crashlog_discovery_table buffer: 0x0 size: 0x800 offset: 0x0
[DEBUG]  cpu_crashlog_discovery_table buffer: 0x1 size: 0x800 offset: 0x4000

With more prints I could see that it is not taking offset properly – address which was supposed to be 0x8d7c1f78 and it was wrongly taken as 0x8d7c1f90 for offset of 8

[DEBUG]  bar address value is: 0x8d7c0000
[DEBUG]  adjusted cpu discovery table offset: 0x1f70
[DEBUG]  disc_tab_addr value is: 0x8d7c1f70
[DEBUG]  The derefernced value is: 0x80300513
[DEBUG]  Header data is: 0x3b80300513
[DEBUG]  cpu_crashlog_discovery_table buffer count: 0x5
[DEBUG]  The 64 bit dw1 data is 0x80000000000
[DEBUG]  The Address value is: 0x0x8d7c1f90
[DEBUG]  The derefernced value is: 0x4000 and the offset is 8

Was able to root cause this issue to pointer typecast  and with this there was an increase in the cpu crashlog

[DEBUG]  cpu_crashlog_discovery_table buffer count: 0x5
[DEBUG]  cpu_crashlog_discovery_table buffer: 0x1 size: 0x800 offset: 0x4000
[DEBUG]  cpu_crashlog_discovery_table buffer: 0x2 size: 0x800 offset: 0x10000
[DEBUG]  cpu_crashlog_discovery_table buffer: 0x3 size: 0x800 offset: 0x14000
[DEBUG]  cpu_crashlog_discovery_table buffer: 0x4 size: 0x93 offset: 0x20000
[DEBUG]  PMC crashLog size : 0x2208
[DEBUG]  Region[0x0].Tag=0x7 offset=0x9b, size=0x400
[DEBUG]  Found metadata tag. PMC crashlog size adjusted to: 0x1208
[DEBUG]  Region[0x1].Tag=0x0 offset=0xa00, size=0x280
[DEBUG]  Region[0x2].Tag=0x0 offset=0x2bb0, size=0xa
[DEBUG]  Region[0x3].Tag=0x0 offset=0x3a00, size=0x80
[DEBUG]  Region[0x4].Tag=0x1 offset=0x500, size=0x100
[DEBUG]  Region[0x5].Tag=0x1 offset=0x12d8, size=0xa
[DEBUG]  Region[0x6].Tag=0x1 offset=0x1600, size=0x6e
[DEBUG]  Invalid data 0xdeadbeef at offset 0x1600 from addr 0x8d6fc000
[DEBUG]  PMC crashlog size adjusted to: 0x1050
[DEBUG]  Region[0x7].Tag=0x0 offset=0x0, size=0x0
[DEBUG]  m_cpu_crashLog_size : 0x624C bytes

There was  one more change
Here in the below code, we are checking for the offset and not size
static bool is_crashlog_data_valid(u32 dw0)
{
                return (dw0 != 0x0 && dw0 != INVALID_CRASHLOG_RECORD);
}

dw0 = read32((u32 *)(uintptr_t)disc_tab_addr + cur_offset);
if (!is_crashlog_data_valid(dw0))
                continue
we should be checking the upper 32 bytes.

[DEBUG]  adjusted cpu discovery table offset: 0x1f70
[DEBUG]  cpu_crashlog_discovery_table buffer count: 0x5
[DEBUG]  cpu_crashlog_discovery_table buffer: 0x0 size: 0x800 offset: 0x0
[DEBUG]  cpu_crashlog_discovery_table buffer: 0x1 size: 0x800 offset: 0x4000
[DEBUG]  cpu_crashlog_discovery_table buffer: 0x2 size: 0x800 offset: 0x10000
[DEBUG]  cpu_crashlog_discovery_table buffer: 0x3 size: 0x800 offset: 0x14000
[DEBUG]  cpu_crashlog_discovery_table buffer: 0x4 size: 0x93 offset: 0x20000
[DEBUG]  PMC crashLog size : 0x2208
[DEBUG]  Region[0x0].Tag=0x7 offset=0x9b, size=0x400
[DEBUG]  Found metadata tag. PMC crashlog size adjusted to: 0x1208
[DEBUG]  Region[0x1].Tag=0x0 offset=0xa00, size=0x280
[DEBUG]  Region[0x2].Tag=0x0 offset=0x2bb0, size=0xa
[DEBUG]  Region[0x3].Tag=0x0 offset=0x3a00, size=0x80
[DEBUG]  Region[0x4].Tag=0x1 offset=0x500, size=0x100
[DEBUG]  Region[0x5].Tag=0x1 offset=0x12d8, size=0xa
[DEBUG]  Region[0x6].Tag=0x1 offset=0x1600, size=0x6e
[DEBUG]  Invalid data 0xdeadbeef at offset 0x1600 from addr 0x8d6fc000
[DEBUG]  PMC crashlog size adjusted to: 0x1050
[DEBUG]  Region[0x7].Tag=0x0 offset=0x0, size=0x0
[DEBUG]  m_cpu_crashLog_size : 0x824C bytes
[DEBUG]  CPU crashLog present.

File src/soc/intel/meteorlake/crashlog.c:

https://review.coreboot.org/c/coreboot/+/83106/comment/b6aaaa9d_67d2430d?usp... :
PS22, Line 355: 		if (!is_crashlog_data_valid(dw0))
> This check is gone, is this intentional?

This is in line 343 .Inside for loop we need to check for size only