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

[Oleksii Moisieiev <Oleksii_Moisieiev@xxxxxxxx>]

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.