Re: [Xen-devel] [PATCH v8 09/13] xen/arm: second irq injection while the first irq is still inflight

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Thu, 22 May 2014, Julien Grall wrote:
> > while the first one is still active.
> > If the first irq is already pending (not active), clear
> > GIC_IRQ_GUEST_QUEUED because the guest doesn't need a second
> > notification.If the irq has already been EOI'ed then just clear the
> > GICH_LR right away and move the interrupt to lr_pending so that it is
> > going to be reinjected by gic_restore_pending_irqs on return to guest.
> > 
> > If the target cpu is not the current cpu, then set GIC_IRQ_GUEST_QUEUED
> > and send an SGI. The target cpu is going to be interrupted and call
> > gic_clear_lrs, that is going to take the same actions.
> > 
> > Do not call vgic_vcpu_inject_irq from gic_inject if
> > evtchn_upcall_pending is set. If we remove that call, we don't need to
> > special case evtchn_irq in vgic_vcpu_inject_irq anymore.
> > We need to force the first injection of evtchn_irq (call
> > gic_vcpu_inject_irq) from vgic_enable_irqs because evtchn_upcall_pending
> > is already set by common code on vcpu creation.
> 
> If you only need it for the first time. Why can't you call vgic_inject_irq
> with the IRQ evtchn when the VCPU is turn on?
> 
> This would remove every hack with this IRQ in the GIC code.

In principle sounds nice, but in practice it is difficult and risks
being racy. In vgic_vcpu_inject_irq we have:

    /* vcpu offline */
    if ( test_bit(_VPF_down, &v->pause_flags) )
    {
        spin_unlock_irqrestore(&v->arch.vgic.lock, flags);
        return;
    }

So we can only inject the irq once the vcpu is properly up, that is
certainly later than vcpu_initialise.


> > ---
> >   xen/arch/arm/gic.c        |   48
> > +++++++++++++++++++++++++++++++++++++--------
> >   xen/arch/arm/vgic.c       |   11 +++++++----
> >   xen/include/asm-arm/gic.h |    1 +
> >   3 files changed, 48 insertions(+), 12 deletions(-)
> > 
> > diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> > index 89d7025..a6fe566 100644
> > --- a/xen/arch/arm/gic.c
> > +++ b/xen/arch/arm/gic.c
> > @@ -66,6 +66,8 @@ static DEFINE_PER_CPU(u8, gic_cpu_id);
> >   /* Maximum cpu interface per GIC */
> >   #define NR_GIC_CPU_IF 8
> > 
> > +#undef GIC_DEBUG
> > +
> 
> Did you intend to keep the debug in the final patch?

Yes, I think it is useful.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel