From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54327) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1byYXL-0001lF-GP for qemu-devel@nongnu.org; Mon, 24 Oct 2016 02:15:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1byYXI-0000rr-6E for qemu-devel@nongnu.org; Mon, 24 Oct 2016 02:15:07 -0400 Received: from ozlabs.org ([103.22.144.67]:50827) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1byYXH-0000rd-C0 for qemu-devel@nongnu.org; Mon, 24 Oct 2016 02:15:04 -0400 Date: Mon, 24 Oct 2016 17:03:43 +1100 From: David Gibson Message-ID: <20161024060343.GD11052@umbus.fritz.box> References: <1476719064-9242-1-git-send-email-bd.aviv@gmail.com> <20161017100736.68a56fd9@t450s.home> <20161020140608.0f373ea9@t450s.home> <20161021005005.GV11140@umbus.fritz.box> <20161021051708.GP15168@pxdev.xzpeter.org> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="8w3uRX/HFJGApMzv" Content-Disposition: inline In-Reply-To: <20161021051708.GP15168@pxdev.xzpeter.org> Subject: Re: [Qemu-devel] [PATCH v4 RESEND 0/3] IOMMU: intel_iommu support map and unmap notifications List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Xu Cc: Alex Williamson , "Aviv B.D." , qemu-devel@nongnu.org, "Michael S. Tsirkin" , Jan Kiszka --8w3uRX/HFJGApMzv Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Oct 21, 2016 at 01:17:08PM +0800, Peter Xu wrote: > On Fri, Oct 21, 2016 at 11:50:05AM +1100, David Gibson wrote: >=20 > [...] >=20 > > > > In my setup the VFIO registered two memory areas with one page of > > > > unregistered memory > > > > between them. > > > >=20 > > > > When I'm calling memory_region_notify_iommu it calls the notifier f= unction > > > > of VFIO twice > > > > when the second time is failing with warning to console as the new = mapping > > > > is already present. > > > >=20 > > > > The notifier function of VFIO should ignore IOMMUTLBEntry that is n= ot in > > > > the correct > > > > range. > > >=20 > > > Hmm, right vfio_listener_region_add() is called for a > > > MemoryRegionSection, but then we add an iommu notifier to the > > > MemoryRegion, so we end up with a notifier per MemoryRegionSection > > > regardless of whether they're backed by the same MemoryRegion. Seems > > > like we need a MemoryRegion based list of VFIOGuestIOMMUs so we only > > > register once per MemoryRegion and then sort though the list of > > > VFIOGuestIOMMUs for a given MemoryRegion to find the one affected. > > > David, does that sound right to you? >=20 > I think we already have such a list (VFIOContainer.giommu_list)? Can > we use that to do it? When we try to add a new IOMMU notifier for > specific MemoryRegion, we can first traverse VFIOContainer.giommu_list > and see whether there are existing MemoryRegion registered, and we > only register if it's the first one. Yes, I think that would work, but I still prefer the alternate approach I describe below. > > Well, I think that would work. But I think it would be better to fix > > it from the other side: > >=20 > > We add the range to be notified into the IOMMUNotifier structure and > > filter based on that range in memory_region_notify_iommu. > >=20 > > It means a little more list searching and filtering on notify, but it > > avoids having to have another list and search on the VFIO side. I > > think it will also better deal with cases where part of an IOMMU > > mapped region is inaccessible due to an intermediate bridge. >=20 > IIUC, this will still need to keep several VFIOGuestIOMMUs which > contains exactly the same content? Well, it would no longer be exactly the same content since the different ranges would be stored within the IOMMUNotifier substructure. But, yes, it would be very similar. I think it's still worth it for improved clarity, and the possibility to handle in future cases where part of the IOMMU region simply isn't accessible at all in the CPU address space. --=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 --8w3uRX/HFJGApMzv Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJYDaQ9AAoJEGw4ysog2bOSO2sP/1xtxrZYD9285JKfaZ4oc/9H fQaglCnWnxtDp3/IPuCAPJmJApTEmAJargiJmSEfSoMzUN1TjDfZWAgkugjiqAo8 Vb0cYHrk3hPGx7v/QW6/nn4oXf12Nr/T0D1WtZjpl7Pf+U+5elHbds1vTccWlU8f 4H5QW/gtrsKYmzIznqEseO/NvYfHWjN8ftaI2Djz9OmUzT7dRLU8r2M6Gi6injMO x6ntXONfnKLUVJ9n3C1hhAksWBFlDvuXMBeUIy4DePzcSFZH+ItbPm7dfyjuhzzb 0IfzS3/WTnXHAeZMKwRB0NJ9swHzyv2T3pZ+VJnFqo6PX+Ch3txq4CKDPZ2cdZgA Yonw0xLvudY5JiLPGmgxwdpPUMqKXkP6wvvhuN6JdxQDxsqjII+tJryNa45uLRJg kGGlke8wuw0783USOoSl4unkY84K2jIJ2+pb9PnR9CG3cMdZJrc9BOCpXNJahQRz WyKZph1uplPisrvl9haBXgQjqnmP/sGyKxpjkvz/EEo0f9SJDWrvOyJYxL4vkim3 vyNprADNBtmI+NTr3WQljdD79GzI3iwEtW8Jwd4unTPKo6qeRHx47YcNItTTzfx4 8LRuF/935R/Fvflm7tWstw3hvdpLMAqE7v1QBGKxtGmKEqtQeVLwRG5lh2/hRGE9 +LK2vhpkYy8vkF8oYZdX =0ppz -----END PGP SIGNATURE----- --8w3uRX/HFJGApMzv--