From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46689) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c9mBc-0006V3-2E for qemu-devel@nongnu.org; Thu, 24 Nov 2016 00:03:05 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c9mBY-0003qE-V0 for qemu-devel@nongnu.org; Thu, 24 Nov 2016 00:03:04 -0500 Received: from ozlabs.org ([103.22.144.67]:48883) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1c9mBY-0003oY-1p for qemu-devel@nongnu.org; Thu, 24 Nov 2016 00:03:00 -0500 Date: Thu, 24 Nov 2016 15:04:35 +1100 From: David Gibson Message-ID: <20161124040435.GA23872@umbus.fritz.box> References: <1479892858-4218-1-git-send-email-peterx@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="ikeVEW9yuYc//A+q" Content-Disposition: inline In-Reply-To: <1479892858-4218-1-git-send-email-peterx@redhat.com> Subject: Re: [Qemu-devel] [PATCH] memory: add section range info for IOMMU notifier List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Xu Cc: qemu-devel@nongnu.org, pbonzini@redhat.com, alex.williamson@redhat.com --ikeVEW9yuYc//A+q Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Nov 23, 2016 at 05:20:58PM +0800, Peter Xu wrote: > In this patch, IOMMUNotifier.{start|end} are introduced to store section > information for a specific notifier. When notification occurs, we not > only check the notification type (MAP|UNMAP), but also check whether the > notified iova is in the range of specific IOMMU notifier, and skip those > notifiers if not in the listened range. >=20 > When removing an region, we need to make sure we removed the correct > VFIOGuestIOMMU by checking the IOMMUNotifier.start address as well. >=20 > Suggested-by: David Gibson > Signed-off-by: Peter Xu >=20 > --- > This patch fixes the same issue with the following one: >=20 > [PATCH] vfio: avoid adding same iommu mr for notify >=20 > Alex/David, would you please help provide some review comments on either > of the two patches? When we can settle down the best way, then I'll drop > the other one (I still prefer the other one...). Thanks, >=20 > Signed-off-by: Peter Xu > --- > hw/vfio/common.c | 7 ++++++- > include/exec/memory.h | 3 +++ > memory.c | 4 +++- > 3 files changed, 12 insertions(+), 2 deletions(-) >=20 > diff --git a/hw/vfio/common.c b/hw/vfio/common.c > index 801578b..c3db115 100644 > --- a/hw/vfio/common.c > +++ b/hw/vfio/common.c > @@ -455,6 +455,10 @@ static void vfio_listener_region_add(MemoryListener = *listener, > giommu->container =3D container; > giommu->n.notify =3D vfio_iommu_map_notify; > giommu->n.notifier_flags =3D IOMMU_NOTIFIER_ALL; > + giommu->n.start =3D section->offset_within_address_space; I think this needs to be offset_within_region rather than offset_within_address_space. The IOVAs used in the IOMMUTLBEntry are relative to the MR, not the enclosing AS (in fact there could be several enclosing ASes with the right aliasing). See for example put_tce_emu() - the (ioba - tcet->bus_offset) expression is effectively converting the AS relative ioba into an MR relative address. > + llend =3D int128_add(int128_make64(giommu->n.start), section->si= ze); > + llend =3D int128_sub(llend, int128_one()); > + giommu->n.end =3D int128_get64(llend); > QLIST_INSERT_HEAD(&container->giommu_list, giommu, giommu_next); > =20 > memory_region_register_iommu_notifier(giommu->iommu, &giommu->n); > @@ -525,7 +529,8 @@ static void vfio_listener_region_del(MemoryListener *= listener, > VFIOGuestIOMMU *giommu; > =20 > QLIST_FOREACH(giommu, &container->giommu_list, giommu_next) { > - if (giommu->iommu =3D=3D section->mr) { > + if (giommu->iommu =3D=3D section->mr && > + giommu->n.start =3D=3D section->offset_within_address_sp= ace) { Same here. > memory_region_unregister_iommu_notifier(giommu->iommu, > &giommu->n); > QLIST_REMOVE(giommu, giommu_next); > diff --git a/include/exec/memory.h b/include/exec/memory.h > index 9728a2f..87357ea 100644 > --- a/include/exec/memory.h > +++ b/include/exec/memory.h > @@ -84,6 +84,9 @@ typedef enum { > struct IOMMUNotifier { > void (*notify)(struct IOMMUNotifier *notifier, IOMMUTLBEntry *data); > IOMMUNotifierFlag notifier_flags; > + /* Notify for address space range start <=3D addr <=3D end */ > + hwaddr start; > + hwaddr end; > QLIST_ENTRY(IOMMUNotifier) node; > }; > typedef struct IOMMUNotifier IOMMUNotifier; > diff --git a/memory.c b/memory.c > index 33110e9..f89d047 100644 > --- a/memory.c > +++ b/memory.c > @@ -1662,7 +1662,9 @@ void memory_region_notify_iommu(MemoryRegion *mr, > } > =20 > QLIST_FOREACH(iommu_notifier, &mr->iommu_notify, node) { > - if (iommu_notifier->notifier_flags & request_flags) { > + if (iommu_notifier->notifier_flags & request_flags && > + iommu_notifier->start <=3D entry.iova && > + iommu_notifier->end >=3D entry.iova) { > iommu_notifier->notify(iommu_notifier, &entry); > } > } Apart from that, I think it looks correct. --=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 --ikeVEW9yuYc//A+q Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJYNmbRAAoJEGw4ysog2bOSjz4QALBDIbmQayDqUamj8cxEn8R2 4SN1dMFW9gBKGxi9Gy3JkXf7pQkA1imTQ0pf8AB6F8m2KehmBFC4m+VAUdMurkoe 7IQyxN6W0Ic0SNZ2M7vYi5Ud7KABSGNq7Zz41noo47l5V7EGOfOB9rDmZ/1NMywN MUt2g9Vtf3UJuD1sgbcW6SzDZqim8qJCEmCU3CujJDRLwzaEBJOQOmBmN+0BUvEu onETK0PbVkFiyduHhhd1euMh9X11zB0fZX25nVJ+dTMbiPKsWXIszoBnEhGjvQXj Px0Rzb9qUDFBXsEa+hnXlQCW0NGOj82vVFjd33PfuFdzPNFDdG/nX3pvCTGvP3DR //aRX0jwl1tSJc/QMBcVYrjYx3hqoTjog8s3f+RB10hdEllpZHDF9AFJhxVqyfz2 ChCm2p6lW/+KJqH7Xd7LX3cKlvKLdx5wGg74/Jp4gHPYGdvBPdw1cszum0Fr/CKt 2zmwDbMbX6jt3al9HHu9eunJgISRvCDkT+3LjxzTxDmuf3FRX9XoXZIDOkzPfqzj dJea8Fy6YLK9AgKBSpI2cCcp4trZb61ovPNYIZPU9pGraBat4iPRpidWt8xLN9lc bAhaFwYTOSCGRFJEYsf5D4BLdFi4yaQ6fMUwbmUXPE6bdU1xz61XJgYB8hPnPp3d EoAUYfIYBbWtf4g2wEeD =ZVNc -----END PGP SIGNATURE----- --ikeVEW9yuYc//A+q--