From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH v3 16/24] xen/passthrough: Introduce iommu_construct Date: Tue, 20 Jan 2015 17:11:38 +0000 Message-ID: <54BE8C4A.7030801@linaro.org> References: <1421159133-31526-1-git-send-email-julien.grall@linaro.org> <1421159133-31526-17-git-send-email-julien.grall@linaro.org> <54BD46C60200007800056A6B@mail.emea.novell.com> <54BE6611.1070701@linaro.org> <54BE932502000078000572A8@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta4.messagelabs.com ([85.158.143.247]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1YDcLX-0003nm-LH for xen-devel@lists.xenproject.org; Tue, 20 Jan 2015 17:12:07 +0000 Received: by mail-we0-f180.google.com with SMTP id m14so13848393wev.11 for ; Tue, 20 Jan 2015 09:12:06 -0800 (PST) In-Reply-To: <54BE932502000078000572A8@mail.emea.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: xen-devel@lists.xenproject.org, stefano.stabellini@citrix.com, ian.campbell@citrix.com, tim@xen.org List-Id: xen-devel@lists.xenproject.org On 20/01/15 16:40, Jan Beulich wrote: >>>> On 20.01.15 at 15:28, wrote: >> On 19/01/15 17:02, Jan Beulich wrote: >>>>>> On 13.01.15 at 15:25, 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. While this function is currently only used for DOM0, this will be used in a later patch for guest non-PCI passthrough. Regards, -- Julien Grall