Re: [Qemu-devel] [PATCH 3/9] linux-user: Don't reset a new thread's CPU

[Andreas Färber]

Am 06.07.2013 12:31, schrieb Peter Maydell:
> On 6 July 2013 01:36, Alexander Graf <address@hidden> wrote:
>> When we create a new thread, there is no reason to reset it. I'm fairly sure
>> the code has mostly been left in there because nobody understood why it was
>> there in the first place.
> 
> 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/77868120cfe93ad7816dfac6546684e5a6c6e256?f=linux-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 identical to
>> the old one's.
> 
> 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

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg