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:25:04 +0100 Message-ID: <5141C1A0.2060709@siemens.com> References: <5140662A.7010209@siemens.com> <20130314000850.GA4499@amt.cnet> <5141A326.3090500@siemens.com> <5141B4AC.2000708@siemens.com> <20130314121229.GN11223@redhat.com> <5141BFA9.3020108@siemens.com> <20130314121842.GA10272@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 david.siemens.de ([192.35.17.14]:17990 "EHLO david.siemens.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757640Ab3CNMZJ (ORCPT ); Thu, 14 Mar 2013 08:25:09 -0400 In-Reply-To: <20130314121842.GA10272@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On 2013-03-14 13:18, Gleb Natapov wrote: > On Thu, Mar 14, 2013 at 01:16:41PM +0100, Jan Kiszka wrote: >> 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. >> > I agree. Lets try second approach. Can you write a patch? Task queued for later today or tomorrow. I suppose you hold back this patch here for now anyway. Jan -- Siemens AG, Corporate Technology, CT RTC ITP SDP-DE Corporate Competence Center Embedded Linux