Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40520 )
Change subject: drivers/smmstore: Implement SMMSTORE version 2 ......................................................................
Patch Set 14:
(13 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
https://review.coreboot.org/c/coreboot/+/40520/14/Documentation/drivers/smms... PS14, Line 34: used used,
https://review.coreboot.org/c/coreboot/+/40520/14/Documentation/drivers/smms... PS14, Line 68: assuptions assumptions
https://review.coreboot.org/c/coreboot/+/40520/14/Documentation/drivers/smms... PS14, Line 82: on in
https://review.coreboot.org/c/coreboot/+/40520/14/Documentation/drivers/smms... PS14, Line 123: to write writing
https://review.coreboot.org/c/coreboot/+/40520/14/Documentation/drivers/smms... PS14, Line 124: meanful meaningful
https://review.coreboot.org/c/coreboot/+/40520/14/Documentation/drivers/smms... PS14, Line 144: to write writing
https://review.coreboot.org/c/coreboot/+/40520/14/Documentation/drivers/smms... PS14, Line 165: to clear clearing
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
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
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
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
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.
I think it's okay to be a little extra paranoid in SMM. Maybe all of the exported functions should check for store_initialized?