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 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 | 19 ++++++++++++++
xen/drivers/passthrough/device_tree.c | 6 +++++
4 files changed, 74 insertions(+)
diff --git a/xen/arch/arm/firmware/sci.c b/xen/arch/arm/firmware/sci.c
index e1522e10e2..db75fc5cb3 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 71fb54852e..b8d1bc8a62 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 f2a7caaf85..35398a0c42 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>
@@ -859,7 +860,25 @@ 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;
+
ret = iommu_do_domctl(op, d, u_domctl);
+ if ( ret < 0 && ret != -ENXIO )
+ return ret;
+
+ /*
+ * 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 after IOMMU
+ * processing 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);
+ 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.1On 22.07.2025 13:41, Oleksii Moisieiev wrote: > @@ -859,7 +860,25 @@ 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; > + > ret = iommu_do_domctl(op, d, u_domctl); > + if ( ret < 0 && ret != -ENXIO ) > + return ret; If this is where you want the ENXIO for that the previous patch switched to, then I see no reason for that earlier change at all. Inside the hypervisor you can simply figure out what the right thing to do is; you could avoid calling iommu_do_domctl() altogether and call ... > + /* > + * 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 after IOMMU > + * processing 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); ... this one right away, for example. (Which of course doesn't eliminate the question towards the overloading done here, which iirc was raised before.) Jan
On 22/07/2025 15:34, Jan Beulich wrote: > On 22.07.2025 13:41, Oleksii Moisieiev wrote: >> @@ -859,7 +860,25 @@ 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; >> + >> ret = iommu_do_domctl(op, d, u_domctl); >> + if ( ret < 0 && ret != -ENXIO ) >> + return ret; > If this is where you want the ENXIO for that the previous patch switched to, > then I see no reason for that earlier change at all. Inside the hypervisor > you can simply figure out what the right thing to do is; you could avoid > calling iommu_do_domctl() altogether and call ... My point was to leave the decision making to the calls themselves. So iommu_do_domctl will make a decision whether to process the node or not, same for the scmi call. I can figure out if there is a need to call iommu_do_domctl or sci_do_domctl here but this means moving part of the logic from specific calls to the common code. >> + /* >> + * 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 after IOMMU >> + * processing 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); > ... this one right away, for example. (Which of course doesn't eliminate the > question towards the overloading done here, which iirc was raised before.) > > Jan
On 28.08.2025 07:48, Oleksii Moisieiev wrote: > > > On 22/07/2025 15:34, Jan Beulich wrote: >> On 22.07.2025 13:41, Oleksii Moisieiev wrote: >>> @@ -859,7 +860,25 @@ 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; >>> + >>> ret = iommu_do_domctl(op, d, u_domctl); >>> + if ( ret < 0 && ret != -ENXIO ) >>> + return ret; >> If this is where you want the ENXIO for that the previous patch switched to, >> then I see no reason for that earlier change at all. Inside the hypervisor >> you can simply figure out what the right thing to do is; you could avoid >> calling iommu_do_domctl() altogether and call ... > > My point was to leave the decision making to the calls themselves. > So iommu_do_domctl will make a decision whether to process the node or > not, same for the scmi call. > I can figure out if there is a need to call iommu_do_domctl or > sci_do_domctl here but this means moving > part of the logic from specific calls to the common code. To avoid that, maybe it needs doing the other way around? I.e. try ... >>> + /* >>> + * 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 after IOMMU >>> + * processing 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); ... this first? Jan
© 2016 - 2025 Red Hat, Inc.