Re: [PATCH v3 1/3] xsm: only search for a policy file when needed
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
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
>>>
>>> Looks technically okay, so
>>> Reviewed-by: Jan Beulich
>>> 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, _buffer,
_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(_buffer, _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
Re: [PATCH v3 1/3] xsm: only search for a policy file when needed
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
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
>>
>> Looks technically okay, so
>> Reviewed-by: Jan Beulich
>> 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, _buffer,
>>> _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(_buffer, _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
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
>
> Looks technically okay, so
> Reviewed-by: Jan Beulich
> 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, _buffer,
>> _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(_buffer, _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
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
Looks technically okay, so
Reviewed-by: Jan Beulich
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, _buffer,
> _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(_buffer, _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
[PATCH v3 1/3] xsm: only search for a policy file when needed
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
---
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, _buffer,
_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(_buffer, _size);
if ( ret )
--
2.20.1
