Attention is currently required from: Fred Reitberger, Jason Glenesk, Martin Roth, Matt DeVillier.
Felix Held 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:
(1 comment)
File src/soc/amd/common/block/psp/psp_gen2.c:
https://review.coreboot.org/c/coreboot/+/83446/comment/fb84876e_b2af1a42?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. […]
with caching i mean put it into a static variable so that we don't have to do the lookup of the psp mmio base address too many times. when the base address lock bit isn't set bit base address and enable bit are set, it's still valid to use, but we can't cache and reuse the base address the next time, since it might have changed when we have called into vendorcode in between. alternatively we can treat the psp mmio base address register not being locked as error case; i think when the register is written, the lock bit is also set. saving the value and then still comparing with the hardware register doesn't make any sense to me; that combines the added complexity with the higher performance impact
-
Felix Held (Code Review)