Re: [PATCH 18/24] XSM: make XSM hooks well-formed ones

["Daniel P. Smith" <dpsmith@xxxxxxxxxxxxxxxxxxxx>]

On 7/28/26 9:22 AM, Jan Beulich wrote:
> For whatever reason they didn't have an xsm_default_t first argument (to
> cope with XSM=n mode), making it impossible to (easily) cover them in
> xsm/hooks.h.
>
> flask_do_xsm_op() is also changed to return int, as all the function ever
> returns is an int. This way no new machinery needs adding to xsm/hooks.h.
> Instead one piece of compat machinery can then be dropped from
> xsm/flask/flask_op.c.
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> ---
> Instead of XSM_HOOK, using XSM_OTHER may also be a sensible option here.
I would say XSM_HOOK is sufficient since at this point it doesn't matter 
what op is passed the dummy policy is just going to return the same 
value. If the dummy policy is extended to support an op, then at that 
point the implementer can switch it to XSM_OTHER.
> Question is whether some/all of the other hooks still declared explicitly
> in struct xsm_ops should follow suit.
I would say either comment these are exceptions or make them consistent, 
though I haven't studied if there would be any consequences (doubtful) 
changing the others.
> --- a/xen/include/xsm/dummy.h
> +++ b/xen/include/xsm/dummy.h
> @@ -439,7 +439,7 @@ static XSM_INLINE int xsm_hypfs_op(XSM_D
>   
>   #ifdef CONFIG_XSM
>   
> -static XSM_INLINE long xsm_do_xsm_op(XEN_GUEST_HANDLE_PARAM(void) op)
> +static XSM_INLINE int xsm_do_xsm_op(XEN_GUEST_HANDLE_PARAM(void) op)
With this change, should there be a comment that the int is going to get 
cast to long before return?
>   {
>       return -ENOSYS;
>   }
> --- a/xen/include/xsm/hooks.h
> +++ b/xen/include/xsm/hooks.h
> @@ -156,6 +156,11 @@ XSM_HOOK(int, dm_op, struct domain *)
>   XSM_HOOK(int, xen_version, uint32_t)
>   XSM_HOOK(int, domain_resource_map, struct domain *)
>   
> +XSM_HOOK(int, do_xsm_op, XEN_GUEST_HANDLE_PARAM(void))
> +#ifdef CONFIG_COMPAT
> +XSM_HOOK(int, do_compat_op, XEN_GUEST_HANDLE_PARAM(void))
> +#endif
> +
>   #ifdef CONFIG_ARGO
>   XSM_HOOK(int, argo_enable, const struct domain *)
>   XSM_HOOK(int, argo_register_single_source, const struct domain *,
> --- a/xen/include/xsm/xsm.h
> +++ b/xen/include/xsm/xsm.h
> @@ -85,11 +85,6 @@ struct xsm_ops {
>       char *(*show_security_evtchn)(struct domain *d, const struct evtchn *chn);
>   
>       char *(*show_irq_sid)(int irq);
> -
> -    long (*do_xsm_op)(XEN_GUEST_HANDLE_PARAM(void) op);
> -#ifdef CONFIG_COMPAT
> -    int (*do_compat_op)(XEN_GUEST_HANDLE_PARAM(void) op);
> -#endif
>   };
>   
>   #ifdef CONFIG_XSM
> @@ -193,18 +188,6 @@ static inline char *xsm_show_irq_sid(int
>       return alternative_call(xsm_ops.show_irq_sid, irq);
>   }
>   
> -static inline long xsm_do_xsm_op(XEN_GUEST_HANDLE_PARAM(void) op)
> -{
> -    return alternative_call(xsm_ops.do_xsm_op, op);
> -}
> -
> -#ifdef CONFIG_COMPAT
> -static inline int xsm_do_compat_op(XEN_GUEST_HANDLE_PARAM(void) op)
> -{
> -    return alternative_call(xsm_ops.do_compat_op, op);
> -}
> -#endif
> -
>   #endif /* XSM_NO_WRAPPERS */
>   
>   #ifdef CONFIG_MULTIBOOT
> --- a/xen/xsm/dummy.c
> +++ b/xen/xsm/dummy.c
> @@ -35,11 +35,6 @@ static const struct xsm_ops __initconst_
>       .show_security_evtchn          = xsm_show_security_evtchn,
>   
>       .show_irq_sid                  = xsm_show_irq_sid,
> -
> -    .do_xsm_op                     = xsm_do_xsm_op,
> -#ifdef CONFIG_COMPAT
> -    .do_compat_op                  = xsm_do_compat_op,
> -#endif
>   };
>   
>   void __init xsm_fixup_ops(struct xsm_ops *ops)
> --- a/xen/xsm/flask/flask_op.c
> +++ b/xen/xsm/flask/flask_op.c
> @@ -23,7 +23,6 @@
>   #include <conditional.h>
>   #include "private.h"
>   
> -#define ret_t long
>   #define _copy_to_guest copy_to_guest
>   #define _copy_from_guest copy_from_guest
>   
> @@ -606,7 +605,7 @@ static int flask_relabel_domain(const st
>   
>   #endif /* !COMPAT */
>   
> -ret_t cf_check do_flask_op(XEN_GUEST_HANDLE_PARAM(void) u_flask_op)
> +int cf_check flask_do_xsm_op(XEN_GUEST_HANDLE_PARAM(void) u_flask_op)
>   {
>       xen_flask_op_t op;
>       int rv;
> @@ -772,9 +771,7 @@ CHECK_flask_transition;
>   #define flask_devicetree_label compat_devicetree_label
>   
>   #define xen_flask_op_t compat_flask_op_t
> -#undef ret_t
> -#define ret_t int
> -#define do_flask_op compat_flask_op
> +#define flask_do_xsm_op flask_do_compat_op
>   
>   #include "flask_op.c"
>   #endif
> --- a/xen/xsm/flask/hooks.c
> +++ b/xen/xsm/flask/hooks.c
> @@ -1971,12 +1971,6 @@ static const struct xsm_ops __initconst_
>       .show_security_evtchn = flask_show_security_evtchn,
>   
>       .show_irq_sid = flask_show_irq_sid,
> -
> -    .do_xsm_op = do_flask_op,
> -
> -#ifdef CONFIG_COMPAT
> -    .do_compat_op = compat_flask_op,
> -#endif
>   };
>   
>   const struct xsm_ops *__init flask_init(
> --- a/xen/xsm/flask/private.h
> +++ b/xen/xsm/flask/private.h
> @@ -3,7 +3,7 @@
>   
>   #include <public/xen.h>
>   
> -long cf_check do_flask_op(XEN_GUEST_HANDLE_PARAM(void) u_flask_op);
> -int cf_check compat_flask_op(XEN_GUEST_HANDLE_PARAM(void) u_flask_op);
> +int cf_check flask_do_xsm_op(XEN_GUEST_HANDLE_PARAM(void) u_flask_op);
> +int cf_check flask_do_compat_op(XEN_GUEST_HANDLE_PARAM(void) u_flask_op);
>   
>   #endif /* XSM_FLASK_PRIVATE */
> --- a/xen/xsm/xsm_core.c
> +++ b/xen/xsm/xsm_core.c
> @@ -216,12 +216,12 @@ bool __init has_xsm_magic(paddr_t start)
>   
>   long do_xsm_op(XEN_GUEST_HANDLE_PARAM(void) op)
>   {
> -    return xsm_do_xsm_op(op);
> +    return xsm_do_xsm_op(XSM_HOOK, op);
>   }
>   
>   #ifdef CONFIG_COMPAT
>   int compat_xsm_op(XEN_GUEST_HANDLE_PARAM(void) op)
>   {
> -    return xsm_do_compat_op(op);
> +    return xsm_do_compat_op(XSM_HOOK, op);
>   }
>   #endif
Acked-by: Daniel P. Smith <dpsmith@xxxxxxxxxxxxxxxxxxxx>