Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40520 )
Change subject: drivers/smmstore: Implement SMMSTORE version 2 ......................................................................
Patch Set 21:
(4 comments)
https://review.coreboot.org/c/coreboot/+/40520/21/Documentation/drivers/smms... File Documentation/drivers/smmstorev2.md:
https://review.coreboot.org/c/coreboot/+/40520/21/Documentation/drivers/smms... PS21, Line 1: # SMM based flash storage driver Version 2 really outstanding job here
https://review.coreboot.org/c/coreboot/+/40520/21/src/drivers/smmstore/Kconf... File src/drivers/smmstore/Kconfig:
https://review.coreboot.org/c/coreboot/+/40520/21/src/drivers/smmstore/Kconf... PS21, Line 12: default y if PAYLOAD_TIANOCORE I feel that Y isn't the best default at the present time, since neither the default Tianocore package (my fork of CorebootPayloadPkg), nor upstream edk2 (the default for UefipayloadPkg) currently support it. And 9elements' current sample implementation fails to boot on older platforms. We should default to N until there is a reasonably-well working implementation that is selectable via Tianocore's Kconfig, and probably add 'depends on !TIANOCORE_COREBOOTPAYLOAD' as well to be safe
https://review.coreboot.org/c/coreboot/+/40520/21/src/drivers/smmstore/smi.c File src/drivers/smmstore/smi.c:
https://review.coreboot.org/c/coreboot/+/40520/21/src/drivers/smmstore/smi.c... PS21, Line 144: usually usually? are there conditions under which these are not true?
https://review.coreboot.org/c/coreboot/+/40520/21/src/include/smmstore.h File src/include/smmstore.h:
https://review.coreboot.org/c/coreboot/+/40520/21/src/include/smmstore.h@43 PS21, Line 43: * FIXME: The data format isn't specified. is this needed?