From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [PATCH] KVM: x86: Convert INIT and SIPI signals into synchronously handled requests Date: Mon, 04 Mar 2013 21:50:20 +0100 Message-ID: <5135090C.8040202@web.de> References: <5133B0D7.8070203@web.de> <5134AF97.8010505@redhat.com> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="----enig2USBVTETFSFVFDWNLRFDL" Cc: Gleb Natapov , Marcelo Tosatti , kvm To: Paolo Bonzini Return-path: Received: from mout.web.de ([212.227.15.4]:52096 "EHLO mout.web.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758717Ab3CDUu1 (ORCPT ); Mon, 4 Mar 2013 15:50:27 -0500 In-Reply-To: <5134AF97.8010505@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: This is an OpenPGP/MIME signed message (RFC 4880 and 3156) ------enig2USBVTETFSFVFDWNLRFDL Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable On 2013-03-04 15:28, Paolo Bonzini wrote: > Il 03/03/2013 21:21, Jan Kiszka ha scritto: >> From: Jan Kiszka >> >> A VCPU sending INIT or SIPI to some other VCPU races for setting the >> remote VCPU's mp_state. When we were unlucky, KVM_MP_STATE_INIT_RECEIV= ED >> was overwritten by kvm_emulate_halt and, thus, got lost. >> >> Fix this by raising requests on the sender side that will then be >> handled synchronously over the target VCPU context. >> >> Signed-off-by: Jan Kiszka >> --- >> >> Turned out to be simpler than expected. I'm no longer able to reproduc= e >> the race I saw before. >> >> arch/x86/kvm/lapic.c | 9 ++++----- >> arch/x86/kvm/x86.c | 16 +++++++++++++++- >> include/linux/kvm_host.h | 2 ++ >> 3 files changed, 21 insertions(+), 6 deletions(-) >> >> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c >> index 02b51dd..be1e37a 100644 >> --- a/arch/x86/kvm/lapic.c >> +++ b/arch/x86/kvm/lapic.c >> @@ -731,8 +731,7 @@ static int __apic_accept_irq(struct kvm_lapic *api= c, int delivery_mode, >> case APIC_DM_INIT: >> if (!trig_mode || level) { >> result =3D 1; >> - vcpu->arch.mp_state =3D KVM_MP_STATE_INIT_RECEIVED; >> - kvm_make_request(KVM_REQ_EVENT, vcpu); >> + kvm_make_request(KVM_REQ_INIT, vcpu); >> kvm_vcpu_kick(vcpu); >> } else { >> apic_debug("Ignoring de-assert INIT to vcpu %d\n", >> @@ -743,11 +742,11 @@ static int __apic_accept_irq(struct kvm_lapic *a= pic, int delivery_mode, >> case APIC_DM_STARTUP: >> apic_debug("SIPI to vcpu %d vector 0x%02x\n", >> vcpu->vcpu_id, vector); >> - if (vcpu->arch.mp_state =3D=3D KVM_MP_STATE_INIT_RECEIVED) { >> + if (vcpu->arch.mp_state =3D=3D KVM_MP_STATE_INIT_RECEIVED || >> + test_bit(KVM_REQ_INIT, &vcpu->requests)) { >> result =3D 1; >> vcpu->arch.sipi_vector =3D vector; >> - vcpu->arch.mp_state =3D KVM_MP_STATE_SIPI_RECEIVED; >> - kvm_make_request(KVM_REQ_EVENT, vcpu); >> + kvm_make_request(KVM_REQ_SIPI, vcpu); >> kvm_vcpu_kick(vcpu); >> } >> break; >> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c >> index d0cf737..8c8843c 100644 >> --- a/arch/x86/kvm/x86.c >> +++ b/arch/x86/kvm/x86.c >> @@ -5641,6 +5641,18 @@ static void update_eoi_exitmap(struct kvm_vcpu = *vcpu) >> kvm_x86_ops->load_eoi_exitmap(vcpu, eoi_exit_bitmap); >> } >> =20 >> +static bool kvm_check_init_and_sipi(struct kvm_vcpu *vcpu) >> +{ >> + if (kvm_check_request(KVM_REQ_INIT, vcpu)) >> + vcpu->arch.mp_state =3D KVM_MP_STATE_INIT_RECEIVED; >> + if (kvm_check_request(KVM_REQ_SIPI, vcpu) && >> + vcpu->arch.mp_state =3D=3D KVM_MP_STATE_INIT_RECEIVED) { >> + vcpu->arch.mp_state =3D KVM_MP_STATE_SIPI_RECEIVED; >=20 > Do you need KVM_MP_STATE_SIPI_RECEIVED at all anymore? Unfortunately, we cannot kill it as it was user-visible: When a VCPU receives KVM_MP_STATE_SIPI_RECEIVED, it leaves __vcpu_run with -EINTR and, thus, KVM_RUN. We actually return to userspace, allowing it to see this mp_state and also migrate the guest in this state= =2E I could avoid this userspace exit (not sure what it is good for) but we will have to keep the logic to accept and convert the state into KVM_MP_STATE_RUNNABLE. So there is not much to simplify here, I'm afraid.= Jan ------enig2USBVTETFSFVFDWNLRFDL 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.16 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iEYEARECAAYFAlE1CQwACgkQitSsb3rl5xR2MwCdFmuBxN/za3omRJ/fnEZQVgM8 1+QAoNX/9Tompot7tjBkJ/50O1B57Sjg =6Hcy -----END PGP SIGNATURE----- ------enig2USBVTETFSFVFDWNLRFDL--