From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53583) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1abdCF-0004xS-Pg for qemu-devel@nongnu.org; Thu, 03 Mar 2016 19:02:20 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1abdCB-0001WB-Kz for qemu-devel@nongnu.org; Thu, 03 Mar 2016 19:02:19 -0500 Received: from mail-pf0-x243.google.com ([2607:f8b0:400e:c00::243]:33945) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1abdCB-0001Vy-B0 for qemu-devel@nongnu.org; Thu, 03 Mar 2016 19:02:15 -0500 Received: by mail-pf0-x243.google.com with SMTP id 184so2187004pff.1 for ; Thu, 03 Mar 2016 16:02:13 -0800 (PST) References: <1456823441-46757-1-git-send-email-aik@ozlabs.ru> <1456823441-46757-16-git-send-email-aik@ozlabs.ru> <20160303112200.GP1620@voom.redhat.com> From: Alexey Kardashevskiy Message-ID: <56D8D080.5000903@ozlabs.ru> Date: Fri, 4 Mar 2016 11:02:08 +1100 MIME-Version: 1.0 In-Reply-To: <20160303112200.GP1620@voom.redhat.com> Content-Type: text/plain; charset=koi8-r; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [Qemu-ppc] [PATCH qemu v13 15/16] vfio: Move iova_pgsizes from container to guest IOMMU List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Gibson Cc: Alex Williamson , qemu-ppc@nongnu.org, qemu-devel@nongnu.org On 03/03/2016 10:22 PM, David Gibson wrote: > On Tue, Mar 01, 2016 at 08:10:40PM +1100, Alexey Kardashevskiy wrote: >> The page size is an attribute of an IOMMU, not a container as a container >> may contain more just one IOMMU. >> >> This moves iova_pgsizes from VFIOContainer to VFIOGuestIOMMU. >> The following patch will use this. >> >> This removes iova_pgsizes from Type1 IOMMU as it is not used there anyway >> and when it will get guest visible IOMMU, it will use VFIOGuestIOMMU's >> iova_pgsizes. >> >> Signed-off-by: Alexey Kardashevskiy > > Hmm. This makes an important semantic change which.. I'm not sure is > wrong, but certainly isn't adequately addressed in your commit > message. > > The current iova_pgsizes is populated with information about the > *host* IOMMU, whereas you're replacing it with information about the > *guest* IOMMU. Ah, did not realize that. Then it should be not a move but an additional giommu->iova_pgsizes. And this probably answers todo#1 in 16/16 about page masks. > >> --- >> hw/vfio/common.c | 16 ++++------------ >> include/hw/vfio/vfio-common.h | 2 +- >> 2 files changed, 5 insertions(+), 13 deletions(-) >> >> diff --git a/hw/vfio/common.c b/hw/vfio/common.c >> index f2a03e0..42ef1eb 100644 >> --- a/hw/vfio/common.c >> +++ b/hw/vfio/common.c >> @@ -313,9 +313,9 @@ out: >> rcu_read_unlock(); >> } >> >> -static hwaddr vfio_container_granularity(VFIOContainer *container) >> +static hwaddr vfio_container_granularity(VFIOGuestIOMMU *giommu) >> { >> - return (hwaddr)1 << ctz64(container->iova_pgsizes); >> + return (hwaddr)1 << ctz64(giommu->iova_pgsizes); >> } >> >> static hwaddr vfio_iommu_page_mask(MemoryRegion *mr) >> @@ -392,12 +392,13 @@ static void vfio_listener_region_add(VFIOMemoryListener *vlistener, >> section->offset_within_address_space; >> giommu->container = container; >> giommu->n.notify = vfio_iommu_map_notify; >> + giommu->iova_pgsizes = section->mr->iommu_ops->get_page_sizes(section->mr); >> QLIST_INSERT_HEAD(&container->giommu_list, giommu, giommu_next); >> >> memory_region_register_iommu_notifier(giommu->iommu, &giommu->n); >> giommu->iommu->iommu_ops->vfio_notify(section->mr, true); >> memory_region_iommu_replay(giommu->iommu, &giommu->n, >> - vfio_container_granularity(container), >> + vfio_container_granularity(giommu), >> false); >> >> return; >> @@ -743,14 +744,8 @@ static int vfio_connect_container(VFIOGroup *group, AddressSpace *as) >> container->min_iova = 0; >> container->max_iova = (hwaddr)-1; >> >> - /* Assume just 4K IOVA page size */ >> - container->iova_pgsizes = 0x1000; >> info.argsz = sizeof(info); >> ret = ioctl(fd, VFIO_IOMMU_GET_INFO, &info); >> - /* Ignore errors */ >> - if ((ret == 0) && (info.flags & VFIO_IOMMU_INFO_PGSIZES)) { >> - container->iova_pgsizes = info.iova_pgsizes; >> - } >> } else if (ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_IOMMU) || >> ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_v2_IOMMU)) { >> struct vfio_iommu_spapr_tce_info info; >> @@ -811,9 +806,6 @@ static int vfio_connect_container(VFIOGroup *group, AddressSpace *as) >> } >> container->min_iova = info.dma32_window_start; >> container->max_iova = container->min_iova + info.dma32_window_size - 1; >> - >> - /* Assume just 4K IOVA pages for now */ >> - container->iova_pgsizes = 0x1000; >> } else { >> error_report("vfio: No available IOMMU models"); >> ret = -EINVAL; >> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h >> index bcbc5cb..48a1d7f 100644 >> --- a/include/hw/vfio/vfio-common.h >> +++ b/include/hw/vfio/vfio-common.h >> @@ -80,7 +80,6 @@ typedef struct VFIOContainer { >> * future >> */ >> hwaddr min_iova, max_iova; >> - uint64_t iova_pgsizes; >> QLIST_HEAD(, VFIOGuestIOMMU) giommu_list; >> QLIST_HEAD(, VFIOGroup) group_list; >> QLIST_ENTRY(VFIOContainer) next; >> @@ -90,6 +89,7 @@ typedef struct VFIOGuestIOMMU { >> VFIOContainer *container; >> MemoryRegion *iommu; >> hwaddr offset_within_address_space; >> + uint64_t iova_pgsizes; >> Notifier n; >> QLIST_ENTRY(VFIOGuestIOMMU) giommu_next; >> } VFIOGuestIOMMU; > -- Alexey