| Message ID | 20220531150857.19727-2-dpsmith@apertussolutions.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xsm: refactor and optimize policy loading | expand |
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
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
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
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
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
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
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 )
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(-)