From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47507) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b6iD9-0005Sc-CN for qemu-devel@nongnu.org; Sat, 28 May 2016 13:39:44 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b6iD4-0002nm-Bt for qemu-devel@nongnu.org; Sat, 28 May 2016 13:39:42 -0400 Received: from mx1.redhat.com ([209.132.183.28]:52020) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b6iD4-0002nh-41 for qemu-devel@nongnu.org; Sat, 28 May 2016 13:39:38 -0400 Date: Sat, 28 May 2016 11:39:36 -0600 From: Alex Williamson Message-ID: <20160528113936.48e67fac@ul30vt.home> In-Reply-To: References: <1463847590-22782-1-git-send-email-bd.aviv@gmail.com> <1463847590-22782-4-git-send-email-bd.aviv@gmail.com> <20160523115342.636a5164@ul30vt.home> <20160526145844.552b21fb@t450s.home> <20160528100220.2d48ff2d@ul30vt.home> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v3 3/3] IOMMU: Integrate between VFIO and vIOMMU to support device assignment List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Aviv B.D." Cc: Jan Kiszka , qemu-devel@nongnu.org, Peter Xu , "Michael S. Tsirkin" On Sat, 28 May 2016 16:10:55 +0000 "Aviv B.D." wrote: > On Sat, May 28, 2016 at 7:02 PM Alex Williamson > wrote: > > > On Sat, 28 May 2016 10:52:58 +0000 > > "Aviv B.D." wrote: > > > > > Hi, > > > Your idea to search the relevent VTDAddressSpace and call it's notifier > > > will > > > probably work. Next week I'll try to implement it (for now with the > > costly > > > scan > > > of each context). > > > > I think an optimization we can make is to use pci_for_each_bus() and > > pci_for_each_device() to scan only context entries where devices are > > present. Then for each context entry, retrieve the DID, if it matches > > the invalidation domain_id, retrieve the VTDAddressSpace and perform a > > memory_region_notify_iommu() using VTDAddressSpace.iommu. Still > > horribly inefficient, but an improvement over walking all context > > entries and avoids gratuitous callbacks between unrelated drivers in > > QEMU. > > > > Thanks for the references on how I can do it. :) > > > > > Overall, I have very little faith that this will be the only change > > required to make this work though. For instance, if a device is added > > or removed from a domain, where is that accounted for? Ideally this > > should trigger the region_add/region_del listener callbacks, but I > > don't see how that works with how VT-d creates a fixed VTDAddressSpace > > per device, and in fact how our QEMU memory model doesn't allow the > > address space of a device to be dynamically aliased against other > > address spaces or really changed at all. > > > > > I still not sure if populating the MemoryRegion will suffice for hot plug > > > vfio > > > device but i'll try to look into it. > > > > > > As far as I understand the memory_region_iommu_replay function, it still > > > scans > > > the whole 64bit address space, and therefore may hang the VM for a long > > > time. > > > > Then we need to fix that problem, one option might be to make a replay > > callback on MemoryRegionIOMMUOps that walks the page tables for a given > > context entry rather than blindly traversing a 64bit address space. We > > can't simply ignore the issue by #ifdef'ing out the code. I suspect > > there's a lot more involved to make VT-d interact properly with a > > physical device than what's been proposed so far. At every > > invalidation, we need to figure out what's changed and update the host > > mappings. We also need better, more dynamic address space management > > to make the virtual hardware reflect physical hardware when we enable > > things like passthrough mode or have multiple devices sharing an iommu > > domain. I think we're just barely scratching the surface here. Thanks, > > > > Alex > > > > > I agree with you regarding hotplug, therefore I only ifdef this code out > and didn't > delete it. With the call to memory_region_iommu_replay QEMU hangs on startup > with a very long loop that prevent any device assignment with vIOMMU > enabled. > > I'm hoping not to enlarge the scope of this patch to include hotplug device > assignment > with iommu enabled. It's not just hotplug, any case where an existing domain can be applied to a device. The series is incomplete without such support and I won't accept any changes into vfio that disables code that's correct in other contexts. Thanks, Alex