Rocky Phagura has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41829 )
Change subject: [WIP] cpu/x86/smm: Enable SMM support for Xeon-SP ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/coreboot/+/41829/3/src/cpu/x86/smm/smm_module_... File src/cpu/x86/smm/smm_module_loader.c:
https://review.coreboot.org/c/coreboot/+/41829/3/src/cpu/x86/smm/smm_module_... PS3, Line 96:
When building these segments the algorithm needs to account for the SMM save state size and location […]
Yes, that's exactly what I have done is tiled the CPU threads. AS for the txt smm descriptor it is included in params->params->per_cpu_save_state_size. So if STM config option is enabled then that params->per_cpu_save_state_size will be updated in mp_init.c, fill_mp_state(). Then this value will get passed eventually to smm_load_module and then to this method smm_create_map. So overall the cpu state save area here should be properly reflected and assigned. Are you seeing any boot/runtime issues with state save after porting to these changes? Line 113 below is the where the calculation is done to include state save area (ss_size).
https://review.coreboot.org/c/coreboot/+/41829/3/src/cpu/x86/smm/smm_module_... PS3, Line 573: fxsave_area -= total_stack_size + fxsave_size;
This calculation can place the location of the fxsave_area below the bottom of the TSEG. […]
This is a bug in my code. Good catch. I will fix the issue and resubmit.