From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH v2 1/5] IOMMU: make page table population preemptible Date: Fri, 13 Dec 2013 15:44:21 +0000 Message-ID: <52AB2B55.9030508@citrix.com> References: <52A744B7020000780010BEF1@nat28.tlf.novell.com> <52A74539020000780010BF1F@nat28.tlf.novell.com> <52A8B1A5.9040302@citrix.com> <52AB20DB020000780010D060@nat28.tlf.novell.com> <52AB230E.90607@citrix.com> <52AB38A3020000780010D1DF@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1VrUuh-0005Yp-US for xen-devel@lists.xenproject.org; Fri, 13 Dec 2013 15:44:28 +0000 In-Reply-To: <52AB38A3020000780010D1DF@nat28.tlf.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: George Dunlap , xen-devel , Keir Fraser , xiantao.zhang@intel.com, Tim Deegan List-Id: xen-devel@lists.xenproject.org On 13/12/2013 15:41, Jan Beulich wrote: >>>> On 13.12.13 at 16:09, Andrew Cooper wrote: >> On 13/12/2013 13:59, Jan Beulich wrote: >>> @@ -290,15 +307,14 @@ static int assign_device(struct domain * >>> rc); >>> } >>> >>> - if ( has_arch_pdevs(d) && !need_iommu(d) ) >>> + done: >>> + if ( !has_arch_pdevs(d) && need_iommu(d) ) >> We now have a case where, for the first device, we could set up >> pagetables for a large domain, get an error with assignment, then tear >> them all back down. (-EBUSY from pci_get_pdev() looks like a good >> non-fatal candidate for causing this behaviour) >> >> I am wondering whether this is better for worse than the race condition >> where a guest couldn't use the device. A guest could not reasonably >> expect to use a device before the toolstack is done setting it up. A >> buggy toolstack could quite easily tie up a lot of Xen time creating and >> destroying complete iommu pagetable sets. > I don't think it's worth worrying about buggy tool stacks here - > they should simply get fixed. > > Furthermore this change in operation ordering is only a (nice) > side effect, the necessary cleanup seemed easier with the > order changed. And said time window would have grown with > the added preemption handling. > > Jan > All true. Reviewed-by: Andrew Cooper