Re: [Xen-devel] [PATCH v3 03/11] arm: support fewer LR registers than virtual irqs

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2012-03-02 at 14:27 +0000, Stefano Stabellini wrote:
> ...

Is this the final version of this patch?

There is a v4 in my queue[0] but it seems to predate this one.

Ian.
[0] <1330016454-19752-1-git-send-email-stefano.stabellini@xxxxxxxxxxxxx>

> If the vgic needs to inject a virtual irq into the guest, but no free
> LR registers are available, add the irq to a list and return.
> Whenever an LR register becomes available we add the queued irq to it
> and remove it from the list.
> We use the gic lock to protect the list and the bitmask.
> 
> 
> Changes in this version:
> 
> - added some comments;
> 
> - rename lr_link to lr_queue;
> 
> - fix list handling in gic_set_guest_irq;
> 
> - use nr_lrs instead of sizeof(uint64_t) as argument to
> find_first_zero_bit.
> 
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> ---
>  xen/arch/arm/gic.c           |  101 ++++++++++++++++++++++++++++++++---------
>  xen/include/asm-arm/domain.h |   10 ++++
>  2 files changed, 89 insertions(+), 22 deletions(-)
> 
> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> index 120ec82..72b122e 100644
> --- a/xen/arch/arm/gic.c
> +++ b/xen/arch/arm/gic.c
> @@ -25,6 +25,7 @@
>  #include <xen/sched.h>
>  #include <xen/errno.h>
>  #include <xen/softirq.h>
> +#include <xen/list.h>
>  #include <asm/p2m.h>
>  #include <asm/domain.h>
>  
> @@ -45,6 +46,14 @@ static struct {
>      unsigned int lines;
>      unsigned int cpus;
>      spinlock_t lock;
> +    uint64_t lr_mask;
> +    /* lr_pending is used to queue IRQs (struct pending_irq) that the
> +     * vgic tried to inject in the guest (calling gic_set_guest_irq) but
> +     * no LRs were available at the time.
> +     * As soon as an LR is freed we remove the first IRQ from this
> +     * list and write it to the LR register.
> +     * lr_pending is a subset of vgic.inflight_irqs. */
> +    struct list_head lr_pending;
>  } gic;
>  
>  irq_desc_t irq_desc[NR_IRQS];
> @@ -247,6 +256,8 @@ static void __cpuinit gic_hyp_init(void)
>  
>      GICH[GICH_HCR] = GICH_HCR_EN;
>      GICH[GICH_MISR] = GICH_MISR_EOI;
> +    gic.lr_mask = 0ULL;
> +    INIT_LIST_HEAD(&gic.lr_pending);
>  }
>  
>  /* Set up the GIC */
> @@ -345,16 +356,50 @@ int __init setup_irq(unsigned int irq, struct irqaction 
> *new)
>      return rc;
>  }
>  
> -void gic_set_guest_irq(unsigned int virtual_irq,
> +static inline void gic_set_lr(int lr, unsigned int virtual_irq,
>          unsigned int state, unsigned int priority)
>  {
> -    BUG_ON(virtual_irq > nr_lrs);
> -    GICH[GICH_LR + virtual_irq] = state |
> +    BUG_ON(lr > nr_lrs);
> +    GICH[GICH_LR + lr] = state |
>          GICH_LR_MAINTENANCE_IRQ |
>          ((priority >> 3) << GICH_LR_PRIORITY_SHIFT) |
>          ((virtual_irq & GICH_LR_VIRTUAL_MASK) << GICH_LR_VIRTUAL_SHIFT);
>  }
>  
> +void gic_set_guest_irq(unsigned int virtual_irq,
> +        unsigned int state, unsigned int priority)
> +{
> +    int i;
> +    struct pending_irq *iter, *n;
> +
> +    spin_lock(&gic.lock);
> +
> +    if ( list_empty(&gic.lr_pending) )
> +    {
> +        i = find_first_zero_bit(&gic.lr_mask, nr_lrs);
> +        if (i < nr_lrs) {
> +            set_bit(i, &gic.lr_mask);
> +            gic_set_lr(i, virtual_irq, state, priority);
> +            goto out;
> +        }
> +    }
> +
> +    n = irq_to_pending(current, virtual_irq);
> +    list_for_each_entry ( iter, &gic.lr_pending, lr_queue )
> +    {
> +        if ( iter->priority > priority )
> +        {
> +            list_add_tail(&n->lr_queue, &iter->lr_queue);
> +            goto out;
> +        }
> +    }
> +    list_add_tail(&n->lr_queue, &gic.lr_pending);
> +
> +out:
> +    spin_unlock(&gic.lock);
> +    return;
> +}
> +
>  void gic_inject_irq_start(void)
>  {
>      uint32_t hcr;
> @@ -431,30 +476,42 @@ void gicv_setup(struct domain *d)
>  
>  static void maintenance_interrupt(int irq, void *dev_id, struct 
> cpu_user_regs *regs)
>  {
> -    int i, virq;
> +    int i = 0, virq;
>      uint32_t lr;
>      uint64_t eisr = GICH[GICH_EISR0] | (((uint64_t) GICH[GICH_EISR1]) << 32);
>  
> -    for ( i = 0; i < 64; i++ ) {
> -        if ( eisr & ((uint64_t)1 << i) ) {
> -            struct pending_irq *p;
> -
> -            lr = GICH[GICH_LR + i];
> -            virq = lr & GICH_LR_VIRTUAL_MASK;
> -            GICH[GICH_LR + i] = 0;
> -
> -            spin_lock(&current->arch.vgic.lock);
> -            p = irq_to_pending(current, virq);
> -            if ( p->desc != NULL ) {
> -                p->desc->status &= ~IRQ_INPROGRESS;
> -                GICC[GICC_DIR] = virq;
> -            }
> +    while ((i = find_next_bit((const long unsigned int *) &eisr,
> +                              sizeof(eisr), i)) < sizeof(eisr)) {
> +        struct pending_irq *p;
> +
> +        spin_lock(&gic.lock);
> +        lr = GICH[GICH_LR + i];
> +        virq = lr & GICH_LR_VIRTUAL_MASK;
> +        GICH[GICH_LR + i] = 0;
> +        clear_bit(i, &gic.lr_mask);
> +
> +        if ( !list_empty(&gic.lr_pending) ) {
> +            p = list_entry(gic.lr_pending.next, typeof(*p), lr_queue);
> +            gic_set_lr(i, p->irq, GICH_LR_PENDING, p->priority);
> +            list_del_init(&p->lr_queue);
> +            set_bit(i, &gic.lr_mask);
> +        } else {
>              gic_inject_irq_stop();
> -            list_del(&p->inflight);
> -            INIT_LIST_HEAD(&p->inflight);
> -            cpu_raise_softirq(current->processor, VGIC_SOFTIRQ);
> -            spin_unlock(&current->arch.vgic.lock);
>          }
> +        spin_unlock(&gic.lock);
> +
> +        spin_lock(&current->arch.vgic.lock);
> +        p = irq_to_pending(current, virq);
> +        if ( p->desc != NULL ) {
> +            p->desc->status &= ~IRQ_INPROGRESS;
> +            GICC[GICC_DIR] = virq;
> +        }
> +        list_del(&p->inflight);
> +        INIT_LIST_HEAD(&p->inflight);
> +        cpu_raise_softirq(current->processor, VGIC_SOFTIRQ);
> +        spin_unlock(&current->arch.vgic.lock);
> +
> +        i++;
>      }
>  }
>  
> diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
> index 9851220..3aa7a8a 100644
> --- a/xen/include/asm-arm/domain.h
> +++ b/xen/include/asm-arm/domain.h
> @@ -23,6 +23,9 @@ struct pending_irq
>      /* inflight is used to append instances of pending_irq to
>       * vgic.inflight_irqs */
>      struct list_head inflight;
> +    /* lr_queue is used to append instances of pending_irq to
> +     * gic.lr_pending */
> +    struct list_head lr_queue;
>  };
>  
>  struct arch_domain
> @@ -56,6 +59,13 @@ struct arch_vcpu
>  
>      struct {
>          struct vgic_irq_rank private_irqs;
> +        /* This list is ordered by IRQ priority and it is used to keep
> +         * track of the IRQs that the VGIC injected into the guest.
> +         * Depending on the availability of LR registers, the IRQs might
> +         * actually be in an LR, and therefore injected into the guest,
> +         * or queued in gic.lr_pending.
> +         * As soon as an IRQ is EOI'd by the guest and removed from the
> +         * corresponding LR it is also removed from this list. */
>          struct list_head inflight_irqs;
>          spinlock_t lock;
>      } vgic;



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