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
