Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36661 )
Change subject: [RFC]cpu/x86/smm: Add a unified way of handling save_states ......................................................................
Patch Set 5:
(3 comments)
https://review.coreboot.org/c/coreboot/+/36661/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36661/5//COMMIT_MSG@9 PS5, Line 9: This adds common code to handle SMM save states based on the SMM it's not clear why runtime detection is done here
https://review.coreboot.org/c/coreboot/+/36661/5/src/cpu/x86/smm/smm_save_st... File src/cpu/x86/smm/smm_save_state.c:
https://review.coreboot.org/c/coreboot/+/36661/5/src/cpu/x86/smm/smm_save_st... PS5, Line 27: uint8_t *top = smm_get_save_state_top(0); is it guaranteed that all CPUs have entered SMM and stored their save state? Is this code only called on CPU 0?
https://review.coreboot.org/c/coreboot/+/36661/5/src/cpu/x86/smm/smm_save_st... PS5, Line 251: uint32_t smm_revision = smm_get_revision(); isn't that known at compile-time? That would allow to link only the savestate ops that's needed on the current CPU