diff mbox series

[v4,07/11] xsm: decouple xsm header inclusion selection

Message ID 20210903190629.11917-8-dpsmith@apertussolutions.com (mailing list archive)
State Superseded
Headers show
Series xsm: refactoring xsm hooks | expand

Commit Message

Daniel P. Smith Sept. 3, 2021, 7:06 p.m. UTC
Multiple preprocessor defines were used as a mechanism to selective include
parts of the xsm.h header file. This makes it difficult to know which portion
is being included at any one time. This commit works to simplify this by
separating the core structures and functions of XSM into xsm-core.h away from
the wrapper functions which remain in xsm.h and dummy.h.

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
 xen/include/xsm/dummy.h    |   2 +-
 xen/include/xsm/xsm-core.h | 274 +++++++++++++++++++++++++++++++++++++
 xen/include/xsm/xsm.h      | 255 +---------------------------------
 xen/xsm/dummy.c            |   1 -
 xen/xsm/silo.c             |   1 -
 xen/xsm/xsm_core.c         |   5 +
 6 files changed, 281 insertions(+), 257 deletions(-)
 create mode 100644 xen/include/xsm/xsm-core.h

Comments

Andrew Cooper Sept. 6, 2021, 6:47 p.m. UTC | #1
On 03/09/2021 20:06, Daniel P. Smith wrote:
> diff --git a/xen/include/xsm/xsm-core.h b/xen/include/xsm/xsm-core.h
> new file mode 100644
> index 0000000000..4555e111dc
> --- /dev/null
> +++ b/xen/include/xsm/xsm-core.h
> @@ -0,0 +1,274 @@
> +/*
> + *  This file contains the XSM hook definitions for Xen.
> + *
> + *  This work is based on the LSM implementation in Linux 2.6.13.4.
> + *
> + *  Author:  George Coker, <gscoker@alpha.ncsc.mil>
> + *
> + *  Contributors: Michael LeMay, <mdlemay@epoch.ncsc.mil>
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License version 2,
> + *  as published by the Free Software Foundation.
> + */
> +
> +#ifndef __XSM_CORE_H__
> +#define __XSM_CORE_H__
> +
> +#include <xen/sched.h>
> +#include <xen/multiboot.h>
> +
> +/* policy magic number (defined by XSM_MAGIC) */
> +typedef uint32_t xsm_magic_t;
> +
> +#ifdef CONFIG_XSM_FLASK
> +#define XSM_MAGIC 0xf97cff8c
> +#else
> +#define XSM_MAGIC 0x0
> +#endif

