From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41664) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1abhig-000176-He for qemu-devel@nongnu.org; Thu, 03 Mar 2016 23:52:08 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1abhid-0006rC-AV for qemu-devel@nongnu.org; Thu, 03 Mar 2016 23:52:06 -0500 Date: Fri, 4 Mar 2016 14:44:40 +1100 From: David Gibson Message-ID: <20160304034440.GW1620@voom.redhat.com> References: <1456823441-46757-1-git-send-email-aik@ozlabs.ru> <1456823441-46757-10-git-send-email-aik@ozlabs.ru> <20160303053633.GJ1620@voom.redhat.com> <56D7D4A5.40602@ozlabs.ru> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="6B5oiwbC31sfIr4n" Content-Disposition: inline In-Reply-To: <56D7D4A5.40602@ozlabs.ru> 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: Alexey Kardashevskiy Cc: Alex Williamson , qemu-ppc@nongnu.org, qemu-devel@nongnu.org --6B5oiwbC31sfIr4n Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Mar 03, 2016 at 05:07:33PM +1100, Alexey Kardashevskiy wrote: > 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, >=20 > They do have different AS. >=20 > >*and* different notification > >functions. >=20 > They do use totally different region_add/region_del, later in the series. >=20 > >Since they have nothing in common, there's no point trying > >to build a common structure for them. >=20 > They use the same VFIOContainer pointer. VFIOMemoryListener is made of > MemoryListener and VFIOContainer, and that's it. Right, but you don't need the container pointer. In both cases you can locate the VFIOContainer with container_of. It's a different container_of invocation in each case, but since they're different callback functions, that's no problem. > 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... I think the arguments to try to make things re-used here were based on a mis understanding of what the prereg listener was for and therefore not realizing that it has basically nothing in common with the regular listener. >=20 > > > >> > >>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(VFIOCont= ainer *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 =3D container_of(listener, VFIOContainer,= listener); > >>+ VFIOContainer *container =3D 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 =3D container_of(listener, VFIOContainer,= listener); > >>+ VFIOContainer *container =3D vlistener->container; > >> hwaddr iova, end; > >> int ret; > >> MemoryRegion *iommu =3D NULL; > >>@@ -492,14 +492,33 @@ static void vfio_listener_region_del(MemoryListen= er *listener, > >> } > >> } > >> > >>-static const MemoryListener vfio_memory_listener =3D { > >>- .region_add =3D vfio_listener_region_add, > >>- .region_del =3D vfio_listener_region_del, > >>+static void vfio_iommu_listener_region_add(MemoryListener *listener, > >>+ MemoryRegionSection *sectio= n) > >>+{ > >>+ VFIOMemoryListener *vlistener =3D container_of(listener, VFIOMemor= yListener, > >>+ listener); > >>+ > >>+ vfio_listener_region_add(vlistener, section); > >>+} > >>+ > >>+ > >>+static void vfio_iommu_listener_region_del(MemoryListener *listener, > >>+ MemoryRegionSection *sectio= n) > >>+{ > >>+ VFIOMemoryListener *vlistener =3D container_of(listener, VFIOMemor= yListener, > >>+ listener); > >>+ > >>+ vfio_listener_region_del(vlistener, section); > >>+} > >>+ > >>+static const MemoryListener vfio_iommu_listener =3D { > >>+ .region_add =3D vfio_iommu_listener_region_add, > >>+ .region_del =3D 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 =3D vfio_memory_listener; > >>+ container->iommu_listener.container =3D container; > >>+ container->iommu_listener.listener =3D vfio_iommu_listener; > >> > >>- memory_listener_register(&container->listener, container->space->a= s); > >>+ memory_listener_register(&container->iommu_listener.listener, > >>+ container->space->as); > >> > >> if (container->error) { > >> ret =3D container->error; > >>diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-commo= n.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; > >> /* > > >=20 >=20 --=20 David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson --6B5oiwbC31sfIr4n Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJW2QSoAAoJEGw4ysog2bOSNUIP/jltWlBH1jW3yWqFkNsz5b6I Mau4eiV9UqpkO6TTjdM7G9d1NKcbE4qvfLEU5P2iCMpoxsYz6vAH7VAHQR3Wtebk WDujl6SguNdXGR6N2YFsP7oLzS5np03SkArGSJK409TLnOKnh7KrDlEyaBjJGE9Z GqP43BUwVNbfCJkPfF8Kz/cxEyAgEMEof4AmUreSNRCAul3qjx4a7GBD59t/Ofl0 Dx5H9jQz97kWOtMeio2Pvcbmx+JR/fsSsGDMiF79yV+P3L+6D0YnW9ZBI303CskA yJ8WM7bBVtY+W9HkgE0pE4x1itkNCzEhIIRB19+WOjykMqoWDyaFktapbFtnwPWh wUhh5myXPyeGkUuFppTnGbzOGszkd15VnyVG84DM8PsZQ1OnXcYRwnjx/bmDSksx grDfIDoaH/HIR3Xaf4rjPixc8dx70vAgEkF21QYkW96HsApIF3ONGCAa2W8mUbcj 0SIlH1EhbkYTywdbWfETUJQN9BW9qVlbcrEiZSs9cfq7noF/Xf4vxelDrWgV+T0K ZH3Mcm/XBR1WDBb5g9QklPX3dG0lpJ9MfBYbxf5DM3HyYQUm6bRQJ7GSePtR8Oi3 GM9s8KKD85GHtTGnrfKaHoPUBAITCdMbqcoyzsLByAofkhob9DXbhGzSSaPRnUev 49/PN5Pe+3R5EhabwnI2 =c6BO -----END PGP SIGNATURE----- --6B5oiwbC31sfIr4n--