[coreboot-gerrit] [M] Change in coreboot[main]: soc/amd/common/block/psp_gen2: add get_psp_mmio_base

[Martin Roth (Code Review)]

Attention is currently required from: Felix Held, Fred Reitberger, Jason Glenesk, Matt DeVillier.

Martin Roth has posted comments on this change by Felix Held. ( https://review.coreboot.org/c/coreboot/+/83446?usp=email )

Change subject: soc/amd/common/block/psp_gen2: add get_psp_mmio_base
......................................................................

Patch Set 2:

(2 comments)

File src/soc/amd/common/block/psp/psp_gen2.c:

https://review.coreboot.org/c/coreboot/+/83446/comment/003e2ea8_4053cf69?usp... :
PS2, Line 32: form 
from

https://review.coreboot.org/c/coreboot/+/83446/comment/327d9f3c_2d06638c?usp... :
PS2, Line 62: /* Don't cache the PSP MMIO base if the register isn't locked */
I don't think this is right. We want to cache the value before the lock happens. If we wait until after it's locked, it could (in theory) be changed before we cache it.

I think we need to cache the value before the SMM register locking, and error out if the value isn't set when we access it after the locking happens.

We can also ignore all caching if we're not in SMM, right? Maybe put a check for being in the SMM component.

I think we should probably also compare the saved base value with what's being read from SMN, and error out if they're not the same.