Eugene Myers 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 4:
(2 comments)
This change is ready for review.
https://review.coreboot.org/c/coreboot/+/41829/3/src/cpu/x86/mp_init.c File src/cpu/x86/mp_init.c:
https://review.coreboot.org/c/coreboot/+/41829/3/src/cpu/x86/mp_init.c@758 PS3, Line 758: mp_state.perm_smbase, For the STM to be properly setup in this method, the above line (758) needs to be changed to 'perm_smbase' (which should be the same as the third parameter)
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:
Yes, that's exactly what I have done is tiled the CPU threads. […]
Thanks, maybe you could clarify what is happening in your comments and diagrams.
Two issues (not sure that they are related).
(1) Looks like a min number segments needed = 0 might be a problem. Haven't traced it out.
smm_create_map: cpus in one segment 6 smm_create_map: min # of segments needed 0
(2) The second issue I resolved myself when I went back and realized what was really changed. The STM issue was that the EIP for the SMI hander was incorrect because of the change in how the SMI handler is started.
The STM is setup to start the SMI handler in step #2.5 - .5 because the STM starts the handler in 32-bit mode, so the start is at an offset where the 32-bit code is. I did this because the STM normally starts the SMI handler in 64-bit mode and the modification for a 32-bit mode start was three line of code. Starting a VM in 16-bit is somewhat more complicated, so I wanted to avoid that.
The previous method had all entries going to a single location and then switching to 32-bit mode. Now the suggested patch to mp_init.c fixes that issue and works well on my test machine.
-
Eugene Myers (Code Review)