Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33234 )
Change subject: security/intel/stm: Add STM support ......................................................................
Patch Set 33:
(13 comments)
https://review.coreboot.org/c/coreboot/+/33234/29//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/33234/29//COMMIT_MSG@16 PS29, Line 16: tables
The STM page tables is a six page region located in the MSEG and are pointed to by the CR3 Offset fi […]
That's an important fact that's need to be mentioned. Currently the SMM doesn't handle IA32e mode. Also adding x86_64 support to SMM is probably out of scope of this patch.
https://review.coreboot.org/c/coreboot/+/33234/29//COMMIT_MSG@16 PS29, Line 16: resource
The BIOS resource list defines the resources that the SMI Handler is allowed to access. […]
Thanks, I found that document. Please reference the document in the commit message or the STM's documentation in Documentation/
https://review.coreboot.org/c/coreboot/+/33234/33/src/security/intel/stm/Kco... File src/security/intel/stm/Kconfig:
https://review.coreboot.org/c/coreboot/+/33234/33/src/security/intel/stm/Kco... PS33, Line 40: config ACPI_BASE_ADDRESS that's defined in the platform headers. the correct approach would be to add get_pmbase() to all intel platforms
https://review.coreboot.org/c/coreboot/+/33234/33/src/security/intel/stm/Smm... File src/security/intel/stm/SmmStm.c:
https://review.coreboot.org/c/coreboot/+/33234/33/src/security/intel/stm/Smm... PS33, Line 603: void stm_gen_4g_pagetable_ia32(uint32_t pagetable_base) this is unused
https://review.coreboot.org/c/coreboot/+/33234/33/src/security/intel/stm/Smm... PS33, Line 621: x86_32 it only creates 2M PTEs
https://review.coreboot.org/c/coreboot/+/33234/33/src/security/intel/stm/Smm... PS33, Line 641: for (index = 0; index < 4; index++) { this can be placed in CBFS and used with CB:36778 applied. If you don't want static page tables in ROM you can simply copy them.
https://review.coreboot.org/c/coreboot/+/33234/33/src/security/intel/stm/Stm... File src/security/intel/stm/StmPlatformSmm.c:
https://review.coreboot.org/c/coreboot/+/33234/33/src/security/intel/stm/Stm... PS33, Line 122: uint32_t uintptr_t
https://review.coreboot.org/c/coreboot/+/33234/33/src/security/intel/stm/Stm... PS33, Line 127: uint32_t uintptr_t
https://review.coreboot.org/c/coreboot/+/33234/33/src/security/intel/stm/Stm... PS33, Line 128: psd->smm_cs = 0x8; ROM_CODE_SEG
https://review.coreboot.org/c/coreboot/+/33234/33/src/security/intel/stm/Stm... PS33, Line 129: 0x10 ROM_DATA_SEG
https://review.coreboot.org/c/coreboot/+/33234/33/src/security/intel/stm/Stm... PS33, Line 130: 0x10 ROM_DATA_SEG
https://review.coreboot.org/c/coreboot/+/33234/33/src/security/intel/stm/Stm... PS33, Line 131: 0x10 ROM_DATA_SEG
https://review.coreboot.org/c/coreboot/+/33234/33/src/security/intel/stm/Stm... PS33, Line 132: psd->smm_tr = 0x18; ROM_CODE_SEG64