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

Oleksii Moisieiev posted 5 patches 3 months, 1 week ago
There is a newer version of this series
[PATCH v6 1/5] xen/domctl: extend XEN_DOMCTL_assign_device to handle not only iommu
Posted by Oleksii Moisieiev 3 months, 1 week ago
From: Grygorii Strashko <grygorii_strashko@epam.com>

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
(for example, device power management through FW interface like SCMI).

The SCI access-controller DT device processing is chained after IOMMU
processing and expected to be executed for any DT device regardless of its
protection by IOMMU (or if IOMMU is disabled).

This allows to pass not only IOMMU protected DT device through
xl.cfg:"dtdev" property for processing:

dtdev = [
    "/soc/video@e6ef0000", <- IOMMU protected device
    "/soc/i2c@e6508000", <- not IOMMU protected device
]

The change is done in two parts:
1) update iommu_do_dt_domctl() to check for dt_device_is_protected() and
not fail if DT device is not protected by IOMMU
2) add chained call to sci_do_domctl() in do_domctl()

Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>
Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
---

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, 81 insertions(+)

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..b89559ef7b 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -29,6 +29,7 @@
 #include <xen/xvmalloc.h>
 
 #include <asm/current.h>
+#include <asm/firmware/sci.h>
 #include <asm/irq.h>
 #include <asm/page.h>
 #include <asm/p2m.h>
