On 23.05.2022 17:40, Daniel P. Smith wrote:
> --- a/xen/xsm/xsm_core.c
> +++ b/xen/xsm/xsm_core.c
> @@ -55,19 +55,35 @@ static enum xsm_bootparam __initdata xsm_bootparam =
> XSM_BOOTPARAM_DUMMY;
> #endif
>
> +static bool __initdata init_policy =
> +#ifdef CONFIG_XSM_FLASK_DEFAULT
> + true;
> +#else
> + false;
> +#endif
Simply IS_ENABLED(CONFIG_XSM_FLASK_DEFAULT) without any #ifdef-ary?
> @@ -148,11 +156,11 @@ int __init xsm_multiboot_init(
>
> printk("XSM Framework v" XSM_FRAMEWORK_VERSION " initialized\n");
>
> - if ( XSM_MAGIC )
> + if ( init_policy && XSM_MAGIC )
> {
> ret = xsm_multiboot_policy_init(module_map, mbi, &policy_buffer,
> &policy_size);
> - if ( ret )
> + if ( ret != 0 )
Nit: Stray change?
> @@ -36,10 +36,17 @@ int __init xsm_multiboot_policy_init(
> {
> int i;
> module_t *mod = (module_t *)__va(mbi->mods_addr);
> - int rc = 0;
> + int rc = -ENOENT;
I'm afraid I can't easily convince myself that this and the other
-ENOENT is really relevant to this change and/or not breaking
anything which currently does work (i.e. not entirely benign).
Please can you extend the description accordingly or split off
this adjustment?
> @@ -79,7 +87,16 @@ int __init xsm_dt_policy_init(void **policy_buffer, size_t
> *policy_size)
> paddr_t paddr, len;
>
> if ( !mod || !mod->size )
> +#ifdef CONFIG_XSM_FLASK_POLICY
> + {
> + *policy_buffer = (void *)xsm_flask_init_policy;
I don't think we want a cast here, especially not when it discards
"const". Instead the local variables' types want adjusting in
xsm_{multiboot,dt}_init() as well as the types of the respective
parameters of xsm_{multiboot,dt}_policy_init().
> + *policy_size = xsm_flask_init_policy_size;
> return 0;
> + }
> +#else
> + /* No policy was loaded */
> + return -ENOENT;
> +#endif
I think this is easier to read if you put the braces there
unconditionally and have the #if / #else inside. And if you wanted
to I think you could get away without any #else then.
Jan
