Re: [PATCH 19/24] XSM: convert "allow" (Flask: "access") parameters to bool

On 7/28/26 9:23 AM, Jan Beulich wrote:
> These are boolean, so they should always have used bool (originally
> bool_t), not uint8_t. Leverage recent changes to arrange for this with
> (now) fewer places which need changing (within the XSM machinery itself).
> Adjust call sites as well, where the conversion wasn't done so far.
> 
> Signed-off-by: Jan Beulich <[email protected]>
> ---
> Why is it that Arm doesn't use xsm_irq_permission() at all? Same for Arm64
> vs xsm_pci_config_permission().
> 
> --- a/xen/arch/x86/domctl.c
> +++ b/xen/arch/x86/domctl.c
> @@ -235,7 +235,7 @@ long arch_do_domctl(
>       {
>           unsigned int fp = domctl->u.ioport_permission.first_port;
>           unsigned int np = domctl->u.ioport_permission.nr_ports;
> -        int allow = domctl->u.ioport_permission.allow_access;
> +        bool allow = domctl->u.ioport_permission.allow_access;
>   
>           ret = -EINVAL;
>           if ( (fp + np) <= fp || (fp + np) > MAX_IOPORTS )
> @@ -306,7 +306,8 @@ long arch_do_domctl(
>               break;
>           }
>   
> -        ret = xsm_irq_permission(XSM_PRIV, d, irq, flags);
> +        ret = xsm_irq_permission(XSM_PRIV, d, irq,
> +                                 flags & XEN_DOMCTL_GSI_ACTION_MASK);
>           if ( ret )
>               break;
>   
> @@ -687,7 +688,7 @@ long arch_do_domctl(
>           unsigned int fgp = domctl->u.ioport_mapping.first_gport;
>           unsigned int fmp = domctl->u.ioport_mapping.first_mport;
>           unsigned int np = domctl->u.ioport_mapping.nr_ports;
> -        unsigned int add = domctl->u.ioport_mapping.add_mapping;
> +        bool add = domctl->u.ioport_mapping.add_mapping;
>           struct hvm_domain *hvm;
>           struct g2m_ioport *g2m_ioport;
>           int found = 0;
> --- a/xen/arch/x86/pci.c
> +++ b/xen/arch/x86/pci.c
> @@ -78,7 +78,7 @@ int pci_conf_write_intercept(unsigned in
>   {
>       struct pci_dev *pdev;
>       int rc = xsm_pci_config_permission(XSM_HOOK, current->domain, bdf,
> -                                       reg, reg + size - 1, 1);
> +                                       reg, reg + size - 1, true);
>   
>       if ( rc < 0 )
>           return rc;
> --- a/xen/arch/x86/pv/emul-priv-op.c
> +++ b/xen/arch/x86/pv/emul-priv-op.c
> @@ -260,7 +260,7 @@ static bool pci_cfg_ok(struct domain *cu
>   
>       return !write ?
>              xsm_pci_config_permission(XSM_HOOK, currd, machine_bdf,
> -                                     start, start + size - 1, 0) == 0 :
> +                                     start, start + size - 1, false) == 0 :
>              pci_conf_write_intercept(0, machine_bdf, start, size, write) >= 0;
>   }
>   
> --- a/xen/include/xsm/dummy.h
> +++ b/xen/include/xsm/dummy.h
> @@ -505,21 +505,21 @@ static XSM_INLINE int xsm_unmap_domain_i
>   }
>   
>   static XSM_INLINE int xsm_irq_permission(
> -    XSM_DEFAULT_ARG struct domain *d, int pirq, uint8_t allow)
> +    XSM_DEFAULT_ARG struct domain *d, int pirq, bool allow)
>   {
>       XSM_ASSERT_ACTION(XSM_PRIV);
>       return xsm_default_action(action, current->domain, d);
>   }
>   
>   static XSM_INLINE int xsm_iomem_permission(
> -    XSM_DEFAULT_ARG struct domain *d, uint64_t s, uint64_t e, uint8_t allow)
> +    XSM_DEFAULT_ARG struct domain *d, uint64_t s, uint64_t e, bool allow)
>   {
>       XSM_ASSERT_ACTION(XSM_PRIV);
>       return xsm_default_action(action, current->domain, d);
>   }
>   
>   static XSM_INLINE int xsm_iomem_mapping(
> -    XSM_DEFAULT_ARG struct domain *d, uint64_t s, uint64_t e, uint8_t allow)
> +    XSM_DEFAULT_ARG struct domain *d, uint64_t s, uint64_t e, bool allow)
>   {
>       XSM_ASSERT_ACTION(XSM_DM_PRIV);
>       return xsm_default_action(action, current->domain, d);
> @@ -527,7 +527,7 @@ static XSM_INLINE int xsm_iomem_mapping(
>   
>   #ifdef CONFIG_HAS_VPCI
>   static XSM_INLINE int xsm_iomem_mapping_vpci(
> -    XSM_DEFAULT_ARG struct domain *d, uint64_t s, uint64_t e, uint8_t allow)
> +    XSM_DEFAULT_ARG struct domain *d, uint64_t s, uint64_t e, bool allow)
>   {
>       XSM_ASSERT_ACTION(XSM_HOOK);
>       return xsm_default_action(action, current->domain, d);
> @@ -537,7 +537,7 @@ static XSM_INLINE int xsm_iomem_mapping_
>   #ifdef CONFIG_HAS_PCI
>   static XSM_INLINE int xsm_pci_config_permission(
>       XSM_DEFAULT_ARG struct domain *d, uint32_t machine_bdf, uint16_t start,
> -    uint16_t end, uint8_t access)
> +    uint16_t end, bool access)
>   {
>       XSM_ASSERT_ACTION(XSM_HOOK);
>       return xsm_default_action(action, current->domain, d);
> @@ -711,14 +711,14 @@ static XSM_INLINE int xsm_priv_mapping(
>   #endif
>   
>   static XSM_INLINE int xsm_ioport_permission(
> -    XSM_DEFAULT_ARG struct domain *d, uint32_t s, uint32_t e, uint8_t allow)
> +    XSM_DEFAULT_ARG struct domain *d, uint32_t s, uint32_t e, bool allow)
>   {
>       XSM_ASSERT_ACTION(XSM_PRIV);
>       return xsm_default_action(action, current->domain, d);
>   }
>   
>   static XSM_INLINE int xsm_ioport_mapping(
> -    XSM_DEFAULT_ARG struct domain *d, uint32_t s, uint32_t e, uint8_t allow)
> +    XSM_DEFAULT_ARG struct domain *d, uint32_t s, uint32_t e, 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
> @@ -77,12 +77,12 @@ XSM_HOOK(int, unmap_domain_irq, struct d
>   XSM_HOOK(int, bind_pt_irq, struct domain *, struct xen_domctl_bind_pt_irq *)
>   XSM_HOOK(int, unbind_pt_irq, struct domain *, struct xen_domctl_bind_pt_irq *)
>   
> -XSM_HOOK(int, irq_permission, struct domain *, int, uint8_t)
> -XSM_HOOK(int, iomem_permission, struct domain *, uint64_t, uint64_t, uint8_t)
> +XSM_HOOK(int, irq_permission, struct domain *, int, bool)
> +XSM_HOOK(int, iomem_permission, struct domain *, uint64_t, uint64_t, bool)
>   
> -XSM_HOOK(int, iomem_mapping, struct domain *, uint64_t, uint64_t, uint8_t)
> +XSM_HOOK(int, iomem_mapping, struct domain *, uint64_t, uint64_t, bool)
>   #ifdef CONFIG_HAS_VPCI
> -XSM_HOOK(int, iomem_mapping_vpci, struct domain *, uint64_t, uint64_t, uint8_t)
> +XSM_HOOK(int, iomem_mapping_vpci, struct domain *, uint64_t, uint64_t, bool)
>   #endif
>   
>   #if defined(CONFIG_HAS_PASSTHROUGH) && defined(CONFIG_HAS_PCI)
> @@ -97,7 +97,7 @@ XSM_HOOK(int, resource_setup_misc)
>   XSM_HOOK(int, resource_setup_pci, uint32_t)
>   XSM_HOOK(int, resource_setup_gsi, int)
>   XSM_HOOK(int, pci_config_permission, struct domain *, uint32_t, uint16_t,
> -                                     uint16_t, uint8_t)
> +                                     uint16_t, bool)
>   #endif
>   
>   #ifdef CONFIG_HYPFS
> @@ -144,8 +144,8 @@ XSM_HOOK(int, update_va_mapping, struct
>   #if defined(CONFIG_PV) || defined(CONFIG_SHADOW_PAGING)
>   XSM_HOOK(int, priv_mapping, struct domain *, struct domain *)
>   #endif
> -XSM_HOOK(int, ioport_permission, struct domain *, uint32_t, uint32_t, uint8_t)
> -XSM_HOOK(int, ioport_mapping, struct domain *, uint32_t, uint32_t, uint8_t)
> +XSM_HOOK(int, ioport_permission, struct domain *, uint32_t, uint32_t, bool)
> +XSM_HOOK(int, ioport_mapping, struct domain *, uint32_t, uint32_t, bool)
>   XSM_HOOK(int, pmu_op, struct domain *, unsigned int)
>   #endif /* CONFIG_X86 */
>   
> --- a/xen/xsm/flask/hooks.c
> +++ b/xen/xsm/flask/hooks.c
> @@ -998,7 +998,7 @@ static int cf_check flask_sysctl(const s
>   }
>   #endif /* CONFIG_SYSCTL */
>   
> -static inline uint32_t resource_to_perm(uint8_t access)
> +static inline uint32_t resource_to_perm(bool access)
>   {
>       if ( access )
>           return RESOURCE__ADD;
> @@ -1166,7 +1166,7 @@ static int cf_check flask_unbind_pt_irq(
>   }
>   
>   static int cf_check flask_irq_permission(
> -    struct domain *d, int pirq, uint8_t access)
> +    struct domain *d, int pirq, bool access)
>   {
>       /* the PIRQ number is not useful; real IRQ is checked during mapping */
>       return current_has_perm(d, SECCLASS_RESOURCE, resource_to_perm(access));
> @@ -1199,7 +1199,7 @@ static int cf_check _iomem_has_perm(
>   }
>   
>   static int cf_check flask_iomem_permission(
> -    struct domain *d, uint64_t start, uint64_t end, uint8_t access)
> +    struct domain *d, uint64_t start, uint64_t end, bool access)
>   {
>       struct iomem_has_perm_data data;
>       int rc;
> @@ -1221,7 +1221,8 @@ static int cf_check flask_iomem_permissi
>       return security_iterate_iomem_sids(start, end, _iomem_has_perm, &data);
>   }
>   
> -static int cf_check flask_iomem_mapping(struct domain *d, uint64_t start, uint64_t end, uint8_t access)
> +static int cf_check flask_iomem_mapping(
> +    struct domain *d, uint64_t start, uint64_t end, bool access)
>   {
>       return flask_iomem_permission(d, start, end, access);
>   }
> @@ -1230,7 +1231,7 @@ static int cf_check flask_iomem_mapping(
>   #ifdef CONFIG_HAS_PCI
>   static int cf_check flask_pci_config_permission(
>       struct domain *d, uint32_t machine_bdf, uint16_t start, uint16_t end,
> -    uint8_t access)
> +    bool access)
>   {
>       uint32_t dsid, rsid;
>       int rc = -EPERM;
> @@ -1709,7 +1710,7 @@ static int cf_check _ioport_has_perm(
>   }
>   
>   static int cf_check flask_ioport_permission(
> -    struct domain *d, uint32_t start, uint32_t end, uint8_t access)
> +    struct domain *d, uint32_t start, uint32_t end, bool access)
>   {
>       int rc;
>       struct ioport_has_perm_data data;
> @@ -1733,7 +1734,7 @@ static int cf_check flask_ioport_permiss
>   }
>   
>   static int cf_check flask_ioport_mapping(
> -    struct domain *d, uint32_t start, uint32_t end, uint8_t access)
> +    struct domain *d, uint32_t start, uint32_t end, bool access)
>   {
>       return flask_ioport_permission(d, start, end, access);
>   }
> 

Acked-by: Daniel P. Smith <[email protected]>