Attention is currently required from: Angel Pons, Arthur Heymans, Chen, Gang C, Christian Walter, David Hendricks, Felix Held, Jincheng Li, Johnny Lin, Jonathan Zhang, Lean Sheng Tan, Nico Huber, Patrick Rudolph, Shuo Liu, TangYiwei, Tim Chu.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81316?usp=email )
Change subject: soc/intel/xeon_sp: Add Granite Rapids initial codes ......................................................................
Patch Set 55:
(8 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/81316/comment/56261c74_c7b6078e : PS53, Line 20:
We have to add this after SoC launch since it is shift-left opensource at the moment. […]
Acknowledged
PS: What does “shift-left” mean?
https://review.coreboot.org/c/coreboot/+/81316/comment/980ca802_81706f52 : PS53, Line 21: TEST=Build and boot on intel/archercity CRB
Sorry, here the test means this patch doesn't impact archercity CRB. […]
Acknowledged
Patchset:
PS55: Thank you for addressing my comments so quickly.
File src/soc/intel/xeon_sp/gnr/cpu.c:
https://review.coreboot.org/c/coreboot/+/81316/comment/83f5daeb_b440676c : PS53, Line 87: return num_virts * soc_get_num_cpus();
This API calculates the thread counts which correspond to num_virts (logical cores), while num_phys […]
Maybe add a comment, because it might be confusing seeing the value in the log message, but then seeing, it’s not used.
https://review.coreboot.org/c/coreboot/+/81316/comment/11f536bb_70896832 : PS53, Line 114: printk(BIOS_ERR, "microcode not found in CBFS!\n");
It will caused the subsequent call to skip the microcode loading. […]
The log level should match then? The other log message could also be extended to say, that no updates are applied. – It’s implicit but better be explicit.)
https://review.coreboot.org/c/coreboot/+/81316/comment/0f3d8e35_efc477d9 : PS53, Line 116: intel_microcode_load_unlocked(microcode_patch);
Yes.
Acknowledged
File src/soc/intel/xeon_sp/gnr/include/soc/vpd.h:
https://review.coreboot.org/c/coreboot/+/81316/comment/779c5b3f_ebd946bb : PS53, Line 6: #define FAST_BOOT_EN "fast_boot_en" /* 1 or 0: enable or disable fast boot for warm/cold reset */
Thank you.
File src/soc/intel/xeon_sp/include/soc/acpi.h:
https://review.coreboot.org/c/coreboot/+/81316/comment/80492703_7738ebab : PS53, Line 21: unsigned long xeonsp_acpi_create_madt_lapics(unsigned long current);
We have to keep the declaration to pass build, however, I remove the null implementation, […]
Acknowledged