Attention is currently required from: Fred Reitberger, Jason Glenesk, Martin L Roth, Matt DeVillier, Nico Huber, Varshit Pandya, ritul guru.
Felix Held has posted comments on this change by Felix Held. ( https://review.coreboot.org/c/coreboot/+/83739?usp=email )
Change subject: soc/amd: add PSP SMI handler stub ......................................................................
Patch Set 2:
(3 comments)
File src/soc/amd/cezanne/smihandler.c:
https://review.coreboot.org/c/coreboot/+/83739/comment/4313c650_6f007900?usp... : PS2, Line 116: psp_smi_handler
Stubbing `psp_smi_handler()` in the header file once would avoid the `#if` […]
while i usually push for avoiding pre-processor usage if possible, but i'm not sure if having a smi handler stub would be the better option; right now i only include psp_smi.c in the build when this option is selected and having a separate file with just the stub doesn't seem like a too great idea. or do you mean having a stub in amdblocks/psp.h that gets preprocessored in in the !CONFIG(SOC_AMD_COMMON_BLOCK_PSP_SMI) case?
File src/soc/amd/common/block/psp/Kconfig:
https://review.coreboot.org/c/coreboot/+/83739/comment/07b07d1f_fcb13798?usp... : PS2, Line 108: SPI flash after the BOOT_DONE PSP command.
NB. Making this some kind of append-only and only erase during boot, […]
that would be something for the psp side to do differently. the current approach seems to be that rom armor is used where the psp owns the spi controller the flash is connected to and doesn't have to call the smi handler to do the flash accsses
File src/soc/amd/common/block/psp/psp_smm.c:
https://review.coreboot.org/c/coreboot/+/83739/comment/95167855_1ad37ffd?usp... : PS2, Line 94: configure_psp_smi();
Is it necessary to do this in SMM? […]
it's not necessary to do this in smm; however the next patch CB:83740 adds the enable_psp_smi call which needs to be done from smm, since it writes a bit in p2c_buffer which is inside the smm ram and having both in the same place is probably a good idea, right?