From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44706) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZXKz2-0003Ys-Ho for qemu-devel@nongnu.org; Wed, 02 Sep 2015 23:14:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZXKyz-000838-BO for qemu-devel@nongnu.org; Wed, 02 Sep 2015 23:14:40 -0400 Received: from mail-pa0-f46.google.com ([209.85.220.46]:33446) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZXKyz-00082Q-4t for qemu-devel@nongnu.org; Wed, 02 Sep 2015 23:14:37 -0400 Received: by pacex6 with SMTP id ex6so26391536pac.0 for ; Wed, 02 Sep 2015 20:14:36 -0700 (PDT) References: <1441181763-4400-1-git-send-email-aik@ozlabs.ru> <1441181763-4400-2-git-send-email-aik@ozlabs.ru> <55E6C4E7.9070203@ozlabs.ru> <20150903020708.GF6537@voom.redhat.com> From: Alexey Kardashevskiy Message-ID: <55E7BB17.9050306@ozlabs.ru> Date: Thu, 3 Sep 2015 13:14:31 +1000 MIME-Version: 1.0 In-Reply-To: <20150903020708.GF6537@voom.redhat.com> Content-Type: text/plain; charset=koi8-r; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH qemu 1/2] spapr_pci_vfio: Remove redundant spapr-pci-vfio-host-bridge List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Gibson Cc: qemu-ppc@nongnu.org, qemu-devel@nongnu.org, Gavin Shan , Alexander Graf On 09/03/2015 12:07 PM, David Gibson wrote: > On Wed, Sep 02, 2015 at 07:44:07PM +1000, Alexey Kardashevskiy wrote: >> On 09/02/2015 06:16 PM, Alexey Kardashevskiy wrote: >>> sPAPRTCETable is handling 2 TCE tables already: >>> >>> 1) guest view of the TCE table - emulated devices use only this table; >>> >>> 2) hardware IOMMU table - VFIO PCI devices use it for actual work but >>> it does not replace 1) and it is not visible to the guest. >>> The initialization of this table is driven by vfio-pci device, >>> DMA map/unmap requests are handled via MemoryListener so there is very >>> little to do in spapr-pci-vfio-host-bridge. >>> >>> This moves VFIO bits to the generic spapr-pci-host-bridge which allows >>> putting emulated and VFIO devices on the same PHB. It is still possible >>> to create multiple PHBs and avoid sharing PHB resouces for emulated and >>> VFIO devices. >>> >>> If there is no VFIO-PCI device attaches, no special ioctls will be called. >>> If there are some VFIO-PCI devices attached, PHB may refuse to attach >>> another VFIO-PCI device if a VFIO container on the host kernel side >>> does not support container sharing. >>> >>> This makes spapr-pci-vfio-host-bridge type equal to spapr-pci-host-bridge >>> except it has an additional "iommu" property so spapr-pci-vfio-host-bridge >>> still should be used for VFIO devices. The next patch will remove IOMMU ID >>> property and allow putting VFIO-PCI devices onto spapr-pci-host-bridge. >>> >>> This adds a number of VFIO-PCI devices currently attached to a PHB as >>> PHB needs to know whether to do DMA setup for VFIO or not. Since >>> at the moment of the PHB's realize() invocation we cannot tell yet >>> how many VFIO-PCI devices are there (they are not attached yet), >>> this moves DMA setup to the reset handler. >>> >>> This moves PCI device lookup from spapr_phb_vfio_eeh_set_option() to >>> rtas_ibm_set_eeh_option() as we need to know if the device is "vfio-pci" >>> and decide whether to call spapr_phb_vfio_eeh_set_option() or not. >>> >>> This should cause no behavioural change. > > [snip] > >>> static void spapr_phb_reset(DeviceState *qdev) >>> { >>> + sPAPRPHBState *sphb = SPAPR_PCI_HOST_BRIDGE(qdev); >>> + sPAPRTCETable *tcet; >>> + >>> /* Reset the IOMMU state */ >>> object_child_foreach(OBJECT(qdev), spapr_phb_children_reset, NULL); >>> + >>> + if (spapr_phb_dma_capabilities_update(sphb)) { >>> + return; >>> + } >>> + >>> + /* Register default 32bit DMA window */ >>> + tcet = spapr_tce_find_by_liobn(sphb->dma_liobn); >>> + if (!tcet) { >>> + const unsigned nb = sphb->dma32_window_size >> SPAPR_TCE_PAGE_SHIFT; >>> + tcet = spapr_tce_new_table(DEVICE(sphb), sphb->dma_liobn, >>> + sphb->dma32_window_start, >>> + SPAPR_TCE_PAGE_SHIFT, nb, >>> + sphb->vfio_num > 0); >>> + if (!tcet) { >>> + error_report("No default TCE table for %s", sphb->dtbusname); >>> + return; >>> + } >>> + >>> + memory_region_add_subregion(&sphb->iommu_root, 0, >> >> >> Of course bug... should be bus_offset instead of 0. I'll wait a bit for more >> reviews and respin later. > > Are you sure? I thought the idea of the iommu_root MR was that it was > always at offset 0, and the "real" IOMMU region sat inside it at the > right offset, allowing things to be moved around in the case of DDW. The "root iommu" patches are coming next. At this point I am just getting rid of vfio phb. > It was at offset 0 in the original realize() time > memory_region_add_subregion(). It was 0 for emulated PHB but it was tcet->bus_offset for VFIO PHB. -- Alexey