Change in coreboot[master]: soc/intel/common/block: Add cache as ram init and teardown code
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/18381 )
Change subject: soc/intel/common/block: Add cache as ram init and teardown code ......................................................................
Patch Set 36:
(12 comments)
https://review.coreboot.org/#/c/18381/36//COMMIT_MSG Commit Message:
PS36, Line 11: ENHACED
ENHANCED?
Done
Line 13: by reading MSR.
Where can these configurations be found?
Paul, those registers are part of x86 MTRR software volume. Normally we do read MSR 0x200-0x201 & 0x202-0x203, right after CAR init done through ITP or any debug mechanism to ensure we have correct range programmed.
More over if this line looks nasty "ensure to have CAR done by reading MSR." then i will remove this.
https://review.coreboot.org/#/c/18381/36/src/soc/intel/common/block/cpu/Kcon... File src/soc/intel/common/block/cpu/Kconfig:
PS36, Line 13: mode
remove "mode" so we're not saying "non-evict mode mode"
Done
PS36, Line 13: block
blocked
Done
PS36, Line 20: set up portion
set up a portion
Done
PS36, Line 21: use remainder
use the remainder
Done
PS36, Line 27: Current limitation NEM mode is that code and data size is derive : from the need not to spill out any modified line: as in NEM mode : there is no memory behind, the modified data will be lost and NEM : results will be inconsistent, hence NEM ENHANCED mode ensure to : have, some “magic” way to guarantee that modified data is always : kept in cache while clean data is replaced.
Maybe:
thanks for rephrasing it well
https://review.coreboot.org/#/c/18381/36/src/soc/intel/common/block/cpu/Make... File src/soc/intel/common/block/cpu/Makefile.inc:
Line 4:
Please remove the blank line at the end of the file.
Done
https://review.coreboot.org/#/c/18381/36/src/soc/intel/common/block/cpu/car/... File src/soc/intel/common/block/cpu/car/cache_as_ram.S:
PS36, Line 108: /* Configure CAR region as write-back (WB) */ : mov $MTRR_PHYS_BASE(0), %ecx : mov $CONFIG_DCACHE_RAM_BASE, %eax : or $MTRR_TYPE_WRBACK, %eax : xor %edx,%edx :
indent with tabs
Done
PS36, Line 124: /* Configure CAR region as write-back (WB) */ : mov $MTRR_PHYS_BASE(0), %ecx : mov $CONFIG_DCACHE_RAM_BASE, %eax : or $MTRR_TYPE_WRBACK, %eax : xor %edx,%edx : wrmsr
indent with tabs
Done
https://review.coreboot.org/#/c/18381/36/src/soc/intel/skylake/Kconfig File src/soc/intel/skylake/Kconfig:
PS36, Line 269: Current limitation NEM mode is that code and data size is derive : from the need not to spill out any modified line: as in NEM mode : there is no memory behind, the modified data will be lost and NEM : results will be inconsistent, hence NEM ENHANCED mode ensure to : have, some “magic” way to guarantee that modified data is always : kept in cache while clean data is replaced.
see previous comment
Done
PS36, Line 280: Tear Down
lowercase - tear down
Done
-
Subrata Banik (Code Review)