[coreboot-gerrit] Change in coreboot[master]: cpu/amd/pi: Split CAR setup for use in multiple stages

[Marshall Dawson (Code Review)]

Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/18437 )

Change subject: cpu/amd/pi: Split CAR setup for use in multiple stages
......................................................................


Patch Set 7:

(4 comments)

https://review.coreboot.org/#/c/18437/7/src/cpu/amd/pi/cache_as_ram.inc
File src/cpu/amd/pi/cache_as_ram.inc:

Line 33:  */
> I take these comments as a reservation of xmm register usage for the lifeti
> I take these comments as a reservation of xmm register usage for the lifetime of the entire CAR environment.

I don't read it the same way.  I believe this is a list of registers used until calling cache_as_ram_main().  In the new file, we don't care about the contents of these.  I've added a comment there regarding the need to have the instructions enabled.

We're anticipating major changes to this file later.  I would prefer to hold off on making unrelated clarifying comments in this patch.


Line 40:   post_code(0xa0)
> Not the scope of this patch but needs to be fixed, post_code() destroys %al
agree


Line 52:   shr $24, %ebx
> Why in assember? In cache_as_ram_main() you can just do 'cpuid_ebx(1) >> 24
We're anticipating major functional changes to this file later.


Line 144:  * no such thing as after RAM init. */
> As noted there's no fall through. You don't need to #include anything here.
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9fe53ca1fd6b1edd4d5e072849f3962c8bc95df0
Gerrit-PatchSet: 7
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Owner: Marc Jones <marc at marcjonesconsulting.com>
Gerrit-Reviewer: Aaron Durbin <adurbin at chromium.org>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki at gmail.com>
Gerrit-Reviewer: Marc Jones <marc at marcjonesconsulting.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd at gmail.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi at google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter at users.sourceforge.net>
Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki at gmail.com>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer at coreboot.org>
Gerrit-Reviewer: build bot (Jenkins)
Gerrit-HasComments: Yes