Re: [PATCH v3 1/3] xsm: only search for a policy file when needed

["Daniel P. Smith" <dpsmith@xxxxxxxxxxxxxxxxxxxx>]

On 6/1/22 02:04, Jan Beulich wrote:
> On 31.05.2022 18:15, Daniel P. Smith wrote:
>>
>> On 5/31/22 11:51, Jan Beulich wrote:
>>> On 31.05.2022 17:08, Daniel P. Smith 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 doing an unnecessary device tree lookup.
>>>>
>>>> This patch introduces the policy_file_required flag for tracking when an 
>>>> XSM
>>>> policy module requires a policy file. Only when the policy_file_required 
>>>> flag
>>>> is set to true, will XSM search the boot modules for a policy file.
>>>>
>>>> Signed-off-by: Daniel P. Smith <dpsmith@xxxxxxxxxxxxxxxxxxxx>
>>>
>>> Looks technically okay, so
>>> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
>>> but couldn't you ...
>>>
>>>> @@ -148,7 +160,7 @@ int __init xsm_multiboot_init(
>>>>  
>>>>      printk("XSM Framework v" XSM_FRAMEWORK_VERSION " initialized\n");
>>>>  
>>>> -    if ( XSM_MAGIC )
>>>> +    if ( policy_file_required && XSM_MAGIC )
>>>>      {
>>>>          ret = xsm_multiboot_policy_init(module_map, mbi, &policy_buffer,
>>>>                                          &policy_size);
>>>> @@ -176,7 +188,7 @@ int __init xsm_dt_init(void)
>>>>  
>>>>      printk("XSM Framework v" XSM_FRAMEWORK_VERSION " initialized\n");
>>>>  
>>>> -    if ( XSM_MAGIC )
>>>> +    if ( policy_file_required && XSM_MAGIC )
>>>>      {
>>>>          ret = xsm_dt_policy_init(&policy_buffer, &policy_size);
>>>>          if ( ret )
>>>
>>> ... drop the two "&& XSM_MAGIC" here at this time? Afaict 
>>> policy_file_required
>>> cannot be true when XSM_MAGIC is zero.
>>
>> I was on the fence about this, as it should be rendered as redundant as
>> you point out. I am good with dropping on next spin.
> 
> I'd also be okay dropping this while committing, unless a v4 appears
> first ...

ack

v/r,
dps