Re: [Xen-devel] [PATCH] xen/arm: Missing +1 when then number of interrupt lines for the GIC is computed

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2013-04-24 at 20:44 +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.

My copy of the manual says "The ITLinesNumber field only indicates the
maximum number of SPIs that the GIC might support", which excludes SGIs
and PPIs. On the other hand it also says that 0b0011 == 128 interrupts,
with ID 0..127, and elsewhere it includes SPI and PPI in the term
interrupts.

So it's not really clear, but I think your interpretation is likely
correct.

The impact of getting this count wrong is that currently we don't
initialise the final 32 interrupts worth of the GICD_FOOn registers, is
that right?

I was concerned that we special case the first 32 SG/PPI interrupts in
various other places, but we don't appear to use gic.lines anywhere
other than that.

BTW, I also happened across section 3.1.2 "Identifying the supported
interrupts" which recommends probing GICD_ISENABLERn to determine which
of the 32*(N+1) interrupts are actually available us, might be one for
the todo list (would lead to a useful debugging message "request_irq:
IRQ%d is not available").

> Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>

Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>

> ---
>  xen/arch/arm/gic.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> index 760c86b..389c217 100644
> --- a/xen/arch/arm/gic.c
> +++ b/xen/arch/arm/gic.c
> @@ -216,7 +216,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);
> @@ -264,7 +264,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",



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