From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753390AbdKWUid (ORCPT ); Thu, 23 Nov 2017 15:38:33 -0500 Received: from gate.crashing.org ([63.228.1.57]:46068 "EHLO gate.crashing.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753297AbdKWUic (ORCPT ); Thu, 23 Nov 2017 15:38:32 -0500 Message-ID: <1511469493.2466.47.camel@kernel.crashing.org> Subject: Re: [PATCH] powerpc/xive: store server for masked interrupt in kvmppc_xive_set_xive() From: Benjamin Herrenschmidt To: Laurent Vivier , linux-kernel@vger.kernel.org Cc: linuxppc-dev@lists.ozlabs.org, kvm-ppc@vger.kernel.org, Paul Mackerras , Paolo Bonzini , Thomas Huth , Sam Bobroff Date: Fri, 24 Nov 2017 07:38:13 +1100 In-Reply-To: <20171123090633.873-1-lvivier@redhat.com> References: <20171123090633.873-1-lvivier@redhat.com> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.26.2 (3.26.2-1.fc27) Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 2017-11-23 at 10:06 +0100, Laurent Vivier wrote: > This is needed to map kvmppc_xive_set_xive() behavior > to kvmppc_xics_set_xive(). > > As we store the server, kvmppc_xive_get_xive() can return > the good value and we can also allow kvmppc_xive_int_on(). > > Signed-off-by: Laurent Vivier > --- > arch/powerpc/kvm/book3s_xive.c | 20 ++++++++------------ > 1 file changed, 8 insertions(+), 12 deletions(-) > > diff --git a/arch/powerpc/kvm/book3s_xive.c b/arch/powerpc/kvm/book3s_xive.c > index bf457843e032..2781b8733038 100644 > --- a/arch/powerpc/kvm/book3s_xive.c > +++ b/arch/powerpc/kvm/book3s_xive.c > @@ -584,10 +584,14 @@ int kvmppc_xive_set_xive(struct kvm *kvm, u32 irq, u32 server, > * we could initialize interrupts with valid default > */ > > - if (new_act_prio != MASKED && > - (state->act_server != server || > - state->act_priority != new_act_prio)) > - rc = xive_target_interrupt(kvm, state, server, new_act_prio); > + if (state->act_server != server || > + state->act_priority != new_act_prio) { > + if (new_act_prio != MASKED) > + rc = xive_target_interrupt(kvm, state, server, > + new_act_prio); > + if (!rc) > + state->act_server = server; > + } That leads to another problem with this code. My current implementation is such that is a target queue is full, it will pick another target. But here, we still update act_server to the passed-in server and not the actual target... > /* > * Perform the final unmasking of the interrupt source > @@ -646,14 +650,6 @@ int kvmppc_xive_int_on(struct kvm *kvm, u32 irq) > > pr_devel("int_on(irq=0x%x)\n", irq); > > - /* > - * Check if interrupt was not targetted > - */ > - if (state->act_priority == MASKED) { > - pr_devel("int_on on untargetted interrupt\n"); > - return -EINVAL; > - } > - So my thinking here was that act_priority was never going to be MASKED except if the interrupt had never been targetted anywhere at machine startup time. Thus if act_priority is masked, the act_server field cannot be trusted. > /* If saved_priority is 0xff, do nothing */ > if (state->saved_priority == MASKED) > return 0;