All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH RFC V5 0/9] Implement GIC-500 from GICv3 family for arm64
@ 2015-10-20 17:22 Shlomo Pongratz
  2015-10-20 17:22 ` [Qemu-devel] [PATCH RFC V5 2/9] hw/intc: arm_gicv3_interrupts Shlomo Pongratz
                   ` (9 more replies)
  0 siblings, 10 replies; 41+ messages in thread
From: Shlomo Pongratz @ 2015-10-20 17:22 UTC (permalink / raw)
  To: qemu-devel
  Cc: peter.maydell, eric.auger, Shlomo Pongratz, p.fedin,
	shannon.zhao, ashoks, imammedo

From: Shlomo Pongratz <shlomo.pongratz@huawei.com>

This patch is a first step multicores support for arm64.

This implemntation was tested up to 100 cores.

Things left to do:

Support SPI, ITS and ITS CONTROL, note that this patch porpose is to enable
running multi cores using the "virt" virtual machine and this goal is achived
without that.

Add GICv2 backwards competability. Since there is a GICv2 implementation I
can't see the pusprose for it.

Special thanks to Peter Crostwaite whose patch to th Linux (kernel) i.e.
Implement cpu_relax as yield solved the problem of the boot process getting
stuck for 24 cores and more.

Figure out why virtual machine name changed from virt-v3 to virt-v3-machine

V5->V4
  - Remove the 64 cores limit by using bitmaps.
  - Break the GICv3 code to multiple files e.g. distributer, redistributer, etc,
    and submit each file in a patch of its own.
  - Pass the cores' affinity to the GIC using property vector since the GIC
    can't access the CPU data structure and it is not aware of the number of the
    ARM_CPUS_PER_CLUSTER as defined in "target-arm/cpu.c"
    Rebase to hash ee9dfed242610e from Oct 20.

V3->V4:
  - Rebase the patch series on Pavel Feding's "vGICv3 support" V14 patch series.
  - Replace the usage of vnic/irqchip in the CPU structure to keep the gic
    object for the system instructions with the usage of the routine
    define_arm_cp_regs_with_opaque() as suggested by Peter Maydell in his mail
    from August the 3rd. 

V2->V3:
  - Replace my original 1 & 4 patches with Pavel's patches.
  - Add groups support to complies with new GICv2 addtions
  - Cosmetic changes.

V1->V2:
  - Split the original patch to 4 patches
  - Add SRE API to the GIC code.
  - Add call to gicv3_update to armv8_gicv3_set_priority_mask.
  - Cosmetic changes.
  - Fix number of irq when reading GICD_TYPER.


Shlomo Pongratz (9):
  hw/intc: Implement GIC-500 support files
  hw/intc: arm_gicv3_interrupts
  hw/intc: arm_gicv3_cpu_interface
  hw/intc: arm_gicv3_dist
  hw/intc arm_gicv3_redist
  hw/intc: arm_gicv3_spi_its
  hw/intc: arm_gicv3
  target-arm/cpu64 GICv3 system instructions support
  hw/arm: Add virt-v3 machine that uses GIC-500

 hw/arm/virt.c                      |  87 ++++-
 hw/intc/Makefile.objs              |   6 +
 hw/intc/arm_gicv3.c                | 134 ++++++++
 hw/intc/arm_gicv3_common.c         | 251 +++++++++++++-
 hw/intc/arm_gicv3_cpu_interface.c  | 130 ++++++++
 hw/intc/arm_gicv3_cpu_interface.h  |  21 ++
 hw/intc/arm_gicv3_dist.c           | 655 +++++++++++++++++++++++++++++++++++++
 hw/intc/arm_gicv3_dist.h           |   9 +
 hw/intc/arm_gicv3_interrupts.c     | 295 +++++++++++++++++
 hw/intc/arm_gicv3_interrupts.h     |  11 +
 hw/intc/arm_gicv3_redist.c         | 460 ++++++++++++++++++++++++++
 hw/intc/arm_gicv3_redist.h         |   9 +
 hw/intc/arm_gicv3_spi_its.c        | 359 ++++++++++++++++++++
 hw/intc/arm_gicv3_spi_its.h        |  11 +
 hw/intc/gicv3_internal.h           | 243 ++++++++++++++
 include/hw/arm/fdt.h               |   2 +
 include/hw/arm/virt.h              |   1 +
 include/hw/intc/arm_gicv3.h        |  44 +++
 include/hw/intc/arm_gicv3_common.h |  78 ++++-
 target-arm/cpu-qom.h               |   1 +
 target-arm/cpu.h                   |  12 +
 target-arm/cpu64.c                 | 118 +++++++
 target-arm/machine.c               |   7 +-
 23 files changed, 2929 insertions(+), 15 deletions(-)
 create mode 100644 hw/intc/arm_gicv3.c
 create mode 100644 hw/intc/arm_gicv3_cpu_interface.c
 create mode 100644 hw/intc/arm_gicv3_cpu_interface.h
 create mode 100644 hw/intc/arm_gicv3_dist.c
 create mode 100644 hw/intc/arm_gicv3_dist.h
 create mode 100644 hw/intc/arm_gicv3_interrupts.c
 create mode 100644 hw/intc/arm_gicv3_interrupts.h
 create mode 100644 hw/intc/arm_gicv3_redist.c
 create mode 100644 hw/intc/arm_gicv3_redist.h
 create mode 100644 hw/intc/arm_gicv3_spi_its.c
 create mode 100644 hw/intc/arm_gicv3_spi_its.h
 create mode 100644 hw/intc/gicv3_internal.h
 create mode 100644 include/hw/intc/arm_gicv3.h

-- 
1.9.1

^ permalink raw reply	[flat|nested] 41+ messages in thread

* [Qemu-devel] [PATCH RFC V5 2/9] hw/intc: arm_gicv3_interrupts
  2015-10-20 17:22 [Qemu-devel] [PATCH RFC V5 0/9] Implement GIC-500 from GICv3 family for arm64 Shlomo Pongratz
@ 2015-10-20 17:22 ` Shlomo Pongratz
  2015-10-20 17:22 ` [Qemu-devel] [PATCH RFC V5 3/9] hw/intc: arm_gicv3_cpu_interface Shlomo Pongratz
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 41+ messages in thread
From: Shlomo Pongratz @ 2015-10-20 17:22 UTC (permalink / raw)
  To: qemu-devel
  Cc: peter.maydell, eric.auger, Shlomo Pongratz, p.fedin,
	shannon.zhao, ashoks, imammedo

From: Shlomo Pongratz <shlomo.pongratz@huawei.com>

This patch includes the part of the GIC's code that handles
the interrupts.

Signed-off-by: Shlomo Pongratz <shlomo.pongratz@huawei.com>
---
 hw/intc/Makefile.objs          |   1 +
 hw/intc/arm_gicv3_interrupts.c | 295 +++++++++++++++++++++++++++++++++++++++++
 hw/intc/arm_gicv3_interrupts.h |  11 ++
 hw/intc/gicv3_internal.h       |  19 +++
 4 files changed, 326 insertions(+)
 create mode 100644 hw/intc/arm_gicv3_interrupts.c
 create mode 100644 hw/intc/arm_gicv3_interrupts.h

diff --git a/hw/intc/Makefile.objs b/hw/intc/Makefile.objs
index 004b0c2..e8cdd27 100644
--- a/hw/intc/Makefile.objs
+++ b/hw/intc/Makefile.objs
@@ -13,6 +13,7 @@ common-obj-$(CONFIG_ARM_GIC) += arm_gic_common.o
 common-obj-$(CONFIG_ARM_GIC) += arm_gic.o
 common-obj-$(CONFIG_ARM_GIC) += arm_gicv2m.o
 common-obj-$(CONFIG_ARM_GIC) += arm_gicv3_common.o
+common-obj-$(CONFIG_ARM_GIC) += arm_gicv3_interrupts.o
 common-obj-$(CONFIG_OPENPIC) += openpic.o
 
 obj-$(CONFIG_APIC) += apic.o apic_common.o
diff --git a/hw/intc/arm_gicv3_interrupts.c b/hw/intc/arm_gicv3_interrupts.c
new file mode 100644
index 0000000..da2293e
--- /dev/null
+++ b/hw/intc/arm_gicv3_interrupts.c
@@ -0,0 +1,295 @@
+#include "gicv3_internal.h"
+#include "qom/cpu.h"
+#include "arm_gicv3_interrupts.h"
+
+/* TODO: Many places that call this routine could be optimized.  */
+/* Update interrupt status after enabled or pending bits have been changed.  */
+void gicv3_update(GICv3State *s)
+{
+    int best_irq;
+    int best_prio;
+    int irq;
+    int irq_level, fiq_level;
+    int cpu;
+
+    for (cpu = 0; cpu < NUM_CPU(s); cpu++) {
+        s->current_pending[cpu] = 1023;
+        if (!(s->ctlr & (GICD_CTLR_EN_GRP0 | GICD_CTLR_EN_GRP1_ALL))
+            || !test_bit(cpu, s->cpu_enabled)
+            || !(s->cpu_ctlr[cpu] & (GICC_CTLR_EN_GRP0 | GICC_CTLR_EN_GRP1))) {
+            qemu_irq_lower(s->parent_irq[cpu]);
+            qemu_irq_lower(s->parent_fiq[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, cpu) && gic_test_pending(s, irq, cpu) &&
+                (irq < GICV3_INTERNAL || GIC_TEST_TARGET(irq, cpu))) {
+                if (GIC_GET_PRIORITY(irq, cpu) < best_prio) {
+                    best_prio = GIC_GET_PRIORITY(irq, cpu);
+                    best_irq = irq;
+                }
+            }
+        }
+
+        irq_level = fiq_level = 0;
+
+        if (best_prio < s->priority_mask[cpu]) {
+            s->current_pending[cpu] = best_irq;
+            if (best_prio < s->running_priority[cpu]) {
+                int group = GIC_TEST_GROUP(best_irq, cpu);
+                if (extract32(s->ctlr, group, 1) &&
+                    extract32(s->cpu_ctlr[cpu], group, 1)) {
+                    if (group == 0 && s->cpu_ctlr[cpu] & GICC_CTLR_FIQ_EN) {
+                        DPRINTF("Raised pending FIQ %d (cpu %d)\n",
+                                best_irq, cpu);
+                        fiq_level = 1;
+                    } else {
+                        DPRINTF("Raised pending IRQ %d (cpu %d)\n",
+                                best_irq, cpu);
+                        irq_level = 1;
+                    }
+                }
+            }
+        }
+
+        qemu_set_irq(s->parent_irq[cpu], irq_level);
+        qemu_set_irq(s->parent_fiq[cpu], fiq_level);
+    }
+}
+
+static void gicv3_set_irq_generic(GICv3State *s, int irq, int level,
+                                  int cm, unsigned long *target)
+{
+    if (level) {
+        /* if cm is -1 then the macro will set them all */
+        GIC_SET_LEVEL(irq, cm);
+        DPRINTF("Set %d pending cpu %d\n", irq, cm);
+        if (GIC_TEST_EDGE_TRIGGER(irq)) {
+            if (cm < 0)
+                GIC_SET_PENDING_MASK(irq, target);
+            else
+                GIC_SET_PENDING(irq, cm);
+        }
+    } else {
+        GIC_CLEAR_LEVEL(irq, cm);
+    }
+}
+
+/* Process a change in an external IRQ input.  */
+void gicv3_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
+     *  ...
+     */
+    GICv3State *s = (GICv3State *)opaque;
+    int cm;
+    unsigned long *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 = cpu;
+        target = NULL;
+    }
+
+    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 uint16_t gic_get_current_pending_irq(GICv3State *s, int cpu,
+                                            MemTxAttrs attrs)
+{
+    uint16_t pending_irq = s->current_pending[cpu];
+
+    if (pending_irq < GICV3_MAXIRQ && gic_has_groups(s)) {
+        /* GICv3 section 4.1.4
+         * In systems that support a single security state,
+         * there is no security distinction between Secure and Non -Secure
+         * interrupt groups. That is, "Secure" and "Non-Secure" interrupts are
+         * always accessible.
+         */
+        if (s->security_levels > 1) {
+            int group = GIC_TEST_GROUP(pending_irq, cpu);
+            bool secure = attrs.secure;
+            if (group == 0) {
+                if (!secure) {
+                    fprintf(stderr, "%s::%d\n", __func__, __LINE__);
+                    /* Group0 interrupts hidden from Non-secure access */
+                    return 1023;
+                }
+            } else {
+                /* Note GICv3 5.6.18: AckCtl was removed in GICv3
+                 * Also look at GICv3 5.3.20 for group 1 secure and non secure
+                 */
+                if (secure) {
+                    if (s->ctlr & GICD_CTLR_EN_GRP1NS) {
+                        /* Secure access to non secure group 1 interrupts */
+                        fprintf(stderr, "%s::%d\n", __func__, __LINE__);
+                        return 1022;
+                    }
+                } else {
+                    if (s->ctlr & GICD_CTLR_EN_GRP1S) {
+                        /* Non secure access to secure group 1 interrupts */
+                        fprintf(stderr, "%s::%d\n", __func__, __LINE__);
+                        return 1022;
+                    }
+                }
+            }
+        }
+    }
+    return pending_irq;
+}
+
+static void gic_set_running_irq(GICv3State *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(GICv3State *s, int cpu, MemTxAttrs attrs)
+{
+    int ret, irq, src;
+
+    /* gic_get_current_pending_irq() will return 1022 or 1023 appropriately
+     * for the case where this GIC supports grouping and the pending interrupt
+     * is in the wrong group.
+     */
+    irq = gic_get_current_pending_irq(s, cpu, attrs);
+
+    if (irq >= GICV3_MAXIRQ) {
+        //DPRINTF("ACK, no pending interrupt or it is hidden: %d\n", irq);
+        return irq;
+    }
+
+    if (GIC_GET_PRIORITY(irq, cpu) >= s->running_priority[cpu]) {
+        //DPRINTF("ACK, pending interrupt (%d) has insufficient priority\n", irq);
+        return 1023;
+    }
+
+    s->irq_state[irq].last_active[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[irq].state[cpu].pending != 0);
+        src = find_first_bit(s->sgi[irq].state[cpu].pending, s->num_cpu);
+        if (src < s->num_cpu)
+            clear_bit(src, s->sgi[irq].state[cpu].pending);
+        if (bitmap_empty(s->sgi[irq].state[cpu].pending, s->num_cpu)) {
+            GIC_CLEAR_PENDING(irq, cpu);
+        }
+        /* 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, cpu);
+        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(GICv3State *s, int cpu, int irq, uint8_t val,
+                        MemTxAttrs attrs)
+{
+    DPRINTF("%s cpu(%d) secure(%d)\n", __func__, cpu, attrs.secure);
+    if (s->security_levels == 1 && !attrs.secure) {
+        if (!GIC_TEST_GROUP(irq, cpu)) {
+            return; /* Ignore Non-secure access of Group0 IRQ */
+        }
+        val = 0x80 | (val >> 1); /* Non-secure view */
+    }
+
+    if (irq < GICV3_INTERNAL) {
+        s->priority1[irq].p[cpu] = val;
+    } else {
+        s->priority2[irq - GICV3_INTERNAL] = val;
+    }
+}
+
+void gicv3_complete_irq(GICv3State *s, int cpu, int irq, MemTxAttrs attrs)
+{
+    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) {
+        DPRINTF("No active IRQ ignored cpu(%d) irq(%d)\n", irq, cpu);
+        return; /* No active IRQ.  */
+    }
+
+    if (s->security_levels == 1 && !attrs.secure && !GIC_TEST_GROUP(irq, cpu)) {
+        DPRINTF("Non-secure EOI for Group0 interrupt %d ignored\n", irq);
+        fprintf(stderr, "Non-secure EOI for Group0 interrupt %d ignored cpu(%d)\n", irq, cpu);
+        return;
+    }
+
+    /* Secure EOI with GICC_CTLR.AckCtl == 0 when the IRQ is a Group 1
+     * interrupt is UNPREDICTABLE. We choose to handle it as if AckCtl == 1,
+     * i.e. go ahead and complete the irq anyway.
+     */
+
+    if (irq != s->running_irq[cpu]) {
+        /* Complete an IRQ that is not currently running.  */
+        int tmp = s->running_irq[cpu];
+        while (s->irq_state[tmp].last_active[cpu] != 1023) {
+            if (s->irq_state[tmp].last_active[cpu] == irq) {
+                s->irq_state[tmp].last_active[cpu] = s->irq_state[irq].last_active[cpu];
+                break;
+            }
+            tmp = s->irq_state[tmp].last_active[cpu];
+        }
+    } else {
+        /* Complete the current running IRQ.  */
+        gic_set_running_irq(s, cpu, s->irq_state[s->running_irq[cpu]].last_active[cpu]);
+    }
+}
diff --git a/hw/intc/arm_gicv3_interrupts.h b/hw/intc/arm_gicv3_interrupts.h
new file mode 100644
index 0000000..2f04c88
--- /dev/null
+++ b/hw/intc/arm_gicv3_interrupts.h
@@ -0,0 +1,11 @@
+#ifndef QEMU_ARM_GICV3_INTERRUPTS_H
+#define QEMU_ARM_GICV3_INTERRUPTS_H
+
+uint32_t gicv3_acknowledge_irq(GICv3State *s, int cpu, MemTxAttrs attrs);
+void gicv3_complete_irq(GICv3State *s, int cpu, int irq, MemTxAttrs attrs);
+void gicv3_update(GICv3State *s);
+void gicv3_set_priority(GICv3State *s, int cpu, int irq, uint8_t val,
+                        MemTxAttrs attrs);
+void gicv3_set_irq(void *opaque, int irq, int level);
+
+#endif /* !QEMU_ARM_GIC_INTERRUPTS_H */
diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h
index e0b4a08..362455c 100644
--- a/hw/intc/gicv3_internal.h
+++ b/hw/intc/gicv3_internal.h
@@ -207,4 +207,23 @@ static inline bool gic_test_pending(GICv3State *s, int irq, int cm)
 #define GICC_CTLR_EOIMODE    (1U << 9)
 #define GICC_CTLR_EOIMODE_NS (1U << 10)
 
