From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754457AbcBHQnX (ORCPT ); Mon, 8 Feb 2016 11:43:23 -0500 Received: from mail-wm0-f65.google.com ([74.125.82.65]:33501 "EHLO mail-wm0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751956AbcBHQnW (ORCPT ); Mon, 8 Feb 2016 11:43:22 -0500 Subject: Re: [PATCH 1/2] KVM: x86: fix ordering of cr0 initialization code in vmx_cpu_reset To: Bruce Rogers , kvm@vger.kernel.org, linux-kernel@vger.kernel.org References: <1454539876-8310-1-git-send-email-brogers@suse.com> <56B8AFAF.7090509@redhat.com> <56B85FFA02000048001251CE@prv-mh.provo.novell.com> Cc: namit@cs.technion.ac.il, jan.kiszka@siemens.com From: Paolo Bonzini Message-ID: <56B8C5A6.3070407@redhat.com> Date: Mon, 8 Feb 2016 17:43:18 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.0 MIME-Version: 1.0 In-Reply-To: <56B85FFA02000048001251CE@prv-mh.provo.novell.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 08/02/2016 17:29, Bruce Rogers wrote: >>>> On 2/8/2016 at 08:09 AM, Paolo Bonzini wrote: > >> >> On 03/02/2016 23:51, Bruce Rogers wrote: >>> >>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c >>> index e2951b6..21507b4 100644 >>> --- a/arch/x86/kvm/vmx.c >>> +++ b/arch/x86/kvm/vmx.c >>> @@ -4993,8 +4993,8 @@ static void vmx_vcpu_reset(struct kvm_vcpu *vcpu, bool >> init_event) >>> vmcs_write16(VIRTUAL_PROCESSOR_ID, vmx->vpid); >>> >>> 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 */ >> >> Your comment that the assignment is redundant is correct, but I am >> afraid that this fix is also wrong. In particular, it would not cause >> exit_lmode and enter_rmode to be called. >> >> You are not describing which call to kvm_mmu_reset_context was messed >> up, so I'm not sure how your patch is fixing things. > > This is in the context of AP sending INIT to BSP with unrestricted_guest=N. > > So the call sequence where I see the issue is: kvm_apic_accept_events() -> > kvm_vcpu_reset() -> vmx_vcpu_reset() -> vmx_set_cr0() -> enter_rmode() -> > kvm_mmu_reset_context(). > > enter_rmode is called in the case I am testing. Please describe the bug as thoroughly as possible, especially the initial state of the BSP and AP and how the bug manifests after the INIT IPI. It would be great to write a kvm-unit-tests testcase for it, but I can do it too if you provide enough information. Paolo