Re: [PATCH 20/24] XSM: fold xsm_{,un}map_domain_pirq() hooks

On 7/28/26 9:23 AM, Jan Beulich wrote:
> Like other resource management hooks they are different in just "add
> resource" vs "remove resource". Hence like in other cases a single hook
> can easily serve both purposes.
> 
> Signed-off-by: Jan Beulich <[email protected]>
> 
> --- a/xen/arch/x86/physdev.c
> +++ b/xen/arch/x86/physdev.c
> @@ -109,7 +109,7 @@ int physdev_map_pirq(struct domain *d, i
>           return physdev_hvm_map_pirq(d, type, index, pirq_p);
>       }
>   
> -    ret = xsm_map_domain_pirq(XSM_DM_PRIV, d);
> +    ret = xsm_map_domain_pirq(XSM_DM_PRIV, d, true);
>       if ( ret )
>           return ret;
>   
> @@ -142,7 +142,7 @@ int physdev_unmap_pirq(struct domain *d,
>       int ret = 0;
>   
>       if ( d != current->domain || !is_hvm_domain(d) || !has_pirq(d) )
> -        ret = xsm_unmap_domain_pirq(XSM_DM_PRIV, d);
> +        ret = xsm_map_domain_pirq(XSM_DM_PRIV, d, false);
>       if ( ret )
>           return ret;
>   
> --- a/xen/include/xsm/dummy.h
> +++ b/xen/include/xsm/dummy.h
> @@ -461,14 +461,7 @@ static XSM_INLINE char *xsm_show_irq_sid
>   #ifdef CONFIG_HAS_PIRQ   
>   static XSM_INLINE int xsm_map_domain_pirq(
> -    XSM_DEFAULT_ARG struct domain *d)
> -{
> -    XSM_ASSERT_ACTION(XSM_DM_PRIV);
> -    return xsm_default_action(action, current->domain, d);
> -}
> -
> -static XSM_INLINE int xsm_unmap_domain_pirq(
> -    XSM_DEFAULT_ARG struct domain *d)
> +    XSM_DEFAULT_ARG struct domain *d, bool allow)
>   {
>       XSM_ASSERT_ACTION(XSM_DM_PRIV);
>       return xsm_default_action(action, current->domain, d);
> --- a/xen/include/xsm/hooks.h
> +++ b/xen/include/xsm/hooks.h
> @@ -68,8 +68,7 @@ XSM_HOOK(int, kexec)
>   XSM_HOOK(int, schedop_shutdown, struct domain *, struct domain *)
>   
>   #ifdef CONFIG_HAS_PIRQ
> -XSM_HOOK(int, map_domain_pirq, struct domain *)
> -XSM_HOOK(int, unmap_domain_pirq, struct domain *)
> +XSM_HOOK(int, map_domain_pirq, struct domain *, bool)
>   #endif
>   
>   XSM_HOOK(int, map_domain_irq, struct domain *, int, const void *)
> --- a/xen/xsm/flask/hooks.c
> +++ b/xen/xsm/flask/hooks.c
> @@ -1022,14 +1022,9 @@ static char *cf_check flask_show_irq_sid
>   
>   #ifdef CONFIG_HAS_PIRQ
>   
> -static int cf_check flask_map_domain_pirq(struct domain *d)
> +static int cf_check flask_map_domain_pirq(struct domain *d, bool access)
>   {
> -    return current_has_perm(d, SECCLASS_RESOURCE, RESOURCE__ADD);
> -}
> -
> -static int cf_check flask_unmap_domain_pirq(struct domain *d)
> -{
> -    return current_has_perm(d, SECCLASS_RESOURCE, RESOURCE__REMOVE);
> +    return current_has_perm(d, SECCLASS_RESOURCE, resource_to_perm(access));
>   }
>   
>   #endif /* CONFIG_HAS_PIRQ */
> 

I am not opposed to collapsing the calls as long as the semantic is not 
lost, which I feel the reuse of the xsm_map_domain_pirq does looses it 
much less provides an opportunity for confusion. Something like 
xsm_domain_pirq(..., access) makes more semantic sense to me, as it 
would read, grant domain pirq access T/F.

v/r,
dps