qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH for-1.4] *-user: Don't reset X86CPU again


From: Igor Mammedov
Subject: Re: [Qemu-devel] [PATCH for-1.4] *-user: Don't reset X86CPU again
Date: Wed, 30 Jan 2013 08:08:10 +0100

On Wed, 30 Jan 2013 01:38:47 +0100
Andreas Färber <address@hidden> wrote:

> Am 21.01.2013 16:54, schrieb Igor Mammedov:
> > On Sun, 20 Jan 2013 08:39:35 +0100
> > Andreas Färber <address@hidden> wrote:
> > Subj could be more specific, something along the lines:
> >   Fix broken breakpoints duplication for i386-{bds,linux}-user
> > 
> >> 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.
> 
> I believe your analysis wrt breakpoints and watchpoints is incorrect
> since the memset() in cpu_reset() handlers only goes up to
> offset(CPUArchState, breakpoints), i.e. not including the breakpoints.
memsetting would lead to leak as minimum, x86_cpu_reset() close to the end
calls cpu_breakpoint_remove_all() and cpu_watchpoint_remove_all() instead. 

> 
> But as discussed with Peter I've posted a v2 that first fixes the reset
> bug and then cleans up the now-superfluous x86 reset.
Cool.

> 
> 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
> 


-- 
Regards,
  Igor



reply via email to

[Prev in Thread] Current Thread [Next in Thread]