From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [PATCH 2/2] Don't sync mpstate to/from kernel when unneeded. Date: Thu, 12 Nov 2009 00:33:07 +0100 Message-ID: <4AFB49B3.3090000@web.de> References: <1255528352-21171-1-git-send-email-gleb@redhat.com> <1255528352-21171-2-git-send-email-gleb@redhat.com> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enig1039F84D7D3A997CAB698A91" Cc: mtosatti@redhat.com, kvm@vger.kernel.org, Glauber Costa To: Gleb Natapov Return-path: Received: from fmmailgate03.web.de ([217.72.192.234]:56868 "EHLO fmmailgate03.web.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1759556AbZKKXc7 (ORCPT ); Wed, 11 Nov 2009 18:32:59 -0500 In-Reply-To: <1255528352-21171-2-git-send-email-gleb@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig1039F84D7D3A997CAB698A91 Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable 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. >=20 > 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? Jan >=20 > Signed-off-by: Gleb Natapov > --- > 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(-) >=20 > 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 > =3D env->halted ? KVM_MP_STATE_UNINITIALIZED : KVM_MP_STAT= E_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; > } > =20 > @@ -377,6 +378,7 @@ static void do_info_cpus(Monitor *mon) > =20 > for(env =3D first_cpu; env !=3D NULL; env =3D env->next_cpu) { > cpu_synchronize_state(env); > + kvm_save_mpstate(env); > monitor_printf(mon, "%c CPU #%d:", > (env =3D=3D 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 =3D (env->mp_state =3D=3D KVM_MP_STATE_HALTED); > -#endif > } > =20 > static void do_kvm_cpu_synchronize_state(void *_env) > @@ -1707,6 +1702,10 @@ static void kvm_do_save_mpstate(void *_env) > CPUState *env =3D _env; > =20 > kvm_arch_save_mpstate(env); > +#ifdef KVM_CAP_MP_STATE > + if (kvm_irqchip_in_kernel(kvm_context)) > + env->halted =3D (env->mp_state =3D=3D KVM_MP_STATE_HALTED); > +#endif > } > =20 > 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); > } > =20 > 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; > =20 > cpu_synchronize_state(env); > + kvm_save_mpstate(env); > =20 > /* FPU */ > env->fpus_vmstate =3D (env->fpus & ~0x3800) | (env->fpstt & 0x7) <= < 11; > @@ -385,6 +386,8 @@ static int cpu_post_load(void *opaque, int version_= id) > } > =20 > tlb_flush(env, 1); > + kvm_load_mpstate(env); > + > return 0; > } > =20 --------------enig1039F84D7D3A997CAB698A91 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.9 (GNU/Linux) Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org iEYEARECAAYFAkr7SbcACgkQitSsb3rl5xRbmgCg4fdC7ySM9lTnbsVdvDXoJxDu iQsAniqmaYqNL7arSNzSY75qamxqwPCT =tyq/ -----END PGP SIGNATURE----- --------------enig1039F84D7D3A997CAB698A91--