On Thu, Nov 12, 2009 at 12:33:07AM +0100, Jan Kiszka wrote:
> Gleb Natapov wrote:
> > mp_state, unlike other cpu state, can be changed not only from vcpu
> > context it belongs to, but by other vcpus too. That makes its loading
> > from kernel/saving back not safe if mp_state value is changed inside
> > kernel between load and save. For example vcpu 1 loads mp_sate into
> > user-space and the state is RUNNING, vcpu 0 sends INIT/SIPI to vcpu 1
> > so in-kernel mp_sate becomes SIPI, vcpu 1 save user-space copy into
> > kernel and calls vcpu_run(). SIPI sate is lost.
> >
> > The patch copies mp_sate into kernel only when it is knows that
> > int-kernel value is outdated. This happens on reset and vmload.
>
> Just stumbled over this commit as it breaks kvm-disabled builds
> (layering violations...). I guess we need this upstream too once the
> irqchips go in. Or do your patches, Glauber, already include this? If
> not, anyone already thought about how to move the kvm logic behind
> qemu's state sync curtain?
It does not, since upstream qemu still does not support smp.
I basically did not worried about it.
>
> Jan
>
> >
> > Signed-off-by: Gleb Natapov <gleb@xxxxxxxxxx>
> > ---
> > hw/apic.c | 1 +
> > monitor.c | 2 ++
> > qemu-kvm.c | 9 ++++-----
> > qemu-kvm.h | 1 -
> > target-i386/machine.c | 3 +++
> > 5 files changed, 10 insertions(+), 6 deletions(-)
> >
> > diff --git a/hw/apic.c b/hw/apic.c
> > index 2952675..7244449 100644
> > --- a/hw/apic.c
> > +++ b/hw/apic.c
> > @@ -512,6 +512,7 @@ void apic_init_reset(CPUState *env)
> > if (kvm_enabled() && qemu_kvm_irqchip_in_kernel()) {
> > env->mp_state
> > = env->halted ? KVM_MP_STATE_UNINITIALIZED : KVM_MP_STATE_RUNNABLE;
> > + kvm_load_mpstate(env);
> > }
> > #endif
> > }
> > diff --git a/monitor.c b/monitor.c
> > index 7f0f5a9..dd8f2ca 100644
> > --- a/monitor.c
> > +++ b/monitor.c
> > @@ -350,6 +350,7 @@ static CPUState *mon_get_cpu(void)
> > mon_set_cpu(0);
> > }
> > cpu_synchronize_state(cur_mon->mon_cpu);
> > + kvm_save_mpstate(cur_mon->mon_cpu);
> > return cur_mon->mon_cpu;
> > }
> >
> > @@ -377,6 +378,7 @@ static void do_info_cpus(Monitor *mon)
> >
> > for(env = first_cpu; env != NULL; env = env->next_cpu) {
> > cpu_synchronize_state(env);
> > + kvm_save_mpstate(env);
> > monitor_printf(mon, "%c CPU #%d:",
> > (env == mon->mon_cpu) ? '*' : ' ',
> > env->cpu_index);
> > diff --git a/qemu-kvm.c b/qemu-kvm.c
> > index a104ab8..267222d 100644
> > --- a/qemu-kvm.c
> > +++ b/qemu-kvm.c
> > @@ -1609,11 +1609,6 @@ static void on_vcpu(CPUState *env, void (*func)(void *data), void *data)
> > void kvm_arch_get_registers(CPUState *env)
> > {
> > kvm_arch_save_regs(env);
> > - kvm_arch_save_mpstate(env);
> > -#ifdef KVM_CAP_MP_STATE
> > - if (kvm_irqchip_in_kernel(kvm_context))
> > - env->halted = (env->mp_state == KVM_MP_STATE_HALTED);
> > -#endif
> > }
> >
> > static void do_kvm_cpu_synchronize_state(void *_env)
> > @@ -1707,6 +1702,10 @@ static void kvm_do_save_mpstate(void *_env)
> > CPUState *env = _env;
> >
> > kvm_arch_save_mpstate(env);
> > +#ifdef KVM_CAP_MP_STATE
> > + if (kvm_irqchip_in_kernel(kvm_context))
> > + env->halted = (env->mp_state == KVM_MP_STATE_HALTED);
> > +#endif
> > }
> >
> > void kvm_save_mpstate(CPUState *env)
> > diff --git a/qemu-kvm.h b/qemu-kvm.h
> > index d6748c7..e2a87b8 100644
> > --- a/qemu-kvm.h
> > +++ b/qemu-kvm.h
> > @@ -1186,7 +1186,6 @@ void kvm_arch_get_registers(CPUState *env);
> > static inline void kvm_arch_put_registers(CPUState *env)
> > {
> > kvm_load_registers(env);
> > - kvm_load_mpstate(env);
> > }
> >
> > void kvm_cpu_synchronize_state(CPUState *env);
> > diff --git a/target-i386/machine.c b/target-i386/machine.c
> > index e640dad..16d9c57 100644
> > --- a/target-i386/machine.c
> > +++ b/target-i386/machine.c
> > @@ -324,6 +324,7 @@ static void cpu_pre_save(void *opaque)
> > int i, bit;
> >
> > cpu_synchronize_state(env);
> > + kvm_save_mpstate(env);
> >
> > /* FPU */
> > env->fpus_vmstate = (env->fpus & ~0x3800) | (env->fpstt & 0x7) << 11;
> > @@ -385,6 +386,8 @@ static int cpu_post_load(void *opaque, int version_id)
> > }
> >
> > tlb_flush(env, 1);
> > + kvm_load_mpstate(env);
> > +
> > return 0;
> > }
> >
>
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
