From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49073) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZXL7o-0007W6-5k for qemu-devel@nongnu.org; Wed, 02 Sep 2015 23:23:45 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZXL7k-0003Wi-RA for qemu-devel@nongnu.org; Wed, 02 Sep 2015 23:23:44 -0400 Date: Thu, 3 Sep 2015 13:22:48 +1000 From: David Gibson Message-ID: <20150903032248.GI6537@voom.redhat.com> References: <1441181763-4400-1-git-send-email-aik@ozlabs.ru> <1441181763-4400-2-git-send-email-aik@ozlabs.ru> <20150903020545.GE6537@voom.redhat.com> <55E7BC25.3050709@ozlabs.ru> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="boAH8PqvUi1v1f55" Content-Disposition: inline In-Reply-To: <55E7BC25.3050709@ozlabs.ru> Subject: Re: [Qemu-devel] [PATCH qemu 1/2] spapr_pci_vfio: Remove redundant spapr-pci-vfio-host-bridge List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexey Kardashevskiy Cc: qemu-ppc@nongnu.org, qemu-devel@nongnu.org, Gavin Shan , Alexander Graf --boAH8PqvUi1v1f55 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Sep 03, 2015 at 01:19:01PM +1000, Alexey Kardashevskiy wrote: > On 09/03/2015 12:05 PM, David Gibson wrote: > >On Wed, Sep 02, 2015 at 06:16:02PM +1000, Alexey Kardashevskiy wrote: > >>sPAPRTCETable is handling 2 TCE tables already: > >> > >>1) guest view of the TCE table - emulated devices use only this table; > >> > >>2) hardware IOMMU table - VFIO PCI devices use it for actual work but > >>it does not replace 1) and it is not visible to the guest. > >>The initialization of this table is driven by vfio-pci device, > >>DMA map/unmap requests are handled via MemoryListener so there is very > >>little to do in spapr-pci-vfio-host-bridge. > >> > >>This moves VFIO bits to the generic spapr-pci-host-bridge which allows > >>putting emulated and VFIO devices on the same PHB. It is still possible > >>to create multiple PHBs and avoid sharing PHB resouces for emulated and > >>VFIO devices. > >> > >>If there is no VFIO-PCI device attaches, no special ioctls will be call= ed. > >>If there are some VFIO-PCI devices attached, PHB may refuse to attach > >>another VFIO-PCI device if a VFIO container on the host kernel side > >>does not support container sharing. > >> > >>This makes spapr-pci-vfio-host-bridge type equal to spapr-pci-host-brid= ge > >>except it has an additional "iommu" property so spapr-pci-vfio-host-bri= dge > >>still should be used for VFIO devices. The next patch will remove IOMMU= ID > >>property and allow putting VFIO-PCI devices onto spapr-pci-host-bridge. > >> > >>This adds a number of VFIO-PCI devices currently attached to a PHB as > >>PHB needs to know whether to do DMA setup for VFIO or not. Since > >>at the moment of the PHB's realize() invocation we cannot tell yet > >>how many VFIO-PCI devices are there (they are not attached yet), > >>this moves DMA setup to the reset handler. > >> > >>This moves PCI device lookup from spapr_phb_vfio_eeh_set_option() to > >>rtas_ibm_set_eeh_option() as we need to know if the device is "vfio-pci" > >>and decide whether to call spapr_phb_vfio_eeh_set_option() or not. > >> > >>This should cause no behavioural change. > >> > >>Signed-off-by: Alexey Kardashevskiy > > > >[snip] > >> static int spapr_phb_children_reset(Object *child, void *opaque) > >> { > >> DeviceState *dev =3D (DeviceState *) object_dynamic_cast(child, T= YPE_DEVICE); > >>@@ -1413,8 +1401,42 @@ static int spapr_phb_children_reset(Object *chil= d, void *opaque) > >> > >> static void spapr_phb_reset(DeviceState *qdev) > >> { > >>+ sPAPRPHBState *sphb =3D SPAPR_PCI_HOST_BRIDGE(qdev); > >>+ sPAPRTCETable *tcet; > >>+ > >> /* Reset the IOMMU state */ > >> object_child_foreach(OBJECT(qdev), spapr_phb_children_reset, NULL= ); > >>+ > >>+ if (spapr_phb_dma_capabilities_update(sphb)) { > >>+ return; > >>+ } > >>+ > >>+ /* Register default 32bit DMA window */ > >>+ tcet =3D spapr_tce_find_by_liobn(sphb->dma_liobn); > >>+ if (!tcet) { > >>+ const unsigned nb =3D sphb->dma32_window_size >> SPAPR_TCE_PAG= E_SHIFT; > >>+ tcet =3D spapr_tce_new_table(DEVICE(sphb), sphb->dma_liobn, > >>+ sphb->dma32_window_start, > >>+ SPAPR_TCE_PAGE_SHIFT, nb, > >>+ sphb->vfio_num > 0); > > > >Could delaying the construction of the TCE table object until reset > >time cause problems with migration? i.e. can you be sure that the > >destination will have the TCE table object present and in a suitable > >state to accept the incoming table information from the source? >=20 >=20 > This is a valid concern but the PHB reset handler is called (just checked) > when QEMU is started with "-incoming tcp:vpl2:33333" so yes, I am sure :) Ok. The rest of these patches look fine, so if you fix the bus offset problem, I'll merge into spapr-next. --=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 --boAH8PqvUi1v1f55 Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJV570IAAoJEGw4ysog2bOSFH4QAIMq5XH4ypfmg7INWZSFwmd9 yXIkTjWXNn8I7zgEUS4/czaVA5jKa6ndJdUmFueWsD//XayYooh+Tpw+3TLLQT54 1zru64RMolfFSKh8J9gtXuw6w/mtI4kEXjcjn5KjU4FtdzvRhfJp/rXN1JI8Axl2 5YXoHijHhqmngtB1e+71MBrJ0CqZis/2UnPiJ+MR8fsDo7hy1fbU7/bWH8n6SECe bMFpkHMCWRQb4UiSQdNeWUl8eHBKbwipSlJhJA9XsFbn7ASimyot9y64JaUQoABH FdGNDPP8rrzi15mv7oN9DwJUJVSmMbxePap0j/7jaBU6tCcje003TV9EKApL+57v pQrmNZZDR4ts501xGqQbkrrVfUhdAiNmensgKDq4rw7EqDQCmf0cN7/kU/Z/JxeZ 5qXiJ5aOniudCOrakx0a7/eYhsCQJ6LVHjbsXwB0bkIs5NHeXJP24Dca0Fm4Ftlg XTrm0S07uu60uz1XmbfHAv1oK4AwFNx+OzLkU28fZZUwE7XJ1YR29vHw+cLVyX37 UdgpIauXD09anB7L9hHokKBXhKih0AXL7Wi7qR1BwLQxucmbpknNfGnFY2qs3Qih TRWLV/JU4e0r5lABf2BQrSRMoq3eD6IsRy4DOtuflnwp9hxgi2Dn+KFrbCQRAHjH 4sIGQCtGYLQ3Do8L+5sV =Rke6 -----END PGP SIGNATURE----- --boAH8PqvUi1v1f55--