From mboxrd@z Thu Jan 1 00:00:00 1970 From: Scott Wood Date: Fri, 10 May 2013 22:53:23 +0000 Subject: Re: [PATCH v2 4/4] kvm/ppc: IRQ disabling cleanup Message-Id: <1368226403.19683.15@snotra> List-Id: 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> In-Reply-To: <6A3DF150A5B70D4F9B66A25E3F7C888D0700F859@039-SN2MPN1-011.039d.mgd.msft.net> (from R65777@freescale.com on Fri May 10 00:01:38 2013) MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Bhushan Bharat-R65777 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 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from tx2outboundpool.messaging.microsoft.com (tx2ehsobe001.messaging.microsoft.com [65.55.88.11]) (using TLSv1 with cipher AES128-SHA (128/128 bits)) (Client CN "mail.global.frontbridge.com", Issuer "MSIT Machine Auth CA 2" (not verified)) by ozlabs.org (Postfix) with ESMTPS id F222A2C00E0 for ; Sat, 11 May 2013 08:53:32 +1000 (EST) Date: Fri, 10 May 2013 17:53:23 -0500 From: Scott Wood Subject: Re: [PATCH v2 4/4] kvm/ppc: IRQ disabling cleanup To: Bhushan Bharat-R65777 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> In-Reply-To: <6A3DF150A5B70D4F9B66A25E3F7C888D0700F859@039-SN2MPN1-011.039d.mgd.msft.net> (from R65777@freescale.com on Fri May 10 00:01:38 2013) Message-ID: <1368226403.19683.15@snotra> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; delsp=Yes; format=Flowed Cc: Wood Scott-B07421 , "kvm@vger.kernel.org" , Alexander Graf , "kvm-ppc@vger.kernel.org" , "linuxppc-dev@lists.ozlabs.org" List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On 05/10/2013 12:01:38 AM, Bhushan Bharat-R65777 wrote: >=20 >=20 > > -----Original Message----- > > From: kvm-ppc-owner@vger.kernel.org =20 > [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; =20 > 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(); >=20 > Here we hard disable in kvmppc_prepare_to_enter(), so my comment in =20 > other patch about interrupt loss is no more valid. >=20 > So here > MSR.EE =3D 0 > local_paca->soft_enabled =3D 0 > local_paca->irq_happened |=3D PACA_IRQ_HARD_DIS; >=20 > > + > > while (true) { > > if (need_resched()) { > > local_irq_enable(); >=20 > This will make the state: > MSR.EE =3D 1 > local_paca->soft_enabled =3D 1 > local_paca->irq_happened =3D PACA_IRQ_HARD_DIS; //same as before >=20 > Is that a valid state where interrupts are fully enabled and =20 > irq_happend in not 0? PACA_IRQ_HARD_DIS will have been cleared by local_irq_enable(), as =20 Tiejun pointed out. > int kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcpu) > { > int r =3D 0; > WARN_ON_ONCE(!irqs_disabled()); >=20 > kvmppc_core_check_exceptions(vcpu); >=20 > if (vcpu->requests) { > /* Exception delivery raised request; start over */ > return 1; > } >=20 > 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 =20 WARN_ON to double check that interrupts are hard-disabled (eventually =20 we'll probably want to make these critical-path assertions dependent on =20 a debug option...). It doesn't really matter all that much, though, =20 since we don't have MSR_WE on any 64-bit booke chips. :-) -Scott= 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