Re: [Xen-devel] [PATCH v2 04/21] xen/arm: vgic: Introduce a function to initialize pending_irq

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Thu, 31 Jul 2014, Julien Grall wrote:
> The structure pending_irq is initialized on the same way in 2 differents
> place. Introduce vgic_init_pending_irq to avoid code duplication.
> 
> Also move the setting of the irq field in this function as we need to
> initialize it once rather than every time an IRQ is injected to the guest.
> 
> Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>
 


>     Changes in v2:
>         - Patch added
> ---
>  xen/arch/arm/vgic.c |   19 ++++++++++---------
>  1 file changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
> index ac34437..17cde7a 100644
> --- a/xen/arch/arm/vgic.c
> +++ b/xen/arch/arm/vgic.c
> @@ -53,6 +53,13 @@ struct vgic_irq_rank *vgic_rank_irq(struct vcpu *v, 
> unsigned int irq)
>      return vgic_rank_offset(v, 8, irq, DABT_WORD);
>  }
>  
> +static void vgic_init_pending_irq(struct pending_irq *p, unsigned virq)
> +{
> +    INIT_LIST_HEAD(&p->inflight);
> +    INIT_LIST_HEAD(&p->lr_queue);
> +    p->irq = virq;
> +}

Reading this code made me realize that the irq field in pending_irq is a
signed integer, while virq is unsigned. We should be consistent.

In any case this patch is still an improvement over what we have now:

Acked-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>


>  int domain_vgic_init(struct domain *d)
>  {
>      int i;
> @@ -88,10 +95,8 @@ int domain_vgic_init(struct domain *d)
>      }
>  
>      for (i=0; i<d->arch.vgic.nr_spis; i++)
> -    {
> -        INIT_LIST_HEAD(&d->arch.vgic.pending_irqs[i].inflight);
> -        INIT_LIST_HEAD(&d->arch.vgic.pending_irqs[i].lr_queue);
> -    }
> +        vgic_init_pending_irq(&d->arch.vgic.pending_irqs[i], i + 32);
> +
>      for (i=0; i<DOMAIN_NR_RANKS(d); i++)
>      {
>          spin_lock_init(&d->arch.vgic.shared_irqs[i].lock);
> @@ -131,10 +136,7 @@ int vcpu_vgic_init(struct vcpu *v)
>  
>      memset(&v->arch.vgic.pending_irqs, 0, sizeof(v->arch.vgic.pending_irqs));
>      for (i = 0; i < 32; i++)
> -    {
> -        INIT_LIST_HEAD(&v->arch.vgic.pending_irqs[i].inflight);
> -        INIT_LIST_HEAD(&v->arch.vgic.pending_irqs[i].lr_queue);
> -    }
> +        vgic_init_pending_irq(&v->arch.vgic.pending_irqs[i], i);
>  
>      INIT_LIST_HEAD(&v->arch.vgic.inflight_irqs);
>      INIT_LIST_HEAD(&v->arch.vgic.lr_pending);
> @@ -386,7 +388,6 @@ void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int 
> irq)
>          goto out;
>      }
>  
> -    n->irq = irq;
>      n->priority = priority;
>  
>      /* the irq is enabled */
> -- 
> 1.7.10.4
> 

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