Julien Grall
2013-Apr-29 13:25 UTC
[PATCH V2] xen/arm: Missing +1 when then number of interrupt lines for the GIC is computed
In the GIC manual, the number of interrupt lines is computed with the following
formula: 32(N + 1) where N is the value retrieved from GICD_TYPER.
Without the +1 Xen doesn''t initialize the last 32 interrupts and can
get
garbagge on these registers.
Signed-off-by: Julien Grall <julien.grall@linaro.org>
---
Modifications since v1:
- Precise the impact in the commit message
- Add description for the variable "lines"
xen/arch/arm/gic.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 377b676..0fd8635 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -44,7 +44,7 @@ static struct {
paddr_t cbase; /* Address of CPU interface registers */
paddr_t hbase; /* Address of virtual interface registers */
paddr_t vbase; /* Address of virtual cpu interface registers */
- unsigned int lines;
+ unsigned int lines; /* Number of interrupts (SPIs + PPIs + SGIs) */
unsigned int cpus;
spinlock_t lock;
} gic;
@@ -214,7 +214,7 @@ static int gic_route_irq(unsigned int irq, bool_t level,
ASSERT(!(cpu_mask & ~0xff)); /* Targets bitmap only supports 8 CPUs */
ASSERT(priority <= 0xff); /* Only 8 bits of priority */
- ASSERT(irq < gic.lines + 32); /* Can''t route interrupts that
don''t exist */
+ ASSERT(irq < gic.lines); /* Can''t route interrupts that
don''t exist */
spin_lock_irqsave(&desc->lock, flags);
spin_lock(&gic.lock);
@@ -251,7 +251,7 @@ static void __init gic_dist_init(void)
GICD[GICD_CTLR] = 0;
type = GICD[GICD_TYPER];
- gic.lines = 32 * (type & GICD_TYPE_LINES);
+ gic.lines = 32 * ((type & GICD_TYPE_LINES) + 1);
gic.cpus = 1 + ((type & GICD_TYPE_CPUS) >> 5);
printk("GIC: %d lines, %d cpu%s%s (IID %8.8x).\n",
gic.lines, gic.cpus, (gic.cpus == 1) ? "" : "s",
--
1.7.10.4
Ian Campbell
2013-Apr-30 10:57 UTC
Re: [PATCH V2] xen/arm: Missing +1 when then number of interrupt lines for the GIC is computed
On Mon, 2013-04-29 at 14:25 +0100, Julien Grall wrote:> In the GIC manual, the number of interrupt lines is computed with the following > formula: 32(N + 1) where N is the value retrieved from GICD_TYPER. > > Without the +1 Xen doesn''t initialize the last 32 interrupts and can get > garbagge on these registers. > > Signed-off-by: Julien Grall <julien.grall@linaro.org>Acked + applied. I rewrote the first line to fit in 80 characters: "xen/arm: correct the computation of the number of interrupt lines for the GIC"> --- > Modifications since v1: > - Precise the impact in the commit message > - Add description for the variable "lines" > > xen/arch/arm/gic.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c > index 377b676..0fd8635 100644 > --- a/xen/arch/arm/gic.c > +++ b/xen/arch/arm/gic.c > @@ -44,7 +44,7 @@ static struct { > paddr_t cbase; /* Address of CPU interface registers */ > paddr_t hbase; /* Address of virtual interface registers */ > paddr_t vbase; /* Address of virtual cpu interface registers */ > - unsigned int lines; > + unsigned int lines; /* Number of interrupts (SPIs + PPIs + SGIs) */ > unsigned int cpus; > spinlock_t lock; > } gic; > @@ -214,7 +214,7 @@ static int gic_route_irq(unsigned int irq, bool_t level, > > ASSERT(!(cpu_mask & ~0xff)); /* Targets bitmap only supports 8 CPUs */ > ASSERT(priority <= 0xff); /* Only 8 bits of priority */ > - ASSERT(irq < gic.lines + 32); /* Can''t route interrupts that don''t exist */ > + ASSERT(irq < gic.lines); /* Can''t route interrupts that don''t exist */ > > spin_lock_irqsave(&desc->lock, flags); > spin_lock(&gic.lock); > @@ -251,7 +251,7 @@ static void __init gic_dist_init(void) > GICD[GICD_CTLR] = 0; > > type = GICD[GICD_TYPER]; > - gic.lines = 32 * (type & GICD_TYPE_LINES); > + gic.lines = 32 * ((type & GICD_TYPE_LINES) + 1); > gic.cpus = 1 + ((type & GICD_TYPE_CPUS) >> 5); > printk("GIC: %d lines, %d cpu%s%s (IID %8.8x).\n", > gic.lines, gic.cpus, (gic.cpus == 1) ? "" : "s",