Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37243 )
Change subject: Documentation: Add SMMSTORE documentation ......................................................................
Patch Set 2:
(10 comments)
https://review.coreboot.org/c/coreboot/+/37243/2/Documentation/drivers/smmst... File Documentation/drivers/smmstore.md:
https://review.coreboot.org/c/coreboot/+/37243/2/Documentation/drivers/smmst... PS2, Line 10: SMMTOR SMMSTORE
https://review.coreboot.org/c/coreboot/+/37243/2/Documentation/drivers/smmst... PS2, Line 16: I'd put a paragraph here that the API provides append-only semantics for key/value pairs.
https://review.coreboot.org/c/coreboot/+/37243/2/Documentation/drivers/smmst... PS2, Line 21: SMMTORE SMMSTORE
https://review.coreboot.org/c/coreboot/+/37243/2/Documentation/drivers/smmst... PS2, Line 34: lacks a way to update : key-value pairs "lacks a way to rewrite entries"?
https://review.coreboot.org/c/coreboot/+/37243/2/Documentation/drivers/smmst... PS2, Line 35: plural multiple
https://review.coreboot.org/c/coreboot/+/37243/2/Documentation/drivers/smmst... PS2, Line 40: The API : is called in the following way. Either end in a colon, or just drop the sentence, adapting the following to something like "%al must contain ..."
https://review.coreboot.org/c/coreboot/+/37243/2/Documentation/drivers/smmst... PS2, Line 49: If a non supported SMMSTORE command : is provided "For unsupported SMMSTORE commands"
https://review.coreboot.org/c/coreboot/+/37243/2/Documentation/drivers/smmst... PS2, Line 53: if `SMMSTORE_RET_SUCCESS` is not : returned. "if eax does not contain SMMSTORE_RET_SUCCESS"?
https://review.coreboot.org/c/coreboot/+/37243/2/Documentation/drivers/smmst... PS2, Line 56: If no SMMSTORE smihandler is installed generally nothing : is returned at all. If the calling argument on `%ax` is the same after : the SMI, it can generally be assumed that there is no smihandler for : the SMMSTORE call. Too many "generally"s ;-)
Maybe: "If the SMI returns without changing %ax, assume that the SMMSTORE feature is not installed."?
https://review.coreboot.org/c/coreboot/+/37243/2/Documentation/drivers/smmst... PS2, Line 77: void *buf; : ssize_t bufsize; note somewhere that these are 32bit?