When PIRQs are not supported (e.g. for arm), XEN_DOMCTL_irq_permission
command is not handled.
This results with default (-ENOSYS) error code returned to control domain.
Update command handling to return -EOPNOTSUPP if control domain
invokes it by mistake when PIRQs are not supported.
Also exclude xsm_irq_permission hooks from compilation when
PIRQs are not supported.
Signed-off-by: Milan Djokic <milan_djokic@epam.com>
---
xen/common/domctl.c | 6 ++++--
xen/include/xsm/dummy.h | 4 ++--
xen/include/xsm/xsm.h | 6 ++++--
xen/xsm/dummy.c | 2 ++
xen/xsm/flask/hooks.c | 5 ++++-
5 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index 29a7726d32..159864bc99 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -638,9 +638,9 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
}
break;
-#ifdef CONFIG_HAS_PIRQ
case XEN_DOMCTL_irq_permission:
{
+#ifdef CONFIG_HAS_PIRQ
unsigned int pirq = op->u.irq_permission.pirq, irq;
int allow = op->u.irq_permission.allow_access;
@@ -656,9 +656,11 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
ret = irq_permit_access(d, irq);
else
ret = irq_deny_access(d, irq);
+#else
+ ret = -EOPNOTSUPP;
+#endif
break;
}
-#endif
case XEN_DOMCTL_iomem_permission:
{
diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
index e801dbcdba..6f6de161f9 100644
--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -555,14 +555,14 @@ static XSM_INLINE int cf_check xsm_unmap_domain_irq(
XSM_ASSERT_ACTION(XSM_HOOK);
return xsm_default_action(action, current->domain, d);
}
-
+#ifdef CONFIG_HAS_PIRQ
static XSM_INLINE int cf_check xsm_irq_permission(
XSM_DEFAULT_ARG struct domain *d, int pirq, uint8_t allow)
{
XSM_ASSERT_ACTION(XSM_HOOK);
return xsm_default_action(action, current->domain, d);
}
-
+#endif
static XSM_INLINE int cf_check xsm_iomem_permission(
XSM_DEFAULT_ARG struct domain *d, uint64_t s, uint64_t e, uint8_t allow)
{
diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
index 2d831d7745..b85cf9933a 100644
--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -113,7 +113,9 @@ struct xsm_ops {
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);
+#ifdef CONFIG_HAS_PIRQ
int (*irq_permission)(struct domain *d, int pirq, uint8_t allow);
+#endif
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,
@@ -504,13 +506,13 @@ static inline int xsm_unbind_pt_irq(
{
return alternative_call(xsm_ops.unbind_pt_irq, d, bind);
}
-
+#ifdef CONFIG_HAS_PIRQ
static inline int xsm_irq_permission(
xsm_default_t def, struct domain *d, int pirq, uint8_t allow)
{
return alternative_call(xsm_ops.irq_permission, d, pirq, allow);
}
-
+#endif
static inline int xsm_iomem_permission(
xsm_default_t def, struct domain *d, uint64_t s, uint64_t e, uint8_t allow)
{
diff --git a/xen/xsm/dummy.c b/xen/xsm/dummy.c
index 96dc82ac2e..28ef4a0beb 100644
--- a/xen/xsm/dummy.c
+++ b/xen/xsm/dummy.c
@@ -73,7 +73,9 @@ static const struct xsm_ops __initconst_cf_clobber dummy_ops = {
.unmap_domain_irq = xsm_unmap_domain_irq,
.bind_pt_irq = xsm_bind_pt_irq,
.unbind_pt_irq = xsm_unbind_pt_irq,
+#ifdef CONFIG_HAS_PIRQ
.irq_permission = xsm_irq_permission,
+#endif
.iomem_permission = xsm_iomem_permission,
.iomem_mapping = xsm_iomem_mapping,
.pci_config_permission = xsm_pci_config_permission,
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index 9f3915617c..63e4b4c353 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -1110,13 +1110,14 @@ static int cf_check flask_unbind_pt_irq(
{
return current_has_perm(d, SECCLASS_RESOURCE, RESOURCE__REMOVE);
}
-
+#ifdef CONFIG_HAS_PIRQ
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
struct iomem_has_perm_data {
uint32_t ssid;
@@ -1943,7 +1944,9 @@ 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_HAS_PIRQ
.irq_permission = flask_irq_permission,
+#endif
.iomem_permission = flask_iomem_permission,
.iomem_mapping = flask_iomem_mapping,
.pci_config_permission = flask_pci_config_permission,
--
2.43.0Hi Milan,
Thank you for your patch.
On 05.12.25 22:36, Milan Djokic wrote:
> When PIRQs are not supported (e.g. for arm), XEN_DOMCTL_irq_permission
> command is not handled.
> This results with default (-ENOSYS) error code returned to control domain.
> Update command handling to return -EOPNOTSUPP if control domain
> invokes it by mistake when PIRQs are not supported.
> Also exclude xsm_irq_permission hooks from compilation when
> PIRQs are not supported.
Could you please do not mix such a clean up a changes with functional changes?
It deserve to be standalone patch.
>
> Signed-off-by: Milan Djokic <milan_djokic@epam.com>
> ---
> xen/common/domctl.c | 6 ++++--
> xen/include/xsm/dummy.h | 4 ++--
> xen/include/xsm/xsm.h | 6 ++++--
> xen/xsm/dummy.c | 2 ++
> xen/xsm/flask/hooks.c | 5 ++++-
> 5 files changed, 16 insertions(+), 7 deletions(-)
>
> diff --git a/xen/common/domctl.c b/xen/common/domctl.c
> index 29a7726d32..159864bc99 100644
> --- a/xen/common/domctl.c
> +++ b/xen/common/domctl.c
> @@ -638,9 +638,9 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
> }
> break;
>
> -#ifdef CONFIG_HAS_PIRQ
> case XEN_DOMCTL_irq_permission:
> {
> +#ifdef CONFIG_HAS_PIRQ
> unsigned int pirq = op->u.irq_permission.pirq, irq;
> int allow = op->u.irq_permission.allow_access;
>
> @@ -656,9 +656,11 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
> ret = irq_permit_access(d, irq);
> else
> ret = irq_deny_access(d, irq);
> +#else
> + ret = -EOPNOTSUPP;
> +#endif
> break;
> }
> -#endif
>
> case XEN_DOMCTL_iomem_permission:
> {
> diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
> index e801dbcdba..6f6de161f9 100644
> --- a/xen/include/xsm/dummy.h
> +++ b/xen/include/xsm/dummy.h
> @@ -555,14 +555,14 @@ static XSM_INLINE int cf_check xsm_unmap_domain_irq(
> XSM_ASSERT_ACTION(XSM_HOOK);
> return xsm_default_action(action, current->domain, d);
> }
> -
> +#ifdef CONFIG_HAS_PIRQ
> static XSM_INLINE int cf_check xsm_irq_permission(
> XSM_DEFAULT_ARG struct domain *d, int pirq, uint8_t allow)
> {
> XSM_ASSERT_ACTION(XSM_HOOK);
> return xsm_default_action(action, current->domain, d);
> }
> -
[...]
--
Best regards,
-grygorii
On 05.12.2025 21:36, Milan Djokic wrote:
> When PIRQs are not supported (e.g. for arm), XEN_DOMCTL_irq_permission
> command is not handled.
> This results with default (-ENOSYS) error code returned to control domain.
> Update command handling to return -EOPNOTSUPP if control domain
> invokes it by mistake when PIRQs are not supported.
Did you consider simply replacing the bogus ENOSYS by EOPNOTSUPP? (Assuming
the difference in error code really makes a difference to callers.)
> --- a/xen/include/xsm/dummy.h
> +++ b/xen/include/xsm/dummy.h
> @@ -555,14 +555,14 @@ static XSM_INLINE int cf_check xsm_unmap_domain_irq(
> XSM_ASSERT_ACTION(XSM_HOOK);
> return xsm_default_action(action, current->domain, d);
> }
> -
> +#ifdef CONFIG_HAS_PIRQ
> static XSM_INLINE int cf_check xsm_irq_permission(
> XSM_DEFAULT_ARG struct domain *d, int pirq, uint8_t allow)
> {
> XSM_ASSERT_ACTION(XSM_HOOK);
> return xsm_default_action(action, current->domain, d);
> }
> -
> +#endif
> static XSM_INLINE int cf_check xsm_iomem_permission(
> XSM_DEFAULT_ARG struct domain *d, uint64_t s, uint64_t e, uint8_t allow)
> {
Why would you zap blank lines like this (also elsewhere)?
Jan
On 12/8/25 14:08, Jan Beulich wrote:
> On 05.12.2025 21:36, Milan Djokic wrote:
>> When PIRQs are not supported (e.g. for arm), XEN_DOMCTL_irq_permission
>> command is not handled.
>> This results with default (-ENOSYS) error code returned to control domain.
>> Update command handling to return -EOPNOTSUPP if control domain
>> invokes it by mistake when PIRQs are not supported.
>
> Did you consider simply replacing the bogus ENOSYS by EOPNOTSUPP? (Assuming
> the difference in error code really makes a difference to callers.)
>
Yes, this is the simplest approach. I didn’t do it because I wanted to
keep a distinction between commands that are part of the Xen interface
but not supported on the current configuration/platform, and commands
that don’t exist at all. I also tried to follow the existing pattern,
where unsupported commands are handled explicitly rather than falling
through to the default case (e.g. set_llc_colors, soft_reset, etc).
If this distinction isn’t useful, then yes, I can definitely just rely
on a default-case error.
>> --- a/xen/include/xsm/dummy.h
>> +++ b/xen/include/xsm/dummy.h
>> @@ -555,14 +555,14 @@ static XSM_INLINE int cf_check xsm_unmap_domain_irq(
>> XSM_ASSERT_ACTION(XSM_HOOK);
>> return xsm_default_action(action, current->domain, d);
>> }
>> -
>> +#ifdef CONFIG_HAS_PIRQ
>> static XSM_INLINE int cf_check xsm_irq_permission(
>> XSM_DEFAULT_ARG struct domain *d, int pirq, uint8_t allow)
>> {
>> XSM_ASSERT_ACTION(XSM_HOOK);
>> return xsm_default_action(action, current->domain, d);
>> }
>> -
>> +#endif
>> static XSM_INLINE int cf_check xsm_iomem_permission(
>> XSM_DEFAULT_ARG struct domain *d, uint64_t s, uint64_t e, uint8_t allow)
>> {
>
> Why would you zap blank lines like this (also elsewhere)?
Will fix this.
On 09.12.2025 13:48, Milan Djokic wrote: > On 12/8/25 14:08, Jan Beulich wrote: >> On 05.12.2025 21:36, Milan Djokic wrote: >>> When PIRQs are not supported (e.g. for arm), XEN_DOMCTL_irq_permission >>> command is not handled. >>> This results with default (-ENOSYS) error code returned to control domain. >>> Update command handling to return -EOPNOTSUPP if control domain >>> invokes it by mistake when PIRQs are not supported. >> >> Did you consider simply replacing the bogus ENOSYS by EOPNOTSUPP? (Assuming >> the difference in error code really makes a difference to callers.) > > Yes, this is the simplest approach. I didn’t do it because I wanted to > keep a distinction between commands that are part of the Xen interface > but not supported on the current configuration/platform, and commands > that don’t exist at all. I also tried to follow the existing pattern, > where unsupported commands are handled explicitly rather than falling > through to the default case (e.g. set_llc_colors, soft_reset, etc). > If this distinction isn’t useful, then yes, I can definitely just rely > on a default-case error. Before you (possibly) follow this route, maybe I should mention that there have been discussions around this in the past (sorry, no pointers, too long ago). IOW ahead of any consolidation (and making things consistent), imo as a community we need to settle on where exactly we consider ENOSYS valid to be used. My take is that almost all of the uses we (still) have are wrong. In particular all the ones for "unknown sub-op". Jan
© 2016 - 2025 Red Hat, Inc.