Attention is currently required from: Andrey Petrov, Julius Werner, Jérémy Compostella, Martin L Roth, Nico Huber, Ronak Kanabar.
Maximilian Brune has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79905?usp=email )
Change subject: [RFC] region: Introduce region_create() functions ......................................................................
Patch Set 6:
(1 comment)
File src/commonlib/include/commonlib/region.h:
https://review.coreboot.org/c/coreboot/+/79905/comment/d321168c_bd8ffdf0 : PS1, Line 103: static inline int region_create_untrusted(
Well you have to repeat that for all sources that we deem "untrusted". […]
Our APIs (the untrusted part) should define the constraints of the parameter. I don't understand why that cannot be independent of the `region_create()`. Do we now need a `untrusted_create` for all our 'subsystems' now?
If I have an untrusted interface like SMM, I know the requirements for the parameters passed through this interface and can therefore verify them. If I pass the 'verified' parameters to trusted code like `region_create()`, I know both interfaces (the SMM interface and the region interface) and can make sure I only pass valid/sane parameters in there.
I think I am missing an actual example where the separation of the validity check and the `region_create()` can lead to issues.
-
Maximilian Brune (Code Review)