Patchwork arm: make sure that number of irqs can be represented in GICD_TYPER.

login
register
mail settings
Submitter Rusty Russell
Date Feb. 19, 2012, 11:07 p.m.
Message ID <87fwe68lt0.fsf@rustcorp.com.au>
Download mbox | patch
Permalink /patch/142079/
State New
Headers show

Comments

Rusty Russell - Feb. 19, 2012, 11:07 p.m.
We currently assume that the number of interrupts (ITLinesNumber in
the architecture reference manual) is divisible by 32, since we
present it to the guest when it reads GICD_TYPER (in gic_dist_readb())
as (N - 32) / 1.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Christoffer Dall - Feb. 19, 2012, 11:40 p.m.
On Sun, Feb 19, 2012 at 6:07 PM, Rusty Russell <rusty@rustcorp.com.au> wrote:
> We currently assume that the number of interrupts (ITLinesNumber in
> the architecture reference manual) is divisible by 32, since we
> present it to the guest when it reads GICD_TYPER (in gic_dist_readb())
> as (N - 32) / 1.
>
> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
>
> diff --git a/hw/arm_gic.c b/hw/arm_gic.c
> index fa6a60a..6446800 100644
> --- a/hw/arm_gic.c
> +++ b/hw/arm_gic.c
> @@ -818,6 +818,15 @@ static void gic_init(gic_state *s, int num_irq)
>         hw_error("requested %u interrupt lines exceeds GIC maximum %d\n",
>                  num_irq, GIC_MAXIRQ);
>     }
> +    /* ITLinesNumber is represented as (N - 32) / 1 (see
> +     * gic_dist_readb) so this is an implementation imposed
> +     * restriction, not an architectural one:
> +     */

What is this division by 1? Isn't (N - 32) / 1 == (N - 32) and how
does that explain the division. Was it (N / 32) - 1? Or is this a
cast? Or am I missing something completely?

> +    if (s->num_irq < 32 || (s->num_irq % 32)) {
> +        hw_error("%d interrupt lines unsupported: not divisible by 32\n",
> +                 num_irq);
> +    }
> +
>     qdev_init_gpio_in(&s->busdev.qdev, gic_set_irq, s->num_irq - GIC_INTERNAL);
>     for (i = 0; i < NUM_CPU(s); i++) {
>         sysbus_init_irq(&s->busdev, &s->parent_irq[i]);
> _______________________________________________
> Android-virt mailing list
> Android-virt@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/cucslists/listinfo/android-virt
Rusty Russell - Feb. 20, 2012, 3:52 a.m.
On Sun, 19 Feb 2012 18:40:08 -0500, Christoffer Dall <c.dall@virtualopensystems.com> wrote:
> On Sun, Feb 19, 2012 at 6:07 PM, Rusty Russell <rusty@rustcorp.com.au> wrote:
> > We currently assume that the number of interrupts (ITLinesNumber in
> > the architecture reference manual) is divisible by 32, since we
> > present it to the guest when it reads GICD_TYPER (in gic_dist_readb())
> > as (N - 32) / 1.
> >
> > Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
> >
> > diff --git a/hw/arm_gic.c b/hw/arm_gic.c
> > index fa6a60a..6446800 100644
> > --- a/hw/arm_gic.c
> > +++ b/hw/arm_gic.c
> > @@ -818,6 +818,15 @@ static void gic_init(gic_state *s, int num_irq)
> >         hw_error("requested %u interrupt lines exceeds GIC maximum %d\n",
> >                  num_irq, GIC_MAXIRQ);
> >     }
> > +    /* ITLinesNumber is represented as (N - 32) / 1 (see
> > +     * gic_dist_readb) so this is an implementation imposed
> > +     * restriction, not an architectural one:
> > +     */
> 
> What is this division by 1? Isn't (N - 32) / 1 == (N - 32) and how
> does that explain the division. Was it (N / 32) - 1? Or is this a
> cast? Or am I missing something completely?

That's hilarious; we were concentrating so hard on the wording of the
comment, we didn't notice that I completely screwed the content :)

It is, of course, (N / 32) - 1:

static uint32_t gic_dist_readb(void *opaque, target_phys_addr_t offset)
{
...
        if (offset == 4)
            return ((GIC_NIRQ / 32) - 1) | ((NUM_CPU(s) - 1) << 5);

I'll fix it, thanks!
Rusty.

Patch

diff --git a/hw/arm_gic.c b/hw/arm_gic.c
index fa6a60a..6446800 100644
--- a/hw/arm_gic.c
+++ b/hw/arm_gic.c
@@ -818,6 +818,15 @@  static void gic_init(gic_state *s, int num_irq)
         hw_error("requested %u interrupt lines exceeds GIC maximum %d\n",
                  num_irq, GIC_MAXIRQ);
     }
+    /* ITLinesNumber is represented as (N - 32) / 1 (see
+     * gic_dist_readb) so this is an implementation imposed
+     * restriction, not an architectural one:
+     */
+    if (s->num_irq < 32 || (s->num_irq % 32)) {
+        hw_error("%d interrupt lines unsupported: not divisible by 32\n",
+                 num_irq);
+    }
+
     qdev_init_gpio_in(&s->busdev.qdev, gic_set_irq, s->num_irq - GIC_INTERNAL);
     for (i = 0; i < NUM_CPU(s); i++) {
         sysbus_init_irq(&s->busdev, &s->parent_irq[i]);