Tim Chu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42242 )
Change subject: drivers/ocp/ipmi: Add ipmi set processor information ......................................................................
Patch Set 7:
(4 comments)
https://review.coreboot.org/c/coreboot/+/42242/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/42242/3//COMMIT_MSG@9 PS3, Line 9: Implement setting processor information to BMC.
If this IPMI OEM command is defined in a public doc (such as OCP doc), please include the link.
It is defined in document YosemiteV3_BMC_Feature_Spec_v1.7 on dropbox.
https://review.coreboot.org/c/coreboot/+/42242/3/src/drivers/ocp/ipmi/ipmi_o... File src/drivers/ocp/ipmi/ipmi_ocp.h:
https://review.coreboot.org/c/coreboot/+/42242/3/src/drivers/ocp/ipmi/ipmi_o... PS3, Line 7: #include <cpu/x86/msr.h>
Please only keep header files needed for this header file.
Done
https://review.coreboot.org/c/coreboot/+/42242/3/src/drivers/ocp/ipmi/ipmi_o... File src/drivers/ocp/ipmi/ipmi_ocp.c:
https://review.coreboot.org/c/coreboot/+/42242/3/src/drivers/ocp/ipmi/ipmi_o... PS3, Line 65: memcpy(&req1.data.manufacturer_id, &mfid, 3);
what if CONFIG_MANU_ID is not defined?
If CONFIG_MANU_ID is not defined, the default value 0x0 may be used and other information may still be send to BMC.
https://review.coreboot.org/c/coreboot/+/42242/3/src/drivers/ocp/ipmi/ipmi_o... PS3, Line 125: /* TBD */
What does the spec say about this? Why TBD? Should we just put stepping_id in req2. […]
There should be a CPU identification table such as in chapter 2.7 of #607480 which shows the corresponding CPUID, Stepping ID and revision. Because the table I have now only shows one kind of CPUID, I think it should be TBD here.