Re: [Xen-devel] [PATCH v3 16/24] xen/passthrough: Introduce iommu_construct

["Jan Beulich" <JBeulich@xxxxxxxx>]

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

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel