Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40520 )
Change subject: drivers/smmstore: Implement SMMSTORE version 2 ......................................................................
Patch Set 15:
(15 comments)
https://review.coreboot.org/c/coreboot/+/40520/14/Documentation/drivers/smms... File Documentation/drivers/smmstorev2.md:
https://review.coreboot.org/c/coreboot/+/40520/14/Documentation/drivers/smms... PS14, Line 17: unformated
unformatted
Done
https://review.coreboot.org/c/coreboot/+/40520/14/Documentation/drivers/smms... PS14, Line 34: used
used,
Done
https://review.coreboot.org/c/coreboot/+/40520/14/Documentation/drivers/smms... PS14, Line 68: assuptions
assumptions
Done
https://review.coreboot.org/c/coreboot/+/40520/14/Documentation/drivers/smms... PS14, Line 82: on
in
Done
https://review.coreboot.org/c/coreboot/+/40520/14/Documentation/drivers/smms... PS14, Line 123: to write
writing
reading, but done
https://review.coreboot.org/c/coreboot/+/40520/14/Documentation/drivers/smms... PS14, Line 124: meanful
meaningful
Done
https://review.coreboot.org/c/coreboot/+/40520/14/Documentation/drivers/smms... PS14, Line 144: to write
writing
Done
https://review.coreboot.org/c/coreboot/+/40520/14/Documentation/drivers/smms... PS14, Line 165: to clear
clearing
Done
https://review.coreboot.org/c/coreboot/+/40520/14/src/commonlib/include/comm... File src/commonlib/include/commonlib/coreboot_tables.h:
https://review.coreboot.org/c/coreboot/+/40520/14/src/commonlib/include/comm... PS14, Line 82: LB_TAG_SMMSTOREV2 = 0x0038,
this will need to be adjusted via a rebase, since LB_TAG_PLATFORM_BLOB_VERSION is now 0x038
Done
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 cleare […]
Done
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 282: ASPM
APM
Done
https://review.coreboot.org/c/coreboot/+/40520/14/src/drivers/smmstore/store... PS14, Line 416: if (offset >= region_device_sz(&com_buf)) {
nit: blank line after if
Done
https://review.coreboot.org/c/coreboot/+/40520/14/src/drivers/smmstore/store... PS14, Line 425: ptr = rdev_mmap(&com_buf, offset, bufsize);
nit: blank line after if
Done
https://review.coreboot.org/c/coreboot/+/40520/14/src/drivers/smmstore/store... PS14, Line 430: ret = rdev_writeat(&store, ptr, 0, bufsize);
nit: blank line after if
Done
https://review.coreboot.org/c/coreboot/+/40520/14/src/drivers/smmstore/store... PS14, Line 450: // NOTE: Not really necessarry..
I think it's okay to be a little extra paranoid in SMM. […]
I added this check to all four functions.