Re: [Qemu-devel] [PATCH v2] hw/intc/arm_gic.c: Implement GICv2 GICC_DIR

[Sergey Fedorov <serge.fdrv@gmail.com>]

On 01.03.2016 20:42, Peter Maydell wrote:
> The GICv2 introduces a new CPU interface register GICC_DIR, which
> allows an OS to split the "priority drop" and "deactivate interrupt"
> parts of interrupt completion. Implement this register.
> (Note that the register is at offset 0x1000 in the CPU interface,
> which means it is on a different 4K page from all the other registers.)
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> changes v1->v2: remove unnecessary gic_update() call
>
>   hw/cpu/a15mpcore.c       |  2 +-
>   hw/intc/arm_gic.c        | 45 ++++++++++++++++++++++++++++++++++++++++++++-
>   hw/intc/arm_gic_common.c |  2 +-
>   3 files changed, 46 insertions(+), 3 deletions(-)
>
> diff --git a/hw/cpu/a15mpcore.c b/hw/cpu/a15mpcore.c
> index e9063ad..a221b8f 100644
> --- a/hw/cpu/a15mpcore.c
> +++ b/hw/cpu/a15mpcore.c
> @@ -109,7 +109,7 @@ static void a15mp_priv_realize(DeviceState *dev, Error **errp)
>       /* Memory map (addresses are offsets from PERIPHBASE):
>        *  0x0000-0x0fff -- reserved
>        *  0x1000-0x1fff -- GIC Distributor
> -     *  0x2000-0x2fff -- GIC CPU interface
> +     *  0x2000-0x3fff -- GIC CPU interface
>        *  0x4000-0x4fff -- GIC virtual interface control (not modelled)
>        *  0x5000-0x5fff -- GIC virtual interface control (not modelled)
>        *  0x6000-0x7fff -- GIC virtual CPU interface (not modelled)
> diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
> index 60ab9b8..0834c2f 100644
> --- a/hw/intc/arm_gic.c
> +++ b/hw/intc/arm_gic.c
> @@ -500,6 +500,41 @@ static uint8_t gic_get_running_priority(GICState *s, int cpu, MemTxAttrs attrs)
>       }
>   }
>   
> +/* Return true if we should split priority drop and interrupt deactivation,
> + * ie whether the relevant EOIMode bit is set.
> + */
> +static bool gic_eoi_split(GICState *s, int cpu, MemTxAttrs attrs)
> +{
> +    if (s->revision != 2) {
> +        /* Before GICv2 prio-drop and deactivate are not separable */
> +        return false;
> +    }
> +    if (s->security_extn && !attrs.secure) {
> +        return s->cpu_ctlr[cpu] & GICC_CTLR_EOIMODE_NS;
> +    }
> +    return s->cpu_ctlr[cpu] & GICC_CTLR_EOIMODE;
> +}
> +
> +static void gic_deactivate_irq(GICState *s, int cpu, int irq, MemTxAttrs attrs)
> +{
> +    int cm = 1 << cpu;
> +    int group = gic_has_groups(s) && GIC_TEST_GROUP(irq, cm);

Looks like we don't have to check for gic_has_groups() here, because 
there is no way to set the interrupt group to 1, if gic_has_groups() is 
not true. Anyway,

Reviewed-by: Sergey Fedorov <serge.fdrv@gmail.com>

> +
> +    if (!gic_eoi_split(s, cpu, attrs)) {
> +        /* This is UNPREDICTABLE; we choose to ignore it */
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "gic_deactivate_irq: GICC_DIR write when EOIMode clear");
> +        return;
> +    }
> +
> +    if (s->security_extn && !attrs.secure && !group) {
> +        DPRINTF("Non-secure DI for Group0 interrupt %d ignored\n", irq);
> +        return;
> +    }
> +
> +    GIC_CLEAR_ACTIVE(irq, cm);
> +}
> +
>   void gic_complete_irq(GICState *s, int cpu, int irq, MemTxAttrs attrs)
>   {
>       int cm = 1 << cpu;
> @@ -544,7 +579,11 @@ void gic_complete_irq(GICState *s, int cpu, int irq, MemTxAttrs attrs)
>        */
>   
>       gic_drop_prio(s, cpu, group);
> -    GIC_CLEAR_ACTIVE(irq, cm);
> +
> +    /* In GICv2 the guest can choose to split priority-drop and deactivate */
> +    if (!gic_eoi_split(s, cpu, attrs)) {
> +        GIC_CLEAR_ACTIVE(irq, cm);
> +    }
>       gic_update(s);
>   }
>   
> @@ -1210,6 +1249,10 @@ static MemTxResult gic_cpu_write(GICState *s, int cpu, int offset,
>           s->nsapr[regno][cpu] = value;
>           break;
>       }
> +    case 0x1000:
> +        /* GICC_DIR */
> +        gic_deactivate_irq(s, cpu, value & 0x3ff, attrs);
> +        break;
>       default:
>           qemu_log_mask(LOG_GUEST_ERROR,
>                         "gic_cpu_write: Bad offset %x\n", (int)offset);
> diff --git a/hw/intc/arm_gic_common.c b/hw/intc/arm_gic_common.c
> index ac8cf42..707d00d 100644
> --- a/hw/intc/arm_gic_common.c
> +++ b/hw/intc/arm_gic_common.c
> @@ -121,7 +121,7 @@ void gic_init_irqs_and_mmio(GICState *s, qemu_irq_handler handler,
>            * neither it can use KVM.
>            */
>           memory_region_init_io(&s->cpuiomem[0], OBJECT(s), ops ? &ops[1] : NULL,
> -                              s, "gic_cpu", s->revision == 2 ? 0x1000 : 0x100);
> +                              s, "gic_cpu", s->revision == 2 ? 0x2000 : 0x100);
>           sysbus_init_mmio(sbd, &s->cpuiomem[0]);
>       }
>   }