From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Subject: Re: [PATCH] xen/arm: introduce
 platform_need_explicit_eoi
Date: Fri, 20 Jun 2014 15:27:26 +0100
Message-ID: <alpine.DEB.2.02.1406201523380.19982@kaball.uk.xensource.com>
References: <1403271316-21635-1-git-send-email-stefano.stabellini@eu.citrix.com>
	<53A440FD.6070303@linaro.org>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <53A440FD.6070303@linaro.org>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Julien Grall <julien.grall@linaro.org>
Cc: xen-devel@lists.xensource.com, apatel@apm.com, Ian.Campbell@citrix.com, psawargaonkar@apm.com, Stefano Stabellini <stefano.stabellini@eu.citrix.com>
List-Id: xen-devel@lists.xenproject.org

On Fri, 20 Jun 2014, Julien Grall wrote:
> Hi Stefano,
> 
> On 20/06/14 14:35, Stefano Stabellini wrote:
> > GICH_LR_HW doesn't work as expected on X-Gene: request maintenance
> > interrupts and perform EOIs in the hypervisor as a workaround.
> > Trigger this behaviour with a per platform option.
> > 
> > No need to find the pcpu that needs to write to GICC_DIR, because after
> > "physical irq follow virtual irq" we always inject the virtual irq on
> > the vcpu that is running on the pcpu that received the interrupt.
> 
> Even without the patch "physical irq follow virtual irq" we can EOI an SPIs on
> any physical CPUs.

Can you point me to the paragraph of the GIC spec that says that?


> Otherwise, even with this patch, you will have to find pcpu when the VCPU has
> been migrated by EOI the IRQ :).
> I would rework this last paragraph to explain this is how the GIC behave.
> 
> >   static void gic_update_one_lr(struct vcpu *v, int i)
> >   {
> >       struct pending_irq *p;
> > @@ -692,7 +699,11 @@ static void gic_update_one_lr(struct vcpu *v, int i)
> >           clear_bit(i, &this_cpu(lr_mask));
> > 
> >           if ( p->desc != NULL )
> > +        {
> >               p->desc->status &= ~IRQ_INPROGRESS;
> > +            if ( platform_has_quirk(PLATFORM_QUIRK_NEED_EOI) )
> > +                gic_irq_eoi(p->irq);
> > +        }
> >           clear_bit(GIC_IRQ_GUEST_VISIBLE, &p->status);
> >           clear_bit(GIC_IRQ_GUEST_ACTIVE, &p->status);
> >           p->lr = GIC_INVALID_LR;
> 
> With this change, shouldn't we clear the bit and the LR before EOI the IRQ?

Conceptually you might be right but it is all happening with the  vgic
lock taken, so it shouldn't make any difference.