On 21.01.2026 19:43, Oleksii Moisieiev wrote:
> --- a/xen/arch/arm/firmware/sci.c
> +++ b/xen/arch/arm/firmware/sci.c
> @@ -126,6 +126,41 @@ int sci_assign_dt_device(struct domain *d, struct
> dt_device_node *dev)
> return 0;
> }
>
> +int sci_do_domctl(struct xen_domctl *domctl, struct domain *d,
> + XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
> +{
> + struct dt_device_node *dev;
> + int ret = 0;
> +
> + switch ( domctl->cmd )
> + {
> + case XEN_DOMCTL_assign_device:
> + ret = -ENXIO;
> + if ( domctl->u.assign_device.dev != XEN_DOMCTL_DEV_DT )
> + break;
> +
> + if ( !cur_mediator )
> + break;
> +
> + if ( !cur_mediator->assign_dt_device )
> + break;
> +
> + ret = dt_find_node_by_gpath(domctl->u.assign_device.u.dt.path,
> + domctl->u.assign_device.u.dt.size, &dev);
> + if ( ret )
> + return ret;
> +
> + ret = sci_assign_dt_device(d, dev);
> +
> + break;
> + default:
Nit: Blank line above here please.
> @@ -274,7 +277,7 @@ static bool is_stable_domctl(uint32_t cmd)
>
> long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
> {
> - long ret = 0;
> + long ret = 0, ret1 = 0;
This initializer isn't really needed, with ...
> @@ -833,7 +836,28 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t)
> u_domctl)
> case XEN_DOMCTL_test_assign_device:
> case XEN_DOMCTL_deassign_device:
> case XEN_DOMCTL_get_device_group:
> + /*
> + * Chain SCI DT handling ahead of the IOMMU path so an SCI mediator
> + * can authorise access-controlled DT devices. Unhandled cases report
> + * -ENXIO, which is ignored.
> + */
> + ret1 = -ENXIO;
... this, is it? In fact, why not use -ENXIO as initializer?
> + #if IS_ENABLED(CONFIG_ARM_SCI)
> + ret1 = sci_do_domctl(op, d, u_domctl);
> + #endif
Why the indentation of the pre-processor directives? At the very least the #-es
want to be in the first column, but I see no reason for the indentation at all.
> ret = iommu_do_domctl(op, d, u_domctl);
> + if ( ret < 0 )
> + return ret;
You can't simply return here, as we may be holding an RCU lock on the domain.
> + /*
> + * If IOMMU processing was successful, check for SCI processing
> return
> + * code and if it failed then overwrite the return code to propagate
> + * SCI failure back to caller.
> + */
> + if ( ret1 != -ENXIO && ret1 < 0 )
> + ret = ret1;
So if IOMMU processing was successful and because of SCI you return an error
here, why would the IOMMU part not need undoing? Or to ask differently, if
SCI said "no", why even try the IOMMU operation?
Jan
