On 25/01/2017 18:53, Claudio Imbrenda wrote: > On 25/01/17 11:21, Paolo Bonzini wrote: >> >> >> On 28/10/2016 19:15, Claudio Imbrenda wrote: >>> * moves the call to qemu_clock_enable away from resume_all_vcpus, and >>> add an explicit call to it before each instance of resume_all_vcpus >>> in the code. >> >> This change adds useless duplication, and isn't matched by a similar >> change to pause_all_vcpus. You need to justify it; I suppose it is >> because the next patch will not call resume_all_cpus? > > Actually it is because the next patch used to call resume_all_vcpus, and > we didn't want it to restart the clock too. But then I ended up not > using resume_all_vcpus. > > And now I have actually no idea why we didn't want to restart the clock. > Maybe we should? After all some CPUs are running. I can patch > gdb_continue_partial to check if any CPU was actually restarted, and if > so start the clock. > > So in the end it should still be correct, but the changes would be kept > small.
Yeah, this sounds like a good solution. Thanks! Paolo >> Most of the callers of pause_all_vcpus/resume_all_vcpus don't let timers >> run, so the clock need not be disabled and enabled. Maybe the right >> places to call qemu_clock_enable are cpu_disable_ticks and >> cpu_enable_ticks? That should work for you. >> >> In that case, please make the first patch the qemu_clock_enable >> movement; the second patch the introduction of vm_prepare_start; the >> third patch the gdbstub change. >> >>> diff --git a/include/sysemu/cpus.h b/include/sysemu/cpus.h >>> index 3728a1e..5fa074b 100644 >>> --- a/include/sysemu/cpus.h >>> +++ b/include/sysemu/cpus.h >>> @@ -5,6 +5,7 @@ >>> bool qemu_in_vcpu_thread(void); >>> void qemu_init_cpu_loop(void); >>> void resume_all_vcpus(void); >>> +void resume_some_vcpus(CPUState **cpus); >>> void pause_all_vcpus(void); >>> void cpu_stop_current(void); >>> void cpu_ticks_init(void); >> >> This function doesn't exist. > > oops. > > > Claudio >
