[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] xsm: refactor and optimize policy loading


  • To: "Daniel P. Smith" <dpsmith@xxxxxxxxxxxxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Tue, 24 May 2022 17:50:55 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=b8ExjgSwA7dRiOPzPOEtjjhk7KUDe/c7dT4XCf5Fh3M=; b=Z0rXC/b6oe8zhsojtz/dcCmMCa2KakeGce9SNl04amzzB6VA2EJKLjdXCCu2aJytmlYE9t55m497LW8RjjNrGTL6iZkOYnrv+iuJ0bQiUZnAm6oeDsPizd5cRgcAyxVbbabMJ5lr5VPikBrqIT/kpVorRk2TbzET++4z32h3oXHhuTp/BeergGlgLZJRn7VTiXanrL3AoyPvXdnl0F5tCtbJwZ9dLuk4ek+mrBXQTO1eUnnY3sqNqyD3x1yIW8/tp0mEtxqejDR2v7GiQdhB+JE53bOYfA6tkKSF7K9vmdHl1vw8O+4aoNSyvLZ12xDmDbnfDO0vuaD0lL+mX7qANA==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=hBfAH8q7aUFjyTwMJpAStqeZThOvCcqtbLN6HOCA4sBcgh/LjUmhzhXiaubyEikNas0INzuu3EutklNsXfxN54oxXGJc6+cur1PqSlwYi2PrPDi3Ms6B4g/XRel6OM1QjJIsG6uUFNTg1gjT649lM7w2fcP1oHzS+FWsEF7O7ylGPSRFFqmBPsUnmYpO53RvQPpNLDzO811PfT9BDyLd4aGHHK8IhyEEa3FIhZw/fPrdVoagzLNwl3BKYq2RNhI7xPyOq4yDX0cIMsEnzc4NEYVeuLm5DaXkTxwUD7MdOPLT0PVAV6FjEqU4DJgMSsZST64n4QZHdMCN1MMUyrivFA==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
  • Cc: scott.davis@xxxxxxxxxx, jandryuk@xxxxxxxxx, christopher.clark@xxxxxxxxxx, Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
  • Delivery-date: Tue, 24 May 2022 15:51:19 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

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




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.