From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marcelo Tosatti Subject: Re: [PATCH v2] KVM: x86: Convert INIT and SIPI signals into synchronously handled requests Date: Wed, 6 Mar 2013 18:50:27 -0300 Message-ID: <20130306215027.GA25030@amt.cnet> References: <70318159.3047162.1362550372481.JavaMail.root@redhat.com> <5136F702.4070101@web.de> <20130306213020.GB23299@amt.cnet> <5137B78F.8020009@web.de> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Paolo Bonzini , Gleb Natapov , kvm To: Jan Kiszka Return-path: Received: from mx1.redhat.com ([209.132.183.28]:28443 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756277Ab3CFVub (ORCPT ); Wed, 6 Mar 2013 16:50:31 -0500 Content-Disposition: inline In-Reply-To: <5137B78F.8020009@web.de> Sender: kvm-owner@vger.kernel.org List-ID: On Wed, Mar 06, 2013 at 10:39:27PM +0100, Jan Kiszka wrote: > On 2013-03-06 22:30, Marcelo Tosatti wrote: > > On Wed, Mar 06, 2013 at 08:57:54AM +0100, Jan Kiszka wrote: > >> On 2013-03-06 07:12, Paolo Bonzini wrote: > >>> > >>>> On Tue, Mar 05, 2013 at 08:16:41PM -0300, Marcelo Tosatti wrote: > >>>>> On Mon, Mar 04, 2013 at 10:41:43PM +0100, Jan Kiszka wrote: > >>>>>> From: Jan Kiszka > >>>>>> > >>>>>> A VCPU sending INIT or SIPI to some other VCPU races for setting > >>>>>> the > >>>>>> remote VCPU's mp_state. When we were unlucky, > >>>>>> KVM_MP_STATE_INIT_RECEIVED > >>>>>> was overwritten by kvm_emulate_halt and, thus, got lost. > >>>>>> > >>>>>> Fix this by raising requests on the sender side that will then be > >>>>>> handled synchronously over the target VCPU context. > >>>>>> > >>>>>> Signed-off-by: Jan Kiszka > >>>>> > >>>>> Why is kvm_emulate_halt being executed from > >>>>> KVM_MP_STATE_INIT_RECEIVED/KVM_MP_STATE_SIPI_RECEIVED again? > >>>>> > >>>>> Why is it not true that the only valid transition from > >>>>> KVM_MP_STATE_HALTED is from KVM_MP_STATE_RUNNABLE? > >>>> > >>>> See Paolo's table, it is. So why fix a race which should not be > >>>> happening in the first place. > >>> > >>> The bad transition happens exactly because of the race. > >>> Are you saying you prefer the solution with cmpxchg? > >> > >> I think we are past that point in our discussion and should really > >> separate signal (INIT/SIPI) from state (INIT/SIPI_RECEIVED etc.). > >> > >> Jan > > > > The sentence "KVM_MP_STATE_INIT_RECEIVED overwritten by > > kvm_emulate_halt" is contradictory, unless i miss something. > > http://thread.gmane.org/gmane.comp.emulators.kvm.devel/105638 > > Jan "A VCPU sending INIT or SIPI to some other VCPU races for setting the remote VCPU's mp_state. When we were unlucky, KVM_MP_STATE_INIT_RECEIVED was overwritten by kvm_emulate_halt and, thus, got lost. Fix this by raising requests on the sender side that will then be handled synchronously over the target VCPU context." The scenario you describe is: vcpu0,bsp vcpu1 vcpu0->mp_state=KVM_MP_STATE_RUNNABLE vcpu1->mp_state=KVM_MP_STATE_UNINIT at __accept_apic_irq() vcpu1->mp_state=KVM_MP_STATE_INIT_RECEIVED kvm_emulate_halt vcpu1->mp_state= KVM_MP_STATE_HALTED This is what the first sentence from the patch refers to, correct?