On 03/09/2021 20:06, Daniel P. Smith wrote:
> This renames the `struct xsm_operations` to the shorter `struct xsm_ops` and
> converts the global xsm_ops from being a pointer to an explicit instance. As
> part of this conversion, it reworks the XSM modules init function to return
> their xsm_ops struct which is copied in to the global xsm_ops instance.
>
> Signed-off-by: Daniel P. Smith <dpsm...@apertussolutions.com>
Acked-by: Andrew Cooper <andrew.coop...@citrix.com>
However, some suggestions...
> diff --git a/xen/xsm/xsm_core.c b/xen/xsm/xsm_core.c
> index 55483292c5..859af3fe9a 100644
> --- a/xen/xsm/xsm_core.c
> +++ b/xen/xsm/xsm_core.c
> @@ -28,9 +28,17 @@
> #include <asm/setup.h>
> #endif
>
> -#define XSM_FRAMEWORK_VERSION "1.0.0"
> +#define XSM_FRAMEWORK_VERSION "1.0.1"
>
> -struct xsm_operations *xsm_ops;
> +struct xsm_ops __read_mostly xsm_ops;
> +
> +enum xsm_ops_state {
> + XSM_OPS_UNREGISTERED,
> + XSM_OPS_REG_FAILED,
> + XSM_OPS_REGISTERED,
> +};
> +
> +static enum xsm_ops_state xsm_ops_registered = XSM_OPS_UNREGISTERED;
__read_mostly, or can this be __initdata ?
> @@ -87,25 +88,35 @@ static int __init xsm_core_init(const void
> *policy_buffer, size_t policy_size)
> }
> #endif
>
> - if ( verify(&dummy_xsm_ops) )
> + if ( xsm_ops_registered != XSM_OPS_UNREGISTERED )
> {
> - printk(XENLOG_ERR "Could not verify dummy_xsm_ops structure\n");
> + printk(XENLOG_ERR
> + "Could not init XSM, xsm_ops register already attempted\n");
> return -EIO;
> }
>
> - xsm_ops = &dummy_xsm_ops;
> -
> switch ( xsm_bootparam )
> {
> case XSM_BOOTPARAM_DUMMY:
> + xsm_ops_registered = XSM_OPS_REGISTERED;
> break;
>
> case XSM_BOOTPARAM_FLASK:
> - flask_init(policy_buffer, policy_size);
> + ops = flask_init(policy_buffer, policy_size);
> + if ( ops )
> + {
> + xsm_ops_registered = XSM_OPS_REGISTERED;
> + xsm_ops = *ops;
> + }
> break;
>
> case XSM_BOOTPARAM_SILO:
> - silo_init();
> + ops = silo_init();
> + if ( ops )
> + {
> + xsm_ops_registered = XSM_OPS_REGISTERED;
> + xsm_ops = *ops;
> + }
This if ( ops ) block can be deduplicated by moving after the switch()
statement. It's going to be common to all everything except dummy.
~Andrew