+#define NUM_CPU(s) ((s)->num_cpu)
+
+/* Return true if this GIC config has interrupt groups, which is
+ * true if we're a GICv3. Keep just
+ */
+static inline bool gic_has_groups(GICv3State *s)
+{
+    return 1;
+}
+
+#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
+
 #endif /* !QEMU_ARM_GIC_INTERNAL_H */
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [Qemu-devel] [PATCH RFC V5 3/9] hw/intc: arm_gicv3_cpu_interface
  2015-10-20 17:22 [Qemu-devel] [PATCH RFC V5 0/9] Implement GIC-500 from GICv3 family for arm64 Shlomo Pongratz
  2015-10-20 17:22 ` [Qemu-devel] [PATCH RFC V5 2/9] hw/intc: arm_gicv3_interrupts Shlomo Pongratz
@ 2015-10-20 17:22 ` Shlomo Pongratz
  2015-10-20 17:22 ` [Qemu-devel] [PATCH RFC V5 4/9] hw/intc: arm_gicv3_dist Shlomo Pongratz
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 41+ messages in thread
From: Shlomo Pongratz @ 2015-10-20 17:22 UTC (permalink / raw)
  To: qemu-devel
  Cc: peter.maydell, eric.auger, Shlomo Pongratz, p.fedin,
	shannon.zhao, ashoks, imammedo

From: Shlomo Pongratz <shlomo.pongratz@huawei.com>

This patch incudes the GIC functionality that is exposed to the CPU
via system instructions. In GICv2 this functionality was exposed via
memory mapped access.

Signed-off-by: Shlomo Pongratz <shlomo.pongratz@huawei.com>
---
 hw/intc/Makefile.objs             |   1 +
 hw/intc/arm_gicv3_cpu_interface.c | 130 ++++++++++++++++++++++++++++++++++++++
 hw/intc/arm_gicv3_cpu_interface.h |  21 ++++++
 hw/intc/gicv3_internal.h          |   6 ++
 4 files changed, 158 insertions(+)
 create mode 100644 hw/intc/arm_gicv3_cpu_interface.c
 create mode 100644 hw/intc/arm_gicv3_cpu_interface.h

diff --git a/hw/intc/Makefile.objs b/hw/intc/Makefile.objs
index e8cdd27..fb6494f 100644
--- a/hw/intc/Makefile.objs
+++ b/hw/intc/Makefile.objs
@@ -14,6 +14,7 @@ common-obj-$(CONFIG_ARM_GIC) += arm_gic.o
 common-obj-$(CONFIG_ARM_GIC) += arm_gicv2m.o
 common-obj-$(CONFIG_ARM_GIC) += arm_gicv3_common.o
 common-obj-$(CONFIG_ARM_GIC) += arm_gicv3_interrupts.o
+common-obj-$(CONFIG_ARM_GIC) += arm_gicv3_cpu_interface.o
 common-obj-$(CONFIG_OPENPIC) += openpic.o
 
 obj-$(CONFIG_APIC) += apic.o apic_common.o
diff --git a/hw/intc/arm_gicv3_cpu_interface.c b/hw/intc/arm_gicv3_cpu_interface.c
new file mode 100644
index 0000000..ba5ee38
--- /dev/null
+++ b/hw/intc/arm_gicv3_cpu_interface.c
@@ -0,0 +1,130 @@
+#include "gicv3_internal.h"
+#include "qom/cpu.h"
+#include "arm_gicv3_cpu_interface.h"
+#include "arm_gicv3_interrupts.h"
+
+/* Cuurently no GICv2 backwards compatibility (no memory mapped regs)
+ * Uses system registers mode.
+ */
+const int gicv3_no_gicv2_bc = 1;
+uint32_t gicv3_sre;
+
+void armv8_gicv3_set_sgi(void *opaque, int cpuindex, uint64_t value)
+{
+    GICv3State *s = (GICv3State *) opaque;
+    int irq, i;
+
+    /* 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++) {
+            set_bit(cpuindex, s->sgi[irq].state[i].pending);
+        }
+        for (i = cpuindex + 1; i < s->num_cpu; i++) {
+            set_bit(cpuindex, s->sgi[irq].state[i].pending);
+        }
+        /* GIC_SET_PENDING(irq, (ALL_CPU_MASK & ~cm)); */
+        bitmap_fill(s->irq_state[irq].pending, s->num_cpu);
+        clear_bit(cpuindex, s->irq_state[irq].pending);
+        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
+         */
+        uint64_t target_affinity;
+        uint64_t target_list;
+        target_affinity  = (value >> (16 - ARM_AFF1_SHIFT)) & ARM_AFF1_MASK;
+        target_affinity |= (value >> (32 - ARM_AFF2_SHIFT)) & ARM_AFF2_MASK;
+        target_affinity |= (value >> (48 - ARM_AFF3_SHIFT)) & ARM_AFF3_MASK;
+        target_list = value & 0xff;
+
+        for (i = 0; i < s->num_cpu; i++) {
+            uint64_t cpu_aff0   = s->mp_affinity[i] & ARM_AFF0_MASK;
+            uint64_t cpu_aff123 = s->mp_affinity[i] & ~ARM_AFF0_MASK;
+            if (cpu_aff123 == target_affinity &&
+                ((1 << cpu_aff0) & target_list)) {
+                set_bit(cpuindex, s->sgi[irq].state[i].pending);
+                GIC_SET_PENDING(irq, i);
+            }
+        }
+    }
+    gicv3_update(s);
+}
+
+uint64_t armv8_gicv3_acknowledge_irq(void *opaque, int cpuindex,
+                                     MemTxAttrs attrs)
+{
+    GICv3State *s = (GICv3State *) opaque;
+    return gicv3_acknowledge_irq(s, cpuindex, attrs);
+}
+
+void armv8_gicv3_complete_irq(void *opaque, int cpuindex, int irq,
+                              MemTxAttrs attrs)
+{
+    GICv3State *s = (GICv3State *) opaque;
+    irq &= 0xffffff;
+    gicv3_complete_irq(s, cpuindex, irq, attrs);
+}
+
+uint64_t armv8_gicv3_get_priority_mask(void *opaque, int cpuindex)
+{
+    GICv3State *s = (GICv3State *) opaque;
+    return s->priority_mask[cpuindex];
+}
+
+void armv8_gicv3_set_priority_mask(void *opaque, int cpuindex, uint32_t mask)
+{
+    GICv3State *s = (GICv3State *) opaque;
+    s->priority_mask[cpuindex] = mask & 0xff;
+    DPRINTF("%s cpu(%d) priority mask 0x%x\n",
+            __func__, cpuindex, s->priority_mask[cpuindex]);
+    gicv3_update(s);
+}
+
+uint64_t armv8_gicv3_get_sre(void *opaque)
+{
+    /* Uses only system registers, no memory mapped access GICv2 mode */
+    return gicv3_sre;
+}
+
+void armv8_gicv3_set_sre(void *opaque, uint64_t sre)
+{
+    if (!(sre & 1) && gicv3_no_gicv2_bc) {
+        /* Cuurently no GICv2 backwards compatibility (no memory mapped regs)
+         * Uses system registers mode
+         */
+        DPRINTF("Try to use memory mapped interface sre(0x%lx)\n", sre);
+        assert(0);
+    }
+    gicv3_sre = sre;
+}
+
+uint64_t armv8_gicv3_get_igrpen1(void *opaque, int cpuindex)
+{
+    GICv3State *s = (GICv3State *) opaque;
+    return !!(s->cpu_ctlr[cpuindex] & GICC_CTLR_EN_GRP1);
+}
+
+void armv8_gicv3_set_igrpen1(void *opaque, int cpuindex, uint64_t igrpen1)
+{
+    GICv3State *s = (GICv3State *) opaque;
+    if (igrpen1)
+        s->cpu_ctlr[cpuindex] |= GICC_CTLR_EN_GRP1;
+    else
+        s->cpu_ctlr[cpuindex] &= ~GICC_CTLR_EN_GRP1;
+
+    DPRINTF("CPU Interface %d: Group0 Interrupts %sabled, "
+            "Group1 Interrupts %sabled\n", cpuindex,
+            (s->cpu_ctlr[cpuindex] & GICC_CTLR_EN_GRP0) ? "En" : "Dis",
+            (s->cpu_ctlr[cpuindex] & GICC_CTLR_EN_GRP1) ? "En" : "Dis");
+}
diff --git a/hw/intc/arm_gicv3_cpu_interface.h b/hw/intc/arm_gicv3_cpu_interface.h
new file mode 100644
index 0000000..fb03c2c
--- /dev/null
+++ b/hw/intc/arm_gicv3_cpu_interface.h
@@ -0,0 +1,21 @@
+#ifndef QEMU_ARM_GICV3_CPU_INTERFACE_H
+#define QEMU_ARM_GICV3_CPU_INTERFACE_H
+
+
+/* These routines are called from cpu64.c and are defined in target-arm/cpu.h
+ * like armv7m_nvic_XXX routines.
+ * I couldn't find how to include it without compilation errors
+ */
+void armv8_gicv3_set_sgi(void *opaque, int cpuindex, uint64_t value);
+uint64_t armv8_gicv3_acknowledge_irq(void *opaque, int cpuindex,
+                                     MemTxAttrs attrs);
+void armv8_gicv3_complete_irq(void *opaque, int cpuindex, int irq,
+                                     MemTxAttrs attrs);
+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);
+uint64_t armv8_gicv3_get_igrpen1(void *opaque, int cpuindex);
+void armv8_gicv3_set_igrpen1(void *opaque, int cpuindex, uint64_t igrpen1);
+
+#endif /* !QEMU_ARM_GIC_CPU_INTERFACE_H */
diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h
index 362455c..14915e0 100644
--- a/hw/intc/gicv3_internal.h
+++ b/hw/intc/gicv3_internal.h
@@ -217,6 +217,12 @@ static inline bool gic_has_groups(GICv3State *s)
     return 1;
 }
 
+/* Cuurently no GICv2 backwards compatibility (no memory mapped regs)
+ * Uses system registers mode.
+ */
+extern const int gicv3_no_gicv2_bc;
+extern uint32_t gicv3_sre;
+
 #undef DEBUG_GICV3
 
 #ifdef DEBUG_GICV3
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [Qemu-devel] [PATCH RFC V5 4/9] hw/intc: arm_gicv3_dist
  2015-10-20 17:22 [Qemu-devel] [PATCH RFC V5 0/9] Implement GIC-500 from GICv3 family for arm64 Shlomo Pongratz
  2015-10-20 17:22 ` [Qemu-devel] [PATCH RFC V5 2/9] hw/intc: arm_gicv3_interrupts Shlomo Pongratz
  2015-10-20 17:22 ` [Qemu-devel] [PATCH RFC V5 3/9] hw/intc: arm_gicv3_cpu_interface Shlomo Pongratz
@ 2015-10-20 17:22 ` Shlomo Pongratz
  2015-10-20 17:22 ` [Qemu-devel] [PATCH RFC V5 5/9] hw/intc arm_gicv3_redist Shlomo Pongratz
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 41+ messages in thread
From: Shlomo Pongratz @ 2015-10-20 17:22 UTC (permalink / raw)
  To: qemu-devel
  Cc: peter.maydell, eric.auger, Shlomo Pongratz, p.fedin,
	shannon.zhao, ashoks, imammedo

From: Shlomo Pongratz <shlomo.pongratz@huawei.com>

Signed-off-by: Shlomo Pongratz <shlomo.pongratz@huawei.com>
---
 hw/intc/Makefile.objs    |   1 +
 hw/intc/arm_gicv3_dist.c | 655 +++++++++++++++++++++++++++++++++++++++++++++++
 hw/intc/arm_gicv3_dist.h |   9 +
 hw/intc/gicv3_internal.h |   8 +
 4 files changed, 673 insertions(+)
 create mode 100644 hw/intc/arm_gicv3_dist.c
 create mode 100644 hw/intc/arm_gicv3_dist.h

diff --git a/hw/intc/Makefile.objs b/hw/intc/Makefile.objs
index fb6494f..cdfb877 100644
--- a/hw/intc/Makefile.objs
+++ b/hw/intc/Makefile.objs
@@ -15,6 +15,7 @@ common-obj-$(CONFIG_ARM_GIC) += arm_gicv2m.o
 common-obj-$(CONFIG_ARM_GIC) += arm_gicv3_common.o
 common-obj-$(CONFIG_ARM_GIC) += arm_gicv3_interrupts.o
 common-obj-$(CONFIG_ARM_GIC) += arm_gicv3_cpu_interface.o
+common-obj-$(CONFIG_ARM_GIC) += arm_gicv3_dist.o
 common-obj-$(CONFIG_OPENPIC) += openpic.o
 
 obj-$(CONFIG_APIC) += apic.o apic_common.o
diff --git a/hw/intc/arm_gicv3_dist.c b/hw/intc/arm_gicv3_dist.c
new file mode 100644
index 0000000..973a1b3
--- /dev/null
+++ b/hw/intc/arm_gicv3_dist.c
@@ -0,0 +1,655 @@
+#include "gicv3_internal.h"
+#include "qom/cpu.h"
+#include "arm_gicv3_dist.h"
+#include "arm_gicv3_interrupts.h"
+
+static const uint8_t gic_dist_ids[] = {
+    0x44, 0x00, 0x00, 0x00, 0x092, 0xB4, 0x3B, 0x00, 0x0D, 0xF0, 0x05, 0xB1
+};
+
+static uint64_t gic_dist_readb(void *opaque, hwaddr offset, MemTxAttrs attrs)
+{
+    GICv3State *s = (GICv3State *)opaque;
+    uint64_t res;
+    int irq;
+    int i;
+    int cpu;
+    int cm;
+
+    cpu = gic_get_current_cpu(s);
+    if (offset < 0x100) {
+        if (offset == 0) {/* GICD_CTLR */
+            /* GICv3 5.3.20 without GICV2 backwards compatibility  */
+            if (s->security_levels > 1) {
+                /* Support TWO security states */
+                if (attrs.secure) {
+                    return s->ctlr;
+                } else {
+                    /* For non secure access bits [30:5] are reserved and ARE_NS
+                     * is RAO/WI note that ARE_NS in this case is bit [4] same
+                     * as ARE_S in the prvious secure case. Now since it is
+                     * RAO/WI than bit [0] EnableGrp1 is reserved and we can
+                     * only use Bit [1] EnableGrp1A to enable Non-secure Group1
+                     * interrupts
+                     */
+                    return s->ctlr & (GICD_CTLR_ARE_NS |
+                                      GICD_CTLR_EN_GRP1NS |
+                                      GICD_CTLR_RWP);
+                }
+            } else {
+                /* Support ONE security state without ARE is RAO/WI*/
+                return s->ctlr & (GICD_CTLR_ARE |
+                                  GICD_CTLR_EN_GRP0 | GICD_CTLR_EN_GRP1NS |
+                                  GICD_CTLR_RWP);
+            }
+        }
+        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 num_irqs as given from virt machine via "num-irq"
+             * 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)
+            return 0x43B; /* GIC_IIDR */
+        if (offset >= 0x80) {
+            /* Interrupt Group Registers: these RAZ/WI if this is an NS
+             * access to a GIC with the security extensions, or if the GIC
+             * doesn't have groups at all.
+             */
+            res = 0;
+            if (!attrs.secure) {
+                /* GIC-500 comment 'f' */
+                goto bad_reg;
+            }
+            /* Every byte offset holds 8 group status bits */
+            irq = (offset - 0x080) * 8 + GICV3_BASE_IRQ;
+            if (irq >= s->num_irq) {
+                goto bad_reg;
+            }
+            for (i = 0; i < 8; i++) {
+                if (GIC_TEST_GROUP(irq + i, cpu)) {
+                    res |= (1 << i);
+                }
+            }
+            return res;
+        }
+        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, cpu)) {
+                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;
+        /* Comment 'o' GICv2 backwards competability support is not set...*/
+        if (irq < GICV3_INTERNAL && gicv3_no_gicv2_bc)
+            goto bad_reg;
+        res = 0;
+        cm = (irq < GICV3_INTERNAL) ?  cpu : ALL_CPU_MASK;
+        for (i = 0; i < 8; i++) {
+            if (gic_test_pending(s, irq + i, cm)) {
+                res |= (1 << i);
+            }
+        }
+    } else if (offset < 0x400) {
+        /* Interrupt Set/Clear Active.  */
+        if (offset < 0x380)
+            irq = (offset - 0x300) * 8 + GICV3_BASE_IRQ;
+        else
+            irq = (offset - 0x380) * 8 + GICV3_BASE_IRQ;
+        if (irq >= s->num_irq)
+            goto bad_reg;
+        /* Comment 'o' GICv2 backwards competability support is not set...*/
+        if (irq < GICV3_INTERNAL && gicv3_no_gicv2_bc)
+            goto bad_reg;
+        res = 0;
+        cm = (irq < GICV3_INTERNAL) ?  cpu : ALL_CPU_MASK;
+        for (i = 0; i < 8; i++) {
+            if (GIC_TEST_ACTIVE(irq + i, cm)) {
+                res |= (1 << i);
+            }
+        }
+    } else if (offset < 0x800) {
+        /* Interrupt Priority.  */
+        irq = (offset - 0x400) + GICV3_BASE_IRQ;
+        if (irq >= s->num_irq)
+            goto bad_reg;
+        /* Comment 'p' GICv2 backwards competability support is not set...*/
+        if (irq < GICV3_INTERNAL * 8 && gicv3_no_gicv2_bc)
+            goto bad_reg;
+        res = GIC_GET_PRIORITY(irq, cpu);
+    } else if (offset < 0xc00) {
+        /* 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;
+            }
+            /* Comment 'p' GICv2 backwards competability support is not set...*/
+            if (irq < GICV3_INTERNAL * 8 && gicv3_no_gicv2_bc)
+                goto bad_reg;
+            if (irq >= 29 && irq <= 31) {
+                res = 1 << cpu;
+            } else {
+                /* Value is a small bitmap can do it directly */
+                res = *GIC_TARGET(irq);
+            }
+        }
+    } else if (offset < 0xd00) {
+        /* Interrupt Configuration.  */
+        irq = (offset - 0xc00) * 4 + GICV3_BASE_IRQ;
+        if (irq >= s->num_irq)
+            goto bad_reg;
+        /* Comment 'l' GICv2 backwards competability support is not set...*/
+        if (irq < GICV3_INTERNAL && gicv3_no_gicv2_bc)
+            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) {
+        goto bad_reg;
+    } else if (offset < 0xf30) {
+        /* Comments 'e' & 't' GICv2 backwards competability support is not set...*/
+        if (gicv3_no_gicv2_bc)
+            goto bad_reg;
+        /* These are 32 bit registers, should not be used with 128 cores. */
+        if (offset < 0xf20) {
+            /* GICD_CPENDSGIRn */
+            irq = (offset - 0xf10);
+        } else {
+            irq = (offset - 0xf20);
+            /* GICD_SPENDSGIRn */
+        }
+        /* Small bitmap */
+        res = *s->sgi[irq].state[cpu].pending;
+    } else if (offset < 0xffd0) {
+        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_readll(void *opaque, hwaddr offset,
+                                MemTxAttrs attrs)
+{
+    GICv3State *s = (GICv3State *)opaque;
+    uint64_t value;
+
+    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;
+        cpu = find_first_bit(s->irq_state[irq].target, s->num_cpu);
+        Aff1 = cpu / 8;
+        Aff0 = cpu % 8;
+        value = (Aff1 << 8) | Aff0;
+        return value;
+    }
+
+    value = gic_dist_readb(opaque, offset, attrs);
+    value |= gic_dist_readb(opaque, offset + 1, attrs) << 8;
+    value |= gic_dist_readb(opaque, offset + 2, attrs) << 16;
+    value |= gic_dist_readb(opaque, offset + 3, attrs) << 24;
+    value |= gic_dist_readb(opaque, offset + 4, attrs) << 32;
+    value |= gic_dist_readb(opaque, offset + 5, attrs) << 40;
+    value |= gic_dist_readb(opaque, offset + 6, attrs) << 48;
+    value |= gic_dist_readb(opaque, offset + 7, attrs) << 56;
+
+    return value;
+}
+
+MemTxResult gic_dist_read(void *opaque, hwaddr offset, uint64_t *data,
+                          unsigned size, MemTxAttrs attrs)
+{
+    switch (size) {
+    case 1:
+        *data = gic_dist_readb(opaque, offset, attrs);
+        return MEMTX_OK;
+    case 2:
+        *data = gic_dist_readb(opaque, offset, attrs);
+        *data |= gic_dist_readb(opaque, offset + 1, attrs) << 8;
+        return MEMTX_OK;
+    case 4:
+        *data = gic_dist_readb(opaque, offset, attrs);
+        *data |= gic_dist_readb(opaque, offset + 1, attrs) << 8;
+        *data |= gic_dist_readb(opaque, offset + 2, attrs) << 16;
+        *data |= gic_dist_readb(opaque, offset + 3, attrs) << 24;
+        return MEMTX_OK;
+    case 8:
+        *data = gic_dist_readll(opaque, offset, attrs);
+        return MEMTX_OK;
+    default:
+        return MEMTX_ERROR;
+    }
+}
+
+static void gic_dist_writeb(void *opaque, hwaddr offset,
+                            uint64_t value, MemTxAttrs attrs)
+{
+    GICv3State *s = (GICv3State *)opaque;
+    int irq;
+    int i;
+    int cpu;
+
+    cpu = gic_get_current_cpu(s);
+
+    if (offset < 0x100) {
+        if (offset < 0x80) {
+            /* Error! should be handeld in writel. */
+            DPRINTF("Distributor: error should be handled in dist_writel \n");
+            goto bad_reg;
+        } else if (offset >= 0x80) {
+            DPRINTF("Distributor: GICD_IGROUPRn\n");
+            /* Interrupt Group Registers: RAZ/WI for NS access to secure
+             * GIC, or for GICs without groups.
+             */
+            if (!attrs.secure) {
+                /* GIC-500 comment 'f' */
+                goto bad_reg;
+            }
+            /* Every byte offset holds 8 group status bits */
+            irq = (offset - 0x80) * 8 + GICV3_BASE_IRQ;
+            if (irq >= s->num_irq) {
+                goto bad_reg;
+            }
+            for (i = 0; i < 8; i++) {
+                /* Group bits are banked for private interrupts */
+                int cm = (irq < GICV3_INTERNAL) ? cpu : ALL_CPU_MASK;
+                if (value & (1 << i)) {
+                    /* Group1 (Non-secure) */
+                    GIC_SET_GROUP(irq + i, cm);
+                } else {
+                    /* Group0 (Secure) */
+                    GIC_CLEAR_GROUP(irq + i, cm);
+                }
+            }
+        } 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) {
+            DPRINTF("ISENABLERn SGI should be only in redistributer %d\n", irq);
+            /* Ignored according to comment 'g' in GIC-500 document.
+             * The comment doen't say that the whole register is reservd
+             */
+            return;
+        }
+
+        for (i = 0; i < 8; i++) {
+            if (value & (1 << i)) {
+                int cm = (irq < GICV3_INTERNAL) ? 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 (irq < GICV3_INTERNAL) {
+                    if (GIC_TEST_LEVEL(irq + i, cpu)
+                            && !GIC_TEST_EDGE_TRIGGER(irq + i)) {
+                        DPRINTF("Set %d pending cpu %d\n", irq + i, cpu);
+                        GIC_SET_PENDING(irq + i, cpu);
+                    }
+                } else {
+                    unsigned long *mask = GIC_TARGET(irq + i);
+                    if (GIC_TEST_LEVEL_MASK(irq + i, mask)
+                            && !GIC_TEST_EDGE_TRIGGER(irq + i)) {
+                        DPRINTF("Set %d pending target\n", irq + i);
+                        GIC_SET_PENDING_MASK(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) {
+            DPRINTF("ICENABLERn SGI should be only in redistributer %d\n", irq);
+            /* Ignored according to comment 'g' in GIC-500 document.
+             * The comment doen't say that the whole register is reservd
+             */
+            return;
+        }
+
+        for (i = 0; i < 8; i++) {
+            if (value & (1 << i)) {
+                int cm = (irq < GICV3_INTERNAL) ? 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;
+        /* Comment 'o' GICv2 backwards competability support is not set...*/
+        if (irq < GICV3_INTERNAL && gicv3_no_gicv2_bc)
+            goto bad_reg;
+
+        for (i = 0; i < 8; i++) {
+            if (value & (1 << i)) {
+                GIC_SET_PENDING_MASK(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;
+        /* Comment 'o' GICv2 backwards competability support is not set...*/
+        if (irq < GICV3_INTERNAL && gicv3_no_gicv2_bc)
+            goto bad_reg;
+
+        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.  */
+        goto bad_reg;
+    } else if (offset < 0x800) {
+        /* Interrupt Priority.  */
+        irq = (offset - 0x400) + GICV3_BASE_IRQ;
+        if (irq >= s->num_irq)
+            goto bad_reg;
+        /* Comment 'p' GICv2 backwards competability support is not set...*/
+        if (irq < GICV3_INTERNAL * 8 && gicv3_no_gicv2_bc)
+            goto bad_reg;
+        gicv3_set_priority(s, cpu, irq, value, attrs);
+    } else if (offset < 0xc00) {
+        /* Interrupt CPU Target. RAZ/WI on uni-processor GICs */
+        if (s->num_cpu != 1) {
+            irq = (offset - 0x800) + GICV3_BASE_IRQ;
+            if (irq >= s->num_irq) {
+                goto bad_reg;
+            }
+            /* Comment 'p' GICv2 backwards competability support is not set...*/
+            if (irq < GICV3_INTERNAL * 8 && gicv3_no_gicv2_bc)
+                goto bad_reg;
+            if (irq < 29) {
+                value = 0;
+            } else if (irq < GICV3_INTERNAL) {
+                value = ALL_CPU_MASK_COMPAT;
+            }
+            /* Small bitmap */
+            *s->irq_state[irq].target = value & ALL_CPU_MASK_COMPAT;
+        }
+    } else if (offset < 0xd00) {
+        /* Interrupt Configuration.  */
+        irq = (offset - 0xc00) * 4 + GICV3_BASE_IRQ;
+        if (irq >= s->num_irq)
+            goto bad_reg;
+        /* Comment 'l' GICv2 backwards competability support is not set...*/
+        if (irq < GICV3_INTERNAL && gicv3_no_gicv2_bc)
+            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);
+            }
+        }
+    } else if (offset < 0xf30) {
+        /* Comments 'e' & 't' GICv2 backwards competability support is not set */
+        if (gicv3_no_gicv2_bc)
+            goto bad_reg;
+        /* These are 32 bit registers, should not be used with 128 cores. */
+        if (offset < 0xf20) {
+            /* GICD_CPENDSGIRn */
+            irq = (offset - 0xf10);
+            DPRINTF("GICD_CPENDSGIRn irq(%d) %lu\n", irq, value);
+            /* Value is a small bitmap can do it directly */
+            *s->sgi[irq].state[cpu].pending &= ~value;
+            if (*s->sgi[irq].state[cpu].pending == 0) {
+                GIC_CLEAR_PENDING(irq, cpu);
+            }
+        } else {
+            irq = (offset - 0xf20);
+            /* GICD_SPENDSGIRn */
+            DPRINTF("GICD_SPENDSGIRn irq(%d) %lu\n", irq, value);
+            GIC_SET_PENDING(irq, cpu);
+            /* Value is a small bitmap can do it directly */
+            *s->sgi[irq].state[cpu].pending |= value;
+        }
+    }
+    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, MemTxAttrs attrs)
+{
+    gic_dist_writeb(opaque, offset, value & 0xff, attrs);
+    gic_dist_writeb(opaque, offset + 1, value >> 8, attrs);
+}
+
+static void gic_dist_writel(void *opaque, hwaddr offset,
+                            uint64_t value, MemTxAttrs attrs)
+{
+    if (offset < 0x80) {
+        if (offset == 0) {
+            GICv3State *s = (GICv3State *)opaque;
+            uint32_t ctlr;
+            /* One day we may have MT QEMU */
+            s->ctlr |= GICD_CTLR_RWP;
+            /* GICv3 5.3.20 */
+            if (s->security_levels > 1) {
+                /* support TWO security states */
+                if (attrs.secure) {
+                    /* for secure access DS is RAZ/WI ARE_NS is RAO/WI and
+                     * ARE_S is RAO/WI
+                     * for non secure access bits [30:5] are reserved and ARE_NS is
+                     * RAO/WI
+                     *
+                     * Can modify bits[2:0] Groups 0 & 1 NS & 1 S
+                     */
+                    ctlr = s->ctlr & ~(GICD_CTLR_EN_GRP0 | GICD_CTLR_EN_GRP1_ALL);
+                    value &= (GICD_CTLR_EN_GRP0 | GICD_CTLR_EN_GRP1_ALL);
+                    value |= GICD_CTLR_ARE_S | GICD_CTLR_ARE_NS;
+                } else {
+                    /* For non secure access bits [30:5] are reserved and ARE_NS is
+                     * RAO/WI note that ARE_NS in this case is bit [4] same as
+                     * ARE_S in the prvious secure case. Now since it is RAO/WI
+                     * thane bit [0] EnableGrp1 is reserved and we can only use
+                     * Bit [1] EnableGrp1A to enable Non-secure Group1 interrupts
+                     */
+                    ctlr = s->ctlr & ~GICD_CTLR_EN_GRP1NS;
+                    value &= GICD_CTLR_EN_GRP1NS;
+                    value |= GICD_CTLR_ARE_NS;
+                }
+            } else {
+                /* support ONE security state */
+                /* if GICv2 backwards cimpatibility is not implemented than ARE
+                 * is RAO/WI
+                 */
+                ctlr = s->ctlr & ~(GICD_CTLR_EN_GRP0 | GICD_CTLR_EN_GRP1NS);
+                value &= (GICD_CTLR_EN_GRP0 | GICD_CTLR_EN_GRP1NS);
+                value |= GICD_CTLR_ARE;
+            }
+            s->ctlr = (ctlr | value) & ~GICD_CTLR_RWP;
+            DPRINTF("Distributor: Group0 %sabled; Group 1S %sabled Group 1NS %sabled\n",
+                    s->ctlr & GICD_CTLR_EN_GRP0 ? "En" : "Dis",
+                    s->ctlr & GICD_CTLR_EN_GRP1S ? "En" : "Dis",
+                    s->ctlr & GICD_CTLR_EN_GRP1NS ? "En" : "Dis");
+            return;
+        }
+        if (offset < 0x40) {
+            /* RO or reserved < 0x40 */
+            return;
+        }
+        if (offset < 0x80) {
+            /* SPI not supported < 0x80 */
+            return;
+        }
+        return;
+    }
+    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 document)
+         * Or table 6 on GICv3 documment
+         */
+        int cpu;
+        int irq;
+        uint32_t mask, cpu_mask;
+        int target_cpu;
+        GICv3State *s = (GICv3State *)opaque;
+
+        if (gicv3_no_gicv2_bc) {
+            DPRINTF("GICv2 backwards computability is not supported\n");
+            return;
+        }
+        /* GICv2 competabiltu code only 8 CPU */
+        cpu = gic_get_current_cpu(s);
+        irq = value & 0x3ff;
+        switch ((value >> 24) & 3) {
+        case 0:
+            mask = (value >> 16) & ALL_CPU_MASK_COMPAT;
+            break;
+        case 1:
+            /* All cpus excpet this one (up to 7) */
+            mask = ALL_CPU_MASK_COMPAT ^ (1ll << cpu);
+            break;
+        case 2:
+            mask = 1ll << cpu;
+            break;
+        default:
+            DPRINTF("Bad Soft Int target filter\n");
+            /* All CPUs */
+            mask = 0xff;
+            break;
+        }
+        cpu_mask = (1ll << cpu);
+        DPRINTF("irq(%d) mask(0x%x)\n", irq, mask);
+        target_cpu = ctz32(mask);
+        while (target_cpu < s->num_cpu) {
+            GIC_SET_PENDING(irq, target_cpu);
+            /* Small mask, can do direct assignment */
+            *s->sgi[irq].state[target_cpu].pending |= cpu_mask;
+            mask &= ~(1ll << target_cpu);
+            target_cpu = ctz32(mask);
+        }
+        gicv3_update(s);
+        return;
+    }
+    gic_dist_writew(opaque, offset, value & 0xffff, attrs);
+    gic_dist_writew(opaque, offset + 2, value >> 16, attrs);
+}
+
+static void gic_dist_writell(void *opaque, hwaddr offset,
+                            uint64_t value, MemTxAttrs attrs)
+{
+    GICv3State *s = (GICv3State *)opaque;
+
+    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;
+        GIC_SET_TARGET(irq, cpu);
+        gicv3_update(s);
+        DPRINTF("irq(%d) cpu(%d)\n", irq, cpu);
+        return;
+    }
+
+    gic_dist_writel(opaque, offset, value & 0xffffffff, attrs);
+    gic_dist_writel(opaque, offset + 4, value >> 32, attrs);
+}
+
+MemTxResult gic_dist_write(void *opaque, hwaddr addr, uint64_t data,
+                           unsigned size, MemTxAttrs attrs)
+{
+    DPRINTF("offset %p data %p secure(%d)\n", (void *) addr, (void *) data, attrs.secure);
+    switch (size) {
+    case 1:
+        gic_dist_writeb(opaque, addr, data, attrs);
+        return MEMTX_OK;
+    case 2:
+        gic_dist_writew(opaque, addr, data, attrs);
+        return MEMTX_OK;
+    case 4:
+        gic_dist_writel(opaque, addr, data, attrs);
+        return MEMTX_OK;
+    case 8:
+        gic_dist_writell(opaque, addr, data, attrs);
+        return MEMTX_OK;
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: size %u\n", __func__, size);
+        return MEMTX_ERROR;
+    }
+}
diff --git a/hw/intc/arm_gicv3_dist.h b/hw/intc/arm_gicv3_dist.h
new file mode 100644
index 0000000..7f38fa1
--- /dev/null
+++ b/hw/intc/arm_gicv3_dist.h
@@ -0,0 +1,9 @@
+#ifndef QEMU_ARM_GICV3_DIST_H
+#define QEMU_ARM_GICV3_DIST_H
+
+MemTxResult gic_dist_read(void *opaque, hwaddr offset, uint64_t *data,
+                          unsigned size, MemTxAttrs attrs);
+MemTxResult gic_dist_write(void *opaque, hwaddr addr, uint64_t data,
+                           unsigned size, MemTxAttrs attrs);
+
+#endif /* !QEMU_ARM_GIC_DIST_H */
diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h
index 14915e0..c438289 100644
--- a/hw/intc/gicv3_internal.h
+++ b/hw/intc/gicv3_internal.h
@@ -209,6 +209,14 @@ static inline bool gic_test_pending(GICv3State *s, int irq, int cm)
 
 #define NUM_CPU(s) ((s)->num_cpu)
 
+static inline int gic_get_current_cpu(GICv3State *s)
+{
+    if (s->num_cpu > 1) {
+        return current_cpu->cpu_index;
+    }
+    return 0;
+}
+
 /* Return true if this GIC config has interrupt groups, which is
  * true if we're a GICv3. Keep just
  */
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [Qemu-devel] [PATCH RFC V5 5/9] hw/intc arm_gicv3_redist
  2015-10-20 17:22 [Qemu-devel] [PATCH RFC V5 0/9] Implement GIC-500 from GICv3 family for arm64 Shlomo Pongratz
                   ` (2 preceding siblings ...)
  2015-10-20 17:22 ` [Qemu-devel] [PATCH RFC V5 4/9] hw/intc: arm_gicv3_dist Shlomo Pongratz
@ 2015-10-20 17:22 ` Shlomo Pongratz
  2015-10-20 17:22 ` [Qemu-devel] [PATCH RFC V5 6/9] hw/intc: arm_gicv3_spi_its Shlomo Pongratz
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 41+ messages in thread
From: Shlomo Pongratz @ 2015-10-20 17:22 UTC (permalink / raw)
  To: qemu-devel
  Cc: peter.maydell, eric.auger, Shlomo Pongratz, p.fedin,
	shannon.zhao, ashoks, imammedo

From: Shlomo Pongratz <shlomo.pongratz@huawei.com>

This patch includes the code that implements the functionality of
the redisributer, which is the main enhancment of GICv3 over GICv3

Signed-off-by: Shlomo Pongratz <shlomo.pongratz@huawei.com>
---
 hw/intc/Makefile.objs      |   1 +
 hw/intc/arm_gicv3_redist.c | 460 +++++++++++++++++++++++++++++++++++++++++++++
 hw/intc/arm_gicv3_redist.h |   9 +
 3 files changed, 470 insertions(+)
 create mode 100644 hw/intc/arm_gicv3_redist.c
 create mode 100644 hw/intc/arm_gicv3_redist.h

diff --git a/hw/intc/Makefile.objs b/hw/intc/Makefile.objs
index cdfb877..5e56acc 100644
--- a/hw/intc/Makefile.objs
+++ b/hw/intc/Makefile.objs
@@ -16,6 +16,7 @@ common-obj-$(CONFIG_ARM_GIC) += arm_gicv3_common.o
 common-obj-$(CONFIG_ARM_GIC) += arm_gicv3_interrupts.o
 common-obj-$(CONFIG_ARM_GIC) += arm_gicv3_cpu_interface.o
 common-obj-$(CONFIG_ARM_GIC) += arm_gicv3_dist.o
+common-obj-$(CONFIG_ARM_GIC) += arm_gicv3_redist.o
 common-obj-$(CONFIG_OPENPIC) += openpic.o
 
 obj-$(CONFIG_APIC) += apic.o apic_common.o
diff --git a/hw/intc/arm_gicv3_redist.c b/hw/intc/arm_gicv3_redist.c
new file mode 100644
index 0000000..cbac184
--- /dev/null
+++ b/hw/intc/arm_gicv3_redist.c
@@ -0,0 +1,460 @@
+#include "gicv3_internal.h"
+#include "qom/cpu.h"
+#include "arm_gicv3_redist.h"
+#include "arm_gicv3_interrupts.h"
+
+static const uint8_t gic_redist_ids[] = {
+    0x44, 0x00, 0x00, 0x00, 0x093, 0xB4, 0x3B, 0x00, 0x0D, 0xF0, 0x05, 0xB1
+};
+
+static uint64_t gic_redist_readb(void *opaque, hwaddr offset, MemTxAttrs attrs)
+{
+    GICv3State *s = (GICv3State *)opaque;
+    uint64_t res = 0;
+    uint64_t sgi_ppi, core, off;
+    uint64_t irq, i;
+
+    /* GIC-500 Table 3-2 page 3-4 (Rev r0p0)
+     * [          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;
+
+    if (sgi_ppi) {
+        /* SGIs, PPIs */
+        /* Interrupt Set/Clear Enable.  */
+        if (offset < 0x100) {
+            if (offset >= 0x80) {
+                /* Interrupt Group Registers: these RAZ/WI if this is an NS
+                 * access to a GIC with the security extensions, or if the GIC
+                 * doesn't have groups at all.
+                 */
+                res = 0;
+                if (!attrs.secure) {
+                    /* GIC-500 comment 'a' */
+                    goto bad_reg;
+                }
+                /* Every byte offset holds 8 group status bits */
+                irq = (offset - 0x080) * 8 + GICV3_BASE_IRQ;
+                if (irq >= s->num_irq) {
+                    goto bad_reg;
+                }
+                if (irq >= GICV3_INTERNAL) {
+                    DPRINTF("non Internal should be only in dist %lu\n", irq);
+                    goto bad_reg;
+                }
+                for (i = 0; i < 8; i++) {
+                    if (GIC_TEST_GROUP(irq + i, core)) {
+                        res |= (1 << i);
+                    }
+                }
+                return res;
+            }
+        } 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;
+            if (irq >= GICV3_INTERNAL) {
+                DPRINTF("non Internal should be only in dist %lu\n", irq);
+                goto bad_reg;
+            }
+            res = 0;
+            for (i = 0; i < 8; i++) {
+                if (GIC_TEST_ENABLED(irq + i, core)) {
+                    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;
+            if (irq >= GICV3_INTERNAL) {
+                DPRINTF("non Internal should be only in dist %lu\n", irq);
+                goto bad_reg;
+            }
+            res = 0;
+            for (i = 0; i < 8; i++) {
+                if (gic_test_pending(s, irq + i, core)) {
+                    res |= (1 << i);
+                }
+            }
+        } else if (offset < 0x400) {
+            /* Interrupt Set/Clear Active.  */
+            if (offset < 0x380)
+                irq = (offset - 0x300) * 8 + GICV3_BASE_IRQ;
+            else
+                irq = (offset - 0x380) * 8 + GICV3_BASE_IRQ;
+            if (irq >= s->num_irq)
+                goto bad_reg;
+            if (irq >= GICV3_INTERNAL)
+                goto bad_reg;
+            res = 0;
+            for (i = 0; i < 8; i++) {
+                if (GIC_TEST_ACTIVE(irq + i, core)) {
+                    res |= (1 << i);
+                }
+            }
+        } else if (offset < 0x800) {
+            /* Interrupt Priority.  */
+            irq = (offset - 0x400) + GICV3_BASE_IRQ;
+            if (irq >= s->num_irq)
+                goto bad_reg;
+            if (irq >= GICV3_INTERNAL * 8) {
+                DPRINTF("non Internal should be only in dist %lu\n", irq);
+                goto bad_reg;
+            }
+            res = GIC_GET_PRIORITY(irq, core);
+        } else if (offset < 0xc00) {
+                goto bad_reg;
+        } else if (offset < 0xd00) {
+            /* Interrupt Configuration.  */
+            irq = (offset - 0xc00) * 4 + GICV3_BASE_IRQ;
+            if (irq >= s->num_irq)
+                goto bad_reg;
+            if (irq >= GICV3_INTERNAL) {
+                DPRINTF("non Internal should be only in dist %lu\n", 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 {
+        /* 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 */
+                uint64_t mp_affinity = s->mp_affinity[core];
+                res = core << 8; /* Linear cpu number */
+                /* See GICv3 section 5.4.8 */
+                res |= (mp_affinity & ARM_AFF0_MASK) << (32 - ARM_AFF0_SHIFT);
+                res |= (mp_affinity & ARM_AFF1_MASK) << (40 - ARM_AFF1_SHIFT);
+                res |= (mp_affinity & ARM_AFF2_MASK) << (48 - ARM_AFF2_SHIFT);
+                res |= (mp_affinity & ARM_AFF3_MASK) << (56 - ARM_AFF3_SHIFT);
+                if (core == s->num_cpu - 1) {
+                    /* Mark last redistributer */
+                    res |= 1 << 4;
+                }
+                return res;
+            }
+            if (offset == 0xc) { /* GICR_TYPER */
+                /* should write readll */
+                return 0;
+            }
+            if (offset == 0x14) { /* GICR_WAKER */
+                if (test_bit(core, s->cpu_enabled))
+                    return 0;
+                else
+                    return GICR_WAKER_ProcessorSleep;
+                DPRINTF("Redist-CPU (%d) is enabled(%lu)\n",
+                        gic_get_current_cpu(s), test_bit(core, s->cpu_enabled));
+
+            }
+            if (offset >= 0x80 && offset < 0xFFD0)
+                return 0;
+            goto bad_reg;
+        }
+        if (offset < 0xffd0) {
+            goto bad_reg;
+        } else /* offset >= 0xffd0 */ {
+            if (offset & 3) {
+                res = 0;
+            } else {
+                res = gic_redist_ids[(offset - 0xffd0) >> 2];
+            }
+        }
+    }
+    return res;
+bad_reg:
+    qemu_log_mask(LOG_GUEST_ERROR,
+                  "%s: Bad offset %x\n", __func__, (int)offset);
+    return 0;
+}
+
+MemTxResult gic_redist_read(void *opaque, hwaddr offset, uint64_t *data,
+                            unsigned size, MemTxAttrs attrs)
+{
+    switch (size) {
+    case 1:
+        *data = gic_redist_readb(opaque, offset, attrs);
+        return MEMTX_OK;
+    case 2:
+        *data = gic_redist_readb(opaque, offset, attrs);
+        *data |= gic_redist_readb(opaque, offset + 1, attrs) << 8;
+        return MEMTX_OK;
+    case 4:
+        *data = gic_redist_readb(opaque, offset, attrs);
+        *data |= gic_redist_readb(opaque, offset + 1, attrs) << 8;
+        *data |= gic_redist_readb(opaque, offset + 2, attrs) << 16;
+        *data |= gic_redist_readb(opaque, offset + 3, attrs) << 24;
+        return MEMTX_OK;
+    case 8:
+        *data = gic_redist_readb(opaque, offset, attrs);
+        *data |= gic_redist_readb(opaque, offset + 1, attrs) << 8;
+        *data |= gic_redist_readb(opaque, offset + 2, attrs) << 16;
+        *data |= gic_redist_readb(opaque, offset + 3, attrs) << 24;
+        *data |= gic_redist_readb(opaque, offset + 4, attrs) << 32;
+        *data |= gic_redist_readb(opaque, offset + 5, attrs) << 40;
+        *data |= gic_redist_readb(opaque, offset + 6, attrs) << 48;
+        *data |= gic_redist_readb(opaque, offset + 7, attrs) << 56;
+        return MEMTX_OK;
+    default:
+        return MEMTX_ERROR;
+    }
+}
+
+static void gic_redist_writeb(void *opaque, hwaddr offset,
+                            uint64_t value, MemTxAttrs attrs)
+{
+    GICv3State *s = (GICv3State *)opaque;
+    uint64_t sgi_ppi, core, off;
+    int irq, i;
+    /* GIC-500 Table 3-2 page 3-4 (Rev r0p0)
+     * [          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;
+
+    if (sgi_ppi) {
+        /* SGIs, PPIs */
+        if (offset < 0x100) {
+            if (offset >= 0x80) {
+                /* Interrupt Group Registers: RAZ/WI for NS access to secure
+                 * GIC, or for GICs without groups.
+                 */
+                if (!attrs.secure) {
+                    /* GIC-500 comment 'f' */
+                    goto bad_reg;
+                }
+                /* Every byte offset holds 8 group status bits */
+                irq = (offset - 0x80) * 8 + GICV3_BASE_IRQ;
+                if (irq >= s->num_irq) {
+                    goto bad_reg;
+                }
+                if (irq >= GICV3_INTERNAL) {
+                    DPRINTF("IGROUPR0 non Internal should be only in distributer %d\n", irq);
+                    return;
+                }
+                for (i = 0; i < 8; i++) {
+                    /* Group bits are banked for private interrupts */
+                    if (value & (1 << i)) {
+                        /* Group1 (Non-secure) */
+                        GIC_SET_GROUP(irq + i, core);
+                    } else {
+                        /* Group0 (Secure) */
+                        GIC_CLEAR_GROUP(irq + i, core);
+                    }
+                }
+            }
+        } 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_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, core)) {
+                        DPRINTF("Enabled IRQ %d\n", irq + i);
+                    }
+                    GIC_SET_ENABLED(irq + i, core);
+                    /* If a raised level triggered IRQ enabled then mark
+                       is as pending.  */
+                    if (GIC_TEST_LEVEL(irq + i, core)
+                            && !GIC_TEST_EDGE_TRIGGER(irq + i)) {
+                        DPRINTF("Set %d pending core %lx\n", irq + i, core);
+                        GIC_SET_PENDING(irq + i, core);
+                    }
+                }
+            }
+        } 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_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, core)) {
+                        DPRINTF("Disabled IRQ %d\n", irq + i);
+                    }
+                    GIC_CLEAR_ENABLED(irq + i, core);
+                }
+            }
+            if (irq >= GICV3_INTERNAL) {
+                DPRINTF("non Internal should be only in distributer %d\n", irq);
+                goto bad_reg;
+            }
+        } 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_INTERNAL) {
+                DPRINTF("non Internal should be only in distributer %d\n", irq);
+                goto bad_reg;
+            }
+            for (i = 0; i < 8; i++) {
+                if (value & (1 << i)) {
+                    GIC_SET_PENDING_MASK(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_INTERNAL) {
+                DPRINTF("non Internal should be only in distributer %d\n", irq);
+                goto bad_reg;
+            }
+            for (i = 0; i < 8; i++) {
+                if (value & (1 << i)) {
+                    GIC_CLEAR_PENDING(irq + i, core);
+                }
+            }
+        } else if (offset < 0x400) {
+            /* Interrupt Active.  */
+            goto bad_reg;
+        } else if (offset < 0x800) {
+            /* Interrupt Priority. */
+            irq = (offset - 0x400) + GICV3_BASE_IRQ;
+            if (irq >= s->num_irq)
+                goto bad_reg;
+            if (irq >= GICV3_INTERNAL * 8) {
+                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, attrs);
+        } else if (offset < 0xc00) {
+            goto bad_reg;
+        } else if (offset < 0xd00) {
+            /* Interrupt Configuration.  */
+            irq = (offset - 0xc00) * 4 + GICV3_BASE_IRQ;
+            if (irq >= s->num_irq)
+                goto bad_reg;
+            if (irq >= GICV3_INTERNAL) {
+                DPRINTF("non Internal should be only in distributer %d\n", 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);
+                }
+            }
+        }
+    } else {
+        /* LPIs */
+        if (offset == 0x14) { /* GICR_WAKER */
+            if (value & GICR_WAKER_ProcessorSleep)
+                clear_bit(core, s->cpu_enabled);
+            else
+                set_bit(core, s->cpu_enabled);
+            DPRINTF("Redist-CPU (%d) core(%lu) set enabled(%d)\n",
+                    gic_get_current_cpu(s), core, test_bit(core, s->cpu_enabled));
+       }
+    }
+    gicv3_update(s);
+    return;
+
+bad_reg:
+    qemu_log_mask(LOG_GUEST_ERROR,
+                  "%s: Bad offset %x\n", __func__, (int)offset);
+}
+
+static void gic_redist_writew(void *opaque, hwaddr offset,
+                            uint64_t value, MemTxAttrs attrs)
+{
+    gic_redist_writeb(opaque, offset, value & 0xff, attrs);
+    gic_redist_writeb(opaque, offset + 1, value >> 8, attrs);
+}
+
+static void gic_redist_writel(void *opaque, hwaddr offset,
+                            uint64_t value, MemTxAttrs attrs)
+{
+    gic_redist_writew(opaque, offset, value & 0xffff, attrs);
+    gic_redist_writew(opaque, offset + 2, value >> 16, attrs);
+}
+
+static void gic_redist_writell(void *opaque, hwaddr offset,
+                            uint64_t value, MemTxAttrs attrs)
+{
+    gic_redist_writel(opaque, offset, value & 0xffffffff, attrs);
+    gic_redist_writel(opaque, offset + 4, value >> 32, attrs);
+}
+
+MemTxResult gic_redist_write(void *opaque, hwaddr addr, uint64_t data,
+                             unsigned size, MemTxAttrs attrs)
+{
+    switch (size) {
+    case 1:
+        gic_redist_writeb(opaque, addr, data, attrs);
+        return MEMTX_OK;
+    case 2:
+        gic_redist_writew(opaque, addr, data, attrs);
+        return MEMTX_OK;
+    case 4:
+        gic_redist_writel(opaque, addr, data, attrs);
+        return MEMTX_OK;
+    case 8:
+        gic_redist_writell(opaque, addr, data, attrs);
+        return MEMTX_OK;
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: size %u\n", __func__, size);
+        return MEMTX_ERROR;
+    }
+}
diff --git a/hw/intc/arm_gicv3_redist.h b/hw/intc/arm_gicv3_redist.h
new file mode 100644
index 0000000..cc65b8f
--- /dev/null
+++ b/hw/intc/arm_gicv3_redist.h
@@ -0,0 +1,9 @@
+#ifndef QEMU_ARM_GICV3_REDIST_H
+#define QEMU_ARM_GICV3_REDIST_H
+
+MemTxResult gic_redist_read(void *opaque, hwaddr offset, uint64_t *data,
+                            unsigned size, MemTxAttrs attrs);
+MemTxResult gic_redist_write(void *opaque, hwaddr addr, uint64_t data,
+                             unsigned size, MemTxAttrs attrs);
+
+#endif /* !QEMU_ARM_GIC_REDIST_H */
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [Qemu-devel] [PATCH RFC V5 6/9] hw/intc: arm_gicv3_spi_its
  2015-10-20 17:22 [Qemu-devel] [PATCH RFC V5 0/9] Implement GIC-500 from GICv3 family for arm64 Shlomo Pongratz
                   ` (3 preceding siblings ...)
  2015-10-20 17:22 ` [Qemu-devel] [PATCH RFC V5 5/9] hw/intc arm_gicv3_redist Shlomo Pongratz
@ 2015-10-20 17:22 ` Shlomo Pongratz
  2015-10-21 12:43   ` Pavel Fedin
  2015-10-20 17:22 ` [Qemu-devel] [PATCH RFC V5 7/9] hw/intc: arm_gicv3 Shlomo Pongratz
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 41+ messages in thread
From: Shlomo Pongratz @ 2015-10-20 17:22 UTC (permalink / raw)
  To: qemu-devel
  Cc: peter.maydell, eric.auger, Shlomo Pongratz, p.fedin,
	shannon.zhao, ashoks, imammedo

