[coreboot-gerrit] Change in coreboot[master]: amd/pi/00670F00: Add AMD PSP support for Stoney Ridge

[Marshall Dawson (Code Review)]

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

Change subject: amd/pi/00670F00: Add AMD PSP support for Stoney Ridge
......................................................................


Patch Set 1:

(3 comments)

https://review.coreboot.org/#/c/19157/1//COMMIT_MSG
Commit Message:

PS1, Line 11: Move  amd_intcpuio()
> This change affects all binaryPI platforms, while title would suggest only 
> This change affects all binaryPI platforms, while title would suggest only 00670f00 is affected with this commit.

I think there's value in incrementally developing PSP functionality, and starting with only Stoney.  So I'd like to keep that portion.  The only all-PI change is the order of amd_initcpuio().  Do you have a preference?  Would you prefer to see that split... the commit message clarified...?

> I recall some requirements that PSP mailboxes have to be in SMM space, does it have relevance here or in future PSP communications?

To be added later.  Once the end-of-POST command is sent (not implemented yet), the PSP will only accept mailbox commands where the buffer sits in SMM memory.  And in order to support S3, we'll need to trap on writes to Pm1Control so that we can tell PSP we're going to S3.  There's also a PSP->BIOS interface yet to be implemented.


PS1, Line 11: Move  amd_intcpuio()
> Remove extra space and fix spelling of the function.
Done


PS1, Line 9: Add PSP communication support that is not covered by AGESA.
           : The PSP must be notified DRAM is ready after amdinitpost().
           : Move  amd_intcpuio() to the end of amdinitpost() so that the PSP
           : decode for notify works.
> If it’s a list/enumeration, please format it like that.
Can you clarify this?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icf111470b261d6516b878e88be471967abe681b3
Gerrit-PatchSet: 1
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: Marshall Dawson <marshalldawson3rd at gmail.com>
Gerrit-Reviewer: Martin Roth <martinroth at google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter at users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins)
Gerrit-HasComments: Yes