Name of user not set #1002358 has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33234 )
Change subject: security/intel/stm: Add STM support ......................................................................
Patch Set 6:
(17 comments)
Will post an update once I am finish dealing with the CamelCase. Not until the middle of next week.
https://review.coreboot.org/#/c/33234/6/src/security/intel/stm/SmmStm.c File src/security/intel/stm/SmmStm.c:
https://review.coreboot.org/#/c/33234/6/src/security/intel/stm/SmmStm.c@369 PS6, Line 369: //
whitespace at begging of line […]
I assume that you mean that all comments are to be left justified. Coding style and the bot were not clear on that issue. Will mode all files to comply.
https://review.coreboot.org/#/c/33234/6/src/security/intel/stm/SmmStm.c@439 PS6, Line 439: for (SubIndex = 0;
suspect code indent for conditional statements (24, 24)
Done
https://review.coreboot.org/#/c/33234/6/src/security/intel/stm/SmmStm.c@442 PS6, Line 442: if ((Resource->PciCfg.PciDevicePath[SubIndex].PciDevice >
try to use functions if the indentation is too much
Done
https://review.coreboot.org/#/c/33234/6/src/security/intel/stm/SmmStm.c@442 PS6, Line 442: if ((Resource->PciCfg.PciDevicePath[SubIndex].PciDevice >
Done
Done
https://review.coreboot.org/#/c/33234/6/src/security/intel/stm/SmmStm.c@442 PS6, Line 442: if ((Resource->PciCfg.PciDevicePath[SubIndex].PciDevice >
suspect code indent for conditional statements (24, 40)
Done
https://review.coreboot.org/#/c/33234/6/src/security/intel/stm/SmmStm.c@442 PS6, Line 442: if ((Resource->PciCfg.PciDevicePath[SubIndex].PciDevice >
Done
Done
https://review.coreboot.org/#/c/33234/6/src/security/intel/stm/SmmStm.c@444 PS6, Line 444: (Resource->PciCfg.PciDevicePath[SubIndex].PciFunction >
How does one deal with statements that cannot be split
Done
https://review.coreboot.org/#/c/33234/6/src/security/intel/stm/SmmStm.c@748 PS6, Line 748: (STM_PAGES_TO_SIZE(STM_SIZE_TO_PAGES(StmHeader->SwStmHdr.StaticImageSize))
How does one deal with statements that really cannot be split
Done
https://review.coreboot.org/#/c/33234/6/src/security/intel/stm/SmmStm.c@750 PS6, Line 750: + (StmHeader->SwStmHdr.PerProcDynamicMemorySize
trailing whitespace
Done
https://review.coreboot.org/#/c/33234/6/src/security/intel/stm/SmmStm.c@750 PS6, Line 750: + (StmHeader->SwStmHdr.PerProcDynamicMemorySize
Done
Done
https://review.coreboot.org/#/c/33234/6/src/security/intel/stm/SmmStm.c@750 PS6, Line 750: + (StmHeader->SwStmHdr.PerProcDynamicMemorySize
Done
Done
https://review.coreboot.org/#/c/33234/6/src/security/intel/stm/SmmStm.c@791 PS6, Line 791: uint32_t
uintptr_t
Done
https://review.coreboot.org/#/c/33234/6/src/security/intel/stm/StmPlatformRe... File src/security/intel/stm/StmPlatformResource.c:
https://review.coreboot.org/#/c/33234/6/src/security/intel/stm/StmPlatformRe... PS6, Line 38: void FixupPciexResource(void);
looks like those can be made static. […]
Done
https://review.coreboot.org/#/c/33234/6/src/security/intel/stm/StmPlatformRe... PS6, Line 186: //RscPcieMmio.Base = PcdGet64 (PcdPciExpressBaseAddress);
CONFIG_MMCONF_BASE_ADDRESS […]
Done
https://review.coreboot.org/#/c/33234/6/src/security/intel/stm/StmPlatformSm... File src/security/intel/stm/StmPlatformSmm.c:
https://review.coreboot.org/#/c/33234/6/src/security/intel/stm/StmPlatformSm... PS6, Line 53: printk(BIOS_DEBUG, "STM loaded into mseg: 0x%08x size: %u\n",
%p
Done
https://review.coreboot.org/#/c/33234/6/src/security/intel/stm/StmPlatformSm... PS6, Line 54: (uint32_t) MsegBase, StmImageSize);
remove case
Done
https://review.coreboot.org/#/c/33234/6/src/security/intel/stm/StmPlatformSm... PS6, Line 87: void ReadGdtr(struct descriptor *gdtr)
static
Done