All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH RFC V3 0/4] Implement GIC-500 from GICv3 family for arm64
@ 2015-06-04 16:40 Shlomo Pongratz
  2015-06-04 16:40 ` [Qemu-devel] [PATCH RFC V3 1/4] Use Aff1 with mpidr This is an improved and KVM-aware alternative to Shlomo Pongratz
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Shlomo Pongratz @ 2015-06-04 16:40 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 toward 128 cores support for arm64.

At first only 64 cores are supported for two reasons:
First the largest integer type has the size of 64 bits and modifying
essential data structures in order to support 128 cores will require
the usage of bitops.
Second currently the Linux (kernel) can be configured to support
up to 64 cores thus there is no urgency with 128 cores support.

Things left to do:

There is a need to support flexible clusters size. The GIC-500 can support
up to 128 cores, up to 32 clusters and up to 8 cores is a cluster.
So for example, if one wishes to have 16 cores, the options are:
2 clusters of 8 cores each, 4 clusters with 4 cores each
Currently only the first option is supported.
There is an issue of passing clock affinity to via the dtb. In the dtb

interrupt section there are only 24 bit left to affinity since the
variable is a 32 bit entity and 8 bits are reserved for flags.
See Documentation/devicetree/bindings/arm/arch_timer.txt.
Note that this issue is not seems to be critical as when checking
/proc/irq/3/smp_affinity with 32 cores all 32 bits are one.

The last issue is to add support for 128 cores. This requires the usage
of bitops and currently can be tested up to 64 cores.

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.

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.

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


Pavel Fedin (2):
  Use Aff1 with mpidr This is an improved and KVM-aware alternative to
  Add virt-v3 machine that uses GIC-500

Shlomo Pongratz (2):
  Implment GIC-500
  GICv3 support

 hw/arm/virt.c                      |  165 ++-
 hw/intc/Makefile.objs              |    2 +
 hw/intc/arm_gicv3.c                | 2086 ++++++++++++++++++++++++++++++++++++
 hw/intc/arm_gicv3_common.c         |  217 ++++
 hw/intc/gicv3_internal.h           |  161 +++
 include/hw/arm/virt.h              |    4 +
 include/hw/boards.h                |    1 +
 include/hw/intc/arm_gicv3.h        |   44 +
 include/hw/intc/arm_gicv3_common.h |  119 ++
 target-arm/cpu-qom.h               |    3 +
 target-arm/cpu.c                   |   17 +
 target-arm/cpu.h                   |   12 +
 target-arm/cpu64.c                 |  105 ++
 target-arm/helper.c                |    9 +-
 target-arm/kvm64.c                 |   25 +
 target-arm/psci.c                  |   20 +-
 16 files changed, 2952 insertions(+), 38 deletions(-)
 create mode 100644 hw/intc/arm_gicv3.c
 create mode 100644 hw/intc/arm_gicv3_common.c
 create mode 100644 hw/intc/gicv3_internal.h
 create mode 100644 include/hw/intc/arm_gicv3.h
 create mode 100644 include/hw/intc/arm_gicv3_common.h

-- 
1.9.1

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

* [Qemu-devel] [PATCH RFC V3 1/4] Use Aff1 with mpidr This is an improved and KVM-aware alternative to
  2015-06-04 16:40 [Qemu-devel] [PATCH RFC V3 0/4] Implement GIC-500 from GICv3 family for arm64 Shlomo Pongratz
@ 2015-06-04 16:40 ` Shlomo Pongratz
  2015-06-04 17:17   ` Peter Maydell
  2015-06-04 16:40 ` [Qemu-devel] [PATCH RFC V3 2/4] Implment GIC-500 Shlomo Pongratz
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 14+ messages in thread
From: Shlomo Pongratz @ 2015-06-04 16:40 UTC (permalink / raw)
  To: qemu-devel
  Cc: peter.maydell, eric.auger, Shlomo Pongratz, p.fedin,
	shannon.zhao, ashoks, imammedo

From: Pavel Fedin <p.fedin@samsung.com>

1. MPIDR value (minus feature bits) is now directly stored in CPU instance.
2. Default assignment is based on original rule (8 CPUs per cluster).
3. If KVM is used, MPIDR values are overridden with those from KVM. This is
necessary for
proper KVM PSCI functioning.
4. Some cosmetic changes which would make further expansion of this code easier.
5. Added some debugging macros because CPU ID assignment is tricky part, and if
there are
some problems, i think there should be a quick way to make sure they are
correct.
 This does not have an RFC mark because it is perfectly okay to be committed
alone, it
does not break anything. Commit message follows. Cc'ed to all interested
parties because i
really hope to get things going somewhere this time.

In order to support up to 128 cores with GIC-500 (GICv3 implementation)
affinity1 must be used. GIC-500 support up to 32 clusters with up to
8 cores in a cluster. So for example, if one wishes to have 16 cores,
the options are: 2 clusters of 8 cores each, 4 clusters with 4 cores each
Currently only the first option is supported for TCG. However, KVM expects
to have the same clusters layout as host machine has. Therefore, if we use
64-bit KVM we override CPU IDs with those provided by the KVM. For 32-bit
systems it is OK to leave the default because so far we do not have more
than 8 CPUs on any of them.
This implementation has a potential to be extended with some methods which
would define cluster layout instead of hardcoded CPUS_PER_CLUSTER
definition. This would allow to emulate real machines with different
layouts. However, in case of KVM we would still have to inherit IDs from
the host.

Signed-off-by: Shlomo Pongratz <shlomo.pongratz@huawei.com>
Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
---
 hw/arm/virt.c        |  6 +++++-
 target-arm/cpu-qom.h |  3 +++
 target-arm/cpu.c     | 17 +++++++++++++++++
 target-arm/helper.c  |  9 +++------
 target-arm/kvm64.c   | 25 +++++++++++++++++++++++++
 target-arm/psci.c    | 20 ++++++++++++++++++--
 6 files changed, 71 insertions(+), 9 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 05db8cb..19c8c3a 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -294,7 +294,11 @@ static void fdt_add_cpu_nodes(const VirtBoardInfo *vbi)
                                         "enable-method", "psci");
         }
 
-        qemu_fdt_setprop_cell(vbi->fdt, nodename, "reg", cpu);
+        /*
+         * If cpus node's #address-cells property is set to 1
+         * The reg cell bits [23:0] must be set to bits [23:0] of MPIDR_EL1.
+         */
+        qemu_fdt_setprop_cell(vbi->fdt, nodename, "reg", armcpu->mpidr);
         g_free(nodename);
     }
 }
diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
index ed5a644..a382a09 100644
--- a/target-arm/cpu-qom.h
+++ b/target-arm/cpu-qom.h
@@ -159,6 +159,7 @@ typedef struct ARMCPU {
     uint64_t id_aa64mmfr1;
     uint32_t dbgdidr;
     uint32_t clidr;
+    uint64_t mpidr; /* Without feature bits */
     /* The elements of this array are the CCSIDR values for each cache,
      * in the order L1DCache, L1ICache, L2DCache, L2ICache, etc.
      */
@@ -171,6 +172,8 @@ typedef struct ARMCPU {
     uint64_t rvbar;
 } ARMCPU;
 
+#define CPUS_PER_CLUSTER 8
+
 #define TYPE_AARCH64_CPU "aarch64-cpu"
 #define AARCH64_CPU_CLASS(klass) \
     OBJECT_CLASS_CHECK(AArch64CPUClass, (klass), TYPE_AARCH64_CPU)
diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index 4a888ab..7272466 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -31,6 +31,12 @@
 #include "sysemu/kvm.h"
 #include "kvm_arm.h"
 
+#ifdef DEBUG
+# define DPRINTF(format, ...) printf("armcpu: " format , ## __VA_ARGS__)
+#else
+# define DPRINTF(format, ...) do { } while (0)
+#endif
+
 static void arm_cpu_set_pc(CPUState *cs, vaddr value)
 {
     ARMCPU *cpu = ARM_CPU(cs);
@@ -388,12 +394,23 @@ static void arm_cpu_initfn(Object *obj)
     CPUState *cs = CPU(obj);
     ARMCPU *cpu = ARM_CPU(obj);
     static bool inited;
+    uint32_t Aff1, Aff0;
 
     cs->env_ptr = &cpu->env;
     cpu_exec_init(&cpu->env);
     cpu->cp_regs = g_hash_table_new_full(g_int_hash, g_int_equal,
                                          g_free, g_free);
 
+    /*
+     * We don't support setting cluster ID ([16..23]) (known as Aff2
+     * in later ARM ARM versions), or any of the higher affinity level fields,
+     * so these bits always RAZ.
+     */
+    Aff1 = cs->cpu_index / CPUS_PER_CLUSTER;
+    Aff0 = cs->cpu_index % CPUS_PER_CLUSTER;
+    cpu->mpidr = (Aff1 << 8) | Aff0;
+    DPRINTF("Init(%p): index %d mpidr 0x%jX\n", cpu, cs->cpu_index, cpu->mpidr);
+
 #ifndef CONFIG_USER_ONLY
     /* Our inbound IRQ and FIQ lines */
     if (kvm_enabled()) {
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 1cc4993..99c7a30 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -2063,12 +2063,9 @@ static const ARMCPRegInfo strongarm_cp_reginfo[] = {
 
 static uint64_t mpidr_read(CPUARMState *env, const ARMCPRegInfo *ri)
 {
-    CPUState *cs = CPU(arm_env_get_cpu(env));
-    uint32_t mpidr = cs->cpu_index;
-    /* We don't support setting cluster ID ([8..11]) (known as Aff1
-     * in later ARM ARM versions), or any of the higher affinity level fields,
-     * so these bits always RAZ.
-     */
+    ARMCPU *cpu = ARM_CPU(arm_env_get_cpu(env));
+    uint64_t mpidr = cpu->mpidr;
+
     if (arm_feature(env, ARM_FEATURE_V7MP)) {
         mpidr |= (1U << 31);
         /* Cores which are uniprocessor (non-coherent)
diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c
index 93c1ca8..99fa34e 100644
--- a/target-arm/kvm64.c
+++ b/target-arm/kvm64.c
@@ -25,6 +25,12 @@
 #include "internals.h"
 #include "hw/arm/arm.h"
 
+#ifdef DEBUG
+# define DPRINTF(format, ...) printf("arm-kvm64: " format , ## __VA_ARGS__)
+#else
+# define DPRINTF(format, ...) do { } while (0)
+#endif
+
 static inline void set_feature(uint64_t *features, int feature)
 {
     *features |= 1ULL << feature;
@@ -77,6 +83,10 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUClass *ahcc)
     return true;
 }
 
+#define ARM_MPIDR_HWID_BITMASK 0xFF00FFFFFFULL
+#define ARM_CPU_ID             3, 0, 0, 0
+#define ARM_CPU_ID_MPIDR       5
+
 int kvm_arch_init_vcpu(CPUState *cs)
 {
     int ret;
@@ -107,6 +117,21 @@ int kvm_arch_init_vcpu(CPUState *cs)
         return ret;
     }
 
+    /*
+     * When KVM is in use, psci is emulated in-kernel and not by qemu.
+     * In order for it to work correctly we should use correct MPIDR values,
+     * which appear to be inherited from the host.
+     * So we override our defaults by asking KVM about these values.
+     */
+    ret = kvm_get_one_reg(cs, ARM64_SYS_REG(ARM_CPU_ID, ARM_CPU_ID_MPIDR),
+                          &cpu->mpidr);
+    if (ret) {
+        return ret;
+    }
+    cpu->mpidr &= ARM_MPIDR_HWID_BITMASK;
+    DPRINTF("Init(%p): index %d MPIDR 0x%jX\n", cpu,
+            cs->cpu_index, cpu->mpidr);
+
     return kvm_arm_init_cpreg_list(cpu);
 }
 
diff --git a/target-arm/psci.c b/target-arm/psci.c
index d8fafab..d03896f 100644
--- a/target-arm/psci.c
+++ b/target-arm/psci.c
@@ -72,6 +72,22 @@ bool arm_is_psci_call(ARMCPU *cpu, int excp_type)
     }
 }
 
+static uint32_t mpidr_to_idx(uint64_t mpidr)
+{
+    uint32_t Aff1, Aff0;
+
+    /*
+     * MPIDR_EL1 [RES0:affinity-3:RES1:U:RES0:MT:affinity-1:affinity-0]
+     * GIC 500 code currently supports 32 clusters with 8 cores each
+     * but no more than 128 cores. Future version will have flexible
+     * affinity selection
+     * GIC 400 supports 8 cores so 0x7 for Aff0 is O.K. too
+     */
+     Aff1 = (mpidr & 0xff00) >> 8;
+     Aff0 = mpidr & 0xff;
+     return Aff1 * CPUS_PER_CLUSTER + Aff0;
+}
+
 void arm_handle_psci_call(ARMCPU *cpu)
 {
     /*
@@ -121,7 +137,7 @@ void arm_handle_psci_call(ARMCPU *cpu)
 
         switch (param[2]) {
         case 0:
-            target_cpu_state = qemu_get_cpu(mpidr & 0xff);
+            target_cpu_state = qemu_get_cpu(mpidr_to_idx(mpidr));
             if (!target_cpu_state) {
                 ret = QEMU_PSCI_RET_INVALID_PARAMS;
                 break;
@@ -153,7 +169,7 @@ void arm_handle_psci_call(ARMCPU *cpu)
         context_id = param[3];
 
         /* change to the cpu we are powering up */
-        target_cpu_state = qemu_get_cpu(mpidr & 0xff);
+        target_cpu_state = qemu_get_cpu(mpidr_to_idx(mpidr));
         if (!target_cpu_state) {
             ret = QEMU_PSCI_RET_INVALID_PARAMS;
             break;
-- 
1.9.1

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

* [Qemu-devel] [PATCH RFC V3 2/4] Implment GIC-500
  2015-06-04 16:40 [Qemu-devel] [PATCH RFC V3 0/4] Implement GIC-500 from GICv3 family for arm64 Shlomo Pongratz
  2015-06-04 16:40 ` [Qemu-devel] [PATCH RFC V3 1/4] Use Aff1 with mpidr This is an improved and KVM-aware alternative to Shlomo Pongratz
@ 2015-06-04 16:40 ` Shlomo Pongratz
  2015-06-04 16:40 ` [Qemu-devel] [PATCH RFC V3 3/4] GICv3 support Shlomo Pongratz
  2015-06-04 16:40 ` [Qemu-devel] [PATCH RFC V3 4/4] Add virt-v3 machine that uses GIC-500 Shlomo Pongratz
  3 siblings, 0 replies; 14+ messages in thread
From: Shlomo Pongratz @ 2015-06-04 16:40 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>

Implement GIC-500 from GICv3 family for arm64

This patch is a first step toward 128 cores support for arm64.

At first only 64 cores are supported for two reasons:
First the largest integer type has the size of 64 bits and modifying
essential data structures in order to support 128 cores will require
the usage of bitops.
Second currently the Linux (kernel) can be configured to support
up to 64 cores thus there is no urgency with 128 cores support.

Things left to do:

Currently the booting Linux may got stuck. The probability of getting stuck
increases with the number of cores. I'll appreciate core review.

Signed-off-by: Shlomo Pongratz <shlomo.pongratz@huawei.com>
---
 hw/intc/Makefile.objs              |    2 +
 hw/intc/arm_gicv3.c                | 2086 ++++++++++++++++++++++++++++++++++++
 hw/intc/arm_gicv3_common.c         |  217 ++++
 hw/intc/gicv3_internal.h           |  161 +++
 include/hw/intc/arm_gicv3.h        |   44 +
 include/hw/intc/arm_gicv3_common.h |  119 ++
 6 files changed, 2629 insertions(+)
 create mode 100644 hw/intc/arm_gicv3.c
 create mode 100644 hw/intc/arm_gicv3_common.c
 create mode 100644 hw/intc/gicv3_internal.h
 create mode 100644 include/hw/intc/arm_gicv3.h
 create mode 100644 include/hw/intc/arm_gicv3_common.h

diff --git a/hw/intc/Makefile.objs b/hw/intc/Makefile.objs
index 843864a..41fe9ec 100644
--- a/hw/intc/Makefile.objs
+++ b/hw/intc/Makefile.objs
@@ -11,6 +11,8 @@ common-obj-$(CONFIG_SLAVIO) += slavio_intctl.o
 common-obj-$(CONFIG_IOAPIC) += ioapic_common.o
 common-obj-$(CONFIG_ARM_GIC) += arm_gic_common.o
 common-obj-$(CONFIG_ARM_GIC) += arm_gic.o
+common-obj-$(CONFIG_ARM_GIC) += arm_gicv3_common.o
+common-obj-$(CONFIG_ARM_GIC) += arm_gicv3.o
 common-obj-$(CONFIG_OPENPIC) += openpic.o
 
 obj-$(CONFIG_APIC) += apic.o apic_common.o
diff --git a/hw/intc/arm_gicv3.c b/hw/intc/arm_gicv3.c
new file mode 100644
index 0000000..477be21
--- /dev/null
+++ b/hw/intc/arm_gicv3.c
@@ -0,0 +1,2086 @@
+/*
+ * 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"
+
+#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
+
+/* 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);
+
+
+static uint32_t gicv3_acknowledge_irq(GICv3State *s, int cpu, MemTxAttrs attrs);
+static void gicv3_complete_irq(GICv3State *s, int cpu, int irq,
+                               MemTxAttrs attrs);
+static void gicv3_update(GICv3State *s);
+static void gicv3_init_irqs_and_distributor(GICv3State *s, int num_irq);
+static void gicv3_set_priority(GICv3State *s, int cpu, int irq, uint8_t val,
+                               MemTxAttrs attrs);
+
+static const uint8_t gic_dist_ids[] = {
+    0x44, 0x00, 0x00, 0x00, 0x092, 0xB4, 0x3B, 0x00, 0x0D, 0xF0, 0x05, 0xB1
+};
+
+static const uint8_t gic_redist_ids[] = {
+    0x44, 0x00, 0x00, 0x00, 0x093, 0xB4, 0x3B, 0x00, 0x0D, 0xF0, 0x05, 0xB1
+};
+
+/* Cuurently no GICv2 backwards compatibility (no memory mapped regs)
+ * Uses system registers mode.
+ */
+static const int no_gicv2_bc = 1;
+static uint32_t gic_sre;
+
+#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
+ */
+static inline bool gic_has_groups(GICv3State *s)
+{
+    return 1;
+}
+
+/* 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;
+    uint64_t cm;
+
+    for (cpu = 0; cpu < NUM_CPU(s); cpu++) {
+        cm = 1ll << cpu;
+        s->current_pending[cpu] = 1023;
+        if (!(s->ctlr & (GICD_CTLR_EN_GRP0 | GICD_CTLR_EN_GRP1_ALL))
+            || !s->cpu_enabled[cpu]
+            || !(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, cm) && gic_test_pending(s, irq, cm) &&
+                (irq < GICV3_INTERNAL || (GIC_TARGET(irq) & cm))) {
+                if (GIC_GET_PRIORITY(irq, cpu) < best_prio) {
+                    best_prio = GIC_GET_PRIORITY(irq, cpu);
+                    best_irq = irq;
+                }
+            }
+        }
+
+        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, cm);
+                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,
+                                  uint64_t cm, uint64_t target)
+{
+    if (level) {
+        GIC_SET_LEVEL(irq, cm);
+        DPRINTF("Set %d pending mask 0x%lx\n", irq, target);
+        if (GIC_TEST_EDGE_TRIGGER(irq)) {
+            GIC_SET_PENDING(irq, target);
+        }
+    } else {
+        GIC_CLEAR_LEVEL(irq, cm);
+    }
+}
+
+/* Process a change in an external IRQ input.  */
+static void gic_set_irq(void *opaque, int irq, int level)
+{
+    /* Meaning of the 'irq' parameter:
+     *  [0..N-1] : external interrupts
+     *  [N..N+31] : PPI (internal) interrupts for CPU 0
+     *  [N+32..N+63] : PPI (internal interrupts for CPU 1
+     *  ...
+     */
+    GICv3State *s = (GICv3State *)opaque;
+    uint64_t cm, target;
+
+    if (irq < (s->num_irq - GICV3_INTERNAL)) {
+        /* The first external input line is internal interrupt 32.  */
+        cm = ALL_CPU_MASK;
+        irq += GICV3_INTERNAL;
+        target = GIC_TARGET(irq);
+    } else {
+        int cpu;
+        irq -= (s->num_irq - GICV3_INTERNAL);
+        cpu = irq / GICV3_INTERNAL;
+        irq %= GICV3_INTERNAL;
+        cm = 1ll << cpu;
+        target = cm;
+    }
+
+    assert(irq >= GICV3_NR_SGIS);
+
+    if (level == GIC_TEST_LEVEL(irq, cm)) {
+        return;
+    }
+
+    gicv3_set_irq_generic(s, irq, level, cm, target);
+
+    gicv3_update(s);
+}
+
+static 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, (1ll << 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);
+}
+
+static uint32_t gicv3_acknowledge_irq(GICv3State *s, int cpu, MemTxAttrs attrs)
+{
+    int ret, irq, src;
+    uint64_t cm = 1ll << cpu;
+
+    /* 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->last_active[irq][cpu] = s->running_irq[cpu];
+
+    if (irq < GICV3_NR_SGIS) {
+        /* Lookup the source CPU for the SGI and clear this in the
+         * sgi_pending map.  Return the src and clear the overall pending
+         * state on this CPU if the SGI is not pending from any CPUs.
+         */
+        assert(s->sgi_state[irq].pending[cpu] != 0);
+        src = ctz64(s->sgi_state[irq].pending[cpu]);
+        s->sgi_state[irq].pending[cpu] &= ~(1ll << src);
+        if (s->sgi_state[irq].pending[cpu] == 0) {
+            GIC_CLEAR_PENDING(irq, cm);
+        }
+        /* GICv3 kernel driver dosen't mask src bits like GICv2 driver
+         * so don't add src i.e. ret = irq | ((src & 0x7) << 10);
+         * Section 4.2.10 in GICv3 specification
+         */
+        ret = irq;
+    } else {
+        //DPRINTF("ACK irq(%d) cpu(%d) \n", irq, cpu);
+        /* Clear pending state for both level and edge triggered
+         * interrupts. (level triggered interrupts with an active line
+         * remain pending, see gic_test_pending)
+         */
+        GIC_CLEAR_PENDING(irq, cm);
+        ret = irq;
+    }
+
+    gic_set_running_irq(s, cpu, irq);
+    DPRINTF("out ACK irq-ret(%d) cpu(%d) \n", ret, cpu);
+    return ret;
+}
+
+static 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, (1ll << cpu))) {
+            return; /* Ignore Non-secure access of Group0 IRQ */
+        }
+        val = 0x80 | (val >> 1); /* Non-secure view */
+    }
+
+    if (irq < GICV3_INTERNAL) {
+        s->priority1[irq][cpu] = val;
+    } else {
+        s->priority2[irq - GICV3_INTERNAL] = val;
+    }
+}
+
+static void gicv3_complete_irq(GICv3State *s, int cpu, int irq, MemTxAttrs attrs)
+{
+    uint64_t cm = 1ll << cpu;
+    DPRINTF("EOI irq(%d) cpu (%d)\n", irq, cpu);
+    if (irq >= s->num_irq) {
+        /* This handles two cases:
+         * 1. If software writes the ID of a spurious interrupt [ie 1023]
+         * to the GICC_EOIR, the GIC ignores that write.
+         * 2. If software writes the number of a non-existent interrupt
+         * this must be a subcase of "value written does not match the last
+         * valid interrupt value read from the Interrupt Acknowledge
+         * register" and so this is UNPREDICTABLE. We choose to ignore it.
+         */
+        return;
+    }
+
+    if (s->running_irq[cpu] == 1023)
+        return; /* No active IRQ.  */
+
+    if (s->security_levels == 1 && !attrs.secure && !GIC_TEST_GROUP(irq, cm)) {
+        DPRINTF("Non-secure EOI for Group0 interrupt %d ignored\n", irq);
+        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->last_active[tmp][cpu] != 1023) {
+            if (s->last_active[tmp][cpu] == irq) {
+                s->last_active[tmp][cpu] = s->last_active[irq][cpu];
+                break;
+            }
+            tmp = s->last_active[tmp][cpu];
+        }
+    } else {
+        /* Complete the current running IRQ.  */
+        gic_set_running_irq(s, cpu, s->last_active[s->running_irq[cpu]][cpu]);
+    }
+}
+
+static uint64_t gic_dist_readb(void *opaque, hwaddr offset, MemTxAttrs attrs)
+{
+    GICv3State *s = (GICv3State *)opaque;
+    uint64_t res;
+    int irq;
+    int i;
+    int cpu;
+    uint64_t cm;
+    uint64_t mask;
+
+    cpu = gic_get_current_cpu(s);
+    cm = 1ll << cpu;
+    if (offset < 0x100) {
+        if (offset == 0) {/* GICD_CTLR */
+            /* 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, cm)) {
+                    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, cm)) {
+                res |= (1 << i);
+            }
+        }
+    } else if (offset < 0x300) {
+        /* Interrupt Set/Clear Pending.  */
+        if (offset < 0x280)
+            irq = (offset - 0x200) * 8;
+        else
+            irq = (offset - 0x280) * 8;
+        irq += GICV3_BASE_IRQ;
+        if (irq >= s->num_irq)
+            goto bad_reg;
+        /* Comment 'o' GICv2 backwards competability support is not set...*/
+        if (irq < GICV3_INTERNAL && no_gicv2_bc)
+            goto bad_reg;
+        res = 0;
+        mask = (irq < GICV3_INTERNAL) ?  cm : ALL_CPU_MASK;
+        for (i = 0; i < 8; i++) {
+            if (gic_test_pending(s, irq + i, mask)) {
+                res |= (1 << i);
+            }
+        }
+    } 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 && no_gicv2_bc)
+            goto bad_reg;
+        res = 0;
+        mask = (irq < GICV3_INTERNAL) ?  cm : ALL_CPU_MASK;
+        for (i = 0; i < 8; i++) {
+            if (GIC_TEST_ACTIVE(irq + i, mask)) {
+                res |= (1 << i);
+            }
+        }
+    } else if (offset < 0x800) {
+        /* Interrupt Priority.  */
+        irq = (offset - 0x400) + GICV3_BASE_IRQ;
+        if (irq >= s->num_irq)
+            goto bad_reg;
+        /* Comment 'p' GICv2 backwards competability support is not set...*/
+        if (irq < GICV3_INTERNAL * 8 && 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 && no_gicv2_bc)
+                goto bad_reg;
+            if (irq >= 29 && irq <= 31) {
+                res = cm;
+            } else {
+                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 && 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 (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 */
+        }
+        res = s->sgi_state[irq].pending[cpu];
+    } 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 = ctz64(s->irq_target[irq]);
+        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;
+}
+
+
+static 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 */
+                uint64_t cm = (irq < GICV3_INTERNAL) ? (1ll << 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)) {
+                uint64_t mask =
+                    (irq < GICV3_INTERNAL) ? (1ll << cpu) : GIC_TARGET(irq + i);
+                uint64_t cm = (irq < GICV3_INTERNAL) ? (1ll << cpu) : ALL_CPU_MASK;
+
+                if (!GIC_TEST_ENABLED(irq + i, cm)) {
+                    DPRINTF("Enabled IRQ %d\n", irq + i);
+                }
+                GIC_SET_ENABLED(irq + i, cm);
+                /* If a raised level triggered IRQ enabled then mark
+                   is as pending.  */
+                if (GIC_TEST_LEVEL(irq + i, mask)
+                        && !GIC_TEST_EDGE_TRIGGER(irq + i)) {
+                    DPRINTF("Set %d pending mask %lx\n", irq + i, mask);
+                    GIC_SET_PENDING(irq + i, mask);
+                }
+            }
+        }
+    } else if (offset < 0x200) {
+        /* Interrupt Clear Enable.  */
+        irq = (offset - 0x180) * 8 + GICV3_BASE_IRQ;
+        if (irq >= s->num_irq)
+            goto bad_reg;
+        if (irq < GICV3_NR_SGIS) {
+            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)) {
+                uint64_t cm = (irq < GICV3_INTERNAL) ? (1ll << cpu) : ALL_CPU_MASK;
+
+                if (GIC_TEST_ENABLED(irq + i, cm)) {
+                    DPRINTF("Disabled IRQ %d\n", irq + i);
+                }
+                GIC_CLEAR_ENABLED(irq + i, cm);
+            }
+        }
+    } else if (offset < 0x280) {
+        /* Interrupt Set Pending.  */
+        irq = (offset - 0x200) * 8 + GICV3_BASE_IRQ;
+        if (irq >= s->num_irq)
+            goto bad_reg;
+        /* Comment 'o' GICv2 backwards competability support is not set...*/
+        if (irq < GICV3_INTERNAL && no_gicv2_bc)
+            goto bad_reg;
+
+        for (i = 0; i < 8; i++) {
+            if (value & (1 << i)) {
+                GIC_SET_PENDING(irq + i, GIC_TARGET(irq + i));
+            }
+        }
+    } else if (offset < 0x300) {
+        /* Interrupt Clear Pending.  */
+        irq = (offset - 0x280) * 8 + GICV3_BASE_IRQ;
+        if (irq >= s->num_irq)
+            goto bad_reg;
+        /* Comment 'o' GICv2 backwards competability support is not set...*/
+        if (irq < GICV3_INTERNAL && 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 && 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 && no_gicv2_bc)
+                goto bad_reg;
+            if (irq < 29) {
+                value = 0;
+            } else if (irq < GICV3_INTERNAL) {
+                value = ALL_CPU_MASK;
+            }
+            s->irq_target[irq] = value & ALL_CPU_MASK;
+        }
+    } else if (offset < 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 && 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 (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);
+
+            s->sgi_state[irq].pending[cpu] &= ~value;
+            if (s->sgi_state[irq].pending[cpu] == 0) {
+                GIC_CLEAR_PENDING(irq, 1ll << cpu);
+            }
+        } else {
+            irq = (offset - 0xf20);
+            /* GICD_SPENDSGIRn */
+            DPRINTF("GICD_SPENDSGIRn irq(%d) %lu\n", irq, value);
+
+            GIC_SET_PENDING(irq, 1ll << cpu);
+            s->sgi_state[irq].pending[cpu] |= 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 doc)
+         */
+        int cpu;
+        int irq;
+        uint64_t mask, cm;
+        int target_cpu;
+        GICv3State *s = (GICv3State *)opaque;
+
+        DPRINTF("GICv2 backwards computability is not supported\n");
+        cpu = gic_get_current_cpu(s);
+        irq = value & 0x3ff;
+        switch ((value >> 24) & 3) {
+        case 0:
+            mask = (value >> 16) & ALL_CPU_MASK;
+            break;
+        case 1:
+            mask = ALL_CPU_MASK ^ (1ll << cpu);
+            break;
+        case 2:
+            mask = 1ll << cpu;
+            break;
+        default:
+            DPRINTF("Bad Soft Int target filter\n");
+            mask = ALL_CPU_MASK;
+            break;
+        }
+        cm = (1ll << cpu);
+        DPRINTF("irq(%d) mask(%lu)\n", irq, mask);
+        GIC_SET_PENDING(irq, mask);
+        target_cpu = ctz64(mask);
+        while (target_cpu < GICV3_NCPU) {
+            s->sgi_state[irq].pending[target_cpu] |= cm;
+            mask &= ~(1ll << target_cpu);
+            target_cpu = ctz64(mask);
+        }
+        gicv3_update(s);
+        return;
+    }
+    gic_dist_writew(opaque, offset, value & 0xffff, 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;
+        s->irq_target[irq] = 1ll << cpu;
+        gicv3_update(s);
+        DPRINTF("irq(%d) cpu(%d)\n", irq, cpu);
+        return;
+    }
+
+    gic_dist_writel(opaque, offset, value & 0xffffffff, attrs);
+    gic_dist_writel(opaque, offset + 4, value >> 32, attrs);
+}
+
+static 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;
+    }
+}
+
+static const MemoryRegionOps gic_dist_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,
+};
+
+/* 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);
+}
+
+static uint64_t gic_its_read(void *opaque, hwaddr addr, unsigned size)
+{
+    uint64_t data;
+    switch (size) {
+    case 1:
+        data = gic_its_readb(opaque, addr);
+        break;
+    case 2:
+        data = gic_its_readw(opaque, addr);
+        break;
+    case 4:
+        data = gic_its_readl(opaque, addr);
+        break;
+    case 8:
+        data = gic_its_readll(opaque, addr);
+        break;
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: size %u\n", __func__, size);
+        assert(0);
+        break;
+    }
+    DPRINTF("offset %p data %p\n", (void *) addr, (void *) data);
+    return data;
+}
+
+static void gic_its_write(void *opaque, hwaddr addr, uint64_t data, unsigned size)
+{
+    DPRINTF("offset %p data %p\n", (void *) addr, (void *) data);
+    switch (size) {
+    case 1:
+        gic_its_writew(opaque, addr, data);
+        break;
+    case 2:
+        gic_its_writew(opaque, addr, data);
+        break;
+    case 4:
+        gic_its_writel(opaque, addr, data);
+        break;
+    case 8:
+        gic_its_writell(opaque, addr, data);
+        break;
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: size %u\n", __func__, size);
+        assert(0);
+        break;
+    }
+}
+
+static const MemoryRegionOps gic_its_ops = {
+    .read = gic_its_read,
+    .write = gic_its_write,
+    .impl = {
+         .min_access_size = 4,
+         .max_access_size = 8,
+     },
+    .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+/* 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);
+}
+
+static uint64_t gic_spi_read(void *opaque, hwaddr addr, unsigned size)
+{
+    uint64_t data;
+    switch (size) {
+    case 1:
+        data = gic_spi_readb(opaque, addr);
+        break;
+    case 2:
+        data = gic_spi_readw(opaque, addr);
+        break;
+    case 4:
+        data = gic_spi_readl(opaque, addr);
+        break;
+    case 8:
+        data = gic_spi_readll(opaque, addr);
+        break;
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: size %u\n", __func__, size);
+        assert(0);
+        break;
+    }
+    DPRINTF("offset %p data %p\n", (void *) addr, (void *) data);
+    return data;
+}
+
+static void gic_spi_write(void *opaque, hwaddr addr, uint64_t data, unsigned size)
+{
+    DPRINTF("offset %p data %p\n", (void *) addr, (void *) data);
+    switch (size) {
+    case 1:
+        gic_spi_writeb(opaque, addr, data);
+        break;
+    case 2:
+        gic_spi_writew(opaque, addr, data);
+        break;
+    case 4:
+        gic_spi_writel(opaque, addr, data);
+        break;
+    case 8:
+        gic_spi_writell(opaque, addr, data);
+        break;
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: size %u\n", __func__, size);
+        assert(0);
+        break;
+    }
+}
+
+static const MemoryRegionOps gic_spi_ops = {
+    .read = gic_spi_read,
+    .write = gic_spi_write,
+    .impl = {
+         .min_access_size = 4,
+         .max_access_size = 8,
+     },
+    .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+/* 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);
+}
+
+static uint64_t gic_its_cntrl_read(void *opaque, hwaddr addr, unsigned size)
+{
+    uint64_t data;
+    switch (size) {
+    case 1:
+        data = gic_its_cntrl_readb(opaque, addr);
+        break;
+    case 2:
+        data = gic_its_cntrl_readw(opaque, addr);
+        break;
+    case 4:
+        data = gic_its_cntrl_readl(opaque, addr);
+        break;
+    case 8:
+        data = gic_its_cntrl_readll(opaque, addr);
+        break;
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: size %u\n", __func__, size);
+        assert(0);
+        break;
+    }
+    DPRINTF("offset %p data %p\n", (void *) addr, (void *) data);
+    return data;
+}
+
+static void gic_its_cntrl_write(void *opaque, hwaddr addr, uint64_t data, unsigned size)
+{
+    DPRINTF("offset %p data %p\n", (void *) addr, (void *) data);
+    switch (size) {
+    case 1:
+        gic_its_cntrl_writeb(opaque, addr, data);
+        break;
+    case 2:
+        gic_its_cntrl_writew(opaque, addr, data);
+        break;
+    case 4:
+        gic_its_cntrl_writel(opaque, addr, data);
+        break;
+    case 8:
+        gic_its_cntrl_writell(opaque, addr, data);
+        break;
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: size %u\n", __func__, size);
+        assert(0);
+        break;
+    }
+}
+
+static const MemoryRegionOps gic_its_cntrl_ops = {
+    .read = gic_its_cntrl_read,
+    .write = gic_its_cntrl_write,
+    .impl = {
+         .min_access_size = 4,
+         .max_access_size = 8,
+     },
+    .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+
+static uint64_t gic_redist_readb(void *opaque, hwaddr offset, MemTxAttrs attrs)
+{
+    GICv3State *s = (GICv3State *)opaque;
+    uint64_t res = 0;
+    uint64_t sgi_ppi, core, off;
+    uint64_t cm, 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;
+    cm = 1ll << core;
+
+    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, cm)) {
+                        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, cm)) {
+                    res |= (1 << i);
+                }
+            }
+        } else if (offset < 0x300) {
+            /* Interrupt Set/Clear Pending.  */
+            if (offset < 0x280)
+                irq = (offset - 0x200) * 8;
+            else
+                irq = (offset - 0x280) * 8;
+            irq += GICV3_BASE_IRQ;
+            if (irq >= s->num_irq)
+                goto bad_reg;
+            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, 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;
+            if (irq >= GICV3_INTERNAL)
+                goto bad_reg;
+            res = 0;
+            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;
+            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 */
+                res = core << 8; /* Linear */
+                /* Simple clustering */
+                res |= (core % 8) << 32; /* Afinity 0 */
+                res |= (core / 8) << 40; /* Afinity 1 */
+                if (core == s->num_cpu - 1) {
+                    /* Last redistributer */
+                    res |= 1 << 4;
+                }
+                return res;
+            }
+            if (offset == 0xc) { /* GICR_TYPER */
+                /* should write readll */
+                return 0;
+            }
+            if (offset == 0x14) { /* GICR_WAKER */
+                if (s->cpu_enabled[core])
+                    return 0;
+                else
+                    return GICR_WAKER_ProcessorSleep;
+                DPRINTF("Redist-CPU (%d) is enabled(%d)\n",
+                        gic_get_current_cpu(s), s->cpu_enabled[core]);
+
+            }
+            if (offset >= 0x80 && offset < 0xFFD0)
+                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;
+}
+
+static 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;
+    uint64_t cm;
+    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;
+    cm = 1ll << core;
+
+    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, cm);
+                    } else {
+                        /* Group0 (Secure) */
+                        GIC_CLEAR_GROUP(irq + i, cm);
+                    }
+                }
+            }
+        } 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, cm)) {
+                        DPRINTF("Enabled IRQ %d\n", irq + i);
+                    }
+                    GIC_SET_ENABLED(irq + i, cm);
+                    /* If a raised level triggered IRQ enabled then mark
+                       is as pending.  */
+                    if (GIC_TEST_LEVEL(irq + i, cm)
+                            && !GIC_TEST_EDGE_TRIGGER(irq + i)) {
+                        DPRINTF("Set %d pending mask %lx\n", irq + i, cm);
+                        GIC_SET_PENDING(irq + i, cm);
+                    }
+                }
+            }
+        } else if (offset < 0x200) {
+            /* Interrupt Clear Enable.  */
+            irq = (offset - 0x180) * 8 + GICV3_BASE_IRQ;
+            if (irq >= s->num_irq)
+                goto bad_reg;
+            if (irq >= GICV3_INTERNAL) {
+                DPRINTF("ICENABLERn non Internal should be only in distributer %d\n", irq);
+                /* The registers after 0x180 are reserved */
+                return;
+            }
+            if (irq < GICV3_NR_SGIS) {
+                value = 0;
+            }
+
+            for (i = 0; i < 8; i++) {
+                if (value & (1 << i)) {
+                    /* This is redistributer ALL doesn't apply */
+                    if (GIC_TEST_ENABLED(irq + i, cm)) {
+                        DPRINTF("Disabled IRQ %d\n", irq + i);
+                    }
+                    GIC_CLEAR_ENABLED(irq + i, cm);
+                }
+            }
+            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(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, cm);
+                }
+            }
+        } 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)
+                s->cpu_enabled[core] = 0;
+            else
+                s->cpu_enabled[core] = 1;
+            DPRINTF("Redist-CPU (%d) core(%lu) set enabled(%d)\n",
+                    gic_get_current_cpu(s), core, s->cpu_enabled[core]);
+       }
+    }
+    gicv3_update(s);
+    return;
+
+bad_reg:
+    qemu_log_mask(LOG_GUEST_ERROR,
+                  "%s: Bad offset %x\n", __func__, (int)offset);
+}
+
+static void gic_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);
+}
+
+static 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;
+    }
+}
+
+static const MemoryRegionOps gic_redist_ops = {
+    .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,
+};
+
+void gicv3_init_irqs_and_distributor(GICv3State *s, int num_irq)
+{
+    SysBusDevice *sbd = SYS_BUS_DEVICE(s);
+    int i;
+
+    DPRINTF(" ---- gicv3_init_irqs_and_distributor   ----- \n");
+    i = s->num_irq - GICV3_INTERNAL;
+    /* For the GIC, also expose incoming GPIO lines for PPIs for each CPU.
+     * GPIO array layout is thus:
+     *  [0..N-1] spi
+     *  [N..N+31] PPIs for CPU 0
+     *  [N+32..N+63] PPIs for CPU 1
+     *   ...
+     */
+    i += (GICV3_INTERNAL * s->num_cpu);
+    qdev_init_gpio_in(DEVICE(s), gic_set_irq, i);
+    for (i = 0; i < NUM_CPU(s); i++) {
+        sysbus_init_irq(sbd, &s->parent_irq[i]);
+    }
+    for (i = 0; i < NUM_CPU(s); i++) {
+        sysbus_init_irq(sbd, &s->parent_fiq[i]);
+    }
+
+    memory_region_init_io(&s->iomem_dist, OBJECT(s), &gic_dist_ops, s,"gic_dist", 0x10000);
+    memory_region_init_io(&s->iomem_spi, OBJECT(s), &gic_spi_ops, s,"gic_spi", 0x10000);
+    memory_region_init_io(&s->iomem_its_cntrl, OBJECT(s), &gic_its_cntrl_ops, s,"gic_its_cntrl", 0x10000);
+    memory_region_init_io(&s->iomem_its, OBJECT(s), &gic_its_ops, s,"gic_its_trans", 0x10000);
+    memory_region_init_io(&s->iomem_lpi, OBJECT(s), &gic_redist_ops, s,"gic_redist", 0x800000);
+}
+
+static void arm_gic_realize(DeviceState *dev, Error **errp)
+{
+    /* Device instance realize function for the GIC sysbus device */
+    int i;
+    GICv3State *s = ARM_GICV3(dev);
+    SysBusDevice *sbd = SYS_BUS_DEVICE(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 registers mode
+     */
+    gic_sre = 1;
+
+    /* Tell the common code we're a GICv3 */
+    s->revision = REV_V3;
+
+    gicv3_init_irqs_and_distributor(s, s->num_irq);
+
+    /* Compute mask for decoding the core number in redistributer */
+    if (is_power_of_2(NUM_CPU(s)))
+        power2 = NUM_CPU(s);
+    else
+        /* QEMU has only  pow2floor !!! */
+        power2 = pow2floor(2 * NUM_CPU(s));
+    s->cpu_mask = (power2 - 1);
+
+    DPRINTF(" -- NUM_CPUS(%d) - cpu mask(0%x) -- \n", NUM_CPU(s), s->cpu_mask);
+
+    for (i = 0; i < GICV3_NCPU; i++) {
+        s->cpu_enabled[i] = 0;
+    }
+
+    /* Init memory regions */
+    sysbus_init_mmio(sbd, &s->iomem_dist);
+    sysbus_init_mmio(sbd, &s->iomem_spi);
+    sysbus_init_mmio(sbd, &s->iomem_its_cntrl);
+    sysbus_init_mmio(sbd, &s->iomem_its);
+    sysbus_init_mmio(sbd, &s->iomem_lpi);
+}
+
+void armv8_gicv3_set_sgi(void *opaque, int cpuindex, uint64_t value)
+{
+    GICv3State *s = (GICv3State *) opaque;
+    uint64_t cm = (1ll << cpuindex);
+    uint32_t target;
+    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++) {
+            s->sgi_state[irq].pending[i] |= cm;
+        }
+        for (i = cpuindex + 1; i < s->num_cpu; i++) {
+            s->sgi_state[irq].pending[i] |= cm;
+        }
+        GIC_SET_PENDING(irq, (ALL_CPU_MASK & ~cm));
+        DPRINTF("cpu(%d) sends irq(%d) to ALL exclude self\n", cpuindex, irq);
+    } else {
+        /* Find linear of first core in cluster. See page 2227 ICC_SGI1R_EL1
+         * With our GIC-500 implementation we can have 16 clusters of 8 cpu each
+         */
+#if 1
+        target = (value & (0xfl << 16)) >> (16 - 3); /* shift 16 mult by 8 */
+#else
+        /* Prep for more advanced GIC */
+        target  = (value & (0xffl << 16)) >> (16 - 8);
+        target |= (value & (0xffl << 32)) >> (32 - 16);
+        target |= (value & (0xffl << 48)) >> (48 - 24);
+#endif
+
+        /* Use 8 and not 16 since only 8 cores can be in a cluster of GIC-500 */
+        assert((value & 0xff00) == 0);
+        for (i = 0; i < 8; i++) {
+            if (value & (1 << i)) {
+                s->sgi_state[irq].pending[target + i] |= cm;
+                GIC_SET_PENDING(irq, (1ll << (target + i)));
+             }
+         }
+    }
+    gicv3_update(s);
+}
+
+uint64_t armv8_gicv3_acknowledge_irq(void *opaque, int cpuindex,
+                                     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 gic_sre;
+}
+
+void armv8_gicv3_set_sre(void *opaque, uint64_t sre)
+{
+    if (!(sre & 1) && 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);
+    }
+    gic_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");
+}
+
+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)
diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
new file mode 100644
index 0000000..50e8f3e
--- /dev/null
+++ b/hw/intc/arm_gicv3_common.c
@@ -0,0 +1,217 @@
+/*
+ * ARM GIC support - common bits of emulated and KVM kernel model
+ *
+ * Copyright (c) 2012 Linaro Limited
+ * Copyright (c) 2015 Huawei.
+ * Written by Peter Maydell
+ * Extended to 64 cores by Shlomo Pongratz
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "gicv3_internal.h"
+
+static void gicv3_pre_save(void *opaque)
+{
+    GICv3State *s = (GICv3State *)opaque;
+    ARMGICv3CommonClass *c = ARM_GICV3_COMMON_GET_CLASS(s);
+
+    if (c->pre_save) {
+        c->pre_save(s);
+    }
+}
+
+static int gicv3_post_load(void *opaque, int version_id)
+{
+    GICv3State *s = (GICv3State *)opaque;
+    ARMGICv3CommonClass *c = ARM_GICV3_COMMON_GET_CLASS(s);
+
+    if (c->post_load) {
+        c->post_load(s);
+    }
+    return 0;
+}
+
+static const VMStateDescription vmstate_gicv3_irq_state = {
+    .name = "arm_gicv3_irq_state",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT64(pending, gicv3_irq_state),
+        VMSTATE_UINT64(active, gicv3_irq_state),
+        VMSTATE_UINT64(level, gicv3_irq_state),
+        VMSTATE_UINT64(group, gicv3_irq_state),
+        VMSTATE_BOOL(edge_trigger, gicv3_irq_state),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static const VMStateDescription vmstate_gicv3_sgi_state = {
+    .name = "arm_gicv3_sgi_state",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT64_ARRAY(pending, gicv3_sgi_state, GICV3_NCPU),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static const VMStateDescription vmstate_gicv3 = {
+    .name = "arm_gicv3",
+    .version_id = 7,
+    .minimum_version_id = 7,
+    .pre_save = gicv3_pre_save,
+    .post_load = gicv3_post_load,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32(ctlr, GICv3State),
+        VMSTATE_UINT32_ARRAY(cpu_ctlr, GICv3State, GICV3_NCPU),
+        VMSTATE_STRUCT_ARRAY(irq_state, GICv3State, GICV3_MAXIRQ, 1,
+                             vmstate_gicv3_irq_state, gicv3_irq_state),
+        VMSTATE_UINT64_ARRAY(irq_target, GICv3State, GICV3_MAXIRQ),
+        VMSTATE_UINT8_2DARRAY(priority1, GICv3State, GICV3_INTERNAL, GICV3_NCPU),
+        VMSTATE_UINT8_ARRAY(priority2, GICv3State, GICV3_MAXIRQ - GICV3_INTERNAL),
+        VMSTATE_UINT16_2DARRAY(last_active, GICv3State, GICV3_MAXIRQ, GICV3_NCPU),
+        VMSTATE_STRUCT_ARRAY(sgi_state, GICv3State, GICV3_NR_SGIS, 1,
+                             vmstate_gicv3_sgi_state, gicv3_sgi_state),
+        VMSTATE_UINT16_ARRAY(priority_mask, GICv3State, GICV3_NCPU),
+        VMSTATE_UINT16_ARRAY(running_irq, GICv3State, GICV3_NCPU),
+        VMSTATE_UINT16_ARRAY(running_priority, GICv3State, GICV3_NCPU),
+        VMSTATE_UINT16_ARRAY(current_pending, GICv3State, GICV3_NCPU),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static void arm_gicv3_common_realize(DeviceState *dev, Error **errp)
+{
+    GICv3State *s = ARM_GICV3_COMMON(dev);
+    int num_irq = s->num_irq;
+
+    if (s->num_cpu > GICV3_NCPU) {
+        error_setg(errp, "requested %u CPUs exceeds GIC maximum %d",
+                   s->num_cpu, GICV3_NCPU);
+        return;
+    }
+    s->num_irq += GICV3_BASE_IRQ;
+    if (s->num_irq > GICV3_MAXIRQ) {
+        error_setg(errp,
+                   "requested %u interrupt lines exceeds GIC maximum %d",
+                   num_irq, GICV3_MAXIRQ);
+        return;
+    }
+    /* ITLinesNumber is represented as (N / 32) - 1 (see
+     * gic_dist_readb) so this is an implementation imposed
+     * restriction, not an architectural one:
+     */
+    if (s->num_irq < 32 || (s->num_irq % 32)) {
+        error_setg(errp,
+                   "%d interrupt lines unsupported: not divisible by 32",
+                   num_irq);
+        return;
+    }
+}
+
+static void arm_gicv3_common_reset(DeviceState *dev)
+{
+    GICv3State *s = ARM_GICV3_COMMON(dev);
+    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;
+    }
+
+    memset(s->irq_state, 0, sizeof(s->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);
+    }
+    memset(s->sgi_state, 0, sizeof(s->sgi_state));
+    memset(s->irq_target, 0, sizeof(s->irq_target));
+    if (s->num_cpu == 1) {
+        /* For uniprocessor GICs all interrupts always target the sole CPU */
+        for (i = 0; i < GICV3_MAXIRQ; i++) {
+            s->irq_target[i] = 1;
+        }
+    }
+    memset(s->priority1, 0, sizeof(s->priority1));
+    memset(s->priority2, 0, sizeof(s->priority2));
+    memset(s->last_active, 0, sizeof(s->last_active));
+
+    /* 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),
+    /* Revision can be 3 for GIC architecture specification
+     * versions 1 or 2, or 0 to indicate the legacy 11MPCore GIC.
+     * (Internally, 0xffffffff also indicates "not a GIC but an NVIC".)
+     */
+    DEFINE_PROP_UINT32("revision", GICv3State, revision, 3),
+    /* How many security levels are supported */
+    DEFINE_PROP_BOOL("security-levels", GICv3State, security_levels, 0),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void arm_gicv3_common_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->reset = arm_gicv3_common_reset;
+    dc->realize = arm_gicv3_common_realize;
+    dc->props = arm_gicv3_common_properties;
+    dc->vmsd = &vmstate_gicv3;
+}
+
+static const TypeInfo arm_gicv3_common_type = {
+    .name = TYPE_ARM_GICV3_COMMON,
+    .parent = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(GICv3State),
+    .class_size = sizeof(ARMGICv3CommonClass),
+    .class_init = arm_gicv3_common_class_init,
+    .abstract = true,
+};
+
+static void register_types(void)
+{
+    type_register_static(&arm_gicv3_common_type);
+}
+
+type_init(register_types)
diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h
new file mode 100644
index 0000000..f41a889
--- /dev/null
+++ b/hw/intc/gicv3_internal.h
@@ -0,0 +1,161 @@
+/*
+ * ARM GIC support - internal interfaces
+ *
+ * Copyright (c) 2012 Linaro Limited
+ * Copyright (c) 2015 Huawei.
+ * Written by Peter Maydell
+ * Extended to 64 cores by Shlomo Pongratz
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef QEMU_ARM_GICV3_INTERNAL_H
+#define QEMU_ARM_GICV3_INTERNAL_H
+
+#include "hw/intc/arm_gicv3.h"
+
+#define ALL_CPU_MASK ((uint64_t) (0xffffffffffffffff))
+
+/* 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)
+
+#define GIC_SET_ENABLED(irq, cm) s->irq_state[irq].enabled |= (cm)
+#define GIC_CLEAR_ENABLED(irq, cm) s->irq_state[irq].enabled &= ~(cm)
+#define GIC_TEST_ENABLED(irq, cm) ((s->irq_state[irq].enabled & (cm)) != 0)
+#define GIC_SET_PENDING(irq, cm) s->irq_state[irq].pending |= (cm)
+#define GIC_TEST_PENDING(irq, cm) ((s->irq_state[irq].pending & (cm)) != 0)
+#define GIC_CLEAR_PENDING(irq, cm) s->irq_state[irq].pending &= ~(cm)
+#define GIC_SET_ACTIVE(irq, cm) s->irq_state[irq].active |= (cm)
+#define GIC_CLEAR_ACTIVE(irq, cm) s->irq_state[irq].active &= ~(cm)
+#define GIC_TEST_ACTIVE(irq, cm) ((s->irq_state[irq].active & (cm)) != 0)
+#define GIC_SET_LEVEL(irq, cm) s->irq_state[irq].level |= (cm)
+#define GIC_CLEAR_LEVEL(irq, cm) s->irq_state[irq].level &= ~(cm)
+#define GIC_TEST_LEVEL(irq, cm) ((s->irq_state[irq].level & (cm)) != 0)
+#define GIC_SET_EDGE_TRIGGER(irq) s->irq_state[irq].edge_trigger = true
+#define GIC_CLEAR_EDGE_TRIGGER(irq) s->irq_state[irq].edge_trigger = false
+#define GIC_TEST_EDGE_TRIGGER(irq) (s->irq_state[irq].edge_trigger)
+#define GIC_GET_PRIORITY(irq, cpu) (((irq) < GICV3_INTERNAL) ?          \
+                                    s->priority1[irq][cpu] :            \
+                                    s->priority2[(irq) - GICV3_INTERNAL])
+#define GIC_TARGET(irq) s->irq_target[irq]
+#define GIC_CLEAR_GROUP(irq, cm) (s->irq_state[irq].group &= ~(cm))
+#define GIC_SET_GROUP(irq, cm) (s->irq_state[irq].group |= (cm))
+#define GIC_TEST_GROUP(irq, cm) ((s->irq_state[irq].group & (cm)) != 0)
+
+/* The special cases for the revision property: */
+#define REV_V3 3
+
+static inline bool gic_test_pending(GICv3State *s, int irq, uint64_t cm)
+{
+    /* Edge-triggered interrupts are marked pending on a rising edge, but
+     * level-triggered interrupts are either considered pending when the
+     * level is active or if software has explicitly written to
+     * GICD_ISPENDR to set the state pending.
+     */
+    return (s->irq_state[irq].pending & cm) ||
+        (!GIC_TEST_EDGE_TRIGGER(irq) && GIC_TEST_LEVEL(irq, cm));
+}
+
+
+#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
+
+/*
+ * 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
new file mode 100644
index 0000000..ca2a5e5
--- /dev/null
+++ b/include/hw/intc/arm_gicv3_common.h
@@ -0,0 +1,119 @@
+/*
+ * ARM GIC support
+ *
+ * Copyright (c) 2012 Linaro Limited
+ * Copyright (c) 2015 Huawei.
+ * Written by Peter Maydell
+ * Extended to 64 cores by Shlomo Pongratz
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef HW_ARM_GICV3_COMMON_H
+#define HW_ARM_GICV3_COMMON_H
+
+#include "hw/sysbus.h"
+
+/* Maximum number of possible interrupts, determined by the GIC architecture */
+#define GICV3_MAXIRQ 1020
+/* First 32 are private to each CPU (SGIs and PPIs). */
+#define GICV3_INTERNAL 32
+#define GICV3_NR_SGIS 16
+#define GICV3_NCPU 64
+
+#define MAX_NR_GROUP_PRIO 128
+
+typedef struct gicv3_irq_state {
+    /* The enable bits are only banked for per-cpu interrupts.  */
+    uint64_t enabled;
+    uint64_t pending;
+    uint64_t active;
+    uint64_t level;
+    uint64_t group;
+    bool edge_trigger; /* true: edge-triggered, false: level-triggered  */
+} gicv3_irq_state;
+
+typedef struct gicv3_sgi_state {
+    uint64_t pending[GICV3_NCPU];
+} gicv3_sgi_state;
+
+typedef struct GICv3State {
+    /*< private >*/
+    SysBusDevice parent_obj;
+    /*< public >*/
+
+    qemu_irq parent_irq[GICV3_NCPU];
+    qemu_irq parent_fiq[GICV3_NCPU];
+    /* 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[GICV3_NCPU];
+    bool cpu_enabled[GICV3_NCPU];
+
+    gicv3_irq_state irq_state[GICV3_MAXIRQ];
+    uint64_t irq_target[GICV3_MAXIRQ];
+    uint8_t priority1[GICV3_INTERNAL][GICV3_NCPU];
+    uint8_t priority2[GICV3_MAXIRQ - GICV3_INTERNAL];
+    uint16_t last_active[GICV3_MAXIRQ][GICV3_NCPU];
+    /* For each SGI on the target CPU, we store 64 bits
+     * indicating which source CPUs have made this SGI
+     * pending on the target CPU. These correspond to
+     * the bytes in the GIC_SPENDSGIR* registers as
+     * read by the target CPU.
+     */
+    gicv3_sgi_state sgi_state[GICV3_NR_SGIS];
+
+    uint16_t priority_mask[GICV3_NCPU];
+    uint16_t running_irq[GICV3_NCPU];
+    uint16_t running_priority[GICV3_NCPU];
+    uint16_t current_pending[GICV3_NCPU];
+
+    uint32_t cpu_mask; /* For redistributer */
+    uint32_t num_cpu;
+    MemoryRegion iomem_dist; /* Distributor */
+    MemoryRegion iomem_spi;
+    MemoryRegion iomem_its_cntrl;
+    MemoryRegion iomem_its;
+    MemoryRegion iomem_lpi; /* Redistributor */
+    /* This is just so we can have an opaque pointer which identifies
+     * both this GIC and which CPU interface we should be accessing.
+     */
+    uint32_t num_irq;
+    uint32_t revision;
+    bool security_levels;
+    int dev_fd; /* kvm device fd if backed by kvm vgic support */
+} GICv3State;
+
+#define TYPE_ARM_GICV3_COMMON "arm_gicv3_common"
+#define ARM_GICV3_COMMON(obj) \
+     OBJECT_CHECK(GICv3State, (obj), TYPE_ARM_GICV3_COMMON)
+#define ARM_GICV3_COMMON_CLASS(klass) \
+     OBJECT_CLASS_CHECK(ARMGICv3CommonClass, (klass), TYPE_ARM_GICV3_COMMON)
+#define ARM_GICV3_COMMON_GET_CLASS(obj) \
+     OBJECT_GET_CLASS(ARMGICv3CommonClass, (obj), TYPE_ARM_GICV3_COMMON)
+
+typedef struct ARMGICv3CommonClass {
+    /*< private >*/
+    SysBusDeviceClass parent_class;
+    /*< public >*/
+
+    void (*pre_save)(GICv3State *s);
+    void (*post_load)(GICv3State *s);
+} ARMGICv3CommonClass;
+
+#endif
-- 
1.9.1

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

