Re: [Qemu-devel] [PATCH v3 1/2] move vm_start to cpus.c

[Claudio Imbrenda]

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.

> 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