From mboxrd@z Thu Jan 1 00:00:00 1970 From: Benjamin Herrenschmidt Date: Sun, 26 Nov 2017 21:55:48 +0000 Subject: Re: [PATCH v2 08/11] powerpc/kvm/xive: Keep escalation interrupt masked unless ceded Message-Id: <1511733348.2466.63.camel@kernel.crashing.org> List-Id: References: <20171123043619.15301-1-benh@kernel.crashing.org> <20171123043619.15301-8-benh@kernel.crashing.org> <20171125045634.GA10920@fergus.ozlabs.ibm.com> In-Reply-To: <20171125045634.GA10920@fergus.ozlabs.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Paul Mackerras Cc: kvm@vger.kernel.org, kvm-ppc@vger.kernel.org On Sat, 2017-11-25 at 15:56 +1100, Paul Mackerras wrote: > On Thu, Nov 23, 2017 at 03:36:16PM +1100, Benjamin Herrenschmidt wrote: > > This works on top of the single escalation support. When in single > > escalation, with this change, we will keep the escalation interrupt > > disabled unless the VCPU is in H_CEDE (idle). In any other case, we > > know the VCPU will be rescheduled and thus there is no need to take > > escalation interrupts in the host whenever a guest interrupt fires. > > Some comments below... > > > @@ -2705,7 +2740,32 @@ kvm_cede_prodded: > > /* we've ceded but we want to give control to the host */ > > kvm_cede_exit: > > ld r9, HSTATE_KVM_VCPU(r13) > > - b guest_exit_cont > > +#ifdef CONFIG_KVM_XICS > > + /* Abort if we still have a pending escalation */ > > This comment might be clearer if you say "Cancel cede" rather than > "Abort". > > > diff --git a/arch/powerpc/kvm/book3s_xive.c b/arch/powerpc/kvm/book3s_xive.c > > index eef9ccafdc09..7a047bc88f11 100644 > > --- a/arch/powerpc/kvm/book3s_xive.c > > +++ b/arch/powerpc/kvm/book3s_xive.c > > @@ -89,6 +89,17 @@ static irqreturn_t xive_esc_irq(int irq, void *data) > > if (vcpu->arch.ceded) > > kvmppc_fast_vcpu_kick(vcpu); > > > > + /* Since we have the no-EOI flag, the interrupt is effectively > > + * disabled now. Clearing xive_esc_on means we won't bother > > + * doing so on the next entry. > > + * > > + * This also allows the entry code to know that if a PQ combination > > + * of 10 is observed while xive_esc_on is true, it means the queue > > + * contains an unprocessed escalation interrupt. We don't make use of > > + * that knowledge today but might (see comment in book3s_hv_rmhandler.S) > > Is "We don't make use of that knowledge" actually true? I thought we > did make use of it in the assembly code in book3s_hv_rmhandlers.S (in > the code that this patch adds). In what way don't we make use of it? Obsolete comment, sorry. Cheers, Ben. From mboxrd@z Thu Jan 1 00:00:00 1970 From: Benjamin Herrenschmidt Subject: Re: [PATCH v2 08/11] powerpc/kvm/xive: Keep escalation interrupt masked unless ceded Date: Mon, 27 Nov 2017 08:55:48 +1100 Message-ID: <1511733348.2466.63.camel@kernel.crashing.org> References: <20171123043619.15301-1-benh@kernel.crashing.org> <20171123043619.15301-8-benh@kernel.crashing.org> <20171125045634.GA10920@fergus.ozlabs.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org, kvm-ppc@vger.kernel.org To: Paul Mackerras Return-path: Received: from gate.crashing.org ([63.228.1.57]:60248 "EHLO gate.crashing.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751816AbdKZVzz (ORCPT ); Sun, 26 Nov 2017 16:55:55 -0500 In-Reply-To: <20171125045634.GA10920@fergus.ozlabs.ibm.com> Sender: kvm-owner@vger.kernel.org List-ID: On Sat, 2017-11-25 at 15:56 +1100, Paul Mackerras wrote: > On Thu, Nov 23, 2017 at 03:36:16PM +1100, Benjamin Herrenschmidt wrote: > > This works on top of the single escalation support. When in single > > escalation, with this change, we will keep the escalation interrupt > > disabled unless the VCPU is in H_CEDE (idle). In any other case, we > > know the VCPU will be rescheduled and thus there is no need to take > > escalation interrupts in the host whenever a guest interrupt fires. > > Some comments below... > > > @@ -2705,7 +2740,32 @@ kvm_cede_prodded: > > /* we've ceded but we want to give control to the host */ > > kvm_cede_exit: > > ld r9, HSTATE_KVM_VCPU(r13) > > - b guest_exit_cont > > +#ifdef CONFIG_KVM_XICS > > + /* Abort if we still have a pending escalation */ > > This comment might be clearer if you say "Cancel cede" rather than > "Abort". > > > diff --git a/arch/powerpc/kvm/book3s_xive.c b/arch/powerpc/kvm/book3s_xive.c > > index eef9ccafdc09..7a047bc88f11 100644 > > --- a/arch/powerpc/kvm/book3s_xive.c > > +++ b/arch/powerpc/kvm/book3s_xive.c > > @@ -89,6 +89,17 @@ static irqreturn_t xive_esc_irq(int irq, void *data) > > if (vcpu->arch.ceded) > > kvmppc_fast_vcpu_kick(vcpu); > > > > + /* Since we have the no-EOI flag, the interrupt is effectively > > + * disabled now. Clearing xive_esc_on means we won't bother > > + * doing so on the next entry. > > + * > > + * This also allows the entry code to know that if a PQ combination > > + * of 10 is observed while xive_esc_on is true, it means the queue > > + * contains an unprocessed escalation interrupt. We don't make use of > > + * that knowledge today but might (see comment in book3s_hv_rmhandler.S) > > Is "We don't make use of that knowledge" actually true? I thought we > did make use of it in the assembly code in book3s_hv_rmhandlers.S (in > the code that this patch adds). In what way don't we make use of it? Obsolete comment, sorry. Cheers, Ben.