Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40520 )
Change subject: drivers/smmstore: Implement SMMSTORE version 2 ......................................................................
Patch Set 14:
(4 comments)
https://review.coreboot.org/c/coreboot/+/40520/14/src/drivers/smmstore/ramst... File src/drivers/smmstore/ramstage.c:
https://review.coreboot.org/c/coreboot/+/40520/14/src/drivers/smmstore/ramst... PS14, Line 63: 0xb2 could use APM_CNT here but you'd have to pass another register like edx.
https://review.coreboot.org/c/coreboot/+/40520/14/src/drivers/smmstore/smi.c File src/drivers/smmstore/smi.c:
https://review.coreboot.org/c/coreboot/+/40520/14/src/drivers/smmstore/smi.c... PS14, Line 144: param the eventual range_check() seems like it will implicitly catch null pointers, but it might be clearer if there was a check at the start since the handlers seem to pass raw EBX values.
https://review.coreboot.org/c/coreboot/+/40520/14/src/drivers/smmstore/store... File src/drivers/smmstore/store.c:
https://review.coreboot.org/c/coreboot/+/40520/14/src/drivers/smmstore/store... PS14, Line 450: // NOTE: Not really necessarry.. agreed, none of the other functions check this so either they all should or none.
https://review.coreboot.org/c/coreboot/+/40520/14/src/include/smmstore.h File src/include/smmstore.h:
https://review.coreboot.org/c/coreboot/+/40520/14/src/include/smmstore.h@39 PS14, Line 39: SMMSTORE of at least 256KiB of size maybe add a compile time check of CONFIG_SMMSTORE_SIZE if CONFIG_SMMSTOREV2 is enabled.