On 19/09/2019 14:26, Oleksandr wrote:
On 19.09.19 15:29, Julien Grall wrote:
Hi,
Hi, Julien
Hi,
+
+int __init iommu_add_dt_device(struct dt_device_node *np)
Sorry to only realise it now. Would it make sense to have this function
implemented in xen/passthrough/device_tree.c?
Not entirely sure. device_tree.c is a common code. The iommu_fwspec stuff
(widely used in this function) is ARM code.
Some of the device_tree.c already contains Arm specific code (such as device.h).
DT has been only used by Arm so far, so it is sadly fairly tie to the
architecture. But it should be easy to make it generic if needs be (such as
for RISCv).
While iommu_fwspec is been implemented in Arm headers, this could potentially
be made common. So I would still prefer this that function is moved in
device_tree.c
Well, will move. Also I will remove __init as it can be called at runtime...
As for runtime:
The current implementation allows us to fail at early stage if something is
wrong with the device which is behind an IOMMU (and needs to be protected). As
we scan for all present devices, but not only for "passthrough".
The "splitting" into handle_device() for hwdom and iommu_do_dt_domctl() for
other guests will postpone an error recognition to the guest domain creation
time. So, we would have non function system anyway. Wouldn't be better to fail
early instead of continue and fail anyway?
Yes your implementation allows us to fail at early stage but then you are
abusing the function handle_device(). There are actually no promise this will be
called for every device going forward. Think about dom0less where the goal is to
have no dom0 at all.
You are also tying the order of the domains creation as dom0 would have to be
always created before any other domains are created.
Similar (ab)use of handle_device() does not exist, so I would rather not start
to introduce them because this will become quickly unmaintainable as we are
mixing dom0 creation and Xen initialization.
Even without this series, assigning a device to the guest may not be a success
because the IOMMU may not have enough context bank (used for configuring the
IOMMU stage-2) or there are not enough memory. Not been able to add the device
to the IOMMU driver is only another example where it may fail.
But I would not consider this as not functional. The rest of your system may
work perfectly even if this particular device is not usable. There are no
security risk as the IOMMU should block any transaction by default.
I am also not in favor of parsing again the Device-Tree (we have enough of
them), so this is the best solution I can come up. Feel free to suggest
something different.
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
