From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60132) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZeuvH-0004Ga-84 for qemu-devel@nongnu.org; Wed, 23 Sep 2015 21:02:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZeuvD-00058a-8B for qemu-devel@nongnu.org; Wed, 23 Sep 2015 21:02:07 -0400 Date: Thu, 24 Sep 2015 09:56:49 +1000 From: David Gibson Message-ID: <20150923235649.GH15944@voom.fritz.box> References: <1442495357-26547-1-git-send-email-david@gibson.dropbear.id.au> <1442495357-26547-8-git-send-email-david@gibson.dropbear.id.au> <56028832.5030708@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="FUaywKC54iCcLzqT" Content-Disposition: inline In-Reply-To: <56028832.5030708@redhat.com> Subject: Re: [Qemu-devel] [RFC PATCH 07/10] spapr_pci: Allow PCI host bridge DMA window to be configured List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Thomas Huth Cc: lvivier@redhat.com, aik@ozlabs.ru, gwshan@linux.vnet.ibm.com, qemu-devel@nongnu.org, alex.williamson@redhat.com, qemu-ppc@nongnu.org, pbonzini@redhat.com --FUaywKC54iCcLzqT Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Sep 23, 2015 at 01:08:34PM +0200, Thomas Huth wrote: > On 17/09/15 15:09, David Gibson wrote: > > At present the PCI host bridge (PHB) for the pseries machine type has a > > fixed DMA window from 0..1GB (in PCI address space) which is mapped to = real > > memory via the PAPR paravirtualized IOMMU. > >=20 > > For better support of VFIO devices, we're going to want to allow for > > different configurations of the DMA window. > >=20 > > Eventually we'll want to allow the guest itself to reconfigure the wind= ow > > via the PAPR dynamic DMA window interface, but as a preliminary this pa= tch > > allows the user to reconfigure the window with new properties on the PHB > > device. > >=20 > > Signed-off-by: David Gibson > > --- > > hw/ppc/spapr_pci.c | 7 +++++-- > > include/hw/pci-host/spapr.h | 3 +-- > > 2 files changed, 6 insertions(+), 4 deletions(-) > >=20 > > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c > > index b088491..622c4ac 100644 > > --- a/hw/ppc/spapr_pci.c > > +++ b/hw/ppc/spapr_pci.c > > @@ -1394,7 +1394,7 @@ static void spapr_phb_finish_realize(sPAPRPHBStat= e *sphb, Error **errp) > > sPAPRTCETable *tcet; > > uint32_t nb_table; > > =20 > > - nb_table =3D SPAPR_PCI_DMA32_SIZE >> SPAPR_TCE_PAGE_SHIFT; > > + nb_table =3D sphb->dma_win_size >> SPAPR_TCE_PAGE_SHIFT; > > tcet =3D spapr_tce_new_table(DEVICE(sphb), sphb->dma_liobn, > > 0, SPAPR_TCE_PAGE_SHIFT, nb_table, fals= e); > > if (!tcet) { > > @@ -1404,7 +1404,7 @@ static void spapr_phb_finish_realize(sPAPRPHBStat= e *sphb, Error **errp) > > } > > =20 > > /* Register default 32bit DMA window */ > > - memory_region_add_subregion(&sphb->iommu_root, 0, > > + memory_region_add_subregion(&sphb->iommu_root, sphb->dma_win_addr, > > spapr_tce_get_iommu(tcet)); > > } > > =20 > > @@ -1437,6 +1437,9 @@ static Property spapr_phb_properties[] =3D { > > SPAPR_PCI_IO_WIN_SIZE), > > DEFINE_PROP_BOOL("dynamic-reconfiguration", sPAPRPHBState, dr_enab= led, > > true), > > + /* Default DMA window is 0..1GB */ > > + DEFINE_PROP_UINT64("dma_win_addr", sPAPRPHBState, dma_win_addr, 0), > > + DEFINE_PROP_UINT64("dma_win_size", sPAPRPHBState, dma_win_size, 0x= 40000000), > > DEFINE_PROP_END_OF_LIST(), > > }; > > =20 > > diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h > > index 5322b56..7de5e02 100644 > > --- a/include/hw/pci-host/spapr.h > > +++ b/include/hw/pci-host/spapr.h > > @@ -78,6 +78,7 @@ struct sPAPRPHBState { > > MemoryRegion memwindow, iowindow, msiwindow; > > =20 > > uint32_t dma_liobn; > > + hwaddr dma_win_addr, dma_win_size; >=20 > Maybe use dma_addr_t for dma_win_addr? And dma_win_size isn't an > address, so I'd maybe use uint64_t here instead. So dma_win_addr is being passed to memory_region_add_subregion() which takes a hwaddr, so I think hwaddr is correct. > > AddressSpace iommu_as; > > MemoryRegion iommu_root; > > =20 > > @@ -115,8 +116,6 @@ struct sPAPRPHBVFIOState { > > =20 > > #define SPAPR_PCI_MSI_WINDOW 0x40000000000ULL > > =20 > > -#define SPAPR_PCI_DMA32_SIZE 0x40000000 > > - > > static inline qemu_irq spapr_phb_lsi_qirq(struct sPAPRPHBState *phb, i= nt pin) > > { > > sPAPRMachineState *spapr =3D SPAPR_MACHINE(qdev_get_machine()); >=20 > Anyway, patch looks fine to me, so: >=20 > Reviewed-by: Thomas Huth >=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 --FUaywKC54iCcLzqT Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJWAzxBAAoJEGw4ysog2bOSKJAQAIvbai/ootTD7eloapwVZ595 A9agy0BuHu0y6l2QXdlhfD3MMTLEu62t4K6wkD1CB74r4FD6XoOGjnWhvoc25bYx Hr8wHLThx3GILq7DMlamdsGITCqB84lZaJqKZDGlJgYzdbTJ8u+PzGKpPr9tOI6c 0E7DTQSCkbwmt/pz4pjy4gZDQwr772trQLYTm8YJnhyLJghqYT7r9Ci5MlXAYgnx VYNz19/n7hTDlK3TST9bBLl3B4ihJojh1Z4RyOlNM1eDd3VWljCb9nw9G6rQVszv 15UM8ZI68iQyXIRWL0//QndSjRO1JrxmB/NvTbbbiQAymNpy9lQ6m62LSPq7kmQy 36fnCYNu/lljmAtPB1O3TB8rTtxgggWklSNhP4N8grwSJt4P81X9/CuJHMpjdN0C aaKg03w9f4CWsY5rmYCqP/C2Xw6DauAFpA2zcYrUXDk2bn5f2s9SAYbrEWUw7OkR CcwAqSLFjLA3J05epTj+4GbHs6iU6hnVfu6VWBIXGEZ5rT8uXENPMvm1Z9dt4zCM 3Hv/Yy+N5xgik0fihgMxmQPocmnF4+Fv+vdE44BmN4fbus3wh8j6L9KOc6LAySv5 aWIZppG7ez+LqIkRtUpmoTqhzFnSJhuodEaB+zoOYL3N1ce203PwxLY7iZxcgq+o giq1Qhq8yqIo4zhSfwNN =AKdB -----END PGP SIGNATURE----- --FUaywKC54iCcLzqT--