Patch queue, part III
cover Patch queue, part III
This round is kind of an assorted collection:
- make kvm_cpu_exec ready for qemu-kvm reuse
- reworked vm_stop handling (as preparation for kvm_cpu_exec reuse)
- the return of kvmclock
- cleanups around KVM's dirty logging/slot management
- style cleanups of cpus.c
The patches related to the first item allowed me to remove another 400 lines of duplicate qemu-kvm code. Current local stats:
# wc -l qemu-kvm.[ch] qemu-kvm-x86.c 1142 qemu-kvm.c 309 qemu-kvm.h 230 qemu-kvm-x86.c 1681 total (vs. 3513 in current master)
I'm about to look into the last issue soon: consolidation of the threading code.
For upstream qemu, there is now only the MCE rework pending in my queue. That will be sent as part IV after this series has passed the review.
Anthony PERARD (1):
Introduce log_start/log_stop in CPUPhysMemoryClient
Jan Kiszka (14):
Refactor kvm&tcg function names in cpus.c
Refactor cpu_has_work/any_cpu_has_work in cpus.c Fix a few coding style violations in cpus.c
Improve vm_stop reason declarations
Refactor debug and vmstop request interface
Move debug exception handling out of cpu_exec
kvm: Separate TCG from KVM cpu execution
kvm: x86: Prepare VCPU loop for in-kernel irqchip
kvm: Drop return values from kvm_arch_pre/post_run
kvm: x86: Catch and report failing IRQ and NMI injections
kvm: Remove unneeded memory slot reservation
cirrus: Remove obsolete kvm.h include
kvm: Make kvm_state globally available
kvm: x86: Introduce kvmclock device to save/restore its state
Makefile.target | 4 +- cpu-all.h | 6 ++ cpu-common.h | 4 + cpu-exec.c | 43 ++----------- cpus.c | 182 +++++++++++++++++++++++++++++++--------------------- cpus.h | 2 - exec.c | 30 +++++++++ gdbstub.c | 19 +++--- hw/cirrus_vga.c | 1 - hw/ide/core.c | 2 +- hw/kvmclock.c | 125 +++++++++++++++++++++++++++++++++++ hw/kvmclock.h | 14 ++++ hw/pc_piix.c | 31 +++++++-- hw/scsi-disk.c | 2 +- hw/vga.c | 31 +++++---- hw/vhost.c | 2 + hw/virtio-blk.c | 2 +- hw/watchdog.c | 2 +- kvm-all.c | 35 +++++----- kvm-stub.c | 10 --- kvm.h | 9 +-- migration.c | 2 +- monitor.c | 4 +- savevm.c | 4 +- sysemu.h | 10 +++ target-i386/kvm.c | 93 +++++++++++++++----------- target-ppc/kvm.c | 6 +- target-s390x/kvm.c | 6 +- vl.c | 22 +++++-- 29 files changed, 462 insertions(+), 241 deletions(-) create mode 100644 hw/kvmclock.c create mode 100644 hw/kvmclock.h
PATCH 1/15 Refactor kvm&tcg function names in cpus.c
Pure interface cosmetics: Ensure that only kvm core services (as declared in kvm.h) start with "kvm_". Prepend "qemu_" to those that violate this rule in cpus.c. Also rename the corresponding tcg functions for the sake of consistency.
--- cpus.c | 16 ++++++++-------- 1 files changed, 8 insertions(+), 8 deletions(-)
cpus.c +8 -8
@@ -774,7 +774,7 @@ static void qemu_kvm_wait_io_event(CPUState *env)static int qemu_cpu_exec (CPUState *env); -static void *kvm_cpu_thread_fn (void *arg) +static void *qemu_kvm_cpu_thread_fn (void *arg) {CPUState *env = arg;int r;
@@ -807,7 +807,7 @@ static void *kvm_cpu_thread_fn(void *arg)return NULL ; } -static void *tcg_cpu_thread_fn (void *arg) +static void *qemu_tcg_cpu_thread_fn (void *arg) {CPUState *env = arg;
@@ -926,7 +926,7 @@ void resume_all_vcpus(void) } } -static void tcg_init_vcpu (void *_env) +static void qemu_tcg_init_vcpu (void *_env) {CPUState *env = _env;/* share a single thread for all cpus with TCG */
@@ -934,7 +934,7 @@ static void tcg_init_vcpu(void *_env) env->thread =qemu_mallocz (sizeof (QemuThread)); env->halt_cond =qemu_mallocz (sizeof (QemuCond));qemu_cond_init (env->halt_cond); -qemu_thread_create (env->thread, tcg_cpu_thread_fn, env); +qemu_thread_create (env->thread, qemu_tcg_cpu_thread_fn, env);while (env->created ==0 )qemu_cond_timedwait (&qemu_cpu_cond, &qemu_global_mutex,100 ); tcg_cpu_thread = env->thread;
@@ -945,12 +945,12 @@ static void tcg_init_vcpu(void *_env) } } -static void kvm_start_vcpu (CPUState *env) +static void qemu_kvm_start_vcpu (CPUState *env) { env->thread =qemu_mallocz (sizeof (QemuThread)); env->halt_cond =qemu_mallocz (sizeof (QemuCond));qemu_cond_init (env->halt_cond); -qemu_thread_create (env->thread, kvm_cpu_thread_fn, env); +qemu_thread_create (env->thread, qemu_kvm_cpu_thread_fn, env);while (env->created ==0 )qemu_cond_timedwait (&qemu_cpu_cond, &qemu_global_mutex,100 ); }
@@ -962,9 +962,9 @@ void qemu_init_vcpu(void *_env) env->nr_cores = smp_cores; env->nr_threads = smp_threads;if (kvm_enabled ()) -kvm_start_vcpu (env); +qemu_kvm_start_vcpu (env);else -tcg_init_vcpu (env); +qemu_tcg_init_vcpu (env); }void qemu_notify_event (void )
Signature · 1 line
1.7.1
PATCH 2/15 Refactor cpu_has_work/any_cpu_has_work in cpus.c
In reply to Jan KiszkaAvoid duplicate use of the function name cpu_has_work, it's confusing. Refactor cpu_has_work to cpu_is_idle and do the same with any_cpu_has_work.
--- cpus.c | 43 +++++++++++++++++++++++-------------------- 1 files changed, 23 insertions(+), 20 deletions(-)
cpus.c +23 -20
@@ -137,29 +137,30 @@ static int cpu_can_run(CPUState *env)return 1 ; } -static int cpu_has_work (CPUState *env) +static bool cpu_is_idle (CPUState *env) { -if (env->stop) -return 1 ; -if (env->queued_work_first) -return 1 ; -if (env->stopped || !vm_running) -return 0 ; -if (!env->halted) -return 1 ; -if (qemu_cpu_has_work (env)) -return 1 ; -return 0 ; +if (env->stop || env->queued_work_first) { +return false; + } +if (env->stopped || !vm_running) { +return true; + } +if (!env->halted ||qemu_cpu_has_work (env)) { +return false; + } +return true; } -static int any_cpu_has_work (void ) +static bool all_cpus_idle (void ) {CPUState *env; -for (env = first_cpu; env !=NULL ; env = env->next_cpu) -if (cpu_has_work (env)) -return 1 ; -return 0 ; +for (env = first_cpu; env !=NULL ; env = env->next_cpu) { +if (!cpu_is_idle (env)) { +return false; + } + } +return true; }static void cpu_debug_handler (CPUState *env)
@@ -743,8 +744,9 @@ static void qemu_tcg_wait_io_event(void) {CPUState *env; -while (!any_cpu_has_work ()) +while (all_cpus_idle ()) {qemu_cond_timedwait (tcg_halt_cond, &qemu_global_mutex,1000 ); + }qemu_mutex_unlock (&qemu_global_mutex);
@@ -765,8 +767,9 @@ static void qemu_tcg_wait_io_event(void)static void qemu_kvm_wait_io_event (CPUState *env) { -while (!cpu_has_work (env)) +while (cpu_is_idle (env)) {qemu_cond_timedwait (env->halt_cond, &qemu_global_mutex,1000 ); + }qemu_kvm_eat_signals (env);qemu_wait_io_event_common (env);
@@ -1070,7 +1073,7 @@ bool cpu_exec_all(void) } } exit_request =0 ; -return any_cpu_has_work (); +return !all_cpus_idle (); }void set_numa_modes (void )
Signature · 1 line
1.7.1
Re: [PATCH 02/15] Refactor cpu_has_work/any_cpu_has_work in cpus.c
Quoted 32 lines · Jan Kiszka
Avoid duplicate use of the function name
cpu_has_work, it's confusing. Refactorcpu_has_worktocpu_is_idleand do the same withany_cpu_has_work.Signed-off-by Jan Kiszka <[email protected]>--- cpus.c | 43 +++++++++++++++++++++++-------------------- 1 files changed, 23 insertions(+), 20 deletions(-)cpus.c +2
-12@@ -137,29 +137,30 @@ static int cpu_can_run(CPUState *env)return 1 ; } -static int cpu_has_work (CPUState *env) +static bool cpu_is_idle (CPUState *env) { -if (env->stop) -return 1 ; -if (env->queued_work_first) -return 1 ; -if (env->stopped || !vm_running) -return 0 ; -if (!env->halted) -return 1 ; -if (qemu_cpu_has_work (env)) -return 1 ; -return 0 ; +if (env->stop || env->queued_work_first) {
cpus.c static int cpu_can_run(CPUState *env) · line 152
Quoted 10 lines
cpus.c +9
-0+return false; + } +if (env->stopped || !vm_running) { +return true; + } +if (!env->halted ||qemu_cpu_has_work (env)) { +return false; + } +return true; }
Do you really find it easier to read evaluations grouped with || ? I don't.
Sorry but the name change does not feel right either: CPU is still idle if the vm is not running.
Re: [PATCH 02/15] Refactor cpu_has_work/any_cpu_has_work in cpus.c
Quoted 38 lines · Marcelo Tosatti
Quoted 37 lines · Jan Kiszka
Avoid duplicate use of the function name
cpu_has_work, it's confusing. Refactorcpu_has_worktocpu_is_idleand do the same withany_cpu_has_work.Signed-off-by Jan Kiszka <[email protected]>--- cpus.c | 43 +++++++++++++++++++++++-------------------- 1 files changed, 23 insertions(+), 20 deletions(-)cpus.c +7
-12@@ -137,29 +137,30 @@ static int cpu_can_run(CPUState *env)return 1 ; } -static int cpu_has_work (CPUState *env) +static bool cpu_is_idle (CPUState *env) { -if (env->stop) -return 1 ; -if (env->queued_work_first) -return 1 ; -if (env->stopped || !vm_running) -return 0 ; -if (!env->halted) -return 1 ; -if (qemu_cpu_has_work (env)) -return 1 ; -return 0 ; +if (env->stop || env->queued_work_first) { +return false; + } +if (env->stopped || !vm_running) { +return true; + }
Quoted 8 lines
cpus.c static int cpu_can_run(CPUState *env) · line 152
Quoted 5 lines
cpus.c +4
-0+if (!env->halted ||qemu_cpu_has_work (env)) { +return false; + } +return true; }Do you really find it easier to read evaluations grouped with || ? I don't.
I do, specifically as the old version was even more confusing in that important detail "return 0" vs. "return 1". But even the new benefits from the grouping IMHO.
Quoted 3 lines
Sorry but the name change does not feel right either: CPU is still idle if the vm is not running.
But that's exactly what the function returns. Or is it confusing if we are talking about the vcpu or the whole thread here? What about "cpu_thread_is_idle" then?
Jan
Re: [PATCH 02/15] Refactor cpu_has_work/any_cpu_has_work in cpus.c
Quoted 6 lines · Jan Kiszka
Quoted 2 lines
Do you really find it easier to read evaluations grouped with || ? I don't.
I do, specifically as the old version was even more confusing in that important detail "return 0" vs. "return 1". But even the new benefits from the grouping IMHO.
Well alright.
Quoted 6 lines
Quoted 2 lines
Sorry but the name change does not feel right either: CPU is still idle if the vm is not running.
But that's exactly what the function returns. Or is it confusing if we are talking about the vcpu or the whole thread here? What about "
cpu_thread_is_idle" then?
Yes thats better.
PATCH v2 2/15 Refactor cpu_has_work/any_cpu_has_work in cpus.c
In reply to Jan KiszkaAvoid duplicate use of the function name cpu_has_work, it's confusing, also their scope. Refactor cpu_has_work to cpu_thread_is_idle and do the same with any_cpu_has_work.
---
Changes in v2:
- renaming: cpu_thread_is_idle and all_cpu_threads_idle
cpus.c | 43 +++++++++++++++++++++++-------------------- 1 files changed, 23 insertions(+), 20 deletions(-)
cpus.c +23 -20
@@ -137,29 +137,30 @@ static int cpu_can_run(CPUState *env)return 1 ; } -static int cpu_has_work (CPUState *env) +static bool cpu_thread_is_idle (CPUState *env) { -if (env->stop) -return 1 ; -if (env->queued_work_first) -return 1 ; -if (env->stopped || !vm_running) -return 0 ; -if (!env->halted) -return 1 ; -if (qemu_cpu_has_work (env)) -return 1 ; -return 0 ; +if (env->stop || env->queued_work_first) { +return false; + } +if (env->stopped || !vm_running) { +return true; + } +if (!env->halted ||qemu_cpu_has_work (env)) { +return false; + } +return true; } -static int any_cpu_has_work (void ) +static bool all_cpu_threads_idle (void ) {CPUState *env; -for (env = first_cpu; env !=NULL ; env = env->next_cpu) -if (cpu_has_work (env)) -return 1 ; -return 0 ; +for (env = first_cpu; env !=NULL ; env = env->next_cpu) { +if (!cpu_thread_is_idle (env)) { +return false; + } + } +return true; }static void cpu_debug_handler (CPUState *env)
@@ -743,8 +744,9 @@ static void qemu_tcg_wait_io_event(void) {CPUState *env; -while (!any_cpu_has_work ()) +while (all_cpu_threads_idle ()) {qemu_cond_timedwait (tcg_halt_cond, &qemu_global_mutex,1000 ); + }qemu_mutex_unlock (&qemu_global_mutex);
@@ -765,8 +767,9 @@ static void qemu_tcg_wait_io_event(void)static void qemu_kvm_wait_io_event (CPUState *env) { -while (!cpu_has_work (env)) +while (cpu_thread_is_idle (env)) {qemu_cond_timedwait (env->halt_cond, &qemu_global_mutex,1000 ); + }qemu_kvm_eat_signals (env);qemu_wait_io_event_common (env);
@@ -1070,7 +1073,7 @@ bool cpu_exec_all(void) } } exit_request =0 ; -return any_cpu_has_work (); +return !all_cpu_threads_idle (); }void set_numa_modes (void )
Signature · 1 line
1.7.1
PATCH 3/15 Fix a few coding style violations in cpus.c
In reply to Jan KiszkaNo functional changes.
--- cpus.c | 71 +++++++++++++++++++++++++++++++++++++++------------------------ 1 files changed, 44 insertions(+), 27 deletions(-)
cpus.c +44 -27
@@ -130,10 +130,12 @@ static void do_vm_stop(int reason)static int cpu_can_run (CPUState *env) { -if (env->stop) +if (env->stop) {return 0 ; -if (env->stopped || !vm_running) + } +if (env->stopped || !vm_running) {return 0 ; + }return 1 ; }
@@ -225,9 +227,9 @@ static void qemu_event_increment(void)static const uint64_t val =1 ;ssize_t ret; -if (io_thread_fd ==-1 ) +if (io_thread_fd ==-1 ) {return ; - + }do { ret =write (io_thread_fd, &val,sizeof (val)); }while (ret <0 && errno ==EINTR );
@@ -258,17 +260,17 @@ static int qemu_event_init(void)int fds[2 ]; err =qemu_eventfd (fds); -if (err ==-1 ) +if (err ==-1 ) {return -errno; - + } err =fcntl_setfl (fds[0 ],O_NONBLOCK ); -if (err <0 ) +if (err <0 ) { goto fail; - + } err =fcntl_setfl (fds[1 ],O_NONBLOCK ); -if (err <0 ) +if (err <0 ) { goto fail; - + }qemu_set_fd_handler2 (fds[0 ],NULL , qemu_event_read,NULL , (void *)(unsigned long )fds[0 ]);
@@ -527,7 +529,6 @@ void pause_all_vcpus(void)void qemu_cpu_kick (void *env) { -return ; }void qemu_cpu_kick_self (void )
@@ -660,13 +661,15 @@ int qemu_init_main_loop(void) blocked_signals =block_io_signals (); ret =qemu_signalfd_init (blocked_signals); -if (ret) +if (ret) {return ret; + }/* Note eventfd must be drained before signalfd handlers run */ ret =qemu_event_init (); -if (ret) +if (ret) {return ret; + }qemu_cond_init (&qemu_pause_cond);qemu_cond_init (&qemu_system_cond);
@@ -696,10 +699,11 @@ void run_on_cpu(CPUState *env, void (*func)(void *data), void *data) wi.func = func; wi.data = data; -if (!env->queued_work_first) +if (!env->queued_work_first) { env->queued_work_first = &wi; -else + }else { env->queued_work_last->next = &wi; + } env->queued_work_last = &wi; wi.next =NULL ; wi.done = false;
@@ -717,8 +721,9 @@ static void flush_queued_work(CPUState *env) {struct qemu_work_item *wi; -if (!env->queued_work_first) +if (!env->queued_work_first) {return ; + }while ((wi = env->queued_work_first)) { env->queued_work_first = wi->next;
@@ -798,12 +803,14 @@ static void *qemu_kvm_cpu_thread_fn(void *arg)qemu_cond_signal (&qemu_cpu_cond);/* and wait for machine initialization */ -while (!qemu_system_ready) +while (!qemu_system_ready) {qemu_cond_timedwait (&qemu_system_cond, &qemu_global_mutex,100 ); + }while (1 ) { -if (cpu_can_run (env)) +if (cpu_can_run (env)) {qemu_cpu_exec (env); + }qemu_kvm_wait_io_event (env); }
@@ -819,13 +826,15 @@ static void *qemu_tcg_cpu_thread_fn(void *arg)/* signal CPU creation */ qemu_mutex_lock (&qemu_global_mutex); -for (env = first_cpu; env !=NULL ; env = env->next_cpu) +for (env = first_cpu; env !=NULL ; env = env->next_cpu) { env->created =1 ; + }qemu_cond_signal (&qemu_cpu_cond);/* and wait for machine initialization */ -while (!qemu_system_ready) +while (!qemu_system_ready) {qemu_cond_timedwait (&qemu_system_cond, &qemu_global_mutex,100 ); + }while (1 ) {cpu_exec_all ();
@@ -838,6 +847,7 @@ static void *qemu_tcg_cpu_thread_fn(void *arg)void qemu_cpu_kick (void *_env) {CPUState *env = _env; +qemu_cond_broadcast (env->halt_cond);if (!env->thread_kicked) {qemu_thread_signal (env->thread,SIG_IPI );
@@ -889,8 +899,9 @@ static int all_vcpus_paused(void)CPUState *penv = first_cpu;while (penv) { -if (!penv->stopped) +if (!penv->stopped) {return 0 ; + } penv = (CPUState *)penv->next_cpu; }
@@ -932,14 +943,16 @@ void resume_all_vcpus(void)static void qemu_tcg_init_vcpu (void *_env) {CPUState *env = _env; +/* share a single thread for all cpus with TCG */ if (!tcg_cpu_thread) { env->thread =qemu_mallocz (sizeof (QemuThread)); env->halt_cond =qemu_mallocz (sizeof (QemuCond));qemu_cond_init (env->halt_cond);qemu_thread_create (env->thread, qemu_tcg_cpu_thread_fn, env); -while (env->created ==0 ) +while (env->created ==0 ) {qemu_cond_timedwait (&qemu_cpu_cond, &qemu_global_mutex,100 ); + } tcg_cpu_thread = env->thread; tcg_halt_cond = env->halt_cond; }else {
@@ -954,8 +967,9 @@ static void qemu_kvm_start_vcpu(CPUState *env) env->halt_cond =qemu_mallocz (sizeof (QemuCond));qemu_cond_init (env->halt_cond);qemu_thread_create (env->thread, qemu_kvm_cpu_thread_fn, env); -while (env->created ==0 ) +while (env->created ==0 ) {qemu_cond_timedwait (&qemu_cpu_cond, &qemu_global_mutex,100 ); + } }void qemu_init_vcpu (void *_env)
@@ -964,10 +978,11 @@ void qemu_init_vcpu(void *_env) env->nr_cores = smp_cores; env->nr_threads = smp_threads; -if (kvm_enabled ()) +if (kvm_enabled ()) {qemu_kvm_start_vcpu (env); -else + }else {qemu_tcg_init_vcpu (env); + } }void qemu_notify_event (void )
@@ -1050,16 +1065,18 @@ bool cpu_exec_all(void) {int r; -if (next_cpu ==NULL ) +if (next_cpu ==NULL ) { next_cpu = first_cpu; + }for (; next_cpu !=NULL && !exit_request; next_cpu = next_cpu->next_cpu) {CPUState *env = next_cpu;qemu_clock_enable (vm_clock, (env->singlestep_enabled &SSTEP_NOTIMER ) ==0 ); -if (qemu_alarm_pending ()) +if (qemu_alarm_pending ()) {break ; + }if (cpu_can_run (env)) { r =qemu_cpu_exec (env);if (kvm_enabled ()) {
Signature · 1 line
1.7.1
PATCH 4/15 Improve vm_stop reason declarations
In reply to Jan KiszkaDefine and use dedicated constants for vm_stop reasons, they actually have nothing to do with the EXCP_* defines used so far. At this chance, specify more detailed reasons so that VM state change handlers can evaluate them.
--- cpus.c | 4 ++-- gdbstub.c | 19 ++++++++++--------- hw/ide/core.c | 2 +- hw/scsi-disk.c | 2 +- hw/virtio-blk.c | 2 +- hw/watchdog.c | 2 +- kvm-all.c | 2 +- migration.c | 2 +- monitor.c | 4 ++-- savevm.c | 4 ++-- sysemu.h | 8 ++++++++ vl.c | 2 +- 12 files changed, 31 insertions(+), 22 deletions(-)
cpus.c +2 -2
@@ -168,8 +168,8 @@ static bool all_cpus_idle(void)static void cpu_debug_handler (CPUState *env) {gdb_set_stop_cpu (env); - debug_requested =EXCP_DEBUG ; -vm_stop (EXCP_DEBUG ); + debug_requested =VMSTOP_DEBUG ; +vm_stop (VMSTOP_DEBUG ); }#ifdef CONFIG_LINUX
gdbstub.c +10 -9
@@ -2194,14 +2194,14 @@ static void gdb_vm_state_change(void *opaque, int running, int reason)const char *type;int ret; -if (running || (reason !=EXCP_DEBUG && reason !=EXCP_INTERRUPT ) || - s->state ==RS_INACTIVE || s->state ==RS_SYSCALL ) +if (running || (reason !=VMSTOP_DEBUG && reason !=VMSTOP_INTERRUPT ) || + s->state ==RS_INACTIVE || s->state ==RS_SYSCALL ) {return ; - + }/* disable single step if it was enable */ cpu_single_step (env,0 ); -if (reason ==EXCP_DEBUG ) { +if (reason ==VMSTOP_DEBUG ) {if (env->watchpoint_hit) {switch (env->watchpoint_hit->flags &BP_MEM_ACCESS ) {case BP_MEM_READ :
@@ -2252,7 +2252,7 @@ void gdb_do_syscall(gdb_syscall_complete_cb cb, const char *fmt, ...) gdb_current_syscall_cb = cb; s->state =RS_SYSCALL ;#ifndef CONFIG_USER_ONLY -vm_stop (EXCP_DEBUG ); +vm_stop (VMSTOP_DEBUG );#endif s->state =RS_IDLE ;va_start (va, fmt);
@@ -2326,7 +2326,7 @@ static void gdb_read_byte(GDBState *s, int ch)if (vm_running) {/* when the CPU is running, we cannot do anything except stop it when receiving a char */ -vm_stop (EXCP_INTERRUPT ); +vm_stop (VMSTOP_INTERRUPT ); } else#endif {
@@ -2588,7 +2588,7 @@ static void gdb_chr_event(void *opaque, int event) {switch (event) {case CHR_EVENT_OPENED : -vm_stop (EXCP_INTERRUPT ); +vm_stop (VMSTOP_INTERRUPT ); gdb_has_xml =0 ;break ;default :
@@ -2628,8 +2628,9 @@ static int gdb_monitor_write(CharDriverState *chr, const uint8_t *buf, int len)#ifndef _WIN32static void gdb_sigterm_handler (int signal) { -if (vm_running) -vm_stop (EXCP_INTERRUPT ); +if (vm_running) { +vm_stop (VMSTOP_INTERRUPT ); + } }#endif
hw/ide/core.c +1 -1
@@ -465,7 +465,7 @@ static int ide_handle_rw_error(IDEState *s, int error, int op) s->bus->dma->ops->set_unit (s->bus->dma, s->unit); s->bus->dma->ops->add_status (s->bus->dma, op);bdrv_mon_event (s->bs,BDRV_ACTION_STOP , is_read); -vm_stop (0 ); +vm_stop (VMSTOP_DISKFULL ); }else {if (op &BM_STATUS_DMA_RETRY ) {dma_buf_commit (s,0 );
hw/scsi-disk.c +1 -1
@@ -239,7 +239,7 @@ static int scsi_handle_rw_error(SCSIDiskReq *r, int error, int type) r->status |=SCSI_REQ_STATUS_RETRY | type;bdrv_mon_event (s->bs,BDRV_ACTION_STOP , is_read); -vm_stop (0 ); +vm_stop (VMSTOP_DISKFULL ); }else {if (type ==SCSI_REQ_STATUS_RETRY_READ ) { r->req.bus->complete (r->req.bus,SCSI_REASON_DATA , r->req.tag,0 );
hw/virtio-blk.c +1 -1
@@ -78,7 +78,7 @@ static int virtio_blk_handle_rw_error(VirtIOBlockReq *req, int error, req->next = s->rq; s->rq = req;bdrv_mon_event (s->bs,BDRV_ACTION_STOP , is_read); -vm_stop (0 ); +vm_stop (VMSTOP_DISKFULL ); }else {virtio_blk_req_complete (req,VIRTIO_BLK_S_IOERR );bdrv_mon_event (s->bs,BDRV_ACTION_REPORT , is_read);
hw/watchdog.c +1 -1
@@ -132,7 +132,7 @@ void watchdog_perform_action(void)case WDT_PAUSE :/* same as 'stop' command in monitor */ watchdog_mon_event ("pause" ); -vm_stop (0 ); +vm_stop (VMSTOP_WATCHDOG );break ;case WDT_DEBUG :
kvm-all.c +1 -1
@@ -994,7 +994,7 @@ int kvm_cpu_exec(CPUState *env)if (ret <0 ) {cpu_dump_state (env, stderr, fprintf,CPU_DUMP_CODE ); -vm_stop (0 ); +vm_stop (VMSTOP_PANIC ); env->exit_request =1 ; }if (env->exit_request) {
migration.c +1 -1
@@ -378,7 +378,7 @@ void migrate_fd_put_ready(void *opaque)int old_vm_running = vm_running;DPRINTF ("done iterating\n" ); -vm_stop (0 ); +vm_stop (VMSTOP_RWSTATE );if ((qemu_savevm_state_complete (s->mon, s->file)) <0 ) {if (old_vm_running) {
monitor.c +2 -2
@@ -1255,7 +1255,7 @@ static void do_singlestep(Monitor *mon, const QDict *qdict)*/ static int do_stop (Monitor *mon,const QDict *qdict,QObject **ret_data) { -vm_stop (EXCP_INTERRUPT ); +vm_stop (VMSTOP_INTERRUPT );return 0 ; }
@@ -2783,7 +2783,7 @@ static void do_loadvm(Monitor *mon, const QDict *qdict)int saved_vm_running = vm_running;const char *name =qdict_get_str (qdict,"name" ); -vm_stop (0 ); +vm_stop (VMSTOP_RWSTATE );if (load_vmstate (name) ==0 && saved_vm_running) {vm_start ();
savevm.c +2 -2
@@ -1575,7 +1575,7 @@ static int qemu_savevm_state(Monitor *mon, QEMUFile *f)int ret; saved_vm_running = vm_running; -vm_stop (0 ); +vm_stop (VMSTOP_RWSTATE );if (qemu_savevm_state_blocked (mon)) { ret = -EINVAL ;
@@ -1896,7 +1896,7 @@ void do_savevm(Monitor *mon, const QDict *qdict) } saved_vm_running = vm_running; -vm_stop (0 ); +vm_stop (VMSTOP_RWSTATE );memset (sn,0 ,sizeof (*sn));
sysemu.h +8 -0
@@ -37,6 +37,14 @@ VMChangeStateEntry *qemu_add_vm_change_state_handler(VMChangeStateHandler *cb, void *opaque);void qemu_del_vm_change_state_handler (VMChangeStateEntry *e); +#define VMSTOP_INTERRUPT 0 +#define VMSTOP_DEBUG 1 +#define VMSTOP_SHUTDOWN 2 +#define VMSTOP_DISKFULL 3 +#define VMSTOP_WATCHDOG 4 +#define VMSTOP_PANIC 5 +#define VMSTOP_RWSTATE 6 +void vm_start (void );void vm_stop (int reason);
vl.c +1 -1
@@ -1433,7 +1433,7 @@ static void main_loop(void)if (qemu_shutdown_requested ()) {monitor_protocol_event (QEVENT_SHUTDOWN ,NULL );if (no_shutdown) { -vm_stop (0 ); +vm_stop (VMSTOP_SHUTDOWN ); no_shutdown =0 ; }else break ;
Signature · 1 line
1.7.1
Re: [PATCH 04/15] Improve vm_stop reason declarations
gdbstub.c static void gdb_vm_state_change(void *opaque, int running, int reason) · line 2199
Quoted 12 lines · Jan Kiszka
index d6556c9..3397566 100644
gdbstub.c +2
-2@@ -2194,14 +2194,14 @@ static void gdb_vm_state_change(void *opaque, int running, int reason)const char *type;int ret; -if (running || (reason !=EXCP_DEBUG && reason !=EXCP_INTERRUPT ) || - s->state ==RS_INACTIVE || s->state ==RS_SYSCALL ) +if (running || (reason !=VMSTOP_DEBUG && reason !=VMSTOP_INTERRUPT ) || + s->state ==RS_INACTIVE || s->state ==RS_SYSCALL ) {return ;
What about VMSTOP_USER ?
VMSTOP_INTERRUPT -> "the VM is stopped by an interrupt".
VMSTOP_USER -> "the VM is stopped by the user".
migration.c void migrate_fd_put_ready(void *opaque) · line 381
Quoted 10 lines
migration.c +1
-1@@ -378,7 +378,7 @@ void migrate_fd_put_ready(void *opaque)int old_vm_running = vm_running;DPRINTF ("done iterating\n" ); -vm_stop (0 ); +vm_stop (VMSTOP_RWSTATE );
VMSTOP_RWSTATE is cryptic. What about VMSTOP_SAVEVM, MIGRATE, etc.
Nice!
Re: [PATCH 04/15] Improve vm_stop reason declarations
Quoted 16 lines · Marcelo Tosatti
gdbstub.c static void gdb_vm_state_change(void *opaque, int running, int reason) · line 2199
Quoted 12 lines · Jan Kiszka
index d6556c9..3397566 100644
gdbstub.c +2
-2@@ -2194,14 +2194,14 @@ static void gdb_vm_state_change(void *opaque, int running, int reason)const char *type;int ret; -if (running || (reason !=EXCP_DEBUG && reason !=EXCP_INTERRUPT ) || - s->state ==RS_INACTIVE || s->state ==RS_SYSCALL ) +if (running || (reason !=VMSTOP_DEBUG && reason !=VMSTOP_INTERRUPT ) || + s->state ==RS_INACTIVE || s->state ==RS_SYSCALL ) {return ;What about
VMSTOP_USER?
Quoted 3 lines
VMSTOP_INTERRUPT-> "the VM is stopped by an interrupt".
VMSTOP_USER-> "the VM is stopped by the user".
Makes a lot of sense, will change.
Quoted 8 lines
Quoted 8 lines
migration.c +0
-0@@ -378,7 +378,7 @@ void migrate_fd_put_ready(void *opaque)int old_vm_running = vm_running;
Quoted 5 lines
migration.c void migrate_fd_put_ready(void *opaque) · line 381
Quoted 3 lines
migration.c +1
-1DPRINTF ("done iterating\n" ); -vm_stop (0 ); +vm_stop (VMSTOP_RWSTATE );
VMSTOP_RWSTATEis cryptic. What aboutVMSTOP_SAVEVM, MIGRATE, etc.
Both alternatives are not completely accurate as we also stop of vmload. What about VMSTOP_SYNC_STATE or UPDATE_STATE?
Jan
Re: [PATCH 04/15] Improve vm_stop reason declarations
Quoted 31 lines · Jan Kiszka
Quoted 16 lines · Marcelo Tosatti
gdbstub.c static void gdb_vm_state_change(void *opaque, int running, int reason) · line 2199
Quoted 12 lines · Jan Kiszka
index d6556c9..3397566 100644
gdbstub.c +2
-2@@ -2194,14 +2194,14 @@ static void gdb_vm_state_change(void *opaque, int running, int reason)const char *type;int ret; -if (running || (reason !=EXCP_DEBUG && reason !=EXCP_INTERRUPT ) || - s->state ==RS_INACTIVE || s->state ==RS_SYSCALL ) +if (running || (reason !=VMSTOP_DEBUG && reason !=VMSTOP_INTERRUPT ) || + s->state ==RS_INACTIVE || s->state ==RS_SYSCALL ) {return ;What about
VMSTOP_USER?Quoted 3 lines
VMSTOP_INTERRUPT-> "the VM is stopped by an interrupt".
VMSTOP_USER-> "the VM is stopped by the user".Makes a lot of sense, will change.
Quoted 8 lines
Quoted 8 lines
migration.c +0
-0@@ -378,7 +378,7 @@ void migrate_fd_put_ready(void *opaque)int old_vm_running = vm_running;
Quoted 8 lines
Quoted 5 lines
migration.c void migrate_fd_put_ready(void *opaque) · line 381
Quoted 3 lines
migration.c +1
-1DPRINTF ("done iterating\n" ); -vm_stop (0 ); +vm_stop (VMSTOP_RWSTATE );
VMSTOP_RWSTATEis cryptic. What aboutVMSTOP_SAVEVM, MIGRATE, etc.Both alternatives are not completely accurate as we also stop of vmload. What about
VMSTOP_SYNC_STATEorUPDATE_STATE?
IMO it would be better to state the specific reason why the vm stopped, not some generic action. VMSTOP_SAVEVM, VMSTOP_VMLOAD and VMSTOP_MIGRATE.
Re: [PATCH 04/15] Improve vm_stop reason declarations
Quoted 38 lines · Marcelo Tosatti
Quoted 37 lines · Jan Kiszka
Quoted 16 lines · Marcelo Tosatti
gdbstub.c static void gdb_vm_state_change(void *opaque, int running, int reason) · line 2199
Quoted 12 lines · Jan Kiszka
index d6556c9..3397566 100644
gdbstub.c +2
-2@@ -2194,14 +2194,14 @@ static void gdb_vm_state_change(void *opaque, int running, int reason)const char *type;int ret; -if (running || (reason !=EXCP_DEBUG && reason !=EXCP_INTERRUPT ) || - s->state ==RS_INACTIVE || s->state ==RS_SYSCALL ) +if (running || (reason !=VMSTOP_DEBUG && reason !=VMSTOP_INTERRUPT ) || + s->state ==RS_INACTIVE || s->state ==RS_SYSCALL ) {return ;What about
VMSTOP_USER?Quoted 3 lines
VMSTOP_INTERRUPT-> "the VM is stopped by an interrupt".
VMSTOP_USER-> "the VM is stopped by the user".Makes a lot of sense, will change.
Quoted 13 lines
migration.c void migrate_fd_put_ready(void *opaque) · line 381
Quoted 11 lines
migration.c +1
-1@@ -378,7 +378,7 @@ void migrate_fd_put_ready(void *opaque)int old_vm_running = vm_running;DPRINTF ("done iterating\n" ); -vm_stop (0 ); +vm_stop (VMSTOP_RWSTATE );
VMSTOP_RWSTATEis cryptic. What aboutVMSTOP_SAVEVM, MIGRATE, etc.
Quoted 7 lines
Quoted 2 lines
Both alternatives are not completely accurate as we also stop of vmload. What about
VMSTOP_SYNC_STATEorUPDATE_STATE?IMO it would be better to state the specific reason why the vm stopped, not some generic action.
VMSTOP_SAVEVM,VMSTOP_VMLOADandVMSTOP_MIGRATE.
Ah, OK. Can be done.
Jan
Signature · 2 lines
Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
PATCH v2 4/15 Improve vm_stop reason declarations
In reply to Jan KiszkaDefine and use dedicated constants for vm_stop reasons, they actually have nothing to do with the EXCP_* defines used so far. At this chance, specify more detailed reasons so that VM state change handlers can evaluate them.
---
Changes in v2:
- VMSTOP_INTERRUPT -> VMSTOP_USER
- differentiate between VMSTOP_SAVEVM/LOADVM/MIGRATE
cpus.c | 4 ++-- gdbstub.c | 19 ++++++++++--------- hw/ide/core.c | 2 +- hw/scsi-disk.c | 2 +- hw/virtio-blk.c | 2 +- hw/watchdog.c | 2 +- kvm-all.c | 2 +- migration.c | 2 +- monitor.c | 4 ++-- savevm.c | 4 ++-- sysemu.h | 10 ++++++++++ vl.c | 2 +- 12 files changed, 33 insertions(+), 22 deletions(-)
cpus.c +2 -2
@@ -168,8 +168,8 @@ static bool all_cpu_threads_idle(void)static void cpu_debug_handler (CPUState *env) {gdb_set_stop_cpu (env); - debug_requested =EXCP_DEBUG ; -vm_stop (EXCP_DEBUG ); + debug_requested =VMSTOP_DEBUG ; +vm_stop (VMSTOP_DEBUG ); }#ifdef CONFIG_LINUX
gdbstub.c +10 -9
@@ -2194,14 +2194,14 @@ static void gdb_vm_state_change(void *opaque, int running, int reason)const char *type;int ret; -if (running || (reason !=EXCP_DEBUG && reason !=EXCP_INTERRUPT ) || - s->state ==RS_INACTIVE || s->state ==RS_SYSCALL ) +if (running || (reason !=VMSTOP_DEBUG && reason !=VMSTOP_USER ) || + s->state ==RS_INACTIVE || s->state ==RS_SYSCALL ) {return ; - + }/* disable single step if it was enable */ cpu_single_step (env,0 ); -if (reason ==EXCP_DEBUG ) { +if (reason ==VMSTOP_DEBUG ) {if (env->watchpoint_hit) {switch (env->watchpoint_hit->flags &BP_MEM_ACCESS ) {case BP_MEM_READ :
@@ -2252,7 +2252,7 @@ void gdb_do_syscall(gdb_syscall_complete_cb cb, const char *fmt, ...) gdb_current_syscall_cb = cb; s->state =RS_SYSCALL ;#ifndef CONFIG_USER_ONLY -vm_stop (EXCP_DEBUG ); +vm_stop (VMSTOP_DEBUG );#endif s->state =RS_IDLE ;va_start (va, fmt);
@@ -2326,7 +2326,7 @@ static void gdb_read_byte(GDBState *s, int ch)if (vm_running) {/* when the CPU is running, we cannot do anything except stop it when receiving a char */ -vm_stop (EXCP_INTERRUPT ); +vm_stop (VMSTOP_USER ); } else#endif {
@@ -2588,7 +2588,7 @@ static void gdb_chr_event(void *opaque, int event) {switch (event) {case CHR_EVENT_OPENED : -vm_stop (EXCP_INTERRUPT ); +vm_stop (VMSTOP_USER ); gdb_has_xml =0 ;break ;default :
@@ -2628,8 +2628,9 @@ static int gdb_monitor_write(CharDriverState *chr, const uint8_t *buf, int len)#ifndef _WIN32static void gdb_sigterm_handler (int signal) { -if (vm_running) -vm_stop (EXCP_INTERRUPT ); +if (vm_running) { +vm_stop (VMSTOP_USER ); + } }#endif
hw/ide/core.c +1 -1
@@ -465,7 +465,7 @@ static int ide_handle_rw_error(IDEState *s, int error, int op) s->bus->dma->ops->set_unit (s->bus->dma, s->unit); s->bus->dma->ops->add_status (s->bus->dma, op);bdrv_mon_event (s->bs,BDRV_ACTION_STOP , is_read); -vm_stop (0 ); +vm_stop (VMSTOP_DISKFULL ); }else {if (op &BM_STATUS_DMA_RETRY ) {dma_buf_commit (s,0 );
hw/scsi-disk.c +1 -1
@@ -239,7 +239,7 @@ static int scsi_handle_rw_error(SCSIDiskReq *r, int error, int type) r->status |=SCSI_REQ_STATUS_RETRY | type;bdrv_mon_event (s->bs,BDRV_ACTION_STOP , is_read); -vm_stop (0 ); +vm_stop (VMSTOP_DISKFULL ); }else {if (type ==SCSI_REQ_STATUS_RETRY_READ ) { r->req.bus->complete (r->req.bus,SCSI_REASON_DATA , r->req.tag,0 );
hw/virtio-blk.c +1 -1
@@ -78,7 +78,7 @@ static int virtio_blk_handle_rw_error(VirtIOBlockReq *req, int error, req->next = s->rq; s->rq = req;bdrv_mon_event (s->bs,BDRV_ACTION_STOP , is_read); -vm_stop (0 ); +vm_stop (VMSTOP_DISKFULL ); }else {virtio_blk_req_complete (req,VIRTIO_BLK_S_IOERR );bdrv_mon_event (s->bs,BDRV_ACTION_REPORT , is_read);
hw/watchdog.c +1 -1
@@ -132,7 +132,7 @@ void watchdog_perform_action(void)case WDT_PAUSE :/* same as 'stop' command in monitor */ watchdog_mon_event ("pause" ); -vm_stop (0 ); +vm_stop (VMSTOP_WATCHDOG );break ;case WDT_DEBUG :
kvm-all.c +1 -1
@@ -994,7 +994,7 @@ int kvm_cpu_exec(CPUState *env)if (ret <0 ) {cpu_dump_state (env, stderr, fprintf,CPU_DUMP_CODE ); -vm_stop (0 ); +vm_stop (VMSTOP_PANIC ); env->exit_request =1 ; }if (env->exit_request) {
migration.c +1 -1
@@ -378,7 +378,7 @@ void migrate_fd_put_ready(void *opaque)int old_vm_running = vm_running;DPRINTF ("done iterating\n" ); -vm_stop (0 ); +vm_stop (VMSTOP_MIGRATE );if ((qemu_savevm_state_complete (s->mon, s->file)) <0 ) {if (old_vm_running) {
monitor.c +2 -2
@@ -1255,7 +1255,7 @@ static void do_singlestep(Monitor *mon, const QDict *qdict)*/ static int do_stop (Monitor *mon,const QDict *qdict,QObject **ret_data) { -vm_stop (EXCP_INTERRUPT ); +vm_stop (VMSTOP_USER );return 0 ; }
@@ -2783,7 +2783,7 @@ static void do_loadvm(Monitor *mon, const QDict *qdict)int saved_vm_running = vm_running;const char *name =qdict_get_str (qdict,"name" ); -vm_stop (0 ); +vm_stop (VMSTOP_LOADVM );if (load_vmstate (name) ==0 && saved_vm_running) {vm_start ();
savevm.c +2 -2
@@ -1575,7 +1575,7 @@ static int qemu_savevm_state(Monitor *mon, QEMUFile *f)int ret; saved_vm_running = vm_running; -vm_stop (0 ); +vm_stop (VMSTOP_SAVEVM );if (qemu_savevm_state_blocked (mon)) { ret = -EINVAL ;
@@ -1904,7 +1904,7 @@ void do_savevm(Monitor *mon, const QDict *qdict) } saved_vm_running = vm_running; -vm_stop (0 ); +vm_stop (VMSTOP_SAVEVM );memset (sn,0 ,sizeof (*sn));
sysemu.h +10 -0
@@ -37,6 +37,16 @@ VMChangeStateEntry *qemu_add_vm_change_state_handler(VMChangeStateHandler *cb, void *opaque);void qemu_del_vm_change_state_handler (VMChangeStateEntry *e); +#define VMSTOP_USER 0 +#define VMSTOP_DEBUG 1 +#define VMSTOP_SHUTDOWN 2 +#define VMSTOP_DISKFULL 3 +#define VMSTOP_WATCHDOG 4 +#define VMSTOP_PANIC 5 +#define VMSTOP_SAVEVM 6 +#define VMSTOP_LOADVM 7 +#define VMSTOP_MIGRATE 8 +void vm_start (void );void vm_stop (int reason);
vl.c +1 -1
@@ -1433,7 +1433,7 @@ static void main_loop(void)if (qemu_shutdown_requested ()) {monitor_protocol_event (QEVENT_SHUTDOWN ,NULL );if (no_shutdown) { -vm_stop (0 ); +vm_stop (VMSTOP_SHUTDOWN ); no_shutdown =0 ; }else break ;
Signature · 1 line
1.7.1
PATCH 5/15 Refactor debug and vmstop request interface
In reply to Jan KiszkaInstead of fiddling with debug_requested and vmstop_requested directly, introduce qemu_system_debug_request and turn qemu_system_vmstop_request into a public interface. This aligns those services with exiting ones in vl.c.
--- cpus.c | 9 +-------- cpus.h | 2 -- sysemu.h | 2 ++ vl.c | 20 ++++++++++++++++---- 4 files changed, 19 insertions(+), 14 deletions(-)
cpus.c +1 -8
@@ -168,8 +168,7 @@ static bool all_cpus_idle(void)static void cpu_debug_handler (CPUState *env) {gdb_set_stop_cpu (env); - debug_requested =VMSTOP_DEBUG ; -vm_stop (VMSTOP_DEBUG ); +qemu_system_debug_request (); }#ifdef CONFIG_LINUX
@@ -990,12 +989,6 @@ void qemu_notify_event(void)qemu_event_increment (); } -static void qemu_system_vmstop_request (int reason) -{ - vmstop_requested = reason; -qemu_notify_event (); -} -void cpu_stop_current (void ) {if (cpu_single_env) {
cpus.h +0 -2
@@ -11,8 +11,6 @@ void cpu_stop_current(void);/* vl.c */ extern int smp_cores;extern int smp_threads; -extern int debug_requested; -extern int vmstop_requested;void vm_state_notify (int running,int reason);bool cpu_exec_all (void );void set_numa_modes (void );
sysemu.h +2 -0
@@ -59,6 +59,8 @@ void cpu_disable_ticks(void);void qemu_system_reset_request (void );void qemu_system_shutdown_request (void );void qemu_system_powerdown_request (void ); +void qemu_system_debug_request (void ); +void qemu_system_vmstop_request (int reason);int qemu_shutdown_requested (void );int qemu_reset_requested (void );int qemu_powerdown_requested (void );
vl.c +16 -4
@@ -1217,8 +1217,8 @@ static QTAILQ_HEAD(reset_handlers, QEMUResetEntry) reset_handlers =static int reset_requested;static int shutdown_requested;static int powerdown_requested; -int debug_requested; -int vmstop_requested; +static int debug_requested; +static int vmstop_requested;int qemu_shutdown_requested (void ) {
@@ -1312,6 +1312,18 @@ void qemu_system_powerdown_request(void)qemu_notify_event (); } +void qemu_system_debug_request (void ) +{ + debug_requested =1 ; +vm_stop (VMSTOP_DEBUG ); +} + +void qemu_system_vmstop_request (int reason) +{ + vmstop_requested = reason; +qemu_notify_event (); +} +void main_loop_wait (int nonblocking) {IOHandlerRecord *ioh;
@@ -1427,8 +1439,8 @@ static void main_loop(void) dev_time +=profile_getclock () - ti;#endif -if ((r =qemu_debug_requested ())) { -vm_stop (r); +if (qemu_debug_requested ()) { +vm_stop (VMSTOP_DEBUG ); }if (qemu_shutdown_requested ()) {monitor_protocol_event (QEVENT_SHUTDOWN ,NULL );
Signature · 1 line
1.7.1
PATCH 6/15 Move debug exception handling out of cpu_exec
In reply to Jan KiszkaTo prepare splitting up KVM and TCG CPU entry/exit, move the debug exception into cpus.c and invoke cpu_handle_debug_exception on return from qemu_cpu_exec.
This also allows to clean up the debug request signaling: We can assign the job of informing main-loop to qemu_system_debug_request and stop the calling cpu directly in cpu_handle_debug_exception. That means a debug stop will now only be signaled via debug_requested and not additionally via vmstop_requested.
--- cpu-exec.c | 24 ------------------------ cpus.c | 35 ++++++++++++++++++++++++++++++----- vl.c | 2 +- 3 files changed, 31 insertions(+), 30 deletions(-)
cpu-exec.c +0 -24
@@ -196,28 +196,6 @@ static inline TranslationBlock *tb_find_fast(void)return tb; } -static CPUDebugExcpHandler *debug_excp_handler; - -CPUDebugExcpHandler *cpu_set_debug_excp_handler (CPUDebugExcpHandler *handler) -{ -CPUDebugExcpHandler *old_handler = debug_excp_handler; - - debug_excp_handler = handler; -return old_handler; -} - -static void cpu_handle_debug_exception (CPUState *env) -{ -CPUWatchpoint *wp; - -if (!env->watchpoint_hit) -QTAILQ_FOREACH (wp, &env->watchpoints, entry) - wp->flags &= ~BP_WATCHPOINT_HIT; - -if (debug_excp_handler) -debug_excp_handler (env); -} -/* main execution loop */ volatile sig_atomic_t exit_request;
@@ -287,8 +265,6 @@ int cpu_exec(CPUState *env1)if (env->exception_index >=EXCP_INTERRUPT ) {/* exit request from the cpu execution loop */ ret = env->exception_index; -if (ret ==EXCP_DEBUG ) -cpu_handle_debug_exception (env);break ; }else {#if defined(CONFIG_USER_ONLY )
cpus.c +30 -5
@@ -165,10 +165,34 @@ static bool all_cpus_idle(void)return true; } -static void cpu_debug_handler (CPUState *env) +static CPUDebugExcpHandler *debug_excp_handler; + +CPUDebugExcpHandler *cpu_set_debug_excp_handler (CPUDebugExcpHandler *handler) +{ +CPUDebugExcpHandler *old_handler = debug_excp_handler; + + debug_excp_handler = handler; +return old_handler; +} + +static void cpu_handle_debug_exception (CPUState *env) { +CPUWatchpoint *wp; + +if (!env->watchpoint_hit) { + QTAILQ_FOREACH(wp, &env->watchpoints, entry) { + wp->flags &= ~BP_WATCHPOINT_HIT ; + } + } +if (debug_excp_handler) { +debug_excp_handler (env); + } +gdb_set_stop_cpu (env);qemu_system_debug_request (); +#ifdef CONFIG_IOTHREAD + env->stopped =1 ; +#endif }#ifdef CONFIG_LINUX
@@ -479,7 +503,6 @@ int qemu_init_main_loop(void) return ret; }#endif -cpu_set_debug_excp_handler (cpu_debug_handler);qemu_init_sigbus ();
@@ -653,8 +676,6 @@ int qemu_init_main_loop(void)int ret;sigset_t blocked_signals; -cpu_set_debug_excp_handler (cpu_debug_handler); -qemu_init_sigbus (); blocked_signals =block_io_signals ();
@@ -808,7 +829,10 @@ static void *qemu_kvm_cpu_thread_fn(void *arg)while (1 ) {if (cpu_can_run (env)) { -qemu_cpu_exec (env); + r =qemu_cpu_exec (env); +if (r ==EXCP_DEBUG ) { +cpu_handle_debug_exception (env); + } }qemu_kvm_wait_io_event (env); }
@@ -1076,6 +1100,7 @@ bool cpu_exec_all(void)qemu_kvm_eat_signals (env); }if (r ==EXCP_DEBUG ) { +cpu_handle_debug_exception (env);break ; } }else if (env->stop) {
vl.c +1 -1
@@ -1315,7 +1315,7 @@ void qemu_system_powerdown_request(void)void qemu_system_debug_request (void ) { debug_requested =1 ; -vm_stop (VMSTOP_DEBUG ); +qemu_notify_event (); }void qemu_system_vmstop_request (int reason)
Signature · 1 line
1.7.1
PATCH 7/15 kvm: Separate TCG from KVM cpu execution
In reply to Jan KiszkaMixing up TCG bits with KVM already led to problems around eflags emulation on x86. Moreover, quite some code that TCG requires on cpu enty/exit is useless for KVM. So dispatch between tcg_cpu_exec and kvm_cpu_exec as early as possible.
The core logic of cpu_halted from cpu_exec is added to kvm_arch_process_irqchip_events. Moving away from cpu_exec makes exception_index meaningless for KVM, we can simply pass the exit reason directly (only "EXCP_DEBUG vs. rest" is relevant).
--- cpu-exec.c | 19 ++++++------------- cpus.c | 10 +++++----- kvm-all.c | 19 +++++++++---------- target-i386/kvm.c | 6 +++--- 4 files changed, 23 insertions(+), 31 deletions(-)
cpu-exec.c +6 -13
@@ -226,13 +226,11 @@ int cpu_exec(CPUState *env1) }#if defined(TARGET_I386 ) -if (!kvm_enabled ()) { -/* put eflags in CPU temporary format */ -CC_SRC = env->eflags & (CC_O |CC_S |CC_Z |CC_A |CC_P |CC_C ); -DF =1 - (2 * ((env->eflags >>10 ) &1 )); -CC_OP =CC_OP_EFLAGS ; - env->eflags &= ~(DF_MASK |CC_O |CC_S |CC_Z |CC_A |CC_P |CC_C ); - } +/* put eflags in CPU temporary format */ +CC_SRC = env->eflags & (CC_O |CC_S |CC_Z |CC_A |CC_P |CC_C ); +DF =1 - (2 * ((env->eflags >>10 ) &1 )); +CC_OP =CC_OP_EFLAGS ; + env->eflags &= ~(DF_MASK |CC_O |CC_S |CC_Z |CC_A |CC_P |CC_C );#elif defined(TARGET_SPARC )#elif defined(TARGET_M68K ) env->cc_op =CC_OP_FLAGS ;
@@ -257,7 +255,7 @@ int cpu_exec(CPUState *env1)if (setjmp (env->jmp_env) ==0 ) {#if defined(__sparc__) && !defined(CONFIG_SOLARIS )#undef env - env = cpu_single_env; + env = cpu_single_env;#define env cpu_single_env#endif /* if an exception is pending, we execute it here */
@@ -316,11 +314,6 @@ int cpu_exec(CPUState *env1) } } -if (kvm_enabled ()) { -kvm_cpu_exec (env); -longjmp (env->jmp_env,1 ); - } - next_tb =0 ;/* force lookup of first TB */ for (;;) { interrupt_request = env->interrupt_request;
cpus.c +5 -5
@@ -800,8 +800,6 @@ static void qemu_kvm_wait_io_event(CPUState *env)qemu_wait_io_event_common (env); } -static int qemu_cpu_exec (CPUState *env); -static void *qemu_kvm_cpu_thread_fn (void *arg) {CPUState *env = arg;
@@ -829,7 +827,7 @@ static void *qemu_kvm_cpu_thread_fn(void *arg)while (1 ) {if (cpu_can_run (env)) { - r =qemu_cpu_exec (env); + r =kvm_cpu_exec (env);if (r ==EXCP_DEBUG ) {cpu_handle_debug_exception (env); }
@@ -1040,7 +1038,7 @@ void vm_stop(int reason)#endif -static int qemu_cpu_exec (CPUState *env) +static int tcg_cpu_exec (CPUState *env) {int ret;#ifdef CONFIG_PROFILER
@@ -1095,9 +1093,11 @@ bool cpu_exec_all(void)break ; }if (cpu_can_run (env)) { - r =qemu_cpu_exec (env);if (kvm_enabled ()) { + r =kvm_cpu_exec (env);qemu_kvm_eat_signals (env); + }else { + r =tcg_cpu_exec (env); }if (r ==EXCP_DEBUG ) {cpu_handle_debug_exception (env);
kvm-all.c +9 -10
@@ -895,10 +895,11 @@ int kvm_cpu_exec(CPUState *env)if (kvm_arch_process_irqchip_events (env)) { env->exit_request =0 ; - env->exception_index =EXCP_HLT ; -return 0 ; +return EXCP_HLT ; } + cpu_single_env = env; + do { if (env->kvm_vcpu_dirty) { kvm_arch_put_registers(env, KVM_PUT_RUNTIME_STATE);
@@ -927,7 +928,6 @@ int kvm_cpu_exec(CPUState *env)kvm_flush_coalesced_mmio_buffer ();if (ret == -EINTR || ret == -EAGAIN ) { -cpu_exit (env);DPRINTF ("io window exit\n" ); ret =0 ;break ;
@@ -978,8 +978,8 @@ int kvm_cpu_exec(CPUState *env)DPRINTF ("kvm_exit_debug\n" );#ifdef KVM_CAP_SET_GUEST_DEBUG if (kvm_arch_debug (&run->debug.arch)) { - env->exception_index =EXCP_DEBUG ; -return 0 ; + ret =EXCP_DEBUG ; + goto out; }/* re-enter, this exception was guest-internal */ ret =1 ;
@@ -995,13 +995,12 @@ int kvm_cpu_exec(CPUState *env)if (ret <0 ) {cpu_dump_state (env, stderr, fprintf,CPU_DUMP_CODE );vm_stop (VMSTOP_PANIC ); - env->exit_request =1 ; - } -if (env->exit_request) { - env->exit_request =0 ; - env->exception_index =EXCP_INTERRUPT ; } + ret =EXCP_INTERRUPT ; +out: + env->exit_request =0 ; + cpu_single_env =NULL ;return ret; }
target-i386/kvm.c +3 -3
@@ -1502,12 +1502,13 @@ int kvm_arch_post_run(CPUState *env, struct kvm_run *run)int kvm_arch_process_irqchip_events (CPUState *env) { +if (env->interrupt_request & (CPU_INTERRUPT_HARD |CPU_INTERRUPT_NMI )) { + env->halted =0 ; + }if (env->interrupt_request &CPU_INTERRUPT_INIT ) {kvm_cpu_synchronize_state (env);do_cpu_init (env); - env->exception_index =EXCP_HALTED ; } -if (env->interrupt_request &CPU_INTERRUPT_SIPI ) {kvm_cpu_synchronize_state (env);do_cpu_sipi (env);
@@ -1522,7 +1523,6 @@ static int kvm_handle_halt(CPUState *env) (env->eflags & IF_MASK)) && !(env->interrupt_request & CPU_INTERRUPT_NMI)) { env->halted =1 ; - env->exception_index =EXCP_HLT ;return 0 ; }
Signature · 1 line
1.7.1
Re: [PATCH 07/15] kvm: Separate TCG from KVM cpu execution
Quoted 23 lines · Jan Kiszka
Mixing up TCG bits with KVM already led to problems around eflags emulation on x86. Moreover, quite some code that TCG requires on cpu enty/exit is useless for KVM. So dispatch between
tcg_cpu_execandkvm_cpu_execas early as possible.The core logic of
cpu_haltedfromcpu_execis added tokvm_arch_process_irqchip_events. Moving away fromcpu_execmakesexception_indexmeaningless for KVM, we can simply pass the exit reason directly (only "EXCP_DEBUGvs. rest" is relevant).Signed-off-by Jan Kiszka <[email protected]>--- cpu-exec.c | 19 ++++++------------- cpus.c | 10 +++++----- kvm-all.c | 19 +++++++++---------- target-i386/kvm.c | 6 +++--- 4 files changed, 23 insertions(+), 31 deletions(-)target-i386/kvm.c +0
-0
target-i386/kvm.c int kvm_arch_post_run(CPUState *env, struct kvm_run *run) · line 1507
Quoted 6 lines
target-i386/kvm.c +3
-0@@ -1502,12 +1502,13 @@ int kvm_arch_post_run(CPUState *env, struct kvm_run *run)int kvm_arch_process_irqchip_events (CPUState *env) { +if (env->interrupt_request & (CPU_INTERRUPT_HARD |CPU_INTERRUPT_NMI )) { + env->halted =0 ; + }
Why is it necessary to clear env->halted here?
Re: [PATCH 07/15] kvm: Separate TCG from KVM cpu execution
Quoted 25 lines · Marcelo Tosatti
Quoted 24 lines · Jan Kiszka
Mixing up TCG bits with KVM already led to problems around eflags emulation on x86. Moreover, quite some code that TCG requires on cpu enty/exit is useless for KVM. So dispatch between
tcg_cpu_execandkvm_cpu_execas early as possible.The core logic of
cpu_haltedfromcpu_execis added tokvm_arch_process_irqchip_events. Moving away fromcpu_execmakesexception_indexmeaningless for KVM, we can simply pass the exit reason directly (only "EXCP_DEBUGvs. rest" is relevant).Signed-off-by Jan Kiszka <[email protected]>--- cpu-exec.c | 19 ++++++------------- cpus.c | 10 +++++----- kvm-all.c | 19 +++++++++---------- target-i386/kvm.c | 6 +++--- 4 files changed, 23 insertions(+), 31 deletions(-)target-i386/kvm.c +0
-0@@ -1502,12 +1502,13 @@ int kvm_arch_post_run(CPUState *env, struct kvm_run *run)
Quoted 7 lines
target-i386/kvm.c int kvm_arch_post_run(CPUState *env, struct kvm_run *run) · line 1507
Quoted 5 lines
target-i386/kvm.c +3
-0int kvm_arch_process_irqchip_events (CPUState *env) { +if (env->interrupt_request & (CPU_INTERRUPT_HARD |CPU_INTERRUPT_NMI )) { + env->halted =0 ; + }Why is it necessary to clear
env->haltedhere?
Because we no longer come along cpu_halted() in cpu_exec(). This corresponds to the tail of process_irqchip_events() in qemu-kvm
Jan
Re: [PATCH 07/15] kvm: Separate TCG from KVM cpu execution
Quoted 34 lines · Jan Kiszka
Quoted 25 lines · Marcelo Tosatti
Quoted 24 lines · Jan Kiszka
Mixing up TCG bits with KVM already led to problems around eflags emulation on x86. Moreover, quite some code that TCG requires on cpu enty/exit is useless for KVM. So dispatch between
tcg_cpu_execandkvm_cpu_execas early as possible.The core logic of
cpu_haltedfromcpu_execis added tokvm_arch_process_irqchip_events. Moving away fromcpu_execmakesexception_indexmeaningless for KVM, we can simply pass the exit reason directly (only "EXCP_DEBUGvs. rest" is relevant).Signed-off-by Jan Kiszka <[email protected]>--- cpu-exec.c | 19 ++++++------------- cpus.c | 10 +++++----- kvm-all.c | 19 +++++++++---------- target-i386/kvm.c | 6 +++--- 4 files changed, 23 insertions(+), 31 deletions(-)target-i386/kvm.c +0
-0@@ -1502,12 +1502,13 @@ int kvm_arch_post_run(CPUState *env, struct kvm_run *run)Quoted 7 lines
target-i386/kvm.c int kvm_arch_post_run(CPUState *env, struct kvm_run *run) · line 1507
Quoted 5 lines
target-i386/kvm.c +3
-0int kvm_arch_process_irqchip_events (CPUState *env) { +if (env->interrupt_request & (CPU_INTERRUPT_HARD |CPU_INTERRUPT_NMI )) { + env->halted =0 ; + }Why is it necessary to clear
env->haltedhere?
Quoted 2 lines
Because we no longer come along
cpu_halted()incpu_exec(). This corresponds to the tail ofprocess_irqchip_events()in qemu-kvm
This is not yet well integrated, we probably don't need env->halted anymore (see cpu_has_work). Can be improved later though.
Re: [PATCH 07/15] kvm: Separate TCG from KVM cpu execution
Quoted 35 lines · Marcelo Tosatti
Quoted 34 lines · Jan Kiszka
Quoted 32 lines · Marcelo Tosatti
Quoted 23 lines · Jan Kiszka
Mixing up TCG bits with KVM already led to problems around eflags emulation on x86. Moreover, quite some code that TCG requires on cpu enty/exit is useless for KVM. So dispatch between
tcg_cpu_execandkvm_cpu_execas early as possible.The core logic of
cpu_haltedfromcpu_execis added tokvm_arch_process_irqchip_events. Moving away fromcpu_execmakesexception_indexmeaningless for KVM, we can simply pass the exit reason directly (only "EXCP_DEBUGvs. rest" is relevant).Signed-off-by Jan Kiszka <[email protected]>--- cpu-exec.c | 19 ++++++------------- cpus.c | 10 +++++----- kvm-all.c | 19 +++++++++---------- target-i386/kvm.c | 6 +++--- 4 files changed, 23 insertions(+), 31 deletions(-)target-i386/kvm.c +0
-0target-i386/kvm.c int kvm_arch_post_run(CPUState *env, struct kvm_run *run) · line 1507
Quoted 6 lines
target-i386/kvm.c +3
-0@@ -1502,12 +1502,13 @@ int kvm_arch_post_run(CPUState *env, struct kvm_run *run)int kvm_arch_process_irqchip_events (CPUState *env) { +if (env->interrupt_request & (CPU_INTERRUPT_HARD |CPU_INTERRUPT_NMI )) { + env->halted =0 ; + }Why is it necessary to clear
env->haltedhere?
Quoted 5 lines
Quoted 2 lines
Because we no longer come along
cpu_halted()incpu_exec(). This corresponds to the tail ofprocess_irqchip_events()in qemu-kvmThis is not yet well integrated, we probably don't need
env->haltedanymore (seecpu_has_work). Can be improved later though.
So far we check for it, at least in cpu_[thread_]is_idle. And that's a generic service.
Jan
Signature · 2 lines
Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
PATCH 8/15 kvm: x86: Prepare VCPU loop for in-kernel irqchip
In reply to Jan KiszkaEffectively no functional change yet as kvm_irqchip_in_kernel still only returns 0, but this patch will allow qemu-kvm to adopt the VCPU loop of upsteam KVM.
--- target-i386/kvm.c | 69 +++++++++++++++++++++++++++++----------------------- 1 files changed, 38 insertions(+), 31 deletions(-)
target-i386/kvm.c +38 -31
@@ -1442,11 +1442,6 @@ int kvm_arch_get_registers(CPUState *env)int kvm_arch_pre_run (CPUState *env,struct kvm_run *run) { -/* Force the VCPU out of its inner loop to process the INIT request */ -if (env->interrupt_request &CPU_INTERRUPT_INIT ) { - env->exit_request =1 ; - } -/* Inject NMI */ if (env->interrupt_request &CPU_INTERRUPT_NMI ) { env->interrupt_request &= ~CPU_INTERRUPT_NMI ;
@@ -1454,35 +1449,43 @@ int kvm_arch_pre_run(CPUState *env, struct kvm_run *run)kvm_vcpu_ioctl (env,KVM_NMI ); } -/* Try to inject an interrupt if the guest can accept it */ -if (run->ready_for_interrupt_injection && - (env->interrupt_request &CPU_INTERRUPT_HARD ) && - (env->eflags &IF_MASK )) { -int irq; - - env->interrupt_request &= ~CPU_INTERRUPT_HARD ; - irq =cpu_get_pic_interrupt (env); -if (irq >=0 ) { -struct kvm_interrupt intr; - intr.irq = irq; -/* FIXME: errors */ -DPRINTF ("injected interrupt %d\n" , irq); -kvm_vcpu_ioctl (env,KVM_INTERRUPT , &intr); +if (!kvm_irqchip_in_kernel ()) { +/* Force the VCPU out of its inner loop to process the INIT request */ +if (env->interrupt_request &CPU_INTERRUPT_INIT ) { + env->exit_request =1 ; } - } -/* If we have an interrupt but the guest is not ready to receive an -* interrupt, request an interrupt window exit. This will -* cause a return to userspace as soon as the guest is ready to -* receive interrupts. */ -if ((env->interrupt_request &CPU_INTERRUPT_HARD )) { - run->request_interrupt_window =1 ; - }else { - run->request_interrupt_window =0 ; - } +/* Try to inject an interrupt if the guest can accept it */ +if (run->ready_for_interrupt_injection && + (env->interrupt_request &CPU_INTERRUPT_HARD ) && + (env->eflags &IF_MASK )) { +int irq; + + env->interrupt_request &= ~CPU_INTERRUPT_HARD ; + irq =cpu_get_pic_interrupt (env); +if (irq >=0 ) { +struct kvm_interrupt intr; + + intr.irq = irq; +/* FIXME: errors */ +DPRINTF ("injected interrupt %d\n" , irq); +kvm_vcpu_ioctl (env,KVM_INTERRUPT , &intr); + } + } -DPRINTF ("setting tpr\n" ); - run->cr8 =cpu_get_apic_tpr (env->apic_state); +/* If we have an interrupt but the guest is not ready to receive an +* interrupt, request an interrupt window exit. This will +* cause a return to userspace as soon as the guest is ready to +* receive interrupts. */ +if ((env->interrupt_request &CPU_INTERRUPT_HARD )) { + run->request_interrupt_window =1 ; + }else { + run->request_interrupt_window =0 ; + } + +DPRINTF ("setting tpr\n" ); + run->cr8 =cpu_get_apic_tpr (env->apic_state); + }return 0 ; }
@@ -1502,6 +1505,10 @@ int kvm_arch_post_run(CPUState *env, struct kvm_run *run)int kvm_arch_process_irqchip_events (CPUState *env) { +if (kvm_irqchip_in_kernel ()) { +return 0 ; + } +if (env->interrupt_request & (CPU_INTERRUPT_HARD |CPU_INTERRUPT_NMI )) { env->halted =0 ; }
Signature · 1 line
1.7.1
PATCH 9/15 kvm: Drop return values from kvm_arch_pre/post_run
In reply to Jan KiszkaWe do not check them, and the only arch with non-empty implementations always returns 0 (this is also true for qemu-kvm).
--- kvm.h | 5 ++--- target-i386/kvm.c | 8 ++------ target-ppc/kvm.c | 6 ++---- target-s390x/kvm.c | 6 ++---- 4 files changed, 8 insertions(+), 17 deletions(-)
kvm.h +2 -3
@@ -99,12 +99,11 @@ int kvm_vcpu_ioctl(CPUState *env, int type, ...);extern const KVMCapabilityInfo kvm_arch_required_capabilities[]; -int kvm_arch_post_run (CPUState *env,struct kvm_run *run); +void kvm_arch_pre_run (CPUState *env,struct kvm_run *run); +void kvm_arch_post_run (CPUState *env,struct kvm_run *run);int kvm_arch_handle_exit (CPUState *env,struct kvm_run *run); -int kvm_arch_pre_run (CPUState *env,struct kvm_run *run); -int kvm_arch_process_irqchip_events (CPUState *env);int kvm_arch_get_registers (CPUState *env);
target-i386/kvm.c +2 -6
@@ -1440,7 +1440,7 @@ int kvm_arch_get_registers(CPUState *env)return 0 ; } -int kvm_arch_pre_run (CPUState *env,struct kvm_run *run) +void kvm_arch_pre_run (CPUState *env,struct kvm_run *run) {/* Inject NMI */ if (env->interrupt_request &CPU_INTERRUPT_NMI ) {
@@ -1486,11 +1486,9 @@ int kvm_arch_pre_run(CPUState *env, struct kvm_run *run)DPRINTF ("setting tpr\n" ); run->cr8 =cpu_get_apic_tpr (env->apic_state); } - -return 0 ; } -int kvm_arch_post_run (CPUState *env,struct kvm_run *run) +void kvm_arch_post_run (CPUState *env,struct kvm_run *run) {if (run->if_flag) { env->eflags |=IF_MASK ;
@@ -1499,8 +1497,6 @@ int kvm_arch_post_run(CPUState *env, struct kvm_run *run) }cpu_set_apic_tpr (env->apic_state, run->cr8);cpu_set_apic_base (env->apic_state, run->apic_base); - -return 0 ; }int kvm_arch_process_irqchip_events (CPUState *env)
target-ppc/kvm.c +2 -4
@@ -256,14 +256,12 @@ int kvm_arch_pre_run(CPUState *env, struct kvm_run *run)return 0 ; } -int kvm_arch_post_run (CPUState *env,struct kvm_run *run) +void kvm_arch_post_run (CPUState *env,struct kvm_run *run) { -return 0 ; } -int kvm_arch_process_irqchip_events (CPUState *env) +void kvm_arch_process_irqchip_events (CPUState *env) { -return 0 ; }static int kvmppc_handle_halt (CPUState *env)
target-s390x/kvm.c +2 -4
@@ -169,14 +169,12 @@ int kvm_arch_remove_sw_breakpoint(CPUState *env, struct kvm_sw_breakpoint *bp)return 0 ; } -int kvm_arch_pre_run (CPUState *env,struct kvm_run *run) +void kvm_arch_pre_run (CPUState *env,struct kvm_run *run) { -return 0 ; } -int kvm_arch_post_run (CPUState *env,struct kvm_run *run) +void kvm_arch_post_run (CPUState *env,struct kvm_run *run) { -return 0 ; }int kvm_arch_process_irqchip_events (CPUState *env)
Signature · 1 line
1.7.1
Re: [PATCH 09/15] kvm: Drop return values from kvm_arch_pre/post_run
Quoted 5 lines · Jan Kiszka
We do not check them, and the only arch with non-empty implementations always returns 0 (this is also true for qemu-kvm).
Signed-off-by Jan Kiszka <[email protected]>CC Alexander Graf <[email protected]>
Yeah, if there's anything going wrong we should do cpu_abort anyways.
Alex
PATCH 1/1 kvm: ppc: Fix breakage of kvm_arch_pre_run/process_irqchip_events
In reply to Jan KiszkaQuoted 12 lines · Jan Kiszka
We do not check them, and the only arch with non-empty implementations always returns 0 (this is also true for qemu-kvm).
Signed-off-by Jan Kiszka <[email protected]>CC Alexander Graf <[email protected]>--- kvm.h | 5 ++--- target-i386/kvm.c | 8 ++------ target-ppc/kvm.c | 6 ++---- target-s390x/kvm.c | 6 ++---- 4 files changed, 8 insertions(+), 17 deletions(-)
...
Quoted 9 lines
target-ppc/kvm.c +0
-1@@ -256,14 +256,12 @@ int kvm_arch_pre_run(CPUState *env, struct kvm_run *run)return 0 ; } -int kvm_arch_post_run (CPUState *env,struct kvm_run *run)
target-ppc/kvm.c int kvm_arch_pre_run(CPUState *env, struct kvm_run *run) · line 265
Quoted 10 lines
target-ppc/kvm.c +2
-3+void kvm_arch_post_run (CPUState *env,struct kvm_run *run) { -return 0 ; } -int kvm_arch_process_irqchip_events (CPUState *env) +void kvm_arch_process_irqchip_events (CPUState *env) { -return 0 ; }
Oops. Do we already have a built-bot for KVM-enabled PPC (and s390) targets somewhere?
Jan
-------8<----------
From: Jan Kiszka <[email protected]>
Commit 7a39fe5882 failed to convert the right arch function.
--- target-ppc/kvm.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)
target-ppc/kvm.c +3 -3
@@ -222,7 +222,7 @@ int kvmppc_set_interrupt(CPUState *env, int irq, int level)#define PPC_INPUT_INT PPC6xx_INPUT_INT#endif -int kvm_arch_pre_run (CPUState *env,struct kvm_run *run) +void kvm_arch_pre_run (CPUState *env,struct kvm_run *run) {int r;unsigned irq;
@@ -253,15 +253,15 @@ int kvm_arch_pre_run(CPUState *env, struct kvm_run *run)/* We don't know if there are more interrupts pending after this. However, * the guest will return to userspace in the course of handling this one * anyways, so we will get a chance to deliver the rest. */ -return 0 ; }void kvm_arch_post_run (CPUState *env,struct kvm_run *run) { } -void kvm_arch_process_irqchip_events (CPUState *env) +int kvm_arch_process_irqchip_events (CPUState *env) { +return 0 ; }static int kvmppc_handle_halt (CPUState *env)
Re: [PATCH] kvm: ppc: Fix breakage of kvm_arch_pre_run/process_irqchip_events
Quoted 30 lines · Jan Kiszka
Quoted 12 lines · Jan Kiszka
We do not check them, and the only arch with non-empty implementations always returns 0 (this is also true for qemu-kvm).
Signed-off-by Jan Kiszka <[email protected]>CC Alexander Graf <[email protected]>---
kvm.h | 5 ++--- target-i386/kvm.c | 8 ++------ target-ppc/kvm.c | 6 ++---- target-s390x/kvm.c | 6 ++---- 4 files changed, 8 insertions(+), 17 deletions(-)...
Quoted 14 lines
target-ppc/kvm.c +1
-2@@ -256,14 +256,12 @@ int kvm_arch_pre_run(CPUState *env, struct kvm_run *run)return 0 ; } -int kvm_arch_post_run (CPUState *env,struct kvm_run *run) +void kvm_arch_post_run (CPUState *env,struct kvm_run *run) { -return 0 ; }
Quoted 8 lines
target-ppc/kvm.c int kvm_arch_pre_run(CPUState *env, struct kvm_run *run) · line 265
Quoted 5 lines
target-ppc/kvm.c +1
-2-int kvm_arch_process_irqchip_events (CPUState *env) +void kvm_arch_process_irqchip_events (CPUState *env) { -return 0 ; }Oops. Do we already have a built-bot for KVM-enabled PPC (and s390) targets somewhere?
Just before leaving for vacation I prepared a machine for each and gave stefan access to them. Looks like they're not officially running though - will try to look at this asap.
Alex
Re: [PATCH] kvm: ppc: Fix breakage of kvm_arch_pre_run/process_irqchip_events
Quoted 42 lines · Alexander Graf
Quoted 30 lines · Jan Kiszka
Quoted 12 lines · Jan Kiszka
We do not check them, and the only arch with non-empty implementations always returns 0 (this is also true for qemu-kvm).
Signed-off-by Jan Kiszka <[email protected]>CC Alexander Graf <[email protected]>---
kvm.h| 5 ++---target-i386/kvm.c| 8 ++------target-ppc/kvm.c| 6 ++----target-s390x/kvm.c| 6 ++----4 files changed, 8 insertions(+), 17 deletions(-)...
Quoted 14 lines
target-ppc/kvm.c +1
-2@@ -256,14 +256,12 @@ int kvm_arch_pre_run(CPUState *env, struct kvm_run *run) return 0; } -int kvm_arch_post_run (CPUState *env,struct kvm_run *run) +void kvm_arch_post_run (CPUState *env,struct kvm_run *run) { - return 0; }Quoted 8 lines
target-ppc/kvm.c int kvm_arch_pre_run(CPUState *env, struct kvm_run *run) · line 265
Quoted 5 lines
target-ppc/kvm.c +1
-2-int kvm_arch_process_irqchip_events (CPUState *env) +void kvm_arch_process_irqchip_events (CPUState *env) { - return 0; }Oops. Do we already have a built-bot for KVM-enabled PPC (and s390) targets somewhere?
Quoted 1 line
Just before leaving for vacation I prepared a machine for each and gave stefan access to them. Looks like they're not officially running though - will try to look at this asap.
They are in the process of being added to the buildbot by Daniel Gollub. However, the ppc box is unable to build qemu.git because it hits ENOMEM while compiling. I doubled swap size but that didn't fix the issue so I need to investigate more. At least s390 should be good to go soon and I will send an update when it is up and running.
Stefan
Re: [PATCH] kvm: ppc: Fix breakage of kvm_arch_pre_run/process_irqchip_events
Quoted 43 lines · Stefan Hajnoczi
Quoted 42 lines · Alexander Graf
Quoted 38 lines · Jan Kiszka
Quoted 12 lines · Jan Kiszka
We do not check them, and the only arch with non-empty implementations always returns 0 (this is also true for qemu-kvm).
Signed-off-by Jan Kiszka <[email protected]>CC Alexander Graf <[email protected]>---
kvm.h | 5 ++--- target-i386/kvm.c | 8 ++------ target-ppc/kvm.c | 6 ++---- target-s390x/kvm.c | 6 ++---- 4 files changed, 8 insertions(+), 17 deletions(-)...
Quoted 9 lines
target-ppc/kvm.c +0
-1@@ -256,14 +256,12 @@ int kvm_arch_pre_run(CPUState *env, struct kvm_run *run)return 0 ; } -int kvm_arch_post_run (CPUState *env,struct kvm_run *run)target-ppc/kvm.c int kvm_arch_pre_run(CPUState *env, struct kvm_run *run) · line 265
Quoted 10 lines
target-ppc/kvm.c +2
-3+void kvm_arch_post_run (CPUState *env,struct kvm_run *run) { -return 0 ; } -int kvm_arch_process_irqchip_events (CPUState *env) +void kvm_arch_process_irqchip_events (CPUState *env) { -return 0 ; }Oops. Do we already have a built-bot for KVM-enabled PPC (and s390) targets somewhere?
Quoted 6 lines
Quoted 1 line
Just before leaving for vacation I prepared a machine for each and gave stefan access to them. Looks like they're not officially running though - will try to look at this asap.
They are in the process of being added to the buildbot by Daniel Gollub. However, the ppc box is unable to build qemu.git because it hits ENOMEM while compiling. I doubled swap size but that didn't fix the issue so I need to investigate more. At least s390 should be good
Hrm, yeah. I hit that one too just before I left. Maybe I should just search for another RAM bar :).
Quoted 1 line
to go soon and I will send an update when it is up and running.
Thanks!
Alex
PATCH 10/15 kvm: x86: Catch and report failing IRQ and NMI injections
In reply to Jan KiszkaWe do not need to abort, but the user should be notified that weird things go on.
--- target-i386/kvm.c | 16 +++++++++++++--- 1 files changed, 13 insertions(+), 3 deletions(-)
target-i386/kvm.c +13 -3
@@ -1442,11 +1442,17 @@ int kvm_arch_get_registers(CPUState *env)void kvm_arch_pre_run (CPUState *env,struct kvm_run *run) { +int ret; +/* Inject NMI */ if (env->interrupt_request &CPU_INTERRUPT_NMI ) { env->interrupt_request &= ~CPU_INTERRUPT_NMI ;DPRINTF ("injected NMI\n" ); -kvm_vcpu_ioctl (env,KVM_NMI ); + ret =kvm_vcpu_ioctl (env,KVM_NMI ); +if (ret <0 ) { +fprintf (stderr,"KVM: injection failed, NMI lost (%s)\n" , +strerror (-ret)); + } }if (!kvm_irqchip_in_kernel ()) {
@@ -1467,9 +1473,13 @@ void kvm_arch_pre_run(CPUState *env, struct kvm_run *run)struct kvm_interrupt intr; intr.irq = irq; -/* FIXME: errors */ DPRINTF ("injected interrupt %d\n" , irq); -kvm_vcpu_ioctl (env,KVM_INTERRUPT , &intr); + ret =kvm_vcpu_ioctl (env,KVM_INTERRUPT , &intr); +if (ret <0 ) { +fprintf (stderr, +"KVM: injection failed, interrupt lost (%s)\n" , +strerror (-ret)); + } } }
Signature · 1 line
1.7.1
PATCH 11/15 kvm: Remove unneeded memory slot reservation
In reply to Jan KiszkaThe number of slots and the location of private ones changed several times in KVM's early days. However, it's stable since 2.6.29 (our required baseline), and slots 8..11 are no longer reserved since then. So remove this unneeded restriction.
--- kvm-all.c | 4 ---- 1 files changed, 0 insertions(+), 4 deletions(-)
kvm-all.c +0 -4
@@ -91,10 +91,6 @@ static KVMSlot *kvm_alloc_slot(KVMState *s)int i;for (i =0 ; i <ARRAY_SIZE (s->slots); i++) { -/* KVM private memory slots */ -if (i >=8 && i <12 ) { -continue ; - }if (s->slots[i].memory_size ==0 ) {return &s->slots[i]; }
Signature · 1 line
1.7.1
Re: [PATCH 11/15] kvm: Remove unneeded memory slot reservation
Quoted 10 lines · Jan Kiszka
The number of slots and the location of private ones changed several times in KVM's early days. However, it's stable since 2.6.29 (our required baseline), and slots 8..11 are no longer reserved since then. So remove this unneeded restriction.
Signed-off-by Jan Kiszka <[email protected]>CC Alex Williamson <[email protected]>--- kvm-all.c | 4 ---- 1 files changed, 0 insertions(+), 4 deletions(-)
Quoted 15 lines
kvm-all.c +0
-4@@ -91,10 +91,6 @@ static KVMSlot *kvm_alloc_slot(KVMState *s)int i;for (i =0 ; i <ARRAY_SIZE (s->slots); i++) { -/* KVM private memory slots */ -if (i >=8 && i <12 ) { -continue ; - }if (s->slots[i].memory_size ==0 ) {return &s->slots[i]; }
PATCH 12/15 Introduce log_start/log_stop in CPUPhysMemoryClient
In reply to Jan KiszkaFrom: Anthony PERARD <[email protected]>
In order to use log_start/log_stop with Xen as well in the vga code, this two operations have been put in CPUPhysMemoryClient.
The two new functions cpu_physical_log_start,cpu_physical_log_stop are used in hw/vga.c and replace the kvm_log_start/stop. With this, vga does no longer depends on kvm header.
[ Jan: rebasing and style fixlets ]
--- cpu-all.h | 6 ++++++ cpu-common.h | 4 ++++ exec.c | 30 ++++++++++++++++++++++++++++++ hw/vga.c | 31 ++++++++++++++++--------------- hw/vhost.c | 2 ++ kvm-all.c | 8 ++++++-- kvm-stub.c | 10 ---------- kvm.h | 3 --- 8 files changed, 64 insertions(+), 30 deletions(-)
cpu-all.h +6 -0
@@ -959,6 +959,12 @@ int cpu_physical_memory_get_dirty_tracking(void);int cpu_physical_sync_dirty_bitmap (target_phys_addr_t start_addr,target_phys_addr_t end_addr); +int cpu_physical_log_start (target_phys_addr_t start_addr, +ram_addr_t size); + +int cpu_physical_log_stop (target_phys_addr_t start_addr, +ram_addr_t size); +void dump_exec_info (FILE *f,fprintf_function cpu_fprintf);#endif /* !CONFIG_USER_ONLY */
cpu-common.h +4 -0
@@ -96,6 +96,10 @@ struct CPUPhysMemoryClient { target_phys_addr_t end_addr);int (*migration_log)(struct CPUPhysMemoryClient *client,int enable); +int (*log_start)(struct CPUPhysMemoryClient *client, +target_phys_addr_t phys_addr,ram_addr_t size); +int (*log_stop)(struct CPUPhysMemoryClient *client, +target_phys_addr_t phys_addr,ram_addr_t size);QLIST_ENTRY (CPUPhysMemoryClient) list; };
exec.c +30 -0
@@ -2078,6 +2078,36 @@ int cpu_physical_sync_dirty_bitmap(target_phys_addr_t start_addr,return ret; } +int cpu_physical_log_start (target_phys_addr_t start_addr, +ram_addr_t size) +{ +CPUPhysMemoryClient *client; + QLIST_FOREACH(client, &memory_client_list, list) { +if (client->log_start) { +int r = client->log_start (client, start_addr, size); +if (r <0 ) { +return r; + } + } + } +return 0 ; +} + +int cpu_physical_log_stop (target_phys_addr_t start_addr, +ram_addr_t size) +{ +CPUPhysMemoryClient *client; + QLIST_FOREACH(client, &memory_client_list, list) { +if (client->log_stop) { +int r = client->log_stop (client, start_addr, size); +if (r <0 ) { +return r; + } + } + } +return 0 ; +} +static inline void tlb_update_dirty (CPUTLBEntry *tlb_entry) {ram_addr_t ram_addr;
hw/vga.c +16 -15
@@ -28,7 +28,6 @@#include "vga_int.h" #include "pixel_ops.h" #include "qemu-timer.h" -#include "kvm.h" //#define DEBUG_VGA //#define DEBUG_VGA_MEM
@@ -1573,34 +1572,36 @@ static void vga_sync_dirty_bitmap(VGACommonState *s)void vga_dirty_log_start (VGACommonState *s) { -if (kvm_enabled () && s->map_addr) -kvm_log_start (s->map_addr, s->map_end - s->map_addr); +if (s->map_addr) { +cpu_physical_log_start (s->map_addr, s->map_end - s->map_addr); + } -if (kvm_enabled () && s->lfb_vram_mapped) { -kvm_log_start (isa_mem_base +0xa0000 ,0x8000 ); -kvm_log_start (isa_mem_base +0xa8000 ,0x8000 ); +if (s->lfb_vram_mapped) { +cpu_physical_log_start (isa_mem_base +0xa0000 ,0x8000 ); +cpu_physical_log_start (isa_mem_base +0xa8000 ,0x8000 ); }#ifdef CONFIG_BOCHS_VBE -if (kvm_enabled () && s->vbe_mapped) { -kvm_log_start (VBE_DISPI_LFB_PHYSICAL_ADDRESS , s->vram_size); +if (s->vbe_mapped) { +cpu_physical_log_start (VBE_DISPI_LFB_PHYSICAL_ADDRESS , s->vram_size); }#endif }void vga_dirty_log_stop (VGACommonState *s) { -if (kvm_enabled () && s->map_addr) -kvm_log_stop (s->map_addr, s->map_end - s->map_addr); +if (s->map_addr) { +cpu_physical_log_stop (s->map_addr, s->map_end - s->map_addr); + } -if (kvm_enabled () && s->lfb_vram_mapped) { -kvm_log_stop (isa_mem_base +0xa0000 ,0x8000 ); -kvm_log_stop (isa_mem_base +0xa8000 ,0x8000 ); +if (s->lfb_vram_mapped) { +cpu_physical_log_stop (isa_mem_base +0xa0000 ,0x8000 ); +cpu_physical_log_stop (isa_mem_base +0xa8000 ,0x8000 ); }#ifdef CONFIG_BOCHS_VBE -if (kvm_enabled () && s->vbe_mapped) { -kvm_log_stop (VBE_DISPI_LFB_PHYSICAL_ADDRESS , s->vram_size); +if (s->vbe_mapped) { +cpu_physical_log_stop (VBE_DISPI_LFB_PHYSICAL_ADDRESS , s->vram_size); }#endif }
hw/vhost.c +2 -0
@@ -607,6 +607,8 @@ int vhost_dev_init(struct vhost_dev *hdev, int devfd, bool force) hdev->client.set_memory = vhost_client_set_memory; hdev->client.sync_dirty_bitmap = vhost_client_sync_dirty_bitmap; hdev->client.migration_log = vhost_client_migration_log; + hdev->client.log_start =NULL ; + hdev->client.log_stop =NULL ; hdev->mem =qemu_mallocz (offsetof(struct vhost_memory , regions)); hdev->log =NULL ; hdev->log_size =0 ;
kvm-all.c +6 -2
@@ -274,13 +274,15 @@ static int kvm_dirty_pages_log_change(target_phys_addr_t phys_addr,return kvm_set_user_memory_region (s, mem); } -int kvm_log_start (target_phys_addr_t phys_addr,ram_addr_t size) +static int kvm_log_start (CPUPhysMemoryClient *client, +target_phys_addr_t phys_addr,ram_addr_t size) {return kvm_dirty_pages_log_change (phys_addr, size,KVM_MEM_LOG_DIRTY_PAGES ,KVM_MEM_LOG_DIRTY_PAGES ); } -int kvm_log_stop (target_phys_addr_t phys_addr,ram_addr_t size) +static int kvm_log_stop (CPUPhysMemoryClient *client, +target_phys_addr_t phys_addr,ram_addr_t size) {return kvm_dirty_pages_log_change (phys_addr, size,0 ,KVM_MEM_LOG_DIRTY_PAGES );
@@ -644,6 +646,8 @@ static CPUPhysMemoryClient kvm_cpu_phys_memory_client = { .set_memory = kvm_client_set_memory, .sync_dirty_bitmap = kvm_client_sync_dirty_bitmap, .migration_log = kvm_client_migration_log, + .log_start = kvm_log_start, + .log_stop = kvm_log_stop, };int kvm_init (void )
kvm-stub.c +0 -10
@@ -33,16 +33,6 @@ int kvm_init_vcpu(CPUState *env)return -ENOSYS ; } -int kvm_log_start (target_phys_addr_t phys_addr,ram_addr_t size) -{ -return -ENOSYS ; -} - -int kvm_log_stop (target_phys_addr_t phys_addr,ram_addr_t size) -{ -return -ENOSYS ; -} -int kvm_coalesce_mmio_region (target_phys_addr_t start,ram_addr_t size) {return -ENOSYS ;
kvm.h +0 -3
@@ -58,9 +58,6 @@ int kvm_init_vcpu(CPUState *env);int kvm_cpu_exec (CPUState *env);#if !defined(CONFIG_USER_ONLY ) -int kvm_log_start (target_phys_addr_t phys_addr,ram_addr_t size); -int kvm_log_stop (target_phys_addr_t phys_addr,ram_addr_t size); -void kvm_setup_guest_memory (void *start,size_t size);int kvm_coalesce_mmio_region (target_phys_addr_t start,ram_addr_t size);
Signature · 1 line
1.7.1
PATCH 13/15 cirrus: Remove obsolete kvm.h include
In reply to Jan Kiszka--- hw/cirrus_vga.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-)
hw/cirrus_vga.c +0 -1
@@ -31,7 +31,6 @@#include "pci.h" #include "console.h" #include "vga_int.h" -#include "kvm.h" #include "loader.h" /*
Signature · 1 line
1.7.1
PATCH 14/15 kvm: Make kvm_state globally available
In reply to Jan KiszkaKVM-assisted devices need access to it but we have no clean channel to distribute a reference. As a workaround until there is a better solution, export kvm_state for global use, though use should remain restricted to the mentioned scenario.
--- kvm-all.c | 2 +- kvm.h | 1 + 2 files changed, 2 insertions(+), 1 deletions(-)
kvm-all.c +1 -1
@@ -78,7 +78,7 @@ struct KVMStateint many_ioeventfds; }; -static KVMState *kvm_state; +KVMState *kvm_state;static const KVMCapabilityInfo kvm_required_capabilites[] = {KVM_CAP_INFO (USER_MEMORY ),
kvm.h +1 -0
@@ -85,6 +85,7 @@ int kvm_on_sigbus(int code, void *addr);struct KVMState ;typedef struct KVMState KVMState ; +extern KVMState *kvm_state;int kvm_ioctl (KVMState *s,int type, ...);
Signature · 1 line
1.7.1
PATCH 15/15 kvm: x86: Introduce kvmclock device to save/restore its state
In reply to Jan KiszkaIf kvmclock is used, which implies the kernel supports it, register a kvmclock device with the sysbus. Its main purpose is to save and restore the kernel state on migration, but this will also allow to visualize it one day.
--- Makefile.target | 4 +- hw/kvmclock.c | 125 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ hw/kvmclock.h | 14 ++++++ hw/pc_piix.c | 31 +++++++++++--- 4 files changed, 165 insertions(+), 9 deletions(-) create mode 100644 hw/kvmclock.c create mode 100644 hw/kvmclock.h
Makefile.target +2 -2
@@ -37,7 +37,7 @@ ifndef CONFIG_HAIKU LIBS+=-lm endif -kvm.o kvm-all.o vhost.o vhost_net.o: QEMU_CFLAGS+=$(KVM_CFLAGS) +kvm.o kvm-all.o vhost.o vhost_net.o kvmclock.o: QEMU_CFLAGS+=$(KVM_CFLAGS) config-target.h: config-target.h-timestamp config-target.h-timestamp: config-target.mak
@@ -218,7 +218,7 @@ obj-i386-y += cirrus_vga.o apic.o ioapic.o piix_pci.o obj-i386-y += vmmouse.o vmport.o hpet.o applesmc.o obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o obj-i386-y += debugcon.o multiboot.o -obj-i386-y += pc_piix.o +obj-i386-y += pc_piix.o kvmclock.o obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o # shared objects
hw/kvmclock.c new file +125 -0
125 lines not on this page · raw message
hw/kvmclock.h new file +14 -0
@@ -0,0 +1,14 @@ +/* +* QEMU KVM support, paravirtual clock device +* +* Copyright (C) 2011 Siemens AG +* +* Authors: +* Jan Kiszka <[email protected]> +* +* This work is licensed under the terms of the GNU GPL version 2. +* See the COPYING file in the top-level directory. +* +*/ + +void kvmclock_create (void );
hw/pc_piix.c +24 -7
@@ -32,6 +32,7 @@#include "boards.h" #include "ide.h" #include "kvm.h" +#include "kvmclock.h" #include "sysemu.h" #include "sysbus.h" #include "arch_init.h"
@@ -66,7 +67,8 @@ static void pc_init1(ram_addr_t ram_size,const char *kernel_cmdline, const char *initrd_filename, const char *cpu_model, - int pci_enabled) + int pci_enabled, + int kvmclock_enabled) { int i;ram_addr_t below_4g_mem_size, above_4g_mem_size;
@@ -87,6 +89,9 @@ static void pc_init1(ram_addr_t ram_size,pc_cpus_init (cpu_model);vmport_init (); +if (kvmclock_enabled) { +kvmclock_create (); + }/* allocate ram and load rom/bios */ pc_memory_init(ram_size, kernel_filename, kernel_cmdline, initrd_filename,
@@ -195,7 +200,19 @@ static void pc_init_pci(ram_addr_t ram_size, {pc_init1 (ram_size, boot_device, kernel_filename, kernel_cmdline, - initrd_filename, cpu_model,1 ); + initrd_filename, cpu_model,1 ,1 ); +} + +static void pc_init_pci_no_kvmclock (ram_addr_t ram_size, +const char *boot_device, +const char *kernel_filename, +const char *kernel_cmdline, +const char *initrd_filename, +const char *cpu_model) +{ +pc_init1 (ram_size, boot_device, + kernel_filename, kernel_cmdline, + initrd_filename, cpu_model,1 ,0 ); } static void pc_init_isa(ram_addr_t ram_size,
@@ -209,7 +226,7 @@ static void pc_init_isa(ram_addr_t ram_size, cpu_model ="486" ;pc_init1 (ram_size, boot_device, kernel_filename, kernel_cmdline, - initrd_filename, cpu_model,0 ); + initrd_filename, cpu_model,0 ,1 ); }static QEMUMachine pc_machine = {
@@ -224,7 +241,7 @@ static QEMUMachine pc_machine = {static QEMUMachine pc_machine_v0_13 = { .name ="pc-0.13" , .desc ="Standard PC" , - .init = pc_init_pci, + .init = pc_init_pci_no_kvmclock, .max_cpus =255 , .compat_props = (GlobalProperty []) { {
@@ -251,7 +268,7 @@ static QEMUMachine pc_machine_v0_13 = {static QEMUMachine pc_machine_v0_12 = { .name ="pc-0.12" , .desc ="Standard PC" , - .init = pc_init_pci, + .init = pc_init_pci_no_kvmclock, .max_cpus =255 , .compat_props = (GlobalProperty []) { {
@@ -282,7 +299,7 @@ static QEMUMachine pc_machine_v0_12 = {static QEMUMachine pc_machine_v0_11 = { .name ="pc-0.11" , .desc ="Standard PC, qemu 0.11" , - .init = pc_init_pci, + .init = pc_init_pci_no_kvmclock, .max_cpus =255 , .compat_props = (GlobalProperty []) { {
@@ -321,7 +338,7 @@ static QEMUMachine pc_machine_v0_11 = {static QEMUMachine pc_machine_v0_10 = { .name ="pc-0.10" , .desc ="Standard PC, qemu 0.10" , - .init = pc_init_pci, + .init = pc_init_pci_no_kvmclock, .max_cpus =255 , .compat_props = (GlobalProperty []) { {
Signature · 1 line
1.7.1
Re: [PATCH 15/15] kvm: x86: Introduce kvmclock device to save/restore its state
Quoted 202 lines · Jan Kiszka
If kvmclock is used, which implies the kernel supports it, register a kvmclock device with the sysbus. Its main purpose is to save and restore the kernel state on migration, but this will also allow to visualize it one day.
Signed-off-by Jan Kiszka <[email protected]>CC Glauber Costa <[email protected]>--- Makefile.target | 4 +- hw/kvmclock.c | 125 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ hw/kvmclock.h | 14 ++++++ hw/pc_piix.c | 31 +++++++++++--- 4 files changed, 165 insertions(+), 9 deletions(-) create mode 100644 hw/kvmclock.c create mode 100644 hw/kvmclock.hMakefile.target +2
-2@@ -37,7 +37,7 @@ ifndef CONFIG_HAIKU LIBS+=-lm endif -kvm.o kvm-all.o vhost.o vhost_net.o: QEMU_CFLAGS+=$(KVM_CFLAGS) +kvm.o kvm-all.o vhost.o vhost_net.o kvmclock.o: QEMU_CFLAGS+=$(KVM_CFLAGS) config-target.h: config-target.h-timestamp config-target.h-timestamp: config-target.mak@@ -218,7 +218,7 @@ obj-i386-y += cirrus_vga.o apic.o ioapic.o piix_pci.o obj-i386-y += vmmouse.o vmport.o hpet.o applesmc.o obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o obj-i386-y += debugcon.o multiboot.o -obj-i386-y += pc_piix.o +obj-i386-y += pc_piix.o kvmclock.o obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o # shared objectshw/kvmclock.c new file +125
-0@@ -0,0 +1,125 @@ +/* +* QEMU KVM support, paravirtual clock device +* +* Copyright (C) 2011 Siemens AG +* +* Authors: +* Jan Kiszka <[email protected]> +* +* This work is licensed under the terms of the GNU GPL version 2. +* See the COPYING file in the top-level directory. +* +*/ + +#include "qemu-common.h" +#include "sysemu.h" +#include "sysbus.h" +#include "kvm.h" +#include "kvmclock.h" + +#if defined(CONFIG_KVM_PARA ) && defined(KVM_CAP_ADJUST_CLOCK ) + +#include <linux/kvm.h> +#include <linux/kvm_para.h> + +typedef struct KVMClockState { +SysBusDevice busdev; +uint64_t clock; +bool clock_valid; +}KVMClockState ; + +static void kvmclock_pre_save (void *opaque) +{ +KVMClockState *s = opaque; +struct kvm_clock_data data; +int ret; + +if (s->clock_valid) { +return ; + } + ret =kvm_vm_ioctl (kvm_state,KVM_GET_CLOCK , &data); +if (ret <0 ) { +fprintf (stderr,"KVM_GET_CLOCK failed: %s\n" ,strerror (ret)); + data.clock =0 ; + } + s->clock = data.clock; +/* +* If the VM is stopped, declare the clock state valid to avoid re-reading +* it on next vmsave (which would return a different value). Will be reset +* when the VM is continued. +*/ + s->clock_valid = !vm_running; +} + +static int kvmclock_post_load (void *opaque,int version_id) +{ +KVMClockState *s = opaque; +struct kvm_clock_data data; + + data.clock = s->clock; + data.flags =0 ; +return kvm_vm_ioctl (kvm_state,KVM_SET_CLOCK , &data); +} + +static void kvmclock_vm_state_change (void *opaque,int running,int reason) +{ +KVMClockState *s = opaque; + +if (running) { + s->clock_valid = false; + } +} + +static int kvmclock_init (SysBusDevice *dev) +{ +KVMClockState *s =FROM_SYSBUS (KVMClockState, dev); + +qemu_add_vm_change_state_handler (kvmclock_vm_state_change, s); +return 0 ; +} + +static const VMStateDescription kvmclock_vmsd = { + .name ="kvmclock" , + .version_id =1 , + .minimum_version_id =1 , + .minimum_version_id_old =1 , + .pre_save = kvmclock_pre_save, + .post_load = kvmclock_post_load, + .fields = (VMStateField []) { +VMSTATE_UINT64 (clock, KVMClockState), +VMSTATE_END_OF_LIST () + } +}; + +static SysBusDeviceInfo kvmclock_info = { + .qdev.name ="kvmclock" , + .qdev.size =sizeof (KVMClockState), + .qdev.vmsd = &kvmclock_vmsd, + .qdev.no_user =1 , + .init = kvmclock_init, +}; + +/* Note: Must be called after VCPU initialization. */ +void kvmclock_create (void ) +{ +if (kvm_enabled () && + first_cpu->cpuid_kvm_features & (1ULL <<KVM_FEATURE_CLOCKSOURCE )) { +sysbus_create_simple ("kvmclock" ,-1 ,NULL ); + } +} + +static void kvmclock_register_device (void ) +{ +if (kvm_enabled ()) { +sysbus_register_withprop (&kvmclock_info); + } +} + +device_init (kvmclock_register_device); + +#else /* !(CONFIG_KVM_PARA && KVM_CAP_ADJUST_CLOCK) */ + +void kvmclock_create (void ) +{ +} +#endif /* !(CONFIG_KVM_PARA && KVM_CAP_ADJUST_CLOCK) */ hw/kvmclock.h new file +14
-0@@ -0,0 +1,14 @@ +/* +* QEMU KVM support, paravirtual clock device +* +* Copyright (C) 2011 Siemens AG +* +* Authors: +* Jan Kiszka <[email protected]> +* +* This work is licensed under the terms of the GNU GPL version 2. +* See the COPYING file in the top-level directory. +* +*/ + +void kvmclock_create (void );hw/pc_piix.c +1
-0@@ -32,6 +32,7 @@#include "boards.h" #include "ide.h" #include "kvm.h" +#include "kvmclock.h" #include "sysemu.h" #include "sysbus.h" #include "arch_init.h"
hw/pc_piix.c static void pc_init1(ram_addr_t ram_size, · line 72
Quoted 7 lines
hw/pc_piix.c +2
-1@@ -66,7 +67,8 @@ static void pc_init1(ram_addr_t ram_size,const char *kernel_cmdline, const char *initrd_filename, const char *cpu_model, - int pci_enabled) + int pci_enabled, + int kvmclock_enabled)
What exactly is your motivation to that ? I think mid/long-term we should be making machine initialization more common among architectures, not introducing more arch specific, or even worse, kvm specific parameters here.
I'd like to understand what do we gain from that, since opting kvmclock in our out is done by cpuid anyway - no need for a specific machine.
Re: [PATCH 15/15] kvm: x86: Introduce kvmclock device to save/restore its state
Quoted 215 lines · Glauber Costa
Quoted 202 lines · Jan Kiszka
If kvmclock is used, which implies the kernel supports it, register a kvmclock device with the sysbus. Its main purpose is to save and restore the kernel state on migration, but this will also allow to visualize it one day.
Signed-off-by Jan Kiszka <[email protected]>CC Glauber Costa <[email protected]>--- Makefile.target | 4 +- hw/kvmclock.c | 125 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ hw/kvmclock.h | 14 ++++++ hw/pc_piix.c | 31 +++++++++++--- 4 files changed, 165 insertions(+), 9 deletions(-) create mode 100644 hw/kvmclock.c create mode 100644 hw/kvmclock.hMakefile.target +2
-2@@ -37,7 +37,7 @@ ifndef CONFIG_HAIKU LIBS+=-lm endif -kvm.o kvm-all.o vhost.o vhost_net.o: QEMU_CFLAGS+=$(KVM_CFLAGS) +kvm.o kvm-all.o vhost.o vhost_net.o kvmclock.o: QEMU_CFLAGS+=$(KVM_CFLAGS) config-target.h: config-target.h-timestamp config-target.h-timestamp: config-target.mak@@ -218,7 +218,7 @@ obj-i386-y += cirrus_vga.o apic.o ioapic.o piix_pci.o obj-i386-y += vmmouse.o vmport.o hpet.o applesmc.o obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o obj-i386-y += debugcon.o multiboot.o -obj-i386-y += pc_piix.o +obj-i386-y += pc_piix.o kvmclock.o obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o # shared objectshw/kvmclock.c new file +125
-0@@ -0,0 +1,125 @@ +/* +* QEMU KVM support, paravirtual clock device +* +* Copyright (C) 2011 Siemens AG +* +* Authors: +* Jan Kiszka <[email protected]> +* +* This work is licensed under the terms of the GNU GPL version 2. +* See the COPYING file in the top-level directory. +* +*/ + +#include "qemu-common.h" +#include "sysemu.h" +#include "sysbus.h" +#include "kvm.h" +#include "kvmclock.h" + +#if defined(CONFIG_KVM_PARA ) && defined(KVM_CAP_ADJUST_CLOCK ) + +#include <linux/kvm.h> +#include <linux/kvm_para.h> + +typedef struct KVMClockState { +SysBusDevice busdev; +uint64_t clock; +bool clock_valid; +}KVMClockState ; + +static void kvmclock_pre_save (void *opaque) +{ +KVMClockState *s = opaque; +struct kvm_clock_data data; +int ret; + +if (s->clock_valid) { +return ; + } + ret =kvm_vm_ioctl (kvm_state,KVM_GET_CLOCK , &data); +if (ret <0 ) { +fprintf (stderr,"KVM_GET_CLOCK failed: %s\n" ,strerror (ret)); + data.clock =0 ; + } + s->clock = data.clock; +/* +* If the VM is stopped, declare the clock state valid to avoid re-reading +* it on next vmsave (which would return a different value). Will be reset +* when the VM is continued. +*/ + s->clock_valid = !vm_running; +} + +static int kvmclock_post_load (void *opaque,int version_id) +{ +KVMClockState *s = opaque; +struct kvm_clock_data data; + + data.clock = s->clock; + data.flags =0 ; +return kvm_vm_ioctl (kvm_state,KVM_SET_CLOCK , &data); +} + +static void kvmclock_vm_state_change (void *opaque,int running,int reason) +{ +KVMClockState *s = opaque; + +if (running) { + s->clock_valid = false; + } +} + +static int kvmclock_init (SysBusDevice *dev) +{ +KVMClockState *s =FROM_SYSBUS (KVMClockState, dev); + +qemu_add_vm_change_state_handler (kvmclock_vm_state_change, s); +return 0 ; +} + +static const VMStateDescription kvmclock_vmsd = { + .name ="kvmclock" , + .version_id =1 , + .minimum_version_id =1 , + .minimum_version_id_old =1 , + .pre_save = kvmclock_pre_save, + .post_load = kvmclock_post_load, + .fields = (VMStateField []) { +VMSTATE_UINT64 (clock, KVMClockState), +VMSTATE_END_OF_LIST () + } +}; + +static SysBusDeviceInfo kvmclock_info = { + .qdev.name ="kvmclock" , + .qdev.size =sizeof (KVMClockState), + .qdev.vmsd = &kvmclock_vmsd, + .qdev.no_user =1 , + .init = kvmclock_init, +}; + +/* Note: Must be called after VCPU initialization. */ +void kvmclock_create (void ) +{ +if (kvm_enabled () && + first_cpu->cpuid_kvm_features & (1ULL <<KVM_FEATURE_CLOCKSOURCE )) { +sysbus_create_simple ("kvmclock" ,-1 ,NULL ); + } +} + +static void kvmclock_register_device (void ) +{ +if (kvm_enabled ()) { +sysbus_register_withprop (&kvmclock_info); + } +} + +device_init (kvmclock_register_device); + +#else /* !(CONFIG_KVM_PARA && KVM_CAP_ADJUST_CLOCK) */ + +void kvmclock_create (void ) +{ +} +#endif /* !(CONFIG_KVM_PARA && KVM_CAP_ADJUST_CLOCK) */ hw/kvmclock.h new file +14
-0@@ -0,0 +1,14 @@ +/* +* QEMU KVM support, paravirtual clock device +* +* Copyright (C) 2011 Siemens AG +* +* Authors: +* Jan Kiszka <[email protected]> +* +* This work is licensed under the terms of the GNU GPL version 2. +* See the COPYING file in the top-level directory. +* +*/ + +void kvmclock_create (void );hw/pc_piix.c +1
-0@@ -32,6 +32,7 @@#include "boards.h" #include "ide.h" #include "kvm.h" +#include "kvmclock.h" #include "sysemu.h" #include "sysbus.h" #include "arch_init.h" hw/pc_piix.c static void pc_init1(ram_addr_t ram_size, · line 72
Quoted 7 lines
hw/pc_piix.c +2
-1@@ -66,7 +67,8 @@ static void pc_init1(ram_addr_t ram_size,const char *kernel_cmdline, const char *initrd_filename, const char *cpu_model, - int pci_enabled) + int pci_enabled, + int kvmclock_enabled)What exactly is your motivation to that ? I think mid/long-term we should be making machine initialization more common among architectures, not introducing more arch specific, or even worse, kvm specific parameters here.
Quoted 2 lines
I'd like to understand what do we gain from that, since opting kvmclock in our out is done by cpuid anyway - no need for a specific machine.
Is that really the case? I thought we were already shipping versions where that CPU feature was enabled by default. If not, I'll happily drop that admittedly clumsy approach above.
Jan
Signature · 2 lines
Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [PATCH 15/15] kvm: x86: Introduce kvmclock device to save/restore its state
Quoted 216 lines · Jan Kiszka
Quoted 215 lines · Glauber Costa
Quoted 202 lines · Jan Kiszka
If kvmclock is used, which implies the kernel supports it, register a kvmclock device with the sysbus. Its main purpose is to save and restore the kernel state on migration, but this will also allow to visualize it one day.
Signed-off-by Jan Kiszka <[email protected]>CC Glauber Costa <[email protected]>--- Makefile.target | 4 +- hw/kvmclock.c | 125 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ hw/kvmclock.h | 14 ++++++ hw/pc_piix.c | 31 +++++++++++--- 4 files changed, 165 insertions(+), 9 deletions(-) create mode 100644 hw/kvmclock.c create mode 100644 hw/kvmclock.hMakefile.target +2
-2@@ -37,7 +37,7 @@ ifndef CONFIG_HAIKU LIBS+=-lm endif -kvm.o kvm-all.o vhost.o vhost_net.o: QEMU_CFLAGS+=$(KVM_CFLAGS) +kvm.o kvm-all.o vhost.o vhost_net.o kvmclock.o: QEMU_CFLAGS+=$(KVM_CFLAGS) config-target.h: config-target.h-timestamp config-target.h-timestamp: config-target.mak@@ -218,7 +218,7 @@ obj-i386-y += cirrus_vga.o apic.o ioapic.o piix_pci.o obj-i386-y += vmmouse.o vmport.o hpet.o applesmc.o obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o obj-i386-y += debugcon.o multiboot.o -obj-i386-y += pc_piix.o +obj-i386-y += pc_piix.o kvmclock.o obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o # shared objectshw/kvmclock.c new file +125
-0@@ -0,0 +1,125 @@ +/* +* QEMU KVM support, paravirtual clock device +* +* Copyright (C) 2011 Siemens AG +* +* Authors: +* Jan Kiszka <[email protected]> +* +* This work is licensed under the terms of the GNU GPL version 2. +* See the COPYING file in the top-level directory. +* +*/ + +#include "qemu-common.h" +#include "sysemu.h" +#include "sysbus.h" +#include "kvm.h" +#include "kvmclock.h" + +#if defined(CONFIG_KVM_PARA ) && defined(KVM_CAP_ADJUST_CLOCK ) + +#include <linux/kvm.h> +#include <linux/kvm_para.h> + +typedef struct KVMClockState { +SysBusDevice busdev; +uint64_t clock; +bool clock_valid; +}KVMClockState ; + +static void kvmclock_pre_save (void *opaque) +{ +KVMClockState *s = opaque; +struct kvm_clock_data data; +int ret; + +if (s->clock_valid) { +return ; + } + ret =kvm_vm_ioctl (kvm_state,KVM_GET_CLOCK , &data); +if (ret <0 ) { +fprintf (stderr,"KVM_GET_CLOCK failed: %s\n" ,strerror (ret)); + data.clock =0 ; + } + s->clock = data.clock; +/* +* If the VM is stopped, declare the clock state valid to avoid re-reading +* it on next vmsave (which would return a different value). Will be reset +* when the VM is continued. +*/ + s->clock_valid = !vm_running; +} + +static int kvmclock_post_load (void *opaque,int version_id) +{ +KVMClockState *s = opaque; +struct kvm_clock_data data; + + data.clock = s->clock; + data.flags =0 ; +return kvm_vm_ioctl (kvm_state,KVM_SET_CLOCK , &data); +} + +static void kvmclock_vm_state_change (void *opaque,int running,int reason) +{ +KVMClockState *s = opaque; + +if (running) { + s->clock_valid = false; + } +} + +static int kvmclock_init (SysBusDevice *dev) +{ +KVMClockState *s =FROM_SYSBUS (KVMClockState, dev); + +qemu_add_vm_change_state_handler (kvmclock_vm_state_change, s); +return 0 ; +} + +static const VMStateDescription kvmclock_vmsd = { + .name ="kvmclock" , + .version_id =1 , + .minimum_version_id =1 , + .minimum_version_id_old =1 , + .pre_save = kvmclock_pre_save, + .post_load = kvmclock_post_load, + .fields = (VMStateField []) { +VMSTATE_UINT64 (clock, KVMClockState), +VMSTATE_END_OF_LIST () + } +}; + +static SysBusDeviceInfo kvmclock_info = { + .qdev.name ="kvmclock" , + .qdev.size =sizeof (KVMClockState), + .qdev.vmsd = &kvmclock_vmsd, + .qdev.no_user =1 , + .init = kvmclock_init, +}; + +/* Note: Must be called after VCPU initialization. */ +void kvmclock_create (void ) +{ +if (kvm_enabled () && + first_cpu->cpuid_kvm_features & (1ULL <<KVM_FEATURE_CLOCKSOURCE )) { +sysbus_create_simple ("kvmclock" ,-1 ,NULL ); + } +} + +static void kvmclock_register_device (void ) +{ +if (kvm_enabled ()) { +sysbus_register_withprop (&kvmclock_info); + } +} + +device_init (kvmclock_register_device); + +#else /* !(CONFIG_KVM_PARA && KVM_CAP_ADJUST_CLOCK) */ + +void kvmclock_create (void ) +{ +} +#endif /* !(CONFIG_KVM_PARA && KVM_CAP_ADJUST_CLOCK) */ hw/kvmclock.h new file +14
-0@@ -0,0 +1,14 @@ +/* +* QEMU KVM support, paravirtual clock device +* +* Copyright (C) 2011 Siemens AG +* +* Authors: +* Jan Kiszka <[email protected]> +* +* This work is licensed under the terms of the GNU GPL version 2. +* See the COPYING file in the top-level directory. +* +*/ + +void kvmclock_create (void );hw/pc_piix.c +1
-0@@ -32,6 +32,7 @@#include "boards.h" #include "ide.h" #include "kvm.h" +#include "kvmclock.h" #include "sysemu.h" #include "sysbus.h" #include "arch_init.h" hw/pc_piix.c static void pc_init1(ram_addr_t ram_size, · line 72
Quoted 7 lines
hw/pc_piix.c +2
-1@@ -66,7 +67,8 @@ static void pc_init1(ram_addr_t ram_size,const char *kernel_cmdline, const char *initrd_filename, const char *cpu_model, - int pci_enabled) + int pci_enabled, + int kvmclock_enabled)What exactly is your motivation to that ? I think mid/long-term we should be making machine initialization more common among architectures, not introducing more arch specific, or even worse, kvm specific parameters here.
Quoted 6 lines
Quoted 2 lines
I'd like to understand what do we gain from that, since opting kvmclock in our out is done by cpuid anyway - no need for a specific machine.
Is that really the case? I thought we were already shipping versions where that CPU feature was enabled by default. If not, I'll happily drop that admittedly clumsy approach above.
Yes, AFAIK, kvmclock is enabled by default, disabled by cpuid-leaf, as in
-cpu kvm64,-kvmclock
So your test for cpuid bit before starting kvmclock should already cover it.
Re: [PATCH 15/15] kvm: x86: Introduce kvmclock device to save/restore its state
Quoted 224 lines · Glauber Costa
Quoted 216 lines · Jan Kiszka
Quoted 215 lines · Glauber Costa
Quoted 202 lines · Jan Kiszka
If kvmclock is used, which implies the kernel supports it, register a kvmclock device with the sysbus. Its main purpose is to save and restore the kernel state on migration, but this will also allow to visualize it one day.
Signed-off-by Jan Kiszka <[email protected]>CC Glauber Costa <[email protected]>--- Makefile.target | 4 +- hw/kvmclock.c | 125 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ hw/kvmclock.h | 14 ++++++ hw/pc_piix.c | 31 +++++++++++--- 4 files changed, 165 insertions(+), 9 deletions(-) create mode 100644 hw/kvmclock.c create mode 100644 hw/kvmclock.hMakefile.target +2
-2@@ -37,7 +37,7 @@ ifndef CONFIG_HAIKU LIBS+=-lm endif -kvm.o kvm-all.o vhost.o vhost_net.o: QEMU_CFLAGS+=$(KVM_CFLAGS) +kvm.o kvm-all.o vhost.o vhost_net.o kvmclock.o: QEMU_CFLAGS+=$(KVM_CFLAGS) config-target.h: config-target.h-timestamp config-target.h-timestamp: config-target.mak@@ -218,7 +218,7 @@ obj-i386-y += cirrus_vga.o apic.o ioapic.o piix_pci.o obj-i386-y += vmmouse.o vmport.o hpet.o applesmc.o obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o obj-i386-y += debugcon.o multiboot.o -obj-i386-y += pc_piix.o +obj-i386-y += pc_piix.o kvmclock.o obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o # shared objectshw/kvmclock.c new file +125
-0@@ -0,0 +1,125 @@ +/* +* QEMU KVM support, paravirtual clock device +* +* Copyright (C) 2011 Siemens AG +* +* Authors: +* Jan Kiszka <[email protected]> +* +* This work is licensed under the terms of the GNU GPL version 2. +* See the COPYING file in the top-level directory. +* +*/ + +#include "qemu-common.h" +#include "sysemu.h" +#include "sysbus.h" +#include "kvm.h" +#include "kvmclock.h" + +#if defined(CONFIG_KVM_PARA ) && defined(KVM_CAP_ADJUST_CLOCK ) + +#include <linux/kvm.h> +#include <linux/kvm_para.h> + +typedef struct KVMClockState { +SysBusDevice busdev; +uint64_t clock; +bool clock_valid; +}KVMClockState ; + +static void kvmclock_pre_save (void *opaque) +{ +KVMClockState *s = opaque; +struct kvm_clock_data data; +int ret; + +if (s->clock_valid) { +return ; + } + ret =kvm_vm_ioctl (kvm_state,KVM_GET_CLOCK , &data); +if (ret <0 ) { +fprintf (stderr,"KVM_GET_CLOCK failed: %s\n" ,strerror (ret)); + data.clock =0 ; + } + s->clock = data.clock; +/* +* If the VM is stopped, declare the clock state valid to avoid re-reading +* it on next vmsave (which would return a different value). Will be reset +* when the VM is continued. +*/ + s->clock_valid = !vm_running; +} + +static int kvmclock_post_load (void *opaque,int version_id) +{ +KVMClockState *s = opaque; +struct kvm_clock_data data; + + data.clock = s->clock; + data.flags =0 ; +return kvm_vm_ioctl (kvm_state,KVM_SET_CLOCK , &data); +} + +static void kvmclock_vm_state_change (void *opaque,int running,int reason) +{ +KVMClockState *s = opaque; + +if (running) { + s->clock_valid = false; + } +} + +static int kvmclock_init (SysBusDevice *dev) +{ +KVMClockState *s =FROM_SYSBUS (KVMClockState, dev); + +qemu_add_vm_change_state_handler (kvmclock_vm_state_change, s); +return 0 ; +} + +static const VMStateDescription kvmclock_vmsd = { + .name ="kvmclock" , + .version_id =1 , + .minimum_version_id =1 , + .minimum_version_id_old =1 , + .pre_save = kvmclock_pre_save, + .post_load = kvmclock_post_load, + .fields = (VMStateField []) { +VMSTATE_UINT64 (clock, KVMClockState), +VMSTATE_END_OF_LIST () + } +}; + +static SysBusDeviceInfo kvmclock_info = { + .qdev.name ="kvmclock" , + .qdev.size =sizeof (KVMClockState), + .qdev.vmsd = &kvmclock_vmsd, + .qdev.no_user =1 , + .init = kvmclock_init, +}; + +/* Note: Must be called after VCPU initialization. */ +void kvmclock_create (void ) +{ +if (kvm_enabled () && + first_cpu->cpuid_kvm_features & (1ULL <<KVM_FEATURE_CLOCKSOURCE )) { +sysbus_create_simple ("kvmclock" ,-1 ,NULL ); + } +} + +static void kvmclock_register_device (void ) +{ +if (kvm_enabled ()) { +sysbus_register_withprop (&kvmclock_info); + } +} + +device_init (kvmclock_register_device); + +#else /* !(CONFIG_KVM_PARA && KVM_CAP_ADJUST_CLOCK) */ + +void kvmclock_create (void ) +{ +} +#endif /* !(CONFIG_KVM_PARA && KVM_CAP_ADJUST_CLOCK) */ hw/kvmclock.h new file +14
-0@@ -0,0 +1,14 @@ +/* +* QEMU KVM support, paravirtual clock device +* +* Copyright (C) 2011 Siemens AG +* +* Authors: +* Jan Kiszka <[email protected]> +* +* This work is licensed under the terms of the GNU GPL version 2. +* See the COPYING file in the top-level directory. +* +*/ + +void kvmclock_create (void );hw/pc_piix.c +1
-0@@ -32,6 +32,7 @@#include "boards.h" #include "ide.h" #include "kvm.h" +#include "kvmclock.h" #include "sysemu.h" #include "sysbus.h" #include "arch_init.h" hw/pc_piix.c static void pc_init1(ram_addr_t ram_size, · line 72
Quoted 7 lines
hw/pc_piix.c +2
-1@@ -66,7 +67,8 @@ static void pc_init1(ram_addr_t ram_size,const char *kernel_cmdline, const char *initrd_filename, const char *cpu_model, - int pci_enabled) + int pci_enabled, + int kvmclock_enabled)What exactly is your motivation to that ? I think mid/long-term we should be making machine initialization more common among architectures, not introducing more arch specific, or even worse, kvm specific parameters here.
Quoted 6 lines
Quoted 2 lines
I'd like to understand what do we gain from that, since opting kvmclock in our out is done by cpuid anyway - no need for a specific machine.
Is that really the case? I thought we were already shipping versions where that CPU feature was enabled by default. If not, I'll happily drop that admittedly clumsy approach above.
Quoted 7 lines
Yes, AFAIK, kvmclock is enabled by default, disabled by cpuid-leaf, as in
-cpu kvm64,-kvmclockSo your test for cpuid bit before starting kvmclock should already cover it.
No, just the contrary: As kvmclock was always enabled in older versions and the compat machines also expose it, we cannot rely on the flag to enable this new (and therefore 0.15-only) vmstate.
Jan
Signature · 2 lines
Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [PATCH 15/15] kvm: x86: Introduce kvmclock device to save/restore its state
Quoted 229 lines · Jan Kiszka
Quoted 228 lines · Glauber Costa
Quoted 216 lines · Jan Kiszka
Quoted 215 lines · Glauber Costa
Quoted 202 lines · Jan Kiszka
If kvmclock is used, which implies the kernel supports it, register a kvmclock device with the sysbus. Its main purpose is to save and restore the kernel state on migration, but this will also allow to visualize it one day.
Signed-off-by Jan Kiszka <[email protected]>CC Glauber Costa <[email protected]>--- Makefile.target | 4 +- hw/kvmclock.c | 125 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ hw/kvmclock.h | 14 ++++++ hw/pc_piix.c | 31 +++++++++++--- 4 files changed, 165 insertions(+), 9 deletions(-) create mode 100644 hw/kvmclock.c create mode 100644 hw/kvmclock.hMakefile.target +2
-2@@ -37,7 +37,7 @@ ifndef CONFIG_HAIKU LIBS+=-lm endif -kvm.o kvm-all.o vhost.o vhost_net.o: QEMU_CFLAGS+=$(KVM_CFLAGS) +kvm.o kvm-all.o vhost.o vhost_net.o kvmclock.o: QEMU_CFLAGS+=$(KVM_CFLAGS) config-target.h: config-target.h-timestamp config-target.h-timestamp: config-target.mak@@ -218,7 +218,7 @@ obj-i386-y += cirrus_vga.o apic.o ioapic.o piix_pci.o obj-i386-y += vmmouse.o vmport.o hpet.o applesmc.o obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o obj-i386-y += debugcon.o multiboot.o -obj-i386-y += pc_piix.o +obj-i386-y += pc_piix.o kvmclock.o obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o # shared objectshw/kvmclock.c new file +125
-0@@ -0,0 +1,125 @@ +/* +* QEMU KVM support, paravirtual clock device +* +* Copyright (C) 2011 Siemens AG +* +* Authors: +* Jan Kiszka <[email protected]> +* +* This work is licensed under the terms of the GNU GPL version 2. +* See the COPYING file in the top-level directory. +* +*/ + +#include "qemu-common.h" +#include "sysemu.h" +#include "sysbus.h" +#include "kvm.h" +#include "kvmclock.h" + +#if defined(CONFIG_KVM_PARA ) && defined(KVM_CAP_ADJUST_CLOCK ) + +#include <linux/kvm.h> +#include <linux/kvm_para.h> + +typedef struct KVMClockState { +SysBusDevice busdev; +uint64_t clock; +bool clock_valid; +}KVMClockState ; + +static void kvmclock_pre_save (void *opaque) +{ +KVMClockState *s = opaque; +struct kvm_clock_data data; +int ret; + +if (s->clock_valid) { +return ; + } + ret =kvm_vm_ioctl (kvm_state,KVM_GET_CLOCK , &data); +if (ret <0 ) { +fprintf (stderr,"KVM_GET_CLOCK failed: %s\n" ,strerror (ret)); + data.clock =0 ; + } + s->clock = data.clock; +/* +* If the VM is stopped, declare the clock state valid to avoid re-reading +* it on next vmsave (which would return a different value). Will be reset +* when the VM is continued. +*/ + s->clock_valid = !vm_running; +} + +static int kvmclock_post_load (void *opaque,int version_id) +{ +KVMClockState *s = opaque; +struct kvm_clock_data data; + + data.clock = s->clock; + data.flags =0 ; +return kvm_vm_ioctl (kvm_state,KVM_SET_CLOCK , &data); +} + +static void kvmclock_vm_state_change (void *opaque,int running,int reason) +{ +KVMClockState *s = opaque; + +if (running) { + s->clock_valid = false; + } +} + +static int kvmclock_init (SysBusDevice *dev) +{ +KVMClockState *s =FROM_SYSBUS (KVMClockState, dev); + +qemu_add_vm_change_state_handler (kvmclock_vm_state_change, s); +return 0 ; +} + +static const VMStateDescription kvmclock_vmsd = { + .name ="kvmclock" , + .version_id =1 , + .minimum_version_id =1 , + .minimum_version_id_old =1 , + .pre_save = kvmclock_pre_save, + .post_load = kvmclock_post_load, + .fields = (VMStateField []) { +VMSTATE_UINT64 (clock, KVMClockState), +VMSTATE_END_OF_LIST () + } +}; + +static SysBusDeviceInfo kvmclock_info = { + .qdev.name ="kvmclock" , + .qdev.size =sizeof (KVMClockState), + .qdev.vmsd = &kvmclock_vmsd, + .qdev.no_user =1 , + .init = kvmclock_init, +}; + +/* Note: Must be called after VCPU initialization. */ +void kvmclock_create (void ) +{ +if (kvm_enabled () && + first_cpu->cpuid_kvm_features & (1ULL <<KVM_FEATURE_CLOCKSOURCE )) { +sysbus_create_simple ("kvmclock" ,-1 ,NULL ); + } +} + +static void kvmclock_register_device (void ) +{ +if (kvm_enabled ()) { +sysbus_register_withprop (&kvmclock_info); + } +} + +device_init (kvmclock_register_device); + +#else /* !(CONFIG_KVM_PARA && KVM_CAP_ADJUST_CLOCK) */ + +void kvmclock_create (void ) +{ +} +#endif /* !(CONFIG_KVM_PARA && KVM_CAP_ADJUST_CLOCK) */ hw/kvmclock.h new file +14
-0@@ -0,0 +1,14 @@ +/* +* QEMU KVM support, paravirtual clock device +* +* Copyright (C) 2011 Siemens AG +* +* Authors: +* Jan Kiszka <[email protected]> +* +* This work is licensed under the terms of the GNU GPL version 2. +* See the COPYING file in the top-level directory. +* +*/ + +void kvmclock_create (void );hw/pc_piix.c +1
-0@@ -32,6 +32,7 @@#include "boards.h" #include "ide.h" #include "kvm.h" +#include "kvmclock.h" #include "sysemu.h" #include "sysbus.h" #include "arch_init.h" hw/pc_piix.c static void pc_init1(ram_addr_t ram_size, · line 72
Quoted 7 lines
hw/pc_piix.c +2
-1@@ -66,7 +67,8 @@ static void pc_init1(ram_addr_t ram_size,const char *kernel_cmdline, const char *initrd_filename, const char *cpu_model, - int pci_enabled) + int pci_enabled, + int kvmclock_enabled)What exactly is your motivation to that ? I think mid/long-term we should be making machine initialization more common among architectures, not introducing more arch specific, or even worse, kvm specific parameters here.
Quoted 6 lines
Quoted 2 lines
I'd like to understand what do we gain from that, since opting kvmclock in our out is done by cpuid anyway - no need for a specific machine.
Is that really the case? I thought we were already shipping versions where that CPU feature was enabled by default. If not, I'll happily drop that admittedly clumsy approach above.
Yes, AFAIK, kvmclock is enabled by default, disabled by cpuid-leaf, as in
-cpu kvm64,-kvmclock
Quoted 7 lines
Quoted 3 lines
So your test for cpuid bit before starting kvmclock should already cover it.
No, just the contrary: As kvmclock was always enabled in older versions and the compat machines also expose it, we cannot rely on the flag to enable this new (and therefore 0.15-only) vmstate.
I see. You're not enabling kvmclock functionality, but rather, kvmclock device.
It makes sense then, but I still would prefer it encoded somewhere else (but not a hard constraint anymore). Can't qdev encode it, say, in the value field of the device?
Re: [PATCH 15/15] kvm: x86: Introduce kvmclock device to save/restore its state
Quoted 238 lines · Glauber Costa
Quoted 229 lines · Jan Kiszka
Quoted 228 lines · Glauber Costa
Quoted 216 lines · Jan Kiszka
Quoted 215 lines · Glauber Costa
Quoted 202 lines · Jan Kiszka
If kvmclock is used, which implies the kernel supports it, register a kvmclock device with the sysbus. Its main purpose is to save and restore the kernel state on migration, but this will also allow to visualize it one day.
Signed-off-by Jan Kiszka <[email protected]>CC Glauber Costa <[email protected]>--- Makefile.target | 4 +- hw/kvmclock.c | 125 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ hw/kvmclock.h | 14 ++++++ hw/pc_piix.c | 31 +++++++++++--- 4 files changed, 165 insertions(+), 9 deletions(-) create mode 100644 hw/kvmclock.c create mode 100644 hw/kvmclock.hMakefile.target +2
-2@@ -37,7 +37,7 @@ ifndef CONFIG_HAIKU LIBS+=-lm endif -kvm.o kvm-all.o vhost.o vhost_net.o: QEMU_CFLAGS+=$(KVM_CFLAGS) +kvm.o kvm-all.o vhost.o vhost_net.o kvmclock.o: QEMU_CFLAGS+=$(KVM_CFLAGS) config-target.h: config-target.h-timestamp config-target.h-timestamp: config-target.mak@@ -218,7 +218,7 @@ obj-i386-y += cirrus_vga.o apic.o ioapic.o piix_pci.o obj-i386-y += vmmouse.o vmport.o hpet.o applesmc.o obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o obj-i386-y += debugcon.o multiboot.o -obj-i386-y += pc_piix.o +obj-i386-y += pc_piix.o kvmclock.o obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o # shared objectshw/kvmclock.c new file +125
-0@@ -0,0 +1,125 @@ +/* +* QEMU KVM support, paravirtual clock device +* +* Copyright (C) 2011 Siemens AG +* +* Authors: +* Jan Kiszka <[email protected]> +* +* This work is licensed under the terms of the GNU GPL version 2. +* See the COPYING file in the top-level directory. +* +*/ + +#include "qemu-common.h" +#include "sysemu.h" +#include "sysbus.h" +#include "kvm.h" +#include "kvmclock.h" + +#if defined(CONFIG_KVM_PARA ) && defined(KVM_CAP_ADJUST_CLOCK ) + +#include <linux/kvm.h> +#include <linux/kvm_para.h> + +typedef struct KVMClockState { +SysBusDevice busdev; +uint64_t clock; +bool clock_valid; +}KVMClockState ; + +static void kvmclock_pre_save (void *opaque) +{ +KVMClockState *s = opaque; +struct kvm_clock_data data; +int ret; + +if (s->clock_valid) { +return ; + } + ret =kvm_vm_ioctl (kvm_state,KVM_GET_CLOCK , &data); +if (ret <0 ) { +fprintf (stderr,"KVM_GET_CLOCK failed: %s\n" ,strerror (ret)); + data.clock =0 ; + } + s->clock = data.clock; +/* +* If the VM is stopped, declare the clock state valid to avoid re-reading +* it on next vmsave (which would return a different value). Will be reset +* when the VM is continued. +*/ + s->clock_valid = !vm_running; +} + +static int kvmclock_post_load (void *opaque,int version_id) +{ +KVMClockState *s = opaque; +struct kvm_clock_data data; + + data.clock = s->clock; + data.flags =0 ; +return kvm_vm_ioctl (kvm_state,KVM_SET_CLOCK , &data); +} + +static void kvmclock_vm_state_change (void *opaque,int running,int reason) +{ +KVMClockState *s = opaque; + +if (running) { + s->clock_valid = false; + } +} + +static int kvmclock_init (SysBusDevice *dev) +{ +KVMClockState *s =FROM_SYSBUS (KVMClockState, dev); + +qemu_add_vm_change_state_handler (kvmclock_vm_state_change, s); +return 0 ; +} + +static const VMStateDescription kvmclock_vmsd = { + .name ="kvmclock" , + .version_id =1 , + .minimum_version_id =1 , + .minimum_version_id_old =1 , + .pre_save = kvmclock_pre_save, + .post_load = kvmclock_post_load, + .fields = (VMStateField []) { +VMSTATE_UINT64 (clock, KVMClockState), +VMSTATE_END_OF_LIST () + } +}; + +static SysBusDeviceInfo kvmclock_info = { + .qdev.name ="kvmclock" , + .qdev.size =sizeof (KVMClockState), + .qdev.vmsd = &kvmclock_vmsd, + .qdev.no_user =1 , + .init = kvmclock_init, +}; + +/* Note: Must be called after VCPU initialization. */ +void kvmclock_create (void ) +{ +if (kvm_enabled () && + first_cpu->cpuid_kvm_features & (1ULL <<KVM_FEATURE_CLOCKSOURCE )) { +sysbus_create_simple ("kvmclock" ,-1 ,NULL ); + } +} + +static void kvmclock_register_device (void ) +{ +if (kvm_enabled ()) { +sysbus_register_withprop (&kvmclock_info); + } +} + +device_init (kvmclock_register_device); + +#else /* !(CONFIG_KVM_PARA && KVM_CAP_ADJUST_CLOCK) */ + +void kvmclock_create (void ) +{ +} +#endif /* !(CONFIG_KVM_PARA && KVM_CAP_ADJUST_CLOCK) */ hw/kvmclock.h new file +14
-0@@ -0,0 +1,14 @@ +/* +* QEMU KVM support, paravirtual clock device +* +* Copyright (C) 2011 Siemens AG +* +* Authors: +* Jan Kiszka <[email protected]> +* +* This work is licensed under the terms of the GNU GPL version 2. +* See the COPYING file in the top-level directory. +* +*/ + +void kvmclock_create (void );hw/pc_piix.c +1
-0@@ -32,6 +32,7 @@#include "boards.h" #include "ide.h" #include "kvm.h" +#include "kvmclock.h" #include "sysemu.h" #include "sysbus.h" #include "arch_init.h" hw/pc_piix.c static void pc_init1(ram_addr_t ram_size, · line 72
Quoted 7 lines
hw/pc_piix.c +2
-1@@ -66,7 +67,8 @@ static void pc_init1(ram_addr_t ram_size,const char *kernel_cmdline, const char *initrd_filename, const char *cpu_model, - int pci_enabled) + int pci_enabled, + int kvmclock_enabled)What exactly is your motivation to that ? I think mid/long-term we should be making machine initialization more common among architectures, not introducing more arch specific, or even worse, kvm specific parameters here.
Quoted 6 lines
Quoted 2 lines
I'd like to understand what do we gain from that, since opting kvmclock in our out is done by cpuid anyway - no need for a specific machine.
Is that really the case? I thought we were already shipping versions where that CPU feature was enabled by default. If not, I'll happily drop that admittedly clumsy approach above.
Yes, AFAIK, kvmclock is enabled by default, disabled by cpuid-leaf, as in
-cpu kvm64,-kvmclockQuoted 7 lines
Quoted 3 lines
So your test for cpuid bit before starting kvmclock should already cover it.
No, just the contrary: As kvmclock was always enabled in older versions and the compat machines also expose it, we cannot rely on the flag to enable this new (and therefore 0.15-only) vmstate.
Quoted 6 lines
I see. You're not enabling kvmclock functionality, but rather, kvmclock device.
It makes sense then, but I still would prefer it encoded somewhere else (but not a hard constraint anymore). Can't qdev encode it, say, in the value field of the device?
It would take some machine properties that could be pre-set according to compat requirements. I have such machine-specific properties in mind anyway in order to implement -machine in a more flexible way than proposed so far.
I'll surely clean up the small ugliness this patch introduces to pc_piix.c once there is a better way, I just don't want to let kvmclock wait for that perfect infrastructure.
Jan
Signature · 2 lines
Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [PATCH 15/15] kvm: x86: Introduce kvmclock device to save/restore its state
Quoted 246 lines · Jan Kiszka
Quoted 238 lines · Glauber Costa
Quoted 229 lines · Jan Kiszka
Quoted 228 lines · Glauber Costa
Quoted 216 lines · Jan Kiszka
Quoted 215 lines · Glauber Costa
Quoted 202 lines · Jan Kiszka
If kvmclock is used, which implies the kernel supports it, register a kvmclock device with the sysbus. Its main purpose is to save and restore the kernel state on migration, but this will also allow to visualize it one day.
Signed-off-by Jan Kiszka <[email protected]>CC Glauber Costa <[email protected]>--- Makefile.target | 4 +- hw/kvmclock.c | 125 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ hw/kvmclock.h | 14 ++++++ hw/pc_piix.c | 31 +++++++++++--- 4 files changed, 165 insertions(+), 9 deletions(-) create mode 100644 hw/kvmclock.c create mode 100644 hw/kvmclock.hMakefile.target +2
-2@@ -37,7 +37,7 @@ ifndef CONFIG_HAIKU LIBS+=-lm endif -kvm.o kvm-all.o vhost.o vhost_net.o: QEMU_CFLAGS+=$(KVM_CFLAGS) +kvm.o kvm-all.o vhost.o vhost_net.o kvmclock.o: QEMU_CFLAGS+=$(KVM_CFLAGS) config-target.h: config-target.h-timestamp config-target.h-timestamp: config-target.mak@@ -218,7 +218,7 @@ obj-i386-y += cirrus_vga.o apic.o ioapic.o piix_pci.o obj-i386-y += vmmouse.o vmport.o hpet.o applesmc.o obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o obj-i386-y += debugcon.o multiboot.o -obj-i386-y += pc_piix.o +obj-i386-y += pc_piix.o kvmclock.o obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o # shared objectshw/kvmclock.c new file +125
-0@@ -0,0 +1,125 @@ +/* +* QEMU KVM support, paravirtual clock device +* +* Copyright (C) 2011 Siemens AG +* +* Authors: +* Jan Kiszka <[email protected]> +* +* This work is licensed under the terms of the GNU GPL version 2. +* See the COPYING file in the top-level directory. +* +*/ + +#include "qemu-common.h" +#include "sysemu.h" +#include "sysbus.h" +#include "kvm.h" +#include "kvmclock.h" + +#if defined(CONFIG_KVM_PARA ) && defined(KVM_CAP_ADJUST_CLOCK ) + +#include <linux/kvm.h> +#include <linux/kvm_para.h> + +typedef struct KVMClockState { +SysBusDevice busdev; +uint64_t clock; +bool clock_valid; +}KVMClockState ; + +static void kvmclock_pre_save (void *opaque) +{ +KVMClockState *s = opaque; +struct kvm_clock_data data; +int ret; + +if (s->clock_valid) { +return ; + } + ret =kvm_vm_ioctl (kvm_state,KVM_GET_CLOCK , &data); +if (ret <0 ) { +fprintf (stderr,"KVM_GET_CLOCK failed: %s\n" ,strerror (ret)); + data.clock =0 ; + } + s->clock = data.clock; +/* +* If the VM is stopped, declare the clock state valid to avoid re-reading +* it on next vmsave (which would return a different value). Will be reset +* when the VM is continued. +*/ + s->clock_valid = !vm_running; +} + +static int kvmclock_post_load (void *opaque,int version_id) +{ +KVMClockState *s = opaque; +struct kvm_clock_data data; + + data.clock = s->clock; + data.flags =0 ; +return kvm_vm_ioctl (kvm_state,KVM_SET_CLOCK , &data); +} + +static void kvmclock_vm_state_change (void *opaque,int running,int reason) +{ +KVMClockState *s = opaque; + +if (running) { + s->clock_valid = false; + } +} + +static int kvmclock_init (SysBusDevice *dev) +{ +KVMClockState *s =FROM_SYSBUS (KVMClockState, dev); + +qemu_add_vm_change_state_handler (kvmclock_vm_state_change, s); +return 0 ; +} + +static const VMStateDescription kvmclock_vmsd = { + .name ="kvmclock" , + .version_id =1 , + .minimum_version_id =1 , + .minimum_version_id_old =1 , + .pre_save = kvmclock_pre_save, + .post_load = kvmclock_post_load, + .fields = (VMStateField []) { +VMSTATE_UINT64 (clock, KVMClockState), +VMSTATE_END_OF_LIST () + } +}; + +static SysBusDeviceInfo kvmclock_info = { + .qdev.name ="kvmclock" , + .qdev.size =sizeof (KVMClockState), + .qdev.vmsd = &kvmclock_vmsd, + .qdev.no_user =1 , + .init = kvmclock_init, +}; + +/* Note: Must be called after VCPU initialization. */ +void kvmclock_create (void ) +{ +if (kvm_enabled () && + first_cpu->cpuid_kvm_features & (1ULL <<KVM_FEATURE_CLOCKSOURCE )) { +sysbus_create_simple ("kvmclock" ,-1 ,NULL ); + } +} + +static void kvmclock_register_device (void ) +{ +if (kvm_enabled ()) { +sysbus_register_withprop (&kvmclock_info); + } +} + +device_init (kvmclock_register_device); + +#else /* !(CONFIG_KVM_PARA && KVM_CAP_ADJUST_CLOCK) */ + +void kvmclock_create (void ) +{ +} +#endif /* !(CONFIG_KVM_PARA && KVM_CAP_ADJUST_CLOCK) */ hw/kvmclock.h new file +14
-0@@ -0,0 +1,14 @@ +/* +* QEMU KVM support, paravirtual clock device +* +* Copyright (C) 2011 Siemens AG +* +* Authors: +* Jan Kiszka <[email protected]> +* +* This work is licensed under the terms of the GNU GPL version 2. +* See the COPYING file in the top-level directory. +* +*/ + +void kvmclock_create (void );hw/pc_piix.c +1
-0@@ -32,6 +32,7 @@#include "boards.h" #include "ide.h" #include "kvm.h" +#include "kvmclock.h" #include "sysemu.h" #include "sysbus.h" #include "arch_init.h" hw/pc_piix.c static void pc_init1(ram_addr_t ram_size, · line 72
Quoted 7 lines
hw/pc_piix.c +2
-1@@ -66,7 +67,8 @@ static void pc_init1(ram_addr_t ram_size,const char *kernel_cmdline, const char *initrd_filename, const char *cpu_model, - int pci_enabled) + int pci_enabled, + int kvmclock_enabled)What exactly is your motivation to that ? I think mid/long-term we should be making machine initialization more common among architectures, not introducing more arch specific, or even worse, kvm specific parameters here.
Quoted 6 lines
Quoted 2 lines
I'd like to understand what do we gain from that, since opting kvmclock in our out is done by cpuid anyway - no need for a specific machine.
Is that really the case? I thought we were already shipping versions where that CPU feature was enabled by default. If not, I'll happily drop that admittedly clumsy approach above.
Yes, AFAIK, kvmclock is enabled by default, disabled by cpuid-leaf, as in
-cpu kvm64,-kvmclockQuoted 7 lines
Quoted 3 lines
So your test for cpuid bit before starting kvmclock should already cover it.
No, just the contrary: As kvmclock was always enabled in older versions and the compat machines also expose it, we cannot rely on the flag to enable this new (and therefore 0.15-only) vmstate.
Quoted 6 lines
I see. You're not enabling kvmclock functionality, but rather, kvmclock device.
It makes sense then, but I still would prefer it encoded somewhere else (but not a hard constraint anymore). Can't qdev encode it, say, in the value field of the device?
Quoted 8 lines
It would take some machine properties that could be pre-set according to compat requirements. I have such machine-specific properties in mind anyway in order to implement -machine in a more flexible way than proposed so far.
I'll surely clean up the small ugliness this patch introduces to
pc_piix.conce there is a better way, I just don't want to let kvmclock wait for that perfect infrastructure.
Perfect is the enemy of good, I'm all for it.
Re: [PATCH 15/15] kvm: x86: Introduce kvmclock device to save/restore its state
In reply to Glauber CostaQuoted 6 lines · Glauber Costa
Quoted 1 line
What exactly is your motivation to that ? I think mid/long-term we should be making machine initialization more common among architectures, not introducing more arch specific, or even worse, kvm specific parameters here.
A general note: there are several ways of being "kvm specific"; one of them is being tied to the implementation of kvm as the implementation of a virtual cpu in qemu. Another, with an example here, is a cpu feature that is only (or at first) present in kvm, but in principle may be supported by any cpu implementation, like tcg or a real cpu. It's important not to confuse the two; only the first needs all those hooks tied into the -accel infrastructure.
Signature · 1 line
error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 15/15] kvm: x86: Introduce kvmclock device to save/restore its state
In reply to Jan KiszkaQuoted 33 lines · Jan Kiszka
If kvmclock is used, which implies the kernel supports it, register a kvmclock device with the sysbus. Its main purpose is to save and restore the kernel state on migration, but this will also allow to visualize it one day.
Signed-off-by Jan Kiszka <[email protected]>CC Glauber Costa <[email protected]>---
Makefile.target | 4 +-
hw/kvmclock.c | 125 +++++++++++++++++++++++++++++++++++++++++++++++++++++++hw/kvmclock.h| 14 ++++++
hw/pc_piix.c | 31 +++++++++++---4 files changed, 165 insertions(+), 9 deletions(-)create mode 100644
hw/kvmclock.c
create mode 100644hw/kvmclock.hMakefile.target +0
-0@@ -37,7 +37,7 @@ ifndef CONFIG_HAIKULIBS+=-lm
endifMakefile.target +1
-1-kvm.o kvm-all.o vhost.o vhost_net.o: QEMU_CFLAGS+=$(KVM_CFLAGS) +kvm.o kvm-all.o vhost.o vhost_net.o kvmclock.o: QEMU_CFLAGS+=$(KVM_CFLAGS)config-target.h: config-target.h-timestamp
config-target.h-timestamp: config-target.makMakefile.target +0
-0@@ -218,7 +218,7 @@ obj-i386-y += cirrus_vga.o apic.o ioapic.o piix_pci.oobj-i386-y += vmmouse.o vmport.o hpet.o applesmc.o obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o obj-i386-y += debugcon.o multiboot.o
Makefile.target obj-i386-y += cirrus_vga.o apic.o ioapic.o piix_pci.o · line 221
Quoted 2 lines
Makefile.target +1
-1-obj-i386-y += pc_piix.o +obj-i386-y += pc_piix.o kvmclock.o
Please build kvmclock.o conditionally to CONFIG_something...
Quoted 108 lines
obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o
# shared objectshw/kvmclock.c new file +99
-0@@ -0,0 +1,125 @@ +/* +* QEMU KVM support, paravirtual clock device +* +* Copyright (C) 2011 Siemens AG +* +* Authors: +* Jan Kiszka <[email protected]> +* +* This work is licensed under the terms of the GNU GPL version 2. +* See the COPYING file in the top-level directory. +* +*/ + +#include "qemu-common.h" +#include "sysemu.h" +#include "sysbus.h" +#include "kvm.h" +#include "kvmclock.h" + +#if defined(CONFIG_KVM_PARA ) && defined(KVM_CAP_ADJUST_CLOCK ) + +#include <linux/kvm.h> +#include <linux/kvm_para.h> + +typedef struct KVMClockState { + SysBusDevice busdev; + uint64_t clock; + bool clock_valid; +}KVMClockState ; + +static void kvmclock_pre_save (void *opaque) +{ + KVMClockState *s = opaque; + struct kvm_clock_data data; + int ret; + + if (s->clock_valid) { + return; + } + ret = kvm_vm_ioctl(kvm_state, KVM_GET_CLOCK, &data); + if (ret < 0) { + fprintf(stderr, "KVM_GET_CLOCK failed: %s\n", strerror(ret)); + data.clock = 0; + } + s->clock = data.clock; + /* + * If the VM is stopped, declare the clock state valid to avoid re-reading + * it on next vmsave (which would return a different value). Will be reset + * when the VM is continued. + */ + s->clock_valid = !vm_running; +} + +static int kvmclock_post_load (void *opaque,int version_id) +{ + KVMClockState *s = opaque; + struct kvm_clock_data data; + + data.clock = s->clock; + data.flags = 0; + return kvm_vm_ioctl(kvm_state, KVM_SET_CLOCK, &data); +} + +static void kvmclock_vm_state_change (void *opaque,int running,int reason) +{ + KVMClockState *s = opaque; + + if (running) { + s->clock_valid = false; + } +} + +static int kvmclock_init (SysBusDevice *dev) +{ + KVMClockState *s = FROM_SYSBUS(KVMClockState, dev); + + qemu_add_vm_change_state_handler(kvmclock_vm_state_change, s); + return 0; +} + +static const VMStateDescription kvmclock_vmsd = { + .name = "kvmclock", + .version_id = 1, + .minimum_version_id = 1, + .minimum_version_id_old = 1, + .pre_save = kvmclock_pre_save, + .post_load = kvmclock_post_load, + .fields = (VMStateField[]) { + VMSTATE_UINT64(clock, KVMClockState), + VMSTATE_END_OF_LIST() + } +}; + +static SysBusDeviceInfo kvmclock_info = { + .qdev.name = "kvmclock", + .qdev.size = sizeof(KVMClockState), + .qdev.vmsd = &kvmclock_vmsd, + .qdev.no_user = 1, + .init = kvmclock_init,
hw/kvmclock.c line 109
Quoted 10 lines
hw/kvmclock.c new file +10
-0+}; + +/* Note: Must be called after VCPU initialization. */ +void kvmclock_create (void ) +{ + if (kvm_enabled() && + first_cpu->cpuid_kvm_features & (1ULL << KVM_FEATURE_CLOCKSOURCE)) { + sysbus_create_simple("kvmclock", -1, NULL); + } +}
... and with this moved to a header as a static inline function, it should be possible to use sysbus_try_create() (coming soon) to try to create the device. Then it's not fatal if the device can't be created, that just means that the capability was not available at build time.
Re: [Qemu-devel] [PATCH 15/15] kvm: x86: Introduce kvmclock device to save/restore its state
Quoted 154 lines · Blue Swirl
Quoted 30 lines · Jan Kiszka
If kvmclock is used, which implies the kernel supports it, register a kvmclock device with the sysbus. Its main purpose is to save and restore the kernel state on migration, but this will also allow to visualize it one day.
Signed-off-by Jan Kiszka <[email protected]>CC Glauber Costa <[email protected]>--- Makefile.target | 4 +- hw/kvmclock.c | 125 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ hw/kvmclock.h | 14 ++++++ hw/pc_piix.c | 31 +++++++++++--- 4 files changed, 165 insertions(+), 9 deletions(-) create mode 100644 hw/kvmclock.c create mode 100644 hw/kvmclock.hMakefile.target +1
-1@@ -37,7 +37,7 @@ ifndef CONFIG_HAIKU LIBS+=-lm endif -kvm.o kvm-all.o vhost.o vhost_net.o: QEMU_CFLAGS+=$(KVM_CFLAGS) +kvm.o kvm-all.o vhost.o vhost_net.o kvmclock.o: QEMU_CFLAGS+=$(KVM_CFLAGS) config-target.h: config-target.h-timestamp config-target.h-timestamp: config-target.makMakefile.target obj-i386-y += cirrus_vga.o apic.o ioapic.o piix_pci.o · line 221
Quoted 5 lines
Makefile.target +1
-1@@ -218,7 +218,7 @@ obj-i386-y += cirrus_vga.o apic.o ioapic.o piix_pci.o obj-i386-y += vmmouse.o vmport.o hpet.o applesmc.o obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o obj-i386-y += debugcon.o multiboot.o -obj-i386-y += pc_piix.o +obj-i386-y += pc_piix.o kvmclock.oPlease build kvmclock.o conditionally to
CONFIG_something...Quoted 115 lines
Makefile.target +0
-0obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o # shared objectshw/kvmclock.c new file +106
-0@@ -0,0 +1,125 @@ +/* +* QEMU KVM support, paravirtual clock device +* +* Copyright (C) 2011 Siemens AG +* +* Authors: +* Jan Kiszka <[email protected]> +* +* This work is licensed under the terms of the GNU GPL version 2. +* See the COPYING file in the top-level directory. +* +*/ + +#include "qemu-common.h" +#include "sysemu.h" +#include "sysbus.h" +#include "kvm.h" +#include "kvmclock.h" + +#if defined(CONFIG_KVM_PARA ) && defined(KVM_CAP_ADJUST_CLOCK ) + +#include <linux/kvm.h> +#include <linux/kvm_para.h> + +typedef struct KVMClockState { +SysBusDevice busdev; +uint64_t clock; +bool clock_valid; +}KVMClockState ; + +static void kvmclock_pre_save (void *opaque) +{ +KVMClockState *s = opaque; +struct kvm_clock_data data; +int ret; + +if (s->clock_valid) { +return ; + } + ret =kvm_vm_ioctl (kvm_state,KVM_GET_CLOCK , &data); +if (ret <0 ) { +fprintf (stderr,"KVM_GET_CLOCK failed: %s\n" ,strerror (ret)); + data.clock =0 ; + } + s->clock = data.clock; +/* +* If the VM is stopped, declare the clock state valid to avoid re-reading +* it on next vmsave (which would return a different value). Will be reset +* when the VM is continued. +*/ + s->clock_valid = !vm_running; +} + +static int kvmclock_post_load (void *opaque,int version_id) +{ +KVMClockState *s = opaque; +struct kvm_clock_data data; + + data.clock = s->clock; + data.flags =0 ; +return kvm_vm_ioctl (kvm_state,KVM_SET_CLOCK , &data); +} + +static void kvmclock_vm_state_change (void *opaque,int running,int reason) +{ +KVMClockState *s = opaque; + +if (running) { + s->clock_valid = false; + } +} + +static int kvmclock_init (SysBusDevice *dev) +{ +KVMClockState *s =FROM_SYSBUS (KVMClockState, dev); + +qemu_add_vm_change_state_handler (kvmclock_vm_state_change, s); +return 0 ; +} + +static const VMStateDescription kvmclock_vmsd = { + .name ="kvmclock" , + .version_id =1 , + .minimum_version_id =1 , + .minimum_version_id_old =1 , + .pre_save = kvmclock_pre_save, + .post_load = kvmclock_post_load, + .fields = (VMStateField []) { +VMSTATE_UINT64 (clock, KVMClockState), +VMSTATE_END_OF_LIST () + } +}; + +static SysBusDeviceInfo kvmclock_info = { + .qdev.name ="kvmclock" , + .qdev.size =sizeof (KVMClockState), + .qdev.vmsd = &kvmclock_vmsd, + .qdev.no_user =1 , + .init = kvmclock_init, +}; + +/* Note: Must be called after VCPU initialization. */ +void kvmclock_create (void ) +{ +if (kvm_enabled () && + first_cpu->cpuid_kvm_features & (1ULL <<KVM_FEATURE_CLOCKSOURCE )) {
Quoted 8 lines
hw/kvmclock.c line 109
Quoted 3 lines
hw/kvmclock.c new file +3
-0+sysbus_create_simple ("kvmclock" ,-1 ,NULL ); + } +}... and with this moved to a header as a static inline function, it should be possible to use
sysbus_try_create()(coming soon) to try to create the device. Then it's not fatal if the device can't be created, that just means that the capability was not available at build time.
I played with this, and while it is generally a nice thing, it doesn't help us here. We would just push the logic around, from kvmclock.c to the header or even to configure (KVM_FEATURE_CLOCKSOURCE is not unconditionally available).
I rather hope we finally agree on merging the required kvm headers into qemu so that all this usually broken #ifdef KVM_CAP_* can be removed.
Jan