From: Grygorii Strashko <grygorii_strashko@epam.com>

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@epam.com>
Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
---

Changes in v8:
- check for CONFIG_ARM_SCI to be ebabled instead of COMFIG_ARM before
calling sci_do_domctl
- rework sci_do_domctl call to avoid extra checks, improved error
handling.
- do not propagate ret1 if sci_do_domctl returned positive ret
- updated comment in domctl.c code

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                     | 26 +++++++++++++++++-
 xen/drivers/passthrough/device_tree.c   |  6 +++++
 4 files changed, 80 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 29a7726d32..cd87c15fe6 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -29,6 +29,9 @@
 #include <xen/xvmalloc.h>
 
 #include <asm/current.h>
+#ifdef 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;
@@ -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;
+    #if IS_ENABLED(CONFIG_ARM_SCI)
+        ret1 = sci_do_domctl(op, d, u_domctl);
+    #endif
+
         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 failed then overwrite the return code to propagate
+         * SCI failure back to caller.
+         */
+        if ( ret1 != -ENXIO && ret1 < 0 )
+            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

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

Hi Jan,
Thank you for your comments.

On 22/01/2026 09:51, Jan Beulich wrote:
> 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
My intention was to preserve the original behavior as much as possible.
This means that if the SCI assign operation returns an error, we still
attempt the IOMMU assignment, but filter the return code and ultimately
return the SCI error if the IOMMU assignment was successful.
However, in this scenario, we would still return an error from
the domctl call, which could lead to unexpected results.

I agree with your point.

With your suggestion, the code becomes much cleaner:

#if IS_ENABLED(CONFIG_ARM_SCI)
         ret = sci_do_domctl(op, d, u_domctl);
         if ( ret < 0 && ret != -ENXIO )
             break;
#endif

         ret = iommu_do_domctl(op, d, u_domctl);
         break;

What do you think about this approach?

Oleksii

On 26.01.2026 11:29, Oleksii Moisieiev wrote:
> Hi Jan,
> Thank you for your comments.
> 
> On 22/01/2026 09:51, Jan Beulich wrote:
>> 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
> My intention was to preserve the original behavior as much as possible.
> This means that if the SCI assign operation returns an error, we still
> attempt the IOMMU assignment, but filter the return code and ultimately
> return the SCI error if the IOMMU assignment was successful.
> However, in this scenario, we would still return an error from
> the domctl call, which could lead to unexpected results.
> 
> I agree with your point.
> 
> With your suggestion, the code becomes much cleaner:
> 
> #if IS_ENABLED(CONFIG_ARM_SCI)
>          ret = sci_do_domctl(op, d, u_domctl);
>          if ( ret < 0 && ret != -ENXIO )
>              break;
> #endif
> 
>          ret = iommu_do_domctl(op, d, u_domctl);
>          break;
> 
> What do you think about this approach?

Yes. Just to double-check though: There's nothing that needs undoing after
a successful sci_do_domctl() when the subsequent iommu_do_domctl() failed?

One other remark: No IS_ENABLED() please in pre-processor directives. It
wants to be #ifdef there. IS_ENABLED() is for use in if().

Jan

On 26/01/2026 12:37, Jan Beulich wrote:
> On 26.01.2026 11:29, Oleksii Moisieiev wrote:
>> Hi Jan,
>> Thank you for your comments.
>>
>> On 22/01/2026 09:51, Jan Beulich wrote:
>>> 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
>> My intention was to preserve the original behavior as much as possible.
>> This means that if the SCI assign operation returns an error, we still
>> attempt the IOMMU assignment, but filter the return code and ultimately
>> return the SCI error if the IOMMU assignment was successful.
>> However, in this scenario, we would still return an error from
>> the domctl call, which could lead to unexpected results.
>>
>> I agree with your point.
>>
>> With your suggestion, the code becomes much cleaner:
>>
>> #if IS_ENABLED(CONFIG_ARM_SCI)
>>           ret = sci_do_domctl(op, d, u_domctl);
>>           if ( ret < 0 && ret != -ENXIO )
>>               break;
>> #endif
>>
>>           ret = iommu_do_domctl(op, d, u_domctl);
>>           break;
>>
>> What do you think about this approach?
> Yes. Just to double-check though: There's nothing that needs undoing after
> a successful sci_do_domctl() when the subsequent iommu_do_domctl() failed?
Just rechecked. for multiagent (apart from parameters check) is sends SCMI
request to the firmware. So I don't see anything that should be undone 
in case
of error.
> One other remark: No IS_ENABLED() please in pre-processor directives. It
> wants to be #ifdef there. IS_ENABLED() is for use in if().
Will fix.
> Jan