[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH v3 16/24] xen/passthrough: Introduce iommu_construct
On 21/01/2015 10:23, Jan Beulich wrote:
On 20.01.15 at 18:11, <julien.grall@xxxxxxxxxx> wrote:
On 20/01/15 16:40, Jan Beulich wrote:
On 20.01.15 at 15:28, <julien.grall@xxxxxxxxxx> wrote:
On 19/01/15 17:02, Jan Beulich wrote:
On 13.01.15 at 15:25, <julien.grall@xxxxxxxxxx> wrote:
--- a/xen/drivers/passthrough/device_tree.c
+++ b/xen/drivers/passthrough/device_tree.c
@@ -41,6 +41,10 @@ int iommu_assign_dt_device(struct domain *d, struct
dt_device_node *dev)
if ( !list_empty(&dev->domain_list) )
goto fail;
+ rc = iommu_construct(d);
+ if ( rc )
+ goto fail;
Considering that the only (current) caller of this it domain_build.c I'm
afraid you're going to get into trouble if you get back -ERESTART
here. Note that on x86 Dom0 setup works via iommu_hwdom_init(),
which deals with the preemption needs (at that point in time) by
calling process_pending_softirqs() every once in a while.
iommu_hwdom_init is also called for ARM (it's part of the common domain
creation code). So, I don't see any issue here as we match the same
behavior as PCI.
FWIW, on the previous version you asked to check the need_iommu(d) in
iommu_construct. For DOM0 it will return 0 and therefore never return
-ERESTART.
Quoting the function:
+int iommu_construct(struct domain *d)
+{
+ int rc = 0;
+
+ if ( need_iommu(d) > 0 )
+ return 0;
+
+ if ( !iommu_use_hap_pt(d) )
+ {
+ rc = arch_iommu_populate_page_table(d);
+ if ( rc )
+ return rc;
+ }
+
+ d->need_iommu = 1;
+
+ return rc;
+}
If need_iommu() returns 0 for Dom0, then the early return won't get
used. Hence I don't follow your comment above. And if what you say
there was correct, then I don't understand why you add the call
quoted at the very top in the first place (again taking into consideration
that - afaict - the only [current] caller is in domain_build.c).
I don't understand what is the issue in the device tree use case. As I
said, assign_device in the pci do exactly the same things.
Sure, but it's not being called for Dom0, but only out of the domctl
handler.
Hmmm right. It's the main difference between non-PCI and PCI passthrough.
While this function is currently only used for DOM0, this will be used
in a later patch for guest non-PCI passthrough.
Okay, but you shouldn't break (or alter in [seemingly] benign ways) the
Dom0 case imo.
As iommu_hwdom_init is initialized correctly the IOMMU for DOM0,
iommu_construct is a no-op.
Would an if ( need_iommu(d) ) will be more clear? Maybe we an assert
(!is_hardware_domain(d)).
Regards,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
