| Message ID | 1390581822-32624-2-git-send-email-julien.grall@linaro.org |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Fri, 2014-01-24 at 16:43 +0000, Julien Grall wrote: > The function gic_set_irq_properties is only called in two places: > - gic_route_irq: the gic.lock is only taken for the call to the > former function. > - gic_route_irq_to_guest: the gic.lock is taken for the duration of > the function. But the lock is only useful when gic_set_irq_properties. > > So we can safely move the lock in gic_set_irq_properties and restrict the > critical section for the gic.lock in gic_route_irq_to_guest. > > Signed-off-by: Julien Grall <julien.grall@linaro.org> Acked-by: Ian Campbell <ian.campbell@citrix.com> Although ISTR Stefano saying he had got rid of the lock altogether. I'll let you to battle that one out ;-)
On 02/19/2014 11:23 AM, Ian Campbell wrote: > On Fri, 2014-01-24 at 16:43 +0000, Julien Grall wrote: >> The function gic_set_irq_properties is only called in two places: >> - gic_route_irq: the gic.lock is only taken for the call to the >> former function. >> - gic_route_irq_to_guest: the gic.lock is taken for the duration of >> the function. But the lock is only useful when gic_set_irq_properties. >> >> So we can safely move the lock in gic_set_irq_properties and restrict the >> critical section for the gic.lock in gic_route_irq_to_guest. >> >> Signed-off-by: Julien Grall <julien.grall@linaro.org> > > Acked-by: Ian Campbell <ian.campbell@citrix.com> Thanks for the ack. > Although ISTR Stefano saying he had got rid of the lock altogether. I'll > let you to battle that one out ;-) I can't find any patch on the mailing which remove the lock in theses functions. I will keep it for now.
diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c index e6257a7..1943f92 100644 --- a/xen/arch/arm/gic.c +++ b/xen/arch/arm/gic.c @@ -228,7 +228,11 @@ static void gic_set_irq_properties(unsigned int irq, bool_t level, { volatile unsigned char *bytereg; uint32_t cfg, edgebit; - unsigned int mask = gic_cpu_mask(cpu_mask); + unsigned int mask; + + spin_lock(&gic.lock); + + mask = gic_cpu_mask(cpu_mask); /* Set edge / level */ cfg = GICD[GICD_ICFGR + irq / 16]; @@ -247,6 +251,7 @@ static void gic_set_irq_properties(unsigned int irq, bool_t level, bytereg = (unsigned char *) (GICD + GICD_IPRIORITYR); bytereg[irq] = priority; + spin_unlock(&gic.lock); } /* Program the GIC to route an interrupt */ @@ -269,9 +274,7 @@ static int gic_route_irq(unsigned int irq, bool_t level, desc->handler = &gic_host_irq_type; - spin_lock(&gic.lock); gic_set_irq_properties(irq, level, cpu_mask, priority); - spin_unlock(&gic.lock); spin_unlock_irqrestore(&desc->lock, flags); return 0; @@ -769,7 +772,6 @@ int gic_route_irq_to_guest(struct domain *d, const struct dt_irq *irq, action->free_on_release = 1; spin_lock_irqsave(&desc->lock, flags); - spin_lock(&gic.lock); desc->handler = &gic_guest_irq_type; desc->status |= IRQ_GUEST; @@ -790,7 +792,6 @@ int gic_route_irq_to_guest(struct domain *d, const struct dt_irq *irq, p->desc = desc; out: - spin_unlock(&gic.lock); spin_unlock_irqrestore(&desc->lock, flags); return retval; }
The function gic_set_irq_properties is only called in two places: - gic_route_irq: the gic.lock is only taken for the call to the former function. - gic_route_irq_to_guest: the gic.lock is taken for the duration of the function. But the lock is only useful when gic_set_irq_properties. So we can safely move the lock in gic_set_irq_properties and restrict the critical section for the gic.lock in gic_route_irq_to_guest. Signed-off-by: Julien Grall <julien.grall@linaro.org> --- xen/arch/arm/gic.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)