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

[Oleksandr <olekstysh@xxxxxxxxx>]

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.

--
Regards,

Oleksandr Tyshchenko


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel