1. Patchew
  2. Xen
  3. [PATCH v3 0/3] xsm: refactor and optimize policy loading
  4. View patch

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

Daniel P. Smith posted 3 patches 3 years, 8 months ago
There is a newer version of this series
[PATCH v3 1/3] xsm: only search for a policy file when needed
Posted by Daniel P. Smith 3 years, 8 months ago 
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@apertussolutions.com>
---
 xen/xsm/xsm_core.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/xen/xsm/xsm_core.c b/xen/xsm/xsm_core.c
index 2286a502e3..4a29ee9558 100644
--- a/xen/xsm/xsm_core.c
+++ b/xen/xsm/xsm_core.c
@@ -55,19 +55,31 @@ static enum xsm_bootparam __initdata xsm_bootparam =
     XSM_BOOTPARAM_DUMMY;
 #endif
 
+static bool __initdata policy_file_required =
+    IS_ENABLED(CONFIG_XSM_FLASK_DEFAULT);
+
 static int __init cf_check parse_xsm_param(const char *s)
 {
     int rc = 0;
 
     if ( !strcmp(s, "dummy") )
+    {
         xsm_bootparam = XSM_BOOTPARAM_DUMMY;
+        policy_file_required = false;
+    }
 #ifdef CONFIG_XSM_FLASK
     else if ( !strcmp(s, "flask") )
+    {
         xsm_bootparam = XSM_BOOTPARAM_FLASK;
+        policy_file_required = true;
+    }
 #endif
 #ifdef CONFIG_XSM_SILO
     else if ( !strcmp(s, "silo") )
+    {
         xsm_bootparam = XSM_BOOTPARAM_SILO;
+        policy_file_required = false;
+    }
 #endif
     else
         rc = -EINVAL;
@@ -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 )
-- 
2.20.1
Re: [PATCH v3 1/3] xsm: only search for a policy file when needed
Posted by Jan Beulich 3 years, 8 months ago 
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.

In light of the "flask=late" aspect of patch 2, I'd like to suggest to
slightly alter wording here: "... requires looking for a policy file."

> --- a/xen/xsm/xsm_core.c
> +++ b/xen/xsm/xsm_core.c
> @@ -55,19 +55,31 @@ static enum xsm_bootparam __initdata xsm_bootparam =
>      XSM_BOOTPARAM_DUMMY;
>  #endif
>  
> +static bool __initdata policy_file_required =
> +    IS_ENABLED(CONFIG_XSM_FLASK_DEFAULT);

The variable may then also want renaming, to e.g. "find_policy_file".

Jan
Re: [PATCH v3 1/3] xsm: only search for a policy file when needed
Posted by Daniel P. Smith 3 years, 8 months ago 
On 6/1/22 02:08, 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.
> 
> In light of the "flask=late" aspect of patch 2, I'd like to suggest to
> slightly alter wording here: "... requires looking for a policy file."

ack

>> --- a/xen/xsm/xsm_core.c
>> +++ b/xen/xsm/xsm_core.c
>> @@ -55,19 +55,31 @@ static enum xsm_bootparam __initdata xsm_bootparam =
>>      XSM_BOOTPARAM_DUMMY;
>>  #endif
>>  
>> +static bool __initdata policy_file_required =
>> +    IS_ENABLED(CONFIG_XSM_FLASK_DEFAULT);
> 
> The variable may then also want renaming, to e.g. "find_policy_file".

ack

v/r,
dps
Re: [PATCH v3 1/3] xsm: only search for a policy file when needed
Posted by Jan Beulich 3 years, 8 months ago 
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@apertussolutions.com>

Looks technically okay, so
Reviewed-by: Jan Beulich <jbeulich@suse.com>
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.

Jan
Re: [PATCH v3 1/3] xsm: only search for a policy file when needed
Posted by Daniel P. Smith 3 years, 8 months ago 
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@apertussolutions.com>
> 
> Looks technically okay, so
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> 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.

v/r,
dps
Re: [PATCH v3 1/3] xsm: only search for a policy file when needed
Posted by Jan Beulich 3 years, 8 months ago 
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@apertussolutions.com>
>>
>> Looks technically okay, so
>> Reviewed-by: Jan Beulich <jbeulich@suse.com>
>> 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 ...

Jan
Re: [PATCH v3 1/3] xsm: only search for a policy file when needed
Posted by Daniel P. Smith 3 years, 8 months ago 
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@apertussolutions.com>
>>>
>>> Looks technically okay, so
>>> Reviewed-by: Jan Beulich <jbeulich@suse.com>
>>> 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

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.