qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH RFC V2 2/4] Implment GIC-500


From: Eric Auger
Subject: Re: [Qemu-devel] [PATCH RFC V2 2/4] Implment GIC-500
Date: Fri, 22 May 2015 15:01:49 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0

Hi Shlomo,
On 05/06/2015 04:04 PM, address@hidden wrote:
> From: Shlomo Pongratz <address@hidden>
> 
> Implement GIC-500 from GICv3 family for arm64
> 
> This patch is a first step toward 128 cores support for arm64.
> 
> At first only 64 cores are supported for two reasons:
> First the largest integer type has the size of 64 bits and modifying
> essential data structures in order to support 128 cores will require
> the usage of bitops.
> Second currently the Linux (kernel) can be configured to support
> up to 64 cores thus there is no urgency with 128 cores support.
> 
> Things left to do:
> 
> Currently the booting Linux may got stuck. The probability of getting stuck
> increases with the number of cores. I'll appreciate core review.

maybe you should precise in the commit message what the model currently
hard freezes wrt the full spec:
- ARE = 1, SRE= 1, DS= 1, no GICv2 backwards compat, no MBI, no LPI
support...
or other limitations

You could also split the patch files into one introducing the basic
functionalities and MMIO regions (dist, re-distrib) and in a separate
patch file, ITS, MBIs.

I would also advise to introduce the gic v3 common base device in a
separate patch file. I think it can ease the review.

Please find some more comments inline. Those are based on GIC arch spec
from Oct 2014, which may not be up-to-date or not the best doc. Since I
am also discovering the IP I may have put some comments what may closer
to notes ;-) Also I did not review ITS part and selection algo. Need
more time to digest ;-)

Hope this will be helpful.

Best Regards

