From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bandan Das Subject: Re: [PATCH] KVM: nVMX: mask unrestricted_guest if disabled on L0 Date: Tue, 24 Feb 2015 13:32:58 -0500 Message-ID: References: <20150219160221.GB19057@potion.brq.redhat.com> <20150219211011.GE28728@tesla.redhat.com> <20150219222832.GA22611@tesla.redhat.com> <20150220161415.GA22152@potion.brq.redhat.com> <20150220194509.GD30563@tesla.home> <20150222154622.GA30296@tesla.redhat.com> <20150223135611.GB2102@potion.brq.redhat.com> <20150223161437.GI30296@tesla.redhat.com> <20150223170906.GA1055@tesla.redhat.com> <20150223180527.GK30296@tesla.redhat.com> <20150224163005.GB2186@potion.brq.redhat.com> <54ECA942.6050308@siemens.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: Radim =?utf-8?B?S3LEjW3DocWZ?= , Kashyap Chamarthy , Paolo Bonzini , kvm@vger.kernel.org, dgilbert@redhat.com, mtosatti@redhat.com To: Jan Kiszka Return-path: Received: from mx1.redhat.com ([209.132.183.28]:60478 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753160AbbBXSdF convert rfc822-to-8bit (ORCPT ); Tue, 24 Feb 2015 13:33:05 -0500 In-Reply-To: <54ECA942.6050308@siemens.com> (Jan Kiszka's message of "Tue, 24 Feb 2015 17:39:30 +0100") Sender: kvm-owner@vger.kernel.org List-ID: Jan Kiszka writes: > On 2015-02-24 17:30, Radim Kr=C4=8Dm=C3=A1=C5=99 wrote: >> 2015-02-23 19:05+0100, Kashyap Chamarthy: >>> Tested with the _correct_ Kernel[1] (that has Radim's patch) now -- >>> applied it on both L0 and L1. >>> >>> Result: Same as before -- Booting L2 causes L1 to reboot. However, = the >>> stack trace from `dmesg` on L0 is took slightly different p= ath than >>> before -- it's using MSR handling: >>=20 >> Thanks, the problem was deeper ... L1 enabled unrestricted mode whil= e L0 >> had it disabled. L1 could then vmrun a L2 state that L0 would have = to >> emulate, but that doesn't work. There are at least these solutions: >>=20 >> 1) don't expose unrestricted_guest when L0 doesn't have it > > Reminds me of a patch called "KVM: nVMX: Disable unrestricted mode if > ept=3D0" by Bandan. I thought that would have caught it - apparently = not. Yeah... Unrestricted guest could be disabled even if ept=3D0, and I incorrectly didn't take that into account. >> 2) fix unrestricted mode emulation code >> 3) handle the failure a without killing L1 >>=20 >> I'd do just (1) -- emulating unrestricted mode is a loss. > > Agreed. > > Jan > >>=20 >> I have done initial testing and at least qemu-sanity-check works now= : >>=20 >> ---8<--- >> If EPT was enabled, unrestricted_guest was allowed in L1 regardless = of >> L0. L1 triple faulted when running L2 guest that required emulation= =2E >>=20 >> Another side effect was 'WARN_ON_ONCE(vmx->nested.nested_run_pending= )' >> in L0's dmesg: >> WARNING: CPU: 0 PID: 0 at arch/x86/kvm/vmx.c:9190 nested_vmx_vmexi= t+0x96e/0xb00 [kvm_intel] () >>=20 >> Prevent this scenario by masking SECONDARY_EXEC_UNRESTRICTED_GUEST w= hen >> the host doesn't have it enabled. >>=20 >> Fixes: 78051e3b7e35 ("KVM: nVMX: Disable unrestricted mode if ept=3D= 0") >> Signed-off-by: Radim Kr=C4=8Dm=C3=A1=C5=99 We should Cc stable on this patch. Bandan >> --- >> arch/x86/kvm/vmx.c | 7 +++++-- >> 1 file changed, 5 insertions(+), 2 deletions(-) >>=20 >> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c >> index f7b20b417a3a..dbabea21357b 100644 >> --- a/arch/x86/kvm/vmx.c >> +++ b/arch/x86/kvm/vmx.c >> @@ -2476,8 +2476,7 @@ static void nested_vmx_setup_ctls_msrs(struct = vcpu_vmx *vmx) >> if (enable_ept) { >> /* nested EPT: emulate EPT also to L1 */ >> vmx->nested.nested_vmx_secondary_ctls_high |=3D >> - SECONDARY_EXEC_ENABLE_EPT | >> - SECONDARY_EXEC_UNRESTRICTED_GUEST; >> + SECONDARY_EXEC_ENABLE_EPT; >> vmx->nested.nested_vmx_ept_caps =3D VMX_EPT_PAGE_WALK_4_BIT | >> VMX_EPTP_WB_BIT | VMX_EPT_2MB_PAGE_BIT | >> VMX_EPT_INVEPT_BIT; >> @@ -2491,6 +2490,10 @@ static void nested_vmx_setup_ctls_msrs(struct= vcpu_vmx *vmx) >> } else >> vmx->nested.nested_vmx_ept_caps =3D 0; >> =20 >> + if (enable_unrestricted_guest) >> + vmx->nested.nested_vmx_secondary_ctls_high |=3D >> + SECONDARY_EXEC_UNRESTRICTED_GUEST; >> + >> /* miscellaneous data */ >> rdmsr(MSR_IA32_VMX_MISC, >> vmx->nested.nested_vmx_misc_low, >>=20