[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH V4 7/8] iommu/arm: Introduce iommu_add_dt_device API

Hi Julien

+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.

I am happy with the explanation, sounds reasonable. Will modify patch as you suggested.


Oleksandr Tyshchenko

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.