Re: [PATCH v7 1/5] xen/domctl: extend XEN_DOMCTL_assign_device to handle not only iommu

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 14 Jan 2026, Oleksii Moisieiev wrote:
> From: Grygorii Strashko <grygorii_strashko@xxxxxxxx>
> 
> Add chained handling of assigned DT devices to support access-controller
> functionality through SCI framework, so a DT device assign request can be
> passed to firmware for processing and enabling VM access to the requested
> device (for example, device power management through SCMI).
> 
> The SCI access-controller DT device processing is called before the IOMMU
> path. It runs for any DT-described device (protected or not, and even when
> the IOMMU is disabled). The IOMMU path remains unchanged for PCI devices;
> only the DT path is relaxed to permit non-IOMMU devices.
> 
> This lets xl.cfg:"dtdev" list both IOMMU-protected and non-protected DT
> devices:
> 
> dtdev = [
>     "/soc/video@e6ef0000", <- IOMMU protected device
>     "/soc/i2c@e6508000", <- not IOMMU protected device
> ]
> 
> The change is done in two parts:
> 1) call sci_do_domctl() in do_domctl() before IOMMU processing and propagate
> its error if it fails while the IOMMU path succeeds (unhandled cases return
> -ENXIO and are ignored);
> 2) update iommu_do_dt_domctl() to check for dt_device_is_protected() and
> not fail if DT device is not protected by IOMMU. iommu_do_pci_domctl
> doesn't need to be updated because iommu_do_domctl first tries
> iommu_do_pci_domctl (when CONFIG_HAS_PCI) and falls back to
> iommu_do_dt_domctl only if PCI returns -ENODEV.
> The new dt_device_is_protected() bypass in iommu_do_dt_domctl only
> applies to DT-described devices; SCI parameters are carried via DT nodes.
> PCI devices handled by iommu_do_pci_domctl do not carry DT/SCI
> metadata in this path, so there is no notion of “SCI parameters on a
> non-IOMMU-protected PCI device” for it to interpret or to skip. The
> PCI path should continue to report errors if assignment cannot be
> performed by the IOMMU layer.
> So we should leave iommu_do_pci_domctl unchanged; the SCI/DT-specific
> relaxations belong only in the DT path.
> Also SCI handling only exists when DT is present.
> 
> Signed-off-by: Grygorii Strashko <grygorii_strashko@xxxxxxxx>
> Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@xxxxxxxx>
> ---
> 
> Changes in v7:
> - update domctl to build on both Arm and x86 platforms
> - move ret1 declaration to the top of the function as required by code
> style
> 
> Changes in v6:
> - change iommu_do_domctl and sci_do_domctl command order and
> call sci_do_domctl first which will produce cleaner code path.
> Also dropped changing return code when iommu was disabled in
> iommu_do_domctl.
> 
> Changes in v5:
> - return -EINVAL if mediator without assign_dt_device was provided
> - invert return code check for iommu_do_domctl in
> XEN_DOMCTL_assign_device domctl processing to make cleaner code
> - change -ENOTSUPP error code to -ENXIO in sci_do_domctl
> - handle -ENXIO return comde of iommu_do_domctl
> - leave !dt_device_is_protected check in iommu_do_dt_domctl to make
> code work the same way it's done in "handle_device" call while
> creating hwdom(dom0) and "handle_passthrough_prop" call for dom0less
> creation
> - drop return check from sci_assign_dt_device call as not needed
> - do not return EINVAL when addign_dt_device is not set. That is
> because this callback is optional and not implemented in single-agent driver
> 
>  xen/arch/arm/firmware/sci.c             | 35 +++++++++++++++++++++++++
>  xen/arch/arm/include/asm/firmware/sci.h | 14 ++++++++++
>  xen/common/domctl.c                     | 35 ++++++++++++++++++++++++-
>  xen/drivers/passthrough/device_tree.c   |  6 +++++
>  4 files changed, 89 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/firmware/sci.c b/xen/arch/arm/firmware/sci.c
> index aa93cda7f0..31a7e5c28b 100644
> --- 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:
> +        /* do not fail here as call is chained with iommu handling */
> +        break;
> +    }
> +
> +    return ret;
> +}
> +
>  static int __init sci_init(void)
>  {
>      struct dt_device_node *np;
> diff --git a/xen/arch/arm/include/asm/firmware/sci.h 
> b/xen/arch/arm/include/asm/firmware/sci.h
> index 3500216bc2..a2d314e627 100644
> --- a/xen/arch/arm/include/asm/firmware/sci.h
> +++ b/xen/arch/arm/include/asm/firmware/sci.h
> @@ -146,6 +146,14 @@ int sci_dt_finalize(struct domain *d, void *fdt);
>   * control" functionality.
>   */
>  int sci_assign_dt_device(struct domain *d, struct dt_device_node *dev);
> +
> +/*
> + * SCI domctl handler
> + *
> + * Only XEN_DOMCTL_assign_device is handled for now.
> + */
> +int sci_do_domctl(struct xen_domctl *domctl, struct domain *d,
> +                  XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl);
>  #else
>  
>  static inline bool sci_domain_is_enabled(struct domain *d)
> @@ -195,6 +203,12 @@ static inline int sci_assign_dt_device(struct domain *d,
>      return 0;
>  }
>  
> +static inline int sci_do_domctl(struct xen_domctl *domctl, struct domain *d,
> +                                XEN_GUEST_HANDLE_PARAM(xen_domctl_t) 
> u_domctl)
> +{
> +    return 0;
> +}
> +
>  #endif /* CONFIG_ARM_SCI */
>  
>  #endif /* __ASM_ARM_SCI_H */
> diff --git a/xen/common/domctl.c b/xen/common/domctl.c
> index 954d790226..5a31cccdd7 100644
> --- a/xen/common/domctl.c
> +++ b/xen/common/domctl.c
> @@ -29,6 +29,9 @@
>  #include <xen/xvmalloc.h>
>  
>  #include <asm/current.h>
> +#if IS_ENABLED(CONFIG_ARM)
> +#include <asm/firmware/sci.h>
> +#endif
>  #include <asm/irq.h>
>  #include <asm/page.h>
>  #include <asm/p2m.h>
> @@ -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;
>      bool copyback = false;
>      struct xen_domctl curop, *op = &curop;
>      struct domain *d;
> @@ -827,7 +830,37 @@ 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:
> +        if ( IS_ENABLED(CONFIG_ARM) )

I skipped the last round of review so if this is addressing a comment
from Jan I am OK with this as is.

However, I would check directly on CONFIG_ARM_SCI.


> +        {
> +            /*
> +             * Add chained handling of assigned DT devices to support
> +             * access-controller functionality through SCI framework, so
> +             * DT device assign request can be passed to FW for processing 
> and
> +             * enabling VM access to requested device.
> +             * The access-controller DT device processing is called before 
> IOMMU
> +             * processing preserving return code and expected to be executed 
> for
> +             * any DT device regardless if DT device is protected by IOMMU or
> +             * not (or IOMMU is disabled).
> +             */
> +            ret1 = sci_do_domctl(op, d, u_domctl);
> +            if ( ret1 < 0 )
> +                return ret1;
> +        }
> +        else
> +            ret1 = -ENXIO;
> +
>          ret = iommu_do_domctl(op, d, u_domctl);
> +        if ( ret < 0 )
> +            return ret;
> +
> +        /*
> +         * If IOMMU processing was successful, check for SCI processing 
> return
> +         * code and if it was failed then overwrite the return code to 
> propagate
> +         * SCI failure back to caller.
> +         */
> +        if ( ret1 != -ENXIO )
> +            ret = ret1;
> +
>          break;
>  
>      case XEN_DOMCTL_get_paging_mempool_size:
> diff --git a/xen/drivers/passthrough/device_tree.c 
> b/xen/drivers/passthrough/device_tree.c
> index f5850a2607..29a44dc773 100644
> --- a/xen/drivers/passthrough/device_tree.c
> +++ b/xen/drivers/passthrough/device_tree.c
> @@ -379,6 +379,12 @@ int iommu_do_dt_domctl(struct xen_domctl *domctl, struct 
> domain *d,
>              break;
>          }
>  
> +        if ( !dt_device_is_protected(dev) )
> +        {
> +            ret = 0;
> +            break;
> +        }
> +
>          ret = iommu_assign_dt_device(d, dev);
>  
>          if ( ret )
> -- 
> 2.34.1
>