>>> On 4/28/2016 at 01:08 PM, Radim Kr*má* <rkrcmar@xxxxxxxxxx> wrote: > 2016-04-22 12:56-0600, Bruce Rogers: >> Commit d28bc9dd25ce reversed the order of two lines which initialize cr0, >> allowing the current (old) cr0 value to mess up vcpu initialization. >> This was observed in the checks for cr0 X86_CR0_WP bit in the context of >> kvm_mmu_reset_context(). Besides, setting vcpu->arch.cr0 after vmx_set_cr0() >> is completely redundant. Change the order back to ensure proper vcpu >> initialization. >> >> The combination of booting with ovmf firmware when guest vcpus > 1 and kvm's >> ept=N option being set results in a VM-entry failure. This patch fixes that. > > Greg pointed out missing, > Cc: stable@xxxxxxxxxxxxxxx > when stable@xxxxxxxxxxxxxxx was Cc'd. Adding > Fixes: d28bc9dd25ce ("KVM: x86: INIT and reset sequences are different") > would be nice too (even when it is redundant). > >> Signed-off-by: Bruce Rogers <brogers@xxxxxxxx> >> --- >> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c >> @@ -5046,8 +5046,8 @@ static void vmx_vcpu_reset(struct kvm_vcpu *vcpu, bool > init_event) >> cr0 = X86_CR0_NW | X86_CR0_CD | X86_CR0_ET; >> - vmx_set_cr0(vcpu, cr0); /* enter rmode */ >> vmx->vcpu.arch.cr0 = cr0; >> + vmx_set_cr0(vcpu, cr0); /* enter rmode */ > > So vmx_set_cr0() has a code that depends on vmx->vcpu.arch.cr0 being > already set the to new value. Do you know what function is it? The one which I partly referred to above is the following: vmx_set_cr0() -> enter_rmode() -> kvm_mmu_reset_context() -> init_kvm_softmmu() -> kvm_init_shadow_mmu() -> is_write_protected() which uses the CR0 WP bit. There may be other problematic references. I haven't tried to do an exhaustive search. > > I think we better set vmx->vcpu.arch.cr0 early in vmx_set_cr0(). > Or do other callsites somehow depend on the old cr0 value? You may be right that that is a better overall fix. I was simply trying to undo the erroneous lines in the commit which broke things, partly to have a patch better suited for applying to stable releases. I'll send a v2 shortly with your suggested additions to the patch header. Thanks, Bruce -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html