[coreboot-gerrit] Change in coreboot[master]: [RFC]cpu/x86/smm: Add a unified way of handling save_states

[Arthur Heymans (Code Review)]

Arthur Heymans 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 6:

(1 comment)

https://review.coreboot.org/c/coreboot/+/36661/6/src/cpu/x86/smm/smm_save_st... 
File src/cpu/x86/smm/smm_save_state.c:

https://review.coreboot.org/c/coreboot/+/36661/6/src/cpu/x86/smm/smm_save_st... 
PS6, Line 249: int smm_save_state_ops_init(void)
             : {
             : 	uint32_t smm_revision = smm_get_revision();
             : 
             : 	switch (smm_revision) {
             : 	case 0x00030002:
             : 	case 0x00030007:
             : 		ops = &legacy_save_state_ops;
             : 		return 0;
             : 	case 0x00030100:
             : 		ops = &em64t100_save_state_ops;
             : 		return 0;
             : 	case 0x00030101: /* SandyBridge, IvyBridge, and Haswell */
             : 		ops = &em64t101_save_state_ops;
             : 		return 0;
             : 	case 0x00020064:
             : 	case 0x00030064:
             : 		ops = &amd64_save_state_ops;
             : 		return 0;
             : 	default:
             : 		printk(BIOS_DEBUG, "smm_revision: 0x%08x\n", smm_revision);
             : 		printk(BIOS_DEBUG, "SMI# not supported on your CPU\n");
             : 		return 1;
             : 	}
             : }
Maybe it's more elegant to have a linker symbol + section CPU states (similar to __cpu_driver). That would allow to link only the ones relevant for a CPU?