From: Shlomo Pongratz <shlomo.pongratz@huawei.com>

This patch includes a placeholder code for future spi and its
implementation.

Signed-off-by: Shlomo Pongratz <shlomo.pongratz@huawei.com>
---
 hw/intc/Makefile.objs       |   1 +
 hw/intc/arm_gicv3_spi_its.c | 359 ++++++++++++++++++++++++++++++++++++++++++++
 hw/intc/arm_gicv3_spi_its.h |  11 ++
 3 files changed, 371 insertions(+)
 create mode 100644 hw/intc/arm_gicv3_spi_its.c
 create mode 100644 hw/intc/arm_gicv3_spi_its.h

diff --git a/hw/intc/Makefile.objs b/hw/intc/Makefile.objs
index 5e56acc..0e6784f 100644
--- a/hw/intc/Makefile.objs
+++ b/hw/intc/Makefile.objs
@@ -17,6 +17,7 @@ common-obj-$(CONFIG_ARM_GIC) += arm_gicv3_interrupts.o
 common-obj-$(CONFIG_ARM_GIC) += arm_gicv3_cpu_interface.o
 common-obj-$(CONFIG_ARM_GIC) += arm_gicv3_dist.o
 common-obj-$(CONFIG_ARM_GIC) += arm_gicv3_redist.o
+common-obj-$(CONFIG_ARM_GIC) += arm_gicv3_spi_its.o
 common-obj-$(CONFIG_OPENPIC) += openpic.o
 
 obj-$(CONFIG_APIC) += apic.o apic_common.o
diff --git a/hw/intc/arm_gicv3_spi_its.c b/hw/intc/arm_gicv3_spi_its.c
new file mode 100644
index 0000000..cd2b56d
--- /dev/null
+++ b/hw/intc/arm_gicv3_spi_its.c
@@ -0,0 +1,359 @@
+#include "gicv3_internal.h"
+#include "qom/cpu.h"
+#include "arm_gicv3_spi_its.h"
+#include "arm_gicv3_interrupts.h"
+
+/* ITS routines are stubs for future development */
+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)
+{
+    GICv3State *s = (GICv3State *)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);
+}
+
+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;
+}
+
+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;
+    }
+}
+
+/* SPI routines are stubs for future development */
+static uint64_t gic_spi_readb(void *opaque, hwaddr offset)
+{
+    return 0;
+}
+
+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)
+{
+    GICv3State *s = (GICv3State *)opaque;
+    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);
+}
+
+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;
+}
+
+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;
+    }
+}
+
+/* ITS control routines are stubs for future development */
+static uint64_t gic_its_cntrl_readb(void *opaque, hwaddr offset)
+{
+    GICv3State *s = (GICv3State *)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)
+{
+    GICv3State *s = (GICv3State *)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);
+}
+
+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;
+}
+
+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;
+    }
+}
diff --git a/hw/intc/arm_gicv3_spi_its.h b/hw/intc/arm_gicv3_spi_its.h
new file mode 100644
index 0000000..18e7c4b
--- /dev/null
+++ b/hw/intc/arm_gicv3_spi_its.h
@@ -0,0 +1,11 @@
+#ifndef QEMU_ARM_GICV3_SPI_ITS_H
+#define QEMU_ARM_GICV3_SPI_ITS_H
+
+uint64_t gic_its_read(void *opaque, hwaddr addr, unsigned size);
+void gic_its_write(void *opaque, hwaddr addr, uint64_t data, unsigned size);
+uint64_t gic_spi_read(void *opaque, hwaddr addr, unsigned size);
+void gic_spi_write(void *opaque, hwaddr addr, uint64_t data, unsigned size);
+uint64_t gic_its_cntrl_read(void *opaque, hwaddr addr, unsigned size);
+void gic_its_cntrl_write(void *opaque, hwaddr addr, uint64_t data, unsigned size);
+
+#endif /* !QEMU_ARM_GIC_SPI_ITS_H */
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [Qemu-devel] [PATCH RFC V5 7/9] hw/intc: arm_gicv3
  2015-10-20 17:22 [Qemu-devel] [PATCH RFC V5 0/9] Implement GIC-500 from GICv3 family for arm64 Shlomo Pongratz
                   ` (4 preceding siblings ...)
  2015-10-20 17:22 ` [Qemu-devel] [PATCH RFC V5 6/9] hw/intc: arm_gicv3_spi_its Shlomo Pongratz
@ 2015-10-20 17:22 ` Shlomo Pongratz
  2015-10-20 17:22 ` [Qemu-devel] [PATCH RFC V5 8/9] target-arm/cpu64 GICv3 system instructions support Shlomo Pongratz
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 41+ messages in thread
From: Shlomo Pongratz @ 2015-10-20 17:22 UTC (permalink / raw)
  To: qemu-devel
  Cc: peter.maydell, eric.auger, Shlomo Pongratz, p.fedin,
	shannon.zhao, ashoks, imammedo

From: Shlomo Pongratz <shlomo.pongratz@huawei.com>

This patch includes the code that binds together the codes of
all the previous patches.

Signed-off-by: Shlomo Pongratz <shlomo.pongratz@huawei.com>
---
 hw/intc/Makefile.objs |   1 +
 hw/intc/arm_gicv3.c   | 134 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 135 insertions(+)
 create mode 100644 hw/intc/arm_gicv3.c

diff --git a/hw/intc/Makefile.objs b/hw/intc/Makefile.objs
index 0e6784f..b7b2288 100644
--- a/hw/intc/Makefile.objs
+++ b/hw/intc/Makefile.objs
@@ -18,6 +18,7 @@ common-obj-$(CONFIG_ARM_GIC) += arm_gicv3_cpu_interface.o
 common-obj-$(CONFIG_ARM_GIC) += arm_gicv3_dist.o
 common-obj-$(CONFIG_ARM_GIC) += arm_gicv3_redist.o
 common-obj-$(CONFIG_ARM_GIC) += arm_gicv3_spi_its.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..62c8299
--- /dev/null
+++ b/hw/intc/arm_gicv3.c
@@ -0,0 +1,134 @@
+/*
+ * 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
+ *
+ * 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
+ * working with bitops.
+ */
+
+#include "hw/sysbus.h"
+#include "gicv3_internal.h"
+#include "qom/cpu.h"
+#include "arm_gicv3_cpu_interface.h"
+#include "arm_gicv3_interrupts.h"
+#include "arm_gicv3_dist.h"
+#include "arm_gicv3_redist.h"
+#include "arm_gicv3_spi_its.h"
+
+static const MemoryRegionOps gic_ops[] = {
+    {
+        .read_with_attrs = gic_dist_read,
+        .write_with_attrs = gic_dist_write,
+        .impl = {
+             .min_access_size = 4,
+             .max_access_size = 8,
+         },
+        .endianness = DEVICE_NATIVE_ENDIAN,
+    },
+    {
+        .read_with_attrs = gic_redist_read,
+        .write_with_attrs = gic_redist_write,
+        .impl = {
+             .min_access_size = 4,
+             .max_access_size = 8,
+         },
+        .endianness = DEVICE_NATIVE_ENDIAN,
+    },
+    {
+        .read = gic_its_read,
+        .write = gic_its_write,
+        .impl = {
+             .min_access_size = 4,
+             .max_access_size = 8,
+         },
+        .endianness = DEVICE_NATIVE_ENDIAN,
+    },
+    {
+        .read = gic_spi_read,
+        .write = gic_spi_write,
+        .impl = {
+             .min_access_size = 4,
+             .max_access_size = 8,
+         },
+        .endianness = DEVICE_NATIVE_ENDIAN,
+    },
+    {
+        .read = gic_its_cntrl_read,
+        .write = gic_its_cntrl_write,
+        .impl = {
+             .min_access_size = 4,
+             .max_access_size = 8,
+         },
+        .endianness = DEVICE_NATIVE_ENDIAN,
+    }
+};
+
+static void arm_gic_realize(DeviceState *dev, Error **errp)
+{
+    /* Device instance realize function for the GIC sysbus device */
+    GICv3State *s = ARM_GICV3(dev);
+    ARMGICv3Class *agc = ARM_GICV3_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;
+    }
+
+    /* Cuurently no GICv2 backwards compatibility (e.g. no memory mapped regs)
+     * Uses system gicv3_no_gicv2_bc mode
+     */
+    gicv3_sre = 1;
+
+    /* Tell the common code we're a GICv3 */
+    s->revision = REV_V3;
+
+    gicv3_init_irqs_and_mmio(s, gicv3_set_irq, gic_ops);
+
+    /* 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);
+
+    bitmap_zero(s->cpu_enabled, s->num_cpu);
+}
+
+static void arm_gicv3_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    ARMGICv3Class *agc = ARM_GICV3_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(GICv3State),
+    .class_init = arm_gicv3_class_init,
+    .class_size = sizeof(ARMGICv3Class),
+};
+
+static void arm_gicv3_register_types(void)
+{
+    type_register_static(&arm_gicv3_info);
+}
+
+type_init(arm_gicv3_register_types)
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [Qemu-devel] [PATCH RFC V5 8/9] target-arm/cpu64 GICv3 system instructions support
  2015-10-20 17:22 [Qemu-devel] [PATCH RFC V5 0/9] Implement GIC-500 from GICv3 family for arm64 Shlomo Pongratz
                   ` (5 preceding siblings ...)
  2015-10-20 17:22 ` [Qemu-devel] [PATCH RFC V5 7/9] hw/intc: arm_gicv3 Shlomo Pongratz
@ 2015-10-20 17:22 ` Shlomo Pongratz
  2015-10-22  7:33   ` Pavel Fedin
  2015-10-20 17:22 ` [Qemu-devel] [PATCH RFC V5 9/9] hw/arm: Add virt-v3 machine that uses GIC-500 Shlomo Pongratz
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 41+ messages in thread
From: Shlomo Pongratz @ 2015-10-20 17:22 UTC (permalink / raw)
  To: qemu-devel
  Cc: peter.maydell, eric.auger, Shlomo Pongratz, p.fedin,
	shannon.zhao, ashoks, imammedo

From: Shlomo Pongratz <shlomo.pongratz@huawei.com>

Add system instructions used by the Linux (kernel) GICv3
device driver

Signed-off-by: Shlomo Pongratz <shlomo.pongratz@huawei.com>
---
 target-arm/cpu-qom.h |   1 +
 target-arm/cpu.h     |  12 ++++++
 target-arm/cpu64.c   | 118 +++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 131 insertions(+)

diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
index 25fb1ce..6a50433 100644
--- a/target-arm/cpu-qom.h
+++ b/target-arm/cpu-qom.h
@@ -220,6 +220,7 @@ hwaddr arm_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
 
 int arm_cpu_gdb_read_register(CPUState *cpu, uint8_t *buf, int reg);
 int arm_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
+void aarch64_registers_with_opaque_set(Object *obj, void *opaque);
 
 /* Callback functions for the generic timer's timers. */
 void arm_gt_ptimer_cb(void *opaque);
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 3daa7f5..d561313 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -1034,6 +1034,18 @@ void armv7m_nvic_set_pending(void *opaque, int irq);
 int armv7m_nvic_acknowledge_irq(void *opaque);
 void armv7m_nvic_complete_irq(void *opaque, int irq);
 
