Re: [Xen-devel] [PATCH v4 10/10] xen/arm: gic_events_need_delivery and irq priorities

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Fri, 21 Mar 2014, Ian Campbell wrote:
> On Wed, 2014-03-19 at 12:32 +0000, Stefano Stabellini wrote:
> > gic_events_need_delivery should only return positive if an outstanding
> > pending irq has an higher priority than the currently active irq and the
> > priority mask.
> > Rewrite the function by going through the priority ordered inflight and
> > lr_queue lists.
> > 
> > In gic_restore_pending_irqs replace lower priority pending (and not
> > active) irqs in GICH_LRs with higher priority irqs if no more GICH_LRs
> > are available.
> > 
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> > 
> > ---
> > Changes in v4:
> > - in gic_events_need_delivery go through inflight_irqs and only consider
> > enabled irqs.
> > ---
> >  xen/arch/arm/gic.c           |   71 
> > +++++++++++++++++++++++++++++++++++++-----
> >  xen/include/asm-arm/domain.h |    5 +--
> >  xen/include/asm-arm/gic.h    |    3 ++
> >  3 files changed, 70 insertions(+), 9 deletions(-)
> > 
> > diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> > index bc9d66d..533964e 100644
> > --- a/xen/arch/arm/gic.c
> > +++ b/xen/arch/arm/gic.c
> > @@ -709,6 +709,7 @@ static void _gic_clear_lr(struct vcpu *v, int i)
> >      p = irq_to_pending(v, irq);
> >      if ( lr & GICH_LR_ACTIVE )
> >      {
> > +        set_bit(GIC_IRQ_GUEST_ACTIVE, &p->status);
> >          /* HW interrupts cannot be ACTIVE and PENDING */
> >          if ( p->desc == NULL &&
> >               test_bit(GIC_IRQ_GUEST_ENABLED, &p->status) &&
> > @@ -723,6 +724,7 @@ static void _gic_clear_lr(struct vcpu *v, int i)
> >          if ( p->desc != NULL )
> >              p->desc->status &= ~IRQ_INPROGRESS;
> >          clear_bit(GIC_IRQ_GUEST_VISIBLE, &p->status);
> > +        clear_bit(GIC_IRQ_GUEST_ACTIVE, &p->status);
> >          p->lr = nr_lrs;
> >          if ( test_bit(GIC_IRQ_GUEST_PENDING, &p->status) &&
> >                  test_bit(GIC_IRQ_GUEST_ENABLED, &p->status))
> > @@ -750,22 +752,47 @@ void gic_clear_lrs(struct vcpu *v)
> >  
> >  static void gic_restore_pending_irqs(struct vcpu *v)
> >  {
> > -    int i;
> > -    struct pending_irq *p, *t;
> > +    int i = 0, lrs = nr_lrs;
> > +    struct pending_irq *p, *t, *p_r;
> >      unsigned long flags;
> >  
> > +    if ( list_empty(&v->arch.vgic.lr_pending) )
> > +        return;
> > +
> > +    spin_lock_irqsave(&v->arch.vgic.lock, flags);
> > +
> > +    p_r = list_entry(v->arch.vgic.inflight_irqs.prev,
> > +                         typeof(*p_r), inflight);
> 
> Is this getting the tail of the list or something else?
> 
> >      list_for_each_entry_safe ( p, t, &v->arch.vgic.lr_pending, lr_queue )
> >      {
> >          i = find_first_zero_bit(&this_cpu(lr_mask), nr_lrs);
> > -        if ( i >= nr_lrs ) return;
> > +        if ( i >= nr_lrs )
> > +        {
> > +            while ( !test_bit(GIC_IRQ_GUEST_VISIBLE, &p_r->status) ||
> > +                    test_bit(GIC_IRQ_GUEST_ACTIVE, &p_r->status) )
> > +            {
> > +                p_r = list_entry(p_r->inflight.prev, typeof(*p_r), 
> > inflight);
> 
> Oh, maybe this (and the thing above) is an open coded list_for_each_prev
> or one of its variants? e.g. list_for_each_entry_reverse?

Yes, it is a list_for_each_entry_reverse that starts from the current
p_r and stops when it finds the first entry that is
GIC_IRQ_GUEST_VISIBLE and GIC_IRQ_GUEST_ACTIVE.

It can also stop if the next entry that would be found going through
inflight in reverse is the same that we are evaluating going through
lr_pending in regular order.


> > +                if ( &p_r->inflight == p->inflight.next )
> > +                    goto out;
> > +            }
> > +            i = p_r->lr;
> > +            p_r->lr = nr_lrs;
> > +            set_bit(GIC_IRQ_GUEST_PENDING, &p_r->status);
> > +            clear_bit(GIC_IRQ_GUEST_VISIBLE, &p_r->status);
> > +            gic_add_to_lr_pending(v, p_r);
> > +        }
> >  
> > -        spin_lock_irqsave(&v->arch.vgic.lock, flags);
> >          gic_set_lr(i, p, GICH_LR_PENDING);
> >          list_del_init(&p->lr_queue);
> >          set_bit(i, &this_cpu(lr_mask));
> > -        spin_unlock_irqrestore(&v->arch.vgic.lock, flags);
> > +
> > +        lrs--;
> > +        if ( lrs == 0 )
> > +            break;
> >      }
> >  
> > +out:
> > +    spin_unlock_irqrestore(&v->arch.vgic.lock, flags);
> >  }
> >  
> >  void gic_clear_pending_irqs(struct vcpu *v)
> 
> 

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