[coreboot-gerrit] [S] Change in coreboot[master]: soc/intel/meteorlake: Adjust discovery table offset based on CPUID

[Pratikkumar V Prajapati (Code Review)]

Attention is currently required from: Eran Mitrani, Jakub Czapiga, Kapil Porwal, Paul Menzel, Subrata Banik, Sumeet R Pawnikar, Tarun Tuli, Tracy Wu.

Pratikkumar V Prajapati has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/76400?usp=email )

Change subject: soc/intel/meteorlake: Adjust discovery table offset based on CPUID
......................................................................

Patch Set 12:

(3 comments)

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

https://review.coreboot.org/c/coreboot/+/76400/comment/fb60ff37_b47dd930 :
PS11, Line 358: u32 bar_addr = 0, disc_tab_addr = 0, disc_tab_offset = 0;
> code-style: AFAIK we are using C11 (`-std=gnu11`), so variable declarations are not required to be a […]

This style is followed at many places in the same file so i used same method. I would address this with a separate patch for the entire crashlog code later.

https://review.coreboot.org/c/coreboot/+/76400/comment/a20339e0_8bd869ce :
PS11, Line 367: disc_tab_offset <<= 3;
> Is there a risk of overflow in this operation?

Nope

https://review.coreboot.org/c/coreboot/+/76400/comment/30508c03_75d2fb0a :
PS11, Line 374: cpu_cl_disc_tab.header.data = ((u64)read32((u32 *)disc_tab_addr) +
              : 					((u64)read32((u32 *)(disc_tab_addr + 4)) << 32));
              :
> Shouldn't it be binary-OR instead of addition? (Just asking for clarification :) )

Addressing this with https://review.coreboot.org/c/coreboot/+/77240/