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 <jan.kis...@siemens.com> >>>> >>>> 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 <jan.kis...@siemens.com> >>>> --- >>>> 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 need >>>> * to apply the same changes to L1's vmcs. We just set cr0 correctly, >>>> >>> >>> This one still applies, is necessary for nested unrestricted guest mode, >>> and I'm still convinced it's an appropriate way to fix the bug. How to >>> proceed? >>> >> What check that is done by kvm_set_cr0() fails? > > Would have to reproduce the bug to confirm, but from the top of my head > and from looking at the code again: > > if (!is_paging(vcpu) && (cr0 & X86_CR0_PG)) { > if ((vcpu->arch.efer & EFER_LME)) { > int cs_db, cs_l; > > if (!is_pae(vcpu)) > return 1; > kvm_x86_ops->get_cs_db_l_bits(vcpu, &cs_db, &cs_l); > if (cs_l) > return 1; > > 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 bail > 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
signature.asc
Description: OpenPGP digital signature