Re: [Qemu-devel] [PATCH 2/3] hw/arm/gic: Kill code duplication

[Peter Maydell]

On 29 July 2015 at 12:54, Pavel Fedin <address@hidden> wrote:
> Extracted duplicated initialization code from SW-emulated and KVM GIC
> implementations and put into gic_init_irqs_and_mmio()
>
> Signed-off-by: Pavel Fedin <address@hidden>

> @@ -1110,28 +1090,19 @@ static void arm_gic_realize(DeviceState *dev, Error 
> **errp)
>          return;
>      }
>
> -    gic_init_irqs_and_distributor(s);
> +    gic_init_irqs_and_mmio(s, gic_set_irq, gic_ops);
>
> -    /* Memory regions for the CPU interfaces (NVIC doesn't have these):
> -     * a region for "CPU interface for this core", then a region for
> -     * "CPU interface for core 0", "for core 1", ...

Losing this bit of comment means we no longer have any documentation
of what the memory region interface between the GIC and its users is
(ie which sysbus memory regions are what).

> +    /* Extra core-specific regions for the CPU interfaces
>       * NB that the memory region size of 0x100 applies for the 11MPCore
>       * and also cores following the GIC v1 spec (ie A9).
>       * GIC v2 defines a larger memory region (0x1000) so this will need
>       * to be extended when we implement A15.
>       */
> -    memory_region_init_io(&s->cpuiomem[0], OBJECT(s), &gic_thiscpu_ops, s,
> -                          "gic_cpu", 0x100);

This memory region is size 0x100, as the comment says it must be...

> +    if (s->revision != REV_NVIC) {
> +        /* CPU interface (NVIC doesn't have this) */
> +        memory_region_init_io(&s->cpuiomem[0], OBJECT(s), ops ? &ops[1] : 
> NULL,
> +                              s, "gic_cpu", 0x1000);

...but here it is 0x1000.

The a9mpcore container component creates a layout where there are
other things immediately after the 0x100 region, so we can't
make it bigger for GICv1. (We will need to have it be bigger
for GICv2 when we eventually get around to implementing the
split-priority-drop feature.)

thanks
-- PMM