Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37243 )
Change subject: Documentation: Add SMMSTORE documentation ......................................................................
Patch Set 3:
(11 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
Done
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.
Done
https://review.coreboot.org/c/coreboot/+/37243/2/Documentation/drivers/smmst... PS2, Line 21: SMMTORE
SMMSTORE
Done
https://review.coreboot.org/c/coreboot/+/37243/2/Documentation/drivers/smmst... PS2, Line 35: plural
multiple
Done
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"?
Done
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 […]
Done
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"
Done
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"?
Done
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 ;-) […]
Done
https://review.coreboot.org/c/coreboot/+/37243/2/Documentation/drivers/smmst... PS2, Line 61: API
should probably be called calling arguments.
Done
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?
It was done below, but I moved it upwards and made it explicitly say that it's 32bits most of the time.