Claudio Imbrenda <imbre...@linux.vnet.ibm.com> writes: > This patch: > > * moves vm_start to cpus.c . > * exports qemu_vmstop_requested, since it's needed by vm_start . > * extracts vm_prepare_start from vm_start; it does what vm_start did, > except restarting the cpus. vm_start now calls vm_prepare_start. > * 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.
Can you be a bit more explicit about this? Shouldn't CPU time still accrue even if you are only starting some of them? I'd prefer making resume_all_vcpus() a private function called from resume_some_vcpus() which can then make the QEMU_CLOCK_VIRTUAL decision in one place - with a comment. > * adds resume_some_vcpus, to selectively restart only some CPUs. > > Signed-off-by: Claudio Imbrenda <imbre...@linux.vnet.ibm.com> > --- > cpus.c | 61 > +++++++++++++++++++++++++++++++++++++++++++++- > hw/i386/kvmvapic.c | 2 ++ > include/sysemu/cpus.h | 1 + > include/sysemu/sysemu.h | 2 ++ > target-s390x/misc_helper.c | 2 ++ > vl.c | 32 +++--------------------- > 6 files changed, 70 insertions(+), 30 deletions(-) > > diff --git a/cpus.c b/cpus.c > index 31204bb..c102624 100644 > --- a/cpus.c > +++ b/cpus.c > @@ -1260,12 +1260,28 @@ void resume_all_vcpus(void) > { > CPUState *cpu; > > - qemu_clock_enable(QEMU_CLOCK_VIRTUAL, true); > CPU_FOREACH(cpu) { > cpu_resume(cpu); > } > } > > +/** > + * resume_some_vcpus - resumes some vCPUs, indicated in a NULL-terminated > + * array of CPUState *; if the parameter is NULL, all CPUs are resumed. > + */ > +void resume_some_vcpus(CPUState **cpus) > +{ > + int idx; > + > + if (!cpus) { > + resume_all_vcpus(); > + return; > + } > + for (idx = 0; cpus[idx]; idx++) { > + cpu_resume(cpus[idx]); > + } > +} > + > void cpu_remove(CPUState *cpu) > { > cpu->stop = true; > @@ -1396,6 +1412,49 @@ int vm_stop(RunState state) > return do_vm_stop(state); > } > > +/** > + * Prepare for (re)starting the VM. > + * Returns -1 if the vCPUs are not to be restarted (e.g. if they are already > + * running or in case of an error condition), 0 otherwise. > + */ > +int vm_prepare_start(void) > +{ > + RunState requested; > + int res = 0; > + > + qemu_vmstop_requested(&requested); > + if (runstate_is_running() && requested == RUN_STATE__MAX) { > + return -1; > + } > + > + /* Ensure that a STOP/RESUME pair of events is emitted if a > + * vmstop request was pending. The BLOCK_IO_ERROR event, for > + * example, according to documentation is always followed by > + * the STOP event. > + */ > + if (runstate_is_running()) { > + qapi_event_send_stop(&error_abort); > + res = -1; > + } else { > + replay_enable_events(); > + cpu_enable_ticks(); > + runstate_set(RUN_STATE_RUNNING); > + vm_state_notify(1, RUN_STATE_RUNNING); > + } > + > + /* XXX: is it ok to send this even before actually resuming the CPUs? */ > + qapi_event_send_resume(&error_abort); > + return res; > +} > + > +void vm_start(void) > +{ > + if (!vm_prepare_start()) { > + qemu_clock_enable(QEMU_CLOCK_VIRTUAL, true); > + resume_all_vcpus(); > + } > +} > + > /* does a state transition even if the VM is already stopped, > current state is forgotten forever */ > int vm_stop_force_state(RunState state) > diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c > index 74a549b..3101860 100644 > --- a/hw/i386/kvmvapic.c > +++ b/hw/i386/kvmvapic.c > @@ -446,6 +446,7 @@ static void patch_instruction(VAPICROMState *s, X86CPU > *cpu, target_ulong ip) > abort(); > } > > + qemu_clock_enable(QEMU_CLOCK_VIRTUAL, true); > resume_all_vcpus(); > > if (!kvm_enabled()) { > @@ -686,6 +687,7 @@ static void vapic_write(void *opaque, hwaddr addr, > uint64_t data, > pause_all_vcpus(); > patch_byte(cpu, env->eip - 2, 0x66); > patch_byte(cpu, env->eip - 1, 0x90); > + qemu_clock_enable(QEMU_CLOCK_VIRTUAL, true); > resume_all_vcpus(); > } > > 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); > diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h > index ef2c50b..ac301d6 100644 > --- a/include/sysemu/sysemu.h > +++ b/include/sysemu/sysemu.h > @@ -37,6 +37,7 @@ void vm_state_notify(int running, RunState state); > #define VMRESET_REPORT true > > void vm_start(void); > +int vm_prepare_start(void); > int vm_stop(RunState state); > int vm_stop_force_state(RunState state); > > @@ -60,6 +61,7 @@ void qemu_register_powerdown_notifier(Notifier *notifier); > void qemu_system_debug_request(void); > void qemu_system_vmstop_request(RunState reason); > void qemu_system_vmstop_request_prepare(void); > +bool qemu_vmstop_requested(RunState *r); > int qemu_shutdown_requested_get(void); > int qemu_reset_requested_get(void); > void qemu_system_killed(int signal, pid_t pid); > diff --git a/target-s390x/misc_helper.c b/target-s390x/misc_helper.c > index 4df2ec6..2b74710 100644 > --- a/target-s390x/misc_helper.c > +++ b/target-s390x/misc_helper.c > @@ -133,6 +133,7 @@ static int modified_clear_reset(S390CPU *cpu) > s390_crypto_reset(); > scc->load_normal(CPU(cpu)); > cpu_synchronize_all_post_reset(); > + qemu_clock_enable(QEMU_CLOCK_VIRTUAL, true); > resume_all_vcpus(); > return 0; > } > @@ -152,6 +153,7 @@ static int load_normal_reset(S390CPU *cpu) > scc->initial_cpu_reset(CPU(cpu)); > scc->load_normal(CPU(cpu)); > cpu_synchronize_all_post_reset(); > + qemu_clock_enable(QEMU_CLOCK_VIRTUAL, true); > resume_all_vcpus(); > return 0; > } > diff --git a/vl.c b/vl.c > index f3abd99..42addbb 100644 > --- a/vl.c > +++ b/vl.c > @@ -746,7 +746,7 @@ StatusInfo *qmp_query_status(Error **errp) > return info; > } > > -static bool qemu_vmstop_requested(RunState *r) > +bool qemu_vmstop_requested(RunState *r) > { > qemu_mutex_lock(&vmstop_lock); > *r = vmstop_requested; > @@ -767,34 +767,6 @@ void qemu_system_vmstop_request(RunState state) > qemu_notify_event(); > } > > -void vm_start(void) > -{ > - RunState requested; > - > - qemu_vmstop_requested(&requested); > - if (runstate_is_running() && requested == RUN_STATE__MAX) { > - return; > - } > - > - /* Ensure that a STOP/RESUME pair of events is emitted if a > - * vmstop request was pending. The BLOCK_IO_ERROR event, for > - * example, according to documentation is always followed by > - * the STOP event. > - */ > - if (runstate_is_running()) { > - qapi_event_send_stop(&error_abort); > - } else { > - replay_enable_events(); > - cpu_enable_ticks(); > - runstate_set(RUN_STATE_RUNNING); > - vm_state_notify(1, RUN_STATE_RUNNING); > - resume_all_vcpus(); > - } > - > - qapi_event_send_resume(&error_abort); > -} > - > - > /***********************************************************/ > /* real time host monotonic timer */ > > @@ -1910,6 +1882,7 @@ static bool main_loop_should_exit(void) > if (qemu_reset_requested()) { > pause_all_vcpus(); > qemu_system_reset(VMRESET_REPORT); > + qemu_clock_enable(QEMU_CLOCK_VIRTUAL, true); > resume_all_vcpus(); > if (!runstate_check(RUN_STATE_RUNNING) && > !runstate_check(RUN_STATE_INMIGRATE)) { > @@ -1921,6 +1894,7 @@ static bool main_loop_should_exit(void) > qemu_system_reset(VMRESET_SILENT); > notifier_list_notify(&wakeup_notifiers, &wakeup_reason); > wakeup_reason = QEMU_WAKEUP_REASON_NONE; > + qemu_clock_enable(QEMU_CLOCK_VIRTUAL, true); > resume_all_vcpus(); > qapi_event_send_wakeup(&error_abort); > } -- Alex Bennée