cedarhouse1@comcast.net has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33234 )
Change subject: security/intel/stm: Add STM support ......................................................................
Patch Set 44:
(10 comments)
https://review.coreboot.org/c/coreboot/+/33234/44/src/security/intel/stm/Stm... File src/security/intel/stm/StmPlatformResource.h:
https://review.coreboot.org/c/coreboot/+/33234/44/src/security/intel/stm/Stm... PS44, Line 24: #define LPC_FUNCTION 0
is this still used?
LPC_DEVICE and LPC_FUNCTION are used in StmPlatformResource.c. LPC_BUS has been removed
https://review.coreboot.org/c/coreboot/+/33234/44/src/security/intel/stm/Stm... PS44, Line 30: SMRR_PHYSBASE_MSR
why aren't those define in msr. […]
all MSR defines have been moved to msr.h
https://review.coreboot.org/c/coreboot/+/33234/44/src/security/intel/stm/Stm... File src/security/intel/stm/StmPlatformResource.c:
https://review.coreboot.org/c/coreboot/+/33234/44/src/security/intel/stm/Stm... PS44, Line 20: #include <southbridge/intel/common/pmutil.h>
that's only correct for SOUTHBRIDGE_INTEL_COMMON_PMCLIB […]
Done
https://review.coreboot.org/c/coreboot/+/33234/44/src/security/intel/stm/Stm... PS44, Line 26: uint32_t m_tseg_base;
can be removed. those are only written and read once.
Done
https://review.coreboot.org/c/coreboot/+/33234/44/src/security/intel/stm/Stm... PS44, Line 31: uint64_t uint64;
can be removed. […]
Done
https://review.coreboot.org/c/coreboot/+/33234/44/src/security/intel/stm/Stm... PS44, Line 113: #ifndef ACPI_BASE_ADDRESS
Eugene in case there is confusion, please remove the whole commented out block.
Done
https://review.coreboot.org/c/coreboot/+/33234/44/src/security/intel/stm/Stm... PS44, Line 181: Status = add_pi_resource((void *)&rsc_lpc_bridge_pci, 1);
return code not checked
Done
https://review.coreboot.org/c/coreboot/+/33234/44/src/security/intel/stm/Stm... PS44, Line 198: Status = add_pi_resource((void *)&rsc_msr_tpl, 1);
return code not checked
Done
https://review.coreboot.org/c/coreboot/+/33234/44/src/security/intel/stm/Stm... File src/security/intel/stm/StmPlatformSmm.c:
https://review.coreboot.org/c/coreboot/+/33234/44/src/security/intel/stm/Stm... PS44, Line 90: #define SMM_CODE_SEG 0x8
that's not the right place. The other segment defines are in […]
created smm_segs.h
https://review.coreboot.org/c/coreboot/+/33234/44/src/security/intel/stm/Stm... PS44, Line 181: load_stm_image(mseg);
return code not checked
Done