Attention is currently required from: Benjamin Doron, Elyes Haouas, Martin L Roth, Maximilian Brune, Patrick Rudolph, Sean Rhodes.
Angel Pons has posted comments on this change by Patrick Rudolph. ( https://review.coreboot.org/c/coreboot/+/77882?usp=email )
The change is no longer submittable: All-Comments-Resolved is unsatisfied now.
Change subject: include/commonlib/sot.h: Add SOT structs ......................................................................
Patch Set 14:
(8 comments)
File Documentation/drivers/sot.md:
https://review.coreboot.org/c/coreboot/+/77882/comment/2da7152c_6e612efe?usp... : PS14, Line 120: Ccontains ```suggestion Contains an expression `struct sot_expression` that needs to be evaluated in order to ```
https://review.coreboot.org/c/coreboot/+/77882/comment/661216f8_a493fdde?usp... : PS14, Line 122: `struct sot_expr` What is a `sot_expr`? It's not an expression but part of one. Would it make sense to call it `sot_atom` instead?
https://review.coreboot.org/c/coreboot/+/77882/comment/189cb628_de34c1c6?usp... : PS14, Line 140: EFI_VARIABLE_RUNTIME_ACCESS attribute. It is out of scope of this specification how non runtime : variables are protected after the payload has hand over control. nit:
```suggestion EFI_VARIABLE_RUNTIME_ACCESS attribute. It is out of scope of this specification how non runtime variables are protected after the payload has hand over control. ```
File src/commonlib/include/commonlib/sot.h:
https://review.coreboot.org/c/coreboot/+/77882/comment/2479344b_0a8ebfa5?usp... : PS14, Line 1: /* SPDX-License-Identifier: GPL-2.0-only */ Would it be possible to license this under BSD?
https://review.coreboot.org/c/coreboot/+/77882/comment/9084a0c4_57339f3a?usp... : PS14, Line 51: #define MAX_SOT_SIZE 5000 Units?
https://review.coreboot.org/c/coreboot/+/77882/comment/3bac48ba_6ccff7ef?usp... : PS14, Line 87: SOT_OPTFALG_HEX = 1 << 2, // SOT_TAG_OPTION_NUM_* options are displayed in hex. ```suggestion SOT_OPTFLAG_HEX = 1 << 2, // SOT_TAG_OPTION_NUM_* options are displayed in hex. ```
https://review.coreboot.org/c/coreboot/+/77882/comment/35b64bc5_be4fd1ea?usp... : PS14, Line 90: typedef uint64_t sot_string; // offset into strings array `sot_stridx` or `sot_strindex`?
https://review.coreboot.org/c/coreboot/+/77882/comment/acc9d177_74000174?usp... : PS14, Line 103: SOT_TAG_DEFAULT_VISIBLE, SOT_TAG_PROMPT_VISIBLE I don't see these values anywhere, is this a tag with flags?