From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43059) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UvRmt-0004vZ-Ml for qemu-devel@nongnu.org; Sat, 06 Jul 2013 08:40:31 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UvRmq-0001tT-Ta for qemu-devel@nongnu.org; Sat, 06 Jul 2013 08:40:27 -0400 Received: from cantor2.suse.de ([195.135.220.15]:54096 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UvRmq-0001t2-Jo for qemu-devel@nongnu.org; Sat, 06 Jul 2013 08:40:24 -0400 Message-ID: <51D81031.2080703@suse.de> Date: Sat, 06 Jul 2013 14:40:17 +0200 From: =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= MIME-Version: 1.0 References: <1373070978-11966-1-git-send-email-agraf@suse.de> <1373070978-11966-4-git-send-email-agraf@suse.de> In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 3/9] linux-user: Don't reset a new thread's CPU List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell , Alexander Graf Cc: Igor Mammedov , Riku Voipio , qemu-devel@nongnu.org, Eduardo Habkost Am 06.07.2013 12:31, schrieb Peter Maydell: > On 6 July 2013 01:36, Alexander Graf wrote: >> When we create a new thread, there is no reason to reset it. I'm fairl= y sure >> the code has mostly been left in there because nobody understood why i= t was >> there in the first place. >=20 > We had a big discussion on this on IRC. This code is here > because of commit b4558d748, which attempted to fix a breakage > introduced when commit b55a37c9 made these CPUs no longer do > a reset as part of their cpu_init(). And as I said on IRC, x86 has been fixed to reset as part of cpu_init(): http://repo.or.cz/w/qemu.git/commit/77868120cfe93ad7816dfac6546684e5a6c6e= 256?f=3Dlinux-user/main.c So since this patch only removes x86 reset and leaves ppc and sparc in place, I think this patch is more correct than what we have now. > However, it's in the > wrong place -- this reset needs to go into cpu_copy(), so > it doesn't undo the copying work done by that function. The remaining question Eduardo raised was whether not resetting after fiddling with fields might cause some kind of inconsistency in the CPU. But since the current code effectively undoes the effects of cpu_copy(), I'm willing to pick up this patch as follow-up to my earlier refactoring (and 6/9 if ack'ed as follow-up to Peter's refactoring) for qom-cpu. The commit message would need to be reworked though to reference why this can be done for x86. And we'd still need a follow-up solution for ppc and sparc. Peter's suggestion was that we run LTP in a chroot to verify whether our respective solutions break anything badly: http://wiki.qemu.org/Testing/LTP (BTW the May 2013 stable version is available now) >> Remove the reset. A new thread's kernel sided state should be identica= l to >> the old one's. >=20 > Just deleting the reset isn't right. We should standardize > whether we think cpu_init() ought to give you a cleanly > reset CPU or not (I think it should); +1 Specifically as part of QOM realize, which - once cpu_init() is gone - linux-user/bsd-user would trivially invoke directly after object_new(). softmmu would do it after the future QMP qom-set phase. The mess there is reset handler registration order: We cannot have most CPUs register a reset handler themselves yet because some machines (including most ARM ones) register reset handlers to tweak registers before the CPU would have reset in that future scenario. > until then, we should > put a cpu_reset() immediately after cpu_init() in cpu_copy(). > It doesn't need to be ifdef-guarded either. Will test and post my patch to that effect. Regards, Andreas --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=C3=BCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=C3=B6rffer; HRB 16746 AG N=C3=BC= rnberg