[coreboot-gerrit] Change in coreboot[master]: soc/intel/common/block: Add cache as ram init and teardown code

[Subrata Banik (Code Review)]

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/Kconfig
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/Makefile.inc
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/cache_as_ram.S
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


-- 
To view, visit https://review.coreboot.org/18381
To unsubscribe, visit https://review.coreboot.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iffd0c3e3ca81a3d283d5f1da115222a222e6b157
Gerrit-PatchSet: 36
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Owner: Aamir Bohra <aamir.bohra at intel.com>
Gerrit-Reviewer: Aaron Durbin <adurbin at chromium.org>
Gerrit-Reviewer: Balaji Manigandan <balaji.manigandan at intel.com>
Gerrit-Reviewer: Barnali Sarkar <barnali.sarkar at intel.com>
Gerrit-Reviewer: Hannah Williams <hannah.williams at intel.com>
Gerrit-Reviewer: Martin Roth <martinroth at google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter at users.sourceforge.net>
Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi at intel.com>
Gerrit-Reviewer: Subrata Banik <subrata.banik at intel.com>
Gerrit-Reviewer: build bot (Jenkins)
Gerrit-Reviewer: dhaval v sharma <dhaval.v.sharma at intel.com>
Gerrit-HasComments: Yes