Attention is currently required from: Arthur Heymans, Benjamin Doron, Lean Sheng Tan, Leon Groß, Nico Huber, Paul Menzel.
Martin L Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/77905?usp=email )
Change subject: mb/emulation: Add SIMICS QSP support ......................................................................
Patch Set 21:
(11 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/77905/comment/ecb6dada_127c525c : PS21, Line 8:
`Possible unwrapped commit description (prefer a maximum 72 chars per line)`
Please fix.
https://review.coreboot.org/c/coreboot/+/77905/comment/f47378a6_88357cc5 : PS21, Line 11:
`Possible unwrapped commit description (prefer a maximum 72 chars per line)`
Please fix.
Patchset:
PS21: I think having this board for training makes a lot of sense.
File Documentation/mainboard/emulation/simics-qsp.md:
https://review.coreboot.org/c/coreboot/+/77905/comment/4d6a4773_206ddd09 : PS17, Line 7: , Remove this comma please.
https://review.coreboot.org/c/coreboot/+/77905/comment/4e8c0474_ad894de9 : PS17, Line 12: . because...
https://review.coreboot.org/c/coreboot/+/77905/comment/cddd6d18_e10fa82c : PS17, Line 14: and , then
https://review.coreboot.org/c/coreboot/+/77905/comment/d6a68d89_e7507e12 : PS17, Line 21: of remove 'of'
https://review.coreboot.org/c/coreboot/+/77905/comment/48a0f300_784dc37a : PS17, Line 36: Make sure to adjust the version numbers and file paths in the commands to your local installation. Please add a final newline.
File src/cpu/qemu-x86/Kconfig:
https://review.coreboot.org/c/coreboot/+/77905/comment/eb3f42f7_a9f16a05 : PS17, Line 17: BOARD_EMULATION_SIMICS_QSP_X86_BOARDX58ICH10 I know that you're not responsible for the initial issue, but we should never have cpu/chipset options that depend on which board is selected.
The correct way to do this is to create a new Kconfig option that is selected by the board, like SMM_UNSUPPORTED or HIDE_SMM_OPTIONS.
I can fix that in a follow on to this patch.
File src/mainboard/emulation/simics-qsp/mainboard.c:
https://review.coreboot.org/c/coreboot/+/77905/comment/bae2b6df_cdf2540f : PS21, Line 76: // static const struct pci_driver nb_driver __pci_driver = { : // .ops = &nb_operations, : // .vendor = 0x8086, : // .device = 0x29c0, : // }; Please don't add commented-out code.
File src/southbridge/intel/i82801jx/Kconfig:
https://review.coreboot.org/c/coreboot/+/77905/comment/106615be_7dcfd531 : PS17, Line 6: if !BOARD_EMULATION_SIMICS_QSP_X86_BOARDX58ICH10 Again, we don't want board configs gating common code. The way to do this is to create a config option "NO_COMMON_MADT_LAPIC" and select that in the board, then gate this option on that.