@@ -827,7 +828,32 @@ 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:
+        int ret1;
+        
+        /*
+         * 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 chained 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);
+
         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
Re: [PATCH v6 1/5] xen/domctl: extend XEN_DOMCTL_assign_device to handle not only iommu
Posted by Jan Beulich 3 months ago
On 01.11.2025 12:56, Oleksii Moisieiev wrote:
> --- a/xen/common/domctl.c
> +++ b/xen/common/domctl.c
> @@ -29,6 +29,7 @@
>  #include <xen/xvmalloc.h>
>  
>  #include <asm/current.h>
> +#include <asm/firmware/sci.h>
>  #include <asm/irq.h>
>  #include <asm/page.h>
>  #include <asm/p2m.h>

Does this build at all on non-Arm?

> @@ -827,7 +828,32 @@ 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:
> +        int ret1;
> +        
> +        /*
> +         * 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 chained 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);

Why would this not be the initializer of the new variable? (I also don't think
that we've decided to permit variable declarations at other than the top of
scopes or within e.g. a for() loop control construct.)

>          ret = iommu_do_domctl(op, d, u_domctl);
> +        if ( ret < 0 )
> +            return ret;

Why would you invoke both in all cases? If sci_do_domctl() handled the request,
there isn't any point in also invoking iommu_do_domctl(), is there? Or else is
there maybe some crucial aspect missing from the description (or not explicit
enough there for a non-SCI person like me)?

Also this doesn't look to fit the description saying "The SCI access-controller
DT device processing is chained after IOMMU processing ..."

> --- 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 )

How are DT and PCI different in this regard?

Jan
Re: [PATCH v6 1/5] xen/domctl: extend XEN_DOMCTL_assign_device to handle not only iommu
Posted by Oleksii Moisieiev 4 weeks ago
Hi Jan,

Sorry for a long silence. Please see my answers below:

On 06/11/2025 12:09, Jan Beulich wrote:
> On 01.11.2025 12:56, Oleksii Moisieiev wrote:
>> --- a/xen/common/domctl.c
>> +++ b/xen/common/domctl.c
>> @@ -29,6 +29,7 @@
>>   #include <xen/xvmalloc.h>
>>   
>>   #include <asm/current.h>
>> +#include <asm/firmware/sci.h>
>>   #include <asm/irq.h>
>>   #include <asm/page.h>
>>   #include <asm/p2m.h>
> Does this build at all on non-Arm?
Good finding. Thanks - will fix.
>> @@ -827,7 +828,32 @@ 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:
>> +        int ret1;
>> +
>> +        /*
>> +         * 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 chained 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);
> Why would this not be the initializer of the new variable? (I also don't think
> that we've decided to permit variable declarations at other than the top of
> scopes or within e.g. a for() loop control construct.)
>
+
>>           ret = iommu_do_domctl(op, d, u_domctl);
>> +        if ( ret < 0 )
>> +            return ret;
> Why would you invoke both in all cases? If sci_do_domctl() handled the request,
> there isn't any point in also invoking iommu_do_domctl(), is there? Or else is
> there maybe some crucial aspect missing from the description (or not explicit
> enough there for a non-SCI person like me)?
>
> Also this doesn't look to fit the description saying "The SCI access-controller
> DT device processing is chained after IOMMU processing ..."
>
We call both because SCI and IOMMU cover different concerns and a DT 
device may need
both: SCI for FW-mediated access control (power/clocks/reset) and IOMMU 
for DMA isolation.
SCI returning success does not mean the IOMMU work is redundant.

- sci_do_domctl() returns -ENXIO when it has nothing to do (non-DT, no 
mediator, mediator lacks assign hook).
That is the “not handled by SCI” sentinel; in that case the code 
proceeds to IOMMU normally.
-  When sci_do_domctl() succeeds (0), the device may still require IOMMU 
programming
(e.g., DT device has an iommus property). Skipping iommu_do_domctl() 
would leave DMA isolation unprogrammed.

The final if (ret1 != -ENXIO) ret = ret1; ensures that if both paths ran 
and IOMMU succeeded,
an SCI failure is still reported to the caller.

Device-tree examples to illustrate the dual roles:
1. Access-controlled DT device (not necessarily IOMMU-protected):

i2c3: i2c@e6508000 {
     compatible = "renesas,rcar-gen3-i2c";
     reg = <0 0xe6508000 0 0x40>;
     power-domains = <&scmi_pd 5>;      // FW-managed power domain
     clocks = <&scmi_clk 12>;
     clock-names = "fck";
     access-controllers = <&scmi_xen 0>;
     // no iommus property: SCI may need to authorize/power this device; 
IOMMU has nothing to do
};

2. IOMMU-protected DT device that still may need SCI mediation:
vpu: video@e6ef0000 {
     compatible = "renesas,rcar-vpu";
     reg = <0 0xe6ef0000 0 0x10000>;
     iommus = <&ipmmu 0 0>;             // needs IOMMU mapping for DMA 
isolation
     power-domains = <&scmi_pd 7>;      // FW-managed power/clock/reset
     clocks = <&scmi_clk 34>;
     access-controllers = <&scmi_xen 0>;
     clock-names = "vpu";
};
>> --- 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 )
> How are DT and PCI different in this regard?
Please find examples above.

BR,

Oleksii
>
> Jan
Re: [PATCH v6 1/5] xen/domctl: extend XEN_DOMCTL_assign_device to handle not only iommu
Posted by Jan Beulich 4 weeks ago
On 12.01.2026 16:16, Oleksii Moisieiev wrote:
> On 06/11/2025 12:09, Jan Beulich wrote:
>> On 01.11.2025 12:56, Oleksii Moisieiev wrote:
>>> @@ -827,7 +828,32 @@ 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:
>>> +        int ret1;
>>> +
>>> +        /*
>>> +         * 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 chained 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);
>> Why would this not be the initializer of the new variable? (I also don't think
>> that we've decided to permit variable declarations at other than the top of
>> scopes or within e.g. a for() loop control construct.)
>>
> +
>>>           ret = iommu_do_domctl(op, d, u_domctl);
>>> +        if ( ret < 0 )
>>> +            return ret;
>> Why would you invoke both in all cases? If sci_do_domctl() handled the request,
>> there isn't any point in also invoking iommu_do_domctl(), is there? Or else is
>> there maybe some crucial aspect missing from the description (or not explicit
>> enough there for a non-SCI person like me)?
>>
>> Also this doesn't look to fit the description saying "The SCI access-controller
>> DT device processing is chained after IOMMU processing ..."
>>
> We call both because SCI and IOMMU cover different concerns and a DT 
> device may need
> both: SCI for FW-mediated access control (power/clocks/reset) and IOMMU 
> for DMA isolation.
> SCI returning success does not mean the IOMMU work is redundant.

Can the comment then please be updated to properly call out this dual
requirement?

> - sci_do_domctl() returns -ENXIO when it has nothing to do (non-DT, no 
> mediator, mediator lacks assign hook).
> That is the “not handled by SCI” sentinel; in that case the code 
> proceeds to IOMMU normally.
> -  When sci_do_domctl() succeeds (0), the device may still require IOMMU 
> programming
> (e.g., DT device has an iommus property). Skipping iommu_do_domctl() 
> would leave DMA isolation unprogrammed.
> 
> The final if (ret1 != -ENXIO) ret = ret1; ensures that if both paths ran 
> and IOMMU succeeded,
> an SCI failure is still reported to the caller.
> 
> Device-tree examples to illustrate the dual roles:
> 1. Access-controlled DT device (not necessarily IOMMU-protected):
> 
> i2c3: i2c@e6508000 {
>      compatible = "renesas,rcar-gen3-i2c";
>      reg = <0 0xe6508000 0 0x40>;
>      power-domains = <&scmi_pd 5>;      // FW-managed power domain
>      clocks = <&scmi_clk 12>;
>      clock-names = "fck";
>      access-controllers = <&scmi_xen 0>;
>      // no iommus property: SCI may need to authorize/power this device; 
> IOMMU has nothing to do
> };
> 
> 2. IOMMU-protected DT device that still may need SCI mediation:
> vpu: video@e6ef0000 {
>      compatible = "renesas,rcar-vpu";
>      reg = <0 0xe6ef0000 0 0x10000>;
>      iommus = <&ipmmu 0 0>;             // needs IOMMU mapping for DMA 
> isolation
>      power-domains = <&scmi_pd 7>;      // FW-managed power/clock/reset
>      clocks = <&scmi_clk 34>;
>      access-controllers = <&scmi_xen 0>;
>      clock-names = "vpu";
> };
>>> --- 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 )
>> How are DT and PCI different in this regard?
> Please find examples above.

Sorry, but I can't spot anything PCI-ish in the examples above. Then again I
also no longer recall why I compared with PCI here. Oh, perhaps because the
PCI side isn't being modified at all.

Jan

Re: [PATCH v6 1/5] xen/domctl: extend XEN_DOMCTL_assign_device to handle not only iommu
Posted by Oleksii Moisieiev 4 weeks ago

On 12/01/2026 17:40, Jan Beulich wrote:
> On 12.01.2026 16:16, Oleksii Moisieiev wrote:
>> On 06/11/2025 12:09, Jan Beulich wrote:
>>> On 01.11.2025 12:56, Oleksii Moisieiev wrote:
>>>> @@ -827,7 +828,32 @@ 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:
>>>> +        int ret1;
>>>> +
>>>> +        /*
>>>> +         * 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 chained 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);
>>> Why would this not be the initializer of the new variable? (I also don't think
>>> that we've decided to permit variable declarations at other than the top of
>>> scopes or within e.g. a for() loop control construct.)
>>>
>> +
>>>>            ret = iommu_do_domctl(op, d, u_domctl);
>>>> +        if ( ret < 0 )
>>>> +            return ret;
>>> Why would you invoke both in all cases? If sci_do_domctl() handled the request,
>>> there isn't any point in also invoking iommu_do_domctl(), is there? Or else is
>>> there maybe some crucial aspect missing from the description (or not explicit
>>> enough there for a non-SCI person like me)?
>>>
>>> Also this doesn't look to fit the description saying "The SCI access-controller
>>> DT device processing is chained after IOMMU processing ..."
>>>
>> We call both because SCI and IOMMU cover different concerns and a DT
>> device may need
>> both: SCI for FW-mediated access control (power/clocks/reset) and IOMMU
>> for DMA isolation.
>> SCI returning success does not mean the IOMMU work is redundant.
> Can the comment then please be updated to properly call out this dual
> requirement?
Yes, for sure.
>> - sci_do_domctl() returns -ENXIO when it has nothing to do (non-DT, no
>> mediator, mediator lacks assign hook).
>> That is the “not handled by SCI” sentinel; in that case the code
>> proceeds to IOMMU normally.
>> -  When sci_do_domctl() succeeds (0), the device may still require IOMMU
>> programming
>> (e.g., DT device has an iommus property). Skipping iommu_do_domctl()
>> would leave DMA isolation unprogrammed.
>>
>> The final if (ret1 != -ENXIO) ret = ret1; ensures that if both paths ran
>> and IOMMU succeeded,
>> an SCI failure is still reported to the caller.
>>
>> Device-tree examples to illustrate the dual roles:
>> 1. Access-controlled DT device (not necessarily IOMMU-protected):
>>
>> i2c3: i2c@e6508000 {
>>       compatible = "renesas,rcar-gen3-i2c";
>>       reg = <0 0xe6508000 0 0x40>;
>>       power-domains = <&scmi_pd 5>;      // FW-managed power domain
>>       clocks = <&scmi_clk 12>;
>>       clock-names = "fck";
>>       access-controllers = <&scmi_xen 0>;
>>       // no iommus property: SCI may need to authorize/power this device;
>> IOMMU has nothing to do
>> };
>>
>> 2. IOMMU-protected DT device that still may need SCI mediation:
>> vpu: video@e6ef0000 {
>>       compatible = "renesas,rcar-vpu";
>>       reg = <0 0xe6ef0000 0 0x10000>;
>>       iommus = <&ipmmu 0 0>;             // needs IOMMU mapping for DMA
>> isolation
>>       power-domains = <&scmi_pd 7>;      // FW-managed power/clock/reset
>>       clocks = <&scmi_clk 34>;
>>       access-controllers = <&scmi_xen 0>;
>>       clock-names = "vpu";
>> };
>>>> --- 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 )
>>> How are DT and PCI different in this regard?
>> Please find examples above.
> Sorry, but I can't spot anything PCI-ish in the examples above. Then again I
> also no longer recall why I compared with PCI here. Oh, perhaps because the
> PCI side isn't being modified at all.
>
> Jan
Correct, pci code wasn't touched by these changes.

BR,
Oleksii
Re: [PATCH v6 1/5] xen/domctl: extend XEN_DOMCTL_assign_device to handle not only iommu
Posted by Jan Beulich 4 weeks ago
On 12.01.2026 16:54, Oleksii Moisieiev wrote:
> 
> 
> On 12/01/2026 17:40, Jan Beulich wrote:
>> On 12.01.2026 16:16, Oleksii Moisieiev wrote:
>>> On 06/11/2025 12:09, Jan Beulich wrote:
>>>> On 01.11.2025 12:56, Oleksii Moisieiev wrote:
>>>>> @@ -827,7 +828,32 @@ 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:
>>>>> +        int ret1;
>>>>> +
>>>>> +        /*
>>>>> +         * 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 chained 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);
>>>> Why would this not be the initializer of the new variable? (I also don't think
>>>> that we've decided to permit variable declarations at other than the top of
>>>> scopes or within e.g. a for() loop control construct.)
>>>>
>>> +
>>>>>            ret = iommu_do_domctl(op, d, u_domctl);
>>>>> +        if ( ret < 0 )
>>>>> +            return ret;
>>>> Why would you invoke both in all cases? If sci_do_domctl() handled the request,
>>>> there isn't any point in also invoking iommu_do_domctl(), is there? Or else is
>>>> there maybe some crucial aspect missing from the description (or not explicit
>>>> enough there for a non-SCI person like me)?
>>>>
>>>> Also this doesn't look to fit the description saying "The SCI access-controller
>>>> DT device processing is chained after IOMMU processing ..."
>>>>
>>> We call both because SCI and IOMMU cover different concerns and a DT
>>> device may need
>>> both: SCI for FW-mediated access control (power/clocks/reset) and IOMMU
>>> for DMA isolation.
>>> SCI returning success does not mean the IOMMU work is redundant.
>> Can the comment then please be updated to properly call out this dual
>> requirement?
> Yes, for sure.
>>> - sci_do_domctl() returns -ENXIO when it has nothing to do (non-DT, no
>>> mediator, mediator lacks assign hook).
>>> That is the “not handled by SCI” sentinel; in that case the code
>>> proceeds to IOMMU normally.
>>> -  When sci_do_domctl() succeeds (0), the device may still require IOMMU
>>> programming
>>> (e.g., DT device has an iommus property). Skipping iommu_do_domctl()
>>> would leave DMA isolation unprogrammed.
>>>
>>> The final if (ret1 != -ENXIO) ret = ret1; ensures that if both paths ran
>>> and IOMMU succeeded,
>>> an SCI failure is still reported to the caller.
>>>
>>> Device-tree examples to illustrate the dual roles:
>>> 1. Access-controlled DT device (not necessarily IOMMU-protected):
>>>
>>> i2c3: i2c@e6508000 {
>>>       compatible = "renesas,rcar-gen3-i2c";
>>>       reg = <0 0xe6508000 0 0x40>;
>>>       power-domains = <&scmi_pd 5>;      // FW-managed power domain
>>>       clocks = <&scmi_clk 12>;
>>>       clock-names = "fck";
>>>       access-controllers = <&scmi_xen 0>;
>>>       // no iommus property: SCI may need to authorize/power this device;
>>> IOMMU has nothing to do
>>> };
>>>
>>> 2. IOMMU-protected DT device that still may need SCI mediation:
>>> vpu: video@e6ef0000 {
>>>       compatible = "renesas,rcar-vpu";
>>>       reg = <0 0xe6ef0000 0 0x10000>;
>>>       iommus = <&ipmmu 0 0>;             // needs IOMMU mapping for DMA
>>> isolation
>>>       power-domains = <&scmi_pd 7>;      // FW-managed power/clock/reset
>>>       clocks = <&scmi_clk 34>;
>>>       access-controllers = <&scmi_xen 0>;
>>>       clock-names = "vpu";
>>> };
>>>>> --- 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 )
>>>> How are DT and PCI different in this regard?
>>> Please find examples above.
>> Sorry, but I can't spot anything PCI-ish in the examples above. Then again I
>> also no longer recall why I compared with PCI here. Oh, perhaps because the
>> PCI side isn't being modified at all.
>>
> Correct, pci code wasn't touched by these changes.

Yet my question boils down to "why", not "whether".

Jan

Re: [PATCH v6 1/5] xen/domctl: extend XEN_DOMCTL_assign_device to handle not only iommu
Posted by Oleksii Moisieiev 4 weeks ago

On 12/01/2026 17:56, Jan Beulich wrote:
> On 12.01.2026 16:54, Oleksii Moisieiev wrote:
>>
>> On 12/01/2026 17:40, Jan Beulich wrote:
>>> On 12.01.2026 16:16, Oleksii Moisieiev wrote:
>>>> On 06/11/2025 12:09, Jan Beulich wrote:
>>>>> On 01.11.2025 12:56, Oleksii Moisieiev wrote:
>>>>>> @@ -827,7 +828,32 @@ 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:
>>>>>> +        int ret1;
>>>>>> +
>>>>>> +        /*
>>>>>> +         * 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 chained 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);
>>>>> Why would this not be the initializer of the new variable? (I also don't think
>>>>> that we've decided to permit variable declarations at other than the top of
>>>>> scopes or within e.g. a for() loop control construct.)
>>>>>
>>>> +
>>>>>>             ret = iommu_do_domctl(op, d, u_domctl);
>>>>>> +        if ( ret < 0 )
>>>>>> +            return ret;
>>>>> Why would you invoke both in all cases? If sci_do_domctl() handled the request,
>>>>> there isn't any point in also invoking iommu_do_domctl(), is there? Or else is
>>>>> there maybe some crucial aspect missing from the description (or not explicit
>>>>> enough there for a non-SCI person like me)?
>>>>>
>>>>> Also this doesn't look to fit the description saying "The SCI access-controller
>>>>> DT device processing is chained after IOMMU processing ..."
>>>>>
>>>> We call both because SCI and IOMMU cover different concerns and a DT
>>>> device may need
>>>> both: SCI for FW-mediated access control (power/clocks/reset) and IOMMU
>>>> for DMA isolation.
>>>> SCI returning success does not mean the IOMMU work is redundant.
>>> Can the comment then please be updated to properly call out this dual
>>> requirement?
>> Yes, for sure.
>>>> - sci_do_domctl() returns -ENXIO when it has nothing to do (non-DT, no
>>>> mediator, mediator lacks assign hook).
>>>> That is the “not handled by SCI” sentinel; in that case the code
>>>> proceeds to IOMMU normally.
>>>> -  When sci_do_domctl() succeeds (0), the device may still require IOMMU
>>>> programming
>>>> (e.g., DT device has an iommus property). Skipping iommu_do_domctl()
>>>> would leave DMA isolation unprogrammed.
>>>>
>>>> The final if (ret1 != -ENXIO) ret = ret1; ensures that if both paths ran
>>>> and IOMMU succeeded,
>>>> an SCI failure is still reported to the caller.
>>>>
>>>> Device-tree examples to illustrate the dual roles:
>>>> 1. Access-controlled DT device (not necessarily IOMMU-protected):
>>>>
>>>> i2c3: i2c@e6508000 {
>>>>        compatible = "renesas,rcar-gen3-i2c";
>>>>        reg = <0 0xe6508000 0 0x40>;
>>>>        power-domains = <&scmi_pd 5>;      // FW-managed power domain
>>>>        clocks = <&scmi_clk 12>;
>>>>        clock-names = "fck";
>>>>        access-controllers = <&scmi_xen 0>;
>>>>        // no iommus property: SCI may need to authorize/power this device;
>>>> IOMMU has nothing to do
>>>> };
>>>>
>>>> 2. IOMMU-protected DT device that still may need SCI mediation:
>>>> vpu: video@e6ef0000 {
>>>>        compatible = "renesas,rcar-vpu";
>>>>        reg = <0 0xe6ef0000 0 0x10000>;
>>>>        iommus = <&ipmmu 0 0>;             // needs IOMMU mapping for DMA
>>>> isolation
>>>>        power-domains = <&scmi_pd 7>;      // FW-managed power/clock/reset
>>>>        clocks = <&scmi_clk 34>;
>>>>        access-controllers = <&scmi_xen 0>;
>>>>        clock-names = "vpu";
>>>> };
>>>>>> --- 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 )
>>>>> How are DT and PCI different in this regard?
>>>> Please find examples above.
>>> Sorry, but I can't spot anything PCI-ish in the examples above. Then again I
>>> also no longer recall why I compared with PCI here. Oh, perhaps because the
>>> PCI side isn't being modified at all.
>>>
>> Correct, pci code wasn't touched by these changes.
> Yet my question boils down to "why", not "whether".
>
> Jan
I have reviewed the previous versions of the patch serie and could not 
find any questions related to PCI prior to this series. Therefore, "How 
are DT and PCI different in this regard?" appears to be the first 
question concerning PCI.

BR,
Oleksii.
Re: [PATCH v6 1/5] xen/domctl: extend XEN_DOMCTL_assign_device to handle not only iommu
Posted by Jan Beulich 4 weeks ago
On 12.01.2026 17:10, Oleksii Moisieiev wrote:
> On 12/01/2026 17:56, Jan Beulich wrote:
>> On 12.01.2026 16:54, Oleksii Moisieiev wrote:
>>> On 12/01/2026 17:40, Jan Beulich wrote:
>>>> On 12.01.2026 16:16, Oleksii Moisieiev wrote:
>>>>> On 06/11/2025 12:09, Jan Beulich wrote:
>>>>>> On 01.11.2025 12:56, Oleksii Moisieiev wrote:
>>>>>>> @@ -827,7 +828,32 @@ 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:
>>>>>>> +        int ret1;
>>>>>>> +
>>>>>>> +        /*
>>>>>>> +         * 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 chained 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);
>>>>>> Why would this not be the initializer of the new variable? (I also don't think
>>>>>> that we've decided to permit variable declarations at other than the top of
>>>>>> scopes or within e.g. a for() loop control construct.)
>>>>>>
>>>>> +
>>>>>>>             ret = iommu_do_domctl(op, d, u_domctl);
>>>>>>> +        if ( ret < 0 )
>>>>>>> +            return ret;
>>>>>> Why would you invoke both in all cases? If sci_do_domctl() handled the request,
>>>>>> there isn't any point in also invoking iommu_do_domctl(), is there? Or else is
>>>>>> there maybe some crucial aspect missing from the description (or not explicit
>>>>>> enough there for a non-SCI person like me)?
>>>>>>
>>>>>> Also this doesn't look to fit the description saying "The SCI access-controller
>>>>>> DT device processing is chained after IOMMU processing ..."
>>>>>>
>>>>> We call both because SCI and IOMMU cover different concerns and a DT
>>>>> device may need
>>>>> both: SCI for FW-mediated access control (power/clocks/reset) and IOMMU
>>>>> for DMA isolation.
>>>>> SCI returning success does not mean the IOMMU work is redundant.
>>>> Can the comment then please be updated to properly call out this dual
>>>> requirement?
>>> Yes, for sure.
>>>>> - sci_do_domctl() returns -ENXIO when it has nothing to do (non-DT, no
>>>>> mediator, mediator lacks assign hook).
>>>>> That is the “not handled by SCI” sentinel; in that case the code
>>>>> proceeds to IOMMU normally.
>>>>> -  When sci_do_domctl() succeeds (0), the device may still require IOMMU
>>>>> programming
>>>>> (e.g., DT device has an iommus property). Skipping iommu_do_domctl()
>>>>> would leave DMA isolation unprogrammed.
>>>>>
>>>>> The final if (ret1 != -ENXIO) ret = ret1; ensures that if both paths ran
>>>>> and IOMMU succeeded,
>>>>> an SCI failure is still reported to the caller.
>>>>>
>>>>> Device-tree examples to illustrate the dual roles:
>>>>> 1. Access-controlled DT device (not necessarily IOMMU-protected):
>>>>>
>>>>> i2c3: i2c@e6508000 {
>>>>>        compatible = "renesas,rcar-gen3-i2c";
>>>>>        reg = <0 0xe6508000 0 0x40>;
>>>>>        power-domains = <&scmi_pd 5>;      // FW-managed power domain
>>>>>        clocks = <&scmi_clk 12>;
>>>>>        clock-names = "fck";
>>>>>        access-controllers = <&scmi_xen 0>;
>>>>>        // no iommus property: SCI may need to authorize/power this device;
>>>>> IOMMU has nothing to do
>>>>> };
>>>>>
>>>>> 2. IOMMU-protected DT device that still may need SCI mediation:
>>>>> vpu: video@e6ef0000 {
>>>>>        compatible = "renesas,rcar-vpu";
>>>>>        reg = <0 0xe6ef0000 0 0x10000>;
>>>>>        iommus = <&ipmmu 0 0>;             // needs IOMMU mapping for DMA
>>>>> isolation
>>>>>        power-domains = <&scmi_pd 7>;      // FW-managed power/clock/reset
>>>>>        clocks = <&scmi_clk 34>;
>>>>>        access-controllers = <&scmi_xen 0>;
>>>>>        clock-names = "vpu";
>>>>> };
>>>>>>> --- 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 )
>>>>>> How are DT and PCI different in this regard?
>>>>> Please find examples above.
>>>> Sorry, but I can't spot anything PCI-ish in the examples above. Then again I
>>>> also no longer recall why I compared with PCI here. Oh, perhaps because the
>>>> PCI side isn't being modified at all.
>>>>
>>> Correct, pci code wasn't touched by these changes.
>> Yet my question boils down to "why", not "whether".
>>
> I have reviewed the previous versions of the patch serie and could not 
> find any questions related to PCI prior to this series. Therefore, "How 
> are DT and PCI different in this regard?" appears to be the first 
> question concerning PCI.

Quite possible, yet does that somehow eliminate the need to address it? I'm
trying to understand why the respective PCI code isn't being touched.

Jan

Re: [PATCH v6 1/5] xen/domctl: extend XEN_DOMCTL_assign_device to handle not only iommu
Posted by Oleksii Moisieiev 3 weeks, 6 days ago

On 12/01/2026 18:13, Jan Beulich wrote:
> On 12.01.2026 17:10, Oleksii Moisieiev wrote:
>> On 12/01/2026 17:56, Jan Beulich wrote:
>>> On 12.01.2026 16:54, Oleksii Moisieiev wrote:
>>>> On 12/01/2026 17:40, Jan Beulich wrote:
>>>>> On 12.01.2026 16:16, Oleksii Moisieiev wrote:
>>>>>> On 06/11/2025 12:09, Jan Beulich wrote:
>>>>>>> On 01.11.2025 12:56, Oleksii Moisieiev wrote:
>>>>>>>> @@ -827,7 +828,32 @@ 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:
>>>>>>>> +        int ret1;
>>>>>>>> +
>>>>>>>> +        /*
>>>>>>>> +         * 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 chained 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);
>>>>>>> Why would this not be the initializer of the new variable? (I also don't think
>>>>>>> that we've decided to permit variable declarations at other than the top of
>>>>>>> scopes or within e.g. a for() loop control construct.)
>>>>>>>
>>>>>> +
>>>>>>>>              ret = iommu_do_domctl(op, d, u_domctl);
>>>>>>>> +        if ( ret < 0 )
>>>>>>>> +            return ret;
>>>>>>> Why would you invoke both in all cases? If sci_do_domctl() handled the request,
>>>>>>> there isn't any point in also invoking iommu_do_domctl(), is there? Or else is
>>>>>>> there maybe some crucial aspect missing from the description (or not explicit
>>>>>>> enough there for a non-SCI person like me)?
>>>>>>>
>>>>>>> Also this doesn't look to fit the description saying "The SCI access-controller
>>>>>>> DT device processing is chained after IOMMU processing ..."
>>>>>>>
>>>>>> We call both because SCI and IOMMU cover different concerns and a DT
>>>>>> device may need
>>>>>> both: SCI for FW-mediated access control (power/clocks/reset) and IOMMU
>>>>>> for DMA isolation.
>>>>>> SCI returning success does not mean the IOMMU work is redundant.
>>>>> Can the comment then please be updated to properly call out this dual
>>>>> requirement?
>>>> Yes, for sure.
>>>>>> - sci_do_domctl() returns -ENXIO when it has nothing to do (non-DT, no
>>>>>> mediator, mediator lacks assign hook).
>>>>>> That is the “not handled by SCI” sentinel; in that case the code
>>>>>> proceeds to IOMMU normally.
>>>>>> -  When sci_do_domctl() succeeds (0), the device may still require IOMMU
>>>>>> programming
>>>>>> (e.g., DT device has an iommus property). Skipping iommu_do_domctl()
>>>>>> would leave DMA isolation unprogrammed.
>>>>>>
>>>>>> The final if (ret1 != -ENXIO) ret = ret1; ensures that if both paths ran
>>>>>> and IOMMU succeeded,
>>>>>> an SCI failure is still reported to the caller.
>>>>>>
>>>>>> Device-tree examples to illustrate the dual roles:
>>>>>> 1. Access-controlled DT device (not necessarily IOMMU-protected):
>>>>>>
>>>>>> i2c3: i2c@e6508000 {
>>>>>>         compatible = "renesas,rcar-gen3-i2c";
>>>>>>         reg = <0 0xe6508000 0 0x40>;
>>>>>>         power-domains = <&scmi_pd 5>;      // FW-managed power domain
>>>>>>         clocks = <&scmi_clk 12>;
>>>>>>         clock-names = "fck";
>>>>>>         access-controllers = <&scmi_xen 0>;
>>>>>>         // no iommus property: SCI may need to authorize/power this device;
>>>>>> IOMMU has nothing to do
>>>>>> };
>>>>>>
>>>>>> 2. IOMMU-protected DT device that still may need SCI mediation:
>>>>>> vpu: video@e6ef0000 {
>>>>>>         compatible = "renesas,rcar-vpu";
>>>>>>         reg = <0 0xe6ef0000 0 0x10000>;
>>>>>>         iommus = <&ipmmu 0 0>;             // needs IOMMU mapping for DMA
>>>>>> isolation
>>>>>>         power-domains = <&scmi_pd 7>;      // FW-managed power/clock/reset
>>>>>>         clocks = <&scmi_clk 34>;
>>>>>>         access-controllers = <&scmi_xen 0>;
>>>>>>         clock-names = "vpu";
>>>>>> };
>>>>>>>> --- 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 )
>>>>>>> How are DT and PCI different in this regard?
>>>>>> Please find examples above.
>>>>> Sorry, but I can't spot anything PCI-ish in the examples above. Then again I
>>>>> also no longer recall why I compared with PCI here. Oh, perhaps because the
>>>>> PCI side isn't being modified at all.
>>>>>
>>>> Correct, pci code wasn't touched by these changes.
>>> Yet my question boils down to "why", not "whether".
>>>
>> I have reviewed the previous versions of the patch serie and could not
>> find any questions related to PCI prior to this series. Therefore, "How
>> are DT and PCI different in this regard?" appears to be the first
>> question concerning PCI.
> Quite possible, yet does that somehow eliminate the need to address it? I'm
> trying to understand why the respective PCI code isn't being touched.
>
> Jan
XEN_DOMCTL_assign_device dispatch: we now chain sci_do_domctl first, 
then iommu_do_domctl.
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.

BR, Oleksii.
Re: [PATCH v6 1/5] xen/domctl: extend XEN_DOMCTL_assign_device to handle not only iommu
Posted by Jan Beulich 3 weeks, 6 days ago
On 13.01.2026 16:50, Oleksii Moisieiev wrote:
> 
> 
> On 12/01/2026 18:13, Jan Beulich wrote:
>> On 12.01.2026 17:10, Oleksii Moisieiev wrote:
>>> On 12/01/2026 17:56, Jan Beulich wrote:
>>>> On 12.01.2026 16:54, Oleksii Moisieiev wrote:
>>>>> On 12/01/2026 17:40, Jan Beulich wrote:
>>>>>> On 12.01.2026 16:16, Oleksii Moisieiev wrote:
>>>>>>> On 06/11/2025 12:09, Jan Beulich wrote:
>>>>>>>> On 01.11.2025 12:56, Oleksii Moisieiev wrote:
>>>>>>>>> @@ -827,7 +828,32 @@ 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:
>>>>>>>>> +        int ret1;
>>>>>>>>> +
>>>>>>>>> +        /*
>>>>>>>>> +         * 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 chained 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);
>>>>>>>> Why would this not be the initializer of the new variable? (I also don't think
>>>>>>>> that we've decided to permit variable declarations at other than the top of
>>>>>>>> scopes or within e.g. a for() loop control construct.)
>>>>>>>>
>>>>>>> +
>>>>>>>>>              ret = iommu_do_domctl(op, d, u_domctl);
>>>>>>>>> +        if ( ret < 0 )
>>>>>>>>> +            return ret;
>>>>>>>> Why would you invoke both in all cases? If sci_do_domctl() handled the request,
>>>>>>>> there isn't any point in also invoking iommu_do_domctl(), is there? Or else is
>>>>>>>> there maybe some crucial aspect missing from the description (or not explicit
>>>>>>>> enough there for a non-SCI person like me)?
>>>>>>>>
>>>>>>>> Also this doesn't look to fit the description saying "The SCI access-controller
>>>>>>>> DT device processing is chained after IOMMU processing ..."
>>>>>>>>
>>>>>>> We call both because SCI and IOMMU cover different concerns and a DT
>>>>>>> device may need
>>>>>>> both: SCI for FW-mediated access control (power/clocks/reset) and IOMMU
>>>>>>> for DMA isolation.
>>>>>>> SCI returning success does not mean the IOMMU work is redundant.
>>>>>> Can the comment then please be updated to properly call out this dual
>>>>>> requirement?
>>>>> Yes, for sure.
>>>>>>> - sci_do_domctl() returns -ENXIO when it has nothing to do (non-DT, no
>>>>>>> mediator, mediator lacks assign hook).
>>>>>>> That is the “not handled by SCI” sentinel; in that case the code
>>>>>>> proceeds to IOMMU normally.
>>>>>>> -  When sci_do_domctl() succeeds (0), the device may still require IOMMU
>>>>>>> programming
>>>>>>> (e.g., DT device has an iommus property). Skipping iommu_do_domctl()
>>>>>>> would leave DMA isolation unprogrammed.
>>>>>>>
>>>>>>> The final if (ret1 != -ENXIO) ret = ret1; ensures that if both paths ran
>>>>>>> and IOMMU succeeded,
>>>>>>> an SCI failure is still reported to the caller.
>>>>>>>
>>>>>>> Device-tree examples to illustrate the dual roles:
>>>>>>> 1. Access-controlled DT device (not necessarily IOMMU-protected):
>>>>>>>
>>>>>>> i2c3: i2c@e6508000 {
>>>>>>>         compatible = "renesas,rcar-gen3-i2c";
>>>>>>>         reg = <0 0xe6508000 0 0x40>;
>>>>>>>         power-domains = <&scmi_pd 5>;      // FW-managed power domain
>>>>>>>         clocks = <&scmi_clk 12>;
>>>>>>>         clock-names = "fck";
>>>>>>>         access-controllers = <&scmi_xen 0>;
>>>>>>>         // no iommus property: SCI may need to authorize/power this device;
>>>>>>> IOMMU has nothing to do
>>>>>>> };
>>>>>>>
>>>>>>> 2. IOMMU-protected DT device that still may need SCI mediation:
>>>>>>> vpu: video@e6ef0000 {
>>>>>>>         compatible = "renesas,rcar-vpu";
>>>>>>>         reg = <0 0xe6ef0000 0 0x10000>;
>>>>>>>         iommus = <&ipmmu 0 0>;             // needs IOMMU mapping for DMA
>>>>>>> isolation
>>>>>>>         power-domains = <&scmi_pd 7>;      // FW-managed power/clock/reset
>>>>>>>         clocks = <&scmi_clk 34>;
>>>>>>>         access-controllers = <&scmi_xen 0>;
>>>>>>>         clock-names = "vpu";
>>>>>>> };
>>>>>>>>> --- 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 )
>>>>>>>> How are DT and PCI different in this regard?
>>>>>>> Please find examples above.
>>>>>> Sorry, but I can't spot anything PCI-ish in the examples above. Then again I
>>>>>> also no longer recall why I compared with PCI here. Oh, perhaps because the
>>>>>> PCI side isn't being modified at all.
>>>>>>
>>>>> Correct, pci code wasn't touched by these changes.
>>>> Yet my question boils down to "why", not "whether".
>>>>
>>> I have reviewed the previous versions of the patch serie and could not
>>> find any questions related to PCI prior to this series. Therefore, "How
>>> are DT and PCI different in this regard?" appears to be the first
>>> question concerning PCI.
>> Quite possible, yet does that somehow eliminate the need to address it? I'm
>> trying to understand why the respective PCI code isn't being touched.
>>
> XEN_DOMCTL_assign_device dispatch: we now chain sci_do_domctl first, 
> then iommu_do_domctl.
> 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.

Can an abridged variant of this please be added to the description?

Jan

Re: [PATCH v6 1/5] xen/domctl: extend XEN_DOMCTL_assign_device to handle not only iommu
Posted by Oleksii Moisieiev 3 weeks, 6 days ago

On 13/01/2026 18:08, Jan Beulich wrote:
> On 13.01.2026 16:50, Oleksii Moisieiev wrote:
>>
>> On 12/01/2026 18:13, Jan Beulich wrote:
>>> On 12.01.2026 17:10, Oleksii Moisieiev wrote:
>>>> On 12/01/2026 17:56, Jan Beulich wrote:
>>>>> On 12.01.2026 16:54, Oleksii Moisieiev wrote:
>>>>>> On 12/01/2026 17:40, Jan Beulich wrote:
>>>>>>> On 12.01.2026 16:16, Oleksii Moisieiev wrote:
>>>>>>>> On 06/11/2025 12:09, Jan Beulich wrote:
>>>>>>>>> On 01.11.2025 12:56, Oleksii Moisieiev wrote:
>>>>>>>>>> @@ -827,7 +828,32 @@ 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:
>>>>>>>>>> +        int ret1;
>>>>>>>>>> +
>>>>>>>>>> +        /*
>>>>>>>>>> +         * 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 chained 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);
>>>>>>>>> Why would this not be the initializer of the new variable? (I also don't think
>>>>>>>>> that we've decided to permit variable declarations at other than the top of
>>>>>>>>> scopes or within e.g. a for() loop control construct.)
>>>>>>>>>
>>>>>>>> +
>>>>>>>>>>               ret = iommu_do_domctl(op, d, u_domctl);
>>>>>>>>>> +        if ( ret < 0 )
>>>>>>>>>> +            return ret;
>>>>>>>>> Why would you invoke both in all cases? If sci_do_domctl() handled the request,
>>>>>>>>> there isn't any point in also invoking iommu_do_domctl(), is there? Or else is
>>>>>>>>> there maybe some crucial aspect missing from the description (or not explicit
>>>>>>>>> enough there for a non-SCI person like me)?
>>>>>>>>>
>>>>>>>>> Also this doesn't look to fit the description saying "The SCI access-controller
>>>>>>>>> DT device processing is chained after IOMMU processing ..."
>>>>>>>>>
>>>>>>>> We call both because SCI and IOMMU cover different concerns and a DT
>>>>>>>> device may need
>>>>>>>> both: SCI for FW-mediated access control (power/clocks/reset) and IOMMU
>>>>>>>> for DMA isolation.
>>>>>>>> SCI returning success does not mean the IOMMU work is redundant.
>>>>>>> Can the comment then please be updated to properly call out this dual
>>>>>>> requirement?
>>>>>> Yes, for sure.
>>>>>>>> - sci_do_domctl() returns -ENXIO when it has nothing to do (non-DT, no
>>>>>>>> mediator, mediator lacks assign hook).
>>>>>>>> That is the “not handled by SCI” sentinel; in that case the code
>>>>>>>> proceeds to IOMMU normally.
>>>>>>>> -  When sci_do_domctl() succeeds (0), the device may still require IOMMU
>>>>>>>> programming
>>>>>>>> (e.g., DT device has an iommus property). Skipping iommu_do_domctl()
>>>>>>>> would leave DMA isolation unprogrammed.
>>>>>>>>
>>>>>>>> The final if (ret1 != -ENXIO) ret = ret1; ensures that if both paths ran
>>>>>>>> and IOMMU succeeded,
>>>>>>>> an SCI failure is still reported to the caller.
>>>>>>>>
>>>>>>>> Device-tree examples to illustrate the dual roles:
>>>>>>>> 1. Access-controlled DT device (not necessarily IOMMU-protected):
>>>>>>>>
>>>>>>>> i2c3: i2c@e6508000 {
>>>>>>>>          compatible = "renesas,rcar-gen3-i2c";
>>>>>>>>          reg = <0 0xe6508000 0 0x40>;
>>>>>>>>          power-domains = <&scmi_pd 5>;      // FW-managed power domain
>>>>>>>>          clocks = <&scmi_clk 12>;
>>>>>>>>          clock-names = "fck";
>>>>>>>>          access-controllers = <&scmi_xen 0>;
>>>>>>>>          // no iommus property: SCI may need to authorize/power this device;
>>>>>>>> IOMMU has nothing to do
>>>>>>>> };
>>>>>>>>
>>>>>>>> 2. IOMMU-protected DT device that still may need SCI mediation:
>>>>>>>> vpu: video@e6ef0000 {
>>>>>>>>          compatible = "renesas,rcar-vpu";
>>>>>>>>          reg = <0 0xe6ef0000 0 0x10000>;
>>>>>>>>          iommus = <&ipmmu 0 0>;             // needs IOMMU mapping for DMA
>>>>>>>> isolation
>>>>>>>>          power-domains = <&scmi_pd 7>;      // FW-managed power/clock/reset
>>>>>>>>          clocks = <&scmi_clk 34>;
>>>>>>>>          access-controllers = <&scmi_xen 0>;
>>>>>>>>          clock-names = "vpu";
>>>>>>>> };
>>>>>>>>>> --- 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 )
>>>>>>>>> How are DT and PCI different in this regard?
>>>>>>>> Please find examples above.
>>>>>>> Sorry, but I can't spot anything PCI-ish in the examples above. Then again I
>>>>>>> also no longer recall why I compared with PCI here. Oh, perhaps because the
>>>>>>> PCI side isn't being modified at all.
>>>>>>>
>>>>>> Correct, pci code wasn't touched by these changes.
>>>>> Yet my question boils down to "why", not "whether".
>>>>>
>>>> I have reviewed the previous versions of the patch serie and could not
>>>> find any questions related to PCI prior to this series. Therefore, "How
>>>> are DT and PCI different in this regard?" appears to be the first
>>>> question concerning PCI.
>>> Quite possible, yet does that somehow eliminate the need to address it? I'm
>>> trying to understand why the respective PCI code isn't being touched.
>>>
>> XEN_DOMCTL_assign_device dispatch: we now chain sci_do_domctl first,
>> then iommu_do_domctl.
>> 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.
> Can an abridged variant of this please be added to the description?
>
> Jan
Sure. I will add this to commit description.

Oleksii.