On 10.09.2025 09:38, Penny Zheng wrote:
> @@ -508,13 +510,21 @@ static inline int xsm_unbind_pt_irq(
> static inline int xsm_irq_permission(
> xsm_default_t def, struct domain *d, int pirq, uint8_t allow)
> {
> +#ifdef CONFIG_MGMT_HYPERCALLS
> return alternative_call(xsm_ops.irq_permission, d, pirq, allow);
> +#else
> + return -EOPNOTSUPP;
> +#endif
> }
>
> static inline int xsm_iomem_permission(
> xsm_default_t def, struct domain *d, uint64_t s, uint64_t e, uint8_t
> allow)
> {
> +#ifdef CONFIG_MGMT_HYPERCALLS
> return alternative_call(xsm_ops.iomem_permission, d, s, e, allow);
> +#else
> + return -EOPNOTSUPP;
> +#endif
> }
Along the lines of Stefano's comment - why would these inline functions stay
around? Them returning an error in the MGMT_HYPERCALLS=n case is actually a
problem: For xsm_iomem_permission() it's only a conceptual one, but for
xsm_irq_permission() you break x86's handling of XEN_DOMCTL_gsi_permission.
I would have added "transiently", but from the titles of later patches I
can't spot where to expect that one to be taken care of.
> --- a/xen/xsm/flask/hooks.c
> +++ b/xen/xsm/flask/hooks.c
> @@ -1111,12 +1111,14 @@ static int cf_check flask_unbind_pt_irq(
> return current_has_perm(d, SECCLASS_RESOURCE, RESOURCE__REMOVE);
> }
>
> +#ifdef CONFIG_MGMT_HYPERCALLS
> static int cf_check flask_irq_permission(
> struct domain *d, int pirq, uint8_t access)
> {
> /* the PIRQ number is not useful; real IRQ is checked during mapping */
> return current_has_perm(d, SECCLASS_RESOURCE, resource_to_perm(access));
> }
> +#endif /* CONFIG_MGMT_HYPERCALLS */
>
> struct iomem_has_perm_data {
> uint32_t ssid;
> @@ -1943,8 +1945,10 @@ static const struct xsm_ops __initconst_cf_clobber
> flask_ops = {
> .unmap_domain_irq = flask_unmap_domain_irq,
> .bind_pt_irq = flask_bind_pt_irq,
> .unbind_pt_irq = flask_unbind_pt_irq,
> +#ifdef CONFIG_MGMT_HYPERCALLS
> .irq_permission = flask_irq_permission,
> .iomem_permission = flask_iomem_permission,
> +#endif
> .iomem_mapping = flask_iomem_mapping,
> .pci_config_permission = flask_pci_config_permission,
>
It's odd that flask_iomem_permission() remains as a function, but for the
moment that looks to be necessary, as it's (oddly enough) called from
flask_iomem_mapping(). However, for that one I again can't drive from
titles of subsequent patches where it would be taken care of.
Daniel - is this layering actually helpful? Can't we either drop
flask_iomem_mapping() (with the benefit of a cf_check disappearing), or
have it do directly what it wants done, rather than calling the other
hook function?
Having reached the bottom of the patch - what about xsm/dummy.h?
Jan
