From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [PATCH] KVM: nVMX: Replace kvm_set_cr0 with vmx_set_cr0 in load_vmcs12_host_state Date: Sun, 05 May 2013 11:02:52 +0200 Message-ID: <5186203C.6030207@web.de> References: <5129361A.7090608@web.de> <517CF7F6.3070405@web.de> <20130430114604.GA22125@redhat.com> <517FBC2C.6040607@siemens.com> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="----enig2DPMPGLWLOCVLKOUXXVPI" Cc: Gleb Natapov , Marcelo Tosatti , kvm , Nadav Har'El , "Nakajima, Jun" , Avi Kivity To: unlisted-recipients:; (no To-header on input) Return-path: Received: from mout.web.de ([212.227.17.11]:62674 "EHLO mout.web.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751602Ab3EEJDb (ORCPT ); Sun, 5 May 2013 05:03:31 -0400 In-Reply-To: <517FBC2C.6040607@siemens.com> Sender: kvm-owner@vger.kernel.org List-ID: This is an OpenPGP/MIME signed message (RFC 4880 and 3156) ------enig2DPMPGLWLOCVLKOUXXVPI Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable On 2013-04-30 14:42, Jan Kiszka wrote: > On 2013-04-30 13:46, Gleb Natapov wrote: >> On Sun, Apr 28, 2013 at 12:20:38PM +0200, Jan Kiszka wrote: >>> On 2013-02-23 22:35, Jan Kiszka wrote: >>>> From: Jan Kiszka >>>> >>>> Likely a typo, but a fatal one as kvm_set_cr0 performs checks on the= >>>> state transition that may prevent loading L1's cr0. >>>> >>>> Signed-off-by: Jan Kiszka >>>> --- >>>> arch/x86/kvm/vmx.c | 2 +- >>>> 1 files changed, 1 insertions(+), 1 deletions(-) >>>> >>>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c >>>> index 26d47e9..94f3b66 100644 >>>> --- a/arch/x86/kvm/vmx.c >>>> +++ b/arch/x86/kvm/vmx.c >>>> @@ -7429,7 +7429,7 @@ static void load_vmcs12_host_state(struct kvm_= vcpu *vcpu, >>>> * fpu_active (which may have changed). >>>> * Note that vmx_set_cr0 refers to efer set above. >>>> */ >>>> - kvm_set_cr0(vcpu, vmcs12->host_cr0); >>>> + vmx_set_cr0(vcpu, vmcs12->host_cr0); >>>> /* >>>> * If we did fpu_activate()/fpu_deactivate() during L2's run, we n= eed >>>> * to apply the same changes to L1's vmcs. We just set cr0 correct= ly, >>>> >>> >>> This one still applies, is necessary for nested unrestricted guest mo= de, >>> and I'm still convinced it's an appropriate way to fix the bug. How t= o >>> proceed? >>> >> What check that is done by kvm_set_cr0() fails? >=20 > Would have to reproduce the bug to confirm, but from the top of my head= > and from looking at the code again: >=20 > if (!is_paging(vcpu) && (cr0 & X86_CR0_PG)) { > if ((vcpu->arch.efer & EFER_LME)) { > int cs_db, cs_l; >=20 > if (!is_pae(vcpu)) > return 1; > kvm_x86_ops->get_cs_db_l_bits(vcpu, &cs_db, &cs_l); > if (cs_l) > return 1; >=20 > I think to remember this last check triggered. When we come from the > guest with paging off, we may run through this check an incorrectly bai= l > out here when the host state fulfills the conditions (PG, EFER_LME, and= > L bit set). Just retried, and actually the first check (!is_pae) fails right now (with nested unrestricted guest mode patched in). The second one stumbles if I set CR4 before CR1 in load_vmcs12_host_state. So, however you put it, calling kvm_set_cr0 remains wrong. Jan ------enig2DPMPGLWLOCVLKOUXXVPI 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/ iEYEARECAAYFAlGGID8ACgkQitSsb3rl5xQXBQCfTws0GvbKFtScw2eeXh9RdM+V DEgAnRbWvKFn4dFXPN2I/Z9eLHu4lEnP =k3JN -----END PGP SIGNATURE----- ------enig2DPMPGLWLOCVLKOUXXVPI--