On 12/27/2011 03:59 PM, Peter Maydell wrote: > On 27 December 2011 20:13, Mark Langsdorf <mark.langsd...@calxeda.com> wrote: >> Increase the maximum number of GIC interrupts for a9mp to 192, and >> create a configurable property defaulting to 96 so that device >> modelers can set the value appropriately for their SoC. > >> /* Configuration for arm_gic.c: >> * number of external IRQ lines, max number of CPUs, how to ID current CPU >> */ >> -#define GIC_NIRQ 96 >> +#define GIC_NIRQ 192 > > This (still) isn't the maximum number of GIC interrupts for > an A9MP. You want 256.
I know, but I figured "more than anyone would be using" would be sufficient. > Moreover: > now we have a per-cpu #define which is setting the compile-time > maximum on a run-time parameter. That's pretty pointless -- > we should just have arm_gic.c have its own (purely internal) > #define of the maximum number of interrupts it can permit, > and the cpu-specific private peripheral code should only be > setting the run-time parameter. You can drop the #define completely > from a9mpcore.c &co. (don't forget to update the comment) Okay. > The GIC architectural limit on number of interrupts is 1020; > that would (I think) imply a ~64K memory usage by the GIC, > which we can live with I think. I think you just said you wanted me to define the gic internal maximum as 1020. I just want to be sure I understood you there. >> --- a/hw/arm11mpcore.c >> +++ b/hw/arm11mpcore.c >> @@ -132,7 +132,7 @@ static int mpcore_priv_init(SysBusDevice *dev) >> { >> mpcore_priv_state *s = FROM_SYSBUSGIC(mpcore_priv_state, dev); >> >> - gic_init(&s->gic, s->num_cpu); >> + gic_init(&s->gic, s->num_cpu, GIC_NIRQ); > > 11MPCore, like A9MP, has a configurable number of interrupt lines > (up to 256 including the 32 internal ones, also like A9MP). I don't have 11mpcore to test with, but I'll make the change. >> -static void gic_init(gic_state *s) >> +static void gic_init(gic_state *s, int num_irq) >> #endif >> { >> int i; >> @@ -808,7 +809,8 @@ static void gic_init(gic_state *s) >> #if NCPU > 1 >> s->num_cpu = num_cpu; >> #endif >> - qdev_init_gpio_in(&s->busdev.qdev, gic_set_irq, GIC_NIRQ - 32); >> + s->num_irq = num_irq; > > This is where you should be hw_error()ing if num_irq is > too big. > > I didn't see any changes to gic_load/gic_save, which means they > will still be saving all GIC_NIRQ entries in each of the per-irq > state arrays; this means that you've broken save/load compatibility. > I think this can be fixed by having save/load only save/load the > entries which are actually used (ie loop up to s->num_irq rather > than GIC_NIRQ). I had considered that, but wasn't sure enough about the pros and cons. I'll make the changes and thanks for the feedback. --Mark Langsdorf Calxeda, Inc.