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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

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?

> +                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