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.
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)
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.
> #define NCPU 4
>
> static inline int
> @@ -37,6 +37,7 @@ typedef struct a9mp_priv_state {
> MemoryRegion ptimer_iomem;
> MemoryRegion container;
> DeviceState *mptimer;
> + uint32_t num_irq;
> } a9mp_priv_state;
>
> static uint64_t a9_scu_read(void *opaque, target_phys_addr_t offset,
> @@ -152,8 +153,11 @@ static int a9mp_priv_init(SysBusDevice *dev)
> if (s->num_cpu > NCPU) {
> hw_error("a9mp_priv_init: num-cpu may not be more than %d\n", NCPU);
> }
> + if (s->num_irq > GIC_NIRQ) {
> + hw_error("a9mp_priv_init: num-irq may not be more than %d\n",
> GIC_NIRQ);
> + }
No need to check this here -- just pass it through
to gic_init() and let gic_init() hw_error if needed.
> --- 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).
> -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).
-- PMM