* [Qemu-devel] [PATCH RFC V3 3/4] GICv3 support
  2015-06-04 16:40 [Qemu-devel] [PATCH RFC V3 0/4] Implement GIC-500 from GICv3 family for arm64 Shlomo Pongratz
  2015-06-04 16:40 ` [Qemu-devel] [PATCH RFC V3 1/4] Use Aff1 with mpidr This is an improved and KVM-aware alternative to Shlomo Pongratz
  2015-06-04 16:40 ` [Qemu-devel] [PATCH RFC V3 2/4] Implment GIC-500 Shlomo Pongratz
@ 2015-06-04 16:40 ` Shlomo Pongratz
  2015-06-04 16:40 ` [Qemu-devel] [PATCH RFC V3 4/4] Add virt-v3 machine that uses GIC-500 Shlomo Pongratz
  3 siblings, 0 replies; 14+ messages in thread
From: Shlomo Pongratz @ 2015-06-04 16:40 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.h   |  12 ++++++
 target-arm/cpu64.c | 105 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 117 insertions(+)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 21b5b8e..810490d 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -1013,6 +1013,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 bf7dd68..2bce6f6 100644
--- a/target-arm/cpu64.c
+++ b/target-arm/cpu64.c
@@ -45,6 +45,72 @@ 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(env->nvic, 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(env->nvic, cpu->cpu_index, attrs);
+    return value;
+}
+
+static void sre_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
+{
+    armv8_gicv3_set_sre(env->nvic, value);
+}
+
+static uint64_t sre_read(CPUARMState *env, const ARMCPRegInfo *ri)
+{
+    uint64_t value;
+    value = armv8_gicv3_get_sre(env->nvic);
+    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(env->nvic, 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(env->nvic, 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(env->nvic, 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(env->nvic, 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(env->nvic, cpu->cpu_index, value);
+}
+#endif
+
 static const ARMCPRegInfo cortex_a57_a53_cp_reginfo[] = {
 #ifndef CONFIG_USER_ONLY
     { .name = "L2CTLR_EL1", .state = ARM_CP_STATE_AA64,
@@ -89,6 +155,45 @@ static const ARMCPRegInfo cortex_a57_a53_cp_reginfo[] = {
     { .name = "L2MERRSR",
       .cp = 15, .opc1 = 3, .crm = 15,
       .access = PL1_RW, .type = ARM_CP_CONST | ARM_CP_64BIT, .resetvalue = 0 },
+    { .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
 };
 
-- 
1.9.1

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

* [Qemu-devel] [PATCH RFC V3 4/4] Add virt-v3 machine that uses GIC-500
  2015-06-04 16:40 [Qemu-devel] [PATCH RFC V3 0/4] Implement GIC-500 from GICv3 family for arm64 Shlomo Pongratz
                   ` (2 preceding siblings ...)
  2015-06-04 16:40 ` [Qemu-devel] [PATCH RFC V3 3/4] GICv3 support Shlomo Pongratz
@ 2015-06-04 16:40 ` Shlomo Pongratz
  3 siblings, 0 replies; 14+ messages in thread
From: Shlomo Pongratz @ 2015-06-04 16:40 UTC (permalink / raw)
  To: qemu-devel
  Cc: peter.maydell, eric.auger, p.fedin, shannon.zhao, ashoks, imammedo

From: Pavel Fedin <p.fedin@samsung.com>

I would like to offer this, slightly improved implementation. The key thing is a new
kernel_irqchip_type member in Machine class. Currently it it used only by virt machine for
its internal purposes, however in future it is to be passed to KVM in
kvm_irqchip_create(). The variable is defined as int in order to be architecture agnostic,
for potential future users.

Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
---
 hw/arm/virt.c         | 159 +++++++++++++++++++++++++++++++++++++++++---------
 include/hw/arm/virt.h |   4 ++
 include/hw/boards.h   |   1 +
 3 files changed, 135 insertions(+), 29 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 19c8c3a..22a39b4 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -45,6 +45,7 @@
 #include "qemu/error-report.h"
 #include "hw/pci-host/gpex.h"
 #include "hw/arm/virt-acpi-build.h"
+#include "linux/kvm.h" /* For KVM_DEV_TYPE_ARM_VGIC_V{2|3} */
 
 /* Number of external interrupt lines to configure the GIC with */
 #define NUM_IRQS 128
@@ -58,7 +59,7 @@
 #define GIC_FDT_IRQ_FLAGS_LEVEL_LO 8
 
 #define GIC_FDT_IRQ_PPI_CPU_START 8
-#define GIC_FDT_IRQ_PPI_CPU_WIDTH 8
+#define GIC_FDT_IRQ_PPI_CPU_WIDTH 24
 
 typedef struct VirtBoardInfo {
     struct arm_boot_info bootinfo;
@@ -69,6 +70,7 @@ typedef struct VirtBoardInfo {
     void *fdt;
     int fdt_size;
     uint32_t clock_phandle;
+    const char *class_name;
 } VirtBoardInfo;
 
 typedef struct {
@@ -82,6 +84,7 @@ typedef struct {
 } VirtMachineState;
 
 #define TYPE_VIRT_MACHINE   "virt"
+#define TYPE_VIRTV3_MACHINE   "virt-v3"
 #define VIRT_MACHINE(obj) \
     OBJECT_CHECK(VirtMachineState, (obj), TYPE_VIRT_MACHINE)
 #define VIRT_MACHINE_GET_CLASS(obj) \
@@ -103,20 +106,24 @@ typedef struct {
  */
 static const MemMapEntry a15memmap[] = {
     /* Space up to 0x8000000 is reserved for a boot ROM */
-    [VIRT_FLASH] =      {          0, 0x08000000 },
-    [VIRT_CPUPERIPHS] = { 0x08000000, 0x00020000 },
+    [VIRT_FLASH] =           {          0, 0x08000000 },
+    [VIRT_CPUPERIPHS] =      { 0x08000000, 0x00020000 },
     /* GIC distributor and CPU interfaces sit inside the CPU peripheral space */
-    [VIRT_GIC_DIST] =   { 0x08000000, 0x00010000 },
-    [VIRT_GIC_CPU] =    { 0x08010000, 0x00010000 },
-    [VIRT_UART] =       { 0x09000000, 0x00001000 },
-    [VIRT_RTC] =        { 0x09010000, 0x00001000 },
-    [VIRT_FW_CFG] =     { 0x09020000, 0x0000000a },
-    [VIRT_MMIO] =       { 0x0a000000, 0x00000200 },
+    [VIRT_GIC_DIST] =        { 0x08000000, 0x00010000 },
+    [VIRT_GIC_CPU] =         { 0x08010000, 0x00010000 },
+    /* On v3 VIRT_GIC_DIST_SPI takes place of VIRT_GIC_CPU */
+    [VIRT_ITS_CONTROL] =     { 0x08020000, 0x00010000 },
+    [VIRT_ITS_TRANSLATION] = { 0x08030000, 0x00010000 },
+    [VIRT_LPI] =             { 0x08040000, 0x00800000 },
+    [VIRT_UART] =            { 0x09000000, 0x00001000 },
+    [VIRT_RTC] =             { 0x09010000, 0x00001000 },
+    [VIRT_FW_CFG] =          { 0x09020000, 0x0000000a },
+    [VIRT_MMIO] =            { 0x0a000000, 0x00000200 },
     /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */
-    [VIRT_PCIE_MMIO] =  { 0x10000000, 0x2eff0000 },
-    [VIRT_PCIE_PIO] =   { 0x3eff0000, 0x00010000 },
-    [VIRT_PCIE_ECAM] =  { 0x3f000000, 0x01000000 },
-    [VIRT_MEM] =        { 0x40000000, 30ULL * 1024 * 1024 * 1024 },
+    [VIRT_PCIE_MMIO] =       { 0x10000000, 0x2eff0000 },
+    [VIRT_PCIE_PIO] =        { 0x3eff0000, 0x00010000 },
+    [VIRT_PCIE_ECAM] =       { 0x3f000000, 0x01000000 },
+    [VIRT_MEM] =             { 0x40000000, 30ULL * 1024 * 1024 * 1024 },
 };
 
 static const int a15irqmap[] = {
@@ -131,16 +138,25 @@ 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,
     },
 };
 
@@ -209,7 +225,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;
@@ -222,7 +237,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;
@@ -249,10 +263,13 @@ static void fdt_add_timer_nodes(const VirtBoardInfo *vbi)
      * they are edge-triggered.
      */
     ARMCPU *armcpu;
+    uint32_t max;
     uint32_t irqflags = GIC_FDT_IRQ_FLAGS_EDGE_LO_HI;
 
+    /* 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,
-                         GIC_FDT_IRQ_PPI_CPU_WIDTH, (1 << vbi->smp_cpus) - 1);
+                         GIC_FDT_IRQ_PPI_CPU_WIDTH, (1 << max) - 1);
 
     qemu_fdt_add_subnode(vbi->fdt, "/timer");
 
@@ -276,6 +293,18 @@ static void fdt_add_cpu_nodes(const VirtBoardInfo *vbi)
 {
     int cpu;
 
+    /*
+     * From Documentation/devicetree/bindings/arm/cpus.txt
+     *  On ARM v8 64-bit systems value should be set to 2,
+     *  that corresponds to the MPIDR_EL1 register size.
+     *  If MPIDR_EL1[63:32] value is equal to 0 on all CPUs
+     *  in the system, #address-cells can be set to 1, since
+     *  MPIDR_EL1[63:32] bits are not used for CPUs
+     *  identification.
+     *
+     *  Now GIC500 doesn't support affinities 2 & 3 so currently
+     *  #address-cells can stay 1 until future GIC
+     */
     qemu_fdt_add_subnode(vbi->fdt, "/cpus");
     qemu_fdt_setprop_cell(vbi->fdt, "/cpus", "#address-cells", 0x1);
     qemu_fdt_setprop_cell(vbi->fdt, "/cpus", "#size-cells", 0x0);
@@ -303,7 +332,7 @@ static void fdt_add_cpu_nodes(const VirtBoardInfo *vbi)
     }
 }
 
-static uint32_t fdt_add_gic_node(const VirtBoardInfo *vbi)
+static uint32_t fdt_add_gic_node(const VirtBoardInfo *vbi, int type)
 {
     uint32_t gic_phandle;
 
@@ -311,35 +340,65 @@ static uint32_t fdt_add_gic_node(const VirtBoardInfo *vbi)
     qemu_fdt_setprop_cell(vbi->fdt, "/", "interrupt-parent", gic_phandle);
 
     qemu_fdt_add_subnode(vbi->fdt, "/intc");
-    /* 'cortex-a15-gic' means 'GIC v2' */
-    qemu_fdt_setprop_string(vbi->fdt, "/intc", "compatible",
-                            "arm,cortex-a15-gic");
     qemu_fdt_setprop_cell(vbi->fdt, "/intc", "#interrupt-cells", 3);
     qemu_fdt_setprop(vbi->fdt, "/intc", "interrupt-controller", NULL, 0);
-    qemu_fdt_setprop_sized_cells(vbi->fdt, "/intc", "reg",
+    if (type == KVM_DEV_TYPE_ARM_VGIC_V3) {
+        qemu_fdt_setprop_string(vbi->fdt, "/intc", "compatible",
+                                "arm,gic-v3");
+        qemu_fdt_setprop_sized_cells(vbi->fdt, "/intc", "reg",
+                                     2, vbi->memmap[VIRT_GIC_DIST].base,
+                                     2, vbi->memmap[VIRT_GIC_DIST].size,
+#if 0                                /* Currently no need for SPI & ITS */
+                                     2, vbi->memmap[VIRT_GIC_DIST_SPI].base,
+                                     2, vbi->memmap[VIRT_GIC_DIST_SPI].size,
+                                     2, vbi->memmap[VIRT_ITS_CONTROL].base,
+                                     2, vbi->memmap[VIRT_ITS_CONTROL].size,
+                                     2, vbi->memmap[VIRT_ITS_TRANSLATION].base,
+                                     2, vbi->memmap[VIRT_ITS_TRANSLATION].size,
+#endif
+                                     2, vbi->memmap[VIRT_LPI].base,
+                                     2, vbi->memmap[VIRT_LPI].size);
+    } else {
+        /* 'cortex-a15-gic' means 'GIC v2' */
+        qemu_fdt_setprop_string(vbi->fdt, "/intc", "compatible",
+                                "arm,cortex-a15-gic");
+        qemu_fdt_setprop_sized_cells(vbi->fdt, "/intc", "reg",
                                      2, vbi->memmap[VIRT_GIC_DIST].base,
                                      2, vbi->memmap[VIRT_GIC_DIST].size,
                                      2, vbi->memmap[VIRT_GIC_CPU].base,
                                      2, vbi->memmap[VIRT_GIC_CPU].size);
+    }
     qemu_fdt_setprop_cell(vbi->fdt, "/intc", "phandle", gic_phandle);
 
     return gic_phandle;
 }
 
-static uint32_t create_gic(const VirtBoardInfo *vbi, qemu_irq *pic)
+static uint32_t create_gic(const VirtBoardInfo *vbi, qemu_irq *pic, int type)
 {
     /* We create a standalone GIC v2 */
     DeviceState *gicdev;
     SysBusDevice *gicbusdev;
-    const char *gictype = "arm_gic";
+    const char *gictype;
     int i;
 
-    if (kvm_irqchip_in_kernel()) {
+    if (type == KVM_DEV_TYPE_ARM_VGIC_V3) {
+        gictype = "arm_gicv3";
+    } else if (kvm_irqchip_in_kernel()) {
         gictype = "kvm-arm-gic";
+    } else {
+        gictype = "arm_gic";
     }
 
     gicdev = qdev_create(NULL, gictype);
-    qdev_prop_set_uint32(gicdev, "revision", 2);
+
+    for (i = 0; i < vbi->smp_cpus; i++) {
+        CPUState *cpu = qemu_get_cpu(i);
+        CPUARMState *env = cpu->env_ptr;
+        env->nvic = gicdev;
+    }
+
+    qdev_prop_set_uint32(gicdev, "revision",
+                         type == KVM_DEV_TYPE_ARM_VGIC_V3 ? 3 : 2);
     qdev_prop_set_uint32(gicdev, "num-cpu", smp_cpus);
     /* Note that the num-irq property counts both internal and external
      * interrupts; there are always 32 of the former (mandated by GIC spec).
@@ -349,6 +408,11 @@ static uint32_t create_gic(const VirtBoardInfo *vbi, qemu_irq *pic)
     gicbusdev = SYS_BUS_DEVICE(gicdev);
     sysbus_mmio_map(gicbusdev, 0, vbi->memmap[VIRT_GIC_DIST].base);
     sysbus_mmio_map(gicbusdev, 1, vbi->memmap[VIRT_GIC_CPU].base);
+    if (type == KVM_DEV_TYPE_ARM_VGIC_V3) {
+        sysbus_mmio_map(gicbusdev, 2, vbi->memmap[VIRT_ITS_CONTROL].base);
+        sysbus_mmio_map(gicbusdev, 3, vbi->memmap[VIRT_ITS_TRANSLATION].base);
+        sysbus_mmio_map(gicbusdev, 4, vbi->memmap[VIRT_LPI].base);
+    }
 
     /* Wire the outputs from each CPU's generic timer to the
      * appropriate GIC PPI inputs, and the GIC's IRQ output to
@@ -375,7 +439,7 @@ static uint32_t create_gic(const VirtBoardInfo *vbi, qemu_irq *pic)
         pic[i] = qdev_get_gpio_in(gicdev, i);
     }
 
-    return fdt_add_gic_node(vbi);
+    return fdt_add_gic_node(vbi, type);
 }
 
 static void create_uart(const VirtBoardInfo *vbi, qemu_irq *pic)
@@ -748,7 +812,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;
@@ -798,7 +862,7 @@ static void machvirt_init(MachineState *machine)
 
     create_flash(vbi);
 
-    gic_phandle = create_gic(vbi, pic);
+    gic_phandle = create_gic(vbi, pic, machine->kernel_irqchip_type);
 
     create_uart(vbi, pic);
 
@@ -848,7 +912,7 @@ static void virt_set_secure(Object *obj, bool value, Error **errp)
     vms->secure = value;
 }
 
-static void virt_instance_init(Object *obj)
+static void virt_instance_init_common(Object *obj)
 {
     VirtMachineState *vms = VIRT_MACHINE(obj);
 
@@ -862,6 +926,14 @@ static void virt_instance_init(Object *obj)
                                     NULL);
 }
 
+static void virt_instance_init(Object *obj)
+{
+    MachineState *ms = MACHINE(obj);
+
+    ms->kernel_irqchip_type = KVM_DEV_TYPE_ARM_VGIC_V2;
+    virt_instance_init_common(obj);
+}
+
 static void virt_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
@@ -881,9 +953,38 @@ static const TypeInfo machvirt_info = {
     .class_init = virt_class_init,
 };
 
+static void virtv3_instance_init(Object *obj)
+{
+    MachineState *ms = MACHINE(obj);
+
+    ms->kernel_irqchip_type = KVM_DEV_TYPE_ARM_VGIC_V3;
+    virt_instance_init_common(obj);
+}
+
+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;
+    /* With gic3 full implementation (with bitops) rase the lmit to 128 */
+    mc->max_cpus = 64;
+}
+
+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/virt.h b/include/hw/arm/virt.h
index ceec8b3..6554630 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -45,6 +45,10 @@ enum {
     VIRT_CPUPERIPHS,
     VIRT_GIC_DIST,
     VIRT_GIC_CPU,
+    VIRT_GIC_DIST_SPI = VIRT_GIC_CPU,
+    VIRT_ITS_CONTROL,
+    VIRT_ITS_TRANSLATION,
+    VIRT_LPI,
     VIRT_UART,
     VIRT_MMIO,
     VIRT_RTC,
diff --git a/include/hw/boards.h b/include/hw/boards.h
index ff79797..0734317 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -125,6 +125,7 @@ struct MachineState {
     char *accel;
     bool kernel_irqchip_allowed;
     bool kernel_irqchip_required;
+    int kernel_irqchip_type;
     int kvm_shadow_mem;
     char *dtb;
     char *dumpdtb;
-- 
1.9.1

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

* Re: [Qemu-devel] [PATCH RFC V3 1/4] Use Aff1 with mpidr This is an improved and KVM-aware alternative to
  2015-06-04 16:40 ` [Qemu-devel] [PATCH RFC V3 1/4] Use Aff1 with mpidr This is an improved and KVM-aware alternative to Shlomo Pongratz
@ 2015-06-04 17:17   ` Peter Maydell
  2015-06-06 18:53     ` Shlomo Pongratz
                       ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Peter Maydell @ 2015-06-04 17:17 UTC (permalink / raw)
  To: Shlomo Pongratz
  Cc: Eric Auger, Shlomo Pongratz, Pavel Fedin, QEMU Developers,
	Shannon Zhao, ashoks, Igor Mammedov

On 4 June 2015 at 17:40, Shlomo Pongratz <shlomopongratz@gmail.com> wrote:
> From: Pavel Fedin <p.fedin@samsung.com>

Hi. I think this patch largely makes sense, but I have some comments
below. If you want to fix these and resend as a standalone patch
I'm happy to apply that.

> 1. MPIDR value (minus feature bits) is now directly stored in CPU instance.
> 2. Default assignment is based on original rule (8 CPUs per cluster).
> 3. If KVM is used, MPIDR values are overridden with those from KVM. This is
> necessary for
> proper KVM PSCI functioning.
> 4. Some cosmetic changes which would make further expansion of this code easier.
> 5. Added some debugging macros because CPU ID assignment is tricky part, and if
> there are
> some problems, i think there should be a quick way to make sure they are
> correct.
>  This does not have an RFC mark because it is perfectly okay to be committed
> alone, it
> does not break anything. Commit message follows. Cc'ed to all interested
> parties because i
> really hope to get things going somewhere this time.

This sort of commentary belongs below the '---' line, so it doesn't
end up in the final git commit message.

> In order to support up to 128 cores with GIC-500 (GICv3 implementation)
> affinity1 must be used. GIC-500 support up to 32 clusters with up to
> 8 cores in a cluster. So for example, if one wishes to have 16 cores,
> the options are: 2 clusters of 8 cores each, 4 clusters with 4 cores each
> Currently only the first option is supported for TCG. However, KVM expects
> to have the same clusters layout as host machine has. Therefore, if we use
> 64-bit KVM we override CPU IDs with those provided by the KVM. For 32-bit
> systems it is OK to leave the default because so far we do not have more
> than 8 CPUs on any of them.
> This implementation has a potential to be extended with some methods which
> would define cluster layout instead of hardcoded CPUS_PER_CLUSTER
> definition. This would allow to emulate real machines with different
> layouts. However, in case of KVM we would still have to inherit IDs from
> the host.

I agree that we want to be using the same idea of MPIDR as KVM
when KVM is enabled, certainly. But this commit message has
a number of inaccuracies:
 * KVM doesn't inherit IDs from the host
 * we don't need to have the same cluster layout as the host machine
 * this isn't specific to 64-bit VMs so we should fix 32 bits at the same time

The bug we're actually trying to fix here is just that the
kernel's mapping from VCPU ID to MPIDR value has changed
from the simplistic implementation it originally had. So in
userspace we need to read the MPIDR value back from the kernel
rather than assuming we know the mapping. (It happens that the
reason the kernel changed was to accommodate GICv3 and its
restrictions on affinity values, but that's not relevant to why
this patch is needed.)

> Signed-off-by: Shlomo Pongratz <shlomo.pongratz@huawei.com>
> Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
> ---
>  hw/arm/virt.c        |  6 +++++-
>  target-arm/cpu-qom.h |  3 +++
>  target-arm/cpu.c     | 17 +++++++++++++++++
>  target-arm/helper.c  |  9 +++------
>  target-arm/kvm64.c   | 25 +++++++++++++++++++++++++
>  target-arm/psci.c    | 20 ++++++++++++++++++--
>  6 files changed, 71 insertions(+), 9 deletions(-)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 05db8cb..19c8c3a 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -294,7 +294,11 @@ static void fdt_add_cpu_nodes(const VirtBoardInfo *vbi)
>                                          "enable-method", "psci");
>          }
>
> -        qemu_fdt_setprop_cell(vbi->fdt, nodename, "reg", cpu);
> +        /*
> +         * If cpus node's #address-cells property is set to 1
> +         * The reg cell bits [23:0] must be set to bits [23:0] of MPIDR_EL1.
> +         */

This comment is either obvious or misleading depending on your
point of view.

> +        qemu_fdt_setprop_cell(vbi->fdt, nodename, "reg", armcpu->mpidr);
>          g_free(nodename);
>      }
>  }
> diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
> index ed5a644..a382a09 100644
> --- a/target-arm/cpu-qom.h
> +++ b/target-arm/cpu-qom.h
> @@ -159,6 +159,7 @@ typedef struct ARMCPU {
>      uint64_t id_aa64mmfr1;
>      uint32_t dbgdidr;
>      uint32_t clidr;
> +    uint64_t mpidr; /* Without feature bits */

I think it would be less confusing to store the entire MPIDR
here, and then mask it out if there are places that only
want the affinity bits.

>      /* The elements of this array are the CCSIDR values for each cache,
>       * in the order L1DCache, L1ICache, L2DCache, L2ICache, etc.
>       */
> @@ -171,6 +172,8 @@ typedef struct ARMCPU {
>      uint64_t rvbar;
>  } ARMCPU;
>
> +#define CPUS_PER_CLUSTER 8

Changes I suggest below will mean this #define is only
used in one place so it won't need to be in a header.

> +
>  #define TYPE_AARCH64_CPU "aarch64-cpu"
>  #define AARCH64_CPU_CLASS(klass) \
>      OBJECT_CLASS_CHECK(AArch64CPUClass, (klass), TYPE_AARCH64_CPU)
> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> index 4a888ab..7272466 100644
> --- a/target-arm/cpu.c
> +++ b/target-arm/cpu.c
> @@ -31,6 +31,12 @@
>  #include "sysemu/kvm.h"
>  #include "kvm_arm.h"
>
> +#ifdef DEBUG
> +# define DPRINTF(format, ...) printf("armcpu: " format , ## __VA_ARGS__)
> +#else
> +# define DPRINTF(format, ...) do { } while (0)
> +#endif

Drop these debug macros, please.

> +
>  static void arm_cpu_set_pc(CPUState *cs, vaddr value)
>  {
>      ARMCPU *cpu = ARM_CPU(cs);
> @@ -388,12 +394,23 @@ static void arm_cpu_initfn(Object *obj)
>      CPUState *cs = CPU(obj);
>      ARMCPU *cpu = ARM_CPU(obj);
>      static bool inited;
> +    uint32_t Aff1, Aff0;
>
>      cs->env_ptr = &cpu->env;
>      cpu_exec_init(&cpu->env);
>      cpu->cp_regs = g_hash_table_new_full(g_int_hash, g_int_equal,
>                                           g_free, g_free);
>
> +    /*
> +     * We don't support setting cluster ID ([16..23]) (known as Aff2
> +     * in later ARM ARM versions), or any of the higher affinity level fields,
> +     * so these bits always RAZ.
> +     */
> +    Aff1 = cs->cpu_index / CPUS_PER_CLUSTER;
> +    Aff0 = cs->cpu_index % CPUS_PER_CLUSTER;
> +    cpu->mpidr = (Aff1 << 8) | Aff0;
> +    DPRINTF("Init(%p): index %d mpidr 0x%jX\n", cpu, cs->cpu_index, cpu->mpidr);
> +
>  #ifndef CONFIG_USER_ONLY
>      /* Our inbound IRQ and FIQ lines */
>      if (kvm_enabled()) {
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 1cc4993..99c7a30 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -2063,12 +2063,9 @@ static const ARMCPRegInfo strongarm_cp_reginfo[] = {
>
>  static uint64_t mpidr_read(CPUARMState *env, const ARMCPRegInfo *ri)
>  {
> -    CPUState *cs = CPU(arm_env_get_cpu(env));
> -    uint32_t mpidr = cs->cpu_index;
> -    /* We don't support setting cluster ID ([8..11]) (known as Aff1
> -     * in later ARM ARM versions), or any of the higher affinity level fields,
> -     * so these bits always RAZ.
> -     */
> +    ARMCPU *cpu = ARM_CPU(arm_env_get_cpu(env));
> +    uint64_t mpidr = cpu->mpidr;
> +
>      if (arm_feature(env, ARM_FEATURE_V7MP)) {
>          mpidr |= (1U << 31);
>          /* Cores which are uniprocessor (non-coherent)
> diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c
> index 93c1ca8..99fa34e 100644
> --- a/target-arm/kvm64.c
> +++ b/target-arm/kvm64.c
> @@ -25,6 +25,12 @@
>  #include "internals.h"
>  #include "hw/arm/arm.h"
>
> +#ifdef DEBUG
> +# define DPRINTF(format, ...) printf("arm-kvm64: " format , ## __VA_ARGS__)
> +#else
> +# define DPRINTF(format, ...) do { } while (0)
> +#endif
> +
>  static inline void set_feature(uint64_t *features, int feature)
>  {
>      *features |= 1ULL << feature;
> @@ -77,6 +83,10 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUClass *ahcc)
>      return true;
>  }
>
> +#define ARM_MPIDR_HWID_BITMASK 0xFF00FFFFFFULL
> +#define ARM_CPU_ID             3, 0, 0, 0
> +#define ARM_CPU_ID_MPIDR       5

I don't see the point in separating the op2 value from the rest.

> +
>  int kvm_arch_init_vcpu(CPUState *cs)
>  {
>      int ret;
> @@ -107,6 +117,21 @@ int kvm_arch_init_vcpu(CPUState *cs)
>          return ret;
>      }
>
> +    /*
> +     * When KVM is in use, psci is emulated in-kernel and not by qemu.

PSCI and QEMU should both be all-caps.

> +     * In order for it to work correctly we should use correct MPIDR values,
> +     * which appear to be inherited from the host.

I don't think they're inherited from the host: it's just that
different host kernels use different mappings from vcpu ID
to MPIDR (see reset_mpidr() in arch/arm64/kvm/sys_regs.c; its
implementation has changed over time).
In any case we don't need to speculate; you can just say:

 In order for it to work correctly we must use MPIDR values in
 the device tree which match the MPIDR values the kernel has picked
 for the vcpus, so ask KVM what those values are.

> +     * So we override our defaults by asking KVM about these values.
> +     */
> +    ret = kvm_get_one_reg(cs, ARM64_SYS_REG(ARM_CPU_ID, ARM_CPU_ID_MPIDR),
> +                          &cpu->mpidr);
> +    if (ret) {
> +        return ret;
> +    }
> +    cpu->mpidr &= ARM_MPIDR_HWID_BITMASK;
> +    DPRINTF("Init(%p): index %d MPIDR 0x%jX\n", cpu,
> +            cs->cpu_index, cpu->mpidr);
> +
>      return kvm_arm_init_cpreg_list(cpu);
>  }
>

There should also be an equivalent patch to kvm32.c, because
PSCI on 32-bit systems has the same issue.

> diff --git a/target-arm/psci.c b/target-arm/psci.c
> index d8fafab..d03896f 100644
> --- a/target-arm/psci.c
> +++ b/target-arm/psci.c
> @@ -72,6 +72,22 @@ bool arm_is_psci_call(ARMCPU *cpu, int excp_type)
>      }
>  }
>
> +static uint32_t mpidr_to_idx(uint64_t mpidr)
> +{
> +    uint32_t Aff1, Aff0;
> +
> +    /*
> +     * MPIDR_EL1 [RES0:affinity-3:RES1:U:RES0:MT:affinity-1:affinity-0]
> +     * GIC 500 code currently supports 32 clusters with 8 cores each
> +     * but no more than 128 cores. Future version will have flexible
> +     * affinity selection
> +     * GIC 400 supports 8 cores so 0x7 for Aff0 is O.K. too
> +     */
> +     Aff1 = (mpidr & 0xff00) >> 8;
> +     Aff0 = mpidr & 0xff;
> +     return Aff1 * CPUS_PER_CLUSTER + Aff0;
> +}

> +
>  void arm_handle_psci_call(ARMCPU *cpu)
>  {
>      /*
> @@ -121,7 +137,7 @@ void arm_handle_psci_call(ARMCPU *cpu)
>
>          switch (param[2]) {
>          case 0:
> -            target_cpu_state = qemu_get_cpu(mpidr & 0xff);
> +            target_cpu_state = qemu_get_cpu(mpidr_to_idx(mpidr));

Rather than doing this, you can just have an
arm_get_cpu_by_mpidr() which does
    CPU_FOREACH(cs) {
        ARMCPU *cpu = ARM_CPU(cs);

        if (cpu->mpidr == mpidr) {
            return cpu;
        }
    }
    return NULL;

ie same as qemu_get_cpu() but checking on mpidr rather than
cpu_index. Then we don't need to make assumptions about the
mpidr-to-index mapping here at all (and this code will
continue to work if in future we decide to expose the
cluster topology via CPU properties for the board to set.)

thanks
-- PMM

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

* Re: [Qemu-devel] [PATCH RFC V3 1/4] Use Aff1 with mpidr This is an improved and KVM-aware alternative to
  2015-06-04 17:17   ` Peter Maydell
@ 2015-06-06 18:53     ` Shlomo Pongratz
  2015-06-08  6:41       ` Pavel Fedin
  2015-06-08  7:50     ` Pavel Fedin
  2015-06-08 10:32     ` Igor Mammedov
  2 siblings, 1 reply; 14+ messages in thread
From: Shlomo Pongratz @ 2015-06-06 18:53 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Eric Auger, Shlomo Pongratz, Pavel Fedin, qemu-devel,
	Shannon Zhao, ashoks, Igor Mammedov

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

Hi all.
Patch #1 is actually Pavel Fedin's patch
https://qemu-devel/2015-05/msg04495.html, which I included as a replacement
to my original patch #1 "as there could be only one". I think that Pavel's
needs to address all the issues in the original thread.
Best regards.
S.P.
On Jun 4, 2015 7:18 PM, "Peter Maydell" <peter.maydell@linaro.org> wrote:

> On 4 June 2015 at 17:40, Shlomo Pongratz <shlomopongratz@gmail.com> wrote:
> > From: Pavel Fedin <p.fedin@samsung.com>
>
> Hi. I think this patch largely makes sense, but I have some comments
> below. If you want to fix these and resend as a standalone patch
> I'm happy to apply that.
>
> > 1. MPIDR value (minus feature bits) is now directly stored in CPU
> instance.
> > 2. Default assignment is based on original rule (8 CPUs per cluster).
> > 3. If KVM is used, MPIDR values are overridden with those from KVM. This
> is
> > necessary for
> > proper KVM PSCI functioning.
> > 4. Some cosmetic changes which would make further expansion of this code
> easier.
> > 5. Added some debugging macros because CPU ID assignment is tricky part,
> and if
> > there are
> > some problems, i think there should be a quick way to make sure they are
> > correct.
> >  This does not have an RFC mark because it is perfectly okay to be
> committed
> > alone, it
> > does not break anything. Commit message follows. Cc'ed to all interested
> > parties because i
> > really hope to get things going somewhere this time.
>
> This sort of commentary belongs below the '---' line, so it doesn't
> end up in the final git commit message.
>
> > In order to support up to 128 cores with GIC-500 (GICv3 implementation)
> > affinity1 must be used. GIC-500 support up to 32 clusters with up to
> > 8 cores in a cluster. So for example, if one wishes to have 16 cores,
> > the options are: 2 clusters of 8 cores each, 4 clusters with 4 cores each
> > Currently only the first option is supported for TCG. However, KVM
> expects
> > to have the same clusters layout as host machine has. Therefore, if we
> use
> > 64-bit KVM we override CPU IDs with those provided by the KVM. For 32-bit
> > systems it is OK to leave the default because so far we do not have more
> > than 8 CPUs on any of them.
> > This implementation has a potential to be extended with some methods
> which
> > would define cluster layout instead of hardcoded CPUS_PER_CLUSTER
> > definition. This would allow to emulate real machines with different
> > layouts. However, in case of KVM we would still have to inherit IDs from
> > the host.
>
> I agree that we want to be using the same idea of MPIDR as KVM
> when KVM is enabled, certainly. But this commit message has
> a number of inaccuracies:
>  * KVM doesn't inherit IDs from the host
>  * we don't need to have the same cluster layout as the host machine
>  * this isn't specific to 64-bit VMs so we should fix 32 bits at the same
> time
>
> The bug we're actually trying to fix here is just that the
> kernel's mapping from VCPU ID to MPIDR value has changed
> from the simplistic implementation it originally had. So in
> userspace we need to read the MPIDR value back from the kernel
> rather than assuming we know the mapping. (It happens that the
> reason the kernel changed was to accommodate GICv3 and its
> restrictions on affinity values, but that's not relevant to why
> this patch is needed.)
>
> > Signed-off-by: Shlomo Pongratz <shlomo.pongratz@huawei.com>
> > Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
> > ---
> >  hw/arm/virt.c        |  6 +++++-
> >  target-arm/cpu-qom.h |  3 +++
> >  target-arm/cpu.c     | 17 +++++++++++++++++
> >  target-arm/helper.c  |  9 +++------
> >  target-arm/kvm64.c   | 25 +++++++++++++++++++++++++
> >  target-arm/psci.c    | 20 ++++++++++++++++++--
> >  6 files changed, 71 insertions(+), 9 deletions(-)
> >
> > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> > index 05db8cb..19c8c3a 100644
> > --- a/hw/arm/virt.c
> > +++ b/hw/arm/virt.c
> > @@ -294,7 +294,11 @@ static void fdt_add_cpu_nodes(const VirtBoardInfo
> *vbi)
> >                                          "enable-method", "psci");
> >          }
> >
> > -        qemu_fdt_setprop_cell(vbi->fdt, nodename, "reg", cpu);
> > +        /*
> > +         * If cpus node's #address-cells property is set to 1
> > +         * The reg cell bits [23:0] must be set to bits [23:0] of
> MPIDR_EL1.
> > +         */
>
> This comment is either obvious or misleading depending on your
> point of view.
>
> > +        qemu_fdt_setprop_cell(vbi->fdt, nodename, "reg", armcpu->mpidr);
> >          g_free(nodename);
> >      }
> >  }
> > diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
> > index ed5a644..a382a09 100644
> > --- a/target-arm/cpu-qom.h
> > +++ b/target-arm/cpu-qom.h
> > @@ -159,6 +159,7 @@ typedef struct ARMCPU {
> >      uint64_t id_aa64mmfr1;
> >      uint32_t dbgdidr;
> >      uint32_t clidr;
> > +    uint64_t mpidr; /* Without feature bits */
>
> I think it would be less confusing to store the entire MPIDR
> here, and then mask it out if there are places that only
> want the affinity bits.
>
> >      /* The elements of this array are the CCSIDR values for each cache,
> >       * in the order L1DCache, L1ICache, L2DCache, L2ICache, etc.
> >       */
> > @@ -171,6 +172,8 @@ typedef struct ARMCPU {
> >      uint64_t rvbar;
> >  } ARMCPU;
> >
> > +#define CPUS_PER_CLUSTER 8
>
> Changes I suggest below will mean this #define is only
> used in one place so it won't need to be in a header.
>
> > +
> >  #define TYPE_AARCH64_CPU "aarch64-cpu"
> >  #define AARCH64_CPU_CLASS(klass) \
> >      OBJECT_CLASS_CHECK(AArch64CPUClass, (klass), TYPE_AARCH64_CPU)
> > diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> > index 4a888ab..7272466 100644
> > --- a/target-arm/cpu.c
> > +++ b/target-arm/cpu.c
> > @@ -31,6 +31,12 @@
> >  #include "sysemu/kvm.h"
> >  #include "kvm_arm.h"
> >
> > +#ifdef DEBUG
> > +# define DPRINTF(format, ...) printf("armcpu: " format , ## __VA_ARGS__)
> > +#else
> > +# define DPRINTF(format, ...) do { } while (0)
> > +#endif
>
> Drop these debug macros, please.
>
> > +
> >  static void arm_cpu_set_pc(CPUState *cs, vaddr value)
> >  {
> >      ARMCPU *cpu = ARM_CPU(cs);
> > @@ -388,12 +394,23 @@ static void arm_cpu_initfn(Object *obj)
> >      CPUState *cs = CPU(obj);
> >      ARMCPU *cpu = ARM_CPU(obj);
> >      static bool inited;
> > +    uint32_t Aff1, Aff0;
> >
> >      cs->env_ptr = &cpu->env;
> >      cpu_exec_init(&cpu->env);
> >      cpu->cp_regs = g_hash_table_new_full(g_int_hash, g_int_equal,
> >                                           g_free, g_free);
> >
> > +    /*
> > +     * We don't support setting cluster ID ([16..23]) (known as Aff2
> > +     * in later ARM ARM versions), or any of the higher affinity level
> fields,
> > +     * so these bits always RAZ.
> > +     */
> > +    Aff1 = cs->cpu_index / CPUS_PER_CLUSTER;
> > +    Aff0 = cs->cpu_index % CPUS_PER_CLUSTER;
> > +    cpu->mpidr = (Aff1 << 8) | Aff0;
> > +    DPRINTF("Init(%p): index %d mpidr 0x%jX\n", cpu, cs->cpu_index,
> cpu->mpidr);
> > +
> >  #ifndef CONFIG_USER_ONLY
> >      /* Our inbound IRQ and FIQ lines */
> >      if (kvm_enabled()) {
> > diff --git a/target-arm/helper.c b/target-arm/helper.c
> > index 1cc4993..99c7a30 100644
> > --- a/target-arm/helper.c
> > +++ b/target-arm/helper.c
> > @@ -2063,12 +2063,9 @@ static const ARMCPRegInfo strongarm_cp_reginfo[]
> = {
> >
> >  static uint64_t mpidr_read(CPUARMState *env, const ARMCPRegInfo *ri)
> >  {
> > -    CPUState *cs = CPU(arm_env_get_cpu(env));
> > -    uint32_t mpidr = cs->cpu_index;
> > -    /* We don't support setting cluster ID ([8..11]) (known as Aff1
> > -     * in later ARM ARM versions), or any of the higher affinity level
> fields,
> > -     * so these bits always RAZ.
> > -     */
> > +    ARMCPU *cpu = ARM_CPU(arm_env_get_cpu(env));
> > +    uint64_t mpidr = cpu->mpidr;
> > +
> >      if (arm_feature(env, ARM_FEATURE_V7MP)) {
> >          mpidr |= (1U << 31);
> >          /* Cores which are uniprocessor (non-coherent)
> > diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c
> > index 93c1ca8..99fa34e 100644
> > --- a/target-arm/kvm64.c
> > +++ b/target-arm/kvm64.c
> > @@ -25,6 +25,12 @@
> >  #include "internals.h"
> >  #include "hw/arm/arm.h"
> >
> > +#ifdef DEBUG
> > +# define DPRINTF(format, ...) printf("arm-kvm64: " format , ##
> __VA_ARGS__)
> > +#else
> > +# define DPRINTF(format, ...) do { } while (0)
> > +#endif
> > +
> >  static inline void set_feature(uint64_t *features, int feature)
> >  {
> >      *features |= 1ULL << feature;
> > @@ -77,6 +83,10 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUClass
> *ahcc)
> >      return true;
> >  }
> >
> > +#define ARM_MPIDR_HWID_BITMASK 0xFF00FFFFFFULL
> > +#define ARM_CPU_ID             3, 0, 0, 0
> > +#define ARM_CPU_ID_MPIDR       5
>
> I don't see the point in separating the op2 value from the rest.
>
> > +
> >  int kvm_arch_init_vcpu(CPUState *cs)
> >  {
> >      int ret;
> > @@ -107,6 +117,21 @@ int kvm_arch_init_vcpu(CPUState *cs)
> >          return ret;
> >      }
> >
> > +    /*
> > +     * When KVM is in use, psci is emulated in-kernel and not by qemu.
>
> PSCI and QEMU should both be all-caps.
>
> > +     * In order for it to work correctly we should use correct MPIDR
> values,
> > +     * which appear to be inherited from the host.
>
> I don't think they're inherited from the host: it's just that
> different host kernels use different mappings from vcpu ID
> to MPIDR (see reset_mpidr() in arch/arm64/kvm/sys_regs.c; its
> implementation has changed over time).
> In any case we don't need to speculate; you can just say:
>
>  In order for it to work correctly we must use MPIDR values in
>  the device tree which match the MPIDR values the kernel has picked
>  for the vcpus, so ask KVM what those values are.
>
> > +     * So we override our defaults by asking KVM about these values.
> > +     */
> > +    ret = kvm_get_one_reg(cs, ARM64_SYS_REG(ARM_CPU_ID,
> ARM_CPU_ID_MPIDR),
> > +                          &cpu->mpidr);
> > +    if (ret) {
> > +        return ret;
> > +    }
> > +    cpu->mpidr &= ARM_MPIDR_HWID_BITMASK;
> > +    DPRINTF("Init(%p): index %d MPIDR 0x%jX\n", cpu,
> > +            cs->cpu_index, cpu->mpidr);
> > +
> >      return kvm_arm_init_cpreg_list(cpu);
> >  }
> >
>
> There should also be an equivalent patch to kvm32.c, because
> PSCI on 32-bit systems has the same issue.
>
> > diff --git a/target-arm/psci.c b/target-arm/psci.c
> > index d8fafab..d03896f 100644
> > --- a/target-arm/psci.c
> > +++ b/target-arm/psci.c
> > @@ -72,6 +72,22 @@ bool arm_is_psci_call(ARMCPU *cpu, int excp_type)
> >      }
> >  }
> >
> > +static uint32_t mpidr_to_idx(uint64_t mpidr)
> > +{
> > +    uint32_t Aff1, Aff0;
> > +
> > +    /*
> > +     * MPIDR_EL1 [RES0:affinity-3:RES1:U:RES0:MT:affinity-1:affinity-0]
> > +     * GIC 500 code currently supports 32 clusters with 8 cores each
> > +     * but no more than 128 cores. Future version will have flexible
> > +     * affinity selection
> > +     * GIC 400 supports 8 cores so 0x7 for Aff0 is O.K. too
> > +     */
> > +     Aff1 = (mpidr & 0xff00) >> 8;
> > +     Aff0 = mpidr & 0xff;
> > +     return Aff1 * CPUS_PER_CLUSTER + Aff0;
> > +}
>
> > +
> >  void arm_handle_psci_call(ARMCPU *cpu)
> >  {
> >      /*
> > @@ -121,7 +137,7 @@ void arm_handle_psci_call(ARMCPU *cpu)
> >
> >          switch (param[2]) {
> >          case 0:
> > -            target_cpu_state = qemu_get_cpu(mpidr & 0xff);
> > +            target_cpu_state = qemu_get_cpu(mpidr_to_idx(mpidr));
>
> Rather than doing this, you can just have an
> arm_get_cpu_by_mpidr() which does
>     CPU_FOREACH(cs) {
>         ARMCPU *cpu = ARM_CPU(cs);
>
>         if (cpu->mpidr == mpidr) {
>             return cpu;
>         }
>     }
>     return NULL;
>
> ie same as qemu_get_cpu() but checking on mpidr rather than
> cpu_index. Then we don't need to make assumptions about the
> mpidr-to-index mapping here at all (and this code will
> continue to work if in future we decide to expose the
> cluster topology via CPU properties for the board to set.)
>
> thanks
> -- PMM
>

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

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

* Re: [Qemu-devel] [PATCH RFC V3 1/4] Use Aff1 with mpidr This is an improved and KVM-aware alternative to
  2015-06-06 18:53     ` Shlomo Pongratz
@ 2015-06-08  6:41       ` Pavel Fedin
  0 siblings, 0 replies; 14+ messages in thread
From: Pavel Fedin @ 2015-06-08  6:41 UTC (permalink / raw)
  To: 'Shlomo Pongratz', 'Peter Maydell'
  Cc: 'Eric Auger', 'Shlomo Pongratz',
	qemu-devel, 'Shannon Zhao',
	ashoks, 'Igor Mammedov'

 Hi!

> I think that Pavel's needs to address all the issues in the original thread.

 Ok. I have read the thread and i'll prepare a fixed version of this shortly.

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

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

* Re: [Qemu-devel] [PATCH RFC V3 1/4] Use Aff1 with mpidr This is an improved and KVM-aware alternative to
  2015-06-04 17:17   ` Peter Maydell
  2015-06-06 18:53     ` Shlomo Pongratz
@ 2015-06-08  7:50     ` Pavel Fedin
  2015-06-08  8:01       ` Peter Maydell
  2015-06-08 10:32     ` Igor Mammedov
  2 siblings, 1 reply; 14+ messages in thread
From: Pavel Fedin @ 2015-06-08  7:50 UTC (permalink / raw)
  To: 'Peter Maydell', 'Shlomo Pongratz'
  Cc: 'Eric Auger', 'Shlomo Pongratz',
	'QEMU Developers', 'Shannon Zhao',
	ashoks, 'Igor Mammedov'

> I think it would be less confusing to store the entire MPIDR
> here, and then mask it out if there are places that only
> want the affinity bits.

 I thought about it when i developed this, but in my implementation affinity bits are assigned during CPU instantiation, while feature bits can be added later, during realize. I could tweak set_feature() to sync up MPIDR value but perhaps it isn't the best thing to do.
 Additionally, currently we support only ARM_FEATURE_V7MP, which is 32-bit only. I believe this would be a more complex modification which would touch more of 32-bit code.
 So would it be acceptable to leave mpidr handling as it is?

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

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

* Re: [Qemu-devel] [PATCH RFC V3 1/4] Use Aff1 with mpidr This is an improved and KVM-aware alternative to
  2015-06-08  7:50     ` Pavel Fedin
@ 2015-06-08  8:01       ` Peter Maydell
  2015-06-08  9:39         ` Pavel Fedin
  0 siblings, 1 reply; 14+ messages in thread
From: Peter Maydell @ 2015-06-08  8:01 UTC (permalink / raw)
  To: Pavel Fedin
  Cc: Eric Auger, Shlomo Pongratz, Shlomo Pongratz, QEMU Developers,
	Shannon Zhao, Ashok Kumar, Igor Mammedov

On 8 June 2015 at 08:50, Pavel Fedin <p.fedin@samsung.com> wrote:
>> I think it would be less confusing to store the entire MPIDR
>> here, and then mask it out if there are places that only
>> want the affinity bits.
>
>  I thought about it when i developed this, but in my implementation
> affinity bits are assigned during CPU instantiation, while feature
> bits can be added later, during realize. I could tweak set_feature()
> to sync up MPIDR value but perhaps it isn't the best thing to do.

Can we not just assign the whole thing at realize?

>  Additionally, currently we support only ARM_FEATURE_V7MP, which is
> 32-bit only. I believe this would be a more complex modification
> which would touch more of 32-bit code.

It's only 32-bit in the sense that all 64-bit systems (indeed, all v8
systems) have the feature and so the bit is effectively RES1.

You need to fix the 32-bit side anyway.

>  So would it be acceptable to leave mpidr handling as it is?

I still think that having the whole mpidr in the struct will
be less confusing.

thanks
-- PMM

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

* Re: [Qemu-devel] [PATCH RFC V3 1/4] Use Aff1 with mpidr This is an improved and KVM-aware alternative to
  2015-06-08  8:01       ` Peter Maydell
@ 2015-06-08  9:39         ` Pavel Fedin
  2015-06-08 10:20           ` Peter Maydell
  2015-06-08 10:39           ` Igor Mammedov
  0 siblings, 2 replies; 14+ messages in thread
From: Pavel Fedin @ 2015-06-08  9:39 UTC (permalink / raw)
  To: 'Peter Maydell'
  Cc: 'Eric Auger', 'Shlomo Pongratz',
	'Shlomo Pongratz', 'QEMU Developers',
	'Shannon Zhao', 'Ashok Kumar',
	'Igor Mammedov'

 Hello!

> >  I thought about it when i developed this, but in my implementation
> > affinity bits are assigned during CPU instantiation, while feature
> > bits can be added later, during realize. I could tweak set_feature()
> > to sync up MPIDR value but perhaps it isn't the best thing to do.
> 
> Can we not just assign the whole thing at realize?

 kvm_arch_init_vcpu() is called before realize, therefore i would have to track down whether KVM is active or not.

> You need to fix the 32-bit side anyway.

 Actually already done in my working tree.

> I still think that having the whole mpidr in the struct will
> be less confusing.

 Ok, if you are really insisting on that, i can assign IDs where this is currently done and add feature bits in arm_cpu_realizefn().
 Last argument: the rest of qemu code (property assignment, lookup in PSCI, etc) actually needs IDs without feature bits. The only function which needs full version is mpidr_read(). So does it still worth of putting AND with bitmasks everywhere? May be just rename 'mpidr' to something like 'mp_id' ?

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

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

* Re: [Qemu-devel] [PATCH RFC V3 1/4] Use Aff1 with mpidr This is an improved and KVM-aware alternative to
  2015-06-08  9:39         ` Pavel Fedin
@ 2015-06-08 10:20           ` Peter Maydell
  2015-06-08 10:39           ` Igor Mammedov
  1 sibling, 0 replies; 14+ messages in thread
From: Peter Maydell @ 2015-06-08 10:20 UTC (permalink / raw)
  To: Pavel Fedin
  Cc: Eric Auger, Shlomo Pongratz, Shlomo Pongratz, QEMU Developers,
	Shannon Zhao, Ashok Kumar, Igor Mammedov

On 8 June 2015 at 10:39, Pavel Fedin <p.fedin@samsung.com> wrote:
>  Last argument: the rest of qemu code (property assignment, lookup
> in PSCI, etc) actually needs IDs without feature bits. The only
> function which needs full version is mpidr_read(). So does it still
> worth of putting AND with bitmasks everywhere? May be just rename
> 'mpidr' to something like 'mp_id' ?

Mmm, OK, if we don't call it mpidr I think this will be OK.
I suggest "mp_affinity".

-- PMM

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

* Re: [Qemu-devel] [PATCH RFC V3 1/4] Use Aff1 with mpidr This is an improved and KVM-aware alternative to
  2015-06-04 17:17   ` Peter Maydell
  2015-06-06 18:53     ` Shlomo Pongratz
  2015-06-08  7:50     ` Pavel Fedin
@ 2015-06-08 10:32     ` Igor Mammedov
  2 siblings, 0 replies; 14+ messages in thread
From: Igor Mammedov @ 2015-06-08 10:32 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Eric Auger, Shlomo Pongratz, Shlomo Pongratz, Pavel Fedin,
	QEMU Developers, Shannon Zhao, ashoks

On Thu, 4 Jun 2015 18:17:39 +0100
Peter Maydell <peter.maydell@linaro.org> wrote:

> On 4 June 2015 at 17:40, Shlomo Pongratz <shlomopongratz@gmail.com> wrote:
> > From: Pavel Fedin <p.fedin@samsung.com>
> 
> Hi. I think this patch largely makes sense, but I have some comments
> below. If you want to fix these and resend as a standalone patch
> I'm happy to apply that.
> 
> > 1. MPIDR value (minus feature bits) is now directly stored in CPU instance.
> > 2. Default assignment is based on original rule (8 CPUs per cluster).
> > 3. If KVM is used, MPIDR values are overridden with those from KVM. This is
> > necessary for
> > proper KVM PSCI functioning.
> > 4. Some cosmetic changes which would make further expansion of this code easier.
> > 5. Added some debugging macros because CPU ID assignment is tricky part, and if
> > there are
> > some problems, i think there should be a quick way to make sure they are
> > correct.
> >  This does not have an RFC mark because it is perfectly okay to be committed
> > alone, it
> > does not break anything. Commit message follows. Cc'ed to all interested
> > parties because i
> > really hope to get things going somewhere this time.
> 
> This sort of commentary belongs below the '---' line, so it doesn't
> end up in the final git commit message.
> 
> > In order to support up to 128 cores with GIC-500 (GICv3 implementation)
> > affinity1 must be used. GIC-500 support up to 32 clusters with up to
> > 8 cores in a cluster. So for example, if one wishes to have 16 cores,
> > the options are: 2 clusters of 8 cores each, 4 clusters with 4 cores each
> > Currently only the first option is supported for TCG. However, KVM expects
> > to have the same clusters layout as host machine has. Therefore, if we use
> > 64-bit KVM we override CPU IDs with those provided by the KVM. For 32-bit
> > systems it is OK to leave the default because so far we do not have more
> > than 8 CPUs on any of them.
> > This implementation has a potential to be extended with some methods which
> > would define cluster layout instead of hardcoded CPUS_PER_CLUSTER
> > definition. This would allow to emulate real machines with different
> > layouts. However, in case of KVM we would still have to inherit IDs from
> > the host.
> 
> I agree that we want to be using the same idea of MPIDR as KVM
> when KVM is enabled, certainly. But this commit message has
> a number of inaccuracies:
>  * KVM doesn't inherit IDs from the host
>  * we don't need to have the same cluster layout as the host machine
>  * this isn't specific to 64-bit VMs so we should fix 32 bits at the same time
> 
> The bug we're actually trying to fix here is just that the
> kernel's mapping from VCPU ID to MPIDR value has changed
> from the simplistic implementation it originally had. So in
> userspace we need to read the MPIDR value back from the kernel
> rather than assuming we know the mapping. (It happens that the
> reason the kernel changed was to accommodate GICv3 and its
> restrictions on affinity values, but that's not relevant to why
> this patch is needed.)
> 
> > Signed-off-by: Shlomo Pongratz <shlomo.pongratz@huawei.com>
> > Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
> > ---
> >  hw/arm/virt.c        |  6 +++++-
> >  target-arm/cpu-qom.h |  3 +++
> >  target-arm/cpu.c     | 17 +++++++++++++++++
> >  target-arm/helper.c  |  9 +++------
> >  target-arm/kvm64.c   | 25 +++++++++++++++++++++++++
> >  target-arm/psci.c    | 20 ++++++++++++++++++--
> >  6 files changed, 71 insertions(+), 9 deletions(-)
> >
> > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> > index 05db8cb..19c8c3a 100644
> > --- a/hw/arm/virt.c
> > +++ b/hw/arm/virt.c
> > @@ -294,7 +294,11 @@ static void fdt_add_cpu_nodes(const VirtBoardInfo *vbi)
> >                                          "enable-method", "psci");
> >          }
> >
> > -        qemu_fdt_setprop_cell(vbi->fdt, nodename, "reg", cpu);
> > +        /*
> > +         * If cpus node's #address-cells property is set to 1
> > +         * The reg cell bits [23:0] must be set to bits [23:0] of MPIDR_EL1.
> > +         */
> 
> This comment is either obvious or misleading depending on your
> point of view.
> 
> > +        qemu_fdt_setprop_cell(vbi->fdt, nodename, "reg", armcpu->mpidr);
> >          g_free(nodename);
> >      }
> >  }
> > diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
> > index ed5a644..a382a09 100644
> > --- a/target-arm/cpu-qom.h
> > +++ b/target-arm/cpu-qom.h
> > @@ -159,6 +159,7 @@ typedef struct ARMCPU {
> >      uint64_t id_aa64mmfr1;
> >      uint32_t dbgdidr;
> >      uint32_t clidr;
> > +    uint64_t mpidr; /* Without feature bits */
> 
> I think it would be less confusing to store the entire MPIDR
> here, and then mask it out if there are places that only
> want the affinity bits.
> 
> >      /* The elements of this array are the CCSIDR values for each cache,
> >       * in the order L1DCache, L1ICache, L2DCache, L2ICache, etc.
> >       */
> > @@ -171,6 +172,8 @@ typedef struct ARMCPU {
> >      uint64_t rvbar;
> >  } ARMCPU;
> >
> > +#define CPUS_PER_CLUSTER 8
> 
> Changes I suggest below will mean this #define is only
> used in one place so it won't need to be in a header.
> 
> > +
> >  #define TYPE_AARCH64_CPU "aarch64-cpu"
> >  #define AARCH64_CPU_CLASS(klass) \
> >      OBJECT_CLASS_CHECK(AArch64CPUClass, (klass), TYPE_AARCH64_CPU)
> > diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> > index 4a888ab..7272466 100644
> > --- a/target-arm/cpu.c
> > +++ b/target-arm/cpu.c
> > @@ -31,6 +31,12 @@
> >  #include "sysemu/kvm.h"
> >  #include "kvm_arm.h"
> >
> > +#ifdef DEBUG
> > +# define DPRINTF(format, ...) printf("armcpu: " format , ## __VA_ARGS__)
> > +#else
> > +# define DPRINTF(format, ...) do { } while (0)
> > +#endif
> 
> Drop these debug macros, please.
or if it's useful then use trace-points instead, which could be turned on/off
without recompiling.

> 
> > +
> >  static void arm_cpu_set_pc(CPUState *cs, vaddr value)
> >  {
> >      ARMCPU *cpu = ARM_CPU(cs);
> > @@ -388,12 +394,23 @@ static void arm_cpu_initfn(Object *obj)
> >      CPUState *cs = CPU(obj);
> >      ARMCPU *cpu = ARM_CPU(obj);
> >      static bool inited;
> > +    uint32_t Aff1, Aff0;
> >
> >      cs->env_ptr = &cpu->env;
> >      cpu_exec_init(&cpu->env);
> >      cpu->cp_regs = g_hash_table_new_full(g_int_hash, g_int_equal,
> >                                           g_free, g_free);
> >
> > +    /*
> > +     * We don't support setting cluster ID ([16..23]) (known as Aff2
> > +     * in later ARM ARM versions), or any of the higher affinity level fields,
> > +     * so these bits always RAZ.
> > +     */
> > +    Aff1 = cs->cpu_index / CPUS_PER_CLUSTER;
> > +    Aff0 = cs->cpu_index % CPUS_PER_CLUSTER;
> > +    cpu->mpidr = (Aff1 << 8) | Aff0;
> > +    DPRINTF("Init(%p): index %d mpidr 0x%jX\n", cpu, cs->cpu_index, cpu->mpidr);
> > +
> >  #ifndef CONFIG_USER_ONLY
> >      /* Our inbound IRQ and FIQ lines */
> >      if (kvm_enabled()) {
> > diff --git a/target-arm/helper.c b/target-arm/helper.c
> > index 1cc4993..99c7a30 100644
> > --- a/target-arm/helper.c
> > +++ b/target-arm/helper.c
> > @@ -2063,12 +2063,9 @@ static const ARMCPRegInfo strongarm_cp_reginfo[] = {
> >
> >  static uint64_t mpidr_read(CPUARMState *env, const ARMCPRegInfo *ri)
> >  {
> > -    CPUState *cs = CPU(arm_env_get_cpu(env));
> > -    uint32_t mpidr = cs->cpu_index;
> > -    /* We don't support setting cluster ID ([8..11]) (known as Aff1
> > -     * in later ARM ARM versions), or any of the higher affinity level fields,
> > -     * so these bits always RAZ.
> > -     */
> > +    ARMCPU *cpu = ARM_CPU(arm_env_get_cpu(env));
> > +    uint64_t mpidr = cpu->mpidr;
> > +
> >      if (arm_feature(env, ARM_FEATURE_V7MP)) {
> >          mpidr |= (1U << 31);
> >          /* Cores which are uniprocessor (non-coherent)
> > diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c
> > index 93c1ca8..99fa34e 100644
> > --- a/target-arm/kvm64.c
> > +++ b/target-arm/kvm64.c
> > @@ -25,6 +25,12 @@
> >  #include "internals.h"
> >  #include "hw/arm/arm.h"
> >
> > +#ifdef DEBUG
> > +# define DPRINTF(format, ...) printf("arm-kvm64: " format , ## __VA_ARGS__)
> > +#else
> > +# define DPRINTF(format, ...) do { } while (0)
> > +#endif
> >
> >  static inline void set_feature(uint64_t *features, int feature)
> >  {
> >      *features |= 1ULL << feature;
> > @@ -77,6 +83,10 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUClass *ahcc)
> >      return true;
> >  }
> >
> > +#define ARM_MPIDR_HWID_BITMASK 0xFF00FFFFFFULL
> > +#define ARM_CPU_ID             3, 0, 0, 0
> > +#define ARM_CPU_ID_MPIDR       5
> 
> I don't see the point in separating the op2 value from the rest.
> 
> > +
> >  int kvm_arch_init_vcpu(CPUState *cs)
> >  {
> >      int ret;
> > @@ -107,6 +117,21 @@ int kvm_arch_init_vcpu(CPUState *cs)
> >          return ret;
> >      }
> >
> > +    /*
> > +     * When KVM is in use, psci is emulated in-kernel and not by qemu.
> 
> PSCI and QEMU should both be all-caps.
> 
> > +     * In order for it to work correctly we should use correct MPIDR values,
> > +     * which appear to be inherited from the host.
> 
> I don't think they're inherited from the host: it's just that
> different host kernels use different mappings from vcpu ID
> to MPIDR (see reset_mpidr() in arch/arm64/kvm/sys_regs.c; its
> implementation has changed over time).
> In any case we don't need to speculate; you can just say:
> 
>  In order for it to work correctly we must use MPIDR values in
>  the device tree which match the MPIDR values the kernel has picked
>  for the vcpus, so ask KVM what those values are.
Could we set QEMU's generated mpidr in kernel instead of pulling it from kernel,
like we do with APIC ID in x86 and fix kernel not to reset it its own value
(i.e. untie mpidr from vcpuid)?

Then later we could move setting mpidr from kvm_arch_init_vcpu() into
board code which should be setting it, since it knows/defines what topology it has.

> > +     * So we override our defaults by asking KVM about these values.
> > +     */
> > +    ret = kvm_get_one_reg(cs, ARM64_SYS_REG(ARM_CPU_ID, ARM_CPU_ID_MPIDR),
> > +                          &cpu->mpidr);
> > +    if (ret) {
> > +        return ret;
> > +    }
> > +    cpu->mpidr &= ARM_MPIDR_HWID_BITMASK;
> > +    DPRINTF("Init(%p): index %d MPIDR 0x%jX\n", cpu,
> > +            cs->cpu_index, cpu->mpidr);
> > +
> >      return kvm_arm_init_cpreg_list(cpu);
> >  }
> >
> 
> There should also be an equivalent patch to kvm32.c, because
> PSCI on 32-bit systems has the same issue.
> 
> > diff --git a/target-arm/psci.c b/target-arm/psci.c
> > index d8fafab..d03896f 100644
> > --- a/target-arm/psci.c
> > +++ b/target-arm/psci.c
> > @@ -72,6 +72,22 @@ bool arm_is_psci_call(ARMCPU *cpu, int excp_type)
> >      }
> >  }
> >
> > +static uint32_t mpidr_to_idx(uint64_t mpidr)
> > +{
> > +    uint32_t Aff1, Aff0;
> > +
> > +    /*
> > +     * MPIDR_EL1 [RES0:affinity-3:RES1:U:RES0:MT:affinity-1:affinity-0]
> > +     * GIC 500 code currently supports 32 clusters with 8 cores each
> > +     * but no more than 128 cores. Future version will have flexible
> > +     * affinity selection
> > +     * GIC 400 supports 8 cores so 0x7 for Aff0 is O.K. too
> > +     */
> > +     Aff1 = (mpidr & 0xff00) >> 8;
> > +     Aff0 = mpidr & 0xff;
> > +     return Aff1 * CPUS_PER_CLUSTER + Aff0;
> > +}
> 
> > +
> >  void arm_handle_psci_call(ARMCPU *cpu)
> >  {
> >      /*
> > @@ -121,7 +137,7 @@ void arm_handle_psci_call(ARMCPU *cpu)
> >
> >          switch (param[2]) {
> >          case 0:
> > -            target_cpu_state = qemu_get_cpu(mpidr & 0xff);
> > +            target_cpu_state = qemu_get_cpu(mpidr_to_idx(mpidr));
> 
> Rather than doing this, you can just have an
> arm_get_cpu_by_mpidr() which does
>     CPU_FOREACH(cs) {
>         ARMCPU *cpu = ARM_CPU(cs);
> 
>         if (cpu->mpidr == mpidr) {
>             return cpu;
>         }
>     }
>     return NULL;
> 
> ie same as qemu_get_cpu() but checking on mpidr rather than
> cpu_index. Then we don't need to make assumptions about the
> mpidr-to-index mapping here at all (and this code will
> continue to work if in future we decide to expose the
> cluster topology via CPU properties for the board to set.)
> 
> thanks
> -- PMM

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

* Re: [Qemu-devel] [PATCH RFC V3 1/4] Use Aff1 with mpidr This is an improved and KVM-aware alternative to
  2015-06-08  9:39         ` Pavel Fedin
  2015-06-08 10:20           ` Peter Maydell
@ 2015-06-08 10:39           ` Igor Mammedov
  1 sibling, 0 replies; 14+ messages in thread
From: Igor Mammedov @ 2015-06-08 10:39 UTC (permalink / raw)
  To: Pavel Fedin
  Cc: 'Peter Maydell', 'Eric Auger',
	'Shlomo Pongratz', 'Shlomo Pongratz',
	'QEMU Developers', 'Shannon Zhao',
	'Ashok Kumar'

On Mon, 08 Jun 2015 12:39:56 +0300
Pavel Fedin <p.fedin@samsung.com> wrote:

>  Hello!
> 
> > >  I thought about it when i developed this, but in my implementation
> > > affinity bits are assigned during CPU instantiation, while feature
> > > bits can be added later, during realize. I could tweak set_feature()
> > > to sync up MPIDR value but perhaps it isn't the best thing to do.
> > 
> > Can we not just assign the whole thing at realize?
> 
>  kvm_arch_init_vcpu() is called before realize, therefore i would have to track down whether KVM is active or not.

that's because call flow looks wrong.
If KVM would be fixed to use QEMU's mpidr then you can like x86 target

1. create vcpu
2. set mpidr property on it from board code
3. call cpu.realize() -> which will push mpidr to KVM


> 
> > You need to fix the 32-bit side anyway.
> 
>  Actually already done in my working tree.
> 
> > I still think that having the whole mpidr in the struct will
> > be less confusing.
> 
>  Ok, if you are really insisting on that, i can assign IDs where this is currently done and add feature bits in arm_cpu_realizefn().
>  Last argument: the rest of qemu code (property assignment, lookup in PSCI, etc) actually needs IDs without feature bits. The only function which needs full version is mpidr_read(). So does it still worth of putting AND with bitmasks everywhere? May be just rename 'mpidr' to something like 'mp_id' ?
> 
> Kind regards,
> Pavel Fedin
> Expert Engineer
> Samsung Electronics Research center Russia
> 

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

end of thread, other threads:[~2015-06-08 10:39 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-04 16:40 [Qemu-devel] [PATCH RFC V3 0/4] Implement GIC-500 from GICv3 family for arm64 Shlomo Pongratz
2015-06-04 16:40 ` [Qemu-devel] [PATCH RFC V3 1/4] Use Aff1 with mpidr This is an improved and KVM-aware alternative to Shlomo Pongratz
2015-06-04 17:17   ` Peter Maydell
2015-06-06 18:53     ` Shlomo Pongratz
2015-06-08  6:41       ` Pavel Fedin
2015-06-08  7:50     ` Pavel Fedin
2015-06-08  8:01       ` Peter Maydell
2015-06-08  9:39         ` Pavel Fedin
2015-06-08 10:20           ` Peter Maydell
2015-06-08 10:39           ` Igor Mammedov
2015-06-08 10:32     ` Igor Mammedov
2015-06-04 16:40 ` [Qemu-devel] [PATCH RFC V3 2/4] Implment GIC-500 Shlomo Pongratz
2015-06-04 16:40 ` [Qemu-devel] [PATCH RFC V3 3/4] GICv3 support Shlomo Pongratz
2015-06-04 16:40 ` [Qemu-devel] [PATCH RFC V3 4/4] Add virt-v3 machine that uses GIC-500 Shlomo Pongratz

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.