From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58202) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YkpJp-000251-QD for qemu-devel@nongnu.org; Wed, 22 Apr 2015 03:43:39 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YkpJn-000770-G0 for qemu-devel@nongnu.org; Wed, 22 Apr 2015 03:43:37 -0400 Date: Wed, 22 Apr 2015 16:14:20 +1000 From: David Gibson Message-ID: <20150422061420.GN31815@voom.redhat.com> References: <1428679484-15451-1-git-send-email-aik@ozlabs.ru> <1428679484-15451-9-git-send-email-aik@ozlabs.ru> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="XLsjFikA86nwwlhe" Content-Disposition: inline In-Reply-To: <1428679484-15451-9-git-send-email-aik@ozlabs.ru> Subject: Re: [Qemu-devel] [PATCH qemu v6 08/15] spapr_iommu: Introduce "enabled" state for TCE table List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexey Kardashevskiy Cc: Alexander Graf , Michael Roth , qemu-devel@nongnu.org, Alex Williamson , qemu-ppc@nongnu.org, Gavin Shan --XLsjFikA86nwwlhe Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sat, Apr 11, 2015 at 01:24:37AM +1000, Alexey Kardashevskiy wrote: > Currently TCE tables are created once at start and their size never > changes. We are going to change that by introducing a Dynamic DMA windows > support where DMA configuration may change during the guest execution. >=20 > This changes spapr_tce_new_table() to create an empty stub object. Only > LIOBN is assigned by the time of creation. It still will be called once > at the owner object (VIO or PHB) creation. >=20 > This introduces an "enabled" state for TCE table objects with two > helper functions - spapr_tce_table_enable()/spapr_tce_table_disable(). > spapr_tce_table_enable() receives TCE table parameters and allocates > a guest view of the TCE table (in the user space or KVM). > spapr_tce_table_disable() disposes the table. >=20 > Follow up patches will disable+enable tables on reset (system reset > or DDW reset). >=20 > No visible change in behaviour is expected except the actual table > will be reallocated every reset. We might optimize this later. >=20 > The other way to implement this would be dynamically create/remove > the TCE table QOM objects but this would make migration impossible > as migration expects all QOM objects to exist at the receiver > so we have to have TCE table objects created when migration begins. >=20 > Signed-off-by: Alexey Kardashevskiy > --- > Changes: > v6: > * got rid of set_props() > --- > hw/ppc/spapr_iommu.c | 104 +++++++++++++++++++++++++++++++-----------= ------ > hw/ppc/spapr_pci.c | 16 +++++--- > hw/ppc/spapr_pci_vfio.c | 10 ++--- > hw/ppc/spapr_vio.c | 9 ++--- > include/hw/ppc/spapr.h | 11 ++--- > 5 files changed, 93 insertions(+), 57 deletions(-) >=20 > diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c > index a14cdc4..64f20f2 100644 > --- a/hw/ppc/spapr_iommu.c > +++ b/hw/ppc/spapr_iommu.c > @@ -126,8 +126,47 @@ static MemoryRegionIOMMUOps spapr_iommu_ops =3D { > static int spapr_tce_table_realize(DeviceState *dev) > { > sPAPRTCETable *tcet =3D SPAPR_TCE_TABLE(dev); > + > + QLIST_INSERT_HEAD(&spapr_tce_tables, tcet, list); > + > + vmstate_register(DEVICE(tcet), tcet->liobn, &vmstate_spapr_tce_table, > + tcet); > + > + return 0; > +} > + > +sPAPRTCETable *spapr_tce_new_table(DeviceState *owner, uint32_t liobn) > +{ > + sPAPRTCETable *tcet; > + char tmp[64]; > + > + if (spapr_tce_find_by_liobn(liobn)) { > + fprintf(stderr, "Attempted to create TCE table with duplicate" > + " LIOBN 0x%x\n", liobn); > + return NULL; > + } > + > + tcet =3D SPAPR_TCE_TABLE(object_new(TYPE_SPAPR_TCE_TABLE)); > + tcet->liobn =3D liobn; > + > + snprintf(tmp, sizeof(tmp), "tce-table-%x", liobn); > + object_property_add_child(OBJECT(owner), tmp, OBJECT(tcet), NULL); > + > + object_property_set_bool(OBJECT(tcet), true, "realized", NULL); > + > + trace_spapr_iommu_new_table(tcet->liobn, tcet, tcet->table, tcet->fd= ); > + > + return tcet; > +} > + > +static void spapr_tce_table_do_enable(sPAPRTCETable *tcet) AFAICT there's only one caller of this, so it's not clear why this isn't just open-coded in spapr_tce_table_enable(). > +{ > uint64_t window_size =3D (uint64_t)tcet->nb_table << tcet->page_shif= t; > =20 > + if (!tcet->nb_table) { > + return; > + } > + > if (kvm_enabled() && !(window_size >> 32)) { > tcet->table =3D kvmppc_create_spapr_tce(tcet->liobn, > window_size, > @@ -140,65 +179,56 @@ static int spapr_tce_table_realize(DeviceState *dev) > tcet->table =3D g_malloc0(table_size); > } > =20 > - trace_spapr_iommu_new_table(tcet->liobn, tcet, tcet->table, tcet->fd= ); > - > - memory_region_init_iommu(&tcet->iommu, OBJECT(dev), &spapr_iommu_ops, > + memory_region_init_iommu(&tcet->iommu, OBJECT(tcet), &spapr_iommu_op= s, > "iommu-spapr", > (uint64_t)tcet->nb_table << tcet->page_shif= t); > =20 > - QLIST_INSERT_HEAD(&spapr_tce_tables, tcet, list); > - > - vmstate_register(DEVICE(tcet), tcet->liobn, &vmstate_spapr_tce_table, > - tcet); > - > - return 0; > + tcet->enabled =3D true; > } > =20 > -sPAPRTCETable *spapr_tce_new_table(DeviceState *owner, uint32_t liobn, > - uint64_t bus_offset, > - uint32_t page_shift, > - uint32_t nb_table, > - bool vfio_accel) > +void spapr_tce_table_enable(sPAPRTCETable *tcet, > + uint64_t bus_offset, uint32_t page_shift, > + uint32_t nb_table, bool vfio_accel) > { > - sPAPRTCETable *tcet; > - char tmp[64]; > - > - if (spapr_tce_find_by_liobn(liobn)) { > - fprintf(stderr, "Attempted to create TCE table with duplicate" > - " LIOBN 0x%x\n", liobn); > - return NULL; > - } > - > - if (!nb_table) { > - return NULL; > + if (tcet->enabled) { > + return; > } > =20 > - tcet =3D SPAPR_TCE_TABLE(object_new(TYPE_SPAPR_TCE_TABLE)); > - tcet->liobn =3D liobn; > tcet->bus_offset =3D bus_offset; > tcet->page_shift =3D page_shift; > tcet->nb_table =3D nb_table; > tcet->vfio_accel =3D vfio_accel; > =20 > - snprintf(tmp, sizeof(tmp), "tce-table-%x", liobn); > - object_property_add_child(OBJECT(owner), tmp, OBJECT(tcet), NULL); > - > - object_property_set_bool(OBJECT(tcet), true, "realized", NULL); > - > - return tcet; > + spapr_tce_table_do_enable(tcet); > } > =20 > -static void spapr_tce_table_unrealize(DeviceState *dev, Error **errp) > +void spapr_tce_table_disable(sPAPRTCETable *tcet) > { > - sPAPRTCETable *tcet =3D SPAPR_TCE_TABLE(dev); > - > - QLIST_REMOVE(tcet, list); > + if (!tcet->enabled) { > + return; > + } > =20 > if (!kvm_enabled() || > (kvmppc_remove_spapr_tce(tcet->table, tcet->fd, > tcet->nb_table) !=3D 0)) { > + tcet->fd =3D -1; > g_free(tcet->table); > } > + tcet->table =3D NULL; > + tcet->enabled =3D false; > + tcet->bus_offset =3D 0; > + tcet->page_shift =3D 0; > + tcet->nb_table =3D 0; > + tcet->vfio_accel =3D false; > +} > + > +static void spapr_tce_table_unrealize(DeviceState *dev, Error **errp) > +{ > + sPAPRTCETable *tcet =3D SPAPR_TCE_TABLE(dev); > + > + QLIST_REMOVE(tcet, list); > + > + spapr_tce_table_disable(tcet); > } > =20 > MemoryRegion *spapr_tce_get_iommu(sPAPRTCETable *tcet) > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c > index 8c0d2eb..c3410b8 100644 > --- a/hw/ppc/spapr_pci.c > +++ b/hw/ppc/spapr_pci.c > @@ -881,6 +881,12 @@ static void spapr_phb_realize(DeviceState *dev, Erro= r **errp) > sphb->lsi_table[i].irq =3D irq; > } > =20 > + tcet =3D spapr_tce_new_table(DEVICE(sphb), sphb->dma_liobn); > + if (!tcet) { > + error_setg(errp, "failed to create TCE table"); > + return; > + } > + > info->dma_capabilities_update(sphb); > info->dma_init_window(sphb, sphb->dma_liobn, SPAPR_TCE_PAGE_SHIFT, > sphb->dma32_window_size); > @@ -908,13 +914,13 @@ static int spapr_phb_dma_init_window(sPAPRPHBState = *sphb, > uint64_t window_size) > { > uint64_t bus_offset =3D sphb->dma32_window_start; > - sPAPRTCETable *tcet; > + sPAPRTCETable *tcet =3D spapr_tce_find_by_liobn(liobn); > =20 > - tcet =3D spapr_tce_new_table(DEVICE(sphb), liobn, bus_offset, page_s= hift, > - window_size >> page_shift, > - false); > + spapr_tce_table_enable(tcet, bus_offset, page_shift, > + window_size >> page_shift, > + false); > =20 > - return tcet ? 0 : -1; > + return 0; > } > =20 > static int spapr_phb_children_reset(Object *child, void *opaque) > diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c > index 0ce8e61..a428166 100644 > --- a/hw/ppc/spapr_pci_vfio.c > +++ b/hw/ppc/spapr_pci_vfio.c > @@ -49,13 +49,13 @@ static int spapr_phb_vfio_dma_init_window(sPAPRPHBSta= te *sphb, > uint64_t window_size) > { > uint64_t bus_offset =3D sphb->dma32_window_start; > - sPAPRTCETable *tcet; > + sPAPRTCETable *tcet =3D spapr_tce_find_by_liobn(liobn); > =20 > - tcet =3D spapr_tce_new_table(DEVICE(sphb), liobn, bus_offset, page_s= hift, > - window_size >> page_shift, > - true); > + spapr_tce_table_enable(tcet, bus_offset, page_shift, > + window_size >> page_shift, > + true); > =20 > - return tcet ? 0 : -1; > + return 0; > } > =20 > static void spapr_phb_vfio_reset(DeviceState *qdev) > diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c > index 174033d..3e28835 100644 > --- a/hw/ppc/spapr_vio.c > +++ b/hw/ppc/spapr_vio.c > @@ -479,11 +479,10 @@ static void spapr_vio_busdev_realize(DeviceState *q= dev, Error **errp) > memory_region_add_subregion_overlap(&dev->mrroot, 0, &dev->mrbyp= ass, 1); > address_space_init(&dev->as, &dev->mrroot, qdev->id); > =20 > - dev->tcet =3D spapr_tce_new_table(qdev, liobn, > - 0, > - SPAPR_TCE_PAGE_SHIFT, > - pc->rtce_window_size >> > - SPAPR_TCE_PAGE_SHIFT, false); > + dev->tcet =3D spapr_tce_new_table(qdev, liobn); > + spapr_tce_table_enable(dev->tcet, 0, SPAPR_TCE_PAGE_SHIFT, > + pc->rtce_window_size >> SPAPR_TCE_PAGE_SH= IFT, > + false); > dev->tcet->vdev =3D dev; > memory_region_add_subregion_overlap(&dev->mrroot, 0, > spapr_tce_get_iommu(dev->tce= t), 2); > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h > index 7d9ab9d..074d837 100644 > --- a/include/hw/ppc/spapr.h > +++ b/include/hw/ppc/spapr.h > @@ -498,6 +498,7 @@ typedef struct sPAPRTCETable sPAPRTCETable; > =20 > struct sPAPRTCETable { > DeviceState parent; > + bool enabled; > uint32_t liobn; > uint32_t nb_table; > uint64_t bus_offset; > @@ -515,11 +516,11 @@ sPAPRTCETable *spapr_tce_find_by_liobn(uint32_t lio= bn); > void spapr_events_init(sPAPREnvironment *spapr); > void spapr_events_fdt_skel(void *fdt, uint32_t epow_irq); > int spapr_h_cas_compose_response(target_ulong addr, target_ulong size); > -sPAPRTCETable *spapr_tce_new_table(DeviceState *owner, uint32_t liobn, > - uint64_t bus_offset, > - uint32_t page_shift, > - uint32_t nb_table, > - bool vfio_accel); > +sPAPRTCETable *spapr_tce_new_table(DeviceState *owner, uint32_t liobn); > +void spapr_tce_table_enable(sPAPRTCETable *tcet, > + uint64_t bus_offset, uint32_t page_shift, > + uint32_t nb_table, bool vfio_accel); > +void spapr_tce_table_disable(sPAPRTCETable *tcet); > MemoryRegion *spapr_tce_get_iommu(sPAPRTCETable *tcet); > int spapr_dma_dt(void *fdt, int node_off, const char *propname, > uint32_t liobn, uint64_t window, uint32_t size); --=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 --XLsjFikA86nwwlhe Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJVNzw8AAoJEGw4ysog2bOSh5kP/j0pV7qoW+ZcEwDQhh9KRMMN 28NjrRHWLq9H7K/4LiZcXuMtGD2MuMD9Ovce7CzVvSdssHFOb9qKgLvru9/g6/0c 3J67nVJGxlKvF4sWH1yjKSSTZ5xPiCHz9vNmy7a88She10cXDLgLa46OvKfvo2Sb 5TQVxNayMcLFeLhPU1zt8SBlj/gltBxkyuXmJgPWzh2VlZDRu50Z9bKMC9kaeKzR pdG+q9noLljHjYhq52U0QixR28WiFYISvyQi4FPGZdvEsC+TmCMhiarf6CgLAK0H fN0NyPklAyh4FtFFCuPOaCyBLERJrom4o/wDba5o5oMzw+vNu0gP/zRvvlTaFnUt kxlbrpjgLkQuIyFAM3I42ExmEwkP0cRYz/hLf6shulwNkuRk+n4Do9icNRBBxgAm 3BlbMKcRXJqILwCEy09U7qv5C1Qn+sl9IGiaXw5V/cokhl5RnyTDIRAT+V99miwh zcm9W1NBEDyv7nF77HfQpcOBLNRSXjB7+hXPcLdvlry7hskWBBSbZ52Gdnu/V3eL oGg9PRQLyNs/lzq2Q51S02ULYV3KvwMojt0ZROGNqiCh8BMEaTFBlbyOVoawhQKn zPazUGUaCKZ63kpW3freE9pbUHfbeXGXD0fXzC4ZGDnDJZpTOytzxCQE2UY1HU9i xY/syZP8D8GAzON+CAE+ =d5OC -----END PGP SIGNATURE----- --XLsjFikA86nwwlhe--