| Message ID | 20090603120756.GC11384@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Gleb Natapov wrote: > kvm_*_mpstate() cannot be called from kvm_arch_*_registers() > since kvm_arch_*_registers() sometimes called from io thread, but > kvm_*_mpstate() can be called only by cpu thread. > > I really dislike vcpu functions to be called from outside the vcpu thread. Who are the callers? > Signed-off-by: Gleb Natapov <gleb@redhat.com> > diff --git a/qemu-kvm.c b/qemu-kvm.c > index 68d3b92..7ed1e06 100644 > --- a/qemu-kvm.c > +++ b/qemu-kvm.c > @@ -1431,26 +1431,3 @@ void qemu_kvm_cpu_stop(CPUState *env) > if (kvm_enabled()) > env->kvm_cpu_state.stopped = 1; > } > - > -void kvm_arch_get_registers(CPUState *env) > -{ > - kvm_save_registers(env); > - kvm_save_mpstate(env); > -} > - > -void kvm_arch_put_registers(CPUState *env) > -{ > - kvm_load_registers(env); > - kvm_load_mpstate(env); > -} > - > - > -void cpu_synchronize_state(CPUState *env, int modified) > -{ > - if (kvm_enabled()) { > - if (modified) > - kvm_arch_put_registers(env); > - else > - kvm_arch_get_registers(env); > - } > -} > diff --git a/qemu-kvm.h b/qemu-kvm.h > index 725589b..5d47e88 100644 > --- a/qemu-kvm.h > +++ b/qemu-kvm.h > @@ -216,10 +216,25 @@ int kvm_log_stop(target_phys_addr_t phys_addr, target_phys_addr_t len); > > static inline int kvm_sync_vcpus(void) { return 0; } > > -void kvm_arch_get_registers(CPUState *env); > -void kvm_arch_put_registers(CPUState *env); > +static inline void kvm_arch_get_registers(CPUState *env) > +{ > + kvm_save_registers(env); > +} > > -void cpu_synchronize_state(CPUState *env, int modified); > +static inline void kvm_arch_put_registers(CPUState *env) > +{ > + kvm_load_registers(env); > +} > + > +static inline void cpu_synchronize_state(CPUState *env, int modified) > +{ > + if (kvm_enabled()) { > + if (modified) > + kvm_arch_put_registers(env); > + else > + kvm_arch_get_registers(env); > + } > +} > > uint32_t kvm_arch_get_supported_cpuid(CPUState *env, uint32_t function, > int reg); > diff --git a/target-i386/machine.c b/target-i386/machine.c > index f280d3d..07df1e1 100644 > --- a/target-i386/machine.c > +++ b/target-i386/machine.c > @@ -32,7 +32,10 @@ void cpu_save(QEMUFile *f, void *opaque) > int32_t pending_irq; > int i, bit; > > - cpu_synchronize_state(env, 0); > + if (kvm_enabled()) { > + kvm_save_registers(env); > + kvm_save_mpstate(env); > + } > > for(i = 0; i < CPU_NB_REGS; i++) > qemu_put_betls(f, &env->regs[i]); > -- > Gleb. >
On Wed, Jun 03, 2009 at 03:16:46PM +0300, Avi Kivity wrote: > Gleb Natapov wrote: >> kvm_*_mpstate() cannot be called from kvm_arch_*_registers() >> since kvm_arch_*_registers() sometimes called from io thread, but >> kvm_*_mpstate() can be called only by cpu thread. >> >> > > I really dislike vcpu functions to be called from outside the vcpu > thread. Who are the callers? > monitor.c. May be others (git grep cpu_synchronize_state). But kvm_save_registers() does the right thing and calls another function on vcpu thread. >> Signed-off-by: Gleb Natapov <gleb@redhat.com> >> diff --git a/qemu-kvm.c b/qemu-kvm.c >> index 68d3b92..7ed1e06 100644 >> --- a/qemu-kvm.c >> +++ b/qemu-kvm.c >> @@ -1431,26 +1431,3 @@ void qemu_kvm_cpu_stop(CPUState *env) >> if (kvm_enabled()) >> env->kvm_cpu_state.stopped = 1; >> } >> - >> -void kvm_arch_get_registers(CPUState *env) >> -{ >> - kvm_save_registers(env); >> - kvm_save_mpstate(env); >> -} >> - >> -void kvm_arch_put_registers(CPUState *env) >> -{ >> - kvm_load_registers(env); >> - kvm_load_mpstate(env); >> -} >> - >> - >> -void cpu_synchronize_state(CPUState *env, int modified) >> -{ >> - if (kvm_enabled()) { >> - if (modified) >> - kvm_arch_put_registers(env); >> - else >> - kvm_arch_get_registers(env); >> - } >> -} >> diff --git a/qemu-kvm.h b/qemu-kvm.h >> index 725589b..5d47e88 100644 >> --- a/qemu-kvm.h >> +++ b/qemu-kvm.h >> @@ -216,10 +216,25 @@ int kvm_log_stop(target_phys_addr_t phys_addr, target_phys_addr_t len); >> static inline int kvm_sync_vcpus(void) { return 0; } >> -void kvm_arch_get_registers(CPUState *env); >> -void kvm_arch_put_registers(CPUState *env); >> +static inline void kvm_arch_get_registers(CPUState *env) >> +{ >> + kvm_save_registers(env); >> +} >> -void cpu_synchronize_state(CPUState *env, int modified); >> +static inline void kvm_arch_put_registers(CPUState *env) >> +{ >> + kvm_load_registers(env); >> +} >> + >> +static inline void cpu_synchronize_state(CPUState *env, int modified) >> +{ >> + if (kvm_enabled()) { >> + if (modified) >> + kvm_arch_put_registers(env); >> + else >> + kvm_arch_get_registers(env); >> + } >> +} >> uint32_t kvm_arch_get_supported_cpuid(CPUState *env, uint32_t >> function, >> int reg); >> diff --git a/target-i386/machine.c b/target-i386/machine.c >> index f280d3d..07df1e1 100644 >> --- a/target-i386/machine.c >> +++ b/target-i386/machine.c >> @@ -32,7 +32,10 @@ void cpu_save(QEMUFile *f, void *opaque) >> int32_t pending_irq; >> int i, bit; >> - cpu_synchronize_state(env, 0); >> + if (kvm_enabled()) { >> + kvm_save_registers(env); >> + kvm_save_mpstate(env); >> + } >> for(i = 0; i < CPU_NB_REGS; i++) >> qemu_put_betls(f, &env->regs[i]); >> -- >> Gleb. >> > > > -- > Do not meddle in the internals of kernels, for they are subtle and quick to panic. -- Gleb. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Gleb Natapov wrote: > kvm_*_mpstate() cannot be called from kvm_arch_*_registers() > since kvm_arch_*_registers() sometimes called from io thread, but > kvm_*_mpstate() can be called only by cpu thread. > Applied, thanks.
diff --git a/qemu-kvm.c b/qemu-kvm.c index 68d3b92..7ed1e06 100644 --- a/qemu-kvm.c +++ b/qemu-kvm.c @@ -1431,26 +1431,3 @@ void qemu_kvm_cpu_stop(CPUState *env) if (kvm_enabled()) env->kvm_cpu_state.stopped = 1; } - -void kvm_arch_get_registers(CPUState *env) -{ - kvm_save_registers(env); - kvm_save_mpstate(env); -} - -void kvm_arch_put_registers(CPUState *env) -{ - kvm_load_registers(env); - kvm_load_mpstate(env); -} - - -void cpu_synchronize_state(CPUState *env, int modified) -{ - if (kvm_enabled()) { - if (modified) - kvm_arch_put_registers(env); - else - kvm_arch_get_registers(env); - } -} diff --git a/qemu-kvm.h b/qemu-kvm.h index 725589b..5d47e88 100644 --- a/qemu-kvm.h +++ b/qemu-kvm.h @@ -216,10 +216,25 @@ int kvm_log_stop(target_phys_addr_t phys_addr, target_phys_addr_t len); static inline int kvm_sync_vcpus(void) { return 0; } -void kvm_arch_get_registers(CPUState *env); -void kvm_arch_put_registers(CPUState *env); +static inline void kvm_arch_get_registers(CPUState *env) +{ + kvm_save_registers(env); +} -void cpu_synchronize_state(CPUState *env, int modified); +static inline void kvm_arch_put_registers(CPUState *env) +{ + kvm_load_registers(env); +} + +static inline void cpu_synchronize_state(CPUState *env, int modified) +{ + if (kvm_enabled()) { + if (modified) + kvm_arch_put_registers(env); + else + kvm_arch_get_registers(env); + } +} uint32_t kvm_arch_get_supported_cpuid(CPUState *env, uint32_t function, int reg); diff --git a/target-i386/machine.c b/target-i386/machine.c index f280d3d..07df1e1 100644 --- a/target-i386/machine.c +++ b/target-i386/machine.c @@ -32,7 +32,10 @@ void cpu_save(QEMUFile *f, void *opaque) int32_t pending_irq; int i, bit; - cpu_synchronize_state(env, 0); + if (kvm_enabled()) { + kvm_save_registers(env); + kvm_save_mpstate(env); + } for(i = 0; i < CPU_NB_REGS; i++) qemu_put_betls(f, &env->regs[i]);
kvm_*_mpstate() cannot be called from kvm_arch_*_registers() since kvm_arch_*_registers() sometimes called from io thread, but kvm_*_mpstate() can be called only by cpu thread. Signed-off-by: Gleb Natapov <gleb@redhat.com> -- Gleb. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html