Name of user not set #1002358 has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33236 )
Change subject: cpu/x86: Add STM Support ......................................................................
Patch Set 6:
(11 comments)
https://review.coreboot.org/#/c/33236/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/33236/1//COMMIT_MSG@4 PS1, Line 4: EugeneDMyers
Eugene D Myers […]
Done
https://review.coreboot.org/#/c/33236/1//COMMIT_MSG@7 PS1, Line 7: src/cpu/x86 STM Support
Please use: […]
Done
https://review.coreboot.org/#/c/33236/1//COMMIT_MSG@9 PS1, Line 9: STM initialization
Please elaborate.
Done
https://review.coreboot.org/#/c/33236/1//COMMIT_MSG@11 PS1, Line 11: Change-Id: I3a0adcefc0f6e22a9da5fe53952481a77737e5eb
Please add your Signed-off-by line below.
Done
https://review.coreboot.org/#/c/33236/1/src/cpu/x86/mp_init.c File src/cpu/x86/mp_init.c:
https://review.coreboot.org/#/c/33236/1/src/cpu/x86/mp_init.c@44 PS1, Line 44: extern int LoadStmImage(uint32_t mseg);
don't use extern
Done
https://review.coreboot.org/#/c/33236/1/src/cpu/x86/mp_init.c@831 PS1, Line 831: uint32_t mseg;
uintptr_t
Done
https://review.coreboot.org/#/c/33236/1/src/cpu/x86/mp_init.c@1042 PS1, Line 1042: #ifdef CONFIG_STM
no need for preprocessor directives
Done
https://review.coreboot.org/#/c/33236/1/src/cpu/x86/mp_init.c@1044 PS1, Line 1044: /* Currently, the CPU SMM save state size is based on a simplistic : * algorithm. (set it to 1K) : * note: In the future, this will need to handle newer x86 processors : * that require 32k alignment of the save state on 32K boundries.*/
Please see the coding style about how to format multi-line comments. […]
Done
https://review.coreboot.org/#/c/33236/1/src/cpu/x86/mp_init.c@1047 PS1, Line 1047: * that require 32k alignment of the save state on 32K boundries.*/
Please add a space before `*/`.
Done
https://review.coreboot.org/#/c/33236/1/src/cpu/x86/mp_init.c@1044 PS1, Line 1044: /* Currently, the CPU SMM save state size is based on a simplistic : * algorithm. (set it to 1K) : * note: In the future, this will need to handle newer x86 processors : * that require 32k alignment of the save state on 32K boundries.*/
Done
Done
https://review.coreboot.org/#/c/33236/1/src/cpu/x86/mp_init.c@1048 PS1, Line 1048: state->smm_save_state_size += (sizeof(TXT_PROCESSOR_SMM_DESCRIPTOR) + 0x1000) & 0xfffff000;
use one of the macros: ALIGN_UP and ALIGN_DOWN
Done