Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48262 )
Change subject: mb/emulation/qemu-q35: Add support for SMM_TSEG with parallel MP init ......................................................................
Patch Set 4:
(5 comments)
https://review.coreboot.org/c/coreboot/+/48262/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/48262/4//COMMIT_MSG@8 PS4, Line 8: Tested w/ / w/o KVM, MP?
https://review.coreboot.org/c/coreboot/+/48262/4/src/mainboard/emulation/qem... File src/mainboard/emulation/qemu-q35/cpu.c:
https://review.coreboot.org/c/coreboot/+/48262/4/src/mainboard/emulation/qem... PS4, Line 17: // *smm_save_state_size = sizeof(amd64_smm_state_save_area_t); : *smm_save_state_size = 0x400; Do you want to add a comment about this? ;)
https://review.coreboot.org/c/coreboot/+/48262/4/src/mainboard/emulation/qem... PS4, Line 30: * SMRAM entry point to here. */ Nit, proper style would be dangling /* and */ each on a line of its own. (or drop the lone asterisks on the left, but at top level I'd prefer the extensive style)
https://review.coreboot.org/c/coreboot/+/48262/4/src/mainboard/emulation/qem... PS4, Line 43: BIOS_SPEW Nit, I guess it's because other code does it, but I don't see a real reason do make this BIOS_SPEW.
https://review.coreboot.org/c/coreboot/+/48262/4/src/mainboard/emulation/qem... PS4, Line 50: * Nit, drop this * like above.
Or maybe just make it a single line?