On 16 May 2012 14:01, Andreas Färber <afaer...@suse.de> wrote: > Am 16.05.2012 14:36, schrieb Igor Mammedov: >> On 05/11/2012 01:26 PM, Andreas Färber wrote: >>> Am 11.05.2012 13:22, schrieb Peter Maydell: >>>> On 10 May 2012 01:14, Andreas Färber<afaer...@suse.de> wrote: >>>>> Eliminates cpu_state_reset() usage. >>>>> >>>>> Signed-off-by: Andreas Färber<afaer...@suse.de> >>>>> --- >>>>> linux-user/main.c | 2 +- >>>>> linux-user/syscall.c | 2 +- >>>>> 2 files changed, 2 insertions(+), 2 deletions(-) >>>>> >>>>> diff --git a/linux-user/main.c b/linux-user/main.c >>>>> index 191b750..49108b8 100644 >>>>> --- a/linux-user/main.c >>>>> +++ b/linux-user/main.c >>>>> @@ -3405,7 +3405,7 @@ int main(int argc, char **argv, char **envp) >>>>> exit(1); >>>>> } >>>>> #if defined(TARGET_I386) || defined(TARGET_SPARC) || >>>>> defined(TARGET_PPC) >>>>> - cpu_state_reset(env); >>>>> + cpu_reset(ENV_GET_CPU(env)); >>>>> #endif >>>>> >>>>> thread_env = env; >>>>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c >>>>> index 20d2a74..539af3f 100644 >>>>> --- a/linux-user/syscall.c >>>>> +++ b/linux-user/syscall.c >>>>> @@ -4262,7 +4262,7 @@ static int do_fork(CPUArchState *env, unsigned >>>>> int flags, abi_ulong newsp, >>>>> /* we create a new CPU instance. */ >>>>> new_env = cpu_copy(env); >>>>> #if defined(TARGET_I386) || defined(TARGET_SPARC) || >>>>> defined(TARGET_PPC) >>>>> - cpu_state_reset(new_env); >>>>> + cpu_reset(ENV_GET_CPU(new_env)); >>>>> #endif >>>>> /* Init regs that differ from the parent. */ >>>>> cpu_clone_regs(new_env, newsp); >>>>> -- >>>> >>>> Do you have any plans to try to rationalise the handling of reset >>>> so that we consistently either do or don't reset the cpu here, >>>> rather than having it done based on a TARGET_* ifdef ? >>> >>> Igor had an RFC for x86; sparc and ppc reset I haven't looked into yet. >>> Cc'ing Alex and Blue. >> I'll rebase RFC for x86 and post patches today and will remove it from here >> by the last patch in patchset so that when this patch applied we could >> remove >> unnecessary call. >> So ACK for target-i386 here. > > Since back then Peter and I have discussed whether we can rather just > remove the #ifdef here and reset for all targets. > > Unfortunately I'm still not clear about some patches that stand in the > way of ObjectClass::realize - if cpu_reset() is moved to realizefn for > all targets then we can just call realize here.
I don't think we need to tangle this up with realize. What I think we should do is: (1) remove the #ifdefs on the cpu reset in linux-user/main.c, so we reset for all target CPU types (2) remove the cpu reset from the do_fork code in linux-user and instead do a cpu reset inside exec.c:cpu_copy(), just after we call cpu_init(). (This is actually fixing a bug -- the thread clone syscall is supposed to leave register values alone, which is why we have cpu_copy in the first place, and calling cpu_reset breaks this.) (3) remove any cpu reset calls from inside target cpu_init functions This cleans things up so all targets work the same (cpu_init never does a reset) and linux-user works the same as system mode (it's the responsibility of the code which creates the cpu to reset it at some point before use). -- PMM