+void armv8_gicv3_set_sgi(void *opaque, int cpuindex, uint64_t value);
+uint64_t armv8_gicv3_acknowledge_irq(void *opaque, int cpuindex,
+                              MemTxAttrs attrs);
+void armv8_gicv3_complete_irq(void *opaque, int cpuindex, int irq,
+                              MemTxAttrs attrs);
+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);
+uint64_t armv8_gicv3_get_igrpen1(void *opaque, int cpuindex);
+void armv8_gicv3_set_igrpen1(void *opaque, int cpuindex, uint64_t igrpen1);
+
 /* Interface for defining coprocessor registers.
  * Registers are defined in tables of arm_cp_reginfo structs
  * which are passed to define_arm_cp_regs().
diff --git a/target-arm/cpu64.c b/target-arm/cpu64.c
index 63c8b1c..4224779 100644
--- a/target-arm/cpu64.c
+++ b/target-arm/cpu64.c
@@ -45,6 +45,115 @@ static uint64_t a57_a53_l2ctlr_read(CPUARMState *env, const ARMCPRegInfo *ri)
 }
 #endif
 
+#ifndef CONFIG_USER_ONLY
+static void sgi_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
+{
+    CPUState *cpu = ENV_GET_CPU(env);
+    armv8_gicv3_set_sgi(ri->opaque, cpu->cpu_index, value);
+}
+
+static uint64_t iar_read(CPUARMState *env, const ARMCPRegInfo *ri)
+{
+    uint64_t value;
+    MemTxAttrs attrs;;
+    CPUState *cpu = ENV_GET_CPU(env);
+    attrs.secure = arm_is_secure_below_el3(env) ? 1 : 0;
+    value = armv8_gicv3_acknowledge_irq(ri->opaque, cpu->cpu_index, attrs);
+    return value;
+}
+
+static void sre_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
+{
+    armv8_gicv3_set_sre(ri->opaque, value);
+}
+
+static uint64_t sre_read(CPUARMState *env, const ARMCPRegInfo *ri)
+{
+    uint64_t value;
+    value = armv8_gicv3_get_sre(ri->opaque);
+    return value;
+}
+
+static void eoir_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
+{
+    MemTxAttrs attrs;
+    CPUState *cpu = ENV_GET_CPU(env);
+    attrs.secure = arm_is_secure_below_el3(env) ? 1 : 0;
+    armv8_gicv3_complete_irq(ri->opaque, cpu->cpu_index, value, attrs);
+}
+
+static uint64_t pmr_read(CPUARMState *env, const ARMCPRegInfo *ri)
+{
+    uint64_t value;
+    CPUState *cpu = ENV_GET_CPU(env);
+    value = armv8_gicv3_get_priority_mask(ri->opaque, cpu->cpu_index);
+    return value;
+}
+
+static void pmr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
+{
+    CPUState *cpu = ENV_GET_CPU(env);
+    armv8_gicv3_set_priority_mask(ri->opaque, cpu->cpu_index, value);
+}
+
+static uint64_t igrpen1_read(CPUARMState *env, const ARMCPRegInfo *ri)
+{
+    uint64_t value;
+    CPUState *cpu = ENV_GET_CPU(env);
+    value = armv8_gicv3_get_igrpen1(ri->opaque, cpu->cpu_index);
+    return value;
+}
+
+static void igrpen1_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
+{
+    CPUState *cpu = ENV_GET_CPU(env);
+    armv8_gicv3_set_igrpen1(ri->opaque, cpu->cpu_index, value);
+}
+#endif
+
+static const ARMCPRegInfo cortex_a57_a53_cp_with_opaque_reginfo[] = {
+    { .name = "EIOR1_EL1", .state = ARM_CP_STATE_AA64,
+#ifndef CONFIG_USER_ONLY
+      .writefn = eoir_write,
+#endif
+      .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 12, .opc2 = 1,
+      .access = PL1_W, .type = ARM_CP_SPECIAL, .resetvalue = 0 },
+    { .name = "IAR1_EL1", .state = ARM_CP_STATE_AA64,
+#ifndef CONFIG_USER_ONLY
+      .readfn = iar_read,
+#endif
+      .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 12, .opc2 = 0,
+      .access = PL1_R, .type = ARM_CP_SPECIAL, .resetvalue = 0 },
+    { .name = "SGI1R_EL1", .state = ARM_CP_STATE_AA64,
+#ifndef CONFIG_USER_ONLY
+      .writefn = sgi_write,
+#endif
+      .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 11, .opc2 = 5,
+      .access = PL1_RW, .type = ARM_CP_SPECIAL, .resetvalue = 0 },
+    { .name = "PMR_EL1", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .opc1 = 0, .crn = 4, .crm = 6, .opc2 = 0,
+#ifndef CONFIG_USER_ONLY
+      .readfn = pmr_read, .writefn = pmr_write,
+#endif
+      .access = PL1_RW, .type = ARM_CP_SPECIAL, .resetvalue = 0 },
+    { .name = "CTLR_EL1", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 12, .opc2 = 4,
+      .access = PL1_RW, .type = ARM_CP_SPECIAL, .resetvalue = 0 },
+    { .name = "SRE_EL1", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 12, .opc2 = 5, .resetvalue = 0,
+#ifndef CONFIG_USER_ONLY
+      .readfn = sre_read, .writefn = sre_write,
+#endif
+      .access = PL1_RW, .type = ARM_CP_SPECIAL, .resetvalue = 0 },
+    { .name = "IGRPEN1_EL1", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 12, .opc2 = 7,
+#ifndef CONFIG_USER_ONLY
+      .readfn = igrpen1_read, .writefn = igrpen1_write,
+#endif
+      .access = PL1_RW, .type = ARM_CP_SPECIAL, .resetvalue = 0 },
+    REGINFO_SENTINEL
+};
+
 static const ARMCPRegInfo cortex_a57_a53_cp_reginfo[] = {
 #ifndef CONFIG_USER_ONLY
     { .name = "L2CTLR_EL1", .state = ARM_CP_STATE_AA64,
@@ -258,6 +367,15 @@ static void aarch64_cpu_set_aarch64(Object *obj, bool value, Error **errp)
     }
 }
 
+void aarch64_registers_with_opaque_set(Object *obj, void *opaque)
+{
+    ARMCPU *cpu = ARM_CPU(obj);
+
+    define_arm_cp_regs_with_opaque(cpu,
+                                   cortex_a57_a53_cp_with_opaque_reginfo,
+                                   opaque);
+}
+
 static void aarch64_cpu_initfn(Object *obj)
 {
     object_property_add_bool(obj, "aarch64", aarch64_cpu_get_aarch64,
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [Qemu-devel] [PATCH RFC V5 9/9] hw/arm: Add virt-v3 machine that uses GIC-500
  2015-10-20 17:22 [Qemu-devel] [PATCH RFC V5 0/9] Implement GIC-500 from GICv3 family for arm64 Shlomo Pongratz
                   ` (6 preceding siblings ...)
  2015-10-20 17:22 ` [Qemu-devel] [PATCH RFC V5 8/9] target-arm/cpu64 GICv3 system instructions support Shlomo Pongratz
@ 2015-10-20 17:22 ` Shlomo Pongratz
  2015-10-21  7:02   ` Pavel Fedin
       [not found] ` <1445361732-16257-2-git-send-email-shlomopongratz@gmail.com>
  2016-01-29 16:58 ` [Qemu-devel] [PATCH RFC V5 0/9] Implement GIC-500 from GICv3 family for arm64 Christopher Covington
  9 siblings, 1 reply; 41+ messages in thread
From: Shlomo Pongratz @ 2015-10-20 17:22 UTC (permalink / raw)
  To: qemu-devel
  Cc: peter.maydell, eric.auger, Shlomo Pongratz, p.fedin,
	shannon.zhao, ashoks, imammedo

From: Shlomo Pongratz <shlomo.pongratz@huawei.com>

Add virt-v3 machine that uses GIC-500.
Registers the CPU system instructions and bind them to the GIC.
Pass the cores' affinity to the GIC.

Signed-off-by: Shlomo Pongratz <shlomo.pongratz@huawei.com>
---
 hw/arm/virt.c         | 87 ++++++++++++++++++++++++++++++++++++++++++++++-----
 include/hw/arm/fdt.h  |  2 ++
 include/hw/arm/virt.h |  1 +
 target-arm/machine.c  |  7 ++++-
 4 files changed, 88 insertions(+), 9 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 5d38c47..d4fb8f3 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -71,6 +71,7 @@ typedef struct VirtBoardInfo {
     uint32_t clock_phandle;
     uint32_t gic_phandle;
     uint32_t v2m_phandle;
+    const char *class_name;
 } VirtBoardInfo;
 
 typedef struct {
@@ -86,6 +87,7 @@ typedef struct {
 } VirtMachineState;
 
 #define TYPE_VIRT_MACHINE   MACHINE_TYPE_NAME("virt")
+#define TYPE_VIRTV3_MACHINE MACHINE_TYPE_NAME("virt-v3")
 #define VIRT_MACHINE(obj) \
     OBJECT_CHECK(VirtMachineState, (obj), TYPE_VIRT_MACHINE)
 #define VIRT_MACHINE_GET_CLASS(obj) \
@@ -111,6 +113,7 @@ static const MemMapEntry a15memmap[] = {
     [VIRT_CPUPERIPHS] =         { 0x08000000, 0x00020000 },
     /* GIC distributor and CPU interfaces sit inside the CPU peripheral space */
     [VIRT_GIC_DIST] =           { 0x08000000, 0x00010000 },
+    /* Note on GICv3 VIRT_GIC_DIST_SPI takes place of VIRT_GIC_CPU */
     [VIRT_GIC_CPU] =            { 0x08010000, 0x00010000 },
     [VIRT_GIC_V2M] =            { 0x08020000, 0x00001000 },
     /* The space in between here is reserved for GICv3 CPU/vCPU/HYP */
@@ -145,21 +148,26 @@ static VirtBoardInfo machines[] = {
         .cpu_model = "cortex-a15",
         .memmap = a15memmap,
         .irqmap = a15irqmap,
+        .class_name = TYPE_ARM_CPU,
+
     },
     {
         .cpu_model = "cortex-a53",
         .memmap = a15memmap,
         .irqmap = a15irqmap,
+        .class_name = TYPE_AARCH64_CPU,
     },
     {
         .cpu_model = "cortex-a57",
         .memmap = a15memmap,
         .irqmap = a15irqmap,
+        .class_name = TYPE_AARCH64_CPU,
     },
     {
         .cpu_model = "host",
         .memmap = a15memmap,
         .irqmap = a15irqmap,
+        .class_name = TYPE_ARM_CPU,
     },
 };
 
@@ -228,7 +236,6 @@ static void fdt_add_psci_node(const VirtBoardInfo *vbi)
     if (armcpu->psci_version == 2) {
         const char comp[] = "arm,psci-0.2\0arm,psci";
         qemu_fdt_setprop(fdt, "/psci", "compatible", comp, sizeof(comp));
-
         cpu_off_fn = QEMU_PSCI_0_2_FN_CPU_OFF;
         if (arm_feature(&armcpu->env, ARM_FEATURE_AARCH64)) {
             cpu_suspend_fn = QEMU_PSCI_0_2_FN64_CPU_SUSPEND;
@@ -241,7 +248,6 @@ static void fdt_add_psci_node(const VirtBoardInfo *vbi)
         }
     } else {
         qemu_fdt_setprop_string(fdt, "/psci", "compatible", "arm,psci");
-
         cpu_suspend_fn = QEMU_PSCI_0_1_FN_CPU_SUSPEND;
         cpu_off_fn = QEMU_PSCI_0_1_FN_CPU_OFF;
         cpu_on_fn = QEMU_PSCI_0_1_FN_CPU_ON;
@@ -274,6 +280,12 @@ static void fdt_add_timer_nodes(const VirtBoardInfo *vbi, int gictype)
         irqflags = deposit32(irqflags, GIC_FDT_IRQ_PPI_CPU_START,
                              GIC_FDT_IRQ_PPI_CPU_WIDTH,
                              (1 << vbi->smp_cpus) - 1);
+    } else if (gictype == 3) {
+        uint32_t max;
+        /* Argument is 32 bit but 8 bits are reserved for flags */
+        max = (vbi->smp_cpus >= 24) ? 24 : vbi->smp_cpus;
+        irqflags = deposit32(irqflags, GIC_FDT_IRQ_PPI_CPU_START,
+                             GICV3_FDT_IRQ_PPI_CPU_WIDTH, (1 << max) - 1);
     }
 
     qemu_fdt_add_subnode(vbi->fdt, "/timer");
@@ -429,12 +441,32 @@ static void create_gic(VirtBoardInfo *vbi, qemu_irq *pic, int type, bool secure)
     gicdev = qdev_create(NULL, gictype);
     qdev_prop_set_uint32(gicdev, "revision", type);
     qdev_prop_set_uint32(gicdev, "num-cpu", smp_cpus);
+
+#ifdef TARGET_AARCH64
+    if (type == 3) {
+        qdev_prop_set_uint32(gicdev, "len-mp-affinity", smp_cpus);
+        /* Connect GIC to CPU */
+        for (i = 0; i < smp_cpus; i++) {
+            char *propname;
+            CPUState *cpu = qemu_get_cpu(i);
+            ARMCPU *armcpu = ARM_CPU(cpu);
+            propname = g_strdup_printf("mp-affinity[%d]", i);
+            qdev_prop_set_uint64(gicdev, propname, armcpu->mp_affinity);
+            g_free(propname);
+        }
+    }
+#endif
     /* Note that the num-irq property counts both internal and external
      * interrupts; there are always 32 of the former (mandated by GIC spec).
      */
     qdev_prop_set_uint32(gicdev, "num-irq", NUM_IRQS + 32);
     if (!kvm_irqchip_in_kernel()) {
-        qdev_prop_set_bit(gicdev, "has-security-extensions", secure);
+        if (type == 3) {
+            /* AARCH64 has 4 security levels */
+            qdev_prop_set_uint8(gicdev, "security-levels", secure ? 1 : 0);
+        } else {
+            qdev_prop_set_bit(gicdev, "has-security-extensions", secure);
+        }
     }
     qdev_init_nofail(gicdev);
     gicbusdev = SYS_BUS_DEVICE(gicdev);
@@ -445,6 +477,16 @@ static void create_gic(VirtBoardInfo *vbi, qemu_irq *pic, int type, bool secure)
         sysbus_mmio_map(gicbusdev, 1, vbi->memmap[VIRT_GIC_CPU].base);
     }
 
+#ifdef TARGET_AARCH64
+    if (type == 3) {
+        /* Connect GIC to CPU */
+        for (i = 0; i < smp_cpus; i++) {
+            CPUState *cpu = qemu_get_cpu(i);
+            aarch64_registers_with_opaque_set(OBJECT(cpu), (void *)gicdev);
+        }
+    }
+#endif
+
     /* Wire the outputs from each CPU's generic timer to the
      * appropriate GIC PPI inputs, and the GIC's IRQ output to
      * the CPU's IRQ input.
@@ -466,7 +508,7 @@ static void create_gic(VirtBoardInfo *vbi, qemu_irq *pic, int type, bool secure)
         for (irq = 0; irq < ARRAY_SIZE(timer_irq); irq++) {
             qdev_connect_gpio_out(cpudev, irq,
                                   qdev_get_gpio_in(gicdev,
-                                                   ppibase + timer_irq[irq]));
+                                  ppibase + timer_irq[irq]));
         }
 
         sysbus_connect_irq(gicbusdev, i, qdev_get_gpio_in(cpudev, ARM_CPU_IRQ));
@@ -967,7 +1009,7 @@ static void machvirt_init(MachineState *machine)
     create_fdt(vbi);
 
     for (n = 0; n < smp_cpus; n++) {
-        ObjectClass *oc = cpu_class_by_name(TYPE_ARM_CPU, cpustr[0]);
+        ObjectClass *oc = cpu_class_by_name(vbi->class_name, cpustr[0]);
         CPUClass *cc = CPU_CLASS(oc);
         Object *cpuobj;
         Error *err = NULL;
@@ -1116,7 +1158,7 @@ static void virt_set_gic_version(Object *obj, const char *value, Error **errp)
     }
 }
 
-static void virt_instance_init(Object *obj)
+static void virt_instance_init_common(Object *obj, int32_t version)
 {
     VirtMachineState *vms = VIRT_MACHINE(obj);
 
@@ -1140,8 +1182,7 @@ static void virt_instance_init(Object *obj)
                                     "Set on/off to enable/disable using "
                                     "physical address space above 32 bits",
                                     NULL);
-    /* Default GIC type is v2 */
-    vms->gic_version = 2;
+    vms->gic_version = version;
     object_property_add_str(obj, "gic-version", virt_get_gic_version,
                         virt_set_gic_version, NULL);
     object_property_set_description(obj, "gic-version",
@@ -1149,6 +1190,12 @@ static void virt_instance_init(Object *obj)
                                     "Valid values are 2, 3 and host", NULL);
 }
 
+static void virt_instance_init(Object *obj)
+{
+    /* Default GIC type is v2 */
+    virt_instance_init_common(obj, 2);
+}
+
 static void virt_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
@@ -1174,9 +1221,33 @@ static const TypeInfo machvirt_info = {
     .class_init = virt_class_init,
 };
 
+static void virtv3_instance_init(Object *obj)
+{
+    virt_instance_init_common(obj, 3);
+}
+
+static void virtv3_class_init(ObjectClass *oc, void *data)
+{
+    MachineClass *mc = MACHINE_CLASS(oc);
+
+    mc->name = TYPE_VIRTV3_MACHINE;
+    mc->desc = "ARM Virtual Machine with GICv3",
+    mc->init = machvirt_init;
+}
+
+static const TypeInfo machvirtv3_info = {
+    .name = TYPE_VIRTV3_MACHINE,
+    .parent = TYPE_VIRT_MACHINE,
+    .instance_size = sizeof(VirtMachineState),
+    .instance_init = virtv3_instance_init,
+    .class_size = sizeof(VirtMachineClass),
+    .class_init = virtv3_class_init,
+};
+
 static void machvirt_machine_init(void)
 {
     type_register_static(&machvirt_info);
+    type_register_static(&machvirtv3_info);
 }
 
 machine_init(machvirt_machine_init);
diff --git a/include/hw/arm/fdt.h b/include/hw/arm/fdt.h
index c3d5015..ad8f85c 100644
--- a/include/hw/arm/fdt.h
+++ b/include/hw/arm/fdt.h
@@ -30,5 +30,7 @@
 
 #define GIC_FDT_IRQ_PPI_CPU_START 8
 #define GIC_FDT_IRQ_PPI_CPU_WIDTH 8
+#define GICV3_FDT_IRQ_PPI_CPU_WIDTH 24
+
 
 #endif
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index f464586..53ff50a 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -46,6 +46,7 @@ enum {
     VIRT_CPUPERIPHS,
     VIRT_GIC_DIST,
     VIRT_GIC_CPU,
+    VIRT_GIC_DIST_SPI = VIRT_GIC_CPU,
     VIRT_GIC_V2M,
     VIRT_GIC_ITS,
     VIRT_GIC_REDIST,
diff --git a/target-arm/machine.c b/target-arm/machine.c
index 36a0d15..33f28be 100644
--- a/target-arm/machine.c
+++ b/target-arm/machine.c
@@ -341,7 +341,12 @@ const char *gicv3_class_name(void)
 #endif
     } else {
         /* TODO: Software emulation is not implemented yet */
-        error_report("KVM is currently required for GICv3 emulation\n");
+#ifdef TARGET_AARCH64
+        return "arm_gicv3";
+#else
+        error_report("KVM GICv3 acceleration is not supported on this "
+                     "platform\n");
+#endif
     }
 
     exit(1);
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* Re: [Qemu-devel] [PATCH RFC V5 9/9] hw/arm: Add virt-v3 machine that uses GIC-500
  2015-10-20 17:22 ` [Qemu-devel] [PATCH RFC V5 9/9] hw/arm: Add virt-v3 machine that uses GIC-500 Shlomo Pongratz
@ 2015-10-21  7:02   ` Pavel Fedin
  2015-10-21  8:52     ` Shlomo Pongratz
  0 siblings, 1 reply; 41+ messages in thread
From: Pavel Fedin @ 2015-10-21  7:02 UTC (permalink / raw)
  To: 'Shlomo Pongratz', qemu-devel
  Cc: peter.maydell, eric.auger, 'Shlomo Pongratz',
	shannon.zhao, ashoks, imammedo

 Hello!

> -----Original Message-----
> From: Shlomo Pongratz [mailto:shlomopongratz@gmail.com]
> Sent: Tuesday, October 20, 2015 8:22 PM
> To: qemu-devel@nongnu.org
> Cc: p.fedin@samsung.com; peter.maydell@linaro.org; eric.auger@linaro.org;
> shannon.zhao@linaro.org; imammedo@redhat.com; ashoks@broadcom.com; Shlomo Pongratz
> Subject: [PATCH RFC V5 9/9] hw/arm: Add virt-v3 machine that uses GIC-500
> 
> From: Shlomo Pongratz <shlomo.pongratz@huawei.com>
> 
> Add virt-v3 machine that uses GIC-500.

 We already concluded long time ago that adding virt-v3 machine is wrong approach. We already have gic-version property for the virt
machine, and especially for you i left a TODO placeholder in gicv3_class_name() function. Just return name of your class there, and
you're done.

> Registers the CPU system instructions and bind them to the GIC.
> Pass the cores' affinity to the GIC.
> 
> Signed-off-by: Shlomo Pongratz <shlomo.pongratz@huawei.com>
> ---
>  hw/arm/virt.c         | 87 ++++++++++++++++++++++++++++++++++++++++++++++-----
>  include/hw/arm/fdt.h  |  2 ++
>  include/hw/arm/virt.h |  1 +
>  target-arm/machine.c  |  7 ++++-
>  4 files changed, 88 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 5d38c47..d4fb8f3 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -71,6 +71,7 @@ typedef struct VirtBoardInfo {
>      uint32_t clock_phandle;
>      uint32_t gic_phandle;
>      uint32_t v2m_phandle;
> +    const char *class_name;
>  } VirtBoardInfo;
> 
>  typedef struct {
> @@ -86,6 +87,7 @@ typedef struct {
>  } VirtMachineState;
> 
>  #define TYPE_VIRT_MACHINE   MACHINE_TYPE_NAME("virt")
> +#define TYPE_VIRTV3_MACHINE MACHINE_TYPE_NAME("virt-v3")
>  #define VIRT_MACHINE(obj) \
>      OBJECT_CHECK(VirtMachineState, (obj), TYPE_VIRT_MACHINE)
>  #define VIRT_MACHINE_GET_CLASS(obj) \
> @@ -111,6 +113,7 @@ static const MemMapEntry a15memmap[] = {
>      [VIRT_CPUPERIPHS] =         { 0x08000000, 0x00020000 },
>      /* GIC distributor and CPU interfaces sit inside the CPU peripheral space */
>      [VIRT_GIC_DIST] =           { 0x08000000, 0x00010000 },
> +    /* Note on GICv3 VIRT_GIC_DIST_SPI takes place of VIRT_GIC_CPU */

 Memory map of the "virt" machine already contains everything needed. BTW, what's "SPI"? "MBI", you meant? Well, we are going to
have an ITS, so this simple message translator will not be needed.

>      [VIRT_GIC_CPU] =            { 0x08010000, 0x00010000 },
>      [VIRT_GIC_V2M] =            { 0x08020000, 0x00001000 },
>      /* The space in between here is reserved for GICv3 CPU/vCPU/HYP */
> @@ -145,21 +148,26 @@ static VirtBoardInfo machines[] = {
>          .cpu_model = "cortex-a15",
>          .memmap = a15memmap,
>          .irqmap = a15irqmap,
> +        .class_name = TYPE_ARM_CPU,
> +
>      },
>      {
>          .cpu_model = "cortex-a53",
>          .memmap = a15memmap,
>          .irqmap = a15irqmap,
> +        .class_name = TYPE_AARCH64_CPU,
>      },
>      {
>          .cpu_model = "cortex-a57",
>          .memmap = a15memmap,
>          .irqmap = a15irqmap,
> +        .class_name = TYPE_AARCH64_CPU,
>      },
>      {
>          .cpu_model = "host",
>          .memmap = a15memmap,
>          .irqmap = a15irqmap,
> +        .class_name = TYPE_ARM_CPU,
>      },
>  };
> 
> @@ -228,7 +236,6 @@ static void fdt_add_psci_node(const VirtBoardInfo *vbi)
>      if (armcpu->psci_version == 2) {
>          const char comp[] = "arm,psci-0.2\0arm,psci";
>          qemu_fdt_setprop(fdt, "/psci", "compatible", comp, sizeof(comp));
> -
>          cpu_off_fn = QEMU_PSCI_0_2_FN_CPU_OFF;
>          if (arm_feature(&armcpu->env, ARM_FEATURE_AARCH64)) {
>              cpu_suspend_fn = QEMU_PSCI_0_2_FN64_CPU_SUSPEND;
> @@ -241,7 +248,6 @@ static void fdt_add_psci_node(const VirtBoardInfo *vbi)
>          }
>      } else {
>          qemu_fdt_setprop_string(fdt, "/psci", "compatible", "arm,psci");
> -
>          cpu_suspend_fn = QEMU_PSCI_0_1_FN_CPU_SUSPEND;
>          cpu_off_fn = QEMU_PSCI_0_1_FN_CPU_OFF;
>          cpu_on_fn = QEMU_PSCI_0_1_FN_CPU_ON;
> @@ -274,6 +280,12 @@ static void fdt_add_timer_nodes(const VirtBoardInfo *vbi, int gictype)
>          irqflags = deposit32(irqflags, GIC_FDT_IRQ_PPI_CPU_START,
>                               GIC_FDT_IRQ_PPI_CPU_WIDTH,
>                               (1 << vbi->smp_cpus) - 1);
> +    } else if (gictype == 3) {
> +        uint32_t max;
> +        /* Argument is 32 bit but 8 bits are reserved for flags */
> +        max = (vbi->smp_cpus >= 24) ? 24 : vbi->smp_cpus;
> +        irqflags = deposit32(irqflags, GIC_FDT_IRQ_PPI_CPU_START,
> +                             GICV3_FDT_IRQ_PPI_CPU_WIDTH, (1 << max) - 1);
>      }
> 
>      qemu_fdt_add_subnode(vbi->fdt, "/timer");
> @@ -429,12 +441,32 @@ static void create_gic(VirtBoardInfo *vbi, qemu_irq *pic, int type, bool
> secure)
>      gicdev = qdev_create(NULL, gictype);
>      qdev_prop_set_uint32(gicdev, "revision", type);
>      qdev_prop_set_uint32(gicdev, "num-cpu", smp_cpus);
> +
> +#ifdef TARGET_AARCH64
> +    if (type == 3) {
> +        qdev_prop_set_uint32(gicdev, "len-mp-affinity", smp_cpus);
> +        /* Connect GIC to CPU */
> +        for (i = 0; i < smp_cpus; i++) {
> +            char *propname;
> +            CPUState *cpu = qemu_get_cpu(i);
> +            ARMCPU *armcpu = ARM_CPU(cpu);
> +            propname = g_strdup_printf("mp-affinity[%d]", i);
> +            qdev_prop_set_uint64(gicdev, propname, armcpu->mp_affinity);
> +            g_free(propname);
> +        }
> +    }
> +#endif
>      /* Note that the num-irq property counts both internal and external
>       * interrupts; there are always 32 of the former (mandated by GIC spec).
>       */
>      qdev_prop_set_uint32(gicdev, "num-irq", NUM_IRQS + 32);
>      if (!kvm_irqchip_in_kernel()) {
> -        qdev_prop_set_bit(gicdev, "has-security-extensions", secure);
> +        if (type == 3) {
> +            /* AARCH64 has 4 security levels */
> +            qdev_prop_set_uint8(gicdev, "security-levels", secure ? 1 : 0);
> +        } else {
> +            qdev_prop_set_bit(gicdev, "has-security-extensions", secure);
> +        }
>      }
>      qdev_init_nofail(gicdev);
>      gicbusdev = SYS_BUS_DEVICE(gicdev);
> @@ -445,6 +477,16 @@ static void create_gic(VirtBoardInfo *vbi, qemu_irq *pic, int type, bool
> secure)
>          sysbus_mmio_map(gicbusdev, 1, vbi->memmap[VIRT_GIC_CPU].base);
>      }
> 
> +#ifdef TARGET_AARCH64
> +    if (type == 3) {
> +        /* Connect GIC to CPU */
> +        for (i = 0; i < smp_cpus; i++) {
> +            CPUState *cpu = qemu_get_cpu(i);
> +            aarch64_registers_with_opaque_set(OBJECT(cpu), (void *)gicdev);
> +        }
> +    }
> +#endif
> +
>      /* Wire the outputs from each CPU's generic timer to the
>       * appropriate GIC PPI inputs, and the GIC's IRQ output to
>       * the CPU's IRQ input.
> @@ -466,7 +508,7 @@ static void create_gic(VirtBoardInfo *vbi, qemu_irq *pic, int type, bool
> secure)
>          for (irq = 0; irq < ARRAY_SIZE(timer_irq); irq++) {
>              qdev_connect_gpio_out(cpudev, irq,
>                                    qdev_get_gpio_in(gicdev,
> -                                                   ppibase + timer_irq[irq]));
> +                                  ppibase + timer_irq[irq]));
>          }
> 
>          sysbus_connect_irq(gicbusdev, i, qdev_get_gpio_in(cpudev, ARM_CPU_IRQ));
> @@ -967,7 +1009,7 @@ static void machvirt_init(MachineState *machine)
>      create_fdt(vbi);
> 
>      for (n = 0; n < smp_cpus; n++) {
> -        ObjectClass *oc = cpu_class_by_name(TYPE_ARM_CPU, cpustr[0]);
> +        ObjectClass *oc = cpu_class_by_name(vbi->class_name, cpustr[0]);
>          CPUClass *cc = CPU_CLASS(oc);
>          Object *cpuobj;
>          Error *err = NULL;
> @@ -1116,7 +1158,7 @@ static void virt_set_gic_version(Object *obj, const char *value, Error
> **errp)
>      }
>  }
> 
> -static void virt_instance_init(Object *obj)
> +static void virt_instance_init_common(Object *obj, int32_t version)
>  {
>      VirtMachineState *vms = VIRT_MACHINE(obj);
> 
> @@ -1140,8 +1182,7 @@ static void virt_instance_init(Object *obj)
>                                      "Set on/off to enable/disable using "
>                                      "physical address space above 32 bits",
>                                      NULL);
> -    /* Default GIC type is v2 */
> -    vms->gic_version = 2;
> +    vms->gic_version = version;
>      object_property_add_str(obj, "gic-version", virt_get_gic_version,
>                          virt_set_gic_version, NULL);
>      object_property_set_description(obj, "gic-version",
> @@ -1149,6 +1190,12 @@ static void virt_instance_init(Object *obj)
>                                      "Valid values are 2, 3 and host", NULL);
>  }
> 
> +static void virt_instance_init(Object *obj)
> +{
> +    /* Default GIC type is v2 */
> +    virt_instance_init_common(obj, 2);
> +}
> +
>  static void virt_class_init(ObjectClass *oc, void *data)
>  {
>      MachineClass *mc = MACHINE_CLASS(oc);
> @@ -1174,9 +1221,33 @@ static const TypeInfo machvirt_info = {
>      .class_init = virt_class_init,
>  };
> 
> +static void virtv3_instance_init(Object *obj)
> +{
> +    virt_instance_init_common(obj, 3);
> +}
> +
> +static void virtv3_class_init(ObjectClass *oc, void *data)
> +{
> +    MachineClass *mc = MACHINE_CLASS(oc);
> +
> +    mc->name = TYPE_VIRTV3_MACHINE;
> +    mc->desc = "ARM Virtual Machine with GICv3",
> +    mc->init = machvirt_init;
> +}
> +
> +static const TypeInfo machvirtv3_info = {
> +    .name = TYPE_VIRTV3_MACHINE,
> +    .parent = TYPE_VIRT_MACHINE,
> +    .instance_size = sizeof(VirtMachineState),
> +    .instance_init = virtv3_instance_init,
> +    .class_size = sizeof(VirtMachineClass),
> +    .class_init = virtv3_class_init,
> +};
> +
>  static void machvirt_machine_init(void)
>  {
>      type_register_static(&machvirt_info);
> +    type_register_static(&machvirtv3_info);
>  }

 As i said before, this stuff is simply not needed.

> 
>  machine_init(machvirt_machine_init);
> diff --git a/include/hw/arm/fdt.h b/include/hw/arm/fdt.h
> index c3d5015..ad8f85c 100644
> --- a/include/hw/arm/fdt.h
> +++ b/include/hw/arm/fdt.h
> @@ -30,5 +30,7 @@
> 
>  #define GIC_FDT_IRQ_PPI_CPU_START 8
>  #define GIC_FDT_IRQ_PPI_CPU_WIDTH 8
> +#define GICV3_FDT_IRQ_PPI_CPU_WIDTH 24
> +
> 
>  #endif
> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
> index f464586..53ff50a 100644
> --- a/include/hw/arm/virt.h
> +++ b/include/hw/arm/virt.h
> @@ -46,6 +46,7 @@ enum {
>      VIRT_CPUPERIPHS,
>      VIRT_GIC_DIST,
>      VIRT_GIC_CPU,
> +    VIRT_GIC_DIST_SPI = VIRT_GIC_CPU,
>      VIRT_GIC_V2M,
>      VIRT_GIC_ITS,
>      VIRT_GIC_REDIST,
> diff --git a/target-arm/machine.c b/target-arm/machine.c
> index 36a0d15..33f28be 100644
> --- a/target-arm/machine.c
> +++ b/target-arm/machine.c
> @@ -341,7 +341,12 @@ const char *gicv3_class_name(void)
>  #endif
>      } else {
>          /* TODO: Software emulation is not implemented yet */
> -        error_report("KVM is currently required for GICv3 emulation\n");
> +#ifdef TARGET_AARCH64
> +        return "arm_gicv3";

 Peter told me, there is a policy to use dash ("-") in class names. So "arm-gicv3".
 By the way, why does it depend on TARGET_AARCH64? Actually, TARGET_ARM and TARGET_AARCH64 are the same. You can make both emulators
running both 64- and 32-bit code. KVM code depends on TARGET_AARCH64 only because some definitions are missing on 32-bit kernels.
You don't have this restriction, so you don't need this #ifdef.

> +#else
> +        error_report("KVM GICv3 acceleration is not supported on this "
> +                     "platform\n");

 Why "KVM" here? Well, rubbish anyway because of the above. :)

> +#endif
>      }
> 
>      exit(1);
> --
> 1.9.1

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [Qemu-devel] [PATCH RFC V5 9/9] hw/arm: Add virt-v3 machine that uses GIC-500
  2015-10-21  7:02   ` Pavel Fedin
