Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40520 )
Change subject: [RFC]drivers/smmstore: Implement SMMSTORE version 2 ......................................................................
Patch Set 7:
(3 comments)
https://review.coreboot.org/c/coreboot/+/40520/7/src/drivers/smmstore/ramsta... File src/drivers/smmstore/ramstage.c:
https://review.coreboot.org/c/coreboot/+/40520/7/src/drivers/smmstore/ramsta... PS7, Line 73: BS_DEV_INIT
isn't this going to run before SMM is set up, leading init to fail?
No, SMM is set up in BS_DEV_INIT_CHIPS on Intel.
https://review.coreboot.org/c/coreboot/+/40520/7/src/drivers/smmstore/store.... File src/drivers/smmstore/store.c:
https://review.coreboot.org/c/coreboot/+/40520/7/src/drivers/smmstore/store.... PS7, Line 307: BIOS_WARNING
warning or error? I assume the latter since we're returning -1
Ack
https://review.coreboot.org/c/coreboot/+/40520/7/src/drivers/smmstore/store.... PS7, Line 315: printk(BIOS_ERR, "smm store: lookup of com buffer failed\n");
we should return -1 after this, otherwise function returns 0 and com_buffer is 0 since not initializ […]
If it's called from ramstage that's actually fine, as the caller knows the position and size of the com buffer. I'll guard this with if ENV_RAMSTAGE to make clear when to call it, in contrast to the other functions that should be called in SMM