Johnny Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39625 )
Change subject: soc/intel/xeon_sp: Modify FSP-T code caching parameters ......................................................................
Patch Set 4:
(3 comments)
Patch Set 3:
We have CACHE_ROM_BASE/_SIZE (cpu/x86/mtrr.h) for such occasions. It's based on CONFIG_ROM_SIZE but I don't see how that would matter? We might mark a little too much as cacheable, but does it hurt if we never access those addresses?
https://review.coreboot.org/c/coreboot/+/39625/1/src/soc/intel/xeon_sp/bootb... File src/soc/intel/xeon_sp/bootblock/bootblock.c:
https://review.coreboot.org/c/coreboot/+/39625/1/src/soc/intel/xeon_sp/bootb... PS1, Line 33: .CodeRegionBase = (UINT32)(0x100000000ULL - CONFIG_CBFS_SIZE), : .CodeRegionLength = (UINT32)CONFIG_CBFS_SIZE,
With the same FSP after setting them to 0, it can boot to OS but sometimes it takes 7 minutes to boo […]
Done
https://review.coreboot.org/c/coreboot/+/39625/2/src/soc/intel/xeon_sp/bootb... File src/soc/intel/xeon_sp/bootblock/bootblock.c:
https://review.coreboot.org/c/coreboot/+/39625/2/src/soc/intel/xeon_sp/bootb... PS2, Line 34: .CodeRegionLength = (UINT32)CONFIG_BIOS_REGION_SIZE,
sounds like CACHE_ROM_SIZE will do the trick here. […]
Done
https://review.coreboot.org/c/coreboot/+/39625/3/src/soc/intel/xeon_sp/bootb... File src/soc/intel/xeon_sp/bootblock/bootblock.c:
https://review.coreboot.org/c/coreboot/+/39625/3/src/soc/intel/xeon_sp/bootb... PS3, Line 33: CONFIG_BIOS_REGION_SIZE
Is this value going to be used anywhere else? If it's only needed here, then please use a #define in […]
Done