Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38548 )
Change subject: soc/intel: Add Intel Xeon Scalable Processor support ......................................................................
Patch Set 25:
(3 comments)
Thanks for the review. All the comments so far are addressed.
We plan to work on this code base (support for Xeon-SP processors and their OCP platforms) in 3 phases: Phase 1: initial code base. It supports the previous generation of Xeon-SP processor (Skylake-SP), and one OCP platform (TiogaPass). This patch set is verified to boot to target OS on several TiogaPass servers with slightly different configurations (such as CPU core count, PCIe config, socket count, etc.). Pending item of this phase is to merge this patch set into coreboot upstream. Phase 2. Support for current Xeon-SP processors. We are working on bring-up of current generation of Xeon-SP processor (Note that SKX-SP was in Mass Production as of 2018). The code base will be refactored/improved, it will support both Xeon-SP processors and multiple OCP platform (Most of the OCP platform are yet to be announced). This is the multi-company teams' focus right now. Phase 3. Nicer integration with coreboot common code. Through lessons learned from Xeon-SP code base, we will see what we could do to improve coreboot common code, if any.
Let us know if you have any further comments, we will address them as soon as we can.
Best, Jonathan
https://review.coreboot.org/c/coreboot/+/38548/15/src/soc/intel/skylake_sp/c... File src/soc/intel/skylake_sp/cpu.c:
https://review.coreboot.org/c/coreboot/+/38548/15/src/soc/intel/skylake_sp/c... PS15, Line 147: msr1
Not sure what the question is. please clarify.
Done
https://review.coreboot.org/c/coreboot/+/38548/15/src/soc/intel/skylake_sp/i... File src/soc/intel/skylake_sp/include/soc/gpio_fsp.h:
https://review.coreboot.org/c/coreboot/+/38548/15/src/soc/intel/skylake_sp/i... PS15, Line 86: (0x1 | (0x1 << 3)), ///< Set pad for both output and input
Why is this odd? With GpioDirInOut, we set two bits, for both output and input.
Done
https://review.coreboot.org/c/coreboot/+/38548/15/src/soc/intel/skylake_sp/i... File src/soc/intel/skylake_sp/include/soc/msr.h:
https://review.coreboot.org/c/coreboot/+/38548/15/src/soc/intel/skylake_sp/i... PS15, Line 23: #define RDMSR(id) \
We need to print out the marco name. A function will not be able to accomplish that.
Done