From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44477) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bv3nf-0003pg-4X for qemu-devel@nongnu.org; Fri, 14 Oct 2016 10:49:35 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bv3nZ-0002AL-2u for qemu-devel@nongnu.org; Fri, 14 Oct 2016 10:49:30 -0400 Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:54716) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bv3nY-00029m-PY for qemu-devel@nongnu.org; Fri, 14 Oct 2016 10:49:25 -0400 Received: from pps.filterd (m0098394.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.17/8.16.0.17) with SMTP id u9EEmSVd100818 for ; Fri, 14 Oct 2016 10:49:23 -0400 Received: from e23smtp03.au.ibm.com (e23smtp03.au.ibm.com [202.81.31.145]) by mx0a-001b2d01.pphosted.com with ESMTP id 262ywjmw7k-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Fri, 14 Oct 2016 10:49:23 -0400 Received: from localhost by e23smtp03.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Sat, 15 Oct 2016 00:49:21 +1000 Date: Fri, 14 Oct 2016 20:19:12 +0530 From: Bharata B Rao Reply-To: bharata@linux.vnet.ibm.com References: <1476375902-11715-1-git-send-email-lvivier@redhat.com> <1476375902-11715-4-git-send-email-lvivier@redhat.com> <20161014040751.GD28562@umbus> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161014040751.GD28562@umbus> Message-Id: <20161014144912.GE28693@in.ibm.com> Subject: Re: [Qemu-devel] [PATCH v2 03/20] target-ppc: move back cpu_exec_init() to init List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Gibson Cc: Laurent Vivier , qemu-devel@nongnu.org, Paolo Bonzini , Peter Maydell , Eduardo Habkost , Markus Armbruster , Matthew Rosato , Alexander Graf , qemu-ppc@nongnu.org On Fri, Oct 14, 2016 at 03:07:51PM +1100, David Gibson wrote: > On Thu, Oct 13, 2016 at 06:24:45PM +0200, Laurent Vivier wrote: > > We have now the cpu_exec_realize() in realize, > > so the init part must be in init. > > > > As cpu_exec_unrealize() is called from cpu_common_finalize(), > > remove the call from ppc_cpu_unrealizefn(). > > > > CC: Bharata B Rao > > CC: Alexander Graf > > CC: qemu-ppc@nongnu.org > > Signed-off-by: Laurent Vivier > > --- > > target-ppc/translate_init.c | 4 +--- > > 1 file changed, 1 insertion(+), 3 deletions(-) > > > > diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c > > index 094f28a..bbca8b5 100644 > > --- a/target-ppc/translate_init.c > > +++ b/target-ppc/translate_init.c > > @@ -9678,7 +9678,6 @@ static void ppc_cpu_realizefn(DeviceState *dev, Error **errp) > > } > > #endif > > > > - cpu_exec_init(cs); > > cpu_exec_realize(cs, &local_err); > > if (local_err != NULL) { > > error_propagate(errp, local_err); > > @@ -9911,8 +9910,6 @@ static void ppc_cpu_unrealizefn(DeviceState *dev, Error **errp) > > opc_handler_t **table, **table_2; > > int i, j, k; > > > > - cpu_exec_unrealize(CPU(dev)); > > - > > This doesn't seem right. As you said in 0/20, cpu_exec_unrealize() is > called from cpu_common_finalize(). But finalize should mirror init, > not unrealize(). So it seems that unrealize() really should belong > here, not in finalize. For archs like PowerPC, cpu_exec_exit() was being called twice: once from PowerPC CPU's unrealize function and once from cpu_common_finalize(). cpu_exec_exit() had two vmstate_unregister() calls and it used to ensure that they are not called twice, but looks like this got changed sometime back and we are now executing these two vmstate_unregister() calls twice. While you are here, could you please take care of this ? Regards, Bharata.