From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53788) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ykqfp-0002LF-Lx for qemu-devel@nongnu.org; Wed, 22 Apr 2015 05:10:27 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Ykqfm-0001lp-6X for qemu-devel@nongnu.org; Wed, 22 Apr 2015 05:10:25 -0400 Received: from mail-pd0-f182.google.com ([209.85.192.182]:36473) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ykqfl-0001lg-UW for qemu-devel@nongnu.org; Wed, 22 Apr 2015 05:10:22 -0400 Received: by pdea3 with SMTP id a3so268741033pde.3 for ; Wed, 22 Apr 2015 02:10:21 -0700 (PDT) Message-ID: <55376576.2060408@ozlabs.ru> Date: Wed, 22 Apr 2015 19:10:14 +1000 From: Alexey Kardashevskiy MIME-Version: 1.0 References: <1428679484-15451-1-git-send-email-aik@ozlabs.ru> <1428679484-15451-9-git-send-email-aik@ozlabs.ru> <20150422061420.GN31815@voom.redhat.com> In-Reply-To: <20150422061420.GN31815@voom.redhat.com> Content-Type: text/plain; charset=koi8-r; format=flowed Content-Transfer-Encoding: 7bit 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: David Gibson Cc: Alexander Graf , Michael Roth , qemu-devel@nongnu.org, Alex Williamson , qemu-ppc@nongnu.org, Gavin Shan On 04/22/2015 04:14 PM, David Gibson wrote: > 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. >> >> 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. >> >> 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. >> >> Follow up patches will disable+enable tables on reset (system reset >> or DDW reset). >> >> No visible change in behaviour is expected except the actual table >> will be reallocated every reset. We might optimize this later. >> >> 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. >> >> 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(-) >> >> 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 = { >> static int spapr_tce_table_realize(DeviceState *dev) >> { >> sPAPRTCETable *tcet = 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 = SPAPR_TCE_TABLE(object_new(TYPE_SPAPR_TCE_TABLE)); >> + tcet->liobn = 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(). There is another call in "[PATCH qemu v6 14/15] spapr_pci/spapr_pci_vfio: Support Dynamic DMA Windows (DDW)", in spapr_tce_table_post_load(). Should I put a note in the commit log? -- Alexey