From mboxrd@z Thu Jan 1 00:00:00 1970 From: Scott Wood Subject: Re: [PATCH v2 4/4] kvm/ppc: IRQ disabling cleanup Date: Fri, 10 May 2013 17:53:23 -0500 Message-ID: <1368226403.19683.15@snotra> References: <1368155384-11035-1-git-send-email-scottwood@freescale.com> <1368155384-11035-5-git-send-email-scottwood@freescale.com> <6A3DF150A5B70D4F9B66A25E3F7C888D0700F859@039-SN2MPN1-011.039d.mgd.msft.net> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; delsp=Yes; format=Flowed Content-Transfer-Encoding: 8BIT Cc: Wood Scott-B07421 , Alexander Graf , Benjamin Herrenschmidt , "kvm-ppc@vger.kernel.org" , "kvm@vger.kernel.org" , "linuxppc-dev@lists.ozlabs.org" , Wood Scott-B07421 To: Bhushan Bharat-R65777 Return-path: In-Reply-To: <6A3DF150A5B70D4F9B66A25E3F7C888D0700F859@039-SN2MPN1-011.039d.mgd.msft.net> (from R65777@freescale.com on Fri May 10 00:01:38 2013) Content-Disposition: inline Sender: kvm-ppc-owner@vger.kernel.org List-Id: kvm.vger.kernel.org On 05/10/2013 12:01:38 AM, Bhushan Bharat-R65777 wrote: > > > > -----Original Message----- > > From: kvm-ppc-owner@vger.kernel.org > [mailto:kvm-ppc-owner@vger.kernel.org] On > > Behalf Of Scott Wood > > Sent: Friday, May 10, 2013 8:40 AM > > To: Alexander Graf; Benjamin Herrenschmidt > > Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; > linuxppc-dev@lists.ozlabs.org; > > Wood Scott-B07421 > > Subject: [PATCH v2 4/4] kvm/ppc: IRQ disabling cleanup > > > > - WARN_ON_ONCE(!irqs_disabled()); > > + WARN_ON(irqs_disabled()); > > + hard_irq_disable(); > > Here we hard disable in kvmppc_prepare_to_enter(), so my comment in > other patch about interrupt loss is no more valid. > > So here > MSR.EE = 0 > local_paca->soft_enabled = 0 > local_paca->irq_happened |= PACA_IRQ_HARD_DIS; > > > + > > while (true) { > > if (need_resched()) { > > local_irq_enable(); > > This will make the state: > MSR.EE = 1 > local_paca->soft_enabled = 1 > local_paca->irq_happened = PACA_IRQ_HARD_DIS; //same as before > > Is that a valid state where interrupts are fully enabled and > irq_happend in not 0? PACA_IRQ_HARD_DIS will have been cleared by local_irq_enable(), as Tiejun pointed out. > int kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcpu) > { > int r = 0; > WARN_ON_ONCE(!irqs_disabled()); > > kvmppc_core_check_exceptions(vcpu); > > if (vcpu->requests) { > /* Exception delivery raised request; start over */ > return 1; > } > > if (vcpu->arch.shared->msr & MSR_WE) { > local_irq_enable(); > kvm_vcpu_block(vcpu); > clear_bit(KVM_REQ_UNHALT, &vcpu->requests); > local_irq_disable(); > ^^^ > We do not require hard_irq_disable() here? Yes, that should be changed to hard_irq_disable(), and I'll add a WARN_ON to double check that interrupts are hard-disabled (eventually we'll probably want to make these critical-path assertions dependent on a debug option...). It doesn't really matter all that much, though, since we don't have MSR_WE on any 64-bit booke chips. :-) -Scott