On 5/24/22 12:17, Jason Andryuk wrote: > On Mon, May 23, 2022 at 11:40 AM Daniel P. Smith > <[email protected]> wrote: >> >> It is possible to select a few different build configurations that results in >> the unnecessary walking of the boot module list looking for a policy module. >> This specifically occurs when the flask policy is enabled but either the >> dummy >> or the SILO policy is selected as the enforcing policy. This is not ideal for >> configurations like hyperlaunch and dom0less when there could be a number of >> modules to be walked or unnecessary device tree lookups >> >> This patch does two things, it moves all policy initialization logic under >> the >> xsm_XXXX_policy_init() functions and introduces the init_policy flag. The >> init_policy flag will be set based on which enforcing policy is selected and >> gates whether the boot modules should be checked for a policy file. > > I can see the use of init_policy to skip the search. (I'm not the > biggest fan of the name, need_policy/uses_policy/has_policy?, but > that's not a big deal). That part seems fine.
Yep, I went through that and several other variations trying to find the one that would "read" best at the places it was set and checked. If anyone has a strong stance for a preferred naming, I have no objections. > I don't care for the movement of `policy_buffer = > xsm_flask_init_policy;` since it replaces the single location with two > locations. I prefer leaving the built-in policy fallback in > xsm_core_init since it is multiboot/devicetree agnostic. i.e. the > boot-method specific code passes a policy if it finds one, and > xsm_core_init can fallback to the built-in policy if none is supplied. > Since a built-in policy is flask specific, it could potentially be > pushed down in flask_init. > > Is there a need for the xsm_flask_init_policy movement I am missing? I would be willing to bet that the de-duplication is the reason it was initially done this way. But as I explained in the response to Jan, doing so means that xsm_XXXX_policy_init() will have to return success even if it did not successfully initialize the policy buffer. I considered making a static inline function to point the policy buffer at the built-in policy, but quickly walked back from it. The reason being is that it is not any real logic duplications, just two lines of variable setting. IMHO a single repeat of setting a pair of variables is better than the bifurcating of the policy buffer initialization. v/r, dps
