Am 21.01.2013 16:54, schrieb Igor Mammedov:
> On Sun, 20 Jan 2013 08:39:35 +0100
> Andreas Färber <afaer...@suse.de> wrote:
> Subj could be more specific, something along the lines:
> Fix broken breakpoints duplication for i386-{bds,linux}-user
I agree I should expand to "linux-user: bsd-user: ..." at least, and I
intended it to carry qom-cpu tag since it's quite silent around
linux-user these days...
My endeavor here was purely reducing #ifdef'ery and not resolving a
particular bug. After all, ppc and sparc will still be broken; moving
their cpu_reset() into cpu_copy() to immediately after cpu_init() would
hopefully fix that.
Andreas
>
>> Since commit 65dee38052597b6285eb208125369f01b29ba6c1 (target-i386:
>> move cpu_reset and reset callback to cpu.c) the x86 CPU is reset through
>> cpu_init() but was still reset immediately after in linux-user and
>> bsd-user. Similarly it was reset again in linux-user after cpu_copy(),
>> defeating its very purpose. Clean this up.
>>
>> Fixing the ppc and sparc cases of cpu_copy() and overhauling its
>> implementation is left for another day.
> Before b55a37c98 cpu_reset() was called at the end of cpu_init() and copying
> CPUState/duplicating breakpoints afterwards in cpu_copy() worked, BUT commit
> b4558d7481 breaks it, by positioning cpu_reset() call after copying
> CPUState/duplicating breakpoints. That should break as minimum breakpoints
> handling since they all should be duplicated on all CPUs and cpu_reset()
> deletes them explicitly.
>
> From my POV patch fixes bug introduced by b4558d7481, Perhaps you should
> update commit message to mention this commit at least and what this patch
> fixes beside cleanups.
>
> It would be nice though to get confirmation from Blue that all I've said above
> is not total nonsense.
>
>>
>> Cc: Igor Mammedov <imamm...@redhat.com>
>> Signed-off-by: Andreas Färber <afaer...@suse.de>
>> Cc: Peter Maydell <peter.mayd...@linaro.org>
>> ---
>> bsd-user/main.c | 2 +-
>> linux-user/main.c | 2 +-
>> linux-user/syscall.c | 2 +-
>> 3 Dateien geändert, 3 Zeilen hinzugefügt(+), 3 Zeilen entfernt(-)
>>
>> diff --git a/bsd-user/main.c b/bsd-user/main.c
>> index 1dc0330..ae24723 100644
>> --- a/bsd-user/main.c
>> +++ b/bsd-user/main.c
>> @@ -917,7 +917,7 @@ int main(int argc, char **argv)
>> fprintf(stderr, "Unable to find CPU definition\n");
>> exit(1);
>> }
>> -#if defined(TARGET_I386) || defined(TARGET_SPARC) || defined(TARGET_PPC)
>> +#if defined(TARGET_SPARC) || defined(TARGET_PPC)
>> cpu_reset(ENV_GET_CPU(env));
>> #endif
>> thread_env = env;
>> diff --git a/linux-user/main.c b/linux-user/main.c
>> index 0181bc2..3df8aa2 100644
>> --- a/linux-user/main.c
>> +++ b/linux-user/main.c
>> @@ -3540,7 +3540,7 @@ int main(int argc, char **argv, char **envp)
>> fprintf(stderr, "Unable to find CPU definition\n");
>> exit(1);
>> }
>> -#if defined(TARGET_I386) || defined(TARGET_SPARC) || defined(TARGET_PPC)
>> +#if defined(TARGET_SPARC) || defined(TARGET_PPC)
>> cpu_reset(ENV_GET_CPU(env));
>> #endif
>>
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index 693e66f..7be6144 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -4361,7 +4361,7 @@ static int do_fork(CPUArchState *env, unsigned int
>> flags, abi_ulong newsp, init_task_state(ts);
>> /* we create a new CPU instance. */
>> new_env = cpu_copy(env);
>> -#if defined(TARGET_I386) || defined(TARGET_SPARC) || defined(TARGET_PPC)
>> +#if defined(TARGET_SPARC) || defined(TARGET_PPC)
>> cpu_reset(ENV_GET_CPU(new_env));
>> #endif
>> /* Init regs that differ from the parent. */
>
>
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
