From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58122) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1abMQK-0006Zs-D2 for qemu-devel@nongnu.org; Thu, 03 Mar 2016 01:07:45 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1abMQF-0002ny-Ie for qemu-devel@nongnu.org; Thu, 03 Mar 2016 01:07:44 -0500 Received: from mail-pf0-x241.google.com ([2607:f8b0:400e:c00::241]:35438) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1abMQF-0002nq-3H for qemu-devel@nongnu.org; Thu, 03 Mar 2016 01:07:39 -0500 Received: by mail-pf0-x241.google.com with SMTP id w128so765298pfb.2 for ; Wed, 02 Mar 2016 22:07:39 -0800 (PST) References: <1456823441-46757-1-git-send-email-aik@ozlabs.ru> <1456823441-46757-10-git-send-email-aik@ozlabs.ru> <20160303053633.GJ1620@voom.redhat.com> From: Alexey Kardashevskiy Message-ID: <56D7D4A5.40602@ozlabs.ru> Date: Thu, 3 Mar 2016 17:07:33 +1100 MIME-Version: 1.0 In-Reply-To: <20160303053633.GJ1620@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 09/16] vfio: Generalize IOMMU memory listener 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 04:36 PM, David Gibson wrote: > On Tue, Mar 01, 2016 at 08:10:34PM +1100, Alexey Kardashevskiy wrote: >> At the moment VFIOContainer uses one memory listener which listens on >> PCI address space for both Type1 and sPAPR IOMMUs. Soon we will need >> another listener to listen on RAM; this will do DMA memory >> pre-registration for sPAPR guests which basically pins all guest >> pages in the host physical RAM. >> >> This introduces VFIOMemoryListener which is wrapper for MemoryListener >> and stores a pointer to the container. This allows having multiple >> memory listeners for the same container. This replaces the existing >> @listener with @iommu_listener. >> >> This should cause no change in behavior. > > This is nonsense. > > The two listeners you're talking about have (or should have) both a > different AS they're listening on, They do have different AS. > *and* different notification > functions. They do use totally different region_add/region_del, later in the series. > Since they have nothing in common, there's no point trying > to build a common structure for them. They use the same VFIOContainer pointer. VFIOMemoryListener is made of MemoryListener and VFIOContainer, and that's it. Ok, I'll get rid of VFIOMemoryListener. It is just hard sometime to understand what bits I have to reuse and which I do not, constant argument... > >> >> Signed-off-by: Alexey Kardashevskiy >> --- >> hw/vfio/common.c | 41 +++++++++++++++++++++++++++++++---------- >> include/hw/vfio/vfio-common.h | 9 ++++++++- >> 2 files changed, 39 insertions(+), 11 deletions(-) >> >> diff --git a/hw/vfio/common.c b/hw/vfio/common.c >> index ca3fd47..0e67a5a 100644 >> --- a/hw/vfio/common.c >> +++ b/hw/vfio/common.c >> @@ -318,10 +318,10 @@ static hwaddr vfio_container_granularity(VFIOContainer *container) >> return (hwaddr)1 << ctz64(container->iova_pgsizes); >> } >> >> -static void vfio_listener_region_add(MemoryListener *listener, >> +static void vfio_listener_region_add(VFIOMemoryListener *vlistener, >> MemoryRegionSection *section) >> { >> - VFIOContainer *container = container_of(listener, VFIOContainer, listener); >> + VFIOContainer *container = vlistener->container; >> hwaddr iova, end; >> Int128 llend; >> void *vaddr; >> @@ -425,10 +425,10 @@ fail: >> } >> } >> >> -static void vfio_listener_region_del(MemoryListener *listener, >> +static void vfio_listener_region_del(VFIOMemoryListener *vlistener, >> MemoryRegionSection *section) >> { >> - VFIOContainer *container = container_of(listener, VFIOContainer, listener); >> + VFIOContainer *container = vlistener->container; >> hwaddr iova, end; >> int ret; >> MemoryRegion *iommu = NULL; >> @@ -492,14 +492,33 @@ static void vfio_listener_region_del(MemoryListener *listener, >> } >> } >> >> -static const MemoryListener vfio_memory_listener = { >> - .region_add = vfio_listener_region_add, >> - .region_del = vfio_listener_region_del, >> +static void vfio_iommu_listener_region_add(MemoryListener *listener, >> + MemoryRegionSection *section) >> +{ >> + VFIOMemoryListener *vlistener = container_of(listener, VFIOMemoryListener, >> + listener); >> + >> + vfio_listener_region_add(vlistener, section); >> +} >> + >> + >> +static void vfio_iommu_listener_region_del(MemoryListener *listener, >> + MemoryRegionSection *section) >> +{ >> + VFIOMemoryListener *vlistener = container_of(listener, VFIOMemoryListener, >> + listener); >> + >> + vfio_listener_region_del(vlistener, section); >> +} >> + >> +static const MemoryListener vfio_iommu_listener = { >> + .region_add = vfio_iommu_listener_region_add, >> + .region_del = vfio_iommu_listener_region_del, >> }; >> >> static void vfio_listener_release(VFIOContainer *container) >> { >> - memory_listener_unregister(&container->listener); >> + memory_listener_unregister(&container->iommu_listener.listener); >> } >> >> int vfio_mmap_region(Object *obj, VFIORegion *region, >> @@ -768,9 +787,11 @@ static int vfio_connect_container(VFIOGroup *group, AddressSpace *as) >> goto free_container_exit; >> } >> >> - container->listener = vfio_memory_listener; >> + container->iommu_listener.container = container; >> + container->iommu_listener.listener = vfio_iommu_listener; >> >> - memory_listener_register(&container->listener, container->space->as); >> + memory_listener_register(&container->iommu_listener.listener, >> + container->space->as); >> >> if (container->error) { >> ret = container->error; >> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h >> index 9ffa681..b6b736c 100644 >> --- a/include/hw/vfio/vfio-common.h >> +++ b/include/hw/vfio/vfio-common.h >> @@ -57,12 +57,19 @@ typedef struct VFIOAddressSpace { >> QLIST_ENTRY(VFIOAddressSpace) list; >> } VFIOAddressSpace; >> >> +typedef struct VFIOContainer VFIOContainer; >> + >> +typedef struct VFIOMemoryListener { >> + struct MemoryListener listener; >> + VFIOContainer *container; >> +} VFIOMemoryListener; >> + >> struct VFIOGroup; >> >> typedef struct VFIOContainer { >> VFIOAddressSpace *space; >> int fd; /* /dev/vfio/vfio, empowered by the attached groups */ >> - MemoryListener listener; >> + VFIOMemoryListener iommu_listener; >> int error; >> bool initialized; >> /* > -- Alexey