Eric
> 
> Signed-off-by: Shlomo Pongratz <address@hidden>
> ---
>  hw/intc/Makefile.objs              |    2 +
>  hw/intc/arm_gicv3.c                | 1626 
> ++++++++++++++++++++++++++++++++++++
>  hw/intc/arm_gicv3_common.c         |  199 +++++
>  hw/intc/gicv3_internal.h           |  151 ++++
>  include/hw/intc/arm_gicv3.h        |   44 +
>  include/hw/intc/arm_gicv3_common.h |  110 +++
>  6 files changed, 2132 insertions(+)
>  create mode 100644 hw/intc/arm_gicv3.c
>  create mode 100644 hw/intc/arm_gicv3_common.c
>  create mode 100644 hw/intc/gicv3_internal.h
>  create mode 100644 include/hw/intc/arm_gicv3.h
>  create mode 100644 include/hw/intc/arm_gicv3_common.h
> 
> diff --git a/hw/intc/Makefile.objs b/hw/intc/Makefile.objs
> index 843864a..41fe9ec 100644
> --- a/hw/intc/Makefile.objs
> +++ b/hw/intc/Makefile.objs
> @@ -11,6 +11,8 @@ common-obj-$(CONFIG_SLAVIO) += slavio_intctl.o
>  common-obj-$(CONFIG_IOAPIC) += ioapic_common.o
>  common-obj-$(CONFIG_ARM_GIC) += arm_gic_common.o
>  common-obj-$(CONFIG_ARM_GIC) += arm_gic.o
> +common-obj-$(CONFIG_ARM_GIC) += arm_gicv3_common.o
> +common-obj-$(CONFIG_ARM_GIC) += arm_gicv3.o
>  common-obj-$(CONFIG_OPENPIC) += openpic.o
>  
>  obj-$(CONFIG_APIC) += apic.o apic_common.o
> diff --git a/hw/intc/arm_gicv3.c b/hw/intc/arm_gicv3.c
> new file mode 100644
> index 0000000..3b9dbda
> --- /dev/null
> +++ b/hw/intc/arm_gicv3.c
> @@ -0,0 +1,1626 @@
> +/*
> + * ARM Generic/Distributed Interrupt Controller
> + *
> + * Copyright (c) 2006-2007 CodeSourcery.
> + * Copyright (c) 2015 Huawei.
> + * Written by Shlomo Pongratz
> + * Base on gic.c by Paul Brook
based
> + *
> + * This code is licensed under the GPL.
> + */
> +
> +/* This file contains implementation code for the GIC-500 interrupt
> + * controller, which is an implementation of the GICv3 architecture.
> + * Curently it supports up to 64 cores. Enhancmet to 128 cores requires
enhancement + typo in the patch title
> + * working with bitops.
> + */
> +
> +#include "hw/sysbus.h"
> +#include "gicv3_internal.h"
> +#include "qom/cpu.h"
> +
> +#undef DEBUG_GICV3
> +
> +#ifdef DEBUG_GICV3
> +#define DPRINTF(fmt, ...) \
> +do { fprintf(stderr, "arm_gicv3::%s: " fmt , __func__, ## __VA_ARGS__); } 
> while (0)
> +#else
> +#define DPRINTF(fmt, ...) do {} while(0)
> +#endif
> +
> +void armv8_gicv3_set_sgi(void *opaque, int cpuindex, uint64_t value);
> +uint64_t armv8_gicv3_acknowledge_irq(void *opaque, int cpuindex);
> +void armv8_gicv3_complete_irq(void *opaque, int cpuindex, int irq);
> +uint64_t armv8_gicv3_get_priority_mask(void *opaque, int cpuindex);
> +void armv8_gicv3_set_priority_mask(void *opaque, int cpuindex, uint32_t 
> mask);
> +uint64_t armv8_gicv3_get_sre(void *opaque);
> +void armv8_gicv3_set_sre(void *opaque, uint64_t sre);
to gicv3_internal.h?
> +
> +static const uint8_t gic_dist_ids[] = {
> +    0x44, 0x00, 0x00, 0x00, 0x092, 0xB4, 0x3B, 0x00, 0x0D, 0xF0, 0x05, 0xB1
> +};
> +
> +static const uint8_t gic_lpi_ids[] = {
> +    0x44, 0x00, 0x00, 0x00, 0x093, 0xB4, 0x3B, 0x00, 0x0D, 0xF0, 0x05, 0xB1
> +};
> +
> +static uint32_t gic_sre;
> +
> +#define NUM_CPU(s) ((s)->num_cpu)
> +
> +static inline int gic_get_current_cpu(GICState *s)
> +{
> +    if (s->num_cpu > 1) {
> +        return current_cpu->cpu_index;
> +    }
> +    return 0;
> +}
> +
> +/* TODO: Many places that call this routine could be optimized.  */
> +/* Update interrupt status after enabled or pending bits have been changed.  
> */
> +void gicv3_update(GICState *s)
static and proto removed gicv3_internal.h?
> +{
> +    int best_irq;
> +    int best_prio;
> +    int irq;
> +    int level;
> +    int cpu;
> +    uint64_t cm;
> +
> +    for (cpu = 0; cpu < NUM_CPU(s); cpu++) {
> +        cm = 1ll << cpu;
> +        s->current_pending[cpu] = 1023;
> +        if (!s->enabled || !s->cpu_enabled[cpu]) {
> +            qemu_irq_lower(s->parent_irq[cpu]);
> +            /* In original GICv2 there is a return here. But if status is
> +             * disabled then all parent IRQs need to be lowered
> +             * And assume CPU i is disabled then with the original GICv2
> +             * implementation CPU - 1 will be considered but not CPU + 1
> +             */
> +            continue;
> +        }
> +        best_prio = 0x100;
> +        best_irq = 1023;
> +        for (irq = 0; irq < s->num_irq; irq++) {
> +            if (GIC_TEST_ENABLED(irq, cm) && gic_test_pending(s, irq, cm) &&
> +                (irq < GICV3_INTERNAL || (GIC_TARGET(irq) & cm))) {
> +                if (GIC_GET_PRIORITY(irq, cpu) < best_prio) {
> +                    best_prio = GIC_GET_PRIORITY(irq, cpu);
> +                    best_irq = irq;
> +                }
> +            }
> +        }
> +        level = 0;
> +        if (best_prio < s->priority_mask[cpu]) {
> +            s->current_pending[cpu] = best_irq;
> +            if (best_prio < s->running_priority[cpu]) {
> +                DPRINTF("Raised pending IRQ %d (cpu %d)\n", best_irq, cpu);
> +                level = 1;
> +            }
> +        }
> +        qemu_set_irq(s->parent_irq[cpu], level);
> +    }
> +}
> +
> +static void gicv3_set_irq_generic(GICState *s, int irq, int level,
> +                                  uint64_t cm, uint64_t target)
> +{
> +    if (level) {
> +        GIC_SET_LEVEL(irq, cm);
> +        DPRINTF("Set %d pending mask 0x%lx\n", irq, target);
> +        if (GIC_TEST_EDGE_TRIGGER(irq)) {
> +            GIC_SET_PENDING(irq, target);
> +        }
> +    } else {
> +        GIC_CLEAR_LEVEL(irq, cm);
> +    }
> +}
> +
> +/* Process a change in an external IRQ input.  */
> +static void gic_set_irq(void *opaque, int irq, int level)
> +{
> +    /* Meaning of the 'irq' parameter:
> +     *  [0..N-1] : external interrupts
> +     *  [N..N+31] : PPI (internal) interrupts for CPU 0
> +     *  [N+32..N+63] : PPI (internal interrupts for CPU 1
> +     *  ...
> +     */
> +    GICState *s = (GICState *)opaque;
> +    uint64_t cm, target;
> +
> +    if (irq < (s->num_irq - GICV3_INTERNAL)) {
> +        /* The first external input line is internal interrupt 32.  */
> +        cm = ALL_CPU_MASK;
> +        irq += GICV3_INTERNAL;
> +        target = GIC_TARGET(irq);
> +    } else {
> +        int cpu;
> +        irq -= (s->num_irq - GICV3_INTERNAL);
> +        cpu = irq / GICV3_INTERNAL;
> +        irq %= GICV3_INTERNAL;
> +        cm = 1ll << cpu;
> +        target = cm;
> +    }
> +
> +    assert(irq >= GICV3_NR_SGIS);
> +
> +    if (level == GIC_TEST_LEVEL(irq, cm)) {
> +        return;
> +    }
> +
> +    gicv3_set_irq_generic(s, irq, level, cm, target);
> +
> +    gicv3_update(s);
> +}
> +
> +static void gic_set_running_irq(GICState *s, int cpu, int irq)
> +{
> +    s->running_irq[cpu] = irq;
> +    if (irq == 1023) {
> +        s->running_priority[cpu] = 0x100;
> +    } else {
> +        s->running_priority[cpu] = GIC_GET_PRIORITY(irq, cpu);
> +    }
> +    gicv3_update(s);
> +}
> +
> +uint32_t gicv3_acknowledge_irq(GICState *s, int cpu)
static?
> +{
> +    int ret, irq, src;
> +    uint64_t cm = 1ll << cpu;
add an empty line here?
> +    irq = s->current_pending[cpu];
> +    if (irq == 1023
> +            || GIC_GET_PRIORITY(irq, cpu) >= s->running_priority[cpu]) {
> +        DPRINTF("ACK no pending IRQ\n");
> +        return 1023;
> +    }
> +    s->last_active[irq][cpu] = s->running_irq[cpu];
> +
> +    if (irq < GICV3_NR_SGIS) {
> +        /* Lookup the source CPU for the SGI and clear this in the
> +         * sgi_pending map.  Return the src and clear the overall pending
> +         * state on this CPU if the SGI is not pending from any CPUs.
> +         */
> +        assert(s->sgi_state[irq].pending[cpu] != 0);
> +        src = ctz64(s->sgi_state[irq].pending[cpu]);
> +        s->sgi_state[irq].pending[cpu] &= ~(1ll << src);
> +        if (s->sgi_state[irq].pending[cpu] == 0) {
> +            GIC_CLEAR_PENDING(irq, cm);
> +        }
> +        /* GICv3 kernel driver dosen't mask src bits like GICv2 driver
> +         * so don't add src i.e. ret = irq | ((src & 0x7) << 10);
> +         * Section 4.2.10 in GICv3 specification
> +         */
> +        ret = irq;
> +    } else {
> +        //DPRINTF("ACK irq(%d) cpu(%d) \n", irq, cpu);
> +        /* Clear pending state for both level and edge triggered
> +         * interrupts. (level triggered interrupts with an active line
> +         * remain pending, see gic_test_pending)
> +         */
> +        GIC_CLEAR_PENDING(irq, cm);
> +        ret = irq;
> +    }
> +
> +    gic_set_running_irq(s, cpu, irq);
> +    DPRINTF("out ACK irq-ret(%d) cpu(%d) \n", ret, cpu);
> +    return ret;
> +}
> +
> +void gicv3_set_priority(GICState *s, int cpu, int irq, uint8_t val)
static?
> +{
> +    if (irq < GICV3_INTERNAL) {
> +        s->priority1[irq][cpu] = val;
> +    } else {
> +        s->priority2[irq - GICV3_INTERNAL] = val;
> +    }
> +}
> +
> +void gicv3_complete_irq(GICState *s, int cpu, int irq)
static?
> +{
> +    DPRINTF("EOI irq(%d) cpu (%d)\n", irq, cpu);
> +    if (irq >= s->num_irq) {
> +        /* This handles two cases:
> +         * 1. If software writes the ID of a spurious interrupt [ie 1023]
> +         * to the GICC_EOIR, the GIC ignores that write.
> +         * 2. If software writes the number of a non-existent interrupt
> +         * this must be a subcase of "value written does not match the last
> +         * valid interrupt value read from the Interrupt Acknowledge
> +         * register" and so this is UNPREDICTABLE. We choose to ignore it.
> +         */
> +        return;
> +    }
> +
> +    if (s->running_irq[cpu] == 1023)
> +        return; /* No active IRQ.  */
> +
> +    if (irq != s->running_irq[cpu]) {
> +        /* Complete an IRQ that is not currently running.  */
> +        int tmp = s->running_irq[cpu];
> +        while (s->last_active[tmp][cpu] != 1023) {
> +            if (s->last_active[tmp][cpu] == irq) {
> +                s->last_active[tmp][cpu] = s->last_active[irq][cpu];
> +                break;
> +            }
> +            tmp = s->last_active[tmp][cpu];
> +        }
> +    } else {
> +        /* Complete the current running IRQ.  */
> +        gic_set_running_irq(s, cpu, 
> s->last_active[s->running_irq[cpu]][cpu]);
> +    }
> +}
> +
> +static uint64_t gic_dist_readb(void *opaque, hwaddr offset)
> +{
> +    GICState *s = (GICState *)opaque;
> +    uint64_t res;
> +    int irq;
> +    int i;
> +    int cpu;
> +    uint64_t cm;
> +    uint64_t mask;
> +
> +    cpu = gic_get_current_cpu(s);
> +    cm = 1ll << cpu;
> +    if (offset < 0x100) {
> +        if (offset == 0) {/* GICD_CTLR */
> +            DPRINTF("GICD_CTLR(%d) enable(%d)\n", cpu, s->gicd_ctlr);
s/enable/GICD_CTRL or apply a mask
> +            return s->gicd_ctlr;
> +        }
> +        if (offset == 4) { /* GICD_TYPER */
> +            uint64_t num = NUM_CPU(s);
> +            /* the number of cores in the system, saturated to 8 minus one. 
> */
> +            if (num > 8)
> +                num = 8;
> +            /* The nun_irqs as given from virt machine via "num-irq"
s/nun_irqs/num_irqs
> +             * includes the internal irqs, so subtract them
> +             */
> +            res = (s->num_irq - GICV3_INTERNAL) / 32;
> +            res |= (num - 1) << 5;
> +            res |= 0xF << 19;

> +            return res;
> +        }
> +        if (offset < 0x08)
can we enter that condition?
> +            return 0;
> +        if (offset == 0x08)
> +            return 0x43B; /* GIC_IIDR */
> +        if (offset >= 0x80) {
> +            /* Interrupt Security , RAZ/WI */
> +            return 0;
> +        }
> +        goto bad_reg;
> +    } else if (offset < 0x200) {
> +        /* Interrupt Set/Clear Enable.  */
> +        if (offset < 0x180)
> +            irq = (offset - 0x100) * 8;
> +        else
> +            irq = (offset - 0x180) * 8;
> +        irq += GICV3_BASE_IRQ;
> +        if (irq >= s->num_irq)
> +            goto bad_reg;
> +        res = 0;
> +        for (i = 0; i < 8; i++) {
> +            if (GIC_TEST_ENABLED(irq + i, cm)) {
> +                res |= (1 << i);
> +            }
> +        }
> +    } else if (offset < 0x300) {
> +        /* Interrupt Set/Clear Pending.  */
> +        if (offset < 0x280)
> +            irq = (offset - 0x200) * 8;
> +        else
> +            irq = (offset - 0x280) * 8;
> +        irq += GICV3_BASE_IRQ;
> +        if (irq >= s->num_irq)
> +            goto bad_reg;
> +        res = 0;
> +        mask = (irq < GICV3_INTERNAL) ?  cm : ALL_CPU_MASK;
> +        for (i = 0; i < 8; i++) {
> +            if (gic_test_pending(s, irq + i, mask)) {
> +                res |= (1 << i);
for SGI and PPI shouldn't we return res0 and not the actual status -
that is supposed to be returned by redistrib -.
same for other similar regs ...
> +            }
> +        }
> +    } else if (offset < 0x400) {
> +        /* Interrupt Active.  */
> +        irq = (offset - 0x300) * 8 + GICV3_BASE_IRQ;
> +        if (irq >= s->num_irq)
> +            goto bad_reg;
> +        res = 0;
> +        mask = (irq < GICV3_INTERNAL) ?  cm : ALL_CPU_MASK;
> +        for (i = 0; i < 8; i++) {
> +            if (GIC_TEST_ACTIVE(irq + i, mask)) {
> +                res |= (1 << i);
> +            }
> +        }
> +    } else if (offset < 0x800) {
> +        /* Interrupt Priority.  */
> +        irq = (offset - 0x400) + GICV3_BASE_IRQ;
> +        if (irq >= s->num_irq)
> +            goto bad_reg;
> +        res = GIC_GET_PRIORITY(irq, cpu);
> +    } else if (offset < 0xc00) {
is is mandated to model ITARGETSRn  since if ARE==1 the routing info is
provided by IROUTERn and ITARGETSRn = Res0
> +        /* Interrupt CPU Target.  */
> +        if (s->num_cpu == 1) {
> +            /* For uniprocessor GICs these RAZ/WI */
> +            res = 0;
> +        } else {
> +            irq = (offset - 0x800) + GICV3_BASE_IRQ;
> +            if (irq >= s->num_irq) {
> +                goto bad_reg;
> +            }
> +            if (irq >= 29 && irq <= 31) {
> +                res = cm;
> +            } else {
> +                res = GIC_TARGET(irq);
> +            }
> +        }
> +    } else if (offset < 0xf00) {
why not using your offset macros defined in internal?
> +        /* Interrupt Configuration.  */
> +        irq = (offset - 0xc00) * 4 + GICV3_BASE_IRQ;
> +        if (irq >= s->num_irq)
> +            goto bad_reg;
> +        res = 0;
> +        for (i = 0; i < 4; i++) {
> +            /* Even bits are reserved */
> +            if (GIC_TEST_EDGE_TRIGGER(irq + i))
> +                res |= (2 << (i * 2));
> +        }
> +    } else if (offset < 0xf10) { 
/* GICD_SGIR */
> +        goto bad_reg;
> +    } else if (offset < 0xf30) {
> +        /* These are 32 bit registers, should not be used with 128 cores. */
> +        if (offset < 0xf20) {
it is said in archi spec "replaced by GICR_ICPENDR0" when ARE is set
bad_reg?
> +            /* GICD_CPENDSGIRn */
> +            irq = (offset - 0xf10);
> +        } else {
> +            irq = (offset - 0xf20);
> +            /* GICD_SPENDSGIRn */
> +        }
> +
> +        res = s->sgi_state[irq].pending[cpu];
> +    } else if (offset < 0xffd0) {
what about IROUTERn (RW) @0x6100 ?
> +        goto bad_reg;
> +    } else /* offset >= 0xffd0 */ {
> +        if (offset & 3) {
> +            res = 0;
> +        } else {
> +            res = gic_dist_ids[(offset - 0xffd0) >> 2];
> +        }
> +    }
> +    return res;
> +bad_reg:
> +    qemu_log_mask(LOG_GUEST_ERROR,
> +                  "%s: Bad offset %x\n", __func__, (int)offset);
> +    return 0;
> +}
> +
> +static uint64_t gic_dist_readw(void *opaque, hwaddr offset)
> +{
> +    uint64_t val;
> +    val = gic_dist_readb(opaque, offset);
> +    val |= gic_dist_readb(opaque, offset + 1) << 8;
> +    return val;
> +}
> +
> +static uint64_t gic_dist_readl(void *opaque, hwaddr offset)
> +{
> +    uint64_t val;
> +    val = gic_dist_readw(opaque, offset);
> +    val |= gic_dist_readw(opaque, offset + 2) << 16;
> +    return val;
> +}
> +
> +static uint64_t gic_dist_readll(void *opaque, hwaddr offset)
> +{
> +    uint64_t val;
> +    val = gic_dist_readl(opaque, offset);
> +    val |= gic_dist_readl(opaque, offset + 4) << 32;
> +    return val;
> +}
> +
> +static void gic_dist_writeb(void *opaque, hwaddr offset,
> +                            uint64_t value)
> +{
> +    GICState *s = (GICState *)opaque;
> +    int irq;
> +    int i;
> +    int cpu;
> +
> +    cpu = gic_get_current_cpu(s);
> +
> +    if (offset < 0x100) {
> +        if (offset == 0) {
> +            s->gicd_ctlr = value;
Shouldn't we prevent any attempt to change fixed model values like:
- DS bit
- ARE bits
- Group enables
all the more so gicd_ctrl has a degult value set at realize time.
> +            s->enabled = value & GICD_CTLR_ENABLE_GRP0;
> +            DPRINTF("Distribution %sabled\n", s->enabled ? "En" : "Dis");
format string typo
> +        } else if (offset < 4) {
> +            /* ignored.  */
don't get this
> +        } else if (offset >= 0x80) {
> +            /* Interrupt Security Registers, RAZ/WI */
> +        } else {
> +            goto bad_reg;
> +        }
> +    } else if (offset < 0x180) {
> +        /* Interrupt Set Enable.  */
> +        irq = (offset - 0x100) * 8 + GICV3_BASE_IRQ;
> +        if (irq >= s->num_irq)
> +            goto bad_reg;
> +        if (irq < GICV3_NR_SGIS) {
Isn't it SGI and PPI (INTERNAL)
> +            DPRINTF("ISENABLERn SGI should be only in redistributer %d\n", 
> irq);
> +            /* Ignored according to comment 'g' in GIC-500 document.*/
> +            return;
> +        }
> +
> +        for (i = 0; i < 8; i++) {
> +            if (value & (1 << i)) {
> +                uint64_t mask =
> +                    (irq < GICV3_INTERNAL) ? (1ll << cpu) : GIC_TARGET(irq + 
> i);
> +                uint64_t cm = (irq < GICV3_INTERNAL) ? (1ll << cpu) : 
> ALL_CPU_MASK;
> +
> +                if (!GIC_TEST_ENABLED(irq + i, cm)) {
> +                    DPRINTF("Enabled IRQ %d\n", irq + i);
> +                }
> +                GIC_SET_ENABLED(irq + i, cm);
> +                /* If a raised level triggered IRQ enabled then mark
> +                   is as pending.  */
> +                if (GIC_TEST_LEVEL(irq + i, mask)
> +                        && !GIC_TEST_EDGE_TRIGGER(irq + i)) {
> +                    DPRINTF("Set %d pending mask %lx\n", irq + i, mask);
> +                    GIC_SET_PENDING(irq + i, mask);
> +                }
> +            }
> +        }
> +    } else if (offset < 0x200) {
> +        /* Interrupt Clear Enable.  */
> +        irq = (offset - 0x180) * 8 + GICV3_BASE_IRQ;
> +        if (irq >= s->num_irq)
> +            goto bad_reg;
> +        if (irq < GICV3_NR_SGIS) {
same
> +            DPRINTF("ICENABLERn SGI should be only in redistributer %d\n", 
> irq);
> +            /* Ignored according to comment 'g' in GIC-500 document.*/
> +            return;
> +        }
> +
> +        for (i = 0; i < 8; i++) {
> +            if (value & (1 << i)) {
> +                uint64_t cm = (irq < GICV3_INTERNAL) ? (1ll << cpu) : 
> ALL_CPU_MASK;
> +
> +                if (GIC_TEST_ENABLED(irq + i, cm)) {
> +                    DPRINTF("Disabled IRQ %d\n", irq + i);
> +                }
> +                GIC_CLEAR_ENABLED(irq + i, cm);
> +            }
> +        }
> +    } else if (offset < 0x280) {
> +        /* Interrupt Set Pending.  */
> +        irq = (offset - 0x200) * 8 + GICV3_BASE_IRQ;
> +        if (irq >= s->num_irq)
> +            goto bad_reg;
> +        if (irq < GICV3_NR_SGIS) {
same
> +            value = 0;
> +        }
> +
> +        for (i = 0; i < 8; i++) {
> +            if (value & (1 << i)) {
> +                GIC_SET_PENDING(irq + i, GIC_TARGET(irq + i));
> +            }
> +        }
> +    } else if (offset < 0x300) {
> +        /* Interrupt Clear Pending.  */
> +        irq = (offset - 0x280) * 8 + GICV3_BASE_IRQ;
> +        if (irq >= s->num_irq)
> +            goto bad_reg;
> +        if (irq < GICV3_NR_SGIS) {
same
> +            value = 0;
> +        }
> +
> +        for (i = 0; i < 8; i++) {
> +            /* ??? This currently clears the pending bit for all CPUs, even
> +               for per-CPU interrupts. It's unclear whether this is the
> +               correct behavior.  */
> +            if (value & (1 << i)) {
> +                GIC_CLEAR_PENDING(irq + i, ALL_CPU_MASK);
> +            }
> +        }
> +    } else if (offset < 0x400) {
> +        /* Interrupt Active.  */
ISACTIVER & ICACTIVER notsupported as in GICv2 model
SGI/PPI case handling
> +        goto bad_reg;
> +    } else if (offset < 0x800) {
> +        /* Interrupt Priority.  */
> +        irq = (offset - 0x400) + GICV3_BASE_IRQ;
> +        if (irq >= s->num_irq)
> +            goto bad_reg;
> +        gicv3_set_priority(s, cpu, irq, value);
> +    } else if (offset < 0xc00) {
> +        /* Interrupt CPU Target. RAZ/WI on uni-processor GICs, with the
> +         * annoying exception of the 11MPCore's GIC.
comment about 11MPcore may be removed.
more generally isn't it replaced by IROUTERn if ARE==1?
> +         */
> +        if (s->num_cpu != 1) {
> +            irq = (offset - 0x800) + GICV3_BASE_IRQ;
> +            if (irq >= s->num_irq) {
> +                goto bad_reg;
> +            }
> +            if (irq < 29) {
> +                value = 0;
> +            } else if (irq < GICV3_INTERNAL) {
> +                value = ALL_CPU_MASK;
> +            }
> +            s->irq_target[irq] = value & ALL_CPU_MASK;
> +        }
> +    } else if (offset < 0xf00) {
isn't it 0xD00
> +        /* Interrupt Configuration.  */
> +        irq = (offset - 0xc00) * 4 + GICV3_BASE_IRQ;
> +        if (irq >= s->num_irq)
> +            goto bad_reg;
> +        if (irq < GICV3_NR_SGIS)
> +            value |= 0xaa; /* 10101010 */
> +        /* Even bits are reserved */
> +        for (i = 0; i < 4; i++) {
> +            if (value & (2 << (i * 2))) {
> +                GIC_SET_EDGE_TRIGGER(irq + i);
> +            } else {
> +                GIC_CLEAR_EDGE_TRIGGER(irq + i);
> +            }
what about IGRPMODRn and NSACRn?
> +        }
> +    } else if (offset < 0xf10) {
/* GICD_SGIR */
> +        /* 0xf00 is only handled for 32-bit writes.  */
> +        goto bad_reg;
> +    } else if (offset < 0xf20) {
replaced by GICR_ICPENDR0 when ARE = 1
> +        /* GICD_CPENDSGIRn */
> +        /* This is a 32 bits register shouldn't be used with 128 cores */
> +        irq = (offset - 0xf10);
> +        DPRINTF("GICD_CPENDSGIRn irq(%d) %lu\n", irq, value);
> +
> +        s->sgi_state[irq].pending[cpu] &= ~value;
> +        if (s->sgi_state[irq].pending[cpu] == 0) {
> +            GIC_CLEAR_PENDING(irq, 1ll << cpu);
> +        }
> +    } else if (offset < 0xf30) {
moved to redistributor if ARE = 1
> +        /* GICD_SPENDSGIRn */
> +        irq = (offset - 0xf20);
> +        DPRINTF("GICD_SPENDSGIRn irq(%d) %lu\n", irq, value);
> +
> +        GIC_SET_PENDING(irq, 1ll << cpu);
> +        s->sgi_state[irq].pending[cpu] |= value;
what about IROUTERn @6100?
> +    } else {
> +        goto bad_reg;
> +    }
> +    gicv3_update(s);
> +    return;
> +bad_reg:
> +    qemu_log_mask(LOG_GUEST_ERROR,
> +                  "%s: Bad offset %x\n", __func__, (int)offset);
> +}
> +
> +static void gic_dist_writew(void *opaque, hwaddr offset,
> +                            uint64_t value)
> +{
> +    gic_dist_writeb(opaque, offset, value & 0xff);
> +    gic_dist_writeb(opaque, offset + 1, value >> 8);
> +}
> +
> +static void gic_dist_writel(void *opaque, hwaddr offset,
> +                            uint64_t value)
> +{
> +    if (offset == 0xf00) {
> +        /* GICD_SGIR Software generated Interrupt register
> +         * This register should not be used if GICv2 backwards computability
> +         * support is not included. (comment t page 3-8 on GIC-500 doc)
> +         */
> +        int cpu;
> +        int irq;
> +        uint64_t mask, cm;
> +        int target_cpu;
> +        GICState *s = (GICState *)opaque;
> +
> +        DPRINTF("GICv2 backwards computability is not supported\n");
> +        cpu = gic_get_current_cpu(s);
> +        irq = value & 0x3ff;
> +        switch ((value >> 24) & 3) {
> +        case 0:
> +            mask = (value >> 16) & ALL_CPU_MASK;
> +            break;
> +        case 1:
> +            mask = ALL_CPU_MASK ^ (1ll << cpu);
> +            break;
> +        case 2:
> +            mask = 1ll << cpu;
> +            break;
> +        default:
> +            DPRINTF("Bad Soft Int target filter\n");
> +            mask = ALL_CPU_MASK;
> +            break;
> +        }
> +        cm = (1ll << cpu);
> +        DPRINTF("irq(%d) mask(%lu)\n", irq, mask);
> +        GIC_SET_PENDING(irq, mask);
> +        target_cpu = ctz64(mask);
> +        while (target_cpu < GICV3_NCPU) {
> +            s->sgi_state[irq].pending[target_cpu] |= cm;
> +            mask &= ~(1ll << target_cpu);
> +            target_cpu = ctz64(mask);
> +        }
> +        gicv3_update(s);
> +        return;
> +    }
> +    gic_dist_writew(opaque, offset, value & 0xffff);
> +    gic_dist_writew(opaque, offset + 2, value >> 16);
> +}
> +
> +static void gic_dist_writell(void *opaque, hwaddr offset,
> +                            uint64_t value)
> +{
> +    GICState *s = (GICState *)opaque;
> +    //DPRINTF("offset %p data %p\n", (void *) offset, (void *) value);
run checkpatch.pl when it becomes PATCH
> +
> +    if (offset >= 0x6100 && offset <= 0x7EF8) {
> +        int irq = (offset - 0x6100) / 8;
> +        /* GCID_IROUTERn [affinity-3:X:affinity-2:affinity-1:affininty-0]
> +         * See kernel code for fields
> +         * GIC 500 currently supports 32 clusters with 8 cores each,
> +         * but virtv2 fills the Aff0 before filling Aff1 so
> +         * 16 = 2 * 8 but not 4 x 4 nor 8 x 2 not 16 x 1
> +         * Note Linux kernel doesn't set bit 31 thus send to all is not 
> needed
> +         */
> +        uint32_t cpu, Aff1, Aff0;
> +        Aff1 = (value & 0xf00) >> (8 - 3); /* Shift by 8 multiply by 8 */
> +        Aff0 = value & 0x7;
> +        cpu = Aff1 + Aff0;
> +        s->irq_target[irq] = 1ll << cpu;
> +        gicv3_update(s);
> +        DPRINTF("irq(%d) cpu(%d)\n", irq, cpu);
> +        return;
> +    }
> +
> +    gic_dist_writel(opaque, offset, value & 0xffffffff);
> +    gic_dist_writel(opaque, offset + 4, value >> 32);
> +}
> +
> +static uint64_t gic_dist_read(void *opaque, hwaddr addr, unsigned size)
> +{
> +    uint64_t data;
line
> +    switch (size) {
> +    case 1:
> +        data = gic_dist_readb(opaque, addr);
> +        break;
> +    case 2:
> +        data = gic_dist_readw(opaque, addr);
> +        break;
> +    case 4:
> +        data = gic_dist_readl(opaque, addr);
> +        break;
> +    case 8:
> +        data = gic_dist_readll(opaque, addr);
> +        break;
> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: size %u\n", __func__, size);
> +        assert(0);
> +        break;
> +    }
> +    //DPRINTF("offset %p data %p\n", (void *) addr, (void *) data);
> +    return data;
> +}
> +
> +static void gic_dist_write(void *opaque, hwaddr addr, uint64_t data, 
> unsigned size)
> +{
> +    //DPRINTF("offset %p data %p\n", (void *) addr, (void *) data);
> +    switch (size) {
> +    case 1:
> +        gic_dist_writeb(opaque, addr, data);
> +        break;
> +    case 2:
> +        gic_dist_writew(opaque, addr, data);
> +        break;
> +    case 4:
> +        gic_dist_writel(opaque, addr, data);
> +        break;
> +    case 8:
> +        gic_dist_writell(opaque, addr, data);
> +        break;
> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: size %u\n", __func__, size);
> +        assert(0);
> +        break;
> +    }
> +}
> +
> +static const MemoryRegionOps gic_dist_ops = {
> +    .read = gic_dist_read,
> +    .write = gic_dist_write,
> +    .impl = {
> +         .min_access_size = 4,
> +         .max_access_size = 8,
> +     },
> +    .endianness = DEVICE_NATIVE_ENDIAN,
> +};
> +
> +static uint64_t gic_its_readb(void *opaque, hwaddr offset)
> +{
> +    return 0;
> +}
> +
> +static uint64_t gic_its_readw(void *opaque, hwaddr offset)
> +{
> +    uint64_t val;
> +    val = gic_its_readb(opaque, offset);
> +    val |= gic_its_readb(opaque, offset + 1) << 8;
> +    return val;
> +}
> +
> +static uint64_t gic_its_readl(void *opaque, hwaddr offset)
> +{
> +    uint64_t val;
> +    val = gic_its_readw(opaque, offset);
> +    val |= gic_its_readw(opaque, offset + 2) << 16;
> +    return val;
> +}
> +
> +static uint64_t gic_its_readll(void *opaque, hwaddr offset)
> +{
> +    uint64_t val;
> +    val = gic_its_readl(opaque, offset);
> +    val |= gic_its_readl(opaque, offset + 4) << 32;
> +    return val;
> +}
> +
> +static void gic_its_writeb(void *opaque, hwaddr offset,
> +                            uint64_t value)
> +{
> +    GICState *s = (GICState *)opaque;
> +    gicv3_update(s);
> +    return;
> +}
> +
> +static void gic_its_writew(void *opaque, hwaddr offset,
> +                            uint64_t value)
> +{
> +    gic_its_writeb(opaque, offset, value & 0xff);
> +    gic_its_writeb(opaque, offset + 1, value >> 8);
> +}
> +
> +static void gic_its_writel(void *opaque, hwaddr offset,
> +                            uint64_t value)
> +{
> +    gic_its_writel(opaque, offset, value & 0xffff);
> +    gic_its_writel(opaque, offset + 2, value >> 16);
> +}
> +
> +static void gic_its_writell(void *opaque, hwaddr offset,
> +                            uint64_t value)
> +{
> +    gic_its_writell(opaque, offset, value & 0xffffffff);
> +    gic_its_writell(opaque, offset + 4, value >> 32);
> +}
> +
> +static uint64_t gic_its_read(void *opaque, hwaddr addr, unsigned size)
> +{
> +    uint64_t data;
> +    switch (size) {
> +    case 1:
> +        data = gic_its_readb(opaque, addr);
> +        break;
> +    case 2:
> +        data = gic_its_readw(opaque, addr);
> +        break;
> +    case 4:
> +        data = gic_its_readl(opaque, addr);
> +        break;
> +    case 8:
> +        data = gic_its_readll(opaque, addr);
> +        break;
> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: size %u\n", __func__, size);
> +        assert(0);
> +        break;
> +    }
> +    DPRINTF("offset %p data %p\n", (void *) addr, (void *) data);
> +    return data;
> +}
> +
> +static void gic_its_write(void *opaque, hwaddr addr, uint64_t data, unsigned 
> size)
> +{
> +    DPRINTF("offset %p data %p\n", (void *) addr, (void *) data);
> +    switch (size) {
> +    case 1:
> +        gic_its_writew(opaque, addr, data);
> +        break;
> +    case 2:
> +        gic_its_writew(opaque, addr, data);
> +        break;
> +    case 4:
> +        gic_its_writel(opaque, addr, data);
> +        break;
> +    case 8:
> +        gic_its_writell(opaque, addr, data);
> +        break;
> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: size %u\n", __func__, size);
> +        assert(0);
> +        break;
> +    }
> +}
> +
> +static const MemoryRegionOps gic_its_ops = {
> +    .read = gic_its_read,
> +    .write = gic_its_write,
> +    .impl = {
> +         .min_access_size = 4,
> +         .max_access_size = 8,
> +     },
> +    .endianness = DEVICE_NATIVE_ENDIAN,
> +};
> +
I guess the following ops are related to Distributor Message Based
Interrupt register map. To me this was not straightforward and this
would deserve a comment here or when defining the region. This being
optional, to me this could be the topic of a separate patch file?
> +static uint64_t gic_spi_readb(void *opaque, hwaddr offset)
> +{
> +    return 0;
Isn't WO or reserved? Maybe I am wrong about the interpretation of the
region itself?
> +}
> +
> +static uint64_t gic_spi_readw(void *opaque, hwaddr offset)
> +{
> +    uint64_t val;
> +    val = gic_spi_readb(opaque, offset);
> +    val |= gic_spi_readb(opaque, offset + 1) << 8;
> +    return val;
> +}
> +
> +static uint64_t gic_spi_readl(void *opaque, hwaddr offset)
> +{
> +    uint64_t val;
> +    val = gic_spi_readw(opaque, offset);
> +    val |= gic_spi_readw(opaque, offset + 2) << 16;
> +    return val;
> +}
> +
> +static uint64_t gic_spi_readll(void *opaque, hwaddr offset)
> +{
> +    uint64_t val;
> +    val = gic_spi_readl(opaque, offset);
> +    val |= gic_spi_readl(opaque, offset + 4) << 32;
> +    return val;
> +}
> +
> +static void gic_spi_writeb(void *opaque, hwaddr offset,
> +                            uint64_t value)
> +{
> +    GICState *s = (GICState *)opaque;
Shouldn't we set/clear the SPI pending before the update?
> +    gicv3_update(s);
> +    return;
> +}
> +
> +static void gic_spi_writew(void *opaque, hwaddr offset,
> +                            uint64_t value)
> +{
> +    gic_spi_writeb(opaque, offset, value & 0xff);
> +    gic_spi_writeb(opaque, offset + 1, value >> 8);
> +}
> +
> +static void gic_spi_writel(void *opaque, hwaddr offset,
> +                            uint64_t value)
> +{
> +    gic_spi_writew(opaque, offset, value & 0xffff);
> +    gic_spi_writew(opaque, offset + 2, value >> 16);
> +}
> +
> +static void gic_spi_writell(void *opaque, hwaddr offset,
> +                            uint64_t value)
> +{
> +    gic_spi_writel(opaque, offset, value & 0xffffffff);
> +    gic_spi_writel(opaque, offset + 4, value >> 32);
> +}
> +
> +static uint64_t gic_spi_read(void *opaque, hwaddr addr, unsigned size)
> +{
> +    uint64_t data;
> +    switch (size) {
> +    case 1:
> +        data = gic_spi_readb(opaque, addr);
> +        break;
> +    case 2:
> +        data = gic_spi_readw(opaque, addr);
> +        break;
> +    case 4:
> +        data = gic_spi_readl(opaque, addr);
> +        break;
> +    case 8:
> +        data = gic_spi_readll(opaque, addr);
> +        break;
> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: size %u\n", __func__, size);
> +        assert(0);
> +        break;
> +    }
> +    DPRINTF("offset %p data %p\n", (void *) addr, (void *) data);
> +    return data;
> +}
> +
> +static void gic_spi_write(void *opaque, hwaddr addr, uint64_t data, unsigned 
> size)
> +{
> +    DPRINTF("offset %p data %p\n", (void *) addr, (void *) data);
> +    switch (size) {
> +    case 1:
> +        gic_spi_writeb(opaque, addr, data);
> +        break;
> +    case 2:
> +        gic_spi_writew(opaque, addr, data);
> +        break;
> +    case 4:
> +        gic_spi_writel(opaque, addr, data);
> +        break;
> +    case 8:
> +        gic_spi_writell(opaque, addr, data);
> +        break;
> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: size %u\n", __func__, size);
> +        assert(0);
> +        break;
> +    }
> +}
> +
> +static const MemoryRegionOps gic_spi_ops = {
> +    .read = gic_spi_read,
> +    .write = gic_spi_write,
> +    .impl = {
> +         .min_access_size = 4,
> +         .max_access_size = 8,
> +     },
> +    .endianness = DEVICE_NATIVE_ENDIAN,
> +};
> +
> +
> +static uint64_t gic_its_cntrl_readb(void *opaque, hwaddr offset)
> +{
> +    GICState *s = (GICState *)opaque;
> +    uint64_t res=0;
> +
> +    if (offset < 0x100) {
> +          if (offset == 0)
> +            return 0;
> +          if (offset == 4)
> +              return 0;
> +          if (offset < 0x08)
> +            return s->num_cpu;
> +          if (offset >= 0x80) {
> +            return 0;
> +          }
> +          goto bad_reg;
> +      }
> +    return res;
> +bad_reg:
> +    qemu_log_mask(LOG_GUEST_ERROR,
> +                  "%s: Bad offset %x\n", __func__, (int)offset);
> +    return 0;
> +}
> +
> +static uint64_t gic_its_cntrl_readw(void *opaque, hwaddr offset)
> +{
> +    uint64_t val;
> +    val = gic_its_cntrl_readb(opaque, offset);
> +    val |= gic_its_cntrl_readb(opaque, offset + 1) << 8;
> +    return val;
> +}
> +
> +static uint64_t gic_its_cntrl_readl(void *opaque, hwaddr offset)
> +{
> +    uint64_t val;
> +    val = gic_its_cntrl_readw(opaque, offset);
> +    val |= gic_its_cntrl_readw(opaque, offset + 2) << 16;
> +    return val;
> +}
> +
> +static uint64_t gic_its_cntrl_readll(void *opaque, hwaddr offset)
> +{
> +    uint64_t val;
> +    val = gic_its_cntrl_readl(opaque, offset);
> +    val |= gic_its_cntrl_readl(opaque, offset + 4) << 32;
> +    return val;
> +}
> +
> +static void gic_its_cntrl_writeb(void *opaque, hwaddr offset,
> +                            uint64_t value)
> +{
> +    GICState *s = (GICState *)opaque;
> +    if (offset < 0x100) {
> +        if (offset < 0x08)
> +            s->num_cpu = value;
> +        else
> +            goto bad_reg;
> +    }
> +    gicv3_update(s);
> +    return;
> +bad_reg:
> +    qemu_log_mask(LOG_GUEST_ERROR,
> +                  "%s: Bad offset %x\n", __func__, (int)offset);
> +}
> +
> +static void gic_its_cntrl_writew(void *opaque, hwaddr offset,
> +                            uint64_t value)
> +{
> +    gic_its_cntrl_writeb(opaque, offset, value & 0xff);
> +    gic_its_cntrl_writeb(opaque, offset + 1, value >> 8);
> +}
> +
> +static void gic_its_cntrl_writel(void *opaque, hwaddr offset,
> +                            uint64_t value)
> +{
> +    gic_its_cntrl_writew(opaque, offset, value & 0xffff);
> +    gic_its_cntrl_writew(opaque, offset + 2, value >> 16);
> +}
> +
> +static void gic_its_cntrl_writell(void *opaque, hwaddr offset,
> +                            uint64_t value)
> +{
> +    gic_its_cntrl_writel(opaque, offset, value & 0xffffffff);
> +    gic_its_cntrl_writel(opaque, offset + 4, value >> 32);
> +}
> +
> +static uint64_t gic_its_cntrl_read(void *opaque, hwaddr addr, unsigned size)
> +{
> +    uint64_t data;
> +    switch (size) {
> +    case 1:
> +        data = gic_its_cntrl_readb(opaque, addr);
> +        break;
> +    case 2:
> +        data = gic_its_cntrl_readw(opaque, addr);
> +        break;
> +    case 4:
> +        data = gic_its_cntrl_readl(opaque, addr);
> +        break;
> +    case 8:
> +        data = gic_its_cntrl_readll(opaque, addr);
> +        break;
> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: size %u\n", __func__, size);
> +        assert(0);
> +        break;
> +    }
> +    DPRINTF("offset %p data %p\n", (void *) addr, (void *) data);
> +    return data;
> +}
> +
> +static void gic_its_cntrl_write(void *opaque, hwaddr addr, uint64_t data, 
> unsigned size)
> +{
> +    DPRINTF("offset %p data %p\n", (void *) addr, (void *) data);
> +    switch (size) {
> +    case 1:
> +        gic_its_cntrl_writeb(opaque, addr, data);
> +        break;
> +    case 2:
> +        gic_its_cntrl_writew(opaque, addr, data);
> +        break;
> +    case 4:
> +        gic_its_cntrl_writel(opaque, addr, data);
> +        break;
> +    case 8:
> +        gic_its_cntrl_writell(opaque, addr, data);
> +        break;
> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: size %u\n", __func__, size);
> +        assert(0);
> +        break;
> +    }
> +}
> +
> +static const MemoryRegionOps gic_its_cntrl_ops = {
> +    .read = gic_its_cntrl_read,
> +    .write = gic_its_cntrl_write,
> +    .impl = {
> +         .min_access_size = 4,
> +         .max_access_size = 8,
> +     },
> +    .endianness = DEVICE_NATIVE_ENDIAN,
> +};
> +
> +
> +static uint64_t gic_lpi_readb(void *opaque, hwaddr offset)
the LPI terminology used here confused me. I guess this corresponds to
re-distributor region ops which is refered as is in the spec and
encompasses more that just LPI management: SGI, PPI, LPI, ...
Why not using re-distrib terminology?
> +{
> +    GICState *s = (GICState *)opaque;
> +    uint64_t res = 0;
> +    uint64_t sgi_ppi, core, off;
> +    uint64_t cm, i;
> +
> +    /* Table 3-2 page 3-4
> +     * [          1<bits-for-core#>x<16-bits-offset>]
> +     * x = 0: LPIs
> +     * x = 1: SGIs & PPIs
> +     */
I do not succeed it matching above desc with 5.4.4 GICv3 archi spec
(22). Maybe I don't have the good spec.
> +    off = offset;
> +    sgi_ppi = off & (1 << 16);
> +    core = (off >> 17) & s->cpu_mask;
> +    offset = off & 0xFFFF;
> +    cm = 1ll << core;
> +
> +    if (sgi_ppi) {
> +        /* SGIs, PPIs */
> +        /* Interrupt Set/Clear Enable.  */
> +        if (offset < 0x200) {
> +            int irq;
> +            if (offset < 0x180)
> +                irq = (offset - 0x100) * 8;
what if offset < 0x100, IGROUPR0 read? irq < 0?
> +            else
> +                irq = (offset - 0x180) * 8;
> +            irq += GICV3_BASE_IRQ;
> +            if (irq >= s->num_irq)
> +                goto bad_reg;
> +            res = 0;
> +            for (i = 0; i < 8; i++) {
> +                if (GIC_TEST_ENABLED(irq + i, cm)) {
> +                    res |= (1 << i);
> +                }
> +            }
what if any ISPENDR0 (0x200), ISACTIVER0 (0x300) read access? This will
enter next else-if? Do I miss something?
Shouldn't follow basically the same struct as for the distributor?
> +        } else if (offset < 0xc00) {
> +            /* Interrupt Priority.  */
> +            int irq;
> +            irq = (offset - 0x400) + GICV3_BASE_IRQ;
> +            if (irq >= s->num_irq)
> +                goto bad_reg;
> +            res = GIC_GET_PRIORITY(irq, core);
> +        }
> +    } else {
> +        /* LPIs */
Control and Physical LPIs?
> +        if (offset & 3)
> +            return 0;
> +        if (offset < 0x100) {
> +            if (offset == 0) {/* GICR_CTLR */
> +                DPRINTF("Redist-GICR_CTLR-CPU caller cpu(%d) core(%lu)\n",
> +                    gic_get_current_cpu(s), core);
> +                return 0;
> +            }
> +            if (offset == 4)
> +                return 0x43B; /* GICR_IIDR */
> +            if (offset == 0x8) { /* GICR_TYPER */
> +                res = core << 8; /* Linear */
> +                /* Simple clustering */
> +                res |= (core % 8) << 32; /* Afinity 0 */
> +                res |= (core / 8) << 40; /* Afinity 1 */
> +                if (core == s->num_cpu - 1) {
> +                    /* Last redistributer */
> +                    res |= 1 << 4;
> +                }
> +                return res;
> +            }
> +            if (offset == 0xc) { /* GICR_TYPER */
> +                /* should write readll */
?
> +                return 0;
> +            }
> +            if (offset == 0x14) { /* GICR_WAKER */
> +                if (s->cpu_enabled[core])
> +                    return 0;
> +                else
> +                    return GICR_WAKER_ProcessorSleep;
> +                DPRINTF("Redist-CPU (%d) is enabled(%d)\n",
> +                        gic_get_current_cpu(s), s->cpu_enabled[core]);
> +
> +            }
> +            if (offset >= 0x80 && offset < 0xFFD0)
some regs are WO
> +                return 0;
> +            goto bad_reg;
> +        }
> +        if (offset < 0xffd0) {
> +            goto bad_reg;
> +        } else /* offset >= 0xffd0 */ {
> +            if (offset & 3) {
> +                res = 0;
> +            } else {
> +                res = gic_lpi_ids[(offset - 0xffd0) >> 2];
> +            }
> +        }
> +    }
> +    return res;
> +bad_reg:
> +    qemu_log_mask(LOG_GUEST_ERROR,
> +                  "%s: Bad offset %x\n", __func__, (int)offset);
> +    return 0;
> +}
> +
> +static uint64_t gic_lpi_readw(void *opaque, hwaddr offset)
> +{
> +    uint64_t val;
> +    val = gic_lpi_readb(opaque, offset);
> +    val |= gic_lpi_readb(opaque, offset + 1) << 8;
> +    return val;
> +}
> +
> +static uint64_t gic_lpi_readl(void *opaque, hwaddr offset)
> +{
> +    uint64_t val;
> +    val = gic_lpi_readw(opaque, offset);
> +    val |= gic_lpi_readw(opaque, offset + 2) << 16;
> +    return val;
> +}
> +
> +static uint64_t gic_lpi_readll(void *opaque, hwaddr offset)
> +{
> +    uint64_t val;
> +    val = gic_lpi_readl(opaque, offset);
> +    val |= gic_lpi_readl(opaque, offset + 4) << 32;
> +    return val;
> +}
> +
> +
> +static void gic_lpi_writeb(void *opaque, hwaddr offset,
> +                            uint64_t value)
> +{
> +    GICState *s = (GICState *)opaque;
> +    uint64_t sgi_ppi, core, off;
> +    uint64_t cm;
> +    /* Table 3-2 page 3-4
> +     * [          1<bits-for-core#>x<16-bits-offset>]
> +     * x = 0: LPIs
> +     * x = 1: SGIs & PPIs
> +     */
> +    off = offset;
> +    sgi_ppi = off & (1 << 16);
> +    core = (off >> 17) & s->cpu_mask;
> +    offset = off & 0xFFFF;
> +    cm = 1ll << core;
> +
> +    if (sgi_ppi) {
> +        /* SGIs, PPIs */
> +        if (offset < 0x180) {
> +            /* Interrupt Set Enable.  */
> +            int irq, i;
> +            irq = (offset - 0x100) * 8 + GICV3_BASE_IRQ;
> +            if (irq >= s->num_irq)
> +                goto bad_reg;
> +            if (irq >= GICV3_INTERNAL) {
> +                DPRINTF("ISENABLERn non Internal should be only in 
> distributer %d\n", irq);
> +                /* The registers after 0x100 are reserved */
> +                return;
> +            }
> +            if (irq < GICV3_NR_SGIS) {
> +                value = 0xff;
> +            }
> +
> +            for (i = 0; i < 8; i++) {
> +                if (value & (1 << i)) {
> +                    /* This is redistributer ALL doesn't apply */
> +                    if (!GIC_TEST_ENABLED(irq + i, cm)) {
> +                        DPRINTF("Enabled IRQ %d\n", irq + i);
> +                    }
> +                    GIC_SET_ENABLED(irq + i, cm);
> +                    /* If a raised level triggered IRQ enabled then mark
> +                       is as pending.  */
> +                    if (GIC_TEST_LEVEL(irq + i, cm)
> +                            && !GIC_TEST_EDGE_TRIGGER(irq + i)) {
> +                        DPRINTF("Set %d pending mask %lx\n", irq + i, cm);
> +                        GIC_SET_PENDING(irq + i, cm);
> +                    }
> +                }
> +            }
> +        } else if (offset < 0x200) {
> +            /* Interrupt Clear Enable.  */
> +            int irq, i;
> +            irq = (offset - 0x180) * 8 + GICV3_BASE_IRQ;
> +            if (irq >= s->num_irq)
> +                goto bad_reg;
> +            if (irq >= GICV3_INTERNAL) {
> +                DPRINTF("ICENABLERn non Internal should be only in 
> distributer %d\n", irq);
> +                /* The registers after 0x180 are reserved */
> +                return;
> +            }
> +            if (irq < GICV3_NR_SGIS) {
> +                value = 0;
> +            }
> +
> +            for (i = 0; i < 8; i++) {
> +                if (value & (1 << i)) {
> +                    /* This is redistributer ALL doesn't apply */
> +                    if (GIC_TEST_ENABLED(irq + i, cm)) {
> +                        DPRINTF("Disabled IRQ %d\n", irq + i);
> +                    }
> +                    GIC_CLEAR_ENABLED(irq + i, cm);
> +                }
> +            }
> +        } else if (offset < 0xc00) {
> +            /* Interrupt Priority. */
> +            int irq;
> +            irq = (offset - 0x400) + GICV3_BASE_IRQ;
> +            if (irq >= s->num_irq)
> +                goto bad_reg;
> +            if (irq >= GICV3_INTERNAL) {
> +                DPRINTF("IPRIORITYRn non Internal should be only in 
> distributer %d\n", irq);
> +                /* The registers after 0x180 are reserved */
> +                return;
> +            }
> +            gicv3_set_priority(s, core, irq, value);
> +        }
> +    } else {
> +        /* LPIs */
> +        if (offset == 0x14) { /* GICR_WAKER */
> +            if (value & GICR_WAKER_ProcessorSleep)
> +                s->cpu_enabled[core] = 0;
> +            else
> +                s->cpu_enabled[core] = 1;
> +            DPRINTF("Redist-CPU (%d) core(%lu) set enabled(%d)\n",
> +                    gic_get_current_cpu(s), core, s->cpu_enabled[core]);
> +       }
> +    }
> +    gicv3_update(s);
> +    return;
> +
> +bad_reg:
> +    qemu_log_mask(LOG_GUEST_ERROR,
> +                  "%s: Bad offset %x\n", __func__, (int)offset);
> +}
> +
> +static void gic_lpi_writew(void *opaque, hwaddr offset,
> +                            uint64_t value)
> +{
> +    gic_lpi_writeb(opaque, offset, value & 0xff);
> +    gic_lpi_writeb(opaque, offset + 1, value >> 8);
> +}
> +
> +static void gic_lpi_writel(void *opaque, hwaddr offset,
> +                            uint64_t value)
> +{
> +    gic_lpi_writew(opaque, offset, value & 0xffff);
> +    gic_lpi_writew(opaque, offset + 2, value >> 16);
> +}
> +
> +static void gic_lpi_writell(void *opaque, hwaddr offset,
> +                            uint64_t value)
> +{
> +    gic_lpi_writel(opaque, offset, value & 0xffffffff);
> +    gic_lpi_writel(opaque, offset + 4, value >> 32);
> +}
> +
> +static uint64_t gic_lpi_read(void *opaque, hwaddr addr, unsigned size)
> +{
> +    uint64_t data;
> +    switch (size) {
> +    case 1:
> +        data = gic_lpi_readb(opaque, addr);
> +        break;
> +    case 2:
> +        data = gic_lpi_readw(opaque, addr);
> +        break;
> +    case 4:
> +        data = gic_lpi_readl(opaque, addr);
> +        break;
> +    case 8:
> +        data = gic_lpi_readll(opaque, addr);
> +        break;
> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: size %u\n", __func__, size);
> +        assert(0);
> +        break;
> +    }
> +//    DPRINTF("[%s] offset %p data %p\n", addr & (1 << 16) ? "SGI/PPI" : 
> "LPI" , (void *) addr, (void *) data);
> +    return data;
> +}
> +
> +static void gic_lpi_write(void *opaque, hwaddr addr, uint64_t data, unsigned 
> size)
> +{
> +//    DPRINTF("[%s] offset %p data %p\n", addr & (1 << 16) ? "SGI/PPI" : 
> "LPI" , (void *) addr, (void *) data);
> +    switch (size) {
> +    case 1:
> +        gic_lpi_writeb(opaque, addr, data);
> +        break;
> +    case 2:
> +        gic_lpi_writew(opaque, addr, data);
> +        break;
> +    case 4:
> +        gic_lpi_writel(opaque, addr, data);
> +        break;
> +    case 8:
> +        gic_lpi_writell(opaque, addr, data);
> +        break;
> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: size %u\n", __func__, size);
> +        assert(0);
> +        break;
> +    }
> +}
> +
> +static const MemoryRegionOps  gic_lpi_ops = {
> +    .read = gic_lpi_read,
> +    .write = gic_lpi_write,
> +    .impl = {
> +         .min_access_size = 4,
> +         .max_access_size = 8,
> +     },
> +    .endianness = DEVICE_NATIVE_ENDIAN,
> +};
> +
> +void gicv3_init_irqs_and_distributor(GICState *s, int num_irq)
> +{
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(s);
> +    int i;
> +
> +    DPRINTF(" ---- gicv3_init_irqs_and_distributor   ----- \n");
> +    i = s->num_irq - GICV3_INTERNAL;
> +    /* For the GIC, also expose incoming GPIO lines for PPIs for each CPU.
> +     * GPIO array layout is thus:
> +     *  [0..N-1] spi
> +     *  [N..N+31] PPIs for CPU 0
> +     *  [N+32..N+63] PPIs for CPU 1
> +     *   ...
> +     */
> +    i += (GICV3_INTERNAL * s->num_cpu);
> +    qdev_init_gpio_in(DEVICE(s), gic_set_irq, i);
> +    for (i = 0; i < NUM_CPU(s); i++) {
> +        sysbus_init_irq(sbd, &s->parent_irq[i]);
> +    }
> +
> +    memory_region_init_io(&s->iomem_dist, OBJECT(s), &gic_dist_ops, 
> s,"gic_dist", 0x10000);
> +    memory_region_init_io(&s->iomem_spi, OBJECT(s), &gic_spi_ops, 
> s,"gic_spi", 0x10000);
as commented above, not straightward to understand what gic_spi memroy
region corresponds to. My guess was it is the so-called Distributor
Messgae Based Interrupt Register Map. Since the doc describes this in
the dist regs why not renaming it into something like gic_dist_mbi?
Also GICD_TYPER == 0 so MBIs are not exposed as supported?
Besides, this seems to target GICv2m compatibility. Is it really usefull
at that stage? I would put it a separate patch file.
> +    memory_region_init_io(&s->iomem_its_cntrl, OBJECT(s), 
> &gic_its_cntrl_ops, s,"gic_its_cntrl", 0x10000);
> +    memory_region_init_io(&s->iomem_its, OBJECT(s), &gic_its_ops, 
> s,"gic_its_trans", 0x10000);
> +    memory_region_init_io(&s->iomem_lpi, OBJECT(s), &gic_lpi_ops, 
> s,"gic_lpi", 0x800000);
why is the LPI region so huge? Don't we have 2 pages starting at RD_base
and SGI_base?
> +}
> +
> +static void arm_gic_realize(DeviceState *dev, Error **errp)
> +{
> +    /* Device instance realize function for the GIC sysbus device */
> +    int i;
> +    GICState *s = ARM_GIC(dev);
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> +    ARMGICClass *agc = ARM_GIC_GET_CLASS(s);
> +    Error *local_err = NULL;
> +    uint32_t power2;
> +
> +    agc->parent_realize(dev, &local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        return;
> +    }
> +
> +    /* Uses system registers mode */
> +    gic_sre = 1;
> +
> +    /* Tell the common code we're a GICv3 */
> +    s->revision = REV_V3;
> +    /* NO GICv2 backwards computability support */
> +    s->gicd_ctlr = GICD_CTLR_ARE_S;
> +    gicv3_init_irqs_and_distributor(s, s->num_irq);
> +
> +    /* Compute mask for decoding the core number in redistributer */
> +    if (is_power_of_2(NUM_CPU(s)))
> +        power2 = NUM_CPU(s);
> +    else
> +        /* QEMU has only  pow2floor !!! */
> +        power2 = pow2floor(2 * NUM_CPU(s));
> +    s->cpu_mask = (power2 - 1);
> +
> +    DPRINTF(" -- NUM_CPUS(%d) - cpu mask(0%x) -- \n", NUM_CPU(s), 
> s->cpu_mask);
> +
> +    for (i = 0; i < GICV3_NCPU; i++)
> +        s->cpu_enabled[i] = 0;
> +
> +    /* Init memory regions */
> +    sysbus_init_mmio(sbd, &s->iomem_dist);
> +    sysbus_init_mmio(sbd, &s->iomem_spi);
> +    sysbus_init_mmio(sbd, &s->iomem_its_cntrl);
> +    sysbus_init_mmio(sbd, &s->iomem_its);
> +    sysbus_init_mmio(sbd, &s->iomem_lpi);
> +}
> +
> +void armv8_gicv3_set_sgi(void *opaque, int cpuindex, uint64_t value)
> +{
> +    GICState *s = (GICState *) opaque;
> +    int irq, i;
> +    uint32_t target;
> +    uint64_t cm = (1ll << cpuindex);
> +
> +    /* Page 2227 ICC_SGI1R_EL1 */
> +
> +    irq = (value >> 24) & 0xf;
> +
> +    /* The external routines use the hardware vector numbering, ie. the first
> +     * IRQ is #16.  The internal GIC routines use #32 as the first IRQ.
> +     */
> +    if (irq >= 16)
> +        irq += 16;
> +
> +    /* IRM bit */
> +    if (value & (1ll << 40)) {
> +        /* Send to all the cores exclude self */
> +        for (i = 0; i < cpuindex; i++) {
> +            s->sgi_state[irq].pending[i] |= cm;
> +        }
> +        for (i = cpuindex + 1; i < s->num_cpu; i++) {
> +            s->sgi_state[irq].pending[i] |= cm;
> +        }
> +        GIC_SET_PENDING(irq, (ALL_CPU_MASK & ~cm));
> +        DPRINTF("cpu(%d) sends irq(%d) to ALL exclude self\n", cpuindex, 
> irq);
> +    } else {
> +        /* Find linear of first core in cluster. See page 2227 ICC_SGI1R_EL1
> +         * With our GIC-500 implementation we can have 16 clusters of 8 cpu 
> each
> +         */
> +#if 1
> +        target = (value & (0xfl << 16)) >> (16 - 3); /* shift 16 mult by 8 */
> +#else
> +        /* Prep for more advanced GIC */
> +        target  = (value & (0xffl << 16)) >> (16 - 8);
> +        target |= (value & (0xffl << 32)) >> (32 - 16);
> +        target |= (value & (0xffl << 48)) >> (48 - 24);
> +#endif
> +
> +        /* Use 8 and not 16 since only 8 cores can be in a cluster of 
> GIC-500 */
> +        assert((value & 0xff00) == 0);
> +        for (i = 0; i < 8; i++) {
> +            if (value & (1 << i)) {
> +                //DPRINTF("cpu(%d) sends irq(%d) to cpu(%d)\n", cpuindex, 
> irq, target + i);
> +                s->sgi_state[irq].pending[target + i] |= cm;
> +                GIC_SET_PENDING(irq, (1ll << (target + i)));
> +             }
> +         }
> +    }
> +    gicv3_update(s);
> +}
> +
> +uint64_t armv8_gicv3_acknowledge_irq(void *opaque, int cpuindex)
> +{
> +    GICState *s = (GICState *) opaque;
> +    return gicv3_acknowledge_irq(s, cpuindex);
> +}
> +
> +void armv8_gicv3_complete_irq(void *opaque, int cpuindex, int irq)
> +{
> +    GICState *s = (GICState *) opaque;
> +    irq &= 0xffffff;
> +    gicv3_complete_irq(s, cpuindex, irq);
> +}
> +
> +uint64_t armv8_gicv3_get_priority_mask(void *opaque, int cpuindex)
> +{
> +    GICState *s = (GICState *) opaque;
> +    return s->priority_mask[cpuindex];
> +}
> +
> +void armv8_gicv3_set_priority_mask(void *opaque, int cpuindex, uint32_t mask)
> +{
> +    GICState *s = (GICState *) opaque;
> +    s->priority_mask[cpuindex] = mask & 0xff;
> +    gicv3_update(s);
> +}
> +
> +uint64_t armv8_gicv3_get_sre(void *opaque)
> +{
> +     /* Uses only system registers, no memory mapped access GICv2 mode */
> +     return gic_sre;
> +}
> +
> +void armv8_gicv3_set_sre(void *opaque, uint64_t sre)
> +{
> +     if (!(sre & 1)) {
> +        DPRINTF("Try to use memory mapped interface sre(0x%lx)\n", sre);
> +        assert(0);
> +     }
> +     gic_sre = sre;
> +}
> +
> +static void arm_gicv3_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    ARMGICClass *agc = ARM_GIC_CLASS(klass);
> +
> +    agc->parent_realize = dc->realize;
> +    dc->realize = arm_gic_realize;
> +}
> +
> +static const TypeInfo arm_gicv3_info = {
> +    .name = TYPE_ARM_GICV3,
> +    .parent = TYPE_ARM_GICV3_COMMON,
> +    .instance_size = sizeof(GICState),
> +    .class_init = arm_gicv3_class_init,
> +    .class_size = sizeof(ARMGICClass),
> +};
> +
> +static void arm_gicv3_register_types(void)
> +{
> +    type_register_static(&arm_gicv3_info);
> +}
> +
> +type_init(arm_gicv3_register_types)
> diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
> new file mode 100644
> index 0000000..5c26e9b
> --- /dev/null
> +++ b/hw/intc/arm_gicv3_common.c
> @@ -0,0 +1,199 @@
> +/*
> + * ARM GIC support - common bits of emulated and KVM kernel model
> + *
> + * Copyright (c) 2012 Linaro Limited
> + * Copyright (c) 2015 Huawei.
> + * Written by Peter Maydell
> + * Extended to 64 cores by Shlomo Pongratz
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "gicv3_internal.h"
> +
> +static void gicv3_pre_save(void *opaque)
> +{
> +    GICState *s = (GICState *)opaque;
> +    ARMGICCommonClass *c = ARM_GIC_COMMON_GET_CLASS(s);
> +
> +    if (c->pre_save) {
> +        c->pre_save(s);
> +    }
> +}
> +
> +static int gicv3_post_load(void *opaque, int version_id)
> +{
> +    GICState *s = (GICState *)opaque;
> +    ARMGICCommonClass *c = ARM_GIC_COMMON_GET_CLASS(s);
> +
> +    if (c->post_load) {
> +        c->post_load(s);
> +    }
> +    return 0;
> +}
> +
> +static const VMStateDescription vmstate_gicv3_irq_state = {
> +    .name = "arm_gicv3_irq_state",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT64(enabled, gicv3_irq_state),
> +        VMSTATE_UINT64(pending, gicv3_irq_state),
> +        VMSTATE_UINT64(active, gicv3_irq_state),
> +        VMSTATE_UINT64(level, gicv3_irq_state),
> +        VMSTATE_BOOL(edge_trigger, gicv3_irq_state),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +static const VMStateDescription vmstate_gicv3_sgi_state = {
> +    .name = "arm_gicv3_sgi_state",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT64_ARRAY(pending, gicv3_sgi_state, GICV3_NCPU),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +static const VMStateDescription vmstate_gicv3 = {
> +    .name = "arm_gicv3",
> +    .version_id = 7,
> +    .minimum_version_id = 7,
> +    .pre_save = gicv3_pre_save,
> +    .post_load = gicv3_post_load,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_BOOL(enabled, GICState),
> +        VMSTATE_BOOL_ARRAY(cpu_enabled, GICState, GICV3_NCPU),
> +        VMSTATE_STRUCT_ARRAY(irq_state, GICState, GICV3_MAXIRQ, 1,
> +                             vmstate_gicv3_irq_state, gicv3_irq_state),
> +        VMSTATE_UINT64_ARRAY(irq_target, GICState, GICV3_MAXIRQ),
> +        VMSTATE_UINT8_2DARRAY(priority1, GICState, GICV3_INTERNAL, 
> GICV3_NCPU),
> +        VMSTATE_UINT8_ARRAY(priority2, GICState, GICV3_MAXIRQ - 
> GICV3_INTERNAL),
> +        VMSTATE_UINT16_2DARRAY(last_active, GICState, GICV3_MAXIRQ, 
> GICV3_NCPU),
> +        VMSTATE_STRUCT_ARRAY(sgi_state, GICState, GICV3_NR_SGIS, 1,
> +                             vmstate_gicv3_sgi_state, gicv3_sgi_state),
> +        VMSTATE_UINT16_ARRAY(priority_mask, GICState, GICV3_NCPU),
> +        VMSTATE_UINT16_ARRAY(running_irq, GICState, GICV3_NCPU),
> +        VMSTATE_UINT16_ARRAY(running_priority, GICState, GICV3_NCPU),
> +        VMSTATE_UINT16_ARRAY(current_pending, GICState, GICV3_NCPU),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +static void arm_gicv3_common_realize(DeviceState *dev, Error **errp)
> +{
> +    GICState *s = ARM_GIC_COMMON(dev);
> +    int num_irq = s->num_irq;
> +
> +    if (s->num_cpu > GICV3_NCPU) {
> +        error_setg(errp, "requested %u CPUs exceeds GIC maximum %d",
> +                   s->num_cpu, GICV3_NCPU);
> +        return;
> +    }
> +    s->num_irq += GICV3_BASE_IRQ;
> +    if (s->num_irq > GICV3_MAXIRQ) {
> +        error_setg(errp,
> +                   "requested %u interrupt lines exceeds GIC maximum %d",
> +                   num_irq, GICV3_MAXIRQ);
> +        return;
> +    }
> +    /* 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)) {
> +        error_setg(errp,
> +                   "%d interrupt lines unsupported: not divisible by 32",
> +                   num_irq);
> +        return;
> +    }
> +}
> +
> +static void arm_gicv3_common_reset(DeviceState *dev)
> +{
> +    GICState *s = ARM_GIC_COMMON(dev);
> +    int i;
> +
> +    /* Note num_cpu and num_irq are properties */
> +
> +    /* don't reset anything assigned in arm_gic_realize or any property */
> +
> +    s->enabled = false;
> +
> +    /* NO GICv2 backwards computability support */
> +    s->gicd_ctlr = GICD_CTLR_ARE_S;
> +    for (i = 0; i < s->num_cpu; i++) {
> +        s->priority_mask[i] = 0;
> +        s->current_pending[i] = 1023;
> +        s->running_irq[i] = 1023;
> +        s->running_priority[i] = 0x100;
> +        s->cpu_enabled[i] = false;
> +    }
> +
> +    memset(s->irq_state, 0, sizeof(s->irq_state));
> +    /* GIC-500 comment 'j' SGI are always enabled */
What doc do you use where I can find those comments?
> +    for (i = 0; i < GICV3_NR_SGIS; i++) {
> +        GIC_SET_ENABLED(i, ALL_CPU_MASK);
> +        GIC_SET_EDGE_TRIGGER(i);
> +    }
> +    memset(s->sgi_state, 0, sizeof(s->sgi_state));
> +    memset(s->irq_target, 0, sizeof(s->irq_target));
> +    if (s->num_cpu == 1) {
> +        /* For uniprocessor GICs all interrupts always target the sole CPU */
> +        for (i = 0; i < GICV3_MAXIRQ; i++) {
> +            s->irq_target[i] = 1;
> +        }
> +    }
> +    memset(s->priority1, 0, sizeof(s->priority1));
> +    memset(s->priority2, 0, sizeof(s->priority2));
> +    memset(s->last_active, 0, sizeof(s->last_active));
> +}
> +
> +static Property arm_gicv3_common_properties[] = {
> +    DEFINE_PROP_UINT32("num-cpu", GICState, num_cpu, 1),
> +    DEFINE_PROP_UINT32("num-irq", GICState, num_irq, 32),
> +    /* Revision can be 3 for GIC architecture specification
> +     * versions 1 or 2, or 0 to indicate the legacy 11MPCore GIC.
above comment to be removed
> +     * (Internally, 0xffffffff also indicates "not a GIC but an NVIC".)
> +     */
> +    DEFINE_PROP_UINT32("revision", GICState, revision, 3),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void arm_gicv3_common_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->reset = arm_gicv3_common_reset;
> +    dc->realize = arm_gicv3_common_realize;
> +    dc->props = arm_gicv3_common_properties;
> +    dc->vmsd = &vmstate_gicv3;
> +}
> +
> +static const TypeInfo arm_gicv3_common_type = {
> +    .name = TYPE_ARM_GICV3_COMMON,
> +    .parent = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(GICState),
> +    .class_size = sizeof(ARMGICCommonClass),
> +    .class_init = arm_gicv3_common_class_init,
> +    .abstract = true,
> +};
> +
> +static void register_types(void)
> +{
> +    type_register_static(&arm_gicv3_common_type);
> +}
> +
> +type_init(register_types)
> diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h
> new file mode 100644
> index 0000000..c3f2487
> --- /dev/null
> +++ b/hw/intc/gicv3_internal.h
> @@ -0,0 +1,151 @@
> +/*
> + * ARM GIC support - internal interfaces
> + *
> + * Copyright (c) 2012 Linaro Limited
> + * Copyright (c) 2015 Huawei.
> + * Written by Peter Maydell
> + * Extended to 64 cores by Shlomo Pongratz
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef QEMU_ARM_GICV3_INTERNAL_H
> +#define QEMU_ARM_GICV3_INTERNAL_H
> +
> +#include "hw/intc/arm_gicv3.h"
> +
> +#define ALL_CPU_MASK ((uint64_t) (0xffffffffffffffff))
> +
> +/* The NVIC has 16 internal vectors.  However these are not exposed
> +   through the normal GIC interface.  */
above comment should be removed?
> +#define GICV3_BASE_IRQ (0)
> +
> +#define GIC_SET_ENABLED(irq, cm) s->irq_state[irq].enabled |= (cm)
> +#define GIC_CLEAR_ENABLED(irq, cm) s->irq_state[irq].enabled &= ~(cm)
> +#define GIC_TEST_ENABLED(irq, cm) ((s->irq_state[irq].enabled & (cm)) != 0)
> +#define GIC_SET_PENDING(irq, cm) s->irq_state[irq].pending |= (cm)
> +#define GIC_TEST_PENDING(irq, cm) ((s->irq_state[irq].pending & (cm)) != 0)
> +#define GIC_CLEAR_PENDING(irq, cm) s->irq_state[irq].pending &= ~(cm)
> +#define GIC_SET_ACTIVE(irq, cm) s->irq_state[irq].active |= (cm)
> +#define GIC_CLEAR_ACTIVE(irq, cm) s->irq_state[irq].active &= ~(cm)
> +#define GIC_TEST_ACTIVE(irq, cm) ((s->irq_state[irq].active & (cm)) != 0)
> +#define GIC_SET_LEVEL(irq, cm) s->irq_state[irq].level |= (cm)
> +#define GIC_CLEAR_LEVEL(irq, cm) s->irq_state[irq].level &= ~(cm)
> +#define GIC_TEST_LEVEL(irq, cm) ((s->irq_state[irq].level & (cm)) != 0)
> +#define GIC_SET_EDGE_TRIGGER(irq) s->irq_state[irq].edge_trigger = true
> +#define GIC_CLEAR_EDGE_TRIGGER(irq) s->irq_state[irq].edge_trigger = false
> +#define GIC_TEST_EDGE_TRIGGER(irq) (s->irq_state[irq].edge_trigger)
> +#define GIC_GET_PRIORITY(irq, cpu) (((irq) < GICV3_INTERNAL) ?          \
> +                                    s->priority1[irq][cpu] :            \
> +                                    s->priority2[(irq) - GICV3_INTERNAL])
> +#define GIC_TARGET(irq) s->irq_target[irq]
> +
> +/* The special cases for the revision property: */
> +#define REV_11MPCORE 0
to be removed
> +#define REV_V3 3
> +#define REV_NVIC 0xffffffff
to be removed
> +
> +uint32_t gicv3_acknowledge_irq(GICState *s, int cpu);
> +void gicv3_complete_irq(GICState *s, int cpu, int irq);
> +void gicv3_update(GICState *s);
> +void gicv3_init_irqs_and_distributor(GICState *s, int num_irq);
> +void gicv3_set_priority(GICState *s, int cpu, int irq, uint8_t val);
you could put armv8_gicv3 declaration here instead of in arm_gicv3?
> +
> +static inline bool gic_test_pending(GICState *s, int irq, uint64_t cm)
> +{
> +    /* Edge-triggered interrupts are marked pending on a rising edge, but
> +     * level-triggered interrupts are either considered pending when the
> +     * level is active or if software has explicitly written to
> +     * GICD_ISPENDR to set the state pending.
> +     */
> +    return (s->irq_state[irq].pending & cm) ||
> +        (!GIC_TEST_EDGE_TRIGGER(irq) && GIC_TEST_LEVEL(irq, cm));
> +}
double line
> +
> +
Some macros below are never used
> +#define GICD_CTLR            0x0000
> +#define GICD_TYPER           0x0004
> +#define GICD_IIDR            0x0008
> +#define GICD_STATUSR         0x0010
> +#define GICD_SETSPI_NSR      0x0040
> +#define GICD_CLRSPI_NSR      0x0048
> +#define GICD_SETSPI_SR       0x0050
> +#define GICD_CLRSPI_SR       0x0058
> +#define GICD_SEIR            0x0068
> +#define GICD_ISENABLER       0x0100
> +#define GICD_ICENABLER       0x0180
> +#define GICD_ISPENDR         0x0200
> +#define GICD_ICPENDR         0x0280
> +#define GICD_ISACTIVER       0x0300
> +#define GICD_ICACTIVER       0x0380
> +#define GICD_IPRIORITYR      0x0400
> +#define GICD_ICFGR           0x0C00
> +#define GICD_IROUTER         0x6000
> +#define GICD_PIDR2           0xFFE8
> +
> +/* GICD_CTLR fields  */
> +#define GICD_CTLR_ENABLE_GRP0           (1U << 0)
> +#define GICD_CTLR_ENABLE_GRP1           (1U << 1)
> +#define GICD_CTLR_ARE_S                 (1U << 4)
> +#define GICD_CTLR_ARE_NS                (1U << 5)
> +#define GICD_CTLR_DS                    (1U << 6)
> +#define GICD_CTLR_RWP                   (1U << 31)
> +
> +
double empty line
> +#define GICD_IROUTER_SPI_MODE_ONE    (0U << 31)
> +#define GICD_IROUTER_SPI_MODE_ANY    (1U << 31)
> +
> +#define GIC_PIDR2_ARCH_MASK   0xf0
> +#define GIC_PIDR2_ARCH_GICv3  0x30
> +#define GIC_PIDR2_ARCH_GICv4  0x40
> +
> +/*
> + * Re-Distributor registers, offsets from RD_base
> + */
> +#define GICR_CTLR             GICD_CTLR
> +#define GICR_IIDR             0x0004
> +#define GICR_TYPER            0x0008
> +#define GICR_STATUSR          GICD_STATUSR
> +#define GICR_WAKER            0x0014
> +#define GICR_SETLPIR          0x0040
> +#define GICR_CLRLPIR          0x0048
> +#define GICR_SEIR             GICD_SEIR
> +#define GICR_PROPBASER        0x0070
> +#define GICR_PENDBASER        0x0078
> +#define GICR_INVLPIR          0x00A0
> +#define GICR_INVALLR          0x00B0
> +#define GICR_SYNCR            0x00C0
> +#define GICR_MOVLPIR          0x0100
> +#define GICR_MOVALLR          0x0110
> +#define GICR_PIDR2            GICD_PIDR2
> +
> +#define GICR_WAKER_ProcessorSleep    (1U << 1)
> +#define GICR_WAKER_ChildrenAsleep    (1U << 2)
> +
> +/*
> + * Re-Distributor registers, offsets from SGI_base
> + */
> +#define GICR_ISENABLER0         GICD_ISENABLER
> +#define GICR_ICENABLER0         GICD_ICENABLER
> +#define GICR_ISPENDR0           GICD_ISPENDR
> +#define GICR_ICPENDR0           GICD_ICPENDR
> +#define GICR_ISACTIVER0         GICD_ISACTIVER
> +#define GICR_ICACTIVER0         GICD_ICACTIVER
> +#define GICR_IPRIORITYR0        GICD_IPRIORITYR
> +#define GICR_ICFGR0             GICD_ICFGR
> +
> +#define GICR_TYPER_VLPIS        (1U << 1)
> +#define GICR_TYPER_LAST         (1U << 4)
> +
> +#endif /* !QEMU_ARM_GIC_INTERNAL_H */
> diff --git a/include/hw/intc/arm_gicv3.h b/include/hw/intc/arm_gicv3.h
> new file mode 100644
> index 0000000..e315bda
> --- /dev/null
> +++ b/include/hw/intc/arm_gicv3.h
> @@ -0,0 +1,44 @@
> +/*
> + * ARM GIC support
> + *
> + * Copyright (c) 2012 Linaro Limited
> + * Copyright (c) 2015 Huawei.
> + * Written by Peter Maydell
> + * Extended to 64 cores by Shlomo Pongratz
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef HW_ARM_GICV3_H
> +#define HW_ARM_GICV3_H
> +
> +#include "arm_gicv3_common.h"
> +
> +#define TYPE_ARM_GICV3 "arm_gicv3"
> +#define ARM_GIC(obj) \
> +     OBJECT_CHECK(GICState, (obj), TYPE_ARM_GICV3)
> +#define ARM_GIC_CLASS(klass) \
> +     OBJECT_CLASS_CHECK(ARMGICClass, (klass), TYPE_ARM_GICV3)
> +#define ARM_GIC_GET_CLASS(obj) \
> +     OBJECT_GET_CLASS(ARMGICClass, (obj), TYPE_ARM_GICV3)
> +
> +typedef struct ARMGICClass {
> +    /*< private >*/
> +    ARMGICCommonClass parent_class;
> +    /*< public >*/
> +
> +    DeviceRealize parent_realize;
> +} ARMGICClass;
> +
> +#endif
> diff --git a/include/hw/intc/arm_gicv3_common.h 
> b/include/hw/intc/arm_gicv3_common.h
> new file mode 100644
> index 0000000..aeb613c
> --- /dev/null
> +++ b/include/hw/intc/arm_gicv3_common.h
> @@ -0,0 +1,110 @@
> +/*
> + * ARM GIC support
> + *
> + * Copyright (c) 2012 Linaro Limited
> + * Copyright (c) 2015 Huawei.
> + * Written by Peter Maydell
> + * Extended to 64 cores by Shlomo Pongratz
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef HW_ARM_GICV3_COMMON_H
> +#define HW_ARM_GICV3_COMMON_H
> +
> +#include "hw/sysbus.h"
> +
> +/* Maximum number of possible interrupts, determined by the GIC architecture 
> */
> +#define GICV3_MAXIRQ 1020
> +/* First 32 are private to each CPU (SGIs and PPIs). */
> +#define GICV3_INTERNAL 32
> +#define GICV3_NR_SGIS 16
> +#define GICV3_NCPU 64
> +
> +#define MAX_NR_GROUP_PRIO 128
> +
> +typedef struct gicv3_irq_state {
> +    /* The enable bits are only banked for per-cpu interrupts.  */
> +    uint64_t enabled;
> +    uint64_t pending;
> +    uint64_t active;
> +    uint64_t level;
> +    bool edge_trigger; /* true: edge-triggered, false: level-triggered  */
> +} gicv3_irq_state;
> +
> +typedef struct gicv3_sgi_state {
> +    uint64_t pending[GICV3_NCPU];
> +} gicv3_sgi_state;
> +
> +typedef struct GICState {
I feel you should rename that one also
> +    /*< private >*/
> +    SysBusDevice parent_obj;
> +    /*< public >*/
> +
> +    qemu_irq parent_irq[GICV3_NCPU];
> +    uint32_t gicd_ctlr;
> +    bool enabled;
> +    bool cpu_enabled[GICV3_NCPU];
> +
> +    gicv3_irq_state irq_state[GICV3_MAXIRQ];
> +    uint64_t irq_target[GICV3_MAXIRQ];
> +    uint8_t priority1[GICV3_INTERNAL][GICV3_NCPU];
> +    uint8_t priority2[GICV3_MAXIRQ - GICV3_INTERNAL];
> +    uint16_t last_active[GICV3_MAXIRQ][GICV3_NCPU];
> +    /* For each SGI on the target CPU, we store 64 bits
> +     * indicating which source CPUs have made this SGI
> +     * pending on the target CPU. These correspond to
> +     * the bytes in the GIC_SPENDSGIR* registers as
> +     * read by the target CPU.
> +     */
> +    gicv3_sgi_state sgi_state[GICV3_NR_SGIS];
> +
> +    uint16_t priority_mask[GICV3_NCPU];
> +    uint16_t running_irq[GICV3_NCPU];
> +    uint16_t running_priority[GICV3_NCPU];
> +    uint16_t current_pending[GICV3_NCPU];
> +
> +    uint32_t cpu_mask; /* For redistributer */
> +    uint32_t num_cpu;
> +    MemoryRegion iomem_dist; /* Distributor */
> +    MemoryRegion iomem_spi;
What's that iomem_spi?
> +    MemoryRegion iomem_its_cntrl;
> +    MemoryRegion iomem_its;
> +    MemoryRegion iomem_lpi; /* Redistributor */
good to see this naming ;-)
> +    /* This is just so we can have an opaque pointer which identifies
> +     * both this GIC and which CPU interface we should be accessing.
> +     */
The above comment should be removed I guess, was corresponding to
struct GICState *backref[GIC_NCPU]
> +    uint32_t num_irq;
> +    uint32_t revision;
> +    int dev_fd; /* kvm device fd if backed by kvm vgic support */
> +} GICState;
> +
> +#define TYPE_ARM_GICV3_COMMON "arm_gicv3_common"
> +#define ARM_GIC_COMMON(obj) \
> +     OBJECT_CHECK(GICState, (obj), TYPE_ARM_GICV3_COMMON)
> +#define ARM_GIC_COMMON_CLASS(klass) \
> +     OBJECT_CLASS_CHECK(ARMGICCommonClass, (klass), TYPE_ARM_GICV3_COMMON)
> +#define ARM_GIC_COMMON_GET_CLASS(obj) \
> +     OBJECT_GET_CLASS(ARMGICCommonClass, (obj), TYPE_ARM_GICV3_COMMON)
> +
> +typedef struct ARMGICCommonClass {
> +    /*< private >*/
> +    SysBusDeviceClass parent_class;
> +    /*< public >*/
> +
> +    void (*pre_save)(GICState *s);
> +    void (*post_load)(GICState *s);
> +} ARMGICCommonClass;
> +
> +#endif
> 




reply via email to

[Prev in Thread] Current Thread [Next in Thread]