From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [PATCH v2] KVM: x86: Rework INIT and SIPI handling Date: Thu, 14 Mar 2013 13:16:41 +0100 Message-ID: <5141BFA9.3020108@siemens.com> References: <5140662A.7010209@siemens.com> <20130314000850.GA4499@amt.cnet> <5141A326.3090500@siemens.com> <5141B4AC.2000708@siemens.com> <20130314121229.GN11223@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: Marcelo Tosatti , kvm , Paolo Bonzini To: Gleb Natapov Return-path: Received: from goliath.siemens.de ([192.35.17.28]:31151 "EHLO goliath.siemens.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755923Ab3CNMQp (ORCPT ); Thu, 14 Mar 2013 08:16:45 -0400 In-Reply-To: <20130314121229.GN11223@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On 2013-03-14 13:12, Gleb Natapov wrote: > On Thu, Mar 14, 2013 at 12:29:48PM +0100, Jan Kiszka wrote: >> On 2013-03-14 11:15, Jan Kiszka wrote: >>>>> >>>>> - if (unlikely(vcpu->arch.mp_state == KVM_MP_STATE_SIPI_RECEIVED)) { >>>>> - pr_debug("vcpu %d received sipi with vector # %x\n", >>>>> - vcpu->vcpu_id, vcpu->arch.sipi_vector); >>>>> - kvm_lapic_reset(vcpu); >>>>> - kvm_vcpu_reset(vcpu); >>>>> - vcpu->arch.mp_state = KVM_MP_STATE_RUNNABLE; >>>>> - } >>>>> - >>>>> vcpu->srcu_idx = srcu_read_lock(&kvm->srcu); >>>>> r = vapic_enter(vcpu); >>>> >>>> vmx_vcpu_reset overwrites vcpu->srcu_idx if ->vcpu_reset is called from >>>> within srcu section. >>> >>> Indeed. >>> >>> Do you know what the look over vmx_set_cr0 actually protects? >> >> Found it: It's not actually protecting anything. enter_rmode is called, >> and that assumes that lock to be held. If enter_rmode faces an >> uninitialized tss, it drops the lock before calling vmx_set_tss_addr. >> >> Well, I wonder if that is a good place to fix the TSS issue. Why not >> make that special case (lacking KVM_SET_TSS_ADDR before first KVM_RUN) a >> static jump key and check for it on KVM_RUN? >> > Or finally break userspace that does not set it before calling kvm_run. > I haven't seen people complain about "kvm: KVM_SET_TSS_ADDR need to be > called before entering vcpu" warning in dmesg. Or create TSS mem slot at > 0xfeffd000 during VM creation and destroy it if userspace overwrites it. Whatever is preferred, I'm not able to decide (about ABI "breakage" specifically). I just think any of them would be better than pulling vcpu_reset out of the inner loop again just to fulfill the locking requirements. Jan -- Siemens AG, Corporate Technology, CT RTC ITP SDP-DE Corporate Competence Center Embedded Linux