Eww.  I know you're only moving code, but this construct is broken
(right from XSM's introduction in c/s d046f361dc937), and creates a
fairly-severe bug.

It causes xsm_multiboot_policy_init() to malfunction and accept a module
which starts with 4 zeroes, rather than the flask magic number.  The one
caller is suitably guarded so this is only a latent bug right now, but I
don't think we could credibly security support the code without this
being fixed.  (Again - fine to add to the todo list.  I know there's
loads to do)

> +
> +/* These annotations are used by callers and in dummy.h to document the
> + * default actions of XSM hooks. They should be compiled out otherwise.
> + */

For the coding style patch, this should be

/*
 * These ...

> +#ifdef CONFIG_XSM
> +
> +#ifdef CONFIG_MULTIBOOT
> +int xsm_multiboot_init(unsigned long *module_map,
> +                       const multiboot_info_t *mbi);
> +int xsm_multiboot_policy_init(unsigned long *module_map,
> +                              const multiboot_info_t *mbi,
> +                              void **policy_buffer,
> +                              size_t *policy_size);
> +#endif
> +
> +#ifdef CONFIG_HAS_DEVICE_TREE
> +/*
> + * Initialize XSM
> + *
> + * On success, return 1 if using SILO mode else 0.
> + */
> +int xsm_dt_init(void);
> +int xsm_dt_policy_init(void **policy_buffer, size_t *policy_size);
> +bool has_xsm_magic(paddr_t);
> +#endif
> +
> +#ifdef CONFIG_XSM_FLASK
> +const struct xsm_ops *flask_init(const void *policy_buffer,
> +                                 size_t policy_size);
> +#else
> +static inline const struct xsm_ops *flask_init(const void *policy_buffer,
> +                                               size_t policy_size)
> +{
> +    return NULL;
> +}
> +#endif
> +
> +#ifdef CONFIG_XSM_SILO
> +const struct xsm_ops *silo_init(void);
> +#else
> +static const inline struct xsm_ops *silo_init(void)
> +{
> +    return NULL;
> +}
> +#endif
> +
> +#else /* CONFIG_XSM */
> +
> +#ifdef CONFIG_MULTIBOOT
> +static inline int xsm_multiboot_init(unsigned long *module_map,
> +                                     const multiboot_info_t *mbi)
> +{
> +    return 0;
> +}
> +#endif
> +
> +#ifdef CONFIG_HAS_DEVICE_TREE
> +static inline int xsm_dt_init(void)
> +{
> +    return 0;
> +}
> +
> +static inline bool has_xsm_magic(paddr_t start)
> +{
> +    return false;
> +}
> +#endif /* CONFIG_HAS_DEVICE_TREE */

Shouldn't this be an #ifndef CONFIG_HAS_DEVICE_TREE ?

And the answer is no because of the #else /* CONFIG_XSM */ higher up,
but it is incredibly deceptive to read.


I think this logic would be far easier to follow as:

#if IS_ENABLED(CONFIG_XSM) && IS_ENABLED(CONFIG_MULTIBOOT)
...
#else
...
#endif

etc.

rather than having two separate #ifdef CONFIG_MULTIBOOT blocks doing
opposite things due to the position of intermixed #ifdef CONFIG_XSM.

~Andrew
Daniel P. Smith Sept. 7, 2021, 1:52 p.m. UTC | #2
On 9/6/21 2:47 PM, Andrew Cooper wrote:
> On 03/09/2021 20:06, Daniel P. Smith wrote:
>> diff --git a/xen/include/xsm/xsm-core.h b/xen/include/xsm/xsm-core.h
>> new file mode 100644
>> index 0000000000..4555e111dc
>> --- /dev/null
>> +++ b/xen/include/xsm/xsm-core.h
>> @@ -0,0 +1,274 @@
>> +/*
>> + *  This file contains the XSM hook definitions for Xen.
>> + *
>> + *  This work is based on the LSM implementation in Linux 2.6.13.4.
>> + *
>> + *  Author:  George Coker, <gscoker@alpha.ncsc.mil>
>> + *
>> + *  Contributors: Michael LeMay, <mdlemay@epoch.ncsc.mil>
>> + *
>> + *  This program is free software; you can redistribute it and/or modify
>> + *  it under the terms of the GNU General Public License version 2,
>> + *  as published by the Free Software Foundation.
>> + */
>> +
>> +#ifndef __XSM_CORE_H__
>> +#define __XSM_CORE_H__
>> +
>> +#include <xen/sched.h>
>> +#include <xen/multiboot.h>
>> +
>> +/* policy magic number (defined by XSM_MAGIC) */
>> +typedef uint32_t xsm_magic_t;
>> +
>> +#ifdef CONFIG_XSM_FLASK
>> +#define XSM_MAGIC 0xf97cff8c
>> +#else
>> +#define XSM_MAGIC 0x0
>> +#endif
> 
> Eww.  I know you're only moving code, but this construct is broken
> (right from XSM's introduction in c/s d046f361dc937), and creates a
> fairly-severe bug.
> 
> It causes xsm_multiboot_policy_init() to malfunction and accept a module
> which starts with 4 zeroes, rather than the flask magic number.  The one
> caller is suitably guarded so this is only a latent bug right now, but I
> don't think we could credibly security support the code without this
> being fixed.  (Again - fine to add to the todo list.  I know there's
> loads to do)

I cannot in good conscience leave a clearly latent bug. Let me see if 
can work a safer means to handling policy module loading.

>> +
>> +/* These annotations are used by callers and in dummy.h to document the
>> + * default actions of XSM hooks. They should be compiled out otherwise.
>> + */
> 
> For the coding style patch, this should be
> 
> /*
>   * These ...

Ack.

>> +#ifdef CONFIG_XSM
>> +
>> +#ifdef CONFIG_MULTIBOOT
>> +int xsm_multiboot_init(unsigned long *module_map,
>> +                       const multiboot_info_t *mbi);
>> +int xsm_multiboot_policy_init(unsigned long *module_map,
>> +                              const multiboot_info_t *mbi,
>> +                              void **policy_buffer,
>> +                              size_t *policy_size);
>> +#endif
>> +
>> +#ifdef CONFIG_HAS_DEVICE_TREE
>> +/*
>> + * Initialize XSM
>> + *
>> + * On success, return 1 if using SILO mode else 0.
>> + */
>> +int xsm_dt_init(void);
>> +int xsm_dt_policy_init(void **policy_buffer, size_t *policy_size);
>> +bool has_xsm_magic(paddr_t);
>> +#endif
>> +
>> +#ifdef CONFIG_XSM_FLASK
>> +const struct xsm_ops *flask_init(const void *policy_buffer,
>> +                                 size_t policy_size);
>> +#else
>> +static inline const struct xsm_ops *flask_init(const void *policy_buffer,
>> +                                               size_t policy_size)
>> +{
>> +    return NULL;
>> +}
>> +#endif
>> +
>> +#ifdef CONFIG_XSM_SILO
>> +const struct xsm_ops *silo_init(void);
>> +#else
>> +static const inline struct xsm_ops *silo_init(void)
>> +{
>> +    return NULL;
>> +}
>> +#endif
>> +
>> +#else /* CONFIG_XSM */
>> +
>> +#ifdef CONFIG_MULTIBOOT
>> +static inline int xsm_multiboot_init(unsigned long *module_map,
>> +                                     const multiboot_info_t *mbi)
>> +{
>> +    return 0;
>> +}
>> +#endif
>> +
>> +#ifdef CONFIG_HAS_DEVICE_TREE
>> +static inline int xsm_dt_init(void)
>> +{
>> +    return 0;
>> +}
>> +
>> +static inline bool has_xsm_magic(paddr_t start)
>> +{
>> +    return false;
>> +}
>> +#endif /* CONFIG_HAS_DEVICE_TREE */
> 
> Shouldn't this be an #ifndef CONFIG_HAS_DEVICE_TREE ?
> 
> And the answer is no because of the #else /* CONFIG_XSM */ higher up,
> but it is incredibly deceptive to read.
> 
> 
> I think this logic would be far easier to follow as:
> 
> #if IS_ENABLED(CONFIG_XSM) && IS_ENABLED(CONFIG_MULTIBOOT)
> ...
> #else
> ...
> #endif
> 
> etc.
> 
> rather than having two separate #ifdef CONFIG_MULTIBOOT blocks doing
> opposite things due to the position of intermixed #ifdef CONFIG_XSM.

Ack.

v/r
dps
diff mbox series

Patch

diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
index deaf23035e..78d0b7d07a 100644
--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -16,7 +16,7 @@ 
  */
 
 #include <xen/sched.h>
-#include <xsm/xsm.h>
+#include <xsm/xsm-core.h>
 #include <public/hvm/params.h>
 
 /* Cannot use BUILD_BUG_ON here because the expressions we check are not
diff --git a/xen/include/xsm/xsm-core.h b/xen/include/xsm/xsm-core.h
new file mode 100644
index 0000000000..4555e111dc
--- /dev/null
+++ b/xen/include/xsm/xsm-core.h
@@ -0,0 +1,274 @@ 
+/*
+ *  This file contains the XSM hook definitions for Xen.
+ *
+ *  This work is based on the LSM implementation in Linux 2.6.13.4.
+ *
+ *  Author:  George Coker, <gscoker@alpha.ncsc.mil>
+ *
+ *  Contributors: Michael LeMay, <mdlemay@epoch.ncsc.mil>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2,
+ *  as published by the Free Software Foundation.
+ */
+
+#ifndef __XSM_CORE_H__
+#define __XSM_CORE_H__
+
+#include <xen/sched.h>
+#include <xen/multiboot.h>
+
+/* policy magic number (defined by XSM_MAGIC) */
+typedef uint32_t xsm_magic_t;
+
+#ifdef CONFIG_XSM_FLASK
+#define XSM_MAGIC 0xf97cff8c
+#else
+#define XSM_MAGIC 0x0
+#endif
+
+/* These annotations are used by callers and in dummy.h to document the
+ * default actions of XSM hooks. They should be compiled out otherwise.
+ */
+enum xsm_default {
+    /* Guests can normally access the hypercall */
+    XSM_HOOK,
+    /* Device model can perform on its target domain */
+    XSM_DM_PRIV,
+    /* Can perform on self or your target domain */
+    XSM_TARGET,
+    /* Privileged - normally restricted to dom0 */
+    XSM_PRIV,
+    /* Xenstore domain - can do some privileged operations */
+    XSM_XS_PRIV,
+    /* Something more complex */
+    XSM_OTHER
+};
+typedef enum xsm_default xsm_default_t;
+
+struct xsm_ops {
+    void (*security_domaininfo)(struct domain *d,
+                                struct xen_domctl_getdomaininfo *info);
+    int (*domain_create)(struct domain *d, uint32_t ssidref);
+    int (*getdomaininfo)(struct domain *d);
+    int (*domctl_scheduler_op)(struct domain *d, int op);
+    int (*sysctl_scheduler_op)(int op);
+    int (*set_target)(struct domain *d, struct domain *e);
+    int (*domctl)(struct domain *d, int cmd);
+    int (*sysctl)(int cmd);
+    int (*readconsole)(uint32_t clear);
+
+    int (*evtchn_unbound)(struct domain *d, struct evtchn *chn, domid_t id2);
+    int (*evtchn_interdomain)(struct domain *d1, struct evtchn *chn1,
+                              struct domain *d2, struct evtchn *chn2);
+    void (*evtchn_close_post)(struct evtchn *chn);
+    int (*evtchn_send)(struct domain *d, struct evtchn *chn);
+    int (*evtchn_status)(struct domain *d, struct evtchn *chn);
+    int (*evtchn_reset)(struct domain *d1, struct domain *d2);
+
+    int (*grant_mapref)(struct domain *d1, struct domain *d2, uint32_t flags);
+    int (*grant_unmapref)(struct domain *d1, struct domain *d2);
+    int (*grant_setup)(struct domain *d1, struct domain *d2);
+    int (*grant_transfer)(struct domain *d1, struct domain *d2);
+    int (*grant_copy)(struct domain *d1, struct domain *d2);
+    int (*grant_query_size)(struct domain *d1, struct domain *d2);
+
+    int (*alloc_security_domain)(struct domain *d);
+    void (*free_security_domain)(struct domain *d);
+    int (*alloc_security_evtchns)(struct evtchn chn[], unsigned int nr);
+    void (*free_security_evtchns)(struct evtchn chn[], unsigned int nr);
+    char *(*show_security_evtchn)(struct domain *d, const struct evtchn *chn);
+    int (*init_hardware_domain)(struct domain *d);
+
+    int (*get_pod_target)(struct domain *d);
+    int (*set_pod_target)(struct domain *d);
+    int (*memory_exchange)(struct domain *d);
+    int (*memory_adjust_reservation)(struct domain *d1, struct domain *d2);
+    int (*memory_stat_reservation)(struct domain *d1, struct domain *d2);
+    int (*memory_pin_page)(struct domain *d1, struct domain *d2,
+                           struct page_info *page);
+    int (*add_to_physmap)(struct domain *d1, struct domain *d2);
+    int (*remove_from_physmap)(struct domain *d1, struct domain *d2);
+    int (*map_gmfn_foreign)(struct domain *d, struct domain *t);
+    int (*claim_pages)(struct domain *d);
+
+    int (*console_io)(struct domain *d, int cmd);
+
+    int (*profile)(struct domain *d, int op);
+
+    int (*kexec)(void);
+    int (*schedop_shutdown)(struct domain *d1, struct domain *d2);
+
+    char *(*show_irq_sid)(int irq);
+    int (*map_domain_pirq)(struct domain *d);
+    int (*map_domain_irq)(struct domain *d, int irq, const void *data);
+    int (*unmap_domain_pirq)(struct domain *d);
+    int (*unmap_domain_irq)(struct domain *d, int irq, const void *data);
+    int (*bind_pt_irq)(struct domain *d, struct xen_domctl_bind_pt_irq *bind);
+    int (*unbind_pt_irq)(struct domain *d, struct xen_domctl_bind_pt_irq *bind);
+    int (*irq_permission)(struct domain *d, int pirq, uint8_t allow);
+    int (*iomem_permission)(struct domain *d, uint64_t s, uint64_t e,
+                            uint8_t allow);
+    int (*iomem_mapping)(struct domain *d, uint64_t s, uint64_t e,
+                         uint8_t allow);
+    int (*pci_config_permission)(struct domain *d, uint32_t machine_bdf,
+                                 uint16_t start, uint16_t end, uint8_t access);
+
+#if defined(CONFIG_HAS_PASSTHROUGH) && defined(CONFIG_HAS_PCI)
+    int (*get_device_group)(uint32_t machine_bdf);
+    int (*assign_device)(struct domain *d, uint32_t machine_bdf);
+    int (*deassign_device)(struct domain *d, uint32_t machine_bdf);
+#endif
+
+#if defined(CONFIG_HAS_PASSTHROUGH) && defined(CONFIG_HAS_DEVICE_TREE)
+    int (*assign_dtdevice)(struct domain *d, const char *dtpath);
+    int (*deassign_dtdevice)(struct domain *d, const char *dtpath);
+#endif
+
+    int (*resource_plug_core)(void);
+    int (*resource_unplug_core)(void);
+    int (*resource_plug_pci)(uint32_t machine_bdf);
+    int (*resource_unplug_pci)(uint32_t machine_bdf);
+    int (*resource_setup_pci)(uint32_t machine_bdf);
+    int (*resource_setup_gsi)(int gsi);
+    int (*resource_setup_misc)(void);
+
+    int (*page_offline)(uint32_t cmd);
+    int (*hypfs_op)(void);
+
+    long (*do_xsm_op)(XEN_GUEST_HANDLE_PARAM(void) op);
+#ifdef CONFIG_COMPAT
+    int (*do_compat_op)(XEN_GUEST_HANDLE_PARAM(void) op);
+#endif
+
+    int (*hvm_param)(struct domain *d, unsigned long op);
+    int (*hvm_control)(struct domain *d, unsigned long op);
+    int (*hvm_param_altp2mhvm)(struct domain *d);
+    int (*hvm_altp2mhvm_op)(struct domain *d, uint64_t mode, uint32_t op);
+    int (*get_vnumainfo)(struct domain *d);
+
+    int (*vm_event_control)(struct domain *d, int mode, int op);
+
+#ifdef CONFIG_MEM_ACCESS
+    int (*mem_access)(struct domain *d);
+#endif
+
+#ifdef CONFIG_MEM_PAGING
+    int (*mem_paging)(struct domain *d);
+#endif
+
+#ifdef CONFIG_MEM_SHARING
+    int (*mem_sharing)(struct domain *d);
+#endif
+
+    int (*platform_op)(uint32_t cmd);
+
+#ifdef CONFIG_X86
+    int (*do_mca)(void);
+    int (*shadow_control)(struct domain *d, uint32_t op);
+    int (*mem_sharing_op)(struct domain *d, struct domain *cd, int op);
+    int (*apic)(struct domain *d, int cmd);
+    int (*memtype)(uint32_t access);
+    int (*machine_memory_map)(void);
+    int (*domain_memory_map)(struct domain *d);
+#define XSM_MMU_UPDATE_READ      1
+#define XSM_MMU_UPDATE_WRITE     2
+#define XSM_MMU_NORMAL_UPDATE    4
+#define XSM_MMU_MACHPHYS_UPDATE  8
+    int (*mmu_update)(struct domain *d, struct domain *t,
+                      struct domain *f, uint32_t flags);
+    int (*mmuext_op)(struct domain *d, struct domain *f);
+    int (*update_va_mapping)(struct domain *d, struct domain *f,
+                             l1_pgentry_t pte);
+    int (*priv_mapping)(struct domain *d, struct domain *t);
+    int (*ioport_permission)(struct domain *d, uint32_t s, uint32_t e,
+                             uint8_t allow);
+    int (*ioport_mapping)(struct domain *d, uint32_t s, uint32_t e,
+                          uint8_t allow);
+    int (*pmu_op)(struct domain *d, unsigned int op);
+#endif
+
+    int (*dm_op)(struct domain *d);
+
+    int (*xen_version)(uint32_t cmd);
+
+    int (*domain_resource_map)(struct domain *d);
+
+#ifdef CONFIG_ARGO
+    int (*argo_enable)(const struct domain *d);
+    int (*argo_register_single_source)(const struct domain *d,
+                                       const struct domain *t);
+    int (*argo_register_any_source)(const struct domain *d);
+    int (*argo_send)(const struct domain *d, const struct domain *t);
+#endif
+};
+
+void xsm_fixup_ops(struct xsm_ops *ops);
+
+#ifdef CONFIG_XSM
+
+#ifdef CONFIG_MULTIBOOT
+int xsm_multiboot_init(unsigned long *module_map,
+                       const multiboot_info_t *mbi);
+int xsm_multiboot_policy_init(unsigned long *module_map,
+                              const multiboot_info_t *mbi,
+                              void **policy_buffer,
+                              size_t *policy_size);
+#endif
+
+#ifdef CONFIG_HAS_DEVICE_TREE
+/*
+ * Initialize XSM
+ *
+ * On success, return 1 if using SILO mode else 0.
+ */
+int xsm_dt_init(void);
+int xsm_dt_policy_init(void **policy_buffer, size_t *policy_size);
+bool has_xsm_magic(paddr_t);
+#endif
+
+#ifdef CONFIG_XSM_FLASK
+const struct xsm_ops *flask_init(const void *policy_buffer,
+                                 size_t policy_size);
+#else
+static inline const struct xsm_ops *flask_init(const void *policy_buffer,
+                                               size_t policy_size)
+{
+    return NULL;
+}
+#endif
+
+#ifdef CONFIG_XSM_SILO
+const struct xsm_ops *silo_init(void);
+#else
+static const inline struct xsm_ops *silo_init(void)
+{
+    return NULL;
+}
+#endif
+
+#else /* CONFIG_XSM */
+
+#ifdef CONFIG_MULTIBOOT
+static inline int xsm_multiboot_init(unsigned long *module_map,
+                                     const multiboot_info_t *mbi)
+{
+    return 0;
+}
+#endif
+
+#ifdef CONFIG_HAS_DEVICE_TREE
+static inline int xsm_dt_init(void)
+{
+    return 0;
+}
+
+static inline bool has_xsm_magic(paddr_t start)
+{
+    return false;
+}
+#endif /* CONFIG_HAS_DEVICE_TREE */
+
+#endif /* CONFIG_XSM */
+
+#endif /* __XSM_CORE_H */
diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
index d7ef412874..1517443173 100644
--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -18,194 +18,12 @@ 
 #include <xen/alternative-call.h>
 #include <xen/sched.h>
 #include <xen/multiboot.h>
-
-/* policy magic number (defined by XSM_MAGIC) */
-typedef uint32_t xsm_magic_t;
-
-#ifdef CONFIG_XSM_FLASK
-#define XSM_MAGIC 0xf97cff8c
-#else
-#define XSM_MAGIC 0x0
-#endif
-
-/* These annotations are used by callers and in dummy.h to document the
- * default actions of XSM hooks. They should be compiled out otherwise.
- */
-enum xsm_default {
-    /* Guests can normally access the hypercall */
-    XSM_HOOK,
-    /* Device model can perform on its target domain */
-    XSM_DM_PRIV,
-    /* Can perform on self or your target domain */
-    XSM_TARGET,
-    /* Privileged - normally restricted to dom0 */
-    XSM_PRIV,
-    /* Xenstore domain - can do some privileged operations */
-    XSM_XS_PRIV,
-    /* Something more complex */
-    XSM_OTHER
-};
-typedef enum xsm_default xsm_default_t;
-
-struct xsm_ops {
-    void (*security_domaininfo)(struct domain *d,
-                                struct xen_domctl_getdomaininfo *info);
-    int (*domain_create)(struct domain *d, uint32_t ssidref);
-    int (*getdomaininfo)(struct domain *d);
-    int (*domctl_scheduler_op)(struct domain *d, int op);
-    int (*sysctl_scheduler_op)(int op);
-    int (*set_target)(struct domain *d, struct domain *e);
-    int (*domctl)(struct domain *d, int cmd);
-    int (*sysctl)(int cmd);
-    int (*readconsole)(uint32_t clear);
-
-    int (*evtchn_unbound)(struct domain *d, struct evtchn *chn, domid_t id2);
-    int (*evtchn_interdomain)(struct domain *d1, struct evtchn *chn1,
-                              struct domain *d2, struct evtchn *chn2);
-    void (*evtchn_close_post)(struct evtchn *chn);
-    int (*evtchn_send)(struct domain *d, struct evtchn *chn);
-    int (*evtchn_status)(struct domain *d, struct evtchn *chn);
-    int (*evtchn_reset)(struct domain *d1, struct domain *d2);
-
-    int (*grant_mapref)(struct domain *d1, struct domain *d2, uint32_t flags);
-    int (*grant_unmapref)(struct domain *d1, struct domain *d2);
-    int (*grant_setup)(struct domain *d1, struct domain *d2);
-    int (*grant_transfer)(struct domain *d1, struct domain *d2);
-    int (*grant_copy)(struct domain *d1, struct domain *d2);
-    int (*grant_query_size)(struct domain *d1, struct domain *d2);
-
-    int (*alloc_security_domain)(struct domain *d);
-    void (*free_security_domain)(struct domain *d);
-    int (*alloc_security_evtchns)(struct evtchn chn[], unsigned int nr);
-    void (*free_security_evtchns)(struct evtchn chn[], unsigned int nr);
-    char *(*show_security_evtchn)(struct domain *d, const struct evtchn *chn);
-    int (*init_hardware_domain)(struct domain *d);
-
-    int (*get_pod_target)(struct domain *d);
-    int (*set_pod_target)(struct domain *d);
-    int (*memory_exchange)(struct domain *d);
-    int (*memory_adjust_reservation)(struct domain *d1, struct domain *d2);
-    int (*memory_stat_reservation)(struct domain *d1, struct domain *d2);
-    int (*memory_pin_page)(struct domain *d1, struct domain *d2,
-                           struct page_info *page);
-    int (*add_to_physmap)(struct domain *d1, struct domain *d2);
-    int (*remove_from_physmap)(struct domain *d1, struct domain *d2);
-    int (*map_gmfn_foreign)(struct domain *d, struct domain *t);
-    int (*claim_pages)(struct domain *d);
-
-    int (*console_io)(struct domain *d, int cmd);
-
-    int (*profile)(struct domain *d, int op);
-
-    int (*kexec)(void);
-    int (*schedop_shutdown)(struct domain *d1, struct domain *d2);
-
-    char *(*show_irq_sid)(int irq);
-    int (*map_domain_pirq)(struct domain *d);
-    int (*map_domain_irq)(struct domain *d, int irq, const void *data);
-    int (*unmap_domain_pirq)(struct domain *d);
-    int (*unmap_domain_irq)(struct domain *d, int irq, const void *data);
-    int (*bind_pt_irq)(struct domain *d, struct xen_domctl_bind_pt_irq *bind);
-    int (*unbind_pt_irq)(struct domain *d, struct xen_domctl_bind_pt_irq *bind);
-    int (*irq_permission)(struct domain *d, int pirq, uint8_t allow);
-    int (*iomem_permission)(struct domain *d, uint64_t s, uint64_t e,
-                            uint8_t allow);
-    int (*iomem_mapping)(struct domain *d, uint64_t s, uint64_t e,
-                         uint8_t allow);
-    int (*pci_config_permission)(struct domain *d, uint32_t machine_bdf,
-                                 uint16_t start, uint16_t end, uint8_t access);
-
-#if defined(CONFIG_HAS_PASSTHROUGH) && defined(CONFIG_HAS_PCI)
-    int (*get_device_group)(uint32_t machine_bdf);
-    int (*assign_device)(struct domain *d, uint32_t machine_bdf);
-    int (*deassign_device)(struct domain *d, uint32_t machine_bdf);
-#endif
-
-#if defined(CONFIG_HAS_PASSTHROUGH) && defined(CONFIG_HAS_DEVICE_TREE)
-    int (*assign_dtdevice)(struct domain *d, const char *dtpath);
-    int (*deassign_dtdevice)(struct domain *d, const char *dtpath);
-#endif
-
-    int (*resource_plug_core)(void);
-    int (*resource_unplug_core)(void);
-    int (*resource_plug_pci)(uint32_t machine_bdf);
-    int (*resource_unplug_pci)(uint32_t machine_bdf);
-    int (*resource_setup_pci)(uint32_t machine_bdf);
-    int (*resource_setup_gsi)(int gsi);
-    int (*resource_setup_misc)(void);
-
-    int (*page_offline)(uint32_t cmd);
-    int (*hypfs_op)(void);
-
-    long (*do_xsm_op)(XEN_GUEST_HANDLE_PARAM(void) op);
-#ifdef CONFIG_COMPAT
-    int (*do_compat_op)(XEN_GUEST_HANDLE_PARAM(void) op);
-#endif
-
-    int (*hvm_param)(struct domain *d, unsigned long op);
-    int (*hvm_control)(struct domain *d, unsigned long op);
-    int (*hvm_param_altp2mhvm)(struct domain *d);
-    int (*hvm_altp2mhvm_op)(struct domain *d, uint64_t mode, uint32_t op);
-    int (*get_vnumainfo)(struct domain *d);
-
-    int (*vm_event_control)(struct domain *d, int mode, int op);
-
-#ifdef CONFIG_MEM_ACCESS
-    int (*mem_access)(struct domain *d);
-#endif
-
-#ifdef CONFIG_MEM_PAGING
-    int (*mem_paging)(struct domain *d);
-#endif
-
-#ifdef CONFIG_MEM_SHARING
-    int (*mem_sharing)(struct domain *d);
-#endif
-
-    int (*platform_op)(uint32_t cmd);
-
-#ifdef CONFIG_X86
-    int (*do_mca)(void);
-    int (*shadow_control)(struct domain *d, uint32_t op);
-    int (*mem_sharing_op)(struct domain *d, struct domain *cd, int op);
-    int (*apic)(struct domain *d, int cmd);
-    int (*memtype)(uint32_t access);
-    int (*machine_memory_map)(void);
-    int (*domain_memory_map)(struct domain *d);
-#define XSM_MMU_UPDATE_READ      1
-#define XSM_MMU_UPDATE_WRITE     2
-#define XSM_MMU_NORMAL_UPDATE    4
-#define XSM_MMU_MACHPHYS_UPDATE  8
-    int (*mmu_update)(struct domain *d, struct domain *t,
-                      struct domain *f, uint32_t flags);
-    int (*mmuext_op)(struct domain *d, struct domain *f);
-    int (*update_va_mapping)(struct domain *d, struct domain *f,
-                             l1_pgentry_t pte);
-    int (*priv_mapping)(struct domain *d, struct domain *t);
-    int (*ioport_permission)(struct domain *d, uint32_t s, uint32_t e,
-                             uint8_t allow);
-    int (*ioport_mapping)(struct domain *d, uint32_t s, uint32_t e,
-                          uint8_t allow);
-    int (*pmu_op)(struct domain *d, unsigned int op);
-#endif
-    int (*dm_op)(struct domain *d);
-    int (*xen_version)(uint32_t cmd);
-    int (*domain_resource_map)(struct domain *d);
-#ifdef CONFIG_ARGO
-    int (*argo_enable)(const struct domain *d);
-    int (*argo_register_single_source)(const struct domain *d,
-                                       const struct domain *t);
-    int (*argo_register_any_source)(const struct domain *d);
-    int (*argo_send)(const struct domain *d, const struct domain *t);
-#endif
-};
+#include <xsm/xsm-core.h>
 
 #ifdef CONFIG_XSM
 
 extern struct xsm_ops xsm_ops;
 
-#ifndef XSM_NO_WRAPPERS
-
 static inline void xsm_security_domaininfo(struct domain *d,
     struct xen_domctl_getdomaininfo *info)
 {
@@ -780,81 +598,10 @@  static inline int xsm_argo_send(const struct domain *d, const struct domain *t)
 
 #endif /* CONFIG_ARGO */
 
-#endif /* XSM_NO_WRAPPERS */
-
-#ifdef CONFIG_MULTIBOOT
-int xsm_multiboot_init(unsigned long *module_map,
-                              const multiboot_info_t *mbi);
-int xsm_multiboot_policy_init(unsigned long *module_map,
-                                     const multiboot_info_t *mbi,
-                                     void **policy_buffer,
-                                     size_t *policy_size);
-#endif
-
-#ifdef CONFIG_HAS_DEVICE_TREE
-/*
- * Initialize XSM
- *
- * On success, return 1 if using SILO mode else 0.
- */
-int xsm_dt_init(void);
-int xsm_dt_policy_init(void **policy_buffer, size_t *policy_size);
-bool has_xsm_magic(paddr_t);
-#endif
-
-extern struct xsm_ops dummy_xsm_ops;
-
-void xsm_fixup_ops(struct xsm_ops *ops);
-
-#ifdef CONFIG_XSM_FLASK
-extern const struct xsm_ops *flask_init(const void *policy_buffer,
-                                        size_t policy_size);
-#else
-static const inline struct xsm_ops *flask_init(const void *policy_buffer,
-                                               size_t policy_size)
-{
-    return NULL;
-}
-#endif
-
-#ifdef CONFIG_XSM_FLASK_POLICY
-extern const unsigned char xsm_flask_init_policy[];
-extern const unsigned int xsm_flask_init_policy_size;
-#endif
-
-#ifdef CONFIG_XSM_SILO
-extern const struct xsm_ops *silo_init(void);
-#else
-static const inline struct xsm_ops *silo_init(void)
-{
-    return NULL;
-}
-#endif
-
 #else /* CONFIG_XSM */
 
 #include <xsm/dummy.h>
 
-#ifdef CONFIG_MULTIBOOT
-static inline int xsm_multiboot_init (unsigned long *module_map,
-                                      const multiboot_info_t *mbi)
-{
-    return 0;
-}
-#endif
-
-#ifdef CONFIG_HAS_DEVICE_TREE
-static inline int xsm_dt_init(void)
-{
-    return 0;
-}
-
-static inline bool has_xsm_magic(paddr_t start)
-{
-    return false;
-}
-#endif /* CONFIG_HAS_DEVICE_TREE */
-
 #endif /* CONFIG_XSM */
 
 #endif /* __XSM_H */
diff --git a/xen/xsm/dummy.c b/xen/xsm/dummy.c
index d8c935328e..b848580eaa 100644
--- a/xen/xsm/dummy.c
+++ b/xen/xsm/dummy.c
@@ -10,7 +10,6 @@ 
  *  as published by the Free Software Foundation.
  */
 
-#define XSM_NO_WRAPPERS
 #include <xsm/dummy.h>
 
 #define set_to_dummy_if_null(ops, function)                            \
diff --git a/xen/xsm/silo.c b/xen/xsm/silo.c
index 3550dded7b..6db793f35c 100644
--- a/xen/xsm/silo.c
+++ b/xen/xsm/silo.c
@@ -17,7 +17,6 @@ 
  * You should have received a copy of the GNU General Public License along with
  * this program; If not, see <http://www.gnu.org/licenses/>.
  */
-#define XSM_NO_WRAPPERS
 #include <xsm/dummy.h>
 
 /*
diff --git a/xen/xsm/xsm_core.c b/xen/xsm/xsm_core.c
index 859af3fe9a..112d11fa6a 100644
--- a/xen/xsm/xsm_core.c
+++ b/xen/xsm/xsm_core.c
@@ -32,6 +32,11 @@ 
 
 struct xsm_ops __read_mostly xsm_ops;
 
+#ifdef CONFIG_XSM_FLASK_POLICY
+extern const unsigned char xsm_flask_init_policy[];
+extern const unsigned int xsm_flask_init_policy_size;
+#endif
+
 enum xsm_ops_state {
     XSM_OPS_UNREGISTERED,
     XSM_OPS_REG_FAILED,