Re: [Xen-devel] [PATCH 27/38] arm: split pending SPIs (global) out from pending PPIs and SGIs (per CPU)

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Thu, 2012-06-07 at 11:35 +0100, Stefano Stabellini wrote:
> On Fri, 1 Jun 2012, Ian Campbell wrote:
> > This tracks SPIs in struct arch_domain and PPIs+SGIs in struct arch_vcpu 
> > which
> > seems more logical.
> > 
> > Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
> > ---
> >  xen/arch/arm/vgic.c          |   17 ++++++++++-------
> >  xen/include/asm-arm/domain.h |   10 ++++++++++
> >  2 files changed, 20 insertions(+), 7 deletions(-)
> > 
> > diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
> > index 629a0da..91d6166 100644
> > --- a/xen/arch/arm/vgic.c
> > +++ b/xen/arch/arm/vgic.c
> > @@ -82,9 +82,8 @@ int domain_vgic_init(struct domain *d)
> >      d->arch.vgic.shared_irqs =
> >          xmalloc_array(struct vgic_irq_rank, DOMAIN_NR_RANKS(d));
> >      d->arch.vgic.pending_irqs =
> > -        xmalloc_array(struct pending_irq,
> > -                d->arch.vgic.nr_lines + (32 * d->max_vcpus));
> > -    for (i=0; i<d->arch.vgic.nr_lines + (32 * d->max_vcpus); i++)
> > +        xzalloc_array(struct pending_irq, d->arch.vgic.nr_lines);
> > +    for (i=0; i<d->arch.vgic.nr_lines; i++)
> >          INIT_LIST_HEAD(&d->arch.vgic.pending_irqs[i].inflight);
> >      for (i=0; i<DOMAIN_NR_RANKS(d); i++)
> >          spin_lock_init(&d->arch.vgic.shared_irqs[i].lock);
> > @@ -98,6 +97,10 @@ int vcpu_vgic_init(struct vcpu *v)
> >  
> >      spin_lock_init(&v->arch.vgic.private_irqs.lock);
> >  
> > +    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);
> > +
> >      /* For SGI and PPI the target is always this CPU */
> >      for ( i = 0 ; i < 8 ; i++ )
> >          v->arch.vgic.private_irqs.itargets[i] =
> > @@ -535,8 +538,7 @@ struct pending_irq *irq_to_pending(struct vcpu *v, 
> > unsigned int irq)
> >      /* Pending irqs allocation strategy: the first vgic.nr_lines irqs
> >       * are used for SPIs; the rests are used for per cpu irqs */
> >      if ( irq < 32 )
> > -        n = &v->domain->arch.vgic.pending_irqs[irq + (v->vcpu_id * 32)
> > -            + v->domain->arch.vgic.nr_lines];
> > +        n = &v->arch.vgic.pending_irqs[irq];
> >      else
> >          n = &v->domain->arch.vgic.pending_irqs[irq - 32];
> >      return n;
> > @@ -548,6 +550,7 @@ void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int 
> > irq, int virtual)
> >      uint8_t priority;
> >      struct vgic_irq_rank *rank = vgic_irq_rank(v, 8, idx);
> >      struct pending_irq *iter, *n = irq_to_pending(v, irq);
> > +    unsigned long flags;
> >  
> >      /* irq still pending */
> >      if (!list_empty(&n->inflight))
> > @@ -564,7 +567,7 @@ void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int 
> > irq, int virtual)
> >  
> >      gic_set_guest_irq(irq, GICH_LR_PENDING, priority);
> >  
> > -    spin_lock(&v->arch.vgic.lock);
> > +    spin_lock_irqsave(&v->arch.vgic.lock, flags);
> >      list_for_each_entry ( iter, &v->arch.vgic.inflight_irqs, inflight )
> >      {
> >          if ( iter->priority > priority )
> > @@ -575,7 +578,7 @@ void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int 
> > irq, int virtual)
> >          }
> >      }
> >      list_add_tail(&n->inflight, &v->arch.vgic.inflight_irqs);
> > -    spin_unlock(&v->arch.vgic.lock);
> > +    spin_unlock_irqrestore(&v->arch.vgic.lock, flags);
> >      /* we have a new higher priority irq, inject it into the guest */
> >  }
> 
> Besides moving PPIs and SGIs to struct vcpu, this patch also turns
> spin_lock into spin_lock_irqsave in vgic_vcpu_inject_irq: I think it is
> correct because it can be called in IRQ context,

good point.

> but it needs to be explicitly stated in the commit message.

I've split it into its own patch instead:

8<----------------------------------------

>From 6d4f199127488480f9a9cc2a94bb73734a75ff88 Mon Sep 17 00:00:00 2001
From: Ian Campbell <ian.campbell@xxxxxxxxxx>
Date: Tue, 26 Jun 2012 09:00:55 +0000
Subject: [PATCH] arm: use interrupt safe spin locks in vgic_vcpu_inject_irq

This function can be called in both interrupt and regular context.

Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
---
 xen/arch/arm/vgic.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index af3523f..91d6166 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -550,6 +550,7 @@ void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int irq, 
int virtual)
     uint8_t priority;
     struct vgic_irq_rank *rank = vgic_irq_rank(v, 8, idx);
     struct pending_irq *iter, *n = irq_to_pending(v, irq);
+    unsigned long flags;
 
     /* irq still pending */
     if (!list_empty(&n->inflight))
@@ -566,7 +567,7 @@ void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int irq, 
int virtual)
 
     gic_set_guest_irq(irq, GICH_LR_PENDING, priority);
 
-    spin_lock(&v->arch.vgic.lock);
+    spin_lock_irqsave(&v->arch.vgic.lock, flags);
     list_for_each_entry ( iter, &v->arch.vgic.inflight_irqs, inflight )
     {
         if ( iter->priority > priority )
@@ -577,7 +578,7 @@ void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int irq, 
int virtual)
         }
     }
     list_add_tail(&n->inflight, &v->arch.vgic.inflight_irqs);
-    spin_unlock(&v->arch.vgic.lock);
+    spin_unlock_irqrestore(&v->arch.vgic.lock, flags);
     /* we have a new higher priority irq, inject it into the guest */
 }
 
-- 
1.7.9.1




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