From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:48885) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UXOoz-00063o-42 for qemu-devel@nongnu.org; Wed, 01 May 2013 00:39:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UXOos-0005Dt-5F for qemu-devel@nongnu.org; Wed, 01 May 2013 00:39:13 -0400 Received: from ozlabs.org ([203.10.76.45]:44046) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UXOor-00058k-IZ for qemu-devel@nongnu.org; Wed, 01 May 2013 00:39:06 -0400 Date: Wed, 1 May 2013 14:38:57 +1000 From: David Gibson Message-ID: <20130501043857.GP20202@truffula.fritz.box> References: <1367378320-9246-1-git-send-email-david@gibson.dropbear.id.au> <1367378320-9246-11-git-send-email-david@gibson.dropbear.id.au> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="SUcp5/vPb8ZOOFiR" Content-Disposition: inline In-Reply-To: <1367378320-9246-11-git-send-email-david@gibson.dropbear.id.au> Subject: Re: [Qemu-devel] [PATCH 11/17] spapr: convert TCE API to use an opaque type List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: pbonzini@redhat.com Cc: aik@truffula.fritz.box, qemu-devel@nongnu.org --SUcp5/vPb8ZOOFiR Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable > From: Paolo Bonzini >=20 > The TCE table is currently returned as a DMAContext, and non-type-safe > APIs are called later passing back the DMAContext. Since we want to move > away from DMAContext, use an opaque type instead, and add an accessor > to retrieve the DMAContext from it. >=20 > Signed-off-by: Paolo Bonzini Acked-by: David Gibson Using DMAContext * as the handle throughout these functions seemed like a good idea at the time, but it was a mistake in hindsight. I might look at merging this in through our tree, since it doesn't really depend on the rest of the iommu stuff. > --- > hw/ppc/spapr_iommu.c | 54 ++++++++++++++++++-------------------= ------ > hw/ppc/spapr_pci.c | 8 +++---- > hw/ppc/spapr_vio.c | 13 ++++++----- > include/hw/pci-host/spapr.h | 2 +- > include/hw/ppc/spapr.h | 12 ++++++---- > include/hw/ppc/spapr_vio.h | 1 + > 6 files changed, 42 insertions(+), 48 deletions(-) >=20 > diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c > index d2782cf..ab34a88 100644 > --- a/hw/ppc/spapr_iommu.c > +++ b/hw/ppc/spapr_iommu.c > @@ -36,8 +36,6 @@ enum sPAPRTCEAccess { > SPAPR_TCE_RW =3D 3, > }; > =20 > -typedef struct sPAPRTCETable sPAPRTCETable; > - > struct sPAPRTCETable { > DMAContext dma; > uint32_t liobn; > @@ -116,7 +114,7 @@ static int spapr_tce_translate(DMAContext *dma, > return 0; > } > =20 > -DMAContext *spapr_tce_new_dma_context(uint32_t liobn, size_t window_size) > +sPAPRTCETable *spapr_tce_new_table(uint32_t liobn, size_t window_size) > { > sPAPRTCETable *tcet; > =20 > @@ -149,43 +147,40 @@ DMAContext *spapr_tce_new_dma_context(uint32_t liob= n, size_t window_size) > } > =20 > #ifdef DEBUG_TCE > - fprintf(stderr, "spapr_iommu: New TCE table, liobn=3D0x%x, context @= %p, " > - "table @ %p, fd=3D%d\n", liobn, &tcet->dma, tcet->table, tce= t->fd); > + fprintf(stderr, "spapr_iommu: New TCE table @ %p, liobn=3D0x%x, " > + "table @ %p, fd=3D%d\n", tcet, liobn, tcet->table, tcet->fd); > #endif > =20 > QLIST_INSERT_HEAD(&spapr_tce_tables, tcet, list); > =20 > - return &tcet->dma; > + return tcet; > } > =20 > -void spapr_tce_free(DMAContext *dma) > +void spapr_tce_free(sPAPRTCETable *tcet) > { > + QLIST_REMOVE(tcet, list); > =20 > - if (dma) { Strictly speaking, removing this test is an unrelated change, but it is correct anyway. > - sPAPRTCETable *tcet =3D DO_UPCAST(sPAPRTCETable, dma, dma); > - > - QLIST_REMOVE(tcet, list); > - > - if (!kvm_enabled() || > - (kvmppc_remove_spapr_tce(tcet->table, tcet->fd, > - tcet->window_size) !=3D 0)) { > - g_free(tcet->table); > - } > - > - g_free(tcet); > + if (!kvm_enabled() || > + (kvmppc_remove_spapr_tce(tcet->table, tcet->fd, > + tcet->window_size) !=3D 0)) { > + g_free(tcet->table); > } > + > + g_free(tcet); > } > =20 > -void spapr_tce_set_bypass(DMAContext *dma, bool bypass) > +DMAContext *spapr_tce_get_dma(sPAPRTCETable *tcet) > { > - sPAPRTCETable *tcet =3D DO_UPCAST(sPAPRTCETable, dma, dma); > + return &tcet->dma; > +} > =20 > +void spapr_tce_set_bypass(sPAPRTCETable *tcet, bool bypass) > +{ > tcet->bypass =3D bypass; > } > =20 > -void spapr_tce_reset(DMAContext *dma) > +void spapr_tce_reset(sPAPRTCETable *tcet) > { > - sPAPRTCETable *tcet =3D DO_UPCAST(sPAPRTCETable, dma, dma); > size_t table_size =3D (tcet->window_size >> SPAPR_TCE_PAGE_SHIFT) > * sizeof(sPAPRTCE); > =20 > @@ -277,17 +272,12 @@ int spapr_dma_dt(void *fdt, int node_off, const cha= r *propname, > } > =20 > int spapr_tcet_dma_dt(void *fdt, int node_off, const char *propname, > - DMAContext *iommu) > + sPAPRTCETable *tcet) > { > - if (!iommu) { > + if (!tcet) { > return 0; > } > =20 > - if (iommu->translate =3D=3D spapr_tce_translate) { > - sPAPRTCETable *tcet =3D DO_UPCAST(sPAPRTCETable, dma, iommu); > - return spapr_dma_dt(fdt, node_off, propname, > - tcet->liobn, 0, tcet->window_size); > - } > - > - return -1; > + return spapr_dma_dt(fdt, node_off, propname, > + tcet->liobn, 0, tcet->window_size); > } > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c > index 62ff323..eb64a8f 100644 > --- a/hw/ppc/spapr_pci.c > +++ b/hw/ppc/spapr_pci.c > @@ -511,7 +511,7 @@ static DMAContext *spapr_pci_dma_context_fn(PCIBus *b= us, void *opaque, > { > sPAPRPHBState *phb =3D opaque; > =20 > - return phb->dma; > + return spapr_tce_get_dma(phb->tcet); > } > =20 > static int spapr_phb_init(SysBusDevice *s) > @@ -646,8 +646,8 @@ static int spapr_phb_init(SysBusDevice *s) > =20 > sphb->dma_window_start =3D 0; > sphb->dma_window_size =3D 0x40000000; > - sphb->dma =3D spapr_tce_new_dma_context(sphb->dma_liobn, sphb->dma_w= indow_size); > - if (!sphb->dma) { > + sphb->tcet =3D spapr_tce_new_table(sphb->dma_liobn, sphb->dma_window= _size); > + if (!sphb->tcet) { > fprintf(stderr, "Unable to create TCE table for %s\n", sphb->dtb= usname); > return -1; > } > @@ -676,7 +676,7 @@ static void spapr_phb_reset(DeviceState *qdev) > sPAPRPHBState *sphb =3D SPAPR_PCI_HOST_BRIDGE(s); > =20 > /* Reset the IOMMU state */ > - spapr_tce_reset(sphb->dma); > + spapr_tce_reset(sphb->tcet); > } > =20 > static Property spapr_phb_properties[] =3D { > diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c > index 4dbc315..7544def 100644 > --- a/hw/ppc/spapr_vio.c > +++ b/hw/ppc/spapr_vio.c > @@ -136,7 +136,7 @@ static int vio_make_devnode(VIOsPAPRDevice *dev, > } > } > =20 > - ret =3D spapr_tcet_dma_dt(fdt, node_off, "ibm,my-dma-window", dev->d= ma); > + ret =3D spapr_tcet_dma_dt(fdt, node_off, "ibm,my-dma-window", dev->t= cet); > if (ret < 0) { > return ret; > } > @@ -310,8 +310,8 @@ int spapr_vio_send_crq(VIOsPAPRDevice *dev, uint8_t *= crq) > =20 > static void spapr_vio_quiesce_one(VIOsPAPRDevice *dev) > { > - if (dev->dma) { > - spapr_tce_reset(dev->dma); > + if (dev->tcet) { > + spapr_tce_reset(dev->tcet); > } > free_crq(dev); > } > @@ -336,12 +336,12 @@ static void rtas_set_tce_bypass(sPAPREnvironment *s= papr, uint32_t token, > return; > } > =20 > - if (!dev->dma) { > + if (!dev->tcet) { > rtas_st(rets, 0, -3); > return; > } > =20 > - spapr_tce_set_bypass(dev->dma, !!enable); > + spapr_tce_set_bypass(dev->tcet, !!enable); > =20 > rtas_st(rets, 0, 0); > } > @@ -448,7 +448,8 @@ static int spapr_vio_busdev_init(DeviceState *qdev) > =20 > if (pc->rtce_window_size) { > uint32_t liobn =3D SPAPR_VIO_BASE_LIOBN | dev->reg; > - dev->dma =3D spapr_tce_new_dma_context(liobn, pc->rtce_window_si= ze); > + dev->tcet =3D spapr_tce_new_table(liobn, pc->rtce_window_size); > + dev->dma =3D spapr_tce_get_dma(dev->tcet); > } > =20 > return pc->init(dev); > diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h > index b21080c..653dd40 100644 > --- a/include/hw/pci-host/spapr.h > +++ b/include/hw/pci-host/spapr.h > @@ -49,7 +49,7 @@ typedef struct sPAPRPHBState { > uint32_t dma_liobn; > uint64_t dma_window_start; > uint64_t dma_window_size; > - DMAContext *dma; > + sPAPRTCETable *tcet; > =20 > struct { > uint32_t irq; > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h > index 864bee9..e8d617b 100644 > --- a/include/hw/ppc/spapr.h > +++ b/include/hw/ppc/spapr.h > @@ -342,17 +342,19 @@ typedef struct sPAPRTCE { > =20 > #define RTAS_ERROR_LOG_MAX 2048 > =20 > +typedef struct sPAPRTCETable sPAPRTCETable; > =20 > void spapr_iommu_init(void); > void spapr_events_init(sPAPREnvironment *spapr); > void spapr_events_fdt_skel(void *fdt, uint32_t epow_irq); > -DMAContext *spapr_tce_new_dma_context(uint32_t liobn, size_t window_size= ); > -void spapr_tce_free(DMAContext *dma); > -void spapr_tce_reset(DMAContext *dma); > -void spapr_tce_set_bypass(DMAContext *dma, bool bypass); > +sPAPRTCETable *spapr_tce_new_table(uint32_t liobn, size_t window_size); > +DMAContext *spapr_tce_get_dma(sPAPRTCETable *tcet); > +void spapr_tce_free(sPAPRTCETable *tcet); > +void spapr_tce_reset(sPAPRTCETable *tcet); > +void spapr_tce_set_bypass(sPAPRTCETable *tcet, bool bypass); > int spapr_dma_dt(void *fdt, int node_off, const char *propname, > uint32_t liobn, uint64_t window, uint32_t size); > int spapr_tcet_dma_dt(void *fdt, int node_off, const char *propname, > - DMAContext *dma); > + sPAPRTCETable *tcet); > =20 > #endif /* !defined (__HW_SPAPR_H__) */ > diff --git a/include/hw/ppc/spapr_vio.h b/include/hw/ppc/spapr_vio.h > index f98ec0a..56f2821 100644 > --- a/include/hw/ppc/spapr_vio.h > +++ b/include/hw/ppc/spapr_vio.h > @@ -63,6 +63,7 @@ struct VIOsPAPRDevice { > uint32_t irq; > target_ulong signal_state; > VIOsPAPR_CRQ crq; > + sPAPRTCETable *tcet; > DMAContext *dma; It would be nice to remove this DMAContext field as well. > }; > =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 --SUcp5/vPb8ZOOFiR Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (GNU/Linux) iEYEARECAAYFAlGAnGEACgkQaILKxv3ab8ZGAQCfafbiluKw52GRSRLfH4TKPn/K L64AmwXyZYaatEfREJgzh9n0U7ELi+J3 =3VuF -----END PGP SIGNATURE----- --SUcp5/vPb8ZOOFiR--