Re: [PATCH v2 08/11] powerpc/kvm/xive: Keep escalation interrupt masked unless ceded

[Benjamin Herrenschmidt <benh@kernel.crashing.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.