@ 2015-10-21  8:52     ` Shlomo Pongratz
  2015-10-21 10:30       ` Pavel Fedin
  0 siblings, 1 reply; 41+ messages in thread
From: Shlomo Pongratz @ 2015-10-21  8:52 UTC (permalink / raw)
  To: Pavel Fedin
  Cc: peter.maydell, eric.auger, Shlomo Pongratz, qemu-devel,
	shannon.zhao, ashoks, imammedo

[-- Attachment #1: Type: text/plain, Size: 12995 bytes --]

See inline.

On Wednesday, October 21, 2015, Pavel Fedin <p.fedin@samsung.com> wrote:

>  Hello!
>
> > -----Original Message-----
> > From: Shlomo Pongratz [mailto:shlomopongratz@gmail.com <javascript:;>]
> > Sent: Tuesday, October 20, 2015 8:22 PM
> > To: qemu-devel@nongnu.org <javascript:;>
> > Cc: p.fedin@samsung.com <javascript:;>; peter.maydell@linaro.org
> <javascript:;>; eric.auger@linaro.org <javascript:;>;
> > shannon.zhao@linaro.org <javascript:;>; imammedo@redhat.com
> <javascript:;>; ashoks@broadcom.com <javascript:;>; Shlomo Pongratz
> > Subject: [PATCH RFC V5 9/9] hw/arm: Add virt-v3 machine that uses GIC-500
> >
> > From: Shlomo Pongratz <shlomo.pongratz@huawei.com <javascript:;>>
> >
> > Add virt-v3 machine that uses GIC-500.
>
>  We already concluded long time ago that adding virt-v3 machine is wrong
> approach. We already have gic-version property for the virt
> machine, and especially for you i left a TODO placeholder in
> gicv3_class_name() function. Just return name of your class there, and
> you're done.
>
>
I see, just how do I pass the gic version from the command line?


> > Registers the CPU system instructions and bind them to the GIC.
> > Pass the cores' affinity to the GIC.
> >
> > Signed-off-by: Shlomo Pongratz <shlomo.pongratz@huawei.com
> <javascript:;>>
> > ---
> >  hw/arm/virt.c         | 87
> ++++++++++++++++++++++++++++++++++++++++++++++-----
> >  include/hw/arm/fdt.h  |  2 ++
> >  include/hw/arm/virt.h |  1 +
> >  target-arm/machine.c  |  7 ++++-
> >  4 files changed, 88 insertions(+), 9 deletions(-)
> >
> > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> > index 5d38c47..d4fb8f3 100644
> > --- a/hw/arm/virt.c
> > +++ b/hw/arm/virt.c
> > @@ -71,6 +71,7 @@ typedef struct VirtBoardInfo {
> >      uint32_t clock_phandle;
> >      uint32_t gic_phandle;
> >      uint32_t v2m_phandle;
> > +    const char *class_name;
> >  } VirtBoardInfo;
> >
> >  typedef struct {
> > @@ -86,6 +87,7 @@ typedef struct {
> >  } VirtMachineState;
> >
> >  #define TYPE_VIRT_MACHINE   MACHINE_TYPE_NAME("virt")
> > +#define TYPE_VIRTV3_MACHINE MACHINE_TYPE_NAME("virt-v3")
> >  #define VIRT_MACHINE(obj) \
> >      OBJECT_CHECK(VirtMachineState, (obj), TYPE_VIRT_MACHINE)
> >  #define VIRT_MACHINE_GET_CLASS(obj) \
> > @@ -111,6 +113,7 @@ static const MemMapEntry a15memmap[] = {
> >      [VIRT_CPUPERIPHS] =         { 0x08000000, 0x00020000 },
> >      /* GIC distributor and CPU interfaces sit inside the CPU peripheral
> space */
> >      [VIRT_GIC_DIST] =           { 0x08000000, 0x00010000 },
> > +    /* Note on GICv3 VIRT_GIC_DIST_SPI takes place of VIRT_GIC_CPU */
>
>  Memory map of the "virt" machine already contains everything needed. BTW,
> what's "SPI"? "MBI", you meant? Well, we are going to
> have an ITS, so this simple message translator will not be needed.
>
Typo.

>
> >      [VIRT_GIC_CPU] =            { 0x08010000, 0x00010000 },
> >      [VIRT_GIC_V2M] =            { 0x08020000, 0x00001000 },
> >      /* The space in between here is reserved for GICv3 CPU/vCPU/HYP */
> > @@ -145,21 +148,26 @@ static VirtBoardInfo machines[] = {
> >          .cpu_model = "cortex-a15",
> >          .memmap = a15memmap,
> >          .irqmap = a15irqmap,
> > +        .class_name = TYPE_ARM_CPU,
> > +
> >      },
> >      {
> >          .cpu_model = "cortex-a53",
> >          .memmap = a15memmap,
> >          .irqmap = a15irqmap,
> > +        .class_name = TYPE_AARCH64_CPU,
> >      },
> >      {
> >          .cpu_model = "cortex-a57",
> >          .memmap = a15memmap,
> >          .irqmap = a15irqmap,
> > +        .class_name = TYPE_AARCH64_CPU,
> >      },
> >      {
> >          .cpu_model = "host",
> >          .memmap = a15memmap,
> >          .irqmap = a15irqmap,
> > +        .class_name = TYPE_ARM_CPU,
> >      },
> >  };
> >
> > @@ -228,7 +236,6 @@ static void fdt_add_psci_node(const VirtBoardInfo
> *vbi)
> >      if (armcpu->psci_version == 2) {
> >          const char comp[] = "arm,psci-0.2\0arm,psci";
> >          qemu_fdt_setprop(fdt, "/psci", "compatible", comp,
> sizeof(comp));
> > -
> >          cpu_off_fn = QEMU_PSCI_0_2_FN_CPU_OFF;
> >          if (arm_feature(&armcpu->env, ARM_FEATURE_AARCH64)) {
> >              cpu_suspend_fn = QEMU_PSCI_0_2_FN64_CPU_SUSPEND;
> > @@ -241,7 +248,6 @@ static void fdt_add_psci_node(const VirtBoardInfo
> *vbi)
> >          }
> >      } else {
> >          qemu_fdt_setprop_string(fdt, "/psci", "compatible", "arm,psci");
> > -
> >          cpu_suspend_fn = QEMU_PSCI_0_1_FN_CPU_SUSPEND;
> >          cpu_off_fn = QEMU_PSCI_0_1_FN_CPU_OFF;
> >          cpu_on_fn = QEMU_PSCI_0_1_FN_CPU_ON;
> > @@ -274,6 +280,12 @@ static void fdt_add_timer_nodes(const VirtBoardInfo
> *vbi, int gictype)
> >          irqflags = deposit32(irqflags, GIC_FDT_IRQ_PPI_CPU_START,
> >                               GIC_FDT_IRQ_PPI_CPU_WIDTH,
> >                               (1 << vbi->smp_cpus) - 1);
> > +    } else if (gictype == 3) {
> > +        uint32_t max;
> > +        /* Argument is 32 bit but 8 bits are reserved for flags */
> > +        max = (vbi->smp_cpus >= 24) ? 24 : vbi->smp_cpus;
> > +        irqflags = deposit32(irqflags, GIC_FDT_IRQ_PPI_CPU_START,
> > +                             GICV3_FDT_IRQ_PPI_CPU_WIDTH, (1 << max) -
> 1);
> >      }
> >
> >      qemu_fdt_add_subnode(vbi->fdt, "/timer");
> > @@ -429,12 +441,32 @@ static void create_gic(VirtBoardInfo *vbi,
> qemu_irq *pic, int type, bool
> > secure)
> >      gicdev = qdev_create(NULL, gictype);
> >      qdev_prop_set_uint32(gicdev, "revision", type);
> >      qdev_prop_set_uint32(gicdev, "num-cpu", smp_cpus);
> > +
> > +#ifdef TARGET_AARCH64
> > +    if (type == 3) {
> > +        qdev_prop_set_uint32(gicdev, "len-mp-affinity", smp_cpus);
> > +        /* Connect GIC to CPU */
> > +        for (i = 0; i < smp_cpus; i++) {
> > +            char *propname;
> > +            CPUState *cpu = qemu_get_cpu(i);
> > +            ARMCPU *armcpu = ARM_CPU(cpu);
> > +            propname = g_strdup_printf("mp-affinity[%d]", i);
> > +            qdev_prop_set_uint64(gicdev, propname, armcpu->mp_affinity);
> > +            g_free(propname);
> > +        }
> > +    }
> > +#endif
> >      /* Note that the num-irq property counts both internal and external
> >       * interrupts; there are always 32 of the former (mandated by GIC
> spec).
> >       */
> >      qdev_prop_set_uint32(gicdev, "num-irq", NUM_IRQS + 32);
> >      if (!kvm_irqchip_in_kernel()) {
> > -        qdev_prop_set_bit(gicdev, "has-security-extensions", secure);
> > +        if (type == 3) {
> > +            /* AARCH64 has 4 security levels */
> > +            qdev_prop_set_uint8(gicdev, "security-levels", secure ? 1 :
> 0);
> > +        } else {
> > +            qdev_prop_set_bit(gicdev, "has-security-extensions",
> secure);
> > +        }
> >      }
> >      qdev_init_nofail(gicdev);
> >      gicbusdev = SYS_BUS_DEVICE(gicdev);
> > @@ -445,6 +477,16 @@ static void create_gic(VirtBoardInfo *vbi, qemu_irq
> *pic, int type, bool
> > secure)
> >          sysbus_mmio_map(gicbusdev, 1, vbi->memmap[VIRT_GIC_CPU].base);
> >      }
> >
> > +#ifdef TARGET_AARCH64
> > +    if (type == 3) {
> > +        /* Connect GIC to CPU */
> > +        for (i = 0; i < smp_cpus; i++) {
> > +            CPUState *cpu = qemu_get_cpu(i);
> > +            aarch64_registers_with_opaque_set(OBJECT(cpu), (void
> *)gicdev);
> > +        }
> > +    }
> > +#endif
> > +
> >      /* Wire the outputs from each CPU's generic timer to the
> >       * appropriate GIC PPI inputs, and the GIC's IRQ output to
> >       * the CPU's IRQ input.
> > @@ -466,7 +508,7 @@ static void create_gic(VirtBoardInfo *vbi, qemu_irq
> *pic, int type, bool
> > secure)
> >          for (irq = 0; irq < ARRAY_SIZE(timer_irq); irq++) {
> >              qdev_connect_gpio_out(cpudev, irq,
> >                                    qdev_get_gpio_in(gicdev,
> > -                                                   ppibase +
> timer_irq[irq]));
> > +                                  ppibase + timer_irq[irq]));
> >          }
> >
> >          sysbus_connect_irq(gicbusdev, i, qdev_get_gpio_in(cpudev,
> ARM_CPU_IRQ));
> > @@ -967,7 +1009,7 @@ static void machvirt_init(MachineState *machine)
> >      create_fdt(vbi);
> >
> >      for (n = 0; n < smp_cpus; n++) {
> > -        ObjectClass *oc = cpu_class_by_name(TYPE_ARM_CPU, cpustr[0]);
> > +        ObjectClass *oc = cpu_class_by_name(vbi->class_name, cpustr[0]);
> >          CPUClass *cc = CPU_CLASS(oc);
> >          Object *cpuobj;
> >          Error *err = NULL;
> > @@ -1116,7 +1158,7 @@ static void virt_set_gic_version(Object *obj,
> const char *value, Error
> > **errp)
> >      }
> >  }
> >
> > -static void virt_instance_init(Object *obj)
> > +static void virt_instance_init_common(Object *obj, int32_t version)
> >  {
> >      VirtMachineState *vms = VIRT_MACHINE(obj);
> >
> > @@ -1140,8 +1182,7 @@ static void virt_instance_init(Object *obj)
> >                                      "Set on/off to enable/disable using
> "
> >                                      "physical address space above 32
> bits",
> >                                      NULL);
> > -    /* Default GIC type is v2 */
> > -    vms->gic_version = 2;
> > +    vms->gic_version = version;
> >      object_property_add_str(obj, "gic-version", virt_get_gic_version,
> >                          virt_set_gic_version, NULL);
> >      object_property_set_description(obj, "gic-version",
> > @@ -1149,6 +1190,12 @@ static void virt_instance_init(Object *obj)
> >                                      "Valid values are 2, 3 and host",
> NULL);
> >  }
> >
> > +static void virt_instance_init(Object *obj)
> > +{
> > +    /* Default GIC type is v2 */
> > +    virt_instance_init_common(obj, 2);
> > +}
> > +
> >  static void virt_class_init(ObjectClass *oc, void *data)
> >  {
> >      MachineClass *mc = MACHINE_CLASS(oc);
> > @@ -1174,9 +1221,33 @@ static const TypeInfo machvirt_info = {
> >      .class_init = virt_class_init,
> >  };
> >
> > +static void virtv3_instance_init(Object *obj)
> > +{
> > +    virt_instance_init_common(obj, 3);
> > +}
> > +
> > +static void virtv3_class_init(ObjectClass *oc, void *data)
> > +{
> > +    MachineClass *mc = MACHINE_CLASS(oc);
> > +
> > +    mc->name = TYPE_VIRTV3_MACHINE;
> > +    mc->desc = "ARM Virtual Machine with GICv3",
> > +    mc->init = machvirt_init;
> > +}
> > +
> > +static const TypeInfo machvirtv3_info = {
> > +    .name = TYPE_VIRTV3_MACHINE,
> > +    .parent = TYPE_VIRT_MACHINE,
> > +    .instance_size = sizeof(VirtMachineState),
> > +    .instance_init = virtv3_instance_init,
> > +    .class_size = sizeof(VirtMachineClass),
> > +    .class_init = virtv3_class_init,
> > +};
> > +
> >  static void machvirt_machine_init(void)
> >  {
> >      type_register_static(&machvirt_info);
> > +    type_register_static(&machvirtv3_info);
> >  }
>
>  As i said before, this stuff is simply not needed.
>
> >
> >  machine_init(machvirt_machine_init);
> > diff --git a/include/hw/arm/fdt.h b/include/hw/arm/fdt.h
> > index c3d5015..ad8f85c 100644
> > --- a/include/hw/arm/fdt.h
> > +++ b/include/hw/arm/fdt.h
> > @@ -30,5 +30,7 @@
> >
> >  #define GIC_FDT_IRQ_PPI_CPU_START 8
> >  #define GIC_FDT_IRQ_PPI_CPU_WIDTH 8
> > +#define GICV3_FDT_IRQ_PPI_CPU_WIDTH 24
> > +
> >
> >  #endif
> > diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
> > index f464586..53ff50a 100644
> > --- a/include/hw/arm/virt.h
> > +++ b/include/hw/arm/virt.h
> > @@ -46,6 +46,7 @@ enum {
> >      VIRT_CPUPERIPHS,
> >      VIRT_GIC_DIST,
> >      VIRT_GIC_CPU,
> > +    VIRT_GIC_DIST_SPI = VIRT_GIC_CPU,
> >      VIRT_GIC_V2M,
> >      VIRT_GIC_ITS,
> >      VIRT_GIC_REDIST,
> > diff --git a/target-arm/machine.c b/target-arm/machine.c
> > index 36a0d15..33f28be 100644
> > --- a/target-arm/machine.c
> > +++ b/target-arm/machine.c
> > @@ -341,7 +341,12 @@ const char *gicv3_class_name(void)
> >  #endif
> >      } else {
> >          /* TODO: Software emulation is not implemented yet */
> > -        error_report("KVM is currently required for GICv3 emulation\n");
> > +#ifdef TARGET_AARCH64
> > +        return "arm_gicv3";
>
>  Peter told me, there is a policy to use dash ("-") in class names. So
> "arm-gicv3".
>  By the way, why does it depend on TARGET_AARCH64? Actually, TARGET_ARM
> and TARGET_AARCH64 are the same. You can make both emulators
> running both 64- and 32-bit code. KVM code depends on TARGET_AARCH64 only
> because some definitions are missing on 32-bit kernels.
> You don't have this restriction, so you don't need this #ifdef.
>

O.K. I'll change to dash.
GICV3 is accessed by system instructions that exists only in ARCH64.


> > +#else
> > +        error_report("KVM GICv3 acceleration is not supported on this "
> > +                     "platform\n");
>
>  Why "KVM" here? Well, rubbish anyway because of the above. :)
>
> > +#endif
> >      }
> >
> >      exit(1);
> > --
> > 1.9.1
>
> Kind regards,
> Pavel Fedin
> Expert Engineer
> Samsung Electronics Research center Russia
>
>

[-- Attachment #2: Type: text/html, Size: 17513 bytes --]

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [Qemu-devel] [PATCH RFC V5 9/9] hw/arm: Add virt-v3 machine that uses GIC-500
  2015-10-21  8:52     ` Shlomo Pongratz
@ 2015-10-21 10:30       ` Pavel Fedin
  2015-10-21 11:33         ` Shlomo Pongratz
  0 siblings, 1 reply; 41+ messages in thread
From: Pavel Fedin @ 2015-10-21 10:30 UTC (permalink / raw)
  To: 'Shlomo Pongratz'
  Cc: peter.maydell, eric.auger, 'Shlomo Pongratz',
	qemu-devel, shannon.zhao, ashoks, imammedo

 Hello!

> I see, just how do I pass the gic version from the command line?

 Easy. -machine virt,gic-version=3

> GICV3 is accessed by system instructions that exists only in ARCH64.

 Wrong. In 32-bit mode the CPU sees them as CP15 registers. See "8.5 AArch32 System register descriptions".
 I would say, system registers are nothing new, these are just renamed CP15 registers, with "CPn" prefix removed.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [Qemu-devel] [PATCH RFC V5 9/9] hw/arm: Add virt-v3 machine that uses GIC-500
  2015-10-21 10:30       ` Pavel Fedin
@ 2015-10-21 11:33         ` Shlomo Pongratz
  2015-10-21 12:24           ` Peter Maydell
  0 siblings, 1 reply; 41+ messages in thread
From: Shlomo Pongratz @ 2015-10-21 11:33 UTC (permalink / raw)
  To: Pavel Fedin
  Cc: peter.maydell, eric.auger, Shlomo Pongratz, qemu-devel,
	shannon.zhao, ashoks, imammedo

[-- Attachment #1: Type: text/plain, Size: 753 bytes --]

See inline

On Wednesday, October 21, 2015, Pavel Fedin <p.fedin@samsung.com> wrote:

>  Hello!
>
> > I see, just how do I pass the gic version from the command line?
>
>  Easy. -machine virt,gic-version=3
>
> > GICV3 is accessed by system instructions that exists only in ARCH64.
>
>  Wrong. In 32-bit mode the CPU sees them as CP15 registers. See "8.5
> AArch32 System register descriptions".
>  I would say, system registers are nothing new, these are just renamed
> CP15 registers, with "CPn" prefix removed.
>

I assume I can add the system registers to target-arm/cpu.c but I wonder if
someone really needs to simulate more than 8 AArch32 CPU(s)


> Kind regards,
> Pavel Fedin
> Expert Engineer
> Samsung Electronics Research center Russia
>
>
>

[-- Attachment #2: Type: text/html, Size: 1151 bytes --]

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [Qemu-devel] [PATCH RFC V5 9/9] hw/arm: Add virt-v3 machine that uses GIC-500
  2015-10-21 11:33         ` Shlomo Pongratz
@ 2015-10-21 12:24           ` Peter Maydell
  2015-10-21 13:01             ` Shlomo Pongratz
  0 siblings, 1 reply; 41+ messages in thread
From: Peter Maydell @ 2015-10-21 12:24 UTC (permalink / raw)
  To: Shlomo Pongratz
  Cc: eric.auger, Shlomo Pongratz, Pavel Fedin, qemu-devel,
	shannon.zhao, ashoks, imammedo

On 21 October 2015 at 12:33, Shlomo Pongratz <shlomopongratz@gmail.com> wrote:
> I assume I can add the system registers to target-arm/cpu.c but I wonder if
> someone really needs to simulate more than 8 AArch32 CPU(s)

The system register implementation belongs in the gic code, not
target-arm/. We already have support for devices that say
"I have some system registers, please add them to this CPU".

The mechanism is the same for system registers for both 32-bit
and 64-bit, incidentally.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [Qemu-devel] [PATCH RFC V5 6/9] hw/intc: arm_gicv3_spi_its
  2015-10-20 17:22 ` [Qemu-devel] [PATCH RFC V5 6/9] hw/intc: arm_gicv3_spi_its Shlomo Pongratz
@ 2015-10-21 12:43   ` Pavel Fedin
  2015-10-21 13:05     ` Shlomo Pongratz
  0 siblings, 1 reply; 41+ messages in thread
From: Pavel Fedin @ 2015-10-21 12:43 UTC (permalink / raw)
  To: 'Shlomo Pongratz', qemu-devel
  Cc: peter.maydell, eric.auger, 'Shlomo Pongratz',
	shannon.zhao, ashoks, imammedo

 Hello!

> This patch includes a placeholder code for future spi and its
> implementation.

 Forgot to comment on this. I see that here you are building an ITS into GIC as a monolithic thing. This can be wrong because we
could want to emulate platforms which have GICv3 but don't have ITS. I would suggest to implement ITS as a separate class, and i
have actually done it in http://lists.nongnu.org/archive/html/qemu-devel/2015-10/msg04613.html.
 So, i think we should not bother about ITS for now and suggest you to leave it out completely and focus on pure GICv3.
 Even more, i have software-emulated ITS code in one of my abandoned branches, it lacks only GICv3's ability to receive LPIs (which
is indeed GIC's functionality). After you're done with GIC, i could rebase it on top and post at least as an RFC. May be i'll
complete it myself, may be you'll want to pick it up, i don't know. Actually, it works with Linux kernel and successfully translates
an MSI event into LPI number, which is then injected into the GIC object. Just it's GIC missing the appropriate LPI hanlders. I
wrote it when there was no in-kernel vITS implementation, but abandoned when vITS patch series was published.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [Qemu-devel] [PATCH RFC V5 9/9] hw/arm: Add virt-v3 machine that uses GIC-500
  2015-10-21 12:24           ` Peter Maydell
@ 2015-10-21 13:01             ` Shlomo Pongratz
  2015-10-21 13:24               ` Peter Maydell
  2015-10-21 13:26               ` Pavel Fedin
  0 siblings, 2 replies; 41+ messages in thread
From: Shlomo Pongratz @ 2015-10-21 13:01 UTC (permalink / raw)
  To: Peter Maydell
  Cc: eric.auger, Shlomo Pongratz, Pavel Fedin, qemu-devel,
	shannon.zhao, ashoks, imammedo

[-- Attachment #1: Type: text/plain, Size: 880 bytes --]

On Wednesday, October 21, 2015, Peter Maydell <peter.maydell@linaro.org>
wrote:

> On 21 October 2015 at 12:33, Shlomo Pongratz <shlomopongratz@gmail.com
> <javascript:;>> wrote:
> > I assume I can add the system registers to target-arm/cpu.c but I wonder
> if
> > someone really needs to simulate more than 8 AArch32 CPU(s)
>
> The system register implementation belongs in the gic code, not
> target-arm/. We already have support for devices that say
> "I have some system registers, please add them to this CPU".
>
>
I don't understand.
The system registers are defined in ARM Architecture reference Manual.
It is true that the real implementation is in arm_gicv3_interrupts.c
But the crn, crm, op0, and op1 of the instructions are in CPU domain.


> The mechanism is the same for system registers for both 32-bit
> and 64-bit, incidentally.
>
> I agree.


> thanks
> -- PMM
>

[-- Attachment #2: Type: text/html, Size: 1569 bytes --]

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [Qemu-devel] [PATCH RFC V5 6/9] hw/intc: arm_gicv3_spi_its
  2015-10-21 12:43   ` Pavel Fedin
@ 2015-10-21 13:05     ` Shlomo Pongratz
  2015-10-21 13:12       ` Pavel Fedin
  0 siblings, 1 reply; 41+ messages in thread
From: Shlomo Pongratz @ 2015-10-21 13:05 UTC (permalink / raw)
  To: Pavel Fedin
  Cc: peter.maydell, eric.auger, Shlomo Pongratz, qemu-devel,
	shannon.zhao, ashoks, imammedo

[-- Attachment #1: Type: text/plain, Size: 1480 bytes --]

Hi,

I just added a placeholder, I didn't add any functionality.

On Wednesday, October 21, 2015, Pavel Fedin <p.fedin@samsung.com> wrote:

>  Hello!
>
> > This patch includes a placeholder code for future spi and its
> > implementation.
>
>  Forgot to comment on this. I see that here you are building an ITS into
> GIC as a monolithic thing. This can be wrong because we
> could want to emulate platforms which have GICv3 but don't have ITS. I
> would suggest to implement ITS as a separate class, and i
> have actually done it in
> http://lists.nongnu.org/archive/html/qemu-devel/2015-10/msg04613.html.
>  So, i think we should not bother about ITS for now and suggest you to
> leave it out completely and focus on pure GICv3.
>  Even more, i have software-emulated ITS code in one of my abandoned
> branches, it lacks only GICv3's ability to receive LPIs (which
> is indeed GIC's functionality). After you're done with GIC, i could rebase
> it on top and post at least as an RFC. May be i'll
> complete it myself, may be you'll want to pick it up, i don't know.
> Actually, it works with Linux kernel and successfully translates
> an MSI event into LPI number, which is then injected into the GIC object.
> Just it's GIC missing the appropriate LPI hanlders. I
> wrote it when there was no in-kernel vITS implementation, but abandoned
> when vITS patch series was published.
>
> Kind regards,
> Pavel Fedin
> Expert Engineer
> Samsung Electronics Research center Russia
>
>
>

[-- Attachment #2: Type: text/html, Size: 1881 bytes --]

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [Qemu-devel] [PATCH RFC V5 6/9] hw/intc: arm_gicv3_spi_its
  2015-10-21 13:05     ` Shlomo Pongratz
@ 2015-10-21 13:12       ` Pavel Fedin
  2015-10-21 13:14         ` Shlomo Pongratz
  0 siblings, 1 reply; 41+ messages in thread
From: Pavel Fedin @ 2015-10-21 13:12 UTC (permalink / raw)
  To: 'Shlomo Pongratz'
  Cc: peter.maydell, eric.auger, 'Shlomo Pongratz',
	qemu-devel, shannon.zhao, ashoks, imammedo

 Hello!

> I just added a placeholder, I didn't add any functionality.

 I see. Just wanted to say, that if we accept my proposal (implementing ITS as a separate object), then the only thing we would do with this placeholder is to remove it. It should go then to something like hw/intc/arm_gicv3_its.c and its object should inherit from TYPE_ARM_GICV3_ITS_COMMON.
 Or do you have some explicit reasons to have everything as a monolith?

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [Qemu-devel] [PATCH RFC V5 6/9] hw/intc: arm_gicv3_spi_its
  2015-10-21 13:12       ` Pavel Fedin
@ 2015-10-21 13:14         ` Shlomo Pongratz
  2015-10-21 13:48           ` Pavel Fedin
  0 siblings, 1 reply; 41+ messages in thread
From: Shlomo Pongratz @ 2015-10-21 13:14 UTC (permalink / raw)
  To: Pavel Fedin
  Cc: peter.maydell, eric.auger, Shlomo Pongratz, qemu-devel,
	shannon.zhao, ashoks, imammedo

[-- Attachment #1: Type: text/plain, Size: 748 bytes --]

On Wednesday, October 21, 2015, Pavel Fedin <p.fedin@samsung.com> wrote:

>  Hello!
>
> > I just added a placeholder, I didn't add any functionality.
>
>  I see. Just wanted to say, that if we accept my proposal (implementing
> ITS as a separate object), then the only thing we would do with this
> placeholder is to remove it. It should go then to something like
> hw/intc/arm_gicv3_its.c and its object should inherit from
> TYPE_ARM_GICV3_ITS_COMMON.
>  Or do you have some explicit reasons to have everything as a monolith?
>

No I just didn't want to have 3 stub files spi, its and its_control.
Do you suggest that I'll split it to 3 files?

>
> Kind regards,
> Pavel Fedin
> Expert Engineer
> Samsung Electronics Research center Russia
>
>
>

[-- Attachment #2: Type: text/html, Size: 1120 bytes --]

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [Qemu-devel] [PATCH RFC V5 9/9] hw/arm: Add virt-v3 machine that uses GIC-500
  2015-10-21 13:01             ` Shlomo Pongratz
@ 2015-10-21 13:24               ` Peter Maydell
  2015-10-21 13:26               ` Pavel Fedin
  1 sibling, 0 replies; 41+ messages in thread
From: Peter Maydell @ 2015-10-21 13:24 UTC (permalink / raw)
  To: Shlomo Pongratz
  Cc: eric.auger, Shlomo Pongratz, Pavel Fedin, qemu-devel,
	shannon.zhao, ashoks, imammedo

On 21 October 2015 at 14:01, Shlomo Pongratz <shlomopongratz@gmail.com> wrote:
> On Wednesday, October 21, 2015, Peter Maydell <peter.maydell@linaro.org>
> wrote:
>> The system register implementation belongs in the gic code, not
>> target-arm/. We already have support for devices that say
>> "I have some system registers, please add them to this CPU".
>>
>
> I don't understand.
> The system registers are defined in ARM Architecture reference Manual.
> It is true that the real implementation is in arm_gicv3_interrupts.c
> But the crn, crm, op0, and op1 of the instructions are in CPU domain.

Well, this comes down to "do you want to design the GICv3
emulation to preserve the split the hardware has between
the cpu interface and the GIC proper". In hardware there's
actually a defined protocol between the two, so you can
have CPUs from one implementor that talk to a GIC from
another implementor. For QEMU that seems like overkill,
as we will only ever have one GICv3 implementation and one
CPU implementation. So we should just have the GICv3
provide the CPU system register implementations. But the
code for those belongs in hw/intc/: that should call
the function for "add these system registers" which we
have already: define_arm_cp_regs_with_opaque().
(We use this in hw/arm/pxa2xx_pic, for instance.)

thanks
-- PMM

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [Qemu-devel] [PATCH RFC V5 9/9] hw/arm: Add virt-v3 machine that uses GIC-500
  2015-10-21 13:01             ` Shlomo Pongratz
  2015-10-21 13:24               ` Peter Maydell
@ 2015-10-21 13:26               ` Pavel Fedin
  2015-10-21 14:05                 ` Shlomo Pongratz
  1 sibling, 1 reply; 41+ messages in thread
From: Pavel Fedin @ 2015-10-21 13:26 UTC (permalink / raw)
  To: 'Shlomo Pongratz', 'Peter Maydell'
  Cc: eric.auger, 'Shlomo Pongratz',
	qemu-devel, shannon.zhao, ashoks, imammedo

 Hello!

>> The system register implementation belongs in the gic code, not
>> target-arm/. We already have support for devices that say
>> "I have some system registers, please add them to this CPU".

> I don't understand.
> The system registers are defined in ARM Architecture reference Manual.
> It is true that the real implementation is in arm_gicv3_interrupts.c
> But the crn, crm, op0, and op1 of the instructions are in CPU domain.

 Not really. If you take a closer look, you'll see that crn, crm, op1, op2 are the same for both ARM64 and ARM32. The only difference is that ARM64 uses op0 = 3, and ARM32 uses cp = 15. Both of these specifiers can coexist in the descriptor table.
 I think Peter wants to tell that you should not insert your register table and registration function into target-arm/cpu64.c. This code should go to hw/intc/arm_gicv3_cpu_interface.c, add .cp = 15, and - voila - it magically works with both ARM32 and ARM64.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [Qemu-devel] [PATCH RFC V5 6/9] hw/intc: arm_gicv3_spi_its
  2015-10-21 13:14         ` Shlomo Pongratz
@ 2015-10-21 13:48           ` Pavel Fedin
  2015-10-21 14:19             ` Shlomo Pongratz
  0 siblings, 1 reply; 41+ messages in thread
From: Pavel Fedin @ 2015-10-21 13:48 UTC (permalink / raw)
  To: 'Shlomo Pongratz'
  Cc: peter.maydell, eric.auger, 'Shlomo Pongratz',
	qemu-devel, shannon.zhao, ashoks, imammedo

 Hello!

>> Or do you have some explicit reasons to have everything as a monolith?
> No I just didn't want to have 3 stub files spi, its and its_control.
> Do you suggest that I'll split it to 3 files?

 You didn't understand my question. It's not about internal structure of ITS implementation. It is about GIC and ITS connection.
 Please review my KVM ITS RFC: http://lists.nongnu.org/archive/html/qemu-devel/2015-10/msg04613.html. You'll see that ITS is a separate object of a separate class, which can even be omitted at all, if machine model doesn't need it for some reason. So, i suggest that all the ITS code should go there, and it would be a completely separate entity, and a separate patch set, after your GICv3 is accepted. I will help you with this.
 Peter, i know you can be very busy, but, could you at least take a glance at my vITS v2 RFC structure and judge us? Should ITS + GICv3 be a monolithic object, or is my suggestion better?
 By the way, gicv3_init_irqs_and_mmio() expects only two regions, so it will not even pay attention to your stubs. You could patch it, of course, but... I don't think it's the good thing to do.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [Qemu-devel] [PATCH RFC V5 9/9] hw/arm: Add virt-v3 machine that uses GIC-500
  2015-10-21 13:26               ` Pavel Fedin
@ 2015-10-21 14:05                 ` Shlomo Pongratz
  0 siblings, 0 replies; 41+ messages in thread
From: Shlomo Pongratz @ 2015-10-21 14:05 UTC (permalink / raw)
  To: Pavel Fedin
  Cc: Peter Maydell, eric.auger, Shlomo Pongratz, qemu-devel,
	shannon.zhao, ashoks, imammedo

[-- Attachment #1: Type: text/plain, Size: 1164 bytes --]

O.K.

On Wednesday, October 21, 2015, Pavel Fedin <p.fedin@samsung.com> wrote:

>  Hello!
>
> >> The system register implementation belongs in the gic code, not
> >> target-arm/. We already have support for devices that say
> >> "I have some system registers, please add them to this CPU".
>
> > I don't understand.
> > The system registers are defined in ARM Architecture reference Manual.
> > It is true that the real implementation is in arm_gicv3_interrupts.c
> > But the crn, crm, op0, and op1 of the instructions are in CPU domain.
>
>  Not really. If you take a closer look, you'll see that crn, crm, op1, op2
> are the same for both ARM64 and ARM32. The only difference is that ARM64
> uses op0 = 3, and ARM32 uses cp = 15. Both of these specifiers can coexist
> in the descriptor table.
>  I think Peter wants to tell that you should not insert your register
> table and registration function into target-arm/cpu64.c. This code should
> go to hw/intc/arm_gicv3_cpu_interface.c, add .cp = 15, and - voila - it
> magically works with both ARM32 and ARM64.
>
> Kind regards,
> Pavel Fedin
> Expert Engineer
> Samsung Electronics Research center Russia
>
>
>

[-- Attachment #2: Type: text/html, Size: 1439 bytes --]

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [Qemu-devel] [PATCH RFC V5 6/9] hw/intc: arm_gicv3_spi_its
  2015-10-21 13:48           ` Pavel Fedin
@ 2015-10-21 14:19             ` Shlomo Pongratz
  2015-10-21 14:26               ` Pavel Fedin
  0 siblings, 1 reply; 41+ messages in thread
From: Shlomo Pongratz @ 2015-10-21 14:19 UTC (permalink / raw)
  To: Pavel Fedin
  Cc: peter.maydell, eric.auger, Shlomo Pongratz, qemu-devel,
	shannon.zhao, ashoks, imammedo

[-- Attachment #1: Type: text/plain, Size: 1705 bytes --]

Hi,

As far as I understand Figure 1 in GICv3 architecture document the
interrupts from device goes to the distributor and from it to the
re-distributors or from the deices via the ITS to the re-distributors.
So eventually ITS should be part of the GIC. That is if the ITS is a
different entity how do you see it work with the rest of the GIC?


On Wednesday, October 21, 2015, Pavel Fedin <p.fedin@samsung.com> wrote:

>  Hello!
>
> >> Or do you have some explicit reasons to have everything as a monolith?
> > No I just didn't want to have 3 stub files spi, its and its_control.
> > Do you suggest that I'll split it to 3 files?
>
>  You didn't understand my question. It's not about internal structure of
> ITS implementation. It is about GIC and ITS connection.
>  Please review my KVM ITS RFC:
> http://lists.nongnu.org/archive/html/qemu-devel/2015-10/msg04613.html.
> You'll see that ITS is a separate object of a separate class, which can
> even be omitted at all, if machine model doesn't need it for some reason.
> So, i suggest that all the ITS code should go there, and it would be a
> completely separate entity, and a separate patch set, after your GICv3 is
> accepted. I will help you with this.
>  Peter, i know you can be very busy, but, could you at least take a glance
> at my vITS v2 RFC structure and judge us? Should ITS + GICv3 be a
> monolithic object, or is my suggestion better?
>  By the way, gicv3_init_irqs_and_mmio() expects only two regions, so it
> will not even pay attention to your stubs. You could patch it, of course,
> but... I don't think it's the good thing to do.
>
> Kind regards,
> Pavel Fedin
> Expert Engineer
> Samsung Electronics Research center Russia
>
>
>

[-- Attachment #2: Type: text/html, Size: 2103 bytes --]

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [Qemu-devel] [PATCH RFC V5 6/9] hw/intc: arm_gicv3_spi_its
  2015-10-21 14:19             ` Shlomo Pongratz
@ 2015-10-21 14:26               ` Pavel Fedin
  2015-10-21 14:35                 ` Shlomo Pongratz
  0 siblings, 1 reply; 41+ messages in thread
From: Pavel Fedin @ 2015-10-21 14:26 UTC (permalink / raw)
  To: 'Shlomo Pongratz'
  Cc: peter.maydell, eric.auger, 'Shlomo Pongratz',
	qemu-devel, shannon.zhao, ashoks, imammedo

 Hello!

> As far as I understand Figure 1 in GICv3 architecture document the interrupts from device goes to the distributor and from it to the
> re-distributors or from the deices via the ITS to the re-distributors.
> So eventually ITS should be part of the GIC. That is if the ITS is a different entity how do you see it work with the rest of the GIC?

 Easy. As i said before, what ITS actually does is transforming device ID + event ID into a single LPI number. Then, an LPI goes to the redistributor. Therefore, all we need from GIC is a method like inject_lpi(int lpi). Well, may be a couple more, for moving LPIs between collections. But this is not a problem and makes up a nice interface.
 Even on the picture you mentioned ITS is a kind of separate thing. It can even be completely missing.

 Well, my reasons end here. If you still disagree... Do what you want then, it will not be a problem to remove these unused stubs.
 Peter, we are summoning you. :)

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [Qemu-devel] [PATCH RFC V5 6/9] hw/intc: arm_gicv3_spi_its
  2015-10-21 14:26               ` Pavel Fedin
@ 2015-10-21 14:35                 ` Shlomo Pongratz
  2015-10-21 14:41                   ` Pavel Fedin
  0 siblings, 1 reply; 41+ messages in thread
From: Shlomo Pongratz @ 2015-10-21 14:35 UTC (permalink / raw)
  To: Pavel Fedin
  Cc: peter.maydell, eric.auger, Shlomo Pongratz, qemu-devel,
	shannon.zhao, ashoks, imammedo

[-- Attachment #1: Type: text/plain, Size: 1236 bytes --]

I just wanted to understand. I don't have any preferences.

On Wednesday, October 21, 2015, Pavel Fedin <p.fedin@samsung.com> wrote:

>  Hello!
>
> > As far as I understand Figure 1 in GICv3 architecture document the
> interrupts from device goes to the distributor and from it to the
> > re-distributors or from the deices via the ITS to the re-distributors.
> > So eventually ITS should be part of the GIC. That is if the ITS is a
> different entity how do you see it work with the rest of the GIC?
>
>  Easy. As i said before, what ITS actually does is transforming device ID
> + event ID into a single LPI number. Then, an LPI goes to the
> redistributor. Therefore, all we need from GIC is a method like
> inject_lpi(int lpi). Well, may be a couple more, for moving LPIs between
> collections. But this is not a problem and makes up a nice interface.
>  Even on the picture you mentioned ITS is a kind of separate thing. It can
> even be completely missing.
>
>  Well, my reasons end here. If you still disagree... Do what you want
> then, it will not be a problem to remove these unused stubs.
>  Peter, we are summoning you. :)
>
> Kind regards,
> Pavel Fedin
> Expert Engineer
> Samsung Electronics Research center Russia
>
>
>

[-- Attachment #2: Type: text/html, Size: 1468 bytes --]

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [Qemu-devel] [PATCH RFC V5 6/9] hw/intc: arm_gicv3_spi_its
  2015-10-21 14:35                 ` Shlomo Pongratz
@ 2015-10-21 14:41                   ` Pavel Fedin
  2015-10-21 14:46                     ` Peter Maydell
  0 siblings, 1 reply; 41+ messages in thread
From: Pavel Fedin @ 2015-10-21 14:41 UTC (permalink / raw)
  To: 'Shlomo Pongratz'
  Cc: peter.maydell, eric.auger, 'Shlomo Pongratz',
	qemu-devel, shannon.zhao, ashoks, imammedo

 Hello!

> I just wanted to understand. I don't have any preferences.

 In other words, in short: spec says that ITS is optional, so we can implement it as a separate component, which gets attached to the GIC using some specified interface. It's not a problem to design such an interface. Actually, i believe real HW does the same thing.
 In my RFC i have implemented a part of this interface. My ITS class has gic-parent property, which is used to attach it to the GIC. KVM implementation fetches vGIC's fd from there, while software emulation can use it to call LPI methods on the GIC. The property is declared as implementation-specific only because it would have different object type, for additional fail-safety. Software-emulated ITS cannot be attached to KVM vGIC and vice versa, actually only because kernel guys don't want direct LPI injection.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [Qemu-devel] [PATCH RFC V5 6/9] hw/intc: arm_gicv3_spi_its
  2015-10-21 14:41                   ` Pavel Fedin
@ 2015-10-21 14:46                     ` Peter Maydell
  2015-10-21 14:54                       ` Pavel Fedin
  0 siblings, 1 reply; 41+ messages in thread
From: Peter Maydell @ 2015-10-21 14:46 UTC (permalink / raw)
  To: Pavel Fedin
  Cc: Eric Auger, Shlomo Pongratz, Shlomo Pongratz, QEMU Developers,
	Shannon Zhao, Ashok Kumar, Igor Mammedov

On 21 October 2015 at 15:41, Pavel Fedin <p.fedin@samsung.com> wrote:
>  Hello!
>
>> I just wanted to understand. I don't have any preferences.
>
>  In other words, in short: spec says that ITS is optional, so
> we can implement it as a separate component, which gets attached
> to the GIC using some specified interface. It's not a problem
> to design such an interface. Actually, i believe real HW does
> the same thing.

For hw it's a design choice: hardware can either have the ITS as
an extra component on the side that only talks to the
redistributors via (impdef) redistributor registers; or
it can be an integrated part of the GIC that's just directly
connected internally.

For QEMU we could in theory do either; I was leaning towards
direct connection just because on the KVM side the in-kernel
GIC isn't going to separate them out as two distinct things.
But even there we can as you say sensibly model the ITS
as its own QOM object with a fairly well-defined interface
to the rest of the GIC.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [Qemu-devel] [PATCH RFC V5 6/9] hw/intc: arm_gicv3_spi_its
  2015-10-21 14:46                     ` Peter Maydell
@ 2015-10-21 14:54                       ` Pavel Fedin
  0 siblings, 0 replies; 41+ messages in thread
From: Pavel Fedin @ 2015-10-21 14:54 UTC (permalink / raw)
  To: 'Peter Maydell'
  Cc: 'Eric Auger', 'Shlomo Pongratz',
	'Shlomo Pongratz', 'QEMU Developers',
	'Shannon Zhao', 'Ashok Kumar',
	'Igor Mammedov'

 Hello!

> For QEMU we could in theory do either; I was leaning towards
> direct connection just because on the KVM side the in-kernel
> GIC isn't going to separate them out as two distinct things.

 I'd say this is not entirely true. With KVM you still can have vGIC without vITS. Just don't set KVM_VGIC_V3_ADDR_TYPE_ITS and that's it. The only thing linked in is LPI support. With KVM you cannot have LPIs without vITS. Neither you can directly inject LPIs.
 So, in-kernel ITS is also optional.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [Qemu-devel] [PATCH RFC V5 8/9] target-arm/cpu64 GICv3 system instructions support
  2015-10-20 17:22 ` [Qemu-devel] [PATCH RFC V5 8/9] target-arm/cpu64 GICv3 system instructions support Shlomo Pongratz
@ 2015-10-22  7:33   ` Pavel Fedin
  2015-10-22  8:19     ` Shlomo Pongratz
  0 siblings, 1 reply; 41+ messages in thread
From: Pavel Fedin @ 2015-10-22  7:33 UTC (permalink / raw)
  To: 'Shlomo Pongratz', qemu-devel
  Cc: peter.maydell, eric.auger, 'Shlomo Pongratz',
	shannon.zhao, ashoks, imammedo

 Hello!

> -----Original Message-----
> From: Shlomo Pongratz [mailto:shlomopongratz@gmail.com]
> Sent: Tuesday, October 20, 2015 8:22 PM
> To: qemu-devel@nongnu.org
> Cc: p.fedin@samsung.com; peter.maydell@linaro.org; eric.auger@linaro.org;
> shannon.zhao@linaro.org; imammedo@redhat.com; ashoks@broadcom.com; Shlomo Pongratz
> Subject: [PATCH RFC V5 8/9] target-arm/cpu64 GICv3 system instructions support
> 
> From: Shlomo Pongratz <shlomo.pongratz@huawei.com>
> 
> Add system instructions used by the Linux (kernel) GICv3
> device driver
> 
> Signed-off-by: Shlomo Pongratz <shlomo.pongratz@huawei.com>
> ---
>  target-arm/cpu-qom.h |   1 +
>  target-arm/cpu.h     |  12 ++++++
>  target-arm/cpu64.c   | 118 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 131 insertions(+)
> 
> diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
> index 25fb1ce..6a50433 100644
> --- a/target-arm/cpu-qom.h
> +++ b/target-arm/cpu-qom.h
> @@ -220,6 +220,7 @@ hwaddr arm_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
> 
>  int arm_cpu_gdb_read_register(CPUState *cpu, uint8_t *buf, int reg);
>  int arm_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
> +void aarch64_registers_with_opaque_set(Object *obj, void *opaque);
> 
>  /* Callback functions for the generic timer's timers. */
>  void arm_gt_ptimer_cb(void *opaque);
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 3daa7f5..d561313 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -1034,6 +1034,18 @@ void armv7m_nvic_set_pending(void *opaque, int irq);
>  int armv7m_nvic_acknowledge_irq(void *opaque);
>  void armv7m_nvic_complete_irq(void *opaque, int irq);
> 
> +void armv8_gicv3_set_sgi(void *opaque, int cpuindex, uint64_t value);
> +uint64_t armv8_gicv3_acknowledge_irq(void *opaque, int cpuindex,
> +                              MemTxAttrs attrs);
> +void armv8_gicv3_complete_irq(void *opaque, int cpuindex, int irq,
> +                              MemTxAttrs attrs);
> +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);
> +uint64_t armv8_gicv3_get_igrpen1(void *opaque, int cpuindex);
> +void armv8_gicv3_set_igrpen1(void *opaque, int cpuindex, uint64_t igrpen1);
> +
>  /* Interface for defining coprocessor registers.
>   * Registers are defined in tables of arm_cp_reginfo structs
>   * which are passed to define_arm_cp_regs().
> diff --git a/target-arm/cpu64.c b/target-arm/cpu64.c
> index 63c8b1c..4224779 100644
> --- a/target-arm/cpu64.c
> +++ b/target-arm/cpu64.c
> @@ -45,6 +45,115 @@ static uint64_t a57_a53_l2ctlr_read(CPUARMState *env, const ARMCPRegInfo
> *ri)
>  }
>  #endif
> 
> +#ifndef CONFIG_USER_ONLY
> +static void sgi_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
> +{
> +    CPUState *cpu = ENV_GET_CPU(env);
> +    armv8_gicv3_set_sgi(ri->opaque, cpu->cpu_index, value);
> +}
> +
> +static uint64_t iar_read(CPUARMState *env, const ARMCPRegInfo *ri)
> +{
> +    uint64_t value;
> +    MemTxAttrs attrs;;
> +    CPUState *cpu = ENV_GET_CPU(env);
> +    attrs.secure = arm_is_secure_below_el3(env) ? 1 : 0;
> +    value = armv8_gicv3_acknowledge_irq(ri->opaque, cpu->cpu_index, attrs);
> +    return value;
> +}
> +
> +static void sre_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
> +{
> +    armv8_gicv3_set_sre(ri->opaque, value);
> +}
> +
> +static uint64_t sre_read(CPUARMState *env, const ARMCPRegInfo *ri)
> +{
> +    uint64_t value;
> +    value = armv8_gicv3_get_sre(ri->opaque);
> +    return value;
> +}
> +
> +static void eoir_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
> +{
> +    MemTxAttrs attrs;
> +    CPUState *cpu = ENV_GET_CPU(env);
> +    attrs.secure = arm_is_secure_below_el3(env) ? 1 : 0;
> +    armv8_gicv3_complete_irq(ri->opaque, cpu->cpu_index, value, attrs);
> +}
> +
> +static uint64_t pmr_read(CPUARMState *env, const ARMCPRegInfo *ri)
> +{
> +    uint64_t value;
> +    CPUState *cpu = ENV_GET_CPU(env);
> +    value = armv8_gicv3_get_priority_mask(ri->opaque, cpu->cpu_index);
> +    return value;
> +}
> +
> +static void pmr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
> +{
> +    CPUState *cpu = ENV_GET_CPU(env);
> +    armv8_gicv3_set_priority_mask(ri->opaque, cpu->cpu_index, value);
> +}
> +
> +static uint64_t igrpen1_read(CPUARMState *env, const ARMCPRegInfo *ri)
> +{
> +    uint64_t value;
> +    CPUState *cpu = ENV_GET_CPU(env);
> +    value = armv8_gicv3_get_igrpen1(ri->opaque, cpu->cpu_index);
> +    return value;
> +}
> +
> +static void igrpen1_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
> +{
> +    CPUState *cpu = ENV_GET_CPU(env);
> +    armv8_gicv3_set_igrpen1(ri->opaque, cpu->cpu_index, value);
> +}
> +#endif
> +
> +static const ARMCPRegInfo cortex_a57_a53_cp_with_opaque_reginfo[] = {
> +    { .name = "EIOR1_EL1", .state = ARM_CP_STATE_AA64,
> +#ifndef CONFIG_USER_ONLY
> +      .writefn = eoir_write,
> +#endif
> +      .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 12, .opc2 = 1,
> +      .access = PL1_W, .type = ARM_CP_SPECIAL, .resetvalue = 0 },
> +    { .name = "IAR1_EL1", .state = ARM_CP_STATE_AA64,
> +#ifndef CONFIG_USER_ONLY
> +      .readfn = iar_read,
> +#endif
> +      .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 12, .opc2 = 0,
> +      .access = PL1_R, .type = ARM_CP_SPECIAL, .resetvalue = 0 },
> +    { .name = "SGI1R_EL1", .state = ARM_CP_STATE_AA64,
> +#ifndef CONFIG_USER_ONLY
> +      .writefn = sgi_write,
> +#endif
> +      .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 11, .opc2 = 5,
> +      .access = PL1_RW, .type = ARM_CP_SPECIAL, .resetvalue = 0 },
> +    { .name = "PMR_EL1", .state = ARM_CP_STATE_AA64,
> +      .opc0 = 3, .opc1 = 0, .crn = 4, .crm = 6, .opc2 = 0,
> +#ifndef CONFIG_USER_ONLY
> +      .readfn = pmr_read, .writefn = pmr_write,
> +#endif
> +      .access = PL1_RW, .type = ARM_CP_SPECIAL, .resetvalue = 0 },
> +    { .name = "CTLR_EL1", .state = ARM_CP_STATE_AA64,
> +      .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 12, .opc2 = 4,
> +      .access = PL1_RW, .type = ARM_CP_SPECIAL, .resetvalue = 0 },
> +    { .name = "SRE_EL1", .state = ARM_CP_STATE_AA64,
> +      .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 12, .opc2 = 5, .resetvalue = 0,
> +#ifndef CONFIG_USER_ONLY
> +      .readfn = sre_read, .writefn = sre_write,
> +#endif
> +      .access = PL1_RW, .type = ARM_CP_SPECIAL, .resetvalue = 0 },
> +    { .name = "IGRPEN1_EL1", .state = ARM_CP_STATE_AA64,
> +      .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 12, .opc2 = 7,
> +#ifndef CONFIG_USER_ONLY
> +      .readfn = igrpen1_read, .writefn = igrpen1_write,
> +#endif
> +      .access = PL1_RW, .type = ARM_CP_SPECIAL, .resetvalue = 0 },
> +    REGINFO_SENTINEL
> +};
> +

 One more note on this table, which i previously forgot. We tried to run EFI here, and it crashes with your emulation because when
setting up GICv3 it also pokes BPR1_EL1 register. You should implement it.

>  static const ARMCPRegInfo cortex_a57_a53_cp_reginfo[] = {
>  #ifndef CONFIG_USER_ONLY
>      { .name = "L2CTLR_EL1", .state = ARM_CP_STATE_AA64,
> @@ -258,6 +367,15 @@ static void aarch64_cpu_set_aarch64(Object *obj, bool value, Error **errp)
>      }
>  }
> 
> +void aarch64_registers_with_opaque_set(Object *obj, void *opaque)
> +{
> +    ARMCPU *cpu = ARM_CPU(obj);
> +
> +    define_arm_cp_regs_with_opaque(cpu,
> +                                   cortex_a57_a53_cp_with_opaque_reginfo,
> +                                   opaque);
> +}
> +
>  static void aarch64_cpu_initfn(Object *obj)
>  {
>      object_property_add_bool(obj, "aarch64", aarch64_cpu_get_aarch64,
> --
> 1.9.1

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [Qemu-devel] [PATCH RFC V5 8/9] target-arm/cpu64 GICv3 system instructions support
  2015-10-22  7:33   ` Pavel Fedin
@ 2015-10-22  8:19     ` Shlomo Pongratz
  2015-10-22  8:33       ` Pavel Fedin
  0 siblings, 1 reply; 41+ messages in thread
From: Shlomo Pongratz @ 2015-10-22  8:19 UTC (permalink / raw)
  To: Pavel Fedin
  Cc: peter.maydell, eric.auger, Shlomo Pongratz, qemu-devel,
	shannon.zhao, ashoks, imammedo

[-- Attachment #1: Type: text/plain, Size: 8673 bytes --]

Hi

On Thursday, October 22, 2015, Pavel Fedin <p.fedin@samsung.com> wrote:

>  Hello!
>
> > -----Original Message-----
> > From: Shlomo Pongratz [mailto:shlomopongratz@gmail.com <javascript:;>]
> > Sent: Tuesday, October 20, 2015 8:22 PM
> > To: qemu-devel@nongnu.org <javascript:;>
> > Cc: p.fedin@samsung.com <javascript:;>; peter.maydell@linaro.org
> <javascript:;>; eric.auger@linaro.org <javascript:;>;
> > shannon.zhao@linaro.org <javascript:;>; imammedo@redhat.com
> <javascript:;>; ashoks@broadcom.com <javascript:;>; Shlomo Pongratz
> > Subject: [PATCH RFC V5 8/9] target-arm/cpu64 GICv3 system instructions
> support
> >
> > From: Shlomo Pongratz <shlomo.pongratz@huawei.com <javascript:;>>
> >
> > Add system instructions used by the Linux (kernel) GICv3
> > device driver
> >
> > Signed-off-by: Shlomo Pongratz <shlomo.pongratz@huawei.com
> <javascript:;>>
> > ---
> >  target-arm/cpu-qom.h |   1 +
> >  target-arm/cpu.h     |  12 ++++++
> >  target-arm/cpu64.c   | 118
> +++++++++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 131 insertions(+)
> >
> > diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
> > index 25fb1ce..6a50433 100644
> > --- a/target-arm/cpu-qom.h
> > +++ b/target-arm/cpu-qom.h
> > @@ -220,6 +220,7 @@ hwaddr arm_cpu_get_phys_page_debug(CPUState *cpu,
> vaddr addr);
> >
> >  int arm_cpu_gdb_read_register(CPUState *cpu, uint8_t *buf, int reg);
> >  int arm_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
> > +void aarch64_registers_with_opaque_set(Object *obj, void *opaque);
> >
> >  /* Callback functions for the generic timer's timers. */
> >  void arm_gt_ptimer_cb(void *opaque);
> > diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> > index 3daa7f5..d561313 100644
> > --- a/target-arm/cpu.h
> > +++ b/target-arm/cpu.h
> > @@ -1034,6 +1034,18 @@ void armv7m_nvic_set_pending(void *opaque, int
> irq);
> >  int armv7m_nvic_acknowledge_irq(void *opaque);
> >  void armv7m_nvic_complete_irq(void *opaque, int irq);
> >
> > +void armv8_gicv3_set_sgi(void *opaque, int cpuindex, uint64_t value);
> > +uint64_t armv8_gicv3_acknowledge_irq(void *opaque, int cpuindex,
> > +                              MemTxAttrs attrs);
> > +void armv8_gicv3_complete_irq(void *opaque, int cpuindex, int irq,
> > +                              MemTxAttrs attrs);
> > +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);
> > +uint64_t armv8_gicv3_get_igrpen1(void *opaque, int cpuindex);
> > +void armv8_gicv3_set_igrpen1(void *opaque, int cpuindex, uint64_t
> igrpen1);
> > +
> >  /* Interface for defining coprocessor registers.
> >   * Registers are defined in tables of arm_cp_reginfo structs
> >   * which are passed to define_arm_cp_regs().
> > diff --git a/target-arm/cpu64.c b/target-arm/cpu64.c
> > index 63c8b1c..4224779 100644
> > --- a/target-arm/cpu64.c
> > +++ b/target-arm/cpu64.c
> > @@ -45,6 +45,115 @@ static uint64_t a57_a53_l2ctlr_read(CPUARMState
> *env, const ARMCPRegInfo
> > *ri)
> >  }
> >  #endif
> >
> > +#ifndef CONFIG_USER_ONLY
> > +static void sgi_write(CPUARMState *env, const ARMCPRegInfo *ri,
> uint64_t value)
> > +{
> > +    CPUState *cpu = ENV_GET_CPU(env);
> > +    armv8_gicv3_set_sgi(ri->opaque, cpu->cpu_index, value);
> > +}
> > +
> > +static uint64_t iar_read(CPUARMState *env, const ARMCPRegInfo *ri)
> > +{
> > +    uint64_t value;
> > +    MemTxAttrs attrs;;
> > +    CPUState *cpu = ENV_GET_CPU(env);
> > +    attrs.secure = arm_is_secure_below_el3(env) ? 1 : 0;
> > +    value = armv8_gicv3_acknowledge_irq(ri->opaque, cpu->cpu_index,
> attrs);
> > +    return value;
> > +}
> > +
> > +static void sre_write(CPUARMState *env, const ARMCPRegInfo *ri,
> uint64_t value)
> > +{
> > +    armv8_gicv3_set_sre(ri->opaque, value);
> > +}
> > +
> > +static uint64_t sre_read(CPUARMState *env, const ARMCPRegInfo *ri)
> > +{
> > +    uint64_t value;
> > +    value = armv8_gicv3_get_sre(ri->opaque);
> > +    return value;
> > +}
> > +
> > +static void eoir_write(CPUARMState *env, const ARMCPRegInfo *ri,
> uint64_t value)
> > +{
> > +    MemTxAttrs attrs;
> > +    CPUState *cpu = ENV_GET_CPU(env);
> > +    attrs.secure = arm_is_secure_below_el3(env) ? 1 : 0;
> > +    armv8_gicv3_complete_irq(ri->opaque, cpu->cpu_index, value, attrs);
> > +}
> > +
> > +static uint64_t pmr_read(CPUARMState *env, const ARMCPRegInfo *ri)
> > +{
> > +    uint64_t value;
> > +    CPUState *cpu = ENV_GET_CPU(env);
> > +    value = armv8_gicv3_get_priority_mask(ri->opaque, cpu->cpu_index);
> > +    return value;
> > +}
> > +
> > +static void pmr_write(CPUARMState *env, const ARMCPRegInfo *ri,
> uint64_t value)
> > +{
> > +    CPUState *cpu = ENV_GET_CPU(env);
> > +    armv8_gicv3_set_priority_mask(ri->opaque, cpu->cpu_index, value);
> > +}
> > +
> > +static uint64_t igrpen1_read(CPUARMState *env, const ARMCPRegInfo *ri)
> > +{
> > +    uint64_t value;
> > +    CPUState *cpu = ENV_GET_CPU(env);
> > +    value = armv8_gicv3_get_igrpen1(ri->opaque, cpu->cpu_index);
> > +    return value;
> > +}
> > +
> > +static void igrpen1_write(CPUARMState *env, const ARMCPRegInfo *ri,
> uint64_t value)
> > +{
> > +    CPUState *cpu = ENV_GET_CPU(env);
> > +    armv8_gicv3_set_igrpen1(ri->opaque, cpu->cpu_index, value);
> > +}
> > +#endif
> > +
> > +static const ARMCPRegInfo cortex_a57_a53_cp_with_opaque_reginfo[] = {
> > +    { .name = "EIOR1_EL1", .state = ARM_CP_STATE_AA64,
> > +#ifndef CONFIG_USER_ONLY
> > +      .writefn = eoir_write,
> > +#endif
> > +      .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 12, .opc2 = 1,
> > +      .access = PL1_W, .type = ARM_CP_SPECIAL, .resetvalue = 0 },
> > +    { .name = "IAR1_EL1", .state = ARM_CP_STATE_AA64,
> > +#ifndef CONFIG_USER_ONLY
> > +      .readfn = iar_read,
> > +#endif
> > +      .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 12, .opc2 = 0,
> > +      .access = PL1_R, .type = ARM_CP_SPECIAL, .resetvalue = 0 },
> > +    { .name = "SGI1R_EL1", .state = ARM_CP_STATE_AA64,
> > +#ifndef CONFIG_USER_ONLY
> > +      .writefn = sgi_write,
> > +#endif
> > +      .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 11, .opc2 = 5,
> > +      .access = PL1_RW, .type = ARM_CP_SPECIAL, .resetvalue = 0 },
> > +    { .name = "PMR_EL1", .state = ARM_CP_STATE_AA64,
> > +      .opc0 = 3, .opc1 = 0, .crn = 4, .crm = 6, .opc2 = 0,
> > +#ifndef CONFIG_USER_ONLY
> > +      .readfn = pmr_read, .writefn = pmr_write,
> > +#endif
> > +      .access = PL1_RW, .type = ARM_CP_SPECIAL, .resetvalue = 0 },
> > +    { .name = "CTLR_EL1", .state = ARM_CP_STATE_AA64,
> > +      .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 12, .opc2 = 4,
> > +      .access = PL1_RW, .type = ARM_CP_SPECIAL, .resetvalue = 0 },
> > +    { .name = "SRE_EL1", .state = ARM_CP_STATE_AA64,
> > +      .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 12, .opc2 = 5,
> .resetvalue = 0,
> > +#ifndef CONFIG_USER_ONLY
> > +      .readfn = sre_read, .writefn = sre_write,
> > +#endif
> > +      .access = PL1_RW, .type = ARM_CP_SPECIAL, .resetvalue = 0 },
> > +    { .name = "IGRPEN1_EL1", .state = ARM_CP_STATE_AA64,
> > +      .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 12, .opc2 = 7,
> > +#ifndef CONFIG_USER_ONLY
> > +      .readfn = igrpen1_read, .writefn = igrpen1_write,
> > +#endif
> > +      .access = PL1_RW, .type = ARM_CP_SPECIAL, .resetvalue = 0 },
> > +    REGINFO_SENTINEL
> > +};
> > +
>
>  One more note on this table, which i previously forgot. We tried to run
> EFI here, and it crashes with your emulation because when
> setting up GICv3 it also pokes BPR1_EL1 register. You should implement it.
>

I've implemented the registers accessed by Linux driver in
drivers/irqchip/irq-gic-v3.c
If this register is used only with KVM e.g. virt/kvm/arm/vgic-v3.c than it
is out of my mandate.



> >  static const ARMCPRegInfo cortex_a57_a53_cp_reginfo[] = {
> >  #ifndef CONFIG_USER_ONLY
> >      { .name = "L2CTLR_EL1", .state = ARM_CP_STATE_AA64,
> > @@ -258,6 +367,15 @@ static void aarch64_cpu_set_aarch64(Object *obj,
> bool value, Error **errp)
> >      }
> >  }
> >
> > +void aarch64_registers_with_opaque_set(Object *obj, void *opaque)
> > +{
> > +    ARMCPU *cpu = ARM_CPU(obj);
> > +
> > +    define_arm_cp_regs_with_opaque(cpu,
> > +
>  cortex_a57_a53_cp_with_opaque_reginfo,
> > +                                   opaque);
> > +}
> > +
> >  static void aarch64_cpu_initfn(Object *obj)
> >  {
> >      object_property_add_bool(obj, "aarch64", aarch64_cpu_get_aarch64,
> > --
> > 1.9.1
>
> Kind regards,
> Pavel Fedin
> Expert Engineer
> Samsung Electronics Research center Russia
>
>
>

[-- Attachment #2: Type: text/html, Size: 11434 bytes --]

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [Qemu-devel] [PATCH RFC V5 8/9] target-arm/cpu64 GICv3 system instructions support
  2015-10-22  8:19     ` Shlomo Pongratz
@ 2015-10-22  8:33       ` Pavel Fedin
  2015-10-22  8:52         ` Shlomo Pongratz
  0 siblings, 1 reply; 41+ messages in thread
From: Pavel Fedin @ 2015-10-22  8:33 UTC (permalink / raw)
  To: 'Shlomo Pongratz'
  Cc: peter.maydell, eric.auger, 'Shlomo Pongratz',
	qemu-devel, shannon.zhao, ashoks, imammedo

 Hello!

> I've implemented the registers accessed by Linux driver in drivers/irqchip/irq-gic-v3.c
> If this register is used only with KVM e.g. virt/kvm/arm/vgic-v3.c than it is out of my mandate.

 It has nothing to do with KVM. EFI is a firmware, which originates from Intel, but now adopted by ARM64 architecture too. You can also run it under qemu, if you want to make kind of "full" machine. And it writes some value to BPR1, which is indeed ignored by Linux kernel.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [Qemu-devel] [PATCH RFC V5 8/9] target-arm/cpu64 GICv3 system instructions support
  2015-10-22  8:33       ` Pavel Fedin
@ 2015-10-22  8:52         ` Shlomo Pongratz
  2015-10-22 10:05           ` Pavel Fedin
  0 siblings, 1 reply; 41+ messages in thread
From: Shlomo Pongratz @ 2015-10-22  8:52 UTC (permalink / raw)
  To: Pavel Fedin
  Cc: peter.maydell, eric.auger, Shlomo Pongratz, qemu-devel,
	shannon.zhao, ashoks, imammedo

[-- Attachment #1: Type: text/plain, Size: 735 bytes --]

Hi

On Thursday, October 22, 2015, Pavel Fedin <p.fedin@samsung.com> wrote:

>  Hello!
>
> > I've implemented the registers accessed by Linux driver in
> drivers/irqchip/irq-gic-v3.c
> > If this register is used only with KVM e.g. virt/kvm/arm/vgic-v3.c than
> it is out of my mandate.
>
>  It has nothing to do with KVM. EFI is a firmware, which originates from
> Intel, but now adopted by ARM64 architecture too. You can also run it under
> qemu, if you want to make kind of "full" machine. And it writes some value
> to BPR1, which is indeed ignored by Linux kernel.
>
>
So were is it used in QEMU?
Which machine in hw/arm needs it?


> Kind regards,
> Pavel Fedin
> Expert Engineer
> Samsung Electronics Research center Russia
>
>

[-- Attachment #2: Type: text/html, Size: 1114 bytes --]

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [Qemu-devel] [PATCH RFC V5 8/9] target-arm/cpu64 GICv3 system instructions support
  2015-10-22  8:52         ` Shlomo Pongratz
@ 2015-10-22 10:05           ` Pavel Fedin
  2015-10-22 10:45             ` Shlomo Pongratz
  0 siblings, 1 reply; 41+ messages in thread
From: Pavel Fedin @ 2015-10-22 10:05 UTC (permalink / raw)
  To: 'Shlomo Pongratz'
  Cc: peter.maydell, eric.auger, 'Shlomo Pongratz',
	qemu-devel, shannon.zhao, ashoks, imammedo

 Hello!

>> It has nothing to do with KVM. EFI is a firmware, which originates from Intel, but now adopted by ARM64 architecture too. You can also run
>> it under qemu, if you want to make kind of "full" machine. And it writes some value to BPR1, which is indeed ignored by Linux kernel.
 
> So were is it used in QEMU?

You can configure any ARM machine to use firmware blob instead of kernel + device tree. This way it looks more like a real machine.

> Which machine in hw/arm needs it?

 We ran it on virt, for example.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [Qemu-devel] [PATCH RFC V5 8/9] target-arm/cpu64 GICv3 system instructions support
  2015-10-22 10:05           ` Pavel Fedin
@ 2015-10-22 10:45             ` Shlomo Pongratz
  0 siblings, 0 replies; 41+ messages in thread
From: Shlomo Pongratz @ 2015-10-22 10:45 UTC (permalink / raw)
  To: Pavel Fedin
  Cc: peter.maydell, eric.auger, Shlomo Pongratz, qemu-devel,
	shannon.zhao, ashoks, imammedo

[-- Attachment #1: Type: text/plain, Size: 953 bytes --]

Hi,

On Thursday, October 22, 2015, Pavel Fedin <p.fedin@samsung.com> wrote:

>  Hello!
>
> >> It has nothing to do with KVM. EFI is a firmware, which originates from
> Intel, but now adopted by ARM64 architecture too. You can also run
> >> it under qemu, if you want to make kind of "full" machine. And it
> writes some value to BPR1, which is indeed ignored by Linux kernel.
>
> > So were is it used in QEMU?
>
> You can configure any ARM machine to use firmware blob instead of kernel +
> device tree. This way it looks more like a real machine.
>
>
I don't have any "blob" I can only run virt.


> > Which machine in hw/arm needs it?
>
>  We ran it on virt, for example.
>
>
And how did you accessed the register and for what purpose?
What should be the expected outcome.


> Kind regards,
> Pavel Fedin
> Expert Engineer
> Samsung Electronics Research center Russia
>
>  In general I can't release something that I don't understand and can't
test.

[-- Attachment #2: Type: text/html, Size: 1549 bytes --]

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [Qemu-devel] [PATCH RFC V5 1/9] hw/intc: Implement GIC-500 support files
       [not found] ` <1445361732-16257-2-git-send-email-shlomopongratz@gmail.com>
@ 2015-10-22 11:53   ` Pavel Fedin
  0 siblings, 0 replies; 41+ messages in thread
From: Pavel Fedin @ 2015-10-22 11:53 UTC (permalink / raw)
  To: 'Shlomo Pongratz', qemu-devel
  Cc: peter.maydell, eric.auger, 'Shlomo Pongratz',
	shannon.zhao, ashoks, imammedo

 Hello!

 Some more notes after more careful reviewing. Actually, i am working on v3 of live migration RFC, i will include some macros which
will help you to integrate better.

> -----Original Message-----
> From: Shlomo Pongratz [mailto:shlomopongratz@gmail.com]
> Sent: Tuesday, October 20, 2015 8:22 PM
> To: qemu-devel@nongnu.org
> Cc: p.fedin@samsung.com; peter.maydell@linaro.org; eric.auger@linaro.org;
> shannon.zhao@linaro.org; imammedo@redhat.com; ashoks@broadcom.com; Shlomo Pongratz
> Subject: [PATCH RFC V5 1/9] hw/intc: Implement GIC-500 support files
> 
> From: Shlomo Pongratz <shlomo.pongratz@huawei.com>
> 
> Add files need to maintain the GIC-500 state.
> Use bitmaps for managing per cpu flags.
> As the largest "atomic" data structure is uint64 supporting
> more than 64 cores requires to change some data structures
> to bitmap.
> 
> Add mp_affinity poroperty vector to hold a copy of the
> cpu's affinity.
> 
> Signed-off-by: Shlomo Pongratz <shlomo.pongratz@huawei.com>
> ---
>  hw/intc/arm_gicv3_common.c         | 251 ++++++++++++++++++++++++++++++++++++-
>  hw/intc/gicv3_internal.h           | 210 +++++++++++++++++++++++++++++++
>  include/hw/intc/arm_gicv3.h        |  44 +++++++
>  include/hw/intc/arm_gicv3_common.h |  78 +++++++++++-
>  4 files changed, 577 insertions(+), 6 deletions(-)
>  create mode 100644 hw/intc/gicv3_internal.h
>  create mode 100644 include/hw/intc/arm_gicv3.h
> 
> diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
> index 032ece2..3c9bd34 100644
> --- a/hw/intc/arm_gicv3_common.c
> +++ b/hw/intc/arm_gicv3_common.c
> @@ -21,6 +21,7 @@
>   */
> 
>  #include "hw/intc/arm_gicv3_common.h"
> +#include "gicv3_internal.h"
> 
>  static void gicv3_pre_save(void *opaque)
>  {
> @@ -43,11 +44,89 @@ static int gicv3_post_load(void *opaque, int version_id)
>      return 0;
>  }
> 
> +static const VMStateDescription vmstate_gicv3_irq_state = {
> +    .name = "arm_gicv3/arm_gicv3_irq_state",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_BITMAP(enabled, gicv3_irq_state, 0, num_cpu),
> +        VMSTATE_BITMAP(pending, gicv3_irq_state, 0, num_cpu),
> +        VMSTATE_BITMAP(active, gicv3_irq_state, 0, num_cpu),
> +        VMSTATE_BITMAP(level, gicv3_irq_state, 0, num_cpu),
> +        VMSTATE_BITMAP(group, gicv3_irq_state, 0, num_cpu),
> +        VMSTATE_BOOL(edge_trigger, gic_irq_state),
> +        VMSTATE_BITMAP(target, gicv3_irq_state, 0, num_cpu),
> +        VMSTATE_VARRAY_INT32(last_active, gicv3_irq_state, num_cpu,
> +                             0, vmstate_info_uint16, uint16_t),
> +        VMSTATE_INT32(num_cpu, gicv3_irq_state),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +static const VMStateDescription vmstate_gicv3_Priority = {
> +    .name = "arm_gicv3/priority1",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_VARRAY_INT32(p, gicv3_Priority, num_cpu,
> +                             0, vmstate_info_uint8, uint8_t),
> +        VMSTATE_INT32(num_cpu, gicv3_Priority),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +static const VMStateDescription vmstate_gicv3_sgi_state = {
> +    .name = "arm_gicv3/arm_gicv3_sgi/arm_gicv3_sgi_state",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_BITMAP(pending, gicv3_sgi_state, 0, num_cpu),
> +        VMSTATE_INT32(num_cpu, gicv3_sgi_state),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +static const VMStateDescription vmstate_gicv3_sgi_vec = {
> +    .name = "arm_gicv3/arm_gicv3_sgi_vec",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_STRUCT_VARRAY_INT32(state, gicv3_sgi_vec, num_cpu,
> +                                    0, vmstate_gicv3_sgi_state, gicv3_sgi_state),
> +        VMSTATE_INT32(num_cpu, gicv3_sgi_vec),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
>  static const VMStateDescription vmstate_gicv3 = {
>      .name = "arm_gicv3",
>      .unmigratable = 1,
>      .pre_save = gicv3_pre_save,
>      .post_load = gicv3_post_load,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT32(ctlr, GICv3State),
> +        VMSTATE_VARRAY_UINT32(cpu_ctlr, GICv3State, num_cpu,
> +                              0, vmstate_info_uint32, uint32_t),
> +        VMSTATE_STRUCT_ARRAY(irq_state, GICv3State, GICV3_MAXIRQ, 1,
> +                             vmstate_gicv3_irq_state, gicv3_irq_state),
> +        VMSTATE_STRUCT_ARRAY(priority1, GICv3State, GICV3_INTERNAL, 1,
> +                             vmstate_gicv3_Priority, gicv3_Priority),
> +        VMSTATE_UINT8_ARRAY(priority2, GICv3State, GICV3_MAXIRQ - GICV3_INTERNAL),
> +        VMSTATE_STRUCT_ARRAY(sgi, GICv3State, GICV3_NR_SGIS, 1,
> +                             vmstate_gicv3_sgi_vec, gicv3_sgi_vec),
> +        VMSTATE_VARRAY_UINT32(priority_mask, GICv3State, num_cpu, 0,
> +                              vmstate_info_uint16, uint16_t),
> +        VMSTATE_VARRAY_UINT32(running_irq, GICv3State, num_cpu, 0,
> +                              vmstate_info_uint16, uint16_t),
> +        VMSTATE_VARRAY_UINT32(running_priority, GICv3State, num_cpu, 0,
> +                              vmstate_info_uint16, uint16_t),
> +        VMSTATE_VARRAY_UINT32(current_pending, GICv3State, num_cpu, 0,
> +                              vmstate_info_uint16, uint16_t),
> +        VMSTATE_VARRAY_UINT32(mp_affinity, GICv3State, num_mp_affinity, 0,
> +                              vmstate_info_uint64, uint64_t),
> +        VMSTATE_UINT32(num_cpu, GICv3State),
> +        VMSTATE_END_OF_LIST()
> +    }
>  };
> 
>  void gicv3_init_irqs_and_mmio(GICv3State *s, qemu_irq_handler handler,
> @@ -63,11 +142,11 @@ void gicv3_init_irqs_and_mmio(GICv3State *s, qemu_irq_handler handler,
>       *  [N+32..N+63] PPIs for CPU 1
>       *   ...
>       */
> -    i = s->num_irq - GIC_INTERNAL + GIC_INTERNAL * s->num_cpu;
> +    i = s->num_irq - GICV3_INTERNAL + GICV3_INTERNAL * s->num_cpu;
>      qdev_init_gpio_in(DEVICE(s), handler, i);
> 
> -    s->parent_irq = g_malloc(s->num_cpu * sizeof(qemu_irq));
> -    s->parent_fiq = g_malloc(s->num_cpu * sizeof(qemu_irq));
> +    s->parent_irq = g_new0(qemu_irq, s->num_cpu);
> +    s->parent_fiq = g_new0(qemu_irq, s->num_cpu);
> 
>      for (i = 0; i < s->num_cpu; i++) {
>          sysbus_init_irq(sbd, &s->parent_irq[i]);
> @@ -88,6 +167,10 @@ void gicv3_init_irqs_and_mmio(GICv3State *s, qemu_irq_handler handler,
>  static void arm_gicv3_common_realize(DeviceState *dev, Error **errp)
>  {
>      GICv3State *s = ARM_GICV3_COMMON(dev);
> +    int num_irq = s->num_irq;
> +    gicv3_irq_state *irq_state;
> +    gicv3_sgi_vec *sgi_vec;
> +    int i;
> 
>      /* revision property is actually reserved and currently used only in order
>       * to keep the interface compatible with GICv2 code, avoiding extra
> @@ -98,18 +181,176 @@ static void arm_gicv3_common_realize(DeviceState *dev, Error **errp)
>          error_setg(errp, "unsupported GIC revision %d", s->revision);
>          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;
> +    }
> +
> +    /* s->parent_irq and s->parent_fiq  are initialized in
> +     * gicv3_init_irqs_and_mmio
> +     */
> +    s->priority_mask = g_new0(uint16_t, s->num_cpu);
> +    s->current_pending = g_new(uint16_t, s->num_cpu);
> +    s->running_irq = g_new(uint16_t, s->num_cpu);
> +    s->running_priority = g_new(uint16_t, s->num_cpu);
> +    s->cpu_ctlr = g_new0(uint32_t, s->num_cpu);
> +    s->cpu_enabled = bitmap_new(s->num_cpu);
> +    bitmap_zero(s->cpu_enabled, s->num_cpu);
> +    /* Don't ovreride mp_affinity it is a prop */
> +
> +    for (i = 0; i < s->num_cpu; i++) {
> +        s->current_pending[i] = 1023;
> +        s->running_irq[i] = 1023;
> +        s->running_priority[i] = 0x100;
> +    }
> +
> +    irq_state = s->irq_state;
> +    for (i = 0; i < GICV3_MAXIRQ; i++) {
> +        irq_state->num_cpu = s->num_cpu;
> +        irq_state->enabled = bitmap_new(s->num_cpu);
> +        bitmap_zero(irq_state->enabled, s->num_cpu);
> +        irq_state->pending = bitmap_new(s->num_cpu);
> +        bitmap_zero(irq_state->pending, s->num_cpu);
> +        irq_state->active = bitmap_new(s->num_cpu);
> +        bitmap_zero(irq_state->active, s->num_cpu);
> +        irq_state->level = bitmap_new(s->num_cpu);
> +        bitmap_zero(irq_state->level, s->num_cpu);
> +        irq_state->group = bitmap_new(s->num_cpu);
> +        bitmap_zero(irq_state->group, s->num_cpu);
> +        irq_state->edge_trigger = false;
> +        irq_state->target = bitmap_new(s->num_cpu);
> +        bitmap_zero(irq_state->target, s->num_cpu);
> +        irq_state->last_active = g_new0(uint16_t, s->num_cpu);
> +        irq_state++;
> +    }
> +
> +    sgi_vec = s->sgi;
> +    for (i = 0; i < GICV3_NR_SGIS; i++) {
> +        int j;
> +        struct gicv3_sgi_state *sgi_state;
> +        sgi_vec->num_cpu = s->num_cpu;
> +        sgi_vec->state = g_new0(gicv3_sgi_state, s->num_cpu);
> +        sgi_state = sgi_vec->state;
> +        for (j = 0; j < s->num_cpu; j++) {
> +            sgi_state->num_cpu = s->num_cpu;
> +            sgi_state->pending = bitmap_new(s->num_cpu);
> +            bitmap_zero(sgi_state->pending, s->num_cpu);
> +            sgi_state++;
> +        }
> +        sgi_vec++;
> +    }
> +
> +    for (i = 0; i < GICV3_INTERNAL; i++) {
> +        s->priority1[i].num_cpu = s->num_cpu;
> +        s->priority1[i].p = g_new0(uint8_t, s->num_cpu);
> +    }
>  }
> 
>  static void arm_gicv3_common_reset(DeviceState *dev)
>  {
> -    /* TODO */
> +    GICv3State *s = ARM_GICV3_COMMON(dev);
> +    gicv3_irq_state *irq_state;
> +    gicv3_sgi_vec *sgi_vec;
> +    int i;
> +
> +    /* Note num_cpu and num_irq are properties */
> +
> +    /* Don't reset anything assigned in arm_gic_realize or any property */
> +
> +    /* No GICv2 backwards computability support */
> +    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_ctlr[i] = 0;
> +        /* Don't tuch mp_affinity set in virt.c */
> +    }
> +
> +    irq_state = s->irq_state;
> +    for (i = 0; i < GICV3_MAXIRQ; i++) {
> +        /* Reset every filed in irq_state */
> +        bitmap_zero(irq_state->enabled, s->num_cpu);
> +        bitmap_zero(irq_state->pending, s->num_cpu);
> +        bitmap_zero(irq_state->active, s->num_cpu);
> +        bitmap_zero(irq_state->level, s->num_cpu);
> +        bitmap_zero(irq_state->group, s->num_cpu);
> +        bitmap_zero(irq_state->target, s->num_cpu);
> +        memset(irq_state->last_active, 0,
> +               s->num_cpu * sizeof(*irq_state->last_active));
> +        irq_state->edge_trigger = 0;
> +        irq_state++;
> +    }
> +    /* GIC-500 comment 'j' SGI are always enabled */
> +    for (i = 0; i < GICV3_NR_SGIS; i++) {
> +        GIC_SET_ENABLED(i, ALL_CPU_MASK);
> +        GIC_SET_EDGE_TRIGGER(i);
> +    }
> +
> +    sgi_vec = s->sgi;
> +    for (i = 0; i < GICV3_NR_SGIS; i++) {
> +        struct gicv3_sgi_state *sgi_state = sgi_vec->state;
> +        int j;
> +        for (j = 0; j < s->num_cpu; j++) {
> +            bitmap_zero(sgi_state->pending, s->num_cpu);
> +            sgi_state++;
> +        }
> +        sgi_vec++;
> +    }
> +
> +    if (s->num_cpu == 1) {
> +        /* For uniprocessor GICs all interrupts always target the sole CPU */
> +        for (i = 0; i < GICV3_MAXIRQ; i++) {
> +            set_bit(0, s->irq_state[i].target);
> +        }
> +    }
> +
> +    for (i = 0; i < GICV3_INTERNAL; i++)
> +        memset(s->priority1[i].p, 0, s->num_cpu * sizeof(*s->priority1[i].p));
> +    memset(s->priority2, 0, sizeof(s->priority2));
> +
> +    /* With all configuration we don't  support GICv2 backwards computability */
> +    if (s->security_levels > 1) {
> +        /* GICv3 5.3.20 With two security So DS is RAZ/WI ARE_NS is RAO/WI
> +         * and ARE_S is RAO/WI
> +         */
> +         s->ctlr = GICD_CTLR_ARE_S | GICD_CTLR_ARE_NS;
> +    } else {
> +        /* GICv3 5.3.20 With one security So DS is RAO/WI ARE is RAO/WI
> +         */
> +        s->ctlr = GICD_CTLR_DS | GICD_CTLR_ARE;
> +    }
> +    /* Workaround!
> +     * Linux (drivers/irqchip/irq-gic-v3.c) is enabling only group one,
> +     * in gic_cpu_sys_reg_init it calls gic_write_grpen1(1);
> +     * but it doesn't conigure any interrupt to be in group one
> +     */
> +    for (i = 0; i < s->num_irq; i++)
> +        GIC_SET_GROUP(i, ALL_CPU_MASK);
>  }
> 
>  static Property arm_gicv3_common_properties[] = {
>      DEFINE_PROP_UINT32("num-cpu", GICv3State, num_cpu, 1),
>      DEFINE_PROP_UINT32("num-irq", GICv3State, num_irq, 32),
>      DEFINE_PROP_UINT32("revision", GICv3State, revision, 3),
> -    DEFINE_PROP_BOOL("has-security-extensions", GICv3State, security_extn, 0),
> +    DEFINE_PROP_UINT8("security-levels", GICv3State, security_levels, 0),
> +    DEFINE_PROP_ARRAY("mp-affinity", GICv3State, num_mp_affinity, mp_affinity,
> +                      qdev_prop_uint64, uint64_t),
>      DEFINE_PROP_END_OF_LIST(),
>  };
> 
> diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h
> new file mode 100644
> index 0000000..e0b4a08
> --- /dev/null
> +++ b/hw/intc/gicv3_internal.h
> @@ -0,0 +1,210 @@
> +/*
> + * 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"
> +
> +/* Marks all CPUs  */
> +#define ALL_CPU_MASK ((int) -1)
> +#define ALL_CPU_MASK_COMPAT ((unsigned)(0xff))
> +
> +/* Keep this macro so it will be easy to compare the code to GICv2 code.
> + * The compiler will optimize any +/- operation involving this macro
> + */
> +#define GICV3_BASE_IRQ (0)

 Does it really worth that? IMHO it only pollutes the code and makes it less readable. If you ever want to compare this with GICv2
code, you should just know that there's no GICV3_BASE_IRQ. Actually, IIRC, it's not even GIC, but NVIC artifact.

> +
> +static inline void auto_or(GICv3State *s, unsigned long *target, unsigned long *bm)
> +{
> +    bitmap_or(target, target, bm, s->num_cpu);
> +}
> +
> +static inline int auto_test(GICv3State *s, unsigned long *target, unsigned long *bm)
> +{
> +    /* Result used for test */
> +    return bitmap_intersects(target, bm, s->num_cpu);
> +}
> +
> +static inline void auto_andnot(GICv3State *s, unsigned long *target, unsigned long *bm)
> +{
> +    /* Ignore status */
> +    (void) bitmap_andnot(target, target, bm, s->num_cpu);
> +}
> +
> +static inline void gic_set(GICv3State *s, unsigned long *bm, int cpu)
> +{
> +    if (cpu < 0) /* All CPUs */
> +        bitmap_fill(bm, s->num_cpu);
> +    else
> +        set_bit(cpu, bm);
> +}
> +
> +static inline void gic_clr(GICv3State *s, unsigned long *bm, int cpu)
> +{
> +    if (cpu < 0) /* All CPUs */
> +        bitmap_zero(bm, s->num_cpu);
> +    else
> +        clear_bit(cpu, bm);
> +}
> +
> +static inline int gic_test(GICv3State *s, unsigned long *bm, int cpu)
> +{
> +    if (cpu < 0) /* ANY CPUs */
> +        return !bitmap_empty(bm, s->num_cpu);
> +    else
> +        return test_bit(cpu, bm);
> +}
> +
> +#define GIC_SET_ENABLED(irq, cm) gic_set(s, s->irq_state[irq].enabled, cm)
> +#define GIC_CLEAR_ENABLED(irq, cm) gic_clr(s, s->irq_state[irq].enabled, cm)
> +#define GIC_TEST_ENABLED(irq, cm) gic_test(s, s->irq_state[irq].enabled, cm)
> +#define GIC_SET_PENDING(irq, cm) gic_set(s, s->irq_state[irq].pending, cm)
> +#define GIC_SET_PENDING_MASK(irq, bm) auto_or(s, s->irq_state[irq].pending, bm)
> +#define GIC_TEST_PENDING(irq, cm) gic_test(s, s->irq_state[irq].pending, cm)
> +#define GIC_CLEAR_PENDING(irq, cm) gic_clr(s, s->irq_state[irq].pending, cm)
> +#define GIC_SET_ACTIVE(irq, cm) gic_set(s, s->irq_state[irq].active, cm)
> +#define GIC_CLEAR_ACTIVE(irq, cm) gic_clr(s, s->irq_state[irq].active, cm)
> +#define GIC_TEST_ACTIVE(irq, cm) gic_test(s, s->irq_state[irq].active, cm)
> +#define GIC_SET_LEVEL(irq, cm) gic_set(s, s->irq_state[irq].level, cm)
> +#define GIC_CLEAR_LEVEL(irq, cm) gic_clr(s, s->irq_state[irq].level, cm)
> +#define GIC_SET_LEVEL_MASK(irq, bm) auto_or(s, s->irq_state[irq].level, bm)
> +#define GIC_CLEAR_LEVEL_MASK(irq, bm) auto_andnot(s, s->irq_state[irq].level, bm)
> +#define GIC_TEST_LEVEL(irq, cm) gic_test(s, s->irq_state[irq].level, cm)
> +#define GIC_TEST_LEVEL_MASK(irq, bm) auto_test(s, s->irq_state[irq].level, bm)
> +#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)

 I know that GICv2 implements them this way. Not sure that it's correct, though. GICR_ICFGR description in the doc says:
--- cut ---
Configurations
A copy of this register is provided for each Redistributor.
--- cut ---
 So, i assume that these are per-CPU too, despite it really doesn't make sense to have different configs on different CPUs.
 My RFC implements per-CPU registers for SGI/PPIs.

> +#define GIC_GET_PRIORITY(irq, cpu) (((irq) < GICV3_INTERNAL) ?          \
> +                                    s->priority1[irq].p[cpu] :          \
> +                                    s->priority2[(irq) - GICV3_INTERNAL])
> +#define GIC_SET_TARGET(irq, cm) gic_set(s, s->irq_state[irq].target, cm)
> +#define GIC_CLEAR_TARGET(irq, cm) gic_clr(s, s->irq_state[irq].target, cm)
> +#define GIC_TEST_TARGET(irq, cm) gic_test(s, s->irq_state[irq].target, cm)
> +#define GIC_TARGET(irq) s->irq_state[irq].target
> +#define GIC_CLEAR_GROUP(irq, cm) gic_clr(s, s->irq_state[irq].group, cm)
> +#define GIC_SET_GROUP(irq, cm) gic_set(s, s->irq_state[irq].group, cm)
> +#define GIC_TEST_GROUP(irq, cm) gic_test(s, s->irq_state[irq].group, cm)
> +
> +/* The special cases for the revision property: */
> +#define REV_V3 3

 Does this make any sense? Value 3 is self-sufficient here, because it's just 3. This reads like "#define NUM_3 3". Isn't it
ridiculous?

> +
> +static inline bool gic_test_pending(GICv3State *s, int irq, int 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 GIC_TEST_PENDING(irq, cm) ||
> +        (!GIC_TEST_EDGE_TRIGGER(irq) && GIC_TEST_LEVEL(irq, cm));
> +}
> +
> +#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_EN_GRP0           (1U << 0)
> +#define GICD_CTLR_EN_GRP1NS         (1U << 1) /* GICv3 5.3.20 */
> +#define GICD_CTLR_EN_GRP1S          (1U << 2)
> +#define GICD_CTLR_EN_GRP1_ALL       (GICD_CTLR_EN_GRP1NS | GICD_CTLR_EN_GRP1S)
> +#define GICD_CTLR_ARE               (1U << 4)
> +#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)
> +
> +
> +#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

 These three are not used anywhere.

> +
> +/*
> + * 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)
> +
> +/*
> + * Simulated used system registers
> + */
> +#define GICC_CTLR_EN_GRP0    (1U << 0)
> +#define GICC_CTLR_EN_GRP1    (1U << 1)
> +#define GICC_CTLR_ACK_CTL    (1U << 2)
> +#define GICC_CTLR_FIQ_EN     (1U << 3)
> +#define GICC_CTLR_CBPR       (1U << 4) /* GICv1: SBPR */
> +#define GICC_CTLR_EOIMODE    (1U << 9)
> +#define GICC_CTLR_EOIMODE_NS (1U << 10)
> +
> +#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..a03af35
> --- /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_GICV3(obj) \
> +     OBJECT_CHECK(GICv3State, (obj), TYPE_ARM_GICV3)
> +#define ARM_GICV3_CLASS(klass) \
> +     OBJECT_CLASS_CHECK(ARMGICv3Class, (klass), TYPE_ARM_GICV3)
> +#define ARM_GICV3_GET_CLASS(obj) \
> +     OBJECT_GET_CLASS(ARMGICv3Class, (obj), TYPE_ARM_GICV3)
> +
> +typedef struct ARMGICv3Class {
> +    /*< private >*/
> +    ARMGICv3CommonClass parent_class;
> +    /*< public >*/
> +
> +    DeviceRealize parent_realize;
> +} ARMGICv3Class;
> +
> +#endif
> diff --git a/include/hw/intc/arm_gicv3_common.h b/include/hw/intc/arm_gicv3_common.h
> index c2fd8da..aa32229 100644
> --- a/include/hw/intc/arm_gicv3_common.h
> +++ b/include/hw/intc/arm_gicv3_common.h
> @@ -26,6 +26,51 @@
>  #include "hw/sysbus.h"
>  #include "hw/intc/arm_gic_common.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 ARM_AFF0_SHIFT 0
> +#define ARM_AFF0_MASK  (0xFFULL << ARM_AFF0_SHIFT)
> +#define ARM_AFF1_SHIFT 8
> +#define ARM_AFF1_MASK  (0xFFULL << ARM_AFF1_SHIFT)
> +#define ARM_AFF2_SHIFT 16
> +#define ARM_AFF2_MASK  (0xFFULL << ARM_AFF2_SHIFT)
> +#define ARM_AFF3_SHIFT 32
> +#define ARM_AFF3_MASK  (0xFFULL << ARM_AFF3_SHIFT)

 Actually, these are defined in target-arm/cpu-qom.h. Looks like it was my fault and they have to be moved to something like
include/hw/cpu/cpu_arm.h. I'll post some patches to move them, but not sure that Peter accepts them, just because "there are no
users for this yet".

> +
> +#define MAX_NR_GROUP_PRIO 128
> +
> +typedef struct gicv3_irq_state {
> +    /* The enable bits are only banked for per-cpu interrupts.  */
> +    unsigned long *enabled;
> +    unsigned long *pending;
> +    unsigned long *active;
> +    unsigned long *level;
> +    unsigned long *group;
> +    unsigned long *target;
> +    uint16_t *last_active;
> +    bool edge_trigger; /* true: edge-triggered, false: level-triggered  */
> +    int32_t num_cpu; /* For VMSTATE_BITMAP & VMSTATE_VARRAY */
> +} gicv3_irq_state;
> +
> +typedef struct gicv3_sgi_state {
> +    unsigned long *pending;
> +    int32_t num_cpu; /* For VMSTATE_BITMAP */
> +} gicv3_sgi_state;
> +
> +typedef struct gicv3_sgi_vec {
> +    struct gicv3_sgi_state *state;
> +    int32_t num_cpu; /* For VMSTATE_VARRAY */
> +} gicv3_sgi_vec;
> +
> +typedef struct gicv3_priority {
> +    uint8_t *p;
> +    int32_t num_cpu; /* For VMSTATE_VARRAY */
> +} gicv3_Priority;
> +
>  typedef struct GICv3State {
>      /*< private >*/
>      SysBusDevice parent_obj;
> @@ -33,14 +78,45 @@ typedef struct GICv3State {
> 
>      qemu_irq *parent_irq;
>      qemu_irq *parent_fiq;
> +    /* GICD_CTLR; for a GIC with the security extensions the NS banked version
> +     * of this register is just an alias of bit 1 of the S banked version.
> +     */
> +    uint32_t ctlr;
> +    /* Sim GICC_CTLR; again, the NS banked version is just aliases of bits of
> +     * the S banked register, so our state only needs to store the S version.
> +     */
> +    uint32_t *cpu_ctlr;
> +    unsigned long *cpu_enabled;
> +
> +    gicv3_irq_state irq_state[GICV3_MAXIRQ];
> +    gicv3_Priority priority1[GICV3_INTERNAL];
> +    uint8_t priority2[GICV3_MAXIRQ - GICV3_INTERNAL];
> +    /* For each SGI on the target CPU, we store bitmap
> +     * 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_vec sgi[GICV3_NR_SGIS];
> +
> +    uint16_t *priority_mask;
> +    uint16_t *running_irq;
> +    uint16_t *running_priority;
> +    uint16_t *current_pending;
> +    uint32_t num_mp_affinity;
> +    uint64_t *mp_affinity;
> 
>      MemoryRegion iomem_dist; /* Distributor */
> +    MemoryRegion iomem_spi;
> +    MemoryRegion iomem_its_cntrl;
> +    MemoryRegion iomem_its;
>      MemoryRegion iomem_redist; /* Redistributors */
> 
> +    uint32_t cpu_mask; /* For redistributer */
>      uint32_t num_cpu;
>      uint32_t num_irq;
>      uint32_t revision;
> -    bool security_extn;
> +    uint8_t security_levels; /* replace seurity extentions */
> 
>      int dev_fd; /* kvm device fd if backed by kvm vgic support */
>  } GICv3State;
> --
> 1.9.1

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [Qemu-devel] [PATCH RFC V5 0/9] Implement GIC-500 from GICv3 family for arm64
  2015-10-20 17:22 [Qemu-devel] [PATCH RFC V5 0/9] Implement GIC-500 from GICv3 family for arm64 Shlomo Pongratz
                   ` (8 preceding siblings ...)
       [not found] ` <1445361732-16257-2-git-send-email-shlomopongratz@gmail.com>
@ 2016-01-29 16:58 ` Christopher Covington
  2016-01-31 15:54   ` Shlomo Pongratz
  9 siblings, 1 reply; 41+ messages in thread
From: Christopher Covington @ 2016-01-29 16:58 UTC (permalink / raw)
  To: Shlomo Pongratz, peter.maydell, Shlomo Pongratz
  Cc: eric.auger, p.fedin, qemu-devel, shannon.zhao, ashoks, imammedo

On 10/20/2015 01:22 PM, Shlomo Pongratz wrote:
> From: Shlomo Pongratz <shlomo.pongratz@huawei.com>
> 
> This patch is a first step multicores support for arm64.
> 
> This implemntation was tested up to 100 cores.
> 
> Things left to do:
> 
> Support SPI, ITS and ITS CONTROL, note that this patch porpose is to enable
> running multi cores using the "virt" virtual machine and this goal is achived
> without that.
> 
> Add GICv2 backwards competability. Since there is a GICv2 implementation I
> can't see the pusprose for it.
> 
> Special thanks to Peter Crostwaite whose patch to th Linux (kernel) i.e.
> Implement cpu_relax as yield solved the problem of the boot process getting
> stuck for 24 cores and more.
> 
> Figure out why virtual machine name changed from virt-v3 to virt-v3-machine

Hi Shlomo,

Were you planning on another revision of this patchset? Are there any
things you would like help with?

Peter,

Do you have any thoughts about what is essential and what isn't for a
first wave of TCG GICv3 patches to be mergeable?

Thanks,
Christopher Covington

-- 
Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [Qemu-devel] [PATCH RFC V5 0/9] Implement GIC-500 from GICv3 family for arm64
  2016-01-29 16:58 ` [Qemu-devel] [PATCH RFC V5 0/9] Implement GIC-500 from GICv3 family for arm64 Christopher Covington
@ 2016-01-31 15:54   ` Shlomo Pongratz
       [not found]     ` <CAFEAcA9yQoPzC917Lw2zDsa7a5NOfgEk+x68wA1o+Y4ocywCgw@mail.gmail.com>
  0 siblings, 1 reply; 41+ messages in thread
From: Shlomo Pongratz @ 2016-01-31 15:54 UTC (permalink / raw)
  To: Christopher Covington
  Cc: peter.maydell, eric.auger, Shlomo Pongratz, p.fedin, qemu-devel,
	shannon.zhao, ashoks, imammedo

[-- Attachment #1: Type: text/plain, Size: 1557 bytes --]

On Friday, January 29, 2016, Christopher Covington <cov@codeaurora.org>
wrote:

> On 10/20/2015 01:22 PM, Shlomo Pongratz wrote:
> > From: Shlomo Pongratz <shlomo.pongratz@huawei.com <javascript:;>>
> >
> > This patch is a first step multicores support for arm64.
> >
> > This implemntation was tested up to 100 cores.
> >
> > Things left to do:
> >
> > Support SPI, ITS and ITS CONTROL, note that this patch porpose is to
> enable
> > running multi cores using the "virt" virtual machine and this goal is
> achived
> > without that.
> >
> > Add GICv2 backwards competability. Since there is a GICv2 implementation
> I
> > can't see the pusprose for it.
> >
> > Special thanks to Peter Crostwaite whose patch to th Linux (kernel) i.e.
> > Implement cpu_relax as yield solved the problem of the boot process
> getting
> > stuck for 24 cores and more.
> >
> > Figure out why virtual machine name changed from virt-v3 to
> virt-v3-machine
>
> Hi Shlomo,
>
> Were you planning on another revision of this patchset? Are there any
> things you would like help with?
>
> Peter,
>
> Do you have any thoughts about what is essential and what isn't for a
> first wave of TCG GICv3 patches to be mergeable?
>
> Thanks,
> Christopher Covington
>
> --
> Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
>

Hi,

I will do a new revision of the GICv3.
I needed to get a time slot from my employee in order to do the work and I
got one starting next week.

Best regards,

S.P.

[-- Attachment #2: Type: text/html, Size: 2120 bytes --]

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [Qemu-devel] [PATCH RFC V5 0/9] Implement GIC-500 from GICv3 family for arm64
       [not found]       ` <CAHzK-V0Ur=qYM_OAKmSf+smp3M1E49swkT7yUj=jBkLp+J6Pag@mail.gmail.com>
@ 2016-02-21 12:30         ` Shlomo Pongratz
  2016-02-21 15:32           ` Peter Maydell
  0 siblings, 1 reply; 41+ messages in thread
From: Shlomo Pongratz @ 2016-02-21 12:30 UTC (permalink / raw)
  To: Peter Maydell, Pavel Fedin; +Cc: qemu-devel

[-- Attachment #1: Type: text/plain, Size: 1518 bytes --]

On Tuesday, February 16, 2016, Shlomo Pongratz <shlomopongratz@gmail.com>
wrote:

>
>
> On Tuesday, February 16, 2016, Peter Maydell <peter.maydell@linaro.org
> <javascript:_e(%7B%7D,'cvml','peter.maydell@linaro.org');>> wrote:
>
>> On 31 January 2016 at 15:54, Shlomo Pongratz <shlomopongratz@gmail.com>
>> wrote:
>> > I will do a new revision of the GICv3.
>> > I needed to get a time slot from my employee in order to do the work
>> and I
>> > got one starting next week.
>>
>> Hi Shlomo; I was just wondering how you were getting on with this?
>>
>> I ask because now I'm getting to the end of the TrustZone stuff
>> I've been working on I expect to have more time to help with
>> review/code/whatever for the GICv3. So I don't want to tread on
>> your toes with stuff you're working on now, but if you do find
>> you don't have time to make forward progress could you let me know?
>> (or if there's something you want help with do say.)
>>
>> thank
>> -- PMM
>>
>
> Hi,
>
> I just got a go ahead.
> I'm reviewing all the comments and pulling latest revision.
> I assume it will take a week or two.
> Best regards,
>
> S.P.
>
>

Hi,

Working of re-basing the GICv3 I've noticed in the mailing list Pavel's
patch series named "GICv3 live migration support". This patch series
modifies GICv3 structures. I can see that this patch series was not merged
in the latest git that I've pulled (commit 80b5d6bfc). So me question is,
should I use the relevant patches in my patch series or ignore them?

Best regards,

S.P.

[-- Attachment #2: Type: text/html, Size: 2151 bytes --]

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [Qemu-devel] [PATCH RFC V5 0/9] Implement GIC-500 from GICv3 family for arm64
  2016-02-21 12:30         ` Shlomo Pongratz
@ 2016-02-21 15:32           ` Peter Maydell
  2016-02-22 11:09             ` Peter Maydell
  0 siblings, 1 reply; 41+ messages in thread
From: Peter Maydell @ 2016-02-21 15:32 UTC (permalink / raw)
  To: Shlomo Pongratz; +Cc: Pavel Fedin, QEMU Developers

On 21 February 2016 at 12:30, Shlomo Pongratz <shlomopongratz@gmail.com> wrote:
> Working of re-basing the GICv3 I've noticed in the mailing list Pavel's
> patch series named "GICv3 live migration support". This patch series
> modifies GICv3 structures. I can see that this patch series was not merged
> in the latest git that I've pulled (commit 80b5d6bfc). So me question is,
> should I use the relevant patches in my patch series or ignore them?

That patchset was definitely moving in the right direction, but
if you look at the review comments on the RFCv3 you'll see that
there were still some things that needed to be addressed and
I don't think Pavel has sent out a v4.

For your purposes you should start with Pavel's patch 1 in that series:
http://patchwork.ozlabs.org/patch/534434/
and will need to make the changes I suggested in the code review of it.
Patches 2 and 3 you can ignore (since they're related to KVM).
We'll want some variant on patch 4 at some point for
state save/restore, but that also I had suggested changes for:
http://patchwork.ozlabs.org/patch/534439/
We can tackle that patch later, though.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [Qemu-devel] [PATCH RFC V5 0/9] Implement GIC-500 from GICv3 family for arm64
  2016-02-21 15:32           ` Peter Maydell
@ 2016-02-22 11:09             ` Peter Maydell
  0 siblings, 0 replies; 41+ messages in thread
From: Peter Maydell @ 2016-02-22 11:09 UTC (permalink / raw)
  To: Shlomo Pongratz; +Cc: Pavel Fedin, QEMU Developers

On 21 February 2016 at 15:32, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 21 February 2016 at 12:30, Shlomo Pongratz <shlomopongratz@gmail.com> wrote:
>> Working of re-basing the GICv3 I've noticed in the mailing list Pavel's
>> patch series named "GICv3 live migration support". This patch series
>> modifies GICv3 structures. I can see that this patch series was not merged
>> in the latest git that I've pulled (commit 80b5d6bfc). So me question is,
>> should I use the relevant patches in my patch series or ignore them?
>
> That patchset was definitely moving in the right direction, but
> if you look at the review comments on the RFCv3 you'll see that
> there were still some things that needed to be addressed and
> I don't think Pavel has sent out a v4.
>
> For your purposes you should start with Pavel's patch 1 in that series:
> http://patchwork.ozlabs.org/patch/534434/
> and will need to make the changes I suggested in the code review of it.

In fact I think what might be simplest is if I do these changes and
resend the patch, since GICv3 is now at the top of my todo list.
I'll try to send something out later today.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 41+ messages in thread

end of thread, other threads:[~2016-02-22 11:09 UTC | newest]

Thread overview: 41+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-20 17:22 [Qemu-devel] [PATCH RFC V5 0/9] Implement GIC-500 from GICv3 family for arm64 Shlomo Pongratz
2015-10-20 17:22 ` [Qemu-devel] [PATCH RFC V5 2/9] hw/intc: arm_gicv3_interrupts Shlomo Pongratz
2015-10-20 17:22 ` [Qemu-devel] [PATCH RFC V5 3/9] hw/intc: arm_gicv3_cpu_interface Shlomo Pongratz
2015-10-20 17:22 ` [Qemu-devel] [PATCH RFC V5 4/9] hw/intc: arm_gicv3_dist Shlomo Pongratz
2015-10-20 17:22 ` [Qemu-devel] [PATCH RFC V5 5/9] hw/intc arm_gicv3_redist Shlomo Pongratz
2015-10-20 17:22 ` [Qemu-devel] [PATCH RFC V5 6/9] hw/intc: arm_gicv3_spi_its Shlomo Pongratz
2015-10-21 12:43   ` Pavel Fedin
2015-10-21 13:05     ` Shlomo Pongratz
2015-10-21 13:12       ` Pavel Fedin
2015-10-21 13:14         ` Shlomo Pongratz
2015-10-21 13:48           ` Pavel Fedin
2015-10-21 14:19             ` Shlomo Pongratz
2015-10-21 14:26               ` Pavel Fedin
2015-10-21 14:35                 ` Shlomo Pongratz
2015-10-21 14:41                   ` Pavel Fedin
2015-10-21 14:46                     ` Peter Maydell
2015-10-21 14:54                       ` Pavel Fedin
2015-10-20 17:22 ` [Qemu-devel] [PATCH RFC V5 7/9] hw/intc: arm_gicv3 Shlomo Pongratz
2015-10-20 17:22 ` [Qemu-devel] [PATCH RFC V5 8/9] target-arm/cpu64 GICv3 system instructions support Shlomo Pongratz
2015-10-22  7:33   ` Pavel Fedin
2015-10-22  8:19     ` Shlomo Pongratz
2015-10-22  8:33       ` Pavel Fedin
2015-10-22  8:52         ` Shlomo Pongratz
2015-10-22 10:05           ` Pavel Fedin
2015-10-22 10:45             ` Shlomo Pongratz
2015-10-20 17:22 ` [Qemu-devel] [PATCH RFC V5 9/9] hw/arm: Add virt-v3 machine that uses GIC-500 Shlomo Pongratz
2015-10-21  7:02   ` Pavel Fedin
2015-10-21  8:52     ` Shlomo Pongratz
2015-10-21 10:30       ` Pavel Fedin
2015-10-21 11:33         ` Shlomo Pongratz
2015-10-21 12:24           ` Peter Maydell
2015-10-21 13:01             ` Shlomo Pongratz
2015-10-21 13:24               ` Peter Maydell
2015-10-21 13:26               ` Pavel Fedin
2015-10-21 14:05                 ` Shlomo Pongratz
     [not found] ` <1445361732-16257-2-git-send-email-shlomopongratz@gmail.com>
2015-10-22 11:53   ` [Qemu-devel] [PATCH RFC V5 1/9] hw/intc: Implement GIC-500 support files Pavel Fedin
2016-01-29 16:58 ` [Qemu-devel] [PATCH RFC V5 0/9] Implement GIC-500 from GICv3 family for arm64 Christopher Covington
2016-01-31 15:54   ` Shlomo Pongratz
     [not found]     ` <CAFEAcA9yQoPzC917Lw2zDsa7a5NOfgEk+x68wA1o+Y4ocywCgw@mail.gmail.com>
     [not found]       ` <CAHzK-V0Ur=qYM_OAKmSf+smp3M1E49swkT7yUj=jBkLp+J6Pag@mail.gmail.com>
2016-02-21 12:30         ` Shlomo Pongratz
2016-02-21 15:32           ` Peter Maydell
2016-02-22 11:09             ` Peter Maydell

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.