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

[Oleksii Moisieiev <Oleksii_Moisieiev@xxxxxxxx>]

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.