[Qemu-devel] [PATCH 0/6] arm_gic: add virtualization extensions support

qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed

* [Qemu-devel] [PATCH 0/6] arm_gic: add virtualization extensions support
@ 2018-06-06  9:30 luc.michel
  2018-06-06  9:30 ` [Qemu-devel] [PATCH 1/6] intc/arm_gic: Refactor operations on the distributor luc.michel
                   ` (6 more replies)
  0 siblings, 7 replies; 17+ messages in thread
From: luc.michel @ 2018-06-06  9:30 UTC (permalink / raw)
  To: qemu-devel
  Cc: Luc MICHEL, qemu-arm, Peter Maydell, saipava, edgari, mark.burton,
	Jan Kiszka

From: Luc MICHEL <luc.michel@greensocs.com>

This patch series add support for the virtualization extensions in the
ARM GICv2 interrupt controller.

The first two commits do some refactoring to prepare for the
implementation. Commits 3 and 4 are the actual implementation. The last
commit updates the ZynqMP implementation to support virtualization.

The current state allows to boot Xen (tested with 4.8 and 4.10) with
Linux Dom0 guest properly.

I also tested in SMP. It does not work directly because Xen expects to
find CPU IDs in the GIC ITARGETSR0 register. This behavior is not
documented in the GICv2 specification, and is not implemented in QEMU.
By hacking this register, I was able to get the whole thing to boot in
SMP properly. This hack is not part of those patches though.

I also tested migration, it works fine AFAIK. I had to add the HYP and
SEC timers in the ARM CPU VMState though (Xen uses the HYP one) (not
part of those patches).

I want to thanks the Xilinx's QEMU team who sponsored this work for
their collaboration.

Luc MICHEL (6):
  intc/arm_gic: Refactor operations on the distributor
  intc/arm_gic: Remove some dead code and put some functions static
  intc/arm_gic: Add the virtualization extensions to the GIC state
  intc/arm_gic: Add virtualization extensions logic
  intc/arm_gic: Improve traces
  xlnx-zynqmp: Improve GIC wiring and MMIO mapping

 hw/arm/xlnx-zynqmp.c             |  92 +++-
 hw/intc/arm_gic.c                | 768 ++++++++++++++++++++++++-------
 hw/intc/arm_gic_common.c         | 159 +++++--
 hw/intc/arm_gic_kvm.c            |  31 +-
 hw/intc/gic_internal.h           | 258 +++++++++--
 hw/intc/trace-events             |  12 +-
 include/hw/arm/xlnx-zynqmp.h     |   4 +-
 include/hw/intc/arm_gic_common.h |  52 ++-
 8 files changed, 1128 insertions(+), 248 deletions(-)

--
2.17.0

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

* [Qemu-devel] [PATCH 1/6] intc/arm_gic: Refactor operations on the distributor
  2018-06-06  9:30 [Qemu-devel] [PATCH 0/6] arm_gic: add virtualization extensions support luc.michel
@ 2018-06-06  9:30 ` luc.michel
  2018-06-06 13:38   ` [Qemu-devel] [Qemu-arm] " Philippe Mathieu-Daudé
                     ` (2 more replies)
  2018-06-06  9:30 ` [Qemu-devel] [PATCH 2/6] intc/arm_gic: Remove some dead code and put some functions static luc.michel
                   ` (5 subsequent siblings)
  6 siblings, 3 replies; 17+ messages in thread
From: luc.michel @ 2018-06-06  9:30 UTC (permalink / raw)
  To: qemu-devel
  Cc: Luc MICHEL, qemu-arm, Peter Maydell, saipava, edgari, mark.burton,
	Jan Kiszka

From: Luc MICHEL <luc.michel@greensocs.com>

In preparation for the virtualization extensions implementation,
refactor the name of the functions and macros that act on the GIC
distributor to make that fact explicit. It will be useful to
differentiate them from the ones that will act on the virtual
interfaces.

Signed-off-by: Luc MICHEL <luc.michel@greensocs.com>
---
 hw/intc/arm_gic.c        | 164 ++++++++++++++++++++-------------------
 hw/intc/arm_gic_common.c |   6 +-
 hw/intc/arm_gic_kvm.c    |  23 +++---
 hw/intc/gic_internal.h   |  51 ++++++------
 4 files changed, 127 insertions(+), 117 deletions(-)

diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
index ea0323f969..141f3e7a48 100644
--- a/hw/intc/arm_gic.c
+++ b/hw/intc/arm_gic.c
@@ -92,11 +92,12 @@ void gic_update(GICState *s)
         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) &&
-                (!GIC_TEST_ACTIVE(irq, cm)) &&
-                (irq < GIC_INTERNAL || GIC_TARGET(irq) & cm)) {
-                if (GIC_GET_PRIORITY(irq, cpu) < best_prio) {
-                    best_prio = GIC_GET_PRIORITY(irq, cpu);
+            if (GIC_DIST_TEST_ENABLED(irq, cm) &&
+                gic_test_pending(s, irq, cm) &&
+                (!GIC_DIST_TEST_ACTIVE(irq, cm)) &&
+                (irq < GIC_INTERNAL || GIC_DIST_TARGET(irq) & cm)) {
+                if (GIC_DIST_GET_PRIORITY(irq, cpu) < best_prio) {
+                    best_prio = GIC_DIST_GET_PRIORITY(irq, cpu);
                     best_irq = irq;
                 }
             }
@@ -112,7 +113,7 @@ void gic_update(GICState *s)
         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);
+                int group = GIC_DIST_TEST_GROUP(best_irq, cm);
 
                 if (extract32(s->ctlr, group, 1) &&
                     extract32(s->cpu_ctlr[cpu], group, 1)) {
@@ -145,7 +146,7 @@ void gic_set_pending_private(GICState *s, int cpu, int irq)
     }
 
     DPRINTF("Set %d pending cpu %d\n", irq, cpu);
-    GIC_SET_PENDING(irq, cm);
+    GIC_DIST_SET_PENDING(irq, cm);
     gic_update(s);
 }
 
@@ -153,13 +154,13 @@ static void gic_set_irq_11mpcore(GICState *s, int irq, int level,
                                  int cm, int target)
 {
     if (level) {
-        GIC_SET_LEVEL(irq, cm);
-        if (GIC_TEST_EDGE_TRIGGER(irq) || GIC_TEST_ENABLED(irq, cm)) {
+        GIC_DIST_SET_LEVEL(irq, cm);
+        if (GIC_DIST_TEST_EDGE_TRIGGER(irq) || GIC_DIST_TEST_ENABLED(irq, cm)) {
             DPRINTF("Set %d pending mask %x\n", irq, target);
-            GIC_SET_PENDING(irq, target);
+            GIC_DIST_SET_PENDING(irq, target);
         }
     } else {
-        GIC_CLEAR_LEVEL(irq, cm);
+        GIC_DIST_CLEAR_LEVEL(irq, cm);
     }
 }
 
@@ -167,13 +168,13 @@ static void gic_set_irq_generic(GICState *s, int irq, int level,
                                 int cm, int target)
 {
     if (level) {
-        GIC_SET_LEVEL(irq, cm);
+        GIC_DIST_SET_LEVEL(irq, cm);
         DPRINTF("Set %d pending mask %x\n", irq, target);
-        if (GIC_TEST_EDGE_TRIGGER(irq)) {
-            GIC_SET_PENDING(irq, target);
+        if (GIC_DIST_TEST_EDGE_TRIGGER(irq)) {
+            GIC_DIST_SET_PENDING(irq, target);
         }
     } else {
-        GIC_CLEAR_LEVEL(irq, cm);
+        GIC_DIST_CLEAR_LEVEL(irq, cm);
     }
 }
 
@@ -192,7 +193,7 @@ static void gic_set_irq(void *opaque, int irq, int level)
         /* The first external input line is internal interrupt 32.  */
         cm = ALL_CPU_MASK;
         irq += GIC_INTERNAL;
-        target = GIC_TARGET(irq);
+        target = GIC_DIST_TARGET(irq);
     } else {
         int cpu;
         irq -= (s->num_irq - GIC_INTERNAL);
@@ -204,7 +205,7 @@ static void gic_set_irq(void *opaque, int irq, int level)
 
     assert(irq >= GIC_NR_SGIS);
 
-    if (level == GIC_TEST_LEVEL(irq, cm)) {
+    if (level == GIC_DIST_TEST_LEVEL(irq, cm)) {
         return;
     }
 
@@ -224,7 +225,7 @@ static uint16_t gic_get_current_pending_irq(GICState *s, int cpu,
     uint16_t pending_irq = s->current_pending[cpu];
 
     if (pending_irq < GIC_MAXIRQ && gic_has_groups(s)) {
-        int group = GIC_TEST_GROUP(pending_irq, (1 << cpu));
+        int group = GIC_DIST_TEST_GROUP(pending_irq, (1 << cpu));
         /* On a GIC without the security extensions, reading this register
          * behaves in the same way as a secure access to a GIC with them.
          */
@@ -255,7 +256,7 @@ static int gic_get_group_priority(GICState *s, int cpu, int irq)
 
     if (gic_has_groups(s) &&
         !(s->cpu_ctlr[cpu] & GICC_CTLR_CBPR) &&
-        GIC_TEST_GROUP(irq, (1 << cpu))) {
+        GIC_DIST_TEST_GROUP(irq, (1 << cpu))) {
         bpr = s->abpr[cpu] - 1;
         assert(bpr >= 0);
     } else {
@@ -268,7 +269,7 @@ static int gic_get_group_priority(GICState *s, int cpu, int irq)
      */
     mask = ~0U << ((bpr & 7) + 1);
 
-    return GIC_GET_PRIORITY(irq, cpu) & mask;
+    return GIC_DIST_GET_PRIORITY(irq, cpu) & mask;
 }
 
 static void gic_activate_irq(GICState *s, int cpu, int irq)
@@ -281,14 +282,14 @@ static void gic_activate_irq(GICState *s, int cpu, int irq)
     int regno = preemption_level / 32;
     int bitno = preemption_level % 32;
 
-    if (gic_has_groups(s) && GIC_TEST_GROUP(irq, (1 << cpu))) {
+    if (gic_has_groups(s) && GIC_DIST_TEST_GROUP(irq, (1 << cpu))) {
         s->nsapr[regno][cpu] |= (1 << bitno);
     } else {
         s->apr[regno][cpu] |= (1 << bitno);
     }
 
     s->running_priority[cpu] = prio;
-    GIC_SET_ACTIVE(irq, 1 << cpu);
+    GIC_DIST_SET_ACTIVE(irq, 1 << cpu);
 }
 
 static int gic_get_prio_from_apr_bits(GICState *s, int cpu)
@@ -357,7 +358,7 @@ uint32_t gic_acknowledge_irq(GICState *s, int cpu, MemTxAttrs attrs)
         return irq;
     }
 
-    if (GIC_GET_PRIORITY(irq, cpu) >= s->running_priority[cpu]) {
+    if (GIC_DIST_GET_PRIORITY(irq, cpu) >= s->running_priority[cpu]) {
         DPRINTF("ACK, pending interrupt (%d) has insufficient priority\n", irq);
         return 1023;
     }
@@ -366,7 +367,8 @@ uint32_t gic_acknowledge_irq(GICState *s, int cpu, MemTxAttrs attrs)
         /* Clear pending flags for both level and edge triggered interrupts.
          * Level triggered IRQs will be reasserted once they become inactive.
          */
-        GIC_CLEAR_PENDING(irq, GIC_TEST_MODEL(irq) ? ALL_CPU_MASK : cm);
+        GIC_DIST_CLEAR_PENDING(irq, GIC_DIST_TEST_MODEL(irq) ? ALL_CPU_MASK
+                                                             : cm);
         ret = irq;
     } else {
         if (irq < GIC_NR_SGIS) {
@@ -378,7 +380,9 @@ uint32_t gic_acknowledge_irq(GICState *s, int cpu, MemTxAttrs attrs)
             src = ctz32(s->sgi_pending[irq][cpu]);
             s->sgi_pending[irq][cpu] &= ~(1 << src);
             if (s->sgi_pending[irq][cpu] == 0) {
-                GIC_CLEAR_PENDING(irq, GIC_TEST_MODEL(irq) ? ALL_CPU_MASK : cm);
+                GIC_DIST_CLEAR_PENDING(irq,
+                                       GIC_DIST_TEST_MODEL(irq) ? ALL_CPU_MASK
+                                                                : cm);
             }
             ret = irq | ((src & 0x7) << 10);
         } else {
@@ -386,7 +390,8 @@ uint32_t gic_acknowledge_irq(GICState *s, int cpu, MemTxAttrs attrs)
              * interrupts. (level triggered interrupts with an active line
              * remain pending, see gic_test_pending)
              */
-            GIC_CLEAR_PENDING(irq, GIC_TEST_MODEL(irq) ? ALL_CPU_MASK : cm);
+            GIC_DIST_CLEAR_PENDING(irq, GIC_DIST_TEST_MODEL(irq) ? ALL_CPU_MASK
+                                                                 : cm);
             ret = irq;
         }
     }
@@ -397,11 +402,11 @@ uint32_t gic_acknowledge_irq(GICState *s, int cpu, MemTxAttrs attrs)
     return ret;
 }
 
-void gic_set_priority(GICState *s, int cpu, int irq, uint8_t val,
+void gic_dist_set_priority(GICState *s, int cpu, int irq, uint8_t val,
                       MemTxAttrs attrs)
 {
     if (s->security_extn && !attrs.secure) {
-        if (!GIC_TEST_GROUP(irq, (1 << cpu))) {
+        if (!GIC_DIST_TEST_GROUP(irq, (1 << cpu))) {
             return; /* Ignore Non-secure access of Group0 IRQ */
         }
         val = 0x80 | (val >> 1); /* Non-secure view */
@@ -414,13 +419,13 @@ void gic_set_priority(GICState *s, int cpu, int irq, uint8_t val,
     }
 }
 
-static uint32_t gic_get_priority(GICState *s, int cpu, int irq,
+static uint32_t gic_dist_get_priority(GICState *s, int cpu, int irq,
                                  MemTxAttrs attrs)
 {
-    uint32_t prio = GIC_GET_PRIORITY(irq, cpu);
+    uint32_t prio = GIC_DIST_GET_PRIORITY(irq, cpu);
 
     if (s->security_extn && !attrs.secure) {
-        if (!GIC_TEST_GROUP(irq, (1 << cpu))) {
+        if (!GIC_DIST_TEST_GROUP(irq, (1 << cpu))) {
             return 0; /* Non-secure access cannot read priority of Group0 IRQ */
         }
         prio = (prio << 1) & 0xff; /* Non-secure view */
@@ -543,7 +548,7 @@ static bool gic_eoi_split(GICState *s, int cpu, MemTxAttrs attrs)
 static void gic_deactivate_irq(GICState *s, int cpu, int irq, MemTxAttrs attrs)
 {
     int cm = 1 << cpu;
-    int group = gic_has_groups(s) && GIC_TEST_GROUP(irq, cm);
+    int group = gic_has_groups(s) && GIC_DIST_TEST_GROUP(irq, cm);
 
     if (!gic_eoi_split(s, cpu, attrs)) {
         /* This is UNPREDICTABLE; we choose to ignore it */
@@ -557,7 +562,7 @@ static void gic_deactivate_irq(GICState *s, int cpu, int irq, MemTxAttrs attrs)
         return;
     }
 
-    GIC_CLEAR_ACTIVE(irq, cm);
+    GIC_DIST_CLEAR_ACTIVE(irq, cm);
 }
 
 void gic_complete_irq(GICState *s, int cpu, int irq, MemTxAttrs attrs)
@@ -584,14 +589,15 @@ void gic_complete_irq(GICState *s, int cpu, int irq, MemTxAttrs attrs)
     if (s->revision == REV_11MPCORE) {
         /* Mark level triggered interrupts as pending if they are still
            raised.  */
-        if (!GIC_TEST_EDGE_TRIGGER(irq) && GIC_TEST_ENABLED(irq, cm)
-            && GIC_TEST_LEVEL(irq, cm) && (GIC_TARGET(irq) & cm) != 0) {
+        if (!GIC_DIST_TEST_EDGE_TRIGGER(irq) && GIC_DIST_TEST_ENABLED(irq, cm)
+            && GIC_DIST_TEST_LEVEL(irq, cm)
+            && (GIC_DIST_TARGET(irq) & cm) != 0) {
             DPRINTF("Set %d pending mask %x\n", irq, cm);
-            GIC_SET_PENDING(irq, cm);
+            GIC_DIST_SET_PENDING(irq, cm);
         }
     }
 
-    group = gic_has_groups(s) && GIC_TEST_GROUP(irq, cm);
+    group = gic_has_groups(s) && GIC_DIST_TEST_GROUP(irq, cm);
 
     if (s->security_extn && !attrs.secure && !group) {
         DPRINTF("Non-secure EOI for Group0 interrupt %d ignored\n", irq);
@@ -607,7 +613,7 @@ void gic_complete_irq(GICState *s, int cpu, int irq, MemTxAttrs attrs)
 
     /* In GICv2 the guest can choose to split priority-drop and deactivate */
     if (!gic_eoi_split(s, cpu, attrs)) {
-        GIC_CLEAR_ACTIVE(irq, cm);
+        GIC_DIST_CLEAR_ACTIVE(irq, cm);
     }
     gic_update(s);
 }
@@ -655,7 +661,7 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr offset, MemTxAttrs attrs)
                     goto bad_reg;
                 }
                 for (i = 0; i < 8; i++) {
-                    if (GIC_TEST_GROUP(irq + i, cm)) {
+                    if (GIC_DIST_TEST_GROUP(irq + i, cm)) {
                         res |= (1 << i);
                     }
                 }
@@ -675,11 +681,11 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr offset, MemTxAttrs attrs)
         res = 0;
         for (i = 0; i < 8; i++) {
             if (s->security_extn && !attrs.secure &&
-                !GIC_TEST_GROUP(irq + i, 1 << cpu)) {
+                !GIC_DIST_TEST_GROUP(irq + i, 1 << cpu)) {
                 continue; /* Ignore Non-secure access of Group0 IRQ */
             }
 
-            if (GIC_TEST_ENABLED(irq + i, cm)) {
+            if (GIC_DIST_TEST_ENABLED(irq + i, cm)) {
                 res |= (1 << i);
             }
         }
@@ -696,7 +702,7 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr offset, MemTxAttrs attrs)
         mask = (irq < GIC_INTERNAL) ?  cm : ALL_CPU_MASK;
         for (i = 0; i < 8; i++) {
             if (s->security_extn && !attrs.secure &&
-                !GIC_TEST_GROUP(irq + i, 1 << cpu)) {
+                !GIC_DIST_TEST_GROUP(irq + i, 1 << cpu)) {
                 continue; /* Ignore Non-secure access of Group0 IRQ */
             }
 
@@ -713,11 +719,11 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr offset, MemTxAttrs attrs)
         mask = (irq < GIC_INTERNAL) ?  cm : ALL_CPU_MASK;
         for (i = 0; i < 8; i++) {
             if (s->security_extn && !attrs.secure &&
-                !GIC_TEST_GROUP(irq + i, 1 << cpu)) {
+                !GIC_DIST_TEST_GROUP(irq + i, 1 << cpu)) {
                 continue; /* Ignore Non-secure access of Group0 IRQ */
             }
 
-            if (GIC_TEST_ACTIVE(irq + i, mask)) {
+            if (GIC_DIST_TEST_ACTIVE(irq + i, mask)) {
                 res |= (1 << i);
             }
         }
@@ -726,7 +732,7 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr offset, MemTxAttrs attrs)
         irq = (offset - 0x400) + GIC_BASE_IRQ;
         if (irq >= s->num_irq)
             goto bad_reg;
-        res = gic_get_priority(s, cpu, irq, attrs);
+        res = gic_dist_get_priority(s, cpu, irq, attrs);
     } else if (offset < 0xc00) {
         /* Interrupt CPU Target.  */
         if (s->num_cpu == 1 && s->revision != REV_11MPCORE) {
@@ -740,7 +746,7 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr offset, MemTxAttrs attrs)
             if (irq >= 29 && irq <= 31) {
                 res = cm;
             } else {
-                res = GIC_TARGET(irq);
+                res = GIC_DIST_TARGET(irq);
             }
         }
     } else if (offset < 0xf00) {
@@ -751,14 +757,16 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr offset, MemTxAttrs attrs)
         res = 0;
         for (i = 0; i < 4; i++) {
             if (s->security_extn && !attrs.secure &&
-                !GIC_TEST_GROUP(irq + i, 1 << cpu)) {
+                !GIC_DIST_TEST_GROUP(irq + i, 1 << cpu)) {
                 continue; /* Ignore Non-secure access of Group0 IRQ */
             }
 
-            if (GIC_TEST_MODEL(irq + i))
+            if (GIC_DIST_TEST_MODEL(irq + i)) {
                 res |= (1 << (i * 2));
-            if (GIC_TEST_EDGE_TRIGGER(irq + i))
+            }
+            if (GIC_DIST_TEST_EDGE_TRIGGER(irq + i)) {
                 res |= (2 << (i * 2));
+            }
         }
     } else if (offset < 0xf10) {
         goto bad_reg;
@@ -776,7 +784,7 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr offset, MemTxAttrs attrs)
         }
 
         if (s->security_extn && !attrs.secure &&
-            !GIC_TEST_GROUP(irq, 1 << cpu)) {
+            !GIC_DIST_TEST_GROUP(irq, 1 << cpu)) {
             res = 0; /* Ignore Non-secure access of Group0 IRQ */
         } else {
             res = s->sgi_pending[irq][cpu];
@@ -872,10 +880,10 @@ static void gic_dist_writeb(void *opaque, hwaddr offset,
                     int cm = (irq < GIC_INTERNAL) ? (1 << cpu) : ALL_CPU_MASK;
                     if (value & (1 << i)) {
                         /* Group1 (Non-secure) */
-                        GIC_SET_GROUP(irq + i, cm);
+                        GIC_DIST_SET_GROUP(irq + i, cm);
                     } else {
                         /* Group0 (Secure) */
-                        GIC_CLEAR_GROUP(irq + i, cm);
+                        GIC_DIST_CLEAR_GROUP(irq + i, cm);
                     }
                 }
             }
@@ -893,26 +901,26 @@ static void gic_dist_writeb(void *opaque, hwaddr offset,
 
         for (i = 0; i < 8; i++) {
             if (value & (1 << i)) {
-                int mask =
-                    (irq < GIC_INTERNAL) ? (1 << cpu) : GIC_TARGET(irq + i);
+                int mask = (irq < GIC_INTERNAL) ? (1 << cpu)
+                                                : GIC_DIST_TARGET(irq + i);
                 int cm = (irq < GIC_INTERNAL) ? (1 << cpu) : ALL_CPU_MASK;
 
                 if (s->security_extn && !attrs.secure &&
-                    !GIC_TEST_GROUP(irq + i, 1 << cpu)) {
+                    !GIC_DIST_TEST_GROUP(irq + i, 1 << cpu)) {
                     continue; /* Ignore Non-secure access of Group0 IRQ */
                 }
 
-                if (!GIC_TEST_ENABLED(irq + i, cm)) {
+                if (!GIC_DIST_TEST_ENABLED(irq + i, cm)) {
                     DPRINTF("Enabled IRQ %d\n", irq + i);
                     trace_gic_enable_irq(irq + i);
                 }
-                GIC_SET_ENABLED(irq + i, cm);
+                GIC_DIST_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)) {
+                if (GIC_DIST_TEST_LEVEL(irq + i, mask)
+                        && !GIC_DIST_TEST_EDGE_TRIGGER(irq + i)) {
                     DPRINTF("Set %d pending mask %x\n", irq + i, mask);
-                    GIC_SET_PENDING(irq + i, mask);
+                    GIC_DIST_SET_PENDING(irq + i, mask);
                 }
             }
         }
@@ -930,15 +938,15 @@ static void gic_dist_writeb(void *opaque, hwaddr offset,
                 int cm = (irq < GIC_INTERNAL) ? (1 << cpu) : ALL_CPU_MASK;
 
                 if (s->security_extn && !attrs.secure &&
-                    !GIC_TEST_GROUP(irq + i, 1 << cpu)) {
+                    !GIC_DIST_TEST_GROUP(irq + i, 1 << cpu)) {
                     continue; /* Ignore Non-secure access of Group0 IRQ */
                 }
 
-                if (GIC_TEST_ENABLED(irq + i, cm)) {
+                if (GIC_DIST_TEST_ENABLED(irq + i, cm)) {
                     DPRINTF("Disabled IRQ %d\n", irq + i);
                     trace_gic_disable_irq(irq + i);
                 }
-                GIC_CLEAR_ENABLED(irq + i, cm);
+                GIC_DIST_CLEAR_ENABLED(irq + i, cm);
             }
         }
     } else if (offset < 0x280) {
@@ -953,11 +961,11 @@ static void gic_dist_writeb(void *opaque, hwaddr offset,
         for (i = 0; i < 8; i++) {
             if (value & (1 << i)) {
                 if (s->security_extn && !attrs.secure &&
-                    !GIC_TEST_GROUP(irq + i, 1 << cpu)) {
+                    !GIC_DIST_TEST_GROUP(irq + i, 1 << cpu)) {
                     continue; /* Ignore Non-secure access of Group0 IRQ */
                 }
 
-                GIC_SET_PENDING(irq + i, GIC_TARGET(irq + i));
+                GIC_DIST_SET_PENDING(irq + i, GIC_DIST_TARGET(irq + i));
             }
         }
     } else if (offset < 0x300) {
@@ -971,7 +979,7 @@ static void gic_dist_writeb(void *opaque, hwaddr offset,
 
         for (i = 0; i < 8; i++) {
             if (s->security_extn && !attrs.secure &&
-                !GIC_TEST_GROUP(irq + i, 1 << cpu)) {
+                !GIC_DIST_TEST_GROUP(irq + i, 1 << cpu)) {
                 continue; /* Ignore Non-secure access of Group0 IRQ */
             }
 
@@ -979,7 +987,7 @@ static void gic_dist_writeb(void *opaque, hwaddr offset,
                for per-CPU interrupts.  It's unclear whether this is the
                corect behavior.  */
             if (value & (1 << i)) {
-                GIC_CLEAR_PENDING(irq + i, ALL_CPU_MASK);
+                GIC_DIST_CLEAR_PENDING(irq + i, ALL_CPU_MASK);
             }
         }
     } else if (offset < 0x400) {
@@ -990,7 +998,7 @@ static void gic_dist_writeb(void *opaque, hwaddr offset,
         irq = (offset - 0x400) + GIC_BASE_IRQ;
         if (irq >= s->num_irq)
             goto bad_reg;
-        gic_set_priority(s, cpu, irq, value, attrs);
+        gic_dist_set_priority(s, cpu, irq, value, attrs);
     } else if (offset < 0xc00) {
         /* Interrupt CPU Target. RAZ/WI on uniprocessor GICs, with the
          * annoying exception of the 11MPCore's GIC.
@@ -1016,21 +1024,21 @@ static void gic_dist_writeb(void *opaque, hwaddr offset,
             value |= 0xaa;
         for (i = 0; i < 4; i++) {
             if (s->security_extn && !attrs.secure &&
-                !GIC_TEST_GROUP(irq + i, 1 << cpu)) {
+                !GIC_DIST_TEST_GROUP(irq + i, 1 << cpu)) {
                 continue; /* Ignore Non-secure access of Group0 IRQ */
             }
 
             if (s->revision == REV_11MPCORE) {
                 if (value & (1 << (i * 2))) {
-                    GIC_SET_MODEL(irq + i);
+                    GIC_DIST_SET_MODEL(irq + i);
                 } else {
-                    GIC_CLEAR_MODEL(irq + i);
+                    GIC_DIST_CLEAR_MODEL(irq + i);
                 }
             }
             if (value & (2 << (i * 2))) {
-                GIC_SET_EDGE_TRIGGER(irq + i);
+                GIC_DIST_SET_EDGE_TRIGGER(irq + i);
             } else {
-                GIC_CLEAR_EDGE_TRIGGER(irq + i);
+                GIC_DIST_CLEAR_EDGE_TRIGGER(irq + i);
             }
         }
     } else if (offset < 0xf10) {
@@ -1044,10 +1052,10 @@ static void gic_dist_writeb(void *opaque, hwaddr offset,
         irq = (offset - 0xf10);
 
         if (!s->security_extn || attrs.secure ||
-            GIC_TEST_GROUP(irq, 1 << cpu)) {
+            GIC_DIST_TEST_GROUP(irq, 1 << cpu)) {
             s->sgi_pending[irq][cpu] &= ~value;
             if (s->sgi_pending[irq][cpu] == 0) {
-                GIC_CLEAR_PENDING(irq, 1 << cpu);
+                GIC_DIST_CLEAR_PENDING(irq, 1 << cpu);
             }
         }
     } else if (offset < 0xf30) {
@@ -1058,8 +1066,8 @@ static void gic_dist_writeb(void *opaque, hwaddr offset,
         irq = (offset - 0xf20);
 
         if (!s->security_extn || attrs.secure ||
-            GIC_TEST_GROUP(irq, 1 << cpu)) {
-            GIC_SET_PENDING(irq, 1 << cpu);
+            GIC_DIST_TEST_GROUP(irq, 1 << cpu)) {
+            GIC_DIST_SET_PENDING(irq, 1 << cpu);
             s->sgi_pending[irq][cpu] |= value;
         }
     } else {
@@ -1106,7 +1114,7 @@ static void gic_dist_writel(void *opaque, hwaddr offset,
             mask = ALL_CPU_MASK;
             break;
         }
-        GIC_SET_PENDING(irq, mask);
+        GIC_DIST_SET_PENDING(irq, mask);
         target_cpu = ctz32(mask);
         while (target_cpu < GIC_NCPU) {
             s->sgi_pending[irq][target_cpu] |= (1 << cpu);
diff --git a/hw/intc/arm_gic_common.c b/hw/intc/arm_gic_common.c
index aee50a20e0..295ee9cc5e 100644
--- a/hw/intc/arm_gic_common.c
+++ b/hw/intc/arm_gic_common.c
@@ -204,8 +204,8 @@ static void arm_gic_common_reset(DeviceState *dev)
         }
     }
     for (i = 0; i < GIC_NR_SGIS; i++) {
-        GIC_SET_ENABLED(i, ALL_CPU_MASK);
-        GIC_SET_EDGE_TRIGGER(i);
+        GIC_DIST_SET_ENABLED(i, ALL_CPU_MASK);
+        GIC_DIST_SET_EDGE_TRIGGER(i);
     }
 
     for (i = 0; i < ARRAY_SIZE(s->priority2); i++) {
@@ -222,7 +222,7 @@ static void arm_gic_common_reset(DeviceState *dev)
     }
     if (s->security_extn && s->irq_reset_nonsecure) {
         for (i = 0; i < GIC_MAXIRQ; i++) {
-            GIC_SET_GROUP(i, ALL_CPU_MASK);
+            GIC_DIST_SET_GROUP(i, ALL_CPU_MASK);
         }
     }
 
diff --git a/hw/intc/arm_gic_kvm.c b/hw/intc/arm_gic_kvm.c
index 204369d0e2..799136732a 100644
--- a/hw/intc/arm_gic_kvm.c
+++ b/hw/intc/arm_gic_kvm.c
@@ -140,10 +140,10 @@ static void translate_group(GICState *s, int irq, int cpu,
     int cm = (irq < GIC_INTERNAL) ? (1 << cpu) : ALL_CPU_MASK;
 
     if (to_kernel) {
-        *field = GIC_TEST_GROUP(irq, cm);
+        *field = GIC_DIST_TEST_GROUP(irq, cm);
     } else {
         if (*field & 1) {
-            GIC_SET_GROUP(irq, cm);
+            GIC_DIST_SET_GROUP(irq, cm);
         }
     }
 }
@@ -154,10 +154,10 @@ static void translate_enabled(GICState *s, int irq, int cpu,
     int cm = (irq < GIC_INTERNAL) ? (1 << cpu) : ALL_CPU_MASK;
 
     if (to_kernel) {
-        *field = GIC_TEST_ENABLED(irq, cm);
+        *field = GIC_DIST_TEST_ENABLED(irq, cm);
     } else {
         if (*field & 1) {
-            GIC_SET_ENABLED(irq, cm);
+            GIC_DIST_SET_ENABLED(irq, cm);
         }
     }
 }
@@ -171,7 +171,7 @@ static void translate_pending(GICState *s, int irq, int cpu,
         *field = gic_test_pending(s, irq, cm);
     } else {
         if (*field & 1) {
-            GIC_SET_PENDING(irq, cm);
+            GIC_DIST_SET_PENDING(irq, cm);
             /* TODO: Capture is level-line is held high in the kernel */
         }
     }
@@ -183,10 +183,10 @@ static void translate_active(GICState *s, int irq, int cpu,
     int cm = (irq < GIC_INTERNAL) ? (1 << cpu) : ALL_CPU_MASK;
 
     if (to_kernel) {
-        *field = GIC_TEST_ACTIVE(irq, cm);
+        *field = GIC_DIST_TEST_ACTIVE(irq, cm);
     } else {
         if (*field & 1) {
-            GIC_SET_ACTIVE(irq, cm);
+            GIC_DIST_SET_ACTIVE(irq, cm);
         }
     }
 }
@@ -195,10 +195,10 @@ static void translate_trigger(GICState *s, int irq, int cpu,
                               uint32_t *field, bool to_kernel)
 {
     if (to_kernel) {
-        *field = (GIC_TEST_EDGE_TRIGGER(irq)) ? 0x2 : 0x0;
+        *field = (GIC_DIST_TEST_EDGE_TRIGGER(irq)) ? 0x2 : 0x0;
     } else {
         if (*field & 0x2) {
-            GIC_SET_EDGE_TRIGGER(irq);
+            GIC_DIST_SET_EDGE_TRIGGER(irq);
         }
     }
 }
@@ -207,9 +207,10 @@ static void translate_priority(GICState *s, int irq, int cpu,
                                uint32_t *field, bool to_kernel)
 {
     if (to_kernel) {
-        *field = GIC_GET_PRIORITY(irq, cpu) & 0xff;
+        *field = GIC_DIST_GET_PRIORITY(irq, cpu) & 0xff;
     } else {
-        gic_set_priority(s, cpu, irq, *field & 0xff, MEMTXATTRS_UNSPECIFIED);
+        gic_dist_set_priority(s, cpu, irq,
+                              *field & 0xff, MEMTXATTRS_UNSPECIFIED);
     }
 }
 
diff --git a/hw/intc/gic_internal.h b/hw/intc/gic_internal.h
index 7fe87b13de..6f8d242904 100644
--- a/hw/intc/gic_internal.h
+++ b/hw/intc/gic_internal.h
@@ -27,30 +27,31 @@
 
 #define GIC_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_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_MODEL(irq) s->irq_state[irq].model = true
-#define GIC_CLEAR_MODEL(irq) s->irq_state[irq].model = false
-#define GIC_TEST_MODEL(irq) s->irq_state[irq].model
-#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) < GIC_INTERNAL) ?            \
+#define GIC_DIST_SET_ENABLED(irq, cm) (s->irq_state[irq].enabled |= (cm))
+#define GIC_DIST_CLEAR_ENABLED(irq, cm) (s->irq_state[irq].enabled &= ~(cm))
+#define GIC_DIST_TEST_ENABLED(irq, cm) ((s->irq_state[irq].enabled & (cm)) != 0)
+#define GIC_DIST_SET_PENDING(irq, cm) (s->irq_state[irq].pending |= (cm))
+#define GIC_DIST_CLEAR_PENDING(irq, cm) (s->irq_state[irq].pending &= ~(cm))
+#define GIC_DIST_SET_ACTIVE(irq, cm) (s->irq_state[irq].active |= (cm))
+#define GIC_DIST_CLEAR_ACTIVE(irq, cm) (s->irq_state[irq].active &= ~(cm))
+#define GIC_DIST_TEST_ACTIVE(irq, cm) ((s->irq_state[irq].active & (cm)) != 0)
+#define GIC_DIST_SET_MODEL(irq) (s->irq_state[irq].model = true)
+#define GIC_DIST_CLEAR_MODEL(irq) (s->irq_state[irq].model = false)
+#define GIC_DIST_TEST_MODEL(irq) (s->irq_state[irq].model)
+#define GIC_DIST_SET_LEVEL(irq, cm) (s->irq_state[irq].level |= (cm))
+#define GIC_DIST_CLEAR_LEVEL(irq, cm) (s->irq_state[irq].level &= ~(cm))
+#define GIC_DIST_TEST_LEVEL(irq, cm) ((s->irq_state[irq].level & (cm)) != 0)
+#define GIC_DIST_SET_EDGE_TRIGGER(irq) (s->irq_state[irq].edge_trigger = true)
+#define GIC_DIST_CLEAR_EDGE_TRIGGER(irq) \
+    (s->irq_state[irq].edge_trigger = false)
+#define GIC_DIST_TEST_EDGE_TRIGGER(irq) (s->irq_state[irq].edge_trigger)
+#define GIC_DIST_GET_PRIORITY(irq, cpu) (((irq) < GIC_INTERNAL) ?            \
                                     s->priority1[irq][cpu] :            \
                                     s->priority2[(irq) - GIC_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)
+#define GIC_DIST_TARGET(irq) (s->irq_target[irq])
+#define GIC_DIST_CLEAR_GROUP(irq, cm) (s->irq_state[irq].group &= ~(cm))
+#define GIC_DIST_SET_GROUP(irq, cm) (s->irq_state[irq].group |= (cm))
+#define GIC_DIST_TEST_GROUP(irq, cm) ((s->irq_state[irq].group & (cm)) != 0)
 
 #define GICD_CTLR_EN_GRP0 (1U << 0)
 #define GICD_CTLR_EN_GRP1 (1U << 1)
@@ -79,8 +80,8 @@ uint32_t gic_acknowledge_irq(GICState *s, int cpu, MemTxAttrs attrs);
 void gic_complete_irq(GICState *s, int cpu, int irq, MemTxAttrs attrs);
 void gic_update(GICState *s);
 void gic_init_irqs_and_distributor(GICState *s);
-void gic_set_priority(GICState *s, int cpu, int irq, uint8_t val,
-                      MemTxAttrs attrs);
+void gic_dist_set_priority(GICState *s, int cpu, int irq, uint8_t val,
+                           MemTxAttrs attrs);
 
 static inline bool gic_test_pending(GICState *s, int irq, int cm)
 {
@@ -93,7 +94,7 @@ static inline bool gic_test_pending(GICState *s, int irq, int cm)
          * GICD_ISPENDR to set the state pending.
          */
         return (s->irq_state[irq].pending & cm) ||
-            (!GIC_TEST_EDGE_TRIGGER(irq) && GIC_TEST_LEVEL(irq, cm));
+            (!GIC_DIST_TEST_EDGE_TRIGGER(irq) && GIC_DIST_TEST_LEVEL(irq, cm));
     }
 }
 
-- 
2.17.0

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

* Re: [Qemu-devel] [Qemu-arm] [PATCH 1/6] intc/arm_gic: Refactor operations on the distributor
  2018-06-06  9:30 ` [Qemu-devel] [PATCH 1/6] intc/arm_gic: Refactor operations on the distributor luc.michel
@ 2018-06-06 13:38   ` Philippe Mathieu-Daudé
  2018-06-07  6:08   ` [Qemu-devel] " Sai Pavan Boddu
  2018-06-11 13:39   ` Peter Maydell
  2 siblings, 0 replies; 17+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-06-06 13:38 UTC (permalink / raw)
  To: luc.michel, qemu-devel
  Cc: Peter Maydell, mark.burton, saipava, edgari, qemu-arm, Jan Kiszka

On 06/06/2018 06:30 AM, luc.michel@greensocs.com wrote:
> From: Luc MICHEL <luc.michel@greensocs.com>
> 
> In preparation for the virtualization extensions implementation,
> refactor the name of the functions and macros that act on the GIC
> distributor to make that fact explicit. It will be useful to
> differentiate them from the ones that will act on the virtual
> interfaces.
> 
> Signed-off-by: Luc MICHEL <luc.michel@greensocs.com>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  hw/intc/arm_gic.c        | 164 ++++++++++++++++++++-------------------
>  hw/intc/arm_gic_common.c |   6 +-
>  hw/intc/arm_gic_kvm.c    |  23 +++---
>  hw/intc/gic_internal.h   |  51 ++++++------
>  4 files changed, 127 insertions(+), 117 deletions(-)
> 
> diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
> index ea0323f969..141f3e7a48 100644
> --- a/hw/intc/arm_gic.c
> +++ b/hw/intc/arm_gic.c
> @@ -92,11 +92,12 @@ void gic_update(GICState *s)
>          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) &&
> -                (!GIC_TEST_ACTIVE(irq, cm)) &&
> -                (irq < GIC_INTERNAL || GIC_TARGET(irq) & cm)) {
> -                if (GIC_GET_PRIORITY(irq, cpu) < best_prio) {
> -                    best_prio = GIC_GET_PRIORITY(irq, cpu);
> +            if (GIC_DIST_TEST_ENABLED(irq, cm) &&
> +                gic_test_pending(s, irq, cm) &&
> +                (!GIC_DIST_TEST_ACTIVE(irq, cm)) &&
> +                (irq < GIC_INTERNAL || GIC_DIST_TARGET(irq) & cm)) {
> +                if (GIC_DIST_GET_PRIORITY(irq, cpu) < best_prio) {
> +                    best_prio = GIC_DIST_GET_PRIORITY(irq, cpu);
>                      best_irq = irq;
>                  }
>              }
> @@ -112,7 +113,7 @@ void gic_update(GICState *s)
>          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);
> +                int group = GIC_DIST_TEST_GROUP(best_irq, cm);
>  
>                  if (extract32(s->ctlr, group, 1) &&
>                      extract32(s->cpu_ctlr[cpu], group, 1)) {
> @@ -145,7 +146,7 @@ void gic_set_pending_private(GICState *s, int cpu, int irq)
>      }
>  
>      DPRINTF("Set %d pending cpu %d\n", irq, cpu);
> -    GIC_SET_PENDING(irq, cm);
> +    GIC_DIST_SET_PENDING(irq, cm);
>      gic_update(s);
>  }
>  
> @@ -153,13 +154,13 @@ static void gic_set_irq_11mpcore(GICState *s, int irq, int level,
>                                   int cm, int target)
>  {
>      if (level) {
> -        GIC_SET_LEVEL(irq, cm);
> -        if (GIC_TEST_EDGE_TRIGGER(irq) || GIC_TEST_ENABLED(irq, cm)) {
> +        GIC_DIST_SET_LEVEL(irq, cm);
> +        if (GIC_DIST_TEST_EDGE_TRIGGER(irq) || GIC_DIST_TEST_ENABLED(irq, cm)) {
>              DPRINTF("Set %d pending mask %x\n", irq, target);
> -            GIC_SET_PENDING(irq, target);
> +            GIC_DIST_SET_PENDING(irq, target);
>          }
>      } else {
> -        GIC_CLEAR_LEVEL(irq, cm);
> +        GIC_DIST_CLEAR_LEVEL(irq, cm);
>      }
>  }
>  
> @@ -167,13 +168,13 @@ static void gic_set_irq_generic(GICState *s, int irq, int level,
>                                  int cm, int target)
>  {
>      if (level) {
> -        GIC_SET_LEVEL(irq, cm);
> +        GIC_DIST_SET_LEVEL(irq, cm);
>          DPRINTF("Set %d pending mask %x\n", irq, target);
> -        if (GIC_TEST_EDGE_TRIGGER(irq)) {
> -            GIC_SET_PENDING(irq, target);
> +        if (GIC_DIST_TEST_EDGE_TRIGGER(irq)) {
> +            GIC_DIST_SET_PENDING(irq, target);
>          }
>      } else {
> -        GIC_CLEAR_LEVEL(irq, cm);
> +        GIC_DIST_CLEAR_LEVEL(irq, cm);
>      }
>  }
>  
> @@ -192,7 +193,7 @@ static void gic_set_irq(void *opaque, int irq, int level)
>          /* The first external input line is internal interrupt 32.  */
>          cm = ALL_CPU_MASK;
>          irq += GIC_INTERNAL;
> -        target = GIC_TARGET(irq);
> +        target = GIC_DIST_TARGET(irq);
>      } else {
>          int cpu;
>          irq -= (s->num_irq - GIC_INTERNAL);
> @@ -204,7 +205,7 @@ static void gic_set_irq(void *opaque, int irq, int level)
>  
>      assert(irq >= GIC_NR_SGIS);
>  
> -    if (level == GIC_TEST_LEVEL(irq, cm)) {
> +    if (level == GIC_DIST_TEST_LEVEL(irq, cm)) {
>          return;
>      }
>  
> @@ -224,7 +225,7 @@ static uint16_t gic_get_current_pending_irq(GICState *s, int cpu,
>      uint16_t pending_irq = s->current_pending[cpu];
>  
>      if (pending_irq < GIC_MAXIRQ && gic_has_groups(s)) {
> -        int group = GIC_TEST_GROUP(pending_irq, (1 << cpu));
> +        int group = GIC_DIST_TEST_GROUP(pending_irq, (1 << cpu));
>          /* On a GIC without the security extensions, reading this register
>           * behaves in the same way as a secure access to a GIC with them.
>           */
> @@ -255,7 +256,7 @@ static int gic_get_group_priority(GICState *s, int cpu, int irq)
>  
>      if (gic_has_groups(s) &&
>          !(s->cpu_ctlr[cpu] & GICC_CTLR_CBPR) &&
> -        GIC_TEST_GROUP(irq, (1 << cpu))) {
> +        GIC_DIST_TEST_GROUP(irq, (1 << cpu))) {
>          bpr = s->abpr[cpu] - 1;
>          assert(bpr >= 0);
>      } else {
> @@ -268,7 +269,7 @@ static int gic_get_group_priority(GICState *s, int cpu, int irq)
>       */
>      mask = ~0U << ((bpr & 7) + 1);
>  
> -    return GIC_GET_PRIORITY(irq, cpu) & mask;
> +    return GIC_DIST_GET_PRIORITY(irq, cpu) & mask;
>  }
>  
>  static void gic_activate_irq(GICState *s, int cpu, int irq)
> @@ -281,14 +282,14 @@ static void gic_activate_irq(GICState *s, int cpu, int irq)
>      int regno = preemption_level / 32;
>      int bitno = preemption_level % 32;
>  
> -    if (gic_has_groups(s) && GIC_TEST_GROUP(irq, (1 << cpu))) {
> +    if (gic_has_groups(s) && GIC_DIST_TEST_GROUP(irq, (1 << cpu))) {
>          s->nsapr[regno][cpu] |= (1 << bitno);
>      } else {
>          s->apr[regno][cpu] |= (1 << bitno);
>      }
>  
>      s->running_priority[cpu] = prio;
> -    GIC_SET_ACTIVE(irq, 1 << cpu);
> +    GIC_DIST_SET_ACTIVE(irq, 1 << cpu);
>  }
>  
>  static int gic_get_prio_from_apr_bits(GICState *s, int cpu)
> @@ -357,7 +358,7 @@ uint32_t gic_acknowledge_irq(GICState *s, int cpu, MemTxAttrs attrs)
>          return irq;
>      }
>  
> -    if (GIC_GET_PRIORITY(irq, cpu) >= s->running_priority[cpu]) {
> +    if (GIC_DIST_GET_PRIORITY(irq, cpu) >= s->running_priority[cpu]) {
>          DPRINTF("ACK, pending interrupt (%d) has insufficient priority\n", irq);
>          return 1023;
>      }
> @@ -366,7 +367,8 @@ uint32_t gic_acknowledge_irq(GICState *s, int cpu, MemTxAttrs attrs)
>          /* Clear pending flags for both level and edge triggered interrupts.
>           * Level triggered IRQs will be reasserted once they become inactive.
>           */
> -        GIC_CLEAR_PENDING(irq, GIC_TEST_MODEL(irq) ? ALL_CPU_MASK : cm);
> +        GIC_DIST_CLEAR_PENDING(irq, GIC_DIST_TEST_MODEL(irq) ? ALL_CPU_MASK
> +                                                             : cm);
>          ret = irq;
>      } else {
>          if (irq < GIC_NR_SGIS) {
> @@ -378,7 +380,9 @@ uint32_t gic_acknowledge_irq(GICState *s, int cpu, MemTxAttrs attrs)
>              src = ctz32(s->sgi_pending[irq][cpu]);
>              s->sgi_pending[irq][cpu] &= ~(1 << src);
>              if (s->sgi_pending[irq][cpu] == 0) {
> -                GIC_CLEAR_PENDING(irq, GIC_TEST_MODEL(irq) ? ALL_CPU_MASK : cm);
> +                GIC_DIST_CLEAR_PENDING(irq,
> +                                       GIC_DIST_TEST_MODEL(irq) ? ALL_CPU_MASK
> +                                                                : cm);
>              }
>              ret = irq | ((src & 0x7) << 10);
>          } else {
> @@ -386,7 +390,8 @@ uint32_t gic_acknowledge_irq(GICState *s, int cpu, MemTxAttrs attrs)
>               * interrupts. (level triggered interrupts with an active line
>               * remain pending, see gic_test_pending)
>               */
> -            GIC_CLEAR_PENDING(irq, GIC_TEST_MODEL(irq) ? ALL_CPU_MASK : cm);
> +            GIC_DIST_CLEAR_PENDING(irq, GIC_DIST_TEST_MODEL(irq) ? ALL_CPU_MASK
> +                                                                 : cm);
>              ret = irq;
>          }
>      }
> @@ -397,11 +402,11 @@ uint32_t gic_acknowledge_irq(GICState *s, int cpu, MemTxAttrs attrs)
>      return ret;
>  }
>  
> -void gic_set_priority(GICState *s, int cpu, int irq, uint8_t val,
> +void gic_dist_set_priority(GICState *s, int cpu, int irq, uint8_t val,
>                        MemTxAttrs attrs)
>  {
>      if (s->security_extn && !attrs.secure) {
> -        if (!GIC_TEST_GROUP(irq, (1 << cpu))) {
> +        if (!GIC_DIST_TEST_GROUP(irq, (1 << cpu))) {
>              return; /* Ignore Non-secure access of Group0 IRQ */
>          }
>          val = 0x80 | (val >> 1); /* Non-secure view */
> @@ -414,13 +419,13 @@ void gic_set_priority(GICState *s, int cpu, int irq, uint8_t val,
>      }
>  }
>  
> -static uint32_t gic_get_priority(GICState *s, int cpu, int irq,
> +static uint32_t gic_dist_get_priority(GICState *s, int cpu, int irq,
>                                   MemTxAttrs attrs)
>  {
> -    uint32_t prio = GIC_GET_PRIORITY(irq, cpu);
> +    uint32_t prio = GIC_DIST_GET_PRIORITY(irq, cpu);
>  
>      if (s->security_extn && !attrs.secure) {
> -        if (!GIC_TEST_GROUP(irq, (1 << cpu))) {
> +        if (!GIC_DIST_TEST_GROUP(irq, (1 << cpu))) {
>              return 0; /* Non-secure access cannot read priority of Group0 IRQ */
>          }
>          prio = (prio << 1) & 0xff; /* Non-secure view */
> @@ -543,7 +548,7 @@ static bool gic_eoi_split(GICState *s, int cpu, MemTxAttrs attrs)
>  static void gic_deactivate_irq(GICState *s, int cpu, int irq, MemTxAttrs attrs)
>  {
>      int cm = 1 << cpu;
> -    int group = gic_has_groups(s) && GIC_TEST_GROUP(irq, cm);
> +    int group = gic_has_groups(s) && GIC_DIST_TEST_GROUP(irq, cm);
>  
>      if (!gic_eoi_split(s, cpu, attrs)) {
>          /* This is UNPREDICTABLE; we choose to ignore it */
> @@ -557,7 +562,7 @@ static void gic_deactivate_irq(GICState *s, int cpu, int irq, MemTxAttrs attrs)
>          return;
>      }
>  
> -    GIC_CLEAR_ACTIVE(irq, cm);
> +    GIC_DIST_CLEAR_ACTIVE(irq, cm);
>  }
>  
>  void gic_complete_irq(GICState *s, int cpu, int irq, MemTxAttrs attrs)
> @@ -584,14 +589,15 @@ void gic_complete_irq(GICState *s, int cpu, int irq, MemTxAttrs attrs)
>      if (s->revision == REV_11MPCORE) {
>          /* Mark level triggered interrupts as pending if they are still
>             raised.  */
> -        if (!GIC_TEST_EDGE_TRIGGER(irq) && GIC_TEST_ENABLED(irq, cm)
> -            && GIC_TEST_LEVEL(irq, cm) && (GIC_TARGET(irq) & cm) != 0) {
> +        if (!GIC_DIST_TEST_EDGE_TRIGGER(irq) && GIC_DIST_TEST_ENABLED(irq, cm)
> +            && GIC_DIST_TEST_LEVEL(irq, cm)
> +            && (GIC_DIST_TARGET(irq) & cm) != 0) {
>              DPRINTF("Set %d pending mask %x\n", irq, cm);
> -            GIC_SET_PENDING(irq, cm);
> +            GIC_DIST_SET_PENDING(irq, cm);
>          }
>      }
>  
> -    group = gic_has_groups(s) && GIC_TEST_GROUP(irq, cm);
> +    group = gic_has_groups(s) && GIC_DIST_TEST_GROUP(irq, cm);
>  
>      if (s->security_extn && !attrs.secure && !group) {
>          DPRINTF("Non-secure EOI for Group0 interrupt %d ignored\n", irq);
> @@ -607,7 +613,7 @@ void gic_complete_irq(GICState *s, int cpu, int irq, MemTxAttrs attrs)
>  
>      /* In GICv2 the guest can choose to split priority-drop and deactivate */
>      if (!gic_eoi_split(s, cpu, attrs)) {
> -        GIC_CLEAR_ACTIVE(irq, cm);
> +        GIC_DIST_CLEAR_ACTIVE(irq, cm);
>      }
>      gic_update(s);
>  }
> @@ -655,7 +661,7 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr offset, MemTxAttrs attrs)
>                      goto bad_reg;
>                  }
>                  for (i = 0; i < 8; i++) {
> -                    if (GIC_TEST_GROUP(irq + i, cm)) {
> +                    if (GIC_DIST_TEST_GROUP(irq + i, cm)) {
>                          res |= (1 << i);
>                      }
>                  }
> @@ -675,11 +681,11 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr offset, MemTxAttrs attrs)
>          res = 0;
>          for (i = 0; i < 8; i++) {
>              if (s->security_extn && !attrs.secure &&
> -                !GIC_TEST_GROUP(irq + i, 1 << cpu)) {
> +                !GIC_DIST_TEST_GROUP(irq + i, 1 << cpu)) {
>                  continue; /* Ignore Non-secure access of Group0 IRQ */
>              }
>  
> -            if (GIC_TEST_ENABLED(irq + i, cm)) {
> +            if (GIC_DIST_TEST_ENABLED(irq + i, cm)) {
>                  res |= (1 << i);
>              }
>          }
> @@ -696,7 +702,7 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr offset, MemTxAttrs attrs)
>          mask = (irq < GIC_INTERNAL) ?  cm : ALL_CPU_MASK;
>          for (i = 0; i < 8; i++) {
>              if (s->security_extn && !attrs.secure &&
> -                !GIC_TEST_GROUP(irq + i, 1 << cpu)) {
> +                !GIC_DIST_TEST_GROUP(irq + i, 1 << cpu)) {
>                  continue; /* Ignore Non-secure access of Group0 IRQ */
>              }
>  
> @@ -713,11 +719,11 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr offset, MemTxAttrs attrs)
>          mask = (irq < GIC_INTERNAL) ?  cm : ALL_CPU_MASK;
>          for (i = 0; i < 8; i++) {
>              if (s->security_extn && !attrs.secure &&
> -                !GIC_TEST_GROUP(irq + i, 1 << cpu)) {
> +                !GIC_DIST_TEST_GROUP(irq + i, 1 << cpu)) {
>                  continue; /* Ignore Non-secure access of Group0 IRQ */
>              }
>  
> -            if (GIC_TEST_ACTIVE(irq + i, mask)) {
> +            if (GIC_DIST_TEST_ACTIVE(irq + i, mask)) {
>                  res |= (1 << i);
>              }
>          }
> @@ -726,7 +732,7 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr offset, MemTxAttrs attrs)
>          irq = (offset - 0x400) + GIC_BASE_IRQ;
>          if (irq >= s->num_irq)
>              goto bad_reg;
> -        res = gic_get_priority(s, cpu, irq, attrs);
> +        res = gic_dist_get_priority(s, cpu, irq, attrs);
>      } else if (offset < 0xc00) {
>          /* Interrupt CPU Target.  */
>          if (s->num_cpu == 1 && s->revision != REV_11MPCORE) {
> @@ -740,7 +746,7 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr offset, MemTxAttrs attrs)
>              if (irq >= 29 && irq <= 31) {
>                  res = cm;
>              } else {
> -                res = GIC_TARGET(irq);
> +                res = GIC_DIST_TARGET(irq);
>              }
>          }
>      } else if (offset < 0xf00) {
> @@ -751,14 +757,16 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr offset, MemTxAttrs attrs)
>          res = 0;
>          for (i = 0; i < 4; i++) {
>              if (s->security_extn && !attrs.secure &&
> -                !GIC_TEST_GROUP(irq + i, 1 << cpu)) {
> +                !GIC_DIST_TEST_GROUP(irq + i, 1 << cpu)) {
>                  continue; /* Ignore Non-secure access of Group0 IRQ */
>              }
>  
> -            if (GIC_TEST_MODEL(irq + i))
> +            if (GIC_DIST_TEST_MODEL(irq + i)) {
>                  res |= (1 << (i * 2));
> -            if (GIC_TEST_EDGE_TRIGGER(irq + i))
> +            }
> +            if (GIC_DIST_TEST_EDGE_TRIGGER(irq + i)) {
>                  res |= (2 << (i * 2));
> +            }
>          }
>      } else if (offset < 0xf10) {
>          goto bad_reg;
> @@ -776,7 +784,7 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr offset, MemTxAttrs attrs)
>          }
>  
>          if (s->security_extn && !attrs.secure &&
> -            !GIC_TEST_GROUP(irq, 1 << cpu)) {
> +            !GIC_DIST_TEST_GROUP(irq, 1 << cpu)) {
>              res = 0; /* Ignore Non-secure access of Group0 IRQ */
>          } else {
>              res = s->sgi_pending[irq][cpu];
> @@ -872,10 +880,10 @@ static void gic_dist_writeb(void *opaque, hwaddr offset,
>                      int cm = (irq < GIC_INTERNAL) ? (1 << cpu) : ALL_CPU_MASK;
>                      if (value & (1 << i)) {
>                          /* Group1 (Non-secure) */
> -                        GIC_SET_GROUP(irq + i, cm);
> +                        GIC_DIST_SET_GROUP(irq + i, cm);
>                      } else {
>                          /* Group0 (Secure) */
> -                        GIC_CLEAR_GROUP(irq + i, cm);
> +                        GIC_DIST_CLEAR_GROUP(irq + i, cm);
>                      }
>                  }
>              }
> @@ -893,26 +901,26 @@ static void gic_dist_writeb(void *opaque, hwaddr offset,
>  
>          for (i = 0; i < 8; i++) {
>              if (value & (1 << i)) {
> -                int mask =
> -                    (irq < GIC_INTERNAL) ? (1 << cpu) : GIC_TARGET(irq + i);
> +                int mask = (irq < GIC_INTERNAL) ? (1 << cpu)
> +                                                : GIC_DIST_TARGET(irq + i);
>                  int cm = (irq < GIC_INTERNAL) ? (1 << cpu) : ALL_CPU_MASK;
>  
>                  if (s->security_extn && !attrs.secure &&
> -                    !GIC_TEST_GROUP(irq + i, 1 << cpu)) {
> +                    !GIC_DIST_TEST_GROUP(irq + i, 1 << cpu)) {
>                      continue; /* Ignore Non-secure access of Group0 IRQ */
>                  }
>  
> -                if (!GIC_TEST_ENABLED(irq + i, cm)) {
> +                if (!GIC_DIST_TEST_ENABLED(irq + i, cm)) {
>                      DPRINTF("Enabled IRQ %d\n", irq + i);
>                      trace_gic_enable_irq(irq + i);
>                  }
> -                GIC_SET_ENABLED(irq + i, cm);
> +                GIC_DIST_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)) {
> +                if (GIC_DIST_TEST_LEVEL(irq + i, mask)
> +                        && !GIC_DIST_TEST_EDGE_TRIGGER(irq + i)) {
>                      DPRINTF("Set %d pending mask %x\n", irq + i, mask);
> -                    GIC_SET_PENDING(irq + i, mask);
> +                    GIC_DIST_SET_PENDING(irq + i, mask);
>                  }
>              }
>          }
> @@ -930,15 +938,15 @@ static void gic_dist_writeb(void *opaque, hwaddr offset,
>                  int cm = (irq < GIC_INTERNAL) ? (1 << cpu) : ALL_CPU_MASK;
>  
>                  if (s->security_extn && !attrs.secure &&
> -                    !GIC_TEST_GROUP(irq + i, 1 << cpu)) {
> +                    !GIC_DIST_TEST_GROUP(irq + i, 1 << cpu)) {
>                      continue; /* Ignore Non-secure access of Group0 IRQ */
>                  }
>  
> -                if (GIC_TEST_ENABLED(irq + i, cm)) {
> +                if (GIC_DIST_TEST_ENABLED(irq + i, cm)) {
>                      DPRINTF("Disabled IRQ %d\n", irq + i);
>                      trace_gic_disable_irq(irq + i);
>                  }
> -                GIC_CLEAR_ENABLED(irq + i, cm);
> +                GIC_DIST_CLEAR_ENABLED(irq + i, cm);
>              }
>          }
>      } else if (offset < 0x280) {
> @@ -953,11 +961,11 @@ static void gic_dist_writeb(void *opaque, hwaddr offset,
>          for (i = 0; i < 8; i++) {
>              if (value & (1 << i)) {
>                  if (s->security_extn && !attrs.secure &&
> -                    !GIC_TEST_GROUP(irq + i, 1 << cpu)) {
> +                    !GIC_DIST_TEST_GROUP(irq + i, 1 << cpu)) {
>                      continue; /* Ignore Non-secure access of Group0 IRQ */
>                  }
>  
> -                GIC_SET_PENDING(irq + i, GIC_TARGET(irq + i));
> +                GIC_DIST_SET_PENDING(irq + i, GIC_DIST_TARGET(irq + i));
>              }
>          }
>      } else if (offset < 0x300) {
> @@ -971,7 +979,7 @@ static void gic_dist_writeb(void *opaque, hwaddr offset,
>  
>          for (i = 0; i < 8; i++) {
>              if (s->security_extn && !attrs.secure &&
> -                !GIC_TEST_GROUP(irq + i, 1 << cpu)) {
> +                !GIC_DIST_TEST_GROUP(irq + i, 1 << cpu)) {
>                  continue; /* Ignore Non-secure access of Group0 IRQ */
>              }
>  
> @@ -979,7 +987,7 @@ static void gic_dist_writeb(void *opaque, hwaddr offset,
>                 for per-CPU interrupts.  It's unclear whether this is the
>                 corect behavior.  */
>              if (value & (1 << i)) {
> -                GIC_CLEAR_PENDING(irq + i, ALL_CPU_MASK);
> +                GIC_DIST_CLEAR_PENDING(irq + i, ALL_CPU_MASK);
>              }
>          }
>      } else if (offset < 0x400) {
> @@ -990,7 +998,7 @@ static void gic_dist_writeb(void *opaque, hwaddr offset,
>          irq = (offset - 0x400) + GIC_BASE_IRQ;
>          if (irq >= s->num_irq)
>              goto bad_reg;
> -        gic_set_priority(s, cpu, irq, value, attrs);
> +        gic_dist_set_priority(s, cpu, irq, value, attrs);
>      } else if (offset < 0xc00) {
>          /* Interrupt CPU Target. RAZ/WI on uniprocessor GICs, with the
>           * annoying exception of the 11MPCore's GIC.
> @@ -1016,21 +1024,21 @@ static void gic_dist_writeb(void *opaque, hwaddr offset,
>              value |= 0xaa;
>          for (i = 0; i < 4; i++) {
>              if (s->security_extn && !attrs.secure &&
> -                !GIC_TEST_GROUP(irq + i, 1 << cpu)) {
> +                !GIC_DIST_TEST_GROUP(irq + i, 1 << cpu)) {
>                  continue; /* Ignore Non-secure access of Group0 IRQ */
>              }
>  
>              if (s->revision == REV_11MPCORE) {
>                  if (value & (1 << (i * 2))) {
> -                    GIC_SET_MODEL(irq + i);
> +                    GIC_DIST_SET_MODEL(irq + i);
>                  } else {
> -                    GIC_CLEAR_MODEL(irq + i);
> +                    GIC_DIST_CLEAR_MODEL(irq + i);
>                  }
>              }
>              if (value & (2 << (i * 2))) {
> -                GIC_SET_EDGE_TRIGGER(irq + i);
> +                GIC_DIST_SET_EDGE_TRIGGER(irq + i);
>              } else {
> -                GIC_CLEAR_EDGE_TRIGGER(irq + i);
> +                GIC_DIST_CLEAR_EDGE_TRIGGER(irq + i);
>              }
>          }
>      } else if (offset < 0xf10) {
> @@ -1044,10 +1052,10 @@ static void gic_dist_writeb(void *opaque, hwaddr offset,
>          irq = (offset - 0xf10);
>  
>          if (!s->security_extn || attrs.secure ||
> -            GIC_TEST_GROUP(irq, 1 << cpu)) {
> +            GIC_DIST_TEST_GROUP(irq, 1 << cpu)) {
>              s->sgi_pending[irq][cpu] &= ~value;
>              if (s->sgi_pending[irq][cpu] == 0) {
> -                GIC_CLEAR_PENDING(irq, 1 << cpu);
> +                GIC_DIST_CLEAR_PENDING(irq, 1 << cpu);
>              }
>          }
>      } else if (offset < 0xf30) {
> @@ -1058,8 +1066,8 @@ static void gic_dist_writeb(void *opaque, hwaddr offset,
>          irq = (offset - 0xf20);
>  
>          if (!s->security_extn || attrs.secure ||
> -            GIC_TEST_GROUP(irq, 1 << cpu)) {
> -            GIC_SET_PENDING(irq, 1 << cpu);
> +            GIC_DIST_TEST_GROUP(irq, 1 << cpu)) {
> +            GIC_DIST_SET_PENDING(irq, 1 << cpu);
>              s->sgi_pending[irq][cpu] |= value;
>          }
>      } else {
> @@ -1106,7 +1114,7 @@ static void gic_dist_writel(void *opaque, hwaddr offset,
>              mask = ALL_CPU_MASK;
>              break;
>          }
> -        GIC_SET_PENDING(irq, mask);
> +        GIC_DIST_SET_PENDING(irq, mask);
>          target_cpu = ctz32(mask);
>          while (target_cpu < GIC_NCPU) {
>              s->sgi_pending[irq][target_cpu] |= (1 << cpu);
> diff --git a/hw/intc/arm_gic_common.c b/hw/intc/arm_gic_common.c
> index aee50a20e0..295ee9cc5e 100644
> --- a/hw/intc/arm_gic_common.c
> +++ b/hw/intc/arm_gic_common.c
> @@ -204,8 +204,8 @@ static void arm_gic_common_reset(DeviceState *dev)
>          }
>      }
>      for (i = 0; i < GIC_NR_SGIS; i++) {
> -        GIC_SET_ENABLED(i, ALL_CPU_MASK);
> -        GIC_SET_EDGE_TRIGGER(i);
> +        GIC_DIST_SET_ENABLED(i, ALL_CPU_MASK);
> +        GIC_DIST_SET_EDGE_TRIGGER(i);
>      }
>  
>      for (i = 0; i < ARRAY_SIZE(s->priority2); i++) {
> @@ -222,7 +222,7 @@ static void arm_gic_common_reset(DeviceState *dev)
>      }
>      if (s->security_extn && s->irq_reset_nonsecure) {
>          for (i = 0; i < GIC_MAXIRQ; i++) {
> -            GIC_SET_GROUP(i, ALL_CPU_MASK);
> +            GIC_DIST_SET_GROUP(i, ALL_CPU_MASK);
>          }
>      }
>  
> diff --git a/hw/intc/arm_gic_kvm.c b/hw/intc/arm_gic_kvm.c
> index 204369d0e2..799136732a 100644
> --- a/hw/intc/arm_gic_kvm.c
> +++ b/hw/intc/arm_gic_kvm.c
> @@ -140,10 +140,10 @@ static void translate_group(GICState *s, int irq, int cpu,
>      int cm = (irq < GIC_INTERNAL) ? (1 << cpu) : ALL_CPU_MASK;
>  
>      if (to_kernel) {
> -        *field = GIC_TEST_GROUP(irq, cm);
> +        *field = GIC_DIST_TEST_GROUP(irq, cm);
>      } else {
>          if (*field & 1) {
> -            GIC_SET_GROUP(irq, cm);
> +            GIC_DIST_SET_GROUP(irq, cm);
>          }
>      }
>  }
> @@ -154,10 +154,10 @@ static void translate_enabled(GICState *s, int irq, int cpu,
>      int cm = (irq < GIC_INTERNAL) ? (1 << cpu) : ALL_CPU_MASK;
>  
>      if (to_kernel) {
> -        *field = GIC_TEST_ENABLED(irq, cm);
> +        *field = GIC_DIST_TEST_ENABLED(irq, cm);
>      } else {
>          if (*field & 1) {
> -            GIC_SET_ENABLED(irq, cm);
> +            GIC_DIST_SET_ENABLED(irq, cm);
>          }
>      }
>  }
> @@ -171,7 +171,7 @@ static void translate_pending(GICState *s, int irq, int cpu,
>          *field = gic_test_pending(s, irq, cm);
>      } else {
>          if (*field & 1) {
> -            GIC_SET_PENDING(irq, cm);
> +            GIC_DIST_SET_PENDING(irq, cm);
>              /* TODO: Capture is level-line is held high in the kernel */
>          }
>      }
> @@ -183,10 +183,10 @@ static void translate_active(GICState *s, int irq, int cpu,
>      int cm = (irq < GIC_INTERNAL) ? (1 << cpu) : ALL_CPU_MASK;
>  
>      if (to_kernel) {
> -        *field = GIC_TEST_ACTIVE(irq, cm);
> +        *field = GIC_DIST_TEST_ACTIVE(irq, cm);
>      } else {
>          if (*field & 1) {
> -            GIC_SET_ACTIVE(irq, cm);
> +            GIC_DIST_SET_ACTIVE(irq, cm);
>          }
>      }
>  }
> @@ -195,10 +195,10 @@ static void translate_trigger(GICState *s, int irq, int cpu,
>                                uint32_t *field, bool to_kernel)
>  {
>      if (to_kernel) {
> -        *field = (GIC_TEST_EDGE_TRIGGER(irq)) ? 0x2 : 0x0;
> +        *field = (GIC_DIST_TEST_EDGE_TRIGGER(irq)) ? 0x2 : 0x0;
>      } else {
>          if (*field & 0x2) {
> -            GIC_SET_EDGE_TRIGGER(irq);
> +            GIC_DIST_SET_EDGE_TRIGGER(irq);
>          }
>      }
>  }
> @@ -207,9 +207,10 @@ static void translate_priority(GICState *s, int irq, int cpu,
>                                 uint32_t *field, bool to_kernel)
>  {
>      if (to_kernel) {
> -        *field = GIC_GET_PRIORITY(irq, cpu) & 0xff;
> +        *field = GIC_DIST_GET_PRIORITY(irq, cpu) & 0xff;
>      } else {
> -        gic_set_priority(s, cpu, irq, *field & 0xff, MEMTXATTRS_UNSPECIFIED);
> +        gic_dist_set_priority(s, cpu, irq,
> +                              *field & 0xff, MEMTXATTRS_UNSPECIFIED);
>      }
>  }
>  
> diff --git a/hw/intc/gic_internal.h b/hw/intc/gic_internal.h
> index 7fe87b13de..6f8d242904 100644
> --- a/hw/intc/gic_internal.h
> +++ b/hw/intc/gic_internal.h
> @@ -27,30 +27,31 @@
>  
>  #define GIC_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_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_MODEL(irq) s->irq_state[irq].model = true
> -#define GIC_CLEAR_MODEL(irq) s->irq_state[irq].model = false
> -#define GIC_TEST_MODEL(irq) s->irq_state[irq].model
> -#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) < GIC_INTERNAL) ?            \
> +#define GIC_DIST_SET_ENABLED(irq, cm) (s->irq_state[irq].enabled |= (cm))
> +#define GIC_DIST_CLEAR_ENABLED(irq, cm) (s->irq_state[irq].enabled &= ~(cm))
> +#define GIC_DIST_TEST_ENABLED(irq, cm) ((s->irq_state[irq].enabled & (cm)) != 0)
> +#define GIC_DIST_SET_PENDING(irq, cm) (s->irq_state[irq].pending |= (cm))
> +#define GIC_DIST_CLEAR_PENDING(irq, cm) (s->irq_state[irq].pending &= ~(cm))
> +#define GIC_DIST_SET_ACTIVE(irq, cm) (s->irq_state[irq].active |= (cm))
> +#define GIC_DIST_CLEAR_ACTIVE(irq, cm) (s->irq_state[irq].active &= ~(cm))
> +#define GIC_DIST_TEST_ACTIVE(irq, cm) ((s->irq_state[irq].active & (cm)) != 0)
> +#define GIC_DIST_SET_MODEL(irq) (s->irq_state[irq].model = true)
> +#define GIC_DIST_CLEAR_MODEL(irq) (s->irq_state[irq].model = false)
> +#define GIC_DIST_TEST_MODEL(irq) (s->irq_state[irq].model)
> +#define GIC_DIST_SET_LEVEL(irq, cm) (s->irq_state[irq].level |= (cm))
> +#define GIC_DIST_CLEAR_LEVEL(irq, cm) (s->irq_state[irq].level &= ~(cm))
> +#define GIC_DIST_TEST_LEVEL(irq, cm) ((s->irq_state[irq].level & (cm)) != 0)
> +#define GIC_DIST_SET_EDGE_TRIGGER(irq) (s->irq_state[irq].edge_trigger = true)
> +#define GIC_DIST_CLEAR_EDGE_TRIGGER(irq) \
> +    (s->irq_state[irq].edge_trigger = false)
> +#define GIC_DIST_TEST_EDGE_TRIGGER(irq) (s->irq_state[irq].edge_trigger)
> +#define GIC_DIST_GET_PRIORITY(irq, cpu) (((irq) < GIC_INTERNAL) ?            \
>                                      s->priority1[irq][cpu] :            \
>                                      s->priority2[(irq) - GIC_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)
> +#define GIC_DIST_TARGET(irq) (s->irq_target[irq])
> +#define GIC_DIST_CLEAR_GROUP(irq, cm) (s->irq_state[irq].group &= ~(cm))
> +#define GIC_DIST_SET_GROUP(irq, cm) (s->irq_state[irq].group |= (cm))
> +#define GIC_DIST_TEST_GROUP(irq, cm) ((s->irq_state[irq].group & (cm)) != 0)
>  
>  #define GICD_CTLR_EN_GRP0 (1U << 0)
>  #define GICD_CTLR_EN_GRP1 (1U << 1)
> @@ -79,8 +80,8 @@ uint32_t gic_acknowledge_irq(GICState *s, int cpu, MemTxAttrs attrs);
>  void gic_complete_irq(GICState *s, int cpu, int irq, MemTxAttrs attrs);
>  void gic_update(GICState *s);
>  void gic_init_irqs_and_distributor(GICState *s);
> -void gic_set_priority(GICState *s, int cpu, int irq, uint8_t val,
> -                      MemTxAttrs attrs);
> +void gic_dist_set_priority(GICState *s, int cpu, int irq, uint8_t val,
> +                           MemTxAttrs attrs);
>  
>  static inline bool gic_test_pending(GICState *s, int irq, int cm)
>  {
> @@ -93,7 +94,7 @@ static inline bool gic_test_pending(GICState *s, int irq, int cm)
>           * GICD_ISPENDR to set the state pending.
>           */
>          return (s->irq_state[irq].pending & cm) ||
> -            (!GIC_TEST_EDGE_TRIGGER(irq) && GIC_TEST_LEVEL(irq, cm));
> +            (!GIC_DIST_TEST_EDGE_TRIGGER(irq) && GIC_DIST_TEST_LEVEL(irq, cm));
>      }
>  }
>  
> 

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

* Re: [Qemu-devel] [PATCH 1/6] intc/arm_gic: Refactor operations on the distributor
  2018-06-06  9:30 ` [Qemu-devel] [PATCH 1/6] intc/arm_gic: Refactor operations on the distributor luc.michel
  2018-06-06 13:38   ` [Qemu-devel] [Qemu-arm] " Philippe Mathieu-Daudé
@ 2018-06-07  6:08   ` Sai Pavan Boddu
  2018-06-11 13:39   ` Peter Maydell
  2 siblings, 0 replies; 17+ messages in thread
From: Sai Pavan Boddu @ 2018-06-07  6:08 UTC (permalink / raw)
  To: luc.michel@greensocs.com, qemu-devel@nongnu.org
  Cc: qemu-arm@nongnu.org, Peter Maydell, Edgar Iglesias,
	mark.burton@greensocs.com, Jan Kiszka



> -----Original Message-----
> From: luc.michel@greensocs.com [mailto:luc.michel@greensocs.com]
> Sent: Wednesday, June 6, 2018 3:01 PM
> To: qemu-devel@nongnu.org
> Cc: Luc MICHEL <luc.michel@greensocs.com>; qemu-arm@nongnu.org; Peter
> Maydell <peter.maydell@linaro.org>; Sai Pavan Boddu <saipava@xilinx.com>;
> Edgar Iglesias <edgari@xilinx.com>; mark.burton@greensocs.com; Jan Kiszka
> <jan.kiszka@web.de>
> Subject: [PATCH 1/6] intc/arm_gic: Refactor operations on the distributor
> 
> From: Luc MICHEL <luc.michel@greensocs.com>

Reviewed-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>

Thanks,
Sai Pavan
> 
> In preparation for the virtualization extensions implementation, refactor the
> name of the functions and macros that act on the GIC distributor to make that
> fact explicit. It will be useful to differentiate them from the ones that will act on
> the virtual interfaces.
> 
> Signed-off-by: Luc MICHEL <luc.michel@greensocs.com>
> ---
>  hw/intc/arm_gic.c        | 164 ++++++++++++++++++++-------------------
>  hw/intc/arm_gic_common.c |   6 +-
>  hw/intc/arm_gic_kvm.c    |  23 +++---
>  hw/intc/gic_internal.h   |  51 ++++++------
>  4 files changed, 127 insertions(+), 117 deletions(-)
> 
> diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c index
> ea0323f969..141f3e7a48 100644
> --- a/hw/intc/arm_gic.c
> +++ b/hw/intc/arm_gic.c
> @@ -92,11 +92,12 @@ void gic_update(GICState *s)
>          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) &&
> -                (!GIC_TEST_ACTIVE(irq, cm)) &&
> -                (irq < GIC_INTERNAL || GIC_TARGET(irq) & cm)) {
> -                if (GIC_GET_PRIORITY(irq, cpu) < best_prio) {
> -                    best_prio = GIC_GET_PRIORITY(irq, cpu);
> +            if (GIC_DIST_TEST_ENABLED(irq, cm) &&
> +                gic_test_pending(s, irq, cm) &&
> +                (!GIC_DIST_TEST_ACTIVE(irq, cm)) &&
> +                (irq < GIC_INTERNAL || GIC_DIST_TARGET(irq) & cm)) {
> +                if (GIC_DIST_GET_PRIORITY(irq, cpu) < best_prio) {
> +                    best_prio = GIC_DIST_GET_PRIORITY(irq, cpu);
>                      best_irq = irq;
>                  }
>              }
> @@ -112,7 +113,7 @@ void gic_update(GICState *s)
>          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);
> +                int group = GIC_DIST_TEST_GROUP(best_irq, cm);
> 
>                  if (extract32(s->ctlr, group, 1) &&
>                      extract32(s->cpu_ctlr[cpu], group, 1)) { @@ -145,7 +146,7 @@ void
> gic_set_pending_private(GICState *s, int cpu, int irq)
>      }
> 
>      DPRINTF("Set %d pending cpu %d\n", irq, cpu);
> -    GIC_SET_PENDING(irq, cm);
> +    GIC_DIST_SET_PENDING(irq, cm);
>      gic_update(s);
>  }
> 
> @@ -153,13 +154,13 @@ static void gic_set_irq_11mpcore(GICState *s, int irq,
> int level,
>                                   int cm, int target)  {
>      if (level) {
> -        GIC_SET_LEVEL(irq, cm);
> -        if (GIC_TEST_EDGE_TRIGGER(irq) || GIC_TEST_ENABLED(irq, cm)) {
> +        GIC_DIST_SET_LEVEL(irq, cm);
> +        if (GIC_DIST_TEST_EDGE_TRIGGER(irq) ||
> + GIC_DIST_TEST_ENABLED(irq, cm)) {
>              DPRINTF("Set %d pending mask %x\n", irq, target);
> -            GIC_SET_PENDING(irq, target);
> +            GIC_DIST_SET_PENDING(irq, target);
>          }
>      } else {
> -        GIC_CLEAR_LEVEL(irq, cm);
> +        GIC_DIST_CLEAR_LEVEL(irq, cm);
>      }
>  }
> 
> @@ -167,13 +168,13 @@ static void gic_set_irq_generic(GICState *s, int irq, int
> level,
>                                  int cm, int target)  {
>      if (level) {
> -        GIC_SET_LEVEL(irq, cm);
> +        GIC_DIST_SET_LEVEL(irq, cm);
>          DPRINTF("Set %d pending mask %x\n", irq, target);
> -        if (GIC_TEST_EDGE_TRIGGER(irq)) {
> -            GIC_SET_PENDING(irq, target);
> +        if (GIC_DIST_TEST_EDGE_TRIGGER(irq)) {
> +            GIC_DIST_SET_PENDING(irq, target);
>          }
>      } else {
> -        GIC_CLEAR_LEVEL(irq, cm);
> +        GIC_DIST_CLEAR_LEVEL(irq, cm);
>      }
>  }
> 
> @@ -192,7 +193,7 @@ static void gic_set_irq(void *opaque, int irq, int level)
>          /* The first external input line is internal interrupt 32.  */
>          cm = ALL_CPU_MASK;
>          irq += GIC_INTERNAL;
> -        target = GIC_TARGET(irq);
> +        target = GIC_DIST_TARGET(irq);
>      } else {
>          int cpu;
>          irq -= (s->num_irq - GIC_INTERNAL); @@ -204,7 +205,7 @@ static void
> gic_set_irq(void *opaque, int irq, int level)
> 
>      assert(irq >= GIC_NR_SGIS);
> 
> -    if (level == GIC_TEST_LEVEL(irq, cm)) {
> +    if (level == GIC_DIST_TEST_LEVEL(irq, cm)) {
>          return;
>      }
> 
> @@ -224,7 +225,7 @@ static uint16_t gic_get_current_pending_irq(GICState
> *s, int cpu,
>      uint16_t pending_irq = s->current_pending[cpu];
> 
>      if (pending_irq < GIC_MAXIRQ && gic_has_groups(s)) {
> -        int group = GIC_TEST_GROUP(pending_irq, (1 << cpu));
> +        int group = GIC_DIST_TEST_GROUP(pending_irq, (1 << cpu));
>          /* On a GIC without the security extensions, reading this register
>           * behaves in the same way as a secure access to a GIC with them.
>           */
> @@ -255,7 +256,7 @@ static int gic_get_group_priority(GICState *s, int cpu, int
> irq)
> 
>      if (gic_has_groups(s) &&
>          !(s->cpu_ctlr[cpu] & GICC_CTLR_CBPR) &&
> -        GIC_TEST_GROUP(irq, (1 << cpu))) {
> +        GIC_DIST_TEST_GROUP(irq, (1 << cpu))) {
>          bpr = s->abpr[cpu] - 1;
>          assert(bpr >= 0);
>      } else {
> @@ -268,7 +269,7 @@ static int gic_get_group_priority(GICState *s, int cpu, int
> irq)
>       */
>      mask = ~0U << ((bpr & 7) + 1);
> 
> -    return GIC_GET_PRIORITY(irq, cpu) & mask;
> +    return GIC_DIST_GET_PRIORITY(irq, cpu) & mask;
>  }
> 
>  static void gic_activate_irq(GICState *s, int cpu, int irq) @@ -281,14 +282,14
> @@ static void gic_activate_irq(GICState *s, int cpu, int irq)
>      int regno = preemption_level / 32;
>      int bitno = preemption_level % 32;
> 
> -    if (gic_has_groups(s) && GIC_TEST_GROUP(irq, (1 << cpu))) {
> +    if (gic_has_groups(s) && GIC_DIST_TEST_GROUP(irq, (1 << cpu))) {
>          s->nsapr[regno][cpu] |= (1 << bitno);
>      } else {
>          s->apr[regno][cpu] |= (1 << bitno);
>      }
> 
>      s->running_priority[cpu] = prio;
> -    GIC_SET_ACTIVE(irq, 1 << cpu);
> +    GIC_DIST_SET_ACTIVE(irq, 1 << cpu);
>  }
> 
>  static int gic_get_prio_from_apr_bits(GICState *s, int cpu) @@ -357,7 +358,7
> @@ uint32_t gic_acknowledge_irq(GICState *s, int cpu, MemTxAttrs attrs)
>          return irq;
>      }
> 
> -    if (GIC_GET_PRIORITY(irq, cpu) >= s->running_priority[cpu]) {
> +    if (GIC_DIST_GET_PRIORITY(irq, cpu) >= s->running_priority[cpu]) {
>          DPRINTF("ACK, pending interrupt (%d) has insufficient priority\n", irq);
>          return 1023;
>      }
> @@ -366,7 +367,8 @@ uint32_t gic_acknowledge_irq(GICState *s, int cpu,
> MemTxAttrs attrs)
>          /* Clear pending flags for both level and edge triggered interrupts.
>           * Level triggered IRQs will be reasserted once they become inactive.
>           */
> -        GIC_CLEAR_PENDING(irq, GIC_TEST_MODEL(irq) ? ALL_CPU_MASK : cm);
> +        GIC_DIST_CLEAR_PENDING(irq, GIC_DIST_TEST_MODEL(irq) ?
> ALL_CPU_MASK
> +                                                             : cm);
>          ret = irq;
>      } else {
>          if (irq < GIC_NR_SGIS) {
> @@ -378,7 +380,9 @@ uint32_t gic_acknowledge_irq(GICState *s, int cpu,
> MemTxAttrs attrs)
>              src = ctz32(s->sgi_pending[irq][cpu]);
>              s->sgi_pending[irq][cpu] &= ~(1 << src);
>              if (s->sgi_pending[irq][cpu] == 0) {
> -                GIC_CLEAR_PENDING(irq, GIC_TEST_MODEL(irq) ? ALL_CPU_MASK :
> cm);
> +                GIC_DIST_CLEAR_PENDING(irq,
> +                                       GIC_DIST_TEST_MODEL(irq) ? ALL_CPU_MASK
> +                                                                : cm);
>              }
>              ret = irq | ((src & 0x7) << 10);
>          } else {
> @@ -386,7 +390,8 @@ uint32_t gic_acknowledge_irq(GICState *s, int cpu,
> MemTxAttrs attrs)
>               * interrupts. (level triggered interrupts with an active line
>               * remain pending, see gic_test_pending)
>               */
> -            GIC_CLEAR_PENDING(irq, GIC_TEST_MODEL(irq) ? ALL_CPU_MASK : cm);
> +            GIC_DIST_CLEAR_PENDING(irq, GIC_DIST_TEST_MODEL(irq) ?
> ALL_CPU_MASK
> +                                                                 : cm);
>              ret = irq;
>          }
>      }
> @@ -397,11 +402,11 @@ uint32_t gic_acknowledge_irq(GICState *s, int cpu,
> MemTxAttrs attrs)
>      return ret;
>  }
> 
> -void gic_set_priority(GICState *s, int cpu, int irq, uint8_t val,
> +void gic_dist_set_priority(GICState *s, int cpu, int irq, uint8_t val,
>                        MemTxAttrs attrs)  {
>      if (s->security_extn && !attrs.secure) {
> -        if (!GIC_TEST_GROUP(irq, (1 << cpu))) {
> +        if (!GIC_DIST_TEST_GROUP(irq, (1 << cpu))) {
>              return; /* Ignore Non-secure access of Group0 IRQ */
>          }
>          val = 0x80 | (val >> 1); /* Non-secure view */ @@ -414,13 +419,13 @@
> void gic_set_priority(GICState *s, int cpu, int irq, uint8_t val,
>      }
>  }
> 
> -static uint32_t gic_get_priority(GICState *s, int cpu, int irq,
> +static uint32_t gic_dist_get_priority(GICState *s, int cpu, int irq,
>                                   MemTxAttrs attrs)  {
> -    uint32_t prio = GIC_GET_PRIORITY(irq, cpu);
> +    uint32_t prio = GIC_DIST_GET_PRIORITY(irq, cpu);
> 
>      if (s->security_extn && !attrs.secure) {
> -        if (!GIC_TEST_GROUP(irq, (1 << cpu))) {
> +        if (!GIC_DIST_TEST_GROUP(irq, (1 << cpu))) {
>              return 0; /* Non-secure access cannot read priority of Group0 IRQ */
>          }
>          prio = (prio << 1) & 0xff; /* Non-secure view */ @@ -543,7 +548,7 @@
> static bool gic_eoi_split(GICState *s, int cpu, MemTxAttrs attrs)  static void
> gic_deactivate_irq(GICState *s, int cpu, int irq, MemTxAttrs attrs)  {
>      int cm = 1 << cpu;
> -    int group = gic_has_groups(s) && GIC_TEST_GROUP(irq, cm);
> +    int group = gic_has_groups(s) && GIC_DIST_TEST_GROUP(irq, cm);
> 
>      if (!gic_eoi_split(s, cpu, attrs)) {
>          /* This is UNPREDICTABLE; we choose to ignore it */ @@ -557,7 +562,7
> @@ static void gic_deactivate_irq(GICState *s, int cpu, int irq, MemTxAttrs
> attrs)
>          return;
>      }
> 
> -    GIC_CLEAR_ACTIVE(irq, cm);
> +    GIC_DIST_CLEAR_ACTIVE(irq, cm);
>  }
> 
>  void gic_complete_irq(GICState *s, int cpu, int irq, MemTxAttrs attrs) @@ -
> 584,14 +589,15 @@ void gic_complete_irq(GICState *s, int cpu, int irq,
> MemTxAttrs attrs)
>      if (s->revision == REV_11MPCORE) {
>          /* Mark level triggered interrupts as pending if they are still
>             raised.  */
> -        if (!GIC_TEST_EDGE_TRIGGER(irq) && GIC_TEST_ENABLED(irq, cm)
> -            && GIC_TEST_LEVEL(irq, cm) && (GIC_TARGET(irq) & cm) != 0) {
> +        if (!GIC_DIST_TEST_EDGE_TRIGGER(irq) && GIC_DIST_TEST_ENABLED(irq,
> cm)
> +            && GIC_DIST_TEST_LEVEL(irq, cm)
> +            && (GIC_DIST_TARGET(irq) & cm) != 0) {
>              DPRINTF("Set %d pending mask %x\n", irq, cm);
> -            GIC_SET_PENDING(irq, cm);
> +            GIC_DIST_SET_PENDING(irq, cm);
>          }
>      }
> 
> -    group = gic_has_groups(s) && GIC_TEST_GROUP(irq, cm);
> +    group = gic_has_groups(s) && GIC_DIST_TEST_GROUP(irq, cm);
> 
>      if (s->security_extn && !attrs.secure && !group) {
>          DPRINTF("Non-secure EOI for Group0 interrupt %d ignored\n", irq); @@ -
> 607,7 +613,7 @@ void gic_complete_irq(GICState *s, int cpu, int irq,
> MemTxAttrs attrs)
> 
>      /* In GICv2 the guest can choose to split priority-drop and deactivate */
>      if (!gic_eoi_split(s, cpu, attrs)) {
> -        GIC_CLEAR_ACTIVE(irq, cm);
> +        GIC_DIST_CLEAR_ACTIVE(irq, cm);
>      }
>      gic_update(s);
>  }
> @@ -655,7 +661,7 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr
> offset, MemTxAttrs attrs)
>                      goto bad_reg;
>                  }
>                  for (i = 0; i < 8; i++) {
> -                    if (GIC_TEST_GROUP(irq + i, cm)) {
> +                    if (GIC_DIST_TEST_GROUP(irq + i, cm)) {
>                          res |= (1 << i);
>                      }
>                  }
> @@ -675,11 +681,11 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr
> offset, MemTxAttrs attrs)
>          res = 0;
>          for (i = 0; i < 8; i++) {
>              if (s->security_extn && !attrs.secure &&
> -                !GIC_TEST_GROUP(irq + i, 1 << cpu)) {
> +                !GIC_DIST_TEST_GROUP(irq + i, 1 << cpu)) {
>                  continue; /* Ignore Non-secure access of Group0 IRQ */
>              }
> 
> -            if (GIC_TEST_ENABLED(irq + i, cm)) {
> +            if (GIC_DIST_TEST_ENABLED(irq + i, cm)) {
>                  res |= (1 << i);
>              }
>          }
> @@ -696,7 +702,7 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr
> offset, MemTxAttrs attrs)
>          mask = (irq < GIC_INTERNAL) ?  cm : ALL_CPU_MASK;
>          for (i = 0; i < 8; i++) {
>              if (s->security_extn && !attrs.secure &&
> -                !GIC_TEST_GROUP(irq + i, 1 << cpu)) {
> +                !GIC_DIST_TEST_GROUP(irq + i, 1 << cpu)) {
>                  continue; /* Ignore Non-secure access of Group0 IRQ */
>              }
> 
> @@ -713,11 +719,11 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr
> offset, MemTxAttrs attrs)
>          mask = (irq < GIC_INTERNAL) ?  cm : ALL_CPU_MASK;
>          for (i = 0; i < 8; i++) {
>              if (s->security_extn && !attrs.secure &&
> -                !GIC_TEST_GROUP(irq + i, 1 << cpu)) {
> +                !GIC_DIST_TEST_GROUP(irq + i, 1 << cpu)) {
>                  continue; /* Ignore Non-secure access of Group0 IRQ */
>              }
> 
> -            if (GIC_TEST_ACTIVE(irq + i, mask)) {
> +            if (GIC_DIST_TEST_ACTIVE(irq + i, mask)) {
>                  res |= (1 << i);
>              }
>          }
> @@ -726,7 +732,7 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr
> offset, MemTxAttrs attrs)
>          irq = (offset - 0x400) + GIC_BASE_IRQ;
>          if (irq >= s->num_irq)
>              goto bad_reg;
> -        res = gic_get_priority(s, cpu, irq, attrs);
> +        res = gic_dist_get_priority(s, cpu, irq, attrs);
>      } else if (offset < 0xc00) {
>          /* Interrupt CPU Target.  */
>          if (s->num_cpu == 1 && s->revision != REV_11MPCORE) { @@ -740,7
> +746,7 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr offset,
> MemTxAttrs attrs)
>              if (irq >= 29 && irq <= 31) {
>                  res = cm;
>              } else {
> -                res = GIC_TARGET(irq);
> +                res = GIC_DIST_TARGET(irq);
>              }
>          }
>      } else if (offset < 0xf00) {
> @@ -751,14 +757,16 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr
> offset, MemTxAttrs attrs)
>          res = 0;
>          for (i = 0; i < 4; i++) {
>              if (s->security_extn && !attrs.secure &&
> -                !GIC_TEST_GROUP(irq + i, 1 << cpu)) {
> +                !GIC_DIST_TEST_GROUP(irq + i, 1 << cpu)) {
>                  continue; /* Ignore Non-secure access of Group0 IRQ */
>              }
> 
> -            if (GIC_TEST_MODEL(irq + i))
> +            if (GIC_DIST_TEST_MODEL(irq + i)) {
>                  res |= (1 << (i * 2));
> -            if (GIC_TEST_EDGE_TRIGGER(irq + i))
> +            }
> +            if (GIC_DIST_TEST_EDGE_TRIGGER(irq + i)) {
>                  res |= (2 << (i * 2));
> +            }
>          }
>      } else if (offset < 0xf10) {
>          goto bad_reg;
> @@ -776,7 +784,7 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr
> offset, MemTxAttrs attrs)
>          }
> 
>          if (s->security_extn && !attrs.secure &&
> -            !GIC_TEST_GROUP(irq, 1 << cpu)) {
> +            !GIC_DIST_TEST_GROUP(irq, 1 << cpu)) {
>              res = 0; /* Ignore Non-secure access of Group0 IRQ */
>          } else {
>              res = s->sgi_pending[irq][cpu]; @@ -872,10 +880,10 @@ static void
> gic_dist_writeb(void *opaque, hwaddr offset,
>                      int cm = (irq < GIC_INTERNAL) ? (1 << cpu) : ALL_CPU_MASK;
>                      if (value & (1 << i)) {
>                          /* Group1 (Non-secure) */
> -                        GIC_SET_GROUP(irq + i, cm);
> +                        GIC_DIST_SET_GROUP(irq + i, cm);
>                      } else {
>                          /* Group0 (Secure) */
> -                        GIC_CLEAR_GROUP(irq + i, cm);
> +                        GIC_DIST_CLEAR_GROUP(irq + i, cm);
>                      }
>                  }
>              }
> @@ -893,26 +901,26 @@ static void gic_dist_writeb(void *opaque, hwaddr
> offset,
> 
>          for (i = 0; i < 8; i++) {
>              if (value & (1 << i)) {
> -                int mask =
> -                    (irq < GIC_INTERNAL) ? (1 << cpu) : GIC_TARGET(irq + i);
> +                int mask = (irq < GIC_INTERNAL) ? (1 << cpu)
> +                                                : GIC_DIST_TARGET(irq +
> + i);
>                  int cm = (irq < GIC_INTERNAL) ? (1 << cpu) : ALL_CPU_MASK;
> 
>                  if (s->security_extn && !attrs.secure &&
> -                    !GIC_TEST_GROUP(irq + i, 1 << cpu)) {
> +                    !GIC_DIST_TEST_GROUP(irq + i, 1 << cpu)) {
>                      continue; /* Ignore Non-secure access of Group0 IRQ */
>                  }
> 
> -                if (!GIC_TEST_ENABLED(irq + i, cm)) {
> +                if (!GIC_DIST_TEST_ENABLED(irq + i, cm)) {
>                      DPRINTF("Enabled IRQ %d\n", irq + i);
>                      trace_gic_enable_irq(irq + i);
>                  }
> -                GIC_SET_ENABLED(irq + i, cm);
> +                GIC_DIST_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)) {
> +                if (GIC_DIST_TEST_LEVEL(irq + i, mask)
> +                        && !GIC_DIST_TEST_EDGE_TRIGGER(irq + i)) {
>                      DPRINTF("Set %d pending mask %x\n", irq + i, mask);
> -                    GIC_SET_PENDING(irq + i, mask);
> +                    GIC_DIST_SET_PENDING(irq + i, mask);
>                  }
>              }
>          }
> @@ -930,15 +938,15 @@ static void gic_dist_writeb(void *opaque, hwaddr
> offset,
>                  int cm = (irq < GIC_INTERNAL) ? (1 << cpu) : ALL_CPU_MASK;
> 
>                  if (s->security_extn && !attrs.secure &&
> -                    !GIC_TEST_GROUP(irq + i, 1 << cpu)) {
> +                    !GIC_DIST_TEST_GROUP(irq + i, 1 << cpu)) {
>                      continue; /* Ignore Non-secure access of Group0 IRQ */
>                  }
> 
> -                if (GIC_TEST_ENABLED(irq + i, cm)) {
> +                if (GIC_DIST_TEST_ENABLED(irq + i, cm)) {
>                      DPRINTF("Disabled IRQ %d\n", irq + i);
>                      trace_gic_disable_irq(irq + i);
>                  }
> -                GIC_CLEAR_ENABLED(irq + i, cm);
> +                GIC_DIST_CLEAR_ENABLED(irq + i, cm);
>              }
>          }
>      } else if (offset < 0x280) {
> @@ -953,11 +961,11 @@ static void gic_dist_writeb(void *opaque, hwaddr
> offset,
>          for (i = 0; i < 8; i++) {
>              if (value & (1 << i)) {
>                  if (s->security_extn && !attrs.secure &&
> -                    !GIC_TEST_GROUP(irq + i, 1 << cpu)) {
> +                    !GIC_DIST_TEST_GROUP(irq + i, 1 << cpu)) {
>                      continue; /* Ignore Non-secure access of Group0 IRQ */
>                  }
> 
> -                GIC_SET_PENDING(irq + i, GIC_TARGET(irq + i));
> +                GIC_DIST_SET_PENDING(irq + i, GIC_DIST_TARGET(irq +
> + i));
>              }
>          }
>      } else if (offset < 0x300) {
> @@ -971,7 +979,7 @@ static void gic_dist_writeb(void *opaque, hwaddr
> offset,
> 
>          for (i = 0; i < 8; i++) {
>              if (s->security_extn && !attrs.secure &&
> -                !GIC_TEST_GROUP(irq + i, 1 << cpu)) {
> +                !GIC_DIST_TEST_GROUP(irq + i, 1 << cpu)) {
>                  continue; /* Ignore Non-secure access of Group0 IRQ */
>              }
> 
> @@ -979,7 +987,7 @@ static void gic_dist_writeb(void *opaque, hwaddr
> offset,
>                 for per-CPU interrupts.  It's unclear whether this is the
>                 corect behavior.  */
>              if (value & (1 << i)) {
> -                GIC_CLEAR_PENDING(irq + i, ALL_CPU_MASK);
> +                GIC_DIST_CLEAR_PENDING(irq + i, ALL_CPU_MASK);
>              }
>          }
>      } else if (offset < 0x400) {
> @@ -990,7 +998,7 @@ static void gic_dist_writeb(void *opaque, hwaddr
> offset,
>          irq = (offset - 0x400) + GIC_BASE_IRQ;
>          if (irq >= s->num_irq)
>              goto bad_reg;
> -        gic_set_priority(s, cpu, irq, value, attrs);
> +        gic_dist_set_priority(s, cpu, irq, value, attrs);
>      } else if (offset < 0xc00) {
>          /* Interrupt CPU Target. RAZ/WI on uniprocessor GICs, with the
>           * annoying exception of the 11MPCore's GIC.
> @@ -1016,21 +1024,21 @@ static void gic_dist_writeb(void *opaque, hwaddr
> offset,
>              value |= 0xaa;
>          for (i = 0; i < 4; i++) {
>              if (s->security_extn && !attrs.secure &&
> -                !GIC_TEST_GROUP(irq + i, 1 << cpu)) {
> +                !GIC_DIST_TEST_GROUP(irq + i, 1 << cpu)) {
>                  continue; /* Ignore Non-secure access of Group0 IRQ */
>              }
> 
>              if (s->revision == REV_11MPCORE) {
>                  if (value & (1 << (i * 2))) {
> -                    GIC_SET_MODEL(irq + i);
> +                    GIC_DIST_SET_MODEL(irq + i);
>                  } else {
> -                    GIC_CLEAR_MODEL(irq + i);
> +                    GIC_DIST_CLEAR_MODEL(irq + i);
>                  }
>              }
>              if (value & (2 << (i * 2))) {
> -                GIC_SET_EDGE_TRIGGER(irq + i);
> +                GIC_DIST_SET_EDGE_TRIGGER(irq + i);
>              } else {
> -                GIC_CLEAR_EDGE_TRIGGER(irq + i);
> +                GIC_DIST_CLEAR_EDGE_TRIGGER(irq + i);
>              }
>          }
>      } else if (offset < 0xf10) {
> @@ -1044,10 +1052,10 @@ static void gic_dist_writeb(void *opaque, hwaddr
> offset,
>          irq = (offset - 0xf10);
> 
>          if (!s->security_extn || attrs.secure ||
> -            GIC_TEST_GROUP(irq, 1 << cpu)) {
> +            GIC_DIST_TEST_GROUP(irq, 1 << cpu)) {
>              s->sgi_pending[irq][cpu] &= ~value;
>              if (s->sgi_pending[irq][cpu] == 0) {
> -                GIC_CLEAR_PENDING(irq, 1 << cpu);
> +                GIC_DIST_CLEAR_PENDING(irq, 1 << cpu);
>              }
>          }
>      } else if (offset < 0xf30) {
> @@ -1058,8 +1066,8 @@ static void gic_dist_writeb(void *opaque, hwaddr
> offset,
>          irq = (offset - 0xf20);
> 
>          if (!s->security_extn || attrs.secure ||
> -            GIC_TEST_GROUP(irq, 1 << cpu)) {
> -            GIC_SET_PENDING(irq, 1 << cpu);
> +            GIC_DIST_TEST_GROUP(irq, 1 << cpu)) {
> +            GIC_DIST_SET_PENDING(irq, 1 << cpu);
>              s->sgi_pending[irq][cpu] |= value;
>          }
>      } else {
> @@ -1106,7 +1114,7 @@ static void gic_dist_writel(void *opaque, hwaddr
> offset,
>              mask = ALL_CPU_MASK;
>              break;
>          }
> -        GIC_SET_PENDING(irq, mask);
> +        GIC_DIST_SET_PENDING(irq, mask);
>          target_cpu = ctz32(mask);
>          while (target_cpu < GIC_NCPU) {
>              s->sgi_pending[irq][target_cpu] |= (1 << cpu); diff --git
> a/hw/intc/arm_gic_common.c b/hw/intc/arm_gic_common.c index
> aee50a20e0..295ee9cc5e 100644
> --- a/hw/intc/arm_gic_common.c
> +++ b/hw/intc/arm_gic_common.c
> @@ -204,8 +204,8 @@ static void arm_gic_common_reset(DeviceState *dev)
>          }
>      }
>      for (i = 0; i < GIC_NR_SGIS; i++) {
> -        GIC_SET_ENABLED(i, ALL_CPU_MASK);
> -        GIC_SET_EDGE_TRIGGER(i);
> +        GIC_DIST_SET_ENABLED(i, ALL_CPU_MASK);
> +        GIC_DIST_SET_EDGE_TRIGGER(i);
>      }
> 
>      for (i = 0; i < ARRAY_SIZE(s->priority2); i++) { @@ -222,7 +222,7 @@ static
> void arm_gic_common_reset(DeviceState *dev)
>      }
>      if (s->security_extn && s->irq_reset_nonsecure) {
>          for (i = 0; i < GIC_MAXIRQ; i++) {
> -            GIC_SET_GROUP(i, ALL_CPU_MASK);
> +            GIC_DIST_SET_GROUP(i, ALL_CPU_MASK);
>          }
>      }
> 
> diff --git a/hw/intc/arm_gic_kvm.c b/hw/intc/arm_gic_kvm.c index
> 204369d0e2..799136732a 100644
> --- a/hw/intc/arm_gic_kvm.c
> +++ b/hw/intc/arm_gic_kvm.c
> @@ -140,10 +140,10 @@ static void translate_group(GICState *s, int irq, int
> cpu,
>      int cm = (irq < GIC_INTERNAL) ? (1 << cpu) : ALL_CPU_MASK;
> 
>      if (to_kernel) {
> -        *field = GIC_TEST_GROUP(irq, cm);
> +        *field = GIC_DIST_TEST_GROUP(irq, cm);
>      } else {
>          if (*field & 1) {
> -            GIC_SET_GROUP(irq, cm);
> +            GIC_DIST_SET_GROUP(irq, cm);
>          }
>      }
>  }
> @@ -154,10 +154,10 @@ static void translate_enabled(GICState *s, int irq, int
> cpu,
>      int cm = (irq < GIC_INTERNAL) ? (1 << cpu) : ALL_CPU_MASK;
> 
>      if (to_kernel) {
> -        *field = GIC_TEST_ENABLED(irq, cm);
> +        *field = GIC_DIST_TEST_ENABLED(irq, cm);
>      } else {
>          if (*field & 1) {
> -            GIC_SET_ENABLED(irq, cm);
> +            GIC_DIST_SET_ENABLED(irq, cm);
>          }
>      }
>  }
> @@ -171,7 +171,7 @@ static void translate_pending(GICState *s, int irq, int
> cpu,
>          *field = gic_test_pending(s, irq, cm);
>      } else {
>          if (*field & 1) {
> -            GIC_SET_PENDING(irq, cm);
> +            GIC_DIST_SET_PENDING(irq, cm);
>              /* TODO: Capture is level-line is held high in the kernel */
>          }
>      }
> @@ -183,10 +183,10 @@ static void translate_active(GICState *s, int irq, int
> cpu,
>      int cm = (irq < GIC_INTERNAL) ? (1 << cpu) : ALL_CPU_MASK;
> 
>      if (to_kernel) {
> -        *field = GIC_TEST_ACTIVE(irq, cm);
> +        *field = GIC_DIST_TEST_ACTIVE(irq, cm);
>      } else {
>          if (*field & 1) {
> -            GIC_SET_ACTIVE(irq, cm);
> +            GIC_DIST_SET_ACTIVE(irq, cm);
>          }
>      }
>  }
> @@ -195,10 +195,10 @@ static void translate_trigger(GICState *s, int irq, int
> cpu,
>                                uint32_t *field, bool to_kernel)  {
>      if (to_kernel) {
> -        *field = (GIC_TEST_EDGE_TRIGGER(irq)) ? 0x2 : 0x0;
> +        *field = (GIC_DIST_TEST_EDGE_TRIGGER(irq)) ? 0x2 : 0x0;
>      } else {
>          if (*field & 0x2) {
> -            GIC_SET_EDGE_TRIGGER(irq);
> +            GIC_DIST_SET_EDGE_TRIGGER(irq);
>          }
>      }
>  }
> @@ -207,9 +207,10 @@ static void translate_priority(GICState *s, int irq, int
> cpu,
>                                 uint32_t *field, bool to_kernel)  {
>      if (to_kernel) {
> -        *field = GIC_GET_PRIORITY(irq, cpu) & 0xff;
> +        *field = GIC_DIST_GET_PRIORITY(irq, cpu) & 0xff;
>      } else {
> -        gic_set_priority(s, cpu, irq, *field & 0xff, MEMTXATTRS_UNSPECIFIED);
> +        gic_dist_set_priority(s, cpu, irq,
> +                              *field & 0xff, MEMTXATTRS_UNSPECIFIED);
>      }
>  }
> 
> diff --git a/hw/intc/gic_internal.h b/hw/intc/gic_internal.h index
> 7fe87b13de..6f8d242904 100644
> --- a/hw/intc/gic_internal.h
> +++ b/hw/intc/gic_internal.h
> @@ -27,30 +27,31 @@
> 
>  #define GIC_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_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_MODEL(irq) s->irq_state[irq].model = true -#define
> GIC_CLEAR_MODEL(irq) s->irq_state[irq].model = false -#define
> GIC_TEST_MODEL(irq) s->irq_state[irq].model -#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) < GIC_INTERNAL) ?            \
> +#define GIC_DIST_SET_ENABLED(irq, cm) (s->irq_state[irq].enabled |=
> +(cm)) #define GIC_DIST_CLEAR_ENABLED(irq, cm)
> +(s->irq_state[irq].enabled &= ~(cm)) #define GIC_DIST_TEST_ENABLED(irq,
> +cm) ((s->irq_state[irq].enabled & (cm)) != 0) #define
> +GIC_DIST_SET_PENDING(irq, cm) (s->irq_state[irq].pending |= (cm))
> +#define GIC_DIST_CLEAR_PENDING(irq, cm) (s->irq_state[irq].pending &=
> +~(cm)) #define GIC_DIST_SET_ACTIVE(irq, cm) (s->irq_state[irq].active
> +|= (cm)) #define GIC_DIST_CLEAR_ACTIVE(irq, cm)
> +(s->irq_state[irq].active &= ~(cm)) #define GIC_DIST_TEST_ACTIVE(irq,
> +cm) ((s->irq_state[irq].active & (cm)) != 0) #define
> +GIC_DIST_SET_MODEL(irq) (s->irq_state[irq].model = true) #define
> +GIC_DIST_CLEAR_MODEL(irq) (s->irq_state[irq].model = false) #define
> +GIC_DIST_TEST_MODEL(irq) (s->irq_state[irq].model) #define
> +GIC_DIST_SET_LEVEL(irq, cm) (s->irq_state[irq].level |= (cm)) #define
> +GIC_DIST_CLEAR_LEVEL(irq, cm) (s->irq_state[irq].level &= ~(cm))
> +#define GIC_DIST_TEST_LEVEL(irq, cm) ((s->irq_state[irq].level & (cm)) != 0)
> #define GIC_DIST_SET_EDGE_TRIGGER(irq) (s->irq_state[irq].edge_trigger =
> true) #define GIC_DIST_CLEAR_EDGE_TRIGGER(irq) \
> +    (s->irq_state[irq].edge_trigger = false) #define
> +GIC_DIST_TEST_EDGE_TRIGGER(irq) (s->irq_state[irq].edge_trigger)
> +#define GIC_DIST_GET_PRIORITY(irq, cpu) (((irq) < GIC_INTERNAL) ?            \
>                                      s->priority1[irq][cpu] :            \
>                                      s->priority2[(irq) - GIC_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)
> +#define GIC_DIST_TARGET(irq) (s->irq_target[irq]) #define
> +GIC_DIST_CLEAR_GROUP(irq, cm) (s->irq_state[irq].group &= ~(cm))
> +#define GIC_DIST_SET_GROUP(irq, cm) (s->irq_state[irq].group |= (cm))
> +#define GIC_DIST_TEST_GROUP(irq, cm) ((s->irq_state[irq].group & (cm))
> +!= 0)
> 
>  #define GICD_CTLR_EN_GRP0 (1U << 0)
>  #define GICD_CTLR_EN_GRP1 (1U << 1)
> @@ -79,8 +80,8 @@ uint32_t gic_acknowledge_irq(GICState *s, int cpu,
> MemTxAttrs attrs);  void gic_complete_irq(GICState *s, int cpu, int irq,
> MemTxAttrs attrs);  void gic_update(GICState *s);  void
> gic_init_irqs_and_distributor(GICState *s); -void gic_set_priority(GICState *s, int
> cpu, int irq, uint8_t val,
> -                      MemTxAttrs attrs);
> +void gic_dist_set_priority(GICState *s, int cpu, int irq, uint8_t val,
> +                           MemTxAttrs attrs);
> 
>  static inline bool gic_test_pending(GICState *s, int irq, int cm)  { @@ -93,7
> +94,7 @@ static inline bool gic_test_pending(GICState *s, int irq, int cm)
>           * GICD_ISPENDR to set the state pending.
>           */
>          return (s->irq_state[irq].pending & cm) ||
> -            (!GIC_TEST_EDGE_TRIGGER(irq) && GIC_TEST_LEVEL(irq, cm));
> +            (!GIC_DIST_TEST_EDGE_TRIGGER(irq) &&
> + GIC_DIST_TEST_LEVEL(irq, cm));
>      }
>  }
> 
> --
> 2.17.0

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

* Re: [Qemu-devel] [PATCH 1/6] intc/arm_gic: Refactor operations on the distributor
  2018-06-06  9:30 ` [Qemu-devel] [PATCH 1/6] intc/arm_gic: Refactor operations on the distributor luc.michel
  2018-06-06 13:38   ` [Qemu-devel] [Qemu-arm] " Philippe Mathieu-Daudé
  2018-06-07  6:08   ` [Qemu-devel] " Sai Pavan Boddu
@ 2018-06-11 13:39   ` Peter Maydell
  2 siblings, 0 replies; 17+ messages in thread
From: Peter Maydell @ 2018-06-11 13:39 UTC (permalink / raw)
  To: luc.michel
  Cc: QEMU Developers, qemu-arm, Sai Pavan Boddu, Edgar Iglesias,
	Mark Burton, Jan Kiszka

On 6 June 2018 at 10:30,  <luc.michel@greensocs.com> wrote:
> From: Luc MICHEL <luc.michel@greensocs.com>
>
> In preparation for the virtualization extensions implementation,
> refactor the name of the functions and macros that act on the GIC
> distributor to make that fact explicit. It will be useful to
> differentiate them from the ones that will act on the virtual
> interfaces.
>
> Signed-off-by: Luc MICHEL <luc.michel@greensocs.com>
> ---
>  hw/intc/arm_gic.c        | 164 ++++++++++++++++++++-------------------
>  hw/intc/arm_gic_common.c |   6 +-
>  hw/intc/arm_gic_kvm.c    |  23 +++---
>  hw/intc/gic_internal.h   |  51 ++++++------
>  4 files changed, 127 insertions(+), 117 deletions(-)

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

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

* [Qemu-devel] [PATCH 2/6] intc/arm_gic: Remove some dead code and put some functions static
  2018-06-06  9:30 [Qemu-devel] [PATCH 0/6] arm_gic: add virtualization extensions support luc.michel
  2018-06-06  9:30 ` [Qemu-devel] [PATCH 1/6] intc/arm_gic: Refactor operations on the distributor luc.michel
@ 2018-06-06  9:30 ` luc.michel
  2018-06-06 13:39   ` [Qemu-devel] [Qemu-arm] " Philippe Mathieu-Daudé
  2018-06-11 13:39   ` [Qemu-devel] " Peter Maydell
  2018-06-06  9:30 ` [Qemu-devel] [PATCH 3/6] intc/arm_gic: Add the virtualization extensions to the GIC state luc.michel
                   ` (4 subsequent siblings)
  6 siblings, 2 replies; 17+ messages in thread
From: luc.michel @ 2018-06-06  9:30 UTC (permalink / raw)
  To: qemu-devel
  Cc: Luc MICHEL, qemu-arm, Peter Maydell, saipava, edgari, mark.burton,
	Jan Kiszka

From: Luc MICHEL <luc.michel@greensocs.com>

Some functions are now only used in arm_gic.c, put them static. Some of
them where only used by the NVIC implementation and are not used
anymore, so remove them.

Signed-off-by: Luc MICHEL <luc.michel@greensocs.com>
---
 hw/intc/arm_gic.c      | 23 ++---------------------
 hw/intc/gic_internal.h |  4 ----
 2 files changed, 2 insertions(+), 25 deletions(-)

diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
index 141f3e7a48..679b19fb94 100644
--- a/hw/intc/arm_gic.c
+++ b/hw/intc/arm_gic.c
@@ -71,7 +71,7 @@ static inline bool gic_has_groups(GICState *s)
 
 /* TODO: Many places that call this routine could be optimized.  */
 /* Update interrupt status after enabled or pending bits have been changed.  */
-void gic_update(GICState *s)
+static void gic_update(GICState *s)
 {
     int best_irq;
     int best_prio;
@@ -137,19 +137,6 @@ void gic_update(GICState *s)
     }
 }
 
-void gic_set_pending_private(GICState *s, int cpu, int irq)
-{
-    int cm = 1 << cpu;
-
-    if (gic_test_pending(s, irq, cm)) {
-        return;
-    }
-
-    DPRINTF("Set %d pending cpu %d\n", irq, cpu);
-    GIC_DIST_SET_PENDING(irq, cm);
-    gic_update(s);
-}
-
 static void gic_set_irq_11mpcore(GICState *s, int irq, int level,
                                  int cm, int target)
 {
@@ -565,7 +552,7 @@ static void gic_deactivate_irq(GICState *s, int cpu, int irq, MemTxAttrs attrs)
     GIC_DIST_CLEAR_ACTIVE(irq, cm);
 }
 
-void gic_complete_irq(GICState *s, int cpu, int irq, MemTxAttrs attrs)
+static void gic_complete_irq(GICState *s, int cpu, int irq, MemTxAttrs attrs)
 {
     int cm = 1 << cpu;
     int group;
@@ -1418,12 +1405,6 @@ static const MemoryRegionOps gic_cpu_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-/* This function is used by nvic model */
-void gic_init_irqs_and_distributor(GICState *s)
-{
-    gic_init_irqs_and_mmio(s, gic_set_irq, gic_ops);
-}
-
 static void arm_gic_realize(DeviceState *dev, Error **errp)
 {
     /* Device instance realize function for the GIC sysbus device */
diff --git a/hw/intc/gic_internal.h b/hw/intc/gic_internal.h
index 6f8d242904..a2075a94db 100644
--- a/hw/intc/gic_internal.h
+++ b/hw/intc/gic_internal.h
@@ -75,11 +75,7 @@
 /* The special cases for the revision property: */
 #define REV_11MPCORE 0
 
-void gic_set_pending_private(GICState *s, int cpu, int irq);
 uint32_t gic_acknowledge_irq(GICState *s, int cpu, MemTxAttrs attrs);
-void gic_complete_irq(GICState *s, int cpu, int irq, MemTxAttrs attrs);
-void gic_update(GICState *s);
-void gic_init_irqs_and_distributor(GICState *s);
 void gic_dist_set_priority(GICState *s, int cpu, int irq, uint8_t val,
                            MemTxAttrs attrs);
 
-- 
2.17.0

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

* Re: [Qemu-devel] [Qemu-arm] [PATCH 2/6] intc/arm_gic: Remove some dead code and put some functions static
  2018-06-06  9:30 ` [Qemu-devel] [PATCH 2/6] intc/arm_gic: Remove some dead code and put some functions static luc.michel
@ 2018-06-06 13:39   ` Philippe Mathieu-Daudé
  2018-06-11 13:39   ` [Qemu-devel] " Peter Maydell
  1 sibling, 0 replies; 17+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-06-06 13:39 UTC (permalink / raw)
  To: luc.michel, qemu-devel
  Cc: Peter Maydell, mark.burton, saipava, edgari, qemu-arm, Jan Kiszka

On 06/06/2018 06:30 AM, luc.michel@greensocs.com wrote:
> From: Luc MICHEL <luc.michel@greensocs.com>
> 
> Some functions are now only used in arm_gic.c, put them static. Some of
> them where only used by the NVIC implementation and are not used
> anymore, so remove them.
> 
> Signed-off-by: Luc MICHEL <luc.michel@greensocs.com>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  hw/intc/arm_gic.c      | 23 ++---------------------
>  hw/intc/gic_internal.h |  4 ----
>  2 files changed, 2 insertions(+), 25 deletions(-)
> 
> diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
> index 141f3e7a48..679b19fb94 100644
> --- a/hw/intc/arm_gic.c
> +++ b/hw/intc/arm_gic.c
> @@ -71,7 +71,7 @@ static inline bool gic_has_groups(GICState *s)
>  
>  /* TODO: Many places that call this routine could be optimized.  */
>  /* Update interrupt status after enabled or pending bits have been changed.  */
> -void gic_update(GICState *s)
> +static void gic_update(GICState *s)
>  {
>      int best_irq;
>      int best_prio;
> @@ -137,19 +137,6 @@ void gic_update(GICState *s)
>      }
>  }
>  
> -void gic_set_pending_private(GICState *s, int cpu, int irq)
> -{
> -    int cm = 1 << cpu;
> -
> -    if (gic_test_pending(s, irq, cm)) {
> -        return;
> -    }
> -
> -    DPRINTF("Set %d pending cpu %d\n", irq, cpu);
> -    GIC_DIST_SET_PENDING(irq, cm);
> -    gic_update(s);
> -}
> -
>  static void gic_set_irq_11mpcore(GICState *s, int irq, int level,
>                                   int cm, int target)
>  {
> @@ -565,7 +552,7 @@ static void gic_deactivate_irq(GICState *s, int cpu, int irq, MemTxAttrs attrs)
>      GIC_DIST_CLEAR_ACTIVE(irq, cm);
>  }
>  
> -void gic_complete_irq(GICState *s, int cpu, int irq, MemTxAttrs attrs)
> +static void gic_complete_irq(GICState *s, int cpu, int irq, MemTxAttrs attrs)
>  {
>      int cm = 1 << cpu;
>      int group;
> @@ -1418,12 +1405,6 @@ static const MemoryRegionOps gic_cpu_ops = {
>      .endianness = DEVICE_NATIVE_ENDIAN,
>  };
>  
> -/* This function is used by nvic model */
> -void gic_init_irqs_and_distributor(GICState *s)
> -{
> -    gic_init_irqs_and_mmio(s, gic_set_irq, gic_ops);
> -}
> -
>  static void arm_gic_realize(DeviceState *dev, Error **errp)
>  {
>      /* Device instance realize function for the GIC sysbus device */
> diff --git a/hw/intc/gic_internal.h b/hw/intc/gic_internal.h
> index 6f8d242904..a2075a94db 100644
> --- a/hw/intc/gic_internal.h
> +++ b/hw/intc/gic_internal.h
> @@ -75,11 +75,7 @@
>  /* The special cases for the revision property: */
>  #define REV_11MPCORE 0
>  
> -void gic_set_pending_private(GICState *s, int cpu, int irq);
>  uint32_t gic_acknowledge_irq(GICState *s, int cpu, MemTxAttrs attrs);
> -void gic_complete_irq(GICState *s, int cpu, int irq, MemTxAttrs attrs);
> -void gic_update(GICState *s);
> -void gic_init_irqs_and_distributor(GICState *s);
>  void gic_dist_set_priority(GICState *s, int cpu, int irq, uint8_t val,
>                             MemTxAttrs attrs);
>  
> 

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

* Re: [Qemu-devel] [PATCH 2/6] intc/arm_gic: Remove some dead code and put some functions static
  2018-06-06  9:30 ` [Qemu-devel] [PATCH 2/6] intc/arm_gic: Remove some dead code and put some functions static luc.michel
  2018-06-06 13:39   ` [Qemu-devel] [Qemu-arm] " Philippe Mathieu-Daudé
@ 2018-06-11 13:39   ` Peter Maydell
  1 sibling, 0 replies; 17+ messages in thread
From: Peter Maydell @ 2018-06-11 13:39 UTC (permalink / raw)
  To: luc.michel
  Cc: QEMU Developers, qemu-arm, Sai Pavan Boddu, Edgar Iglesias,
	Mark Burton, Jan Kiszka

On 6 June 2018 at 10:30,  <luc.michel@greensocs.com> wrote:
> From: Luc MICHEL <luc.michel@greensocs.com>
>
> Some functions are now only used in arm_gic.c, put them static. Some of
> them where only used by the NVIC implementation and are not used
> anymore, so remove them.
>
> Signed-off-by: Luc MICHEL <luc.michel@greensocs.com>
> ---
>  hw/intc/arm_gic.c      | 23 ++---------------------
>  hw/intc/gic_internal.h |  4 ----
>  2 files changed, 2 insertions(+), 25 deletions(-)
>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

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

* [Qemu-devel] [PATCH 3/6] intc/arm_gic: Add the virtualization extensions to the GIC state
  2018-06-06  9:30 [Qemu-devel] [PATCH 0/6] arm_gic: add virtualization extensions support luc.michel
  2018-06-06  9:30 ` [Qemu-devel] [PATCH 1/6] intc/arm_gic: Refactor operations on the distributor luc.michel
  2018-06-06  9:30 ` [Qemu-devel] [PATCH 2/6] intc/arm_gic: Remove some dead code and put some functions static luc.michel
@ 2018-06-06  9:30 ` luc.michel
  2018-06-11 13:38   ` Peter Maydell
  2018-06-06  9:30 ` [Qemu-devel] [PATCH 4/6] intc/arm_gic: Add virtualization extensions logic luc.michel
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 17+ messages in thread
From: luc.michel @ 2018-06-06  9:30 UTC (permalink / raw)
  To: qemu-devel
  Cc: Luc MICHEL, qemu-arm, Peter Maydell, saipava, edgari, mark.burton,
	Jan Kiszka

From: Luc MICHEL <luc.michel@greensocs.com>

Add the necessary parts of the virtualization extensions state to the
GIC state. We choose to increase the size of the CPU interfaces state to
add space for the vCPU interfaces (the GIC_NCPU_VCPU macro). This way,
we'll be able to reuse most of the CPU interface code for the vCPUs.

The vCPUs are numbered from GIC_NCPU to (GIC_NCPU * 2) - 1. The
`gic_is_vcpu` function help to determine if a given CPU id correspond to
a physical CPU or a virtual one.

The GIC VMState is updated to account for this modification. We add a
subsection for the virtual interface state. The virtual CPU interfaces
state however cannot be put in the subsection because of some 2D arrays
in the GIC state. This implies an increase in the VMState version id.

For the in-kernel KVM VGIC, since the exposed VGIC does not implement
the virtualization extensions, we report an error if the corresponding
property is set to true.

Signed-off-by: Luc MICHEL <luc.michel@greensocs.com>
---
 hw/intc/arm_gic.c                |   2 +-
 hw/intc/arm_gic_common.c         | 153 +++++++++++++++++++++++++------
 hw/intc/arm_gic_kvm.c            |   8 +-
 hw/intc/gic_internal.h           |   5 +
 include/hw/intc/arm_gic_common.h |  52 +++++++++--
 5 files changed, 183 insertions(+), 37 deletions(-)

diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
index 679b19fb94..e96f195997 100644
--- a/hw/intc/arm_gic.c
+++ b/hw/intc/arm_gic.c
@@ -1427,7 +1427,7 @@ static void arm_gic_realize(DeviceState *dev, Error **errp)
     }
 
     /* This creates distributor and main CPU interface (s->cpuiomem[0]) */
-    gic_init_irqs_and_mmio(s, gic_set_irq, gic_ops);
+    gic_init_irqs_and_mmio(s, gic_set_irq, gic_ops, NULL);
 
     /* Extra core-specific regions for the CPU interfaces. This is
      * necessary for "franken-GIC" implementations, for example on
diff --git a/hw/intc/arm_gic_common.c b/hw/intc/arm_gic_common.c
index 295ee9cc5e..e75823c56d 100644
--- a/hw/intc/arm_gic_common.c
+++ b/hw/intc/arm_gic_common.c
@@ -46,6 +46,13 @@ static int gic_post_load(void *opaque, int version_id)
     return 0;
 }
 
+static bool gic_virt_state_needed(void *opaque)
+{
+    GICState *s = (GICState *)opaque;
+
+    return s->virt_extn;
+}
+
 static const VMStateDescription vmstate_gic_irq_state = {
     .name = "arm_gic_irq_state",
     .version_id = 1,
@@ -62,34 +69,58 @@ static const VMStateDescription vmstate_gic_irq_state = {
     }
 };
 
+/* Note: We cannot put the vCPUs state in this subsection because of some 2D
+ * arrays that mix CPU and vCPU states. */
+static const VMStateDescription vmstate_gic_virt_state = {
+    .name = "arm_gic_virt_state",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = gic_virt_state_needed,
+    .fields = (VMStateField[]) {
+        /* Virtual interface */
+        VMSTATE_UINT32_ARRAY(h_hcr, GICState, GIC_NCPU),
+        VMSTATE_UINT32_ARRAY(h_misr, GICState, GIC_NCPU),
+        VMSTATE_UINT32_2DARRAY(h_lr, GICState, GIC_NR_LR, GIC_NCPU),
+        VMSTATE_UINT64_ARRAY(h_eisr, GICState, GIC_NCPU),
+        VMSTATE_UINT64_ARRAY(h_elrsr, GICState, GIC_NCPU),
+        VMSTATE_UINT32_ARRAY(h_apr, GICState, GIC_NCPU),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static const VMStateDescription vmstate_gic = {
     .name = "arm_gic",
-    .version_id = 12,
-    .minimum_version_id = 12,
+    .version_id = 13,
+    .minimum_version_id = 13,
     .pre_save = gic_pre_save,
     .post_load = gic_post_load,
     .fields = (VMStateField[]) {
         VMSTATE_UINT32(ctlr, GICState),
-        VMSTATE_UINT32_ARRAY(cpu_ctlr, GICState, GIC_NCPU),
+        VMSTATE_UINT32_ARRAY(cpu_ctlr, GICState, GIC_NCPU_VCPU),
         VMSTATE_STRUCT_ARRAY(irq_state, GICState, GIC_MAXIRQ, 1,
                              vmstate_gic_irq_state, gic_irq_state),
         VMSTATE_UINT8_ARRAY(irq_target, GICState, GIC_MAXIRQ),
         VMSTATE_UINT8_2DARRAY(priority1, GICState, GIC_INTERNAL, GIC_NCPU),
         VMSTATE_UINT8_ARRAY(priority2, GICState, GIC_MAXIRQ - GIC_INTERNAL),
         VMSTATE_UINT8_2DARRAY(sgi_pending, GICState, GIC_NR_SGIS, GIC_NCPU),
-        VMSTATE_UINT16_ARRAY(priority_mask, GICState, GIC_NCPU),
-        VMSTATE_UINT16_ARRAY(running_priority, GICState, GIC_NCPU),
-        VMSTATE_UINT16_ARRAY(current_pending, GICState, GIC_NCPU),
-        VMSTATE_UINT8_ARRAY(bpr, GICState, GIC_NCPU),
-        VMSTATE_UINT8_ARRAY(abpr, GICState, GIC_NCPU),
-        VMSTATE_UINT32_2DARRAY(apr, GICState, GIC_NR_APRS, GIC_NCPU),
+        VMSTATE_UINT16_ARRAY(priority_mask, GICState, GIC_NCPU_VCPU),
+        VMSTATE_UINT16_ARRAY(running_priority, GICState, GIC_NCPU_VCPU),
+        VMSTATE_UINT16_ARRAY(current_pending, GICState, GIC_NCPU_VCPU),
+        VMSTATE_UINT8_ARRAY(bpr, GICState, GIC_NCPU_VCPU),
+        VMSTATE_UINT8_ARRAY(abpr, GICState, GIC_NCPU_VCPU),
+        VMSTATE_UINT32_2DARRAY(apr, GICState, GIC_NR_APRS, GIC_NCPU_VCPU),
         VMSTATE_UINT32_2DARRAY(nsapr, GICState, GIC_NR_APRS, GIC_NCPU),
         VMSTATE_END_OF_LIST()
+    },
+    .subsections = (const VMStateDescription * []) {
+        &vmstate_gic_virt_state,
+        NULL
     }
 };
 
 void gic_init_irqs_and_mmio(GICState *s, qemu_irq_handler handler,
-                            const MemoryRegionOps *ops)
+                            const MemoryRegionOps *ops,
+                            const MemoryRegionOps *virt_ops)
 {
     SysBusDevice *sbd = SYS_BUS_DEVICE(s);
     int i = s->num_irq - GIC_INTERNAL;
@@ -116,6 +147,11 @@ void gic_init_irqs_and_mmio(GICState *s, qemu_irq_handler handler,
     for (i = 0; i < s->num_cpu; i++) {
         sysbus_init_irq(sbd, &s->parent_vfiq[i]);
     }
+    if (s->virt_extn) {
+        for (i = 0; i < s->num_cpu; i++) {
+            sysbus_init_irq(sbd, &s->maintenance_irq[i]);
+        }
+    }
 
     /* Distributor */
     memory_region_init_io(&s->iomem, OBJECT(s), ops, s, "gic_dist", 0x1000);
@@ -127,6 +163,17 @@ void gic_init_irqs_and_mmio(GICState *s, qemu_irq_handler handler,
     memory_region_init_io(&s->cpuiomem[0], OBJECT(s), ops ? &ops[1] : NULL,
                           s, "gic_cpu", s->revision == 2 ? 0x2000 : 0x100);
     sysbus_init_mmio(sbd, &s->cpuiomem[0]);
+
+    if (s->virt_extn) {
+        memory_region_init_io(&s->vifaceiomem, OBJECT(s), virt_ops,
+                              s, "gic_viface", 0x200);
+        sysbus_init_mmio(sbd, &s->vifaceiomem);
+
+        memory_region_init_io(&s->vcpuiomem[0], OBJECT(s),
+                              virt_ops ? &virt_ops[1] : NULL,
+                              s, "gic_vcpu", 0x2000);
+        sysbus_init_mmio(sbd, &s->vcpuiomem[0]);
+    }
 }
 
 static void arm_gic_common_realize(DeviceState *dev, Error **errp)
@@ -163,6 +210,40 @@ static void arm_gic_common_realize(DeviceState *dev, Error **errp)
                    "the security extensions");
         return;
     }
+
+    if (s->virt_extn && (s->revision != 2)) {
+        error_setg(errp, "GIC virtualization extensions are only "
+                   "supported by revision 2");
+        return;
+    }
+}
+
+static inline void arm_gic_common_reset_irq_state(GICState *s, int first_cpu,
+                                                  int resetprio)
+{
+    int i, j;
+
+    for (i = first_cpu; i < first_cpu + s->num_cpu; i++) {
+        if (s->revision == REV_11MPCORE) {
+            s->priority_mask[i] = 0xf0;
+        } else {
+            s->priority_mask[i] = resetprio;
+        }
+        s->current_pending[i] = 1023;
+        s->running_priority[i] = 0x100;
+        s->cpu_ctlr[i] = 0;
+        s->bpr[i] = gic_is_vcpu(i) ? GIC_VIRT_MIN_BPR : GIC_MIN_BPR;
+        s->abpr[i] = gic_is_vcpu(i) ? GIC_VIRT_MIN_ABPR : GIC_MIN_ABPR;
+
+        if (!gic_is_vcpu(i)) {
+            for (j = 0; j < GIC_INTERNAL; j++) {
+                s->priority1[j][i] = resetprio;
+            }
+            for (j = 0; j < GIC_NR_SGIS; j++) {
+                s->sgi_pending[j][i] = 0;
+            }
+        }
+    }
 }
 
 static void arm_gic_common_reset(DeviceState *dev)
@@ -185,24 +266,14 @@ static void arm_gic_common_reset(DeviceState *dev)
     }
 
     memset(s->irq_state, 0, GIC_MAXIRQ * sizeof(gic_irq_state));
-    for (i = 0 ; i < s->num_cpu; i++) {
-        if (s->revision == REV_11MPCORE) {
-            s->priority_mask[i] = 0xf0;
-        } else {
-            s->priority_mask[i] = resetprio;
-        }
-        s->current_pending[i] = 1023;
-        s->running_priority[i] = 0x100;
-        s->cpu_ctlr[i] = 0;
-        s->bpr[i] = GIC_MIN_BPR;
-        s->abpr[i] = GIC_MIN_ABPR;
-        for (j = 0; j < GIC_INTERNAL; j++) {
-            s->priority1[j][i] = resetprio;
-        }
-        for (j = 0; j < GIC_NR_SGIS; j++) {
-            s->sgi_pending[j][i] = 0;
-        }
+    arm_gic_common_reset_irq_state(s, 0, resetprio);
+
+    if (s->virt_extn) {
+        /* vCPU states are stored at indexes GIC_NCPU .. GIC_NCPU+num_cpu.
+         * The exposed vCPU interface does not have security extensions. */
+        arm_gic_common_reset_irq_state(s, GIC_NCPU, 0);
     }
+
     for (i = 0; i < GIC_NR_SGIS; i++) {
         GIC_DIST_SET_ENABLED(i, ALL_CPU_MASK);
         GIC_DIST_SET_EDGE_TRIGGER(i);
@@ -226,6 +297,32 @@ static void arm_gic_common_reset(DeviceState *dev)
         }
     }
 
+    if (s->virt_extn) {
+        for (i = 0; i < GIC_MAXIRQ; i++) {
+            for (j = 0; j < s->num_cpu; j++) {
+                s->virq_lr_entry[i][j] = GIC_NR_LR;
+            }
+        }
+
+        for (i = 0; i < GIC_NR_LR; i++) {
+            for (j = 0; j < s->num_cpu; j++) {
+                s->h_lr[i][j] = 0;
+            }
+        }
+
+        for (i = 0; i < s->num_cpu; i++) {
+            s->h_hcr[i] = 0;
+            s->h_eisr[i] = 0;
+#if GIC_NR_LR == 64
+            s->h_elrsr[i] = UINT64_MAX;
+#else
+            /* Bits corresponding to non-implemented LRs in ELRSR are RAZ */
+            s->h_elrsr[i] = (1ull << GIC_NR_LR) - 1;
+#endif
+            s->pending_lrs[i] = 0;
+        }
+    }
+
     s->ctlr = 0;
 }
 
@@ -255,6 +352,8 @@ static Property arm_gic_common_properties[] = {
     DEFINE_PROP_UINT32("revision", GICState, revision, 1),
     /* True if the GIC should implement the security extensions */
     DEFINE_PROP_BOOL("has-security-extensions", GICState, security_extn, 0),
+    /* True if the GIC should implement the virtualization extensions */
+    DEFINE_PROP_BOOL("has-virtualization-extensions", GICState, virt_extn, 0),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/intc/arm_gic_kvm.c b/hw/intc/arm_gic_kvm.c
index 799136732a..6e562411cc 100644
--- a/hw/intc/arm_gic_kvm.c
+++ b/hw/intc/arm_gic_kvm.c
@@ -511,6 +511,12 @@ static void kvm_arm_gic_realize(DeviceState *dev, Error **errp)
         return;
     }
 
+    if (s->virt_extn) {
+        error_setg(errp, "the in-kernel VGIC does not implement the "
+                   "virtualization extensions");
+        return;
+    }
+
     if (!kvm_arm_gic_can_save_restore(s)) {
         error_setg(&s->migration_blocker, "This operating system kernel does "
                                           "not support vGICv2 migration");
@@ -522,7 +528,7 @@ static void kvm_arm_gic_realize(DeviceState *dev, Error **errp)
         }
     }
 
-    gic_init_irqs_and_mmio(s, kvm_arm_gicv2_set_irq, NULL);
+    gic_init_irqs_and_mmio(s, kvm_arm_gicv2_set_irq, NULL, NULL);
 
     for (i = 0; i < s->num_irq - GIC_INTERNAL; i++) {
         qemu_irq irq = qdev_get_gpio_in(dev, i);
diff --git a/hw/intc/gic_internal.h b/hw/intc/gic_internal.h
index a2075a94db..c85427c8e3 100644
--- a/hw/intc/gic_internal.h
+++ b/hw/intc/gic_internal.h
@@ -94,4 +94,9 @@ static inline bool gic_test_pending(GICState *s, int irq, int cm)
     }
 }
 
+static inline bool gic_is_vcpu(int cpu)
+{
+    return cpu >= GIC_NCPU;
+}
+
 #endif /* QEMU_ARM_GIC_INTERNAL_H */
diff --git a/include/hw/intc/arm_gic_common.h b/include/hw/intc/arm_gic_common.h
index af3ca18e2f..3f19ea3cb9 100644
--- a/include/hw/intc/arm_gic_common.h
+++ b/include/hw/intc/arm_gic_common.h
@@ -30,6 +30,8 @@
 #define GIC_NR_SGIS 16
 /* Maximum number of possible CPU interfaces, determined by GIC architecture */
 #define GIC_NCPU 8
+/* Maximum number of possible CPU interfaces with their respective vCPU */
+#define GIC_NCPU_VCPU (GIC_NCPU * 2)
 
 #define MAX_NR_GROUP_PRIO 128
 #define GIC_NR_APRS (MAX_NR_GROUP_PRIO / 32)
@@ -37,6 +39,17 @@
 #define GIC_MIN_BPR 0
 #define GIC_MIN_ABPR (GIC_MIN_BPR + 1)
 
+/* Number of list registers in the virtual interface */
+#define GIC_NR_LR 64
+
+/* Only 32 priority levels and 32 preemption levels in the vCPU interfaces */
+#define GIC_VIRT_MAX_GROUP_PRIO_BITS 5
+#define GIC_VIRT_MAX_NR_GROUP_PRIO (1 << GIC_VIRT_MAX_GROUP_PRIO_BITS)
+#define GIC_VIRT_NR_APRS (GIC_VIRT_MAX_NR_GROUP_PRIO / 32)
+
+#define GIC_VIRT_MIN_BPR 2
+#define GIC_VIRT_MIN_ABPR (GIC_VIRT_MIN_BPR + 1)
+
 typedef struct gic_irq_state {
     /* The enable bits are only banked for per-cpu interrupts.  */
     uint8_t enabled;
@@ -57,6 +70,8 @@ typedef struct GICState {
     qemu_irq parent_fiq[GIC_NCPU];
     qemu_irq parent_virq[GIC_NCPU];
     qemu_irq parent_vfiq[GIC_NCPU];
+    qemu_irq maintenance_irq[GIC_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.
      */
@@ -64,7 +79,7 @@ typedef struct GICState {
     /* 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[GIC_NCPU];
+    uint32_t cpu_ctlr[GIC_NCPU_VCPU];
 
     gic_irq_state irq_state[GIC_MAXIRQ];
     uint8_t irq_target[GIC_MAXIRQ];
@@ -78,9 +93,9 @@ typedef struct GICState {
      */
     uint8_t sgi_pending[GIC_NR_SGIS][GIC_NCPU];
 
-    uint16_t priority_mask[GIC_NCPU];
-    uint16_t running_priority[GIC_NCPU];
-    uint16_t current_pending[GIC_NCPU];
+    uint16_t priority_mask[GIC_NCPU_VCPU];
+    uint16_t running_priority[GIC_NCPU_VCPU];
+    uint16_t current_pending[GIC_NCPU_VCPU];
 
     /* If we present the GICv2 without security extensions to a guest,
      * the guest can configure the GICC_CTLR to configure group 1 binary point
@@ -88,8 +103,8 @@ typedef struct GICState {
      * For a GIC with Security Extensions we use use bpr for the
      * secure copy and abpr as storage for the non-secure copy of the register.
      */
-    uint8_t  bpr[GIC_NCPU];
-    uint8_t  abpr[GIC_NCPU];
+    uint8_t  bpr[GIC_NCPU_VCPU];
+    uint8_t  abpr[GIC_NCPU_VCPU];
 
     /* The APR is implementation defined, so we choose a layout identical to
      * the KVM ABI layout for QEMU's implementation of the gic:
@@ -97,9 +112,25 @@ typedef struct GICState {
      *   APRn[X mod 32] == 0b1,  where n = X / 32
      * otherwise the bit is clear.
      */
-    uint32_t apr[GIC_NR_APRS][GIC_NCPU];
+    uint32_t apr[GIC_NR_APRS][GIC_NCPU_VCPU];
     uint32_t nsapr[GIC_NR_APRS][GIC_NCPU];
 
+    /* Virtual interface control registers */
+    uint32_t h_hcr[GIC_NCPU];
+    uint32_t h_misr[GIC_NCPU];
+    uint32_t h_lr[GIC_NR_LR][GIC_NCPU];
+    uint64_t h_eisr[GIC_NCPU];
+    uint64_t h_elrsr[GIC_NCPU];
+    uint32_t h_apr[GIC_NCPU];
+
+    /* For a vIRQ, gives its LR entry number,
+     * or GIC_NR_LR if it has no entry. */
+    size_t virq_lr_entry[GIC_MAXIRQ][GIC_NCPU];
+
+    /* LR entries in the pending state. Used to compute NP maintenance
+     * interrupt */
+    uint64_t pending_lrs[GIC_NCPU];
+
     uint32_t num_cpu;
 
     MemoryRegion iomem; /* Distributor */
@@ -108,9 +139,13 @@ typedef struct GICState {
      */
     struct GICState *backref[GIC_NCPU];
     MemoryRegion cpuiomem[GIC_NCPU + 1]; /* CPU interfaces */
+    MemoryRegion vifaceiomem; /* Virtual interface */
+    MemoryRegion vcpuiomem[GIC_NCPU + 1]; /* vCPU interface */
+
     uint32_t num_irq;
     uint32_t revision;
     bool security_extn;
+    bool virt_extn;
     bool irq_reset_nonsecure; /* configure IRQs as group 1 (NS) on reset? */
     int dev_fd; /* kvm device fd if backed by kvm vgic support */
     Error *migration_blocker;
@@ -134,6 +169,7 @@ typedef struct ARMGICCommonClass {
 } ARMGICCommonClass;
 
 void gic_init_irqs_and_mmio(GICState *s, qemu_irq_handler handler,
-                            const MemoryRegionOps *ops);
+                            const MemoryRegionOps *ops,
+                            const MemoryRegionOps *virt_ops);
 
 #endif
-- 
2.17.0

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

* Re: [Qemu-devel] [PATCH 3/6] intc/arm_gic: Add the virtualization extensions to the GIC state
  2018-06-06  9:30 ` [Qemu-devel] [PATCH 3/6] intc/arm_gic: Add the virtualization extensions to the GIC state luc.michel
@ 2018-06-11 13:38   ` Peter Maydell
  2018-06-18 11:50     ` Luc Michel
  0 siblings, 1 reply; 17+ messages in thread
From: Peter Maydell @ 2018-06-11 13:38 UTC (permalink / raw)
  To: luc.michel
  Cc: QEMU Developers, qemu-arm, Sai Pavan Boddu, Edgar Iglesias,
	Mark Burton, Jan Kiszka

On 6 June 2018 at 10:30,  <luc.michel@greensocs.com> wrote:
> From: Luc MICHEL <luc.michel@greensocs.com>
>
> Add the necessary parts of the virtualization extensions state to the
> GIC state. We choose to increase the size of the CPU interfaces state to
> add space for the vCPU interfaces (the GIC_NCPU_VCPU macro). This way,
> we'll be able to reuse most of the CPU interface code for the vCPUs.
>
> The vCPUs are numbered from GIC_NCPU to (GIC_NCPU * 2) - 1. The
> `gic_is_vcpu` function help to determine if a given CPU id correspond to
> a physical CPU or a virtual one.
>
> The GIC VMState is updated to account for this modification. We add a
> subsection for the virtual interface state. The virtual CPU interfaces
> state however cannot be put in the subsection because of some 2D arrays
> in the GIC state. This implies an increase in the VMState version id.
>
> For the in-kernel KVM VGIC, since the exposed VGIC does not implement
> the virtualization extensions, we report an error if the corresponding
> property is set to true.
>
> Signed-off-by: Luc MICHEL <luc.michel@greensocs.com>
> ---

> +/* Note: We cannot put the vCPUs state in this subsection because of some 2D
> + * arrays that mix CPU and vCPU states. */
> +static const VMStateDescription vmstate_gic_virt_state = {
> +    .name = "arm_gic_virt_state",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .needed = gic_virt_state_needed,
> +    .fields = (VMStateField[]) {
> +        /* Virtual interface */
> +        VMSTATE_UINT32_ARRAY(h_hcr, GICState, GIC_NCPU),
> +        VMSTATE_UINT32_ARRAY(h_misr, GICState, GIC_NCPU),
> +        VMSTATE_UINT32_2DARRAY(h_lr, GICState, GIC_NR_LR, GIC_NCPU),
> +        VMSTATE_UINT64_ARRAY(h_eisr, GICState, GIC_NCPU),
> +        VMSTATE_UINT64_ARRAY(h_elrsr, GICState, GIC_NCPU),
> +        VMSTATE_UINT32_ARRAY(h_apr, GICState, GIC_NCPU),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
>  static const VMStateDescription vmstate_gic = {
>      .name = "arm_gic",
> -    .version_id = 12,
> -    .minimum_version_id = 12,
> +    .version_id = 13,
> +    .minimum_version_id = 13,

This breaks migration compatibility, which we can't do for the GIC
(it is used by some KVM VM configs, where we strongly care about
compatibility).

>      .pre_save = gic_pre_save,
>      .post_load = gic_post_load,
>      .fields = (VMStateField[]) {
>          VMSTATE_UINT32(ctlr, GICState),
> -        VMSTATE_UINT32_ARRAY(cpu_ctlr, GICState, GIC_NCPU),
> +        VMSTATE_UINT32_ARRAY(cpu_ctlr, GICState, GIC_NCPU_VCPU),

If you want to put the VCPU state in the same array as the CPU state
like this, you need to use subarrays, so here
           VMSTATE_UINT32_SUB_ARRAY(cpu_ctlr, GICState, 0, GIC_NCPU),
and in the vmstate_gic_virt_state
           VMSTATE_UINT32_SUB_ARRAY(cpu_ctlr, GICState, GIC_NCPU, GIC_NCPU)

Similarly for the other arrays. You'll need a patch adding
VMSTATE_UINT16_SUB_ARRAY to vmstate.h:

#define VMSTATE_UINT16_SUB_ARRAY(_f, _s, _start, _num)                \
    VMSTATE_SUB_ARRAY(_f, _s, _start, _num, 0, vmstate_info_uint16, uint16_t)

>          VMSTATE_STRUCT_ARRAY(irq_state, GICState, GIC_MAXIRQ, 1,
>                               vmstate_gic_irq_state, gic_irq_state),
>          VMSTATE_UINT8_ARRAY(irq_target, GICState, GIC_MAXIRQ),
>          VMSTATE_UINT8_2DARRAY(priority1, GICState, GIC_INTERNAL, GIC_NCPU),
>          VMSTATE_UINT8_ARRAY(priority2, GICState, GIC_MAXIRQ - GIC_INTERNAL),
>          VMSTATE_UINT8_2DARRAY(sgi_pending, GICState, GIC_NR_SGIS, GIC_NCPU),
> -        VMSTATE_UINT16_ARRAY(priority_mask, GICState, GIC_NCPU),
> -        VMSTATE_UINT16_ARRAY(running_priority, GICState, GIC_NCPU),
> -        VMSTATE_UINT16_ARRAY(current_pending, GICState, GIC_NCPU),
> -        VMSTATE_UINT8_ARRAY(bpr, GICState, GIC_NCPU),
> -        VMSTATE_UINT8_ARRAY(abpr, GICState, GIC_NCPU),
> -        VMSTATE_UINT32_2DARRAY(apr, GICState, GIC_NR_APRS, GIC_NCPU),
> +        VMSTATE_UINT16_ARRAY(priority_mask, GICState, GIC_NCPU_VCPU),
> +        VMSTATE_UINT16_ARRAY(running_priority, GICState, GIC_NCPU_VCPU),
> +        VMSTATE_UINT16_ARRAY(current_pending, GICState, GIC_NCPU_VCPU),
> +        VMSTATE_UINT8_ARRAY(bpr, GICState, GIC_NCPU_VCPU),
> +        VMSTATE_UINT8_ARRAY(abpr, GICState, GIC_NCPU_VCPU),
> +        VMSTATE_UINT32_2DARRAY(apr, GICState, GIC_NR_APRS, GIC_NCPU_VCPU),

The 2D array is more painful. You need to describe it as a set of
1D slices, something like
           VMSTATE_UINT32_SUB_ARRAY(apr, GICState, 0, GIC_NCPU),
           VMSTATE_UINT32_SUB_ARRAY(apr, GICState, GIC_NCPU_VCPU, GIC_NCPU),
           VMSTATE_UINT32_SUB_ARRAY(apr, GICState, GIC_NCPU_VCPU * 2, GIC_NCPU),
           [...]
           VMSTATE_UINT32_SUB_ARRAY(apr, GICState, GIC_NCPU_VCPU * n, GIC_NCPU),
where n is GIC_NR_APRS - 1

and then the other slices in the vmstate_gic_virt_state.

But conveniently the only 2D array is for the APR registers, which
you don't actually want to have state for for the virtualization
extensions anyway. The only state here is in the GICH_APR, and
(as per the recommendation in the spec in the GICV_APRn documentation)
the GICV_APRn should just be either RAZ/WI or aliases of GICH_APR,
with no state of their own to migrate.

More generally, there's something odd going on here. The way the
GIC virtualization extensions are defined, there are registers
which expose the state to the guest (the GIC virtual CPU interface),
and there are registers which expose the state to the hypervisor
(the GIC virtual interface), but the underlying state is identical.
In the spec all the various fields in the virtual CPU interface
are defined as aliases to register fields in the virtual interface
(for instance GICV_PMR is an alias of GICH_VMCR.VMPriMask).

So you don't want to be migrating the data twice -- depending on
the implementation we either hold the state in struct fields that
look like the CPU interface, and the virtual interface register
read and write code does the mapping to access the right parts of
those, or we can do it the other way round and store the state in
struct fields matching the virtual interface registers, with the
read and write functions for the virtual CPU interface doing the
mapping. But we don't want struct fields and migration state
descriptions for both.

>          VMSTATE_UINT32_2DARRAY(nsapr, GICState, GIC_NR_APRS, GIC_NCPU),
>          VMSTATE_END_OF_LIST()
> +    },
> +    .subsections = (const VMStateDescription * []) {
> +        &vmstate_gic_virt_state,
> +        NULL
>      }
>  };

thanks
-- PMM

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

* Re: [Qemu-devel] [PATCH 3/6] intc/arm_gic: Add the virtualization extensions to the GIC state
  2018-06-11 13:38   ` Peter Maydell
@ 2018-06-18 11:50     ` Luc Michel
  2018-06-18 12:11       ` Peter Maydell
  0 siblings, 1 reply; 17+ messages in thread
From: Luc Michel @ 2018-06-18 11:50 UTC (permalink / raw)
  To: Peter Maydell
  Cc: QEMU Developers, qemu-arm, Sai Pavan Boddu, Edgar Iglesias,
	Mark Burton, Jan Kiszka

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

On 06/11/2018 03:38 PM, Peter Maydell wrote:
> On 6 June 2018 at 10:30,  <luc.michel@greensocs.com> wrote:
>> From: Luc MICHEL <luc.michel@greensocs.com>
>>
>> Add the necessary parts of the virtualization extensions state to the
>> GIC state. We choose to increase the size of the CPU interfaces state to
>> add space for the vCPU interfaces (the GIC_NCPU_VCPU macro). This way,
>> we'll be able to reuse most of the CPU interface code for the vCPUs.
>>
>> The vCPUs are numbered from GIC_NCPU to (GIC_NCPU * 2) - 1. The
>> `gic_is_vcpu` function help to determine if a given CPU id correspond to
>> a physical CPU or a virtual one.
>>
>> The GIC VMState is updated to account for this modification. We add a
>> subsection for the virtual interface state. The virtual CPU interfaces
>> state however cannot be put in the subsection because of some 2D arrays
>> in the GIC state. This implies an increase in the VMState version id.
>>
>> For the in-kernel KVM VGIC, since the exposed VGIC does not implement
>> the virtualization extensions, we report an error if the corresponding
>> property is set to true.
>>
>> Signed-off-by: Luc MICHEL <luc.michel@greensocs.com>
>> ---
> 
>> +/* Note: We cannot put the vCPUs state in this subsection because of some 2D
>> + * arrays that mix CPU and vCPU states. */
>> +static const VMStateDescription vmstate_gic_virt_state = {
>> +    .name = "arm_gic_virt_state",
>> +    .version_id = 1,
>> +    .minimum_version_id = 1,
>> +    .needed = gic_virt_state_needed,
>> +    .fields = (VMStateField[]) {
>> +        /* Virtual interface */
>> +        VMSTATE_UINT32_ARRAY(h_hcr, GICState, GIC_NCPU),
>> +        VMSTATE_UINT32_ARRAY(h_misr, GICState, GIC_NCPU),
>> +        VMSTATE_UINT32_2DARRAY(h_lr, GICState, GIC_NR_LR, GIC_NCPU),
>> +        VMSTATE_UINT64_ARRAY(h_eisr, GICState, GIC_NCPU),
>> +        VMSTATE_UINT64_ARRAY(h_elrsr, GICState, GIC_NCPU),
>> +        VMSTATE_UINT32_ARRAY(h_apr, GICState, GIC_NCPU),
>> +        VMSTATE_END_OF_LIST()
>> +    }
>> +};
>> +
>>  static const VMStateDescription vmstate_gic = {
>>      .name = "arm_gic",
>> -    .version_id = 12,
>> -    .minimum_version_id = 12,
>> +    .version_id = 13,
>> +    .minimum_version_id = 13,
> 
> This breaks migration compatibility, which we can't do for the GIC
> (it is used by some KVM VM configs, where we strongly care about
> compatibility).
> 
>>      .pre_save = gic_pre_save,
>>      .post_load = gic_post_load,
>>      .fields = (VMStateField[]) {
>>          VMSTATE_UINT32(ctlr, GICState),
>> -        VMSTATE_UINT32_ARRAY(cpu_ctlr, GICState, GIC_NCPU),
>> +        VMSTATE_UINT32_ARRAY(cpu_ctlr, GICState, GIC_NCPU_VCPU),
> 
> If you want to put the VCPU state in the same array as the CPU state
> like this, you need to use subarrays, so here
>            VMSTATE_UINT32_SUB_ARRAY(cpu_ctlr, GICState, 0, GIC_NCPU),
> and in the vmstate_gic_virt_state
>            VMSTATE_UINT32_SUB_ARRAY(cpu_ctlr, GICState, GIC_NCPU, GIC_NCPU)
> 
> Similarly for the other arrays. You'll need a patch adding
> VMSTATE_UINT16_SUB_ARRAY to vmstate.h:
> 
> #define VMSTATE_UINT16_SUB_ARRAY(_f, _s, _start, _num)                \
>     VMSTATE_SUB_ARRAY(_f, _s, _start, _num, 0, vmstate_info_uint16, uint16_t)
> 
>>          VMSTATE_STRUCT_ARRAY(irq_state, GICState, GIC_MAXIRQ, 1,
>>                               vmstate_gic_irq_state, gic_irq_state),
>>          VMSTATE_UINT8_ARRAY(irq_target, GICState, GIC_MAXIRQ),
>>          VMSTATE_UINT8_2DARRAY(priority1, GICState, GIC_INTERNAL, GIC_NCPU),
>>          VMSTATE_UINT8_ARRAY(priority2, GICState, GIC_MAXIRQ - GIC_INTERNAL),
>>          VMSTATE_UINT8_2DARRAY(sgi_pending, GICState, GIC_NR_SGIS, GIC_NCPU),
>> -        VMSTATE_UINT16_ARRAY(priority_mask, GICState, GIC_NCPU),
>> -        VMSTATE_UINT16_ARRAY(running_priority, GICState, GIC_NCPU),
>> -        VMSTATE_UINT16_ARRAY(current_pending, GICState, GIC_NCPU),
>> -        VMSTATE_UINT8_ARRAY(bpr, GICState, GIC_NCPU),
>> -        VMSTATE_UINT8_ARRAY(abpr, GICState, GIC_NCPU),
>> -        VMSTATE_UINT32_2DARRAY(apr, GICState, GIC_NR_APRS, GIC_NCPU),
>> +        VMSTATE_UINT16_ARRAY(priority_mask, GICState, GIC_NCPU_VCPU),
>> +        VMSTATE_UINT16_ARRAY(running_priority, GICState, GIC_NCPU_VCPU),
>> +        VMSTATE_UINT16_ARRAY(current_pending, GICState, GIC_NCPU_VCPU),
>> +        VMSTATE_UINT8_ARRAY(bpr, GICState, GIC_NCPU_VCPU),
>> +        VMSTATE_UINT8_ARRAY(abpr, GICState, GIC_NCPU_VCPU),
>> +        VMSTATE_UINT32_2DARRAY(apr, GICState, GIC_NR_APRS, GIC_NCPU_VCPU),
> 
> The 2D array is more painful. You need to describe it as a set of
> 1D slices, something like
>            VMSTATE_UINT32_SUB_ARRAY(apr, GICState, 0, GIC_NCPU),
>            VMSTATE_UINT32_SUB_ARRAY(apr, GICState, GIC_NCPU_VCPU, GIC_NCPU),
>            VMSTATE_UINT32_SUB_ARRAY(apr, GICState, GIC_NCPU_VCPU * 2, GIC_NCPU),
>            [...]
>            VMSTATE_UINT32_SUB_ARRAY(apr, GICState, GIC_NCPU_VCPU * n, GIC_NCPU),
> where n is GIC_NR_APRS - 1
> 
> and then the other slices in the vmstate_gic_virt_state.
> 
> But conveniently the only 2D array is for the APR registers, which
> you don't actually want to have state for for the virtualization
> extensions anyway. The only state here is in the GICH_APR, and
> (as per the recommendation in the spec in the GICV_APRn documentation)
> the GICV_APRn should just be either RAZ/WI or aliases of GICH_APR,
> with no state of their own to migrate.
> 
> More generally, there's something odd going on here. The way the
> GIC virtualization extensions are defined, there are registers
> which expose the state to the guest (the GIC virtual CPU interface),
> and there are registers which expose the state to the hypervisor
> (the GIC virtual interface), but the underlying state is identical.
> In the spec all the various fields in the virtual CPU interface
> are defined as aliases to register fields in the virtual interface
> (for instance GICV_PMR is an alias of GICH_VMCR.VMPriMask).
> 
> So you don't want to be migrating the data twice -- depending on
> the implementation we either hold the state in struct fields that
> look like the CPU interface, and the virtual interface register
> read and write code does the mapping to access the right parts of
> those, or we can do it the other way round and store the state in
> struct fields matching the virtual interface registers, with the
> read and write functions for the virtual CPU interface doing the
> mapping. But we don't want struct fields and migration state
> descriptions for both.
Indeed you are right, h_apr is a leftover that is not used in my
implementation. I use apr[0] of the vCPU interface to store the APR
value. If it's ok like this, I can simply remove h_apr from the struct
and from the virt VMState.

The same goes for h_eisr and h_elrsr, which are in the struct as a
cached data, but that can be recomputed from the LRs, and thus don't
need to be stored in the VMState. In my implementation they are
recomputed in the VMState post_load function.

If you agree with that, I'm going to publish a v2 with migration
compatibility and without those three registers in the VMState.

Thanks,

-- 
Luc

> 
>>          VMSTATE_UINT32_2DARRAY(nsapr, GICState, GIC_NR_APRS, GIC_NCPU),
>>          VMSTATE_END_OF_LIST()
>> +    },
>> +    .subsections = (const VMStateDescription * []) {
>> +        &vmstate_gic_virt_state,
>> +        NULL
>>      }
>>  };
> 
> thanks
> -- PMM
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [Qemu-devel] [PATCH 3/6] intc/arm_gic: Add the virtualization extensions to the GIC state
  2018-06-18 11:50     ` Luc Michel
@ 2018-06-18 12:11       ` Peter Maydell
  0 siblings, 0 replies; 17+ messages in thread
From: Peter Maydell @ 2018-06-18 12:11 UTC (permalink / raw)
  To: Luc Michel
  Cc: QEMU Developers, qemu-arm, Sai Pavan Boddu, Edgar Iglesias,
	Mark Burton, Jan Kiszka

On 18 June 2018 at 12:50, Luc Michel <luc.michel@greensocs.com> wrote:
> Indeed you are right, h_apr is a leftover that is not used in my
> implementation. I use apr[0] of the vCPU interface to store the APR
> value. If it's ok like this, I can simply remove h_apr from the struct
> and from the virt VMState.
>
> The same goes for h_eisr and h_elrsr, which are in the struct as a
> cached data, but that can be recomputed from the LRs, and thus don't
> need to be stored in the VMState. In my implementation they are
> recomputed in the VMState post_load function.
>
> If you agree with that, I'm going to publish a v2 with migration
> compatibility and without those three registers in the VMState.

Sounds fine to me (though keeping the APR in particular in the
same array as the physical APR values is the ugliest part of
maintaining migration compat so in that case I think it's worth
at least considering whether having the code look in h_apr
would be nicer overall).

Where struct fields are just cached-data, a brief comment in
the struct definition to that effect is helpful.

thanks
-- PMM

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

* [Qemu-devel] [PATCH 4/6] intc/arm_gic: Add virtualization extensions logic
  2018-06-06  9:30 [Qemu-devel] [PATCH 0/6] arm_gic: add virtualization extensions support luc.michel
                   ` (2 preceding siblings ...)
  2018-06-06  9:30 ` [Qemu-devel] [PATCH 3/6] intc/arm_gic: Add the virtualization extensions to the GIC state luc.michel
@ 2018-06-06  9:30 ` luc.michel
  2018-06-06  9:31 ` [Qemu-devel] [PATCH 5/6] intc/arm_gic: Improve traces luc.michel
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 17+ messages in thread
From: luc.michel @ 2018-06-06  9:30 UTC (permalink / raw)
  To: qemu-devel
  Cc: Luc MICHEL, qemu-arm, Peter Maydell, saipava, edgari, mark.burton,
	Jan Kiszka

From: Luc MICHEL <luc.michel@greensocs.com>

This commit adds the actual implementation of the GICv2 virtualization
extensions logic.

For the vCPU interfaces, most of the existing CPU interface code is
reused. Calls to macros or functions modifying the distributor state are
replaced with equivalent generic inline functions that act either on the
distributor or on the virtual interface, given that the current CPU is a
physical or a virtual one. For vCPU MMIO, the call is simply forwarded
to the CPU read/write functions, with the CPU id being incremented by
GIC_CPU_NR (vCPU id = CPU id + GIC_CPU_NR), and by faking a secure
access (exposed vCPU interfaces do not implement security extensions.
Faking a secure access allows to go through the "secure or no security
extension" logic in the CPU interface).

The gic_update logic is generalized to handle vCPUs. The main difference
is the way the best IRQ is selected. This part has been split into two
inline functions `gic_get_best_(v)irq`. The other difference is how we
determine if IRQ distribution is enabled or not (split in
`gic_dist_enabled`).

Regarding list registers (LR) handling, some caching is done in the GIC
state to avoid repetitive LR iteration. The following information is
cached:
  * vIRQ to LR mapping, in s->virq_lr_entry,
  * the EISR and ELRSR registers,
  * the LRs in the pending state, in s->pending_lrs.
This cache is kept up-to-date with some maintenance functions, and is
recomputed when the GIC state is loaded from a VMState restoration.

Signed-off-by: Luc MICHEL <luc.michel@greensocs.com>
---
 hw/intc/arm_gic.c      | 626 +++++++++++++++++++++++++++++++++++------
 hw/intc/gic_internal.h | 198 +++++++++++++
 2 files changed, 734 insertions(+), 90 deletions(-)

diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
index e96f195997..6105f930c4 100644
--- a/hw/intc/arm_gic.c
+++ b/hw/intc/arm_gic.c
@@ -61,6 +61,11 @@ static inline int gic_get_current_cpu(GICState *s)
     return 0;
 }
 
+static inline int gic_get_current_vcpu(GICState *s)
+{
+    return gic_get_current_cpu(s) + GIC_NCPU;
+}
+
 /* Return true if this GIC config has interrupt groups, which is
  * true if we're a GICv2, or a GICv1 with the security extensions.
  */
@@ -69,74 +74,215 @@ static inline bool gic_has_groups(GICState *s)
     return s->revision == 2 || s->security_extn;
 }
 
+static inline void gic_get_best_irq(GICState *s, int cpu,
+                                    int *best_irq, int *best_prio, int *group)
+{
+    int irq;
+    int cm = 1 << cpu;
+
+    *best_irq = 1023;
+    *best_prio = 0x100;
+
+    for (irq = 0; irq < s->num_irq; irq++) {
+        if (GIC_DIST_TEST_ENABLED(irq, cm) && gic_test_pending(s, irq, cm) &&
+            (!GIC_DIST_TEST_ACTIVE(irq, cm)) &&
+            (irq < GIC_INTERNAL || GIC_DIST_TARGET(irq) & cm)) {
+            if (GIC_DIST_GET_PRIORITY(irq, cpu) < *best_prio) {
+                *best_prio = GIC_DIST_GET_PRIORITY(irq, cpu);
+                *best_irq = irq;
+            }
+        }
+    }
+
+    if (*best_irq < 1023) {
+        *group = GIC_DIST_TEST_GROUP(*best_irq, cm);
+    }
+}
+
+static inline void gic_get_best_virq(GICState *s, int cpu,
+                                     int *best_irq, int *best_prio, int *group)
+{
+    int lr = 0;
+    uint64_t lrs_in_use;
+
+    /* In-use LRs have their corresponding bit cleared. */
+    lrs_in_use = ~(s->h_elrsr[cpu]);
+
+#if GIC_NR_LR < 64
+    /* Ignore unimplemented LRs */
+    lrs_in_use &= (1 << GIC_NR_LR) - 1;
+#endif
+
+    *best_irq = 1023;
+    *best_prio = 0x100;
+
+    while (lrs_in_use) {
+        if (lrs_in_use & 0x1) {
+            uint32_t lr_entry = s->h_lr[lr][cpu];
+            int state = GICH_LR_STATE(lr_entry);
+
+            if (state == GICH_LR_STATE_PENDING) {
+                int prio = GICH_LR_PRIORITY(lr_entry);
+
+                if (prio < *best_prio) {
+                    *best_prio = prio;
+                    *best_irq = GICH_LR_VIRT_ID(lr_entry);
+                    *group = GICH_LR_GROUP(lr_entry);
+                }
+            }
+        }
+
+        lrs_in_use >>= 1;
+        lr++;
+    }
+}
+
+/* Return true if IRQ distribution is enabled:
+ *      - in the distributor, for the given group mask if virt if false,
+ *      - in the given CPU virtual interface if virt is true. */
+static inline bool gic_dist_enabled(GICState *s, int cpu, bool virt,
+                                    int group_mask)
+{
+    return (virt && (s->h_hcr[cpu] & GICH_HCR_EN))
+        || (!virt && (s->ctlr & group_mask));
+}
+
 /* TODO: Many places that call this routine could be optimized.  */
 /* Update interrupt status after enabled or pending bits have been changed.  */
-static void gic_update(GICState *s)
+static inline void gic_update_internal(GICState *s, bool virt)
 {
     int best_irq;
     int best_prio;
-    int irq;
     int irq_level, fiq_level;
-    int cpu;
-    int cm;
+    int cpu, cpu_iface;
+    int group = 0;
+    qemu_irq *irq_lines = virt ? s->parent_virq : s->parent_irq;
+    qemu_irq *fiq_lines = virt ? s->parent_vfiq : s->parent_fiq;
 
     for (cpu = 0; cpu < s->num_cpu; cpu++) {
-        cm = 1 << cpu;
-        s->current_pending[cpu] = 1023;
-        if (!(s->ctlr & (GICD_CTLR_EN_GRP0 | GICD_CTLR_EN_GRP1))
-            || !(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]);
+        cpu_iface = virt ? (cpu + GIC_NCPU) : cpu;
+
+        s->current_pending[cpu_iface] = 1023;
+        if (!gic_dist_enabled(s, cpu, virt,
+                              GICD_CTLR_EN_GRP0 | GICD_CTLR_EN_GRP1)
+            || !(s->cpu_ctlr[cpu_iface] &
+                 (GICC_CTLR_EN_GRP0 | GICC_CTLR_EN_GRP1))) {
+            qemu_irq_lower(irq_lines[cpu]);
+            qemu_irq_lower(fiq_lines[cpu]);
             continue;
         }
-        best_prio = 0x100;
-        best_irq = 1023;
-        for (irq = 0; irq < s->num_irq; irq++) {
-            if (GIC_DIST_TEST_ENABLED(irq, cm) &&
-                gic_test_pending(s, irq, cm) &&
-                (!GIC_DIST_TEST_ACTIVE(irq, cm)) &&
-                (irq < GIC_INTERNAL || GIC_DIST_TARGET(irq) & cm)) {
-                if (GIC_DIST_GET_PRIORITY(irq, cpu) < best_prio) {
-                    best_prio = GIC_DIST_GET_PRIORITY(irq, cpu);
-                    best_irq = irq;
-                }
-            }
+
+        if (virt) {
+            gic_get_best_virq(s, cpu, &best_irq, &best_prio, &group);
+        } else {
+            gic_get_best_irq(s, cpu, &best_irq, &best_prio, &group);
         }
 
         if (best_irq != 1023) {
             trace_gic_update_bestirq(cpu, best_irq, best_prio,
-                s->priority_mask[cpu], s->running_priority[cpu]);
+                s->priority_mask[cpu_iface], s->running_priority[cpu_iface]);
         }
 
         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_DIST_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) {
+        if (best_prio < s->priority_mask[cpu_iface]) {
+            s->current_pending[cpu_iface] = best_irq;
+            if (best_prio < s->running_priority[cpu_iface]) {
+                if (gic_dist_enabled(s, cpu, virt, 1 << group) &&
+                    extract32(s->cpu_ctlr[cpu_iface], group, 1)) {
+                    if (group == 0 &&
+                        s->cpu_ctlr[cpu_iface] & GICC_CTLR_FIQ_EN) {
                         DPRINTF("Raised pending FIQ %d (cpu %d)\n",
-                                best_irq, cpu);
+                                best_irq, cpu_iface);
                         fiq_level = 1;
-                        trace_gic_update_set_irq(cpu, "fiq", fiq_level);
+                        trace_gic_update_set_irq(cpu, virt ? "vfiq" : "fiq",
+                                                 fiq_level);
                     } else {
                         DPRINTF("Raised pending IRQ %d (cpu %d)\n",
-                                best_irq, cpu);
+                                best_irq, cpu_iface);
                         irq_level = 1;
-                        trace_gic_update_set_irq(cpu, "irq", irq_level);
+                        trace_gic_update_set_irq(cpu, virt ? "virq" : "irq",
+                                                 irq_level);
                     }
                 }
             }
         }
 
-        qemu_set_irq(s->parent_irq[cpu], irq_level);
-        qemu_set_irq(s->parent_fiq[cpu], fiq_level);
+        qemu_set_irq(irq_lines[cpu], irq_level);
+        qemu_set_irq(fiq_lines[cpu], fiq_level);
     }
 }
 
+static void gic_compute_misr(GICState *s, int cpu)
+{
+    int val;
+    int vcpu = cpu + GIC_NCPU;
+
+    /* EOI */
+    val = s->h_eisr[cpu] != 0;
+    s->h_misr[cpu] = val << 0;
+
+    /* U: true if only 0 or 1 LR entry is valid */
+    val = s->h_hcr[cpu] & GICH_HCR_UIE &&
+        ((GIC_NR_LR - ctpop64(s->h_elrsr[cpu])) < 2);
+    s->h_misr[cpu] |= val << 1;
+
+    /* LRENP: EOICount is not 0 */
+    val = s->h_hcr[cpu] & GICH_HCR_LRENPIE &&
+        ((s->h_hcr[cpu] & GICH_HCR_EOICOUNT) != 0);
+    s->h_misr[cpu] |= val << 2;
+
+    /* NP: no pending interrupts. The number of LRs in pending state is cached
+     * in s->pending_lrs[cpu]. */
+    val = s->h_hcr[cpu] & GICH_HCR_NPIE &&
+        (s->pending_lrs[cpu] == 0);
+    s->h_misr[cpu] |= val << 3;
+
+    /* VGrp0E: group0 virq signaling enabled */
+    val = s->h_hcr[cpu] & GICH_HCR_VGRP0EIE &&
+        (s->cpu_ctlr[vcpu] & GICC_CTLR_EN_GRP0);
+    s->h_misr[cpu] |= val << 4;
+
+    /* VGrp0D: group0 virq signaling disabled */
+    val = s->h_hcr[cpu] & GICH_HCR_VGRP0DIE &&
+        !(s->cpu_ctlr[vcpu] & GICC_CTLR_EN_GRP0);
+    s->h_misr[cpu] |= val << 5;
+
+    /* VGrp1E: group1 virq signaling enabled */
+    val = s->h_hcr[cpu] & GICH_HCR_VGRP1EIE &&
+        (s->cpu_ctlr[vcpu] & GICC_CTLR_EN_GRP1);
+    s->h_misr[cpu] |= val << 6;
+
+    /* VGrp1D: group1 virq signaling disabled */
+    val = s->h_hcr[cpu] & GICH_HCR_VGRP1DIE &&
+        !(s->cpu_ctlr[vcpu] & GICC_CTLR_EN_GRP1);
+    s->h_misr[cpu] |= val << 7;
+}
+
+static void gic_update_maintenance(GICState *s)
+{
+    int cpu = 0;
+    int maint_level;
+
+    for (cpu = 0; cpu < s->num_cpu; cpu++) {
+        gic_compute_misr(s, cpu);
+        maint_level = (s->h_hcr[cpu] & GICH_HCR_EN) && s->h_misr[cpu];
+
+        qemu_set_irq(s->maintenance_irq[cpu], maint_level);
+    }
+}
+
+static void gic_update(GICState *s)
+{
+    gic_update_internal(s, false);
+}
+
+static void gic_update_virt(GICState *s)
+{
+    gic_update_internal(s, true);
+    gic_update_maintenance(s);
+}
+
 static void gic_set_irq_11mpcore(GICState *s, int irq, int level,
                                  int cm, int target)
 {
@@ -212,7 +358,8 @@ static uint16_t gic_get_current_pending_irq(GICState *s, int cpu,
     uint16_t pending_irq = s->current_pending[cpu];
 
     if (pending_irq < GIC_MAXIRQ && gic_has_groups(s)) {
-        int group = GIC_DIST_TEST_GROUP(pending_irq, (1 << cpu));
+        int group = gic_test_group(s, pending_irq, cpu);
+
         /* On a GIC without the security extensions, reading this register
          * behaves in the same way as a secure access to a GIC with them.
          */
@@ -243,7 +390,7 @@ static int gic_get_group_priority(GICState *s, int cpu, int irq)
 
     if (gic_has_groups(s) &&
         !(s->cpu_ctlr[cpu] & GICC_CTLR_CBPR) &&
-        GIC_DIST_TEST_GROUP(irq, (1 << cpu))) {
+        gic_test_group(s, irq, cpu)) {
         bpr = s->abpr[cpu] - 1;
         assert(bpr >= 0);
     } else {
@@ -256,7 +403,7 @@ static int gic_get_group_priority(GICState *s, int cpu, int irq)
      */
     mask = ~0U << ((bpr & 7) + 1);
 
-    return GIC_DIST_GET_PRIORITY(irq, cpu) & mask;
+    return gic_get_priority(s, irq, cpu) & mask;
 }
 
 static void gic_activate_irq(GICState *s, int cpu, int irq)
@@ -265,18 +412,19 @@ static void gic_activate_irq(GICState *s, int cpu, int irq)
      * and update the running priority.
      */
     int prio = gic_get_group_priority(s, cpu, irq);
-    int preemption_level = prio >> (GIC_MIN_BPR + 1);
+    int min_bpr = gic_is_vcpu(cpu) ? GIC_VIRT_MIN_BPR : GIC_MIN_BPR;
+    int preemption_level = prio >> (min_bpr + 1);
     int regno = preemption_level / 32;
     int bitno = preemption_level % 32;
 
-    if (gic_has_groups(s) && GIC_DIST_TEST_GROUP(irq, (1 << cpu))) {
+    if (!gic_is_vcpu(cpu) && gic_has_groups(s) && gic_test_group(s, irq, cpu)) {
         s->nsapr[regno][cpu] |= (1 << bitno);
     } else {
         s->apr[regno][cpu] |= (1 << bitno);
     }
 
     s->running_priority[cpu] = prio;
-    GIC_DIST_SET_ACTIVE(irq, 1 << cpu);
+    gic_set_active(s, irq, cpu);
 }
 
 static int gic_get_prio_from_apr_bits(GICState *s, int cpu)
@@ -285,12 +433,20 @@ static int gic_get_prio_from_apr_bits(GICState *s, int cpu)
      * on the set bits in the Active Priority Registers.
      */
     int i;
-    for (i = 0; i < GIC_NR_APRS; i++) {
-        uint32_t apr = s->apr[i][cpu] | s->nsapr[i][cpu];
+    int min_bpr = gic_is_vcpu(cpu) ? GIC_VIRT_MIN_BPR : GIC_MIN_BPR;
+    int nr_aprs = gic_is_vcpu(cpu) ? GIC_VIRT_NR_APRS : GIC_NR_APRS;
+
+    for (i = 0; i < nr_aprs; i++) {
+        uint32_t apr = s->apr[i][cpu];
+
+        if (!gic_is_vcpu(cpu)) {
+            apr |= s->nsapr[i][cpu];
+        }
+
         if (!apr) {
             continue;
         }
-        return (i * 32 + ctz32(apr)) << (GIC_MIN_BPR + 1);
+        return (i * 32 + ctz32(apr)) << (min_bpr + 1);
     }
     return 0x100;
 }
@@ -314,9 +470,12 @@ static void gic_drop_prio(GICState *s, int cpu, int group)
      * might not do so, and interrupts that should not preempt might do so.
      */
     int i;
+    int nr_aprs = gic_is_vcpu(cpu) ? GIC_VIRT_NR_APRS : GIC_NR_APRS;
+
+    for (i = 0; i < nr_aprs; i++) {
+        uint32_t *papr = (!gic_is_vcpu(cpu) && group) ?
+            &s->nsapr[i][cpu] : &s->apr[i][cpu];
 
-    for (i = 0; i < GIC_NR_APRS; i++) {
-        uint32_t *papr = group ? &s->nsapr[i][cpu] : &s->apr[i][cpu];
         if (!*papr) {
             continue;
         }
@@ -328,24 +487,51 @@ static void gic_drop_prio(GICState *s, int cpu, int group)
     s->running_priority[cpu] = gic_get_prio_from_apr_bits(s, cpu);
 }
 
+static inline uint32_t gic_clear_pending_sgi(GICState *s, int irq, int cpu)
+{
+    int src;
+    uint32_t ret;
+
+    if (!gic_is_vcpu(cpu)) {
+        /* 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_pending[irq][cpu] != 0);
+        src = ctz32(s->sgi_pending[irq][cpu]);
+        s->sgi_pending[irq][cpu] &= ~(1 << src);
+        if (s->sgi_pending[irq][cpu] == 0) {
+            gic_clear_pending(s, irq, cpu);
+        }
+        ret = irq | ((src & 0x7) << 10);
+    } else {
+        uint32_t *lr_entry = gic_get_lr_entry(s, irq, cpu);
+        src = GICH_LR_CPUID(*lr_entry);
+
+        gic_clear_pending(s, irq, cpu);
+        ret = irq | (src << 10);
+    }
+
+    return ret;
+}
+
 uint32_t gic_acknowledge_irq(GICState *s, int cpu, MemTxAttrs attrs)
 {
-    int ret, irq, src;
-    int cm = 1 << cpu;
+    int ret, irq;
 
     /* 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);
-    trace_gic_acknowledge_irq(cpu, irq);
+    trace_gic_acknowledge_irq(gic_get_vcpu_real_id(cpu), irq);
 
     if (irq >= GIC_MAXIRQ) {
         DPRINTF("ACK, no pending interrupt or it is hidden: %d\n", irq);
         return irq;
     }
 
-    if (GIC_DIST_GET_PRIORITY(irq, cpu) >= s->running_priority[cpu]) {
+    if (gic_get_priority(s, irq, cpu) >= s->running_priority[cpu]) {
         DPRINTF("ACK, pending interrupt (%d) has insufficient priority\n", irq);
         return 1023;
     }
@@ -354,37 +540,23 @@ uint32_t gic_acknowledge_irq(GICState *s, int cpu, MemTxAttrs attrs)
         /* Clear pending flags for both level and edge triggered interrupts.
          * Level triggered IRQs will be reasserted once they become inactive.
          */
-        GIC_DIST_CLEAR_PENDING(irq, GIC_DIST_TEST_MODEL(irq) ? ALL_CPU_MASK
-                                                             : cm);
+        gic_clear_pending(s, irq, cpu);
         ret = irq;
     } else {
         if (irq < GIC_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_pending[irq][cpu] != 0);
-            src = ctz32(s->sgi_pending[irq][cpu]);
-            s->sgi_pending[irq][cpu] &= ~(1 << src);
-            if (s->sgi_pending[irq][cpu] == 0) {
-                GIC_DIST_CLEAR_PENDING(irq,
-                                       GIC_DIST_TEST_MODEL(irq) ? ALL_CPU_MASK
-                                                                : cm);
-            }
-            ret = irq | ((src & 0x7) << 10);
+            ret = gic_clear_pending_sgi(s, irq, cpu);
         } else {
-            /* Clear pending state for both level and edge triggered
-             * interrupts. (level triggered interrupts with an active line
-             * remain pending, see gic_test_pending)
-             */
-            GIC_DIST_CLEAR_PENDING(irq, GIC_DIST_TEST_MODEL(irq) ? ALL_CPU_MASK
-                                                                 : cm);
+            gic_clear_pending(s, irq, cpu);
             ret = irq;
         }
     }
 
     gic_activate_irq(s, cpu, irq);
-    gic_update(s);
+    if (gic_is_vcpu(cpu)) {
+        gic_update_virt(s);
+    } else {
+        gic_update(s);
+    }
     DPRINTF("ACK %d\n", irq);
     return ret;
 }
@@ -534,8 +706,7 @@ static bool gic_eoi_split(GICState *s, int cpu, MemTxAttrs attrs)
 
 static void gic_deactivate_irq(GICState *s, int cpu, int irq, MemTxAttrs attrs)
 {
-    int cm = 1 << cpu;
-    int group = gic_has_groups(s) && GIC_DIST_TEST_GROUP(irq, cm);
+    int group = gic_has_groups(s) && gic_test_group(s, irq, cpu);
 
     if (!gic_eoi_split(s, cpu, attrs)) {
         /* This is UNPREDICTABLE; we choose to ignore it */
@@ -549,7 +720,7 @@ static void gic_deactivate_irq(GICState *s, int cpu, int irq, MemTxAttrs attrs)
         return;
     }
 
-    GIC_DIST_CLEAR_ACTIVE(irq, cm);
+    gic_clear_active(s, irq, cpu);
 }
 
 static void gic_complete_irq(GICState *s, int cpu, int irq, MemTxAttrs attrs)
@@ -558,6 +729,20 @@ static void gic_complete_irq(GICState *s, int cpu, int irq, MemTxAttrs attrs)
     int group;
 
     DPRINTF("EOI %d\n", irq);
+    if (gic_is_vcpu(cpu) && !gic_virq_is_valid(s, irq, cpu)) {
+        /* This vIRQ does not have a valid LR entry. Increment EOICount and
+         * ignore the write. */
+
+        int rcpu = gic_get_vcpu_real_id(cpu);
+        int eoicount = extract32(s->h_hcr[rcpu], 27, 5) + 1;
+
+        /* This 5 bits counter wraps to 0 */
+        eoicount &= 0x1f;
+
+        s->h_hcr[rcpu] = deposit32(s->h_hcr[rcpu], 27, 5, eoicount);
+        return;
+    }
+
     if (irq >= s->num_irq) {
         /* This handles two cases:
          * 1. If software writes the ID of a spurious interrupt [ie 1023]
@@ -584,7 +769,7 @@ static void gic_complete_irq(GICState *s, int cpu, int irq, MemTxAttrs attrs)
         }
     }
 
-    group = gic_has_groups(s) && GIC_DIST_TEST_GROUP(irq, cm);
+    group = gic_has_groups(s) && gic_test_group(s, irq, cpu);
 
     if (s->security_extn && !attrs.secure && !group) {
         DPRINTF("Non-secure EOI for Group0 interrupt %d ignored\n", irq);
@@ -600,9 +785,14 @@ static void gic_complete_irq(GICState *s, int cpu, int irq, MemTxAttrs attrs)
 
     /* In GICv2 the guest can choose to split priority-drop and deactivate */
     if (!gic_eoi_split(s, cpu, attrs)) {
-        GIC_DIST_CLEAR_ACTIVE(irq, cm);
+        gic_clear_active(s, irq, cpu);
+    }
+
+    if (gic_is_vcpu(cpu)) {
+        gic_update_virt(s);
+    } else {
+        gic_update(s);
     }
-    gic_update(s);
 }
 
 static uint32_t gic_dist_readb(void *opaque, hwaddr offset, MemTxAttrs attrs)
@@ -888,8 +1078,9 @@ static void gic_dist_writeb(void *opaque, hwaddr offset,
 
         for (i = 0; i < 8; i++) {
             if (value & (1 << i)) {
-                int mask = (irq < GIC_INTERNAL) ? (1 << cpu)
-                                                : GIC_DIST_TARGET(irq + i);
+                int mask =
+                    (irq < GIC_INTERNAL) ? (1 << cpu)
+                                         : GIC_DIST_TARGET(irq + i);
                 int cm = (irq < GIC_INTERNAL) ? (1 << cpu) : ALL_CPU_MASK;
 
                 if (s->security_extn && !attrs.secure &&
@@ -1242,8 +1433,9 @@ static MemTxResult gic_cpu_read(GICState *s, int cpu, int offset,
     case 0xd0: case 0xd4: case 0xd8: case 0xdc:
     {
         int regno = (offset - 0xd0) / 4;
+        int nr_aprs = gic_is_vcpu(cpu) ? GIC_VIRT_NR_APRS : GIC_NR_APRS;
 
-        if (regno >= GIC_NR_APRS || s->revision != 2) {
+        if (regno >= nr_aprs || s->revision != 2) {
             *data = 0;
         } else if (s->security_extn && !attrs.secure) {
             /* NS view of GICC_APR<n> is the top half of GIC_NSAPR<n> */
@@ -1258,7 +1450,7 @@ static MemTxResult gic_cpu_read(GICState *s, int cpu, int offset,
         int regno = (offset - 0xe0) / 4;
 
         if (regno >= GIC_NR_APRS || s->revision != 2 || !gic_has_groups(s) ||
-            (s->security_extn && !attrs.secure)) {
+            (s->security_extn && !attrs.secure) || gic_is_vcpu(cpu)) {
             *data = 0;
         } else {
             *data = s->nsapr[regno][cpu];
@@ -1293,7 +1485,8 @@ static MemTxResult gic_cpu_write(GICState *s, int cpu, int offset,
                 s->abpr[cpu] = MAX(value & 0x7, GIC_MIN_ABPR);
             }
         } else {
-            s->bpr[cpu] = MAX(value & 0x7, GIC_MIN_BPR);
+            int min_bpr = gic_is_vcpu(cpu) ? GIC_VIRT_MIN_BPR : GIC_MIN_BPR;
+            s->bpr[cpu] = MAX(value & 0x7, min_bpr);
         }
         break;
     case 0x10: /* End Of Interrupt */
@@ -1310,8 +1503,9 @@ static MemTxResult gic_cpu_write(GICState *s, int cpu, int offset,
     case 0xd0: case 0xd4: case 0xd8: case 0xdc:
     {
         int regno = (offset - 0xd0) / 4;
+        int nr_aprs = gic_is_vcpu(cpu) ? GIC_VIRT_NR_APRS : GIC_NR_APRS;
 
-        if (regno >= GIC_NR_APRS || s->revision != 2) {
+        if (regno >= nr_aprs || s->revision != 2) {
             return MEMTX_OK;
         }
         if (s->security_extn && !attrs.secure) {
@@ -1332,6 +1526,9 @@ static MemTxResult gic_cpu_write(GICState *s, int cpu, int offset,
         if (!gic_has_groups(s) || (s->security_extn && !attrs.secure)) {
             return MEMTX_OK;
         }
+        if (gic_is_vcpu(cpu)) {
+            return MEMTX_OK;
+        }
         s->nsapr[regno][cpu] = value;
         break;
     }
@@ -1344,7 +1541,13 @@ static MemTxResult gic_cpu_write(GICState *s, int cpu, int offset,
                       "gic_cpu_write: Bad offset %x\n", (int)offset);
         return MEMTX_OK;
     }
-    gic_update(s);
+
+    if (gic_is_vcpu(cpu)) {
+        gic_update_virt(s);
+    } else {
+        gic_update(s);
+    }
+
     return MEMTX_OK;
 }
 
@@ -1384,6 +1587,218 @@ static MemTxResult gic_do_cpu_write(void *opaque, hwaddr addr,
     GICState *s = *backref;
     int id = (backref - s->backref);
     return gic_cpu_write(s, id, addr, value, attrs);
+
+}
+
+static MemTxResult gic_thisvcpu_read(void *opaque, hwaddr addr, uint64_t *data,
+                                    unsigned size, MemTxAttrs attrs)
+{
+    GICState *s = (GICState *)opaque;
+
+    /* The exposed vCPU interface does not implement security extensions.
+     * Pretend this access is secure to go through the "no security extension
+     * or secure access" path in the CPU interface. */
+    attrs.secure = 1;
+
+    return gic_cpu_read(s, gic_get_current_cpu(s) + GIC_NCPU,
+                        addr, data, attrs);
+}
+
+static MemTxResult gic_thisvcpu_write(void *opaque, hwaddr addr,
+                                     uint64_t value, unsigned size,
+                                     MemTxAttrs attrs)
+{
+    GICState *s = (GICState *)opaque;
+
+    attrs.secure = 1;
+
+    return gic_cpu_write(s, gic_get_current_cpu(s) + GIC_NCPU,
+                         addr, value, attrs);
+}
+
+static MemTxResult gic_do_vcpu_read(void *opaque, hwaddr addr, uint64_t *data,
+                                    unsigned size, MemTxAttrs attrs)
+{
+    GICState **backref = (GICState **)opaque;
+    GICState *s = *backref;
+    int id = (backref - s->backref);
+
+    attrs.secure = 1;
+
+    return gic_cpu_read(s, id + GIC_NCPU, addr, data, attrs);
+}
+
+static MemTxResult gic_do_vcpu_write(void *opaque, hwaddr addr,
+                                     uint64_t value, unsigned size,
+                                     MemTxAttrs attrs)
+{
+    GICState **backref = (GICState **)opaque;
+    GICState *s = *backref;
+    int id = (backref - s->backref);
+
+    attrs.secure = 1;
+
+    return gic_cpu_write(s, id + GIC_NCPU, addr, value, attrs);
+
+}
+
+static void gic_vmcr_write(GICState *s, uint32_t value)
+{
+    int vcpu = gic_get_current_vcpu(s);
+    uint32_t ctlr;
+    uint32_t abpr;
+    uint32_t bpr;
+    uint32_t prio_mask;
+
+    /* Always pretend to do a secure access */
+    MemTxAttrs attrs = { .secure = 1 };
+
+    ctlr = extract32(value, 0, 5) & 0x0000021f;
+    abpr = extract32(value, 18, 3);
+    bpr = extract32(value, 21, 3);
+    prio_mask = extract32(value, 27, 5) << 3;
+
+    gic_set_cpu_control(s, vcpu, ctlr, attrs);
+    s->abpr[vcpu] = MAX(abpr, GIC_VIRT_MIN_ABPR);
+    s->bpr[vcpu] = MAX(bpr, GIC_VIRT_MIN_BPR);
+    gic_set_priority_mask(s, vcpu, prio_mask, attrs);
+}
+
+static void gic_set_lr_entry(GICState *s, int cpu, int lr_num, uint32_t entry)
+{
+    assert(lr_num < GIC_NR_LR);
+
+    uint32_t prev_entry = s->h_lr[lr_num][cpu];
+    int irq;
+    bool is_free;
+
+    if (!gic_lr_entry_is_free(prev_entry)) {
+        /* The entry was valid, flush it */
+        irq = GICH_LR_VIRT_ID(prev_entry);
+        gic_clear_virq_cache(s, irq, cpu);
+    }
+
+    s->h_lr[lr_num][cpu] = entry;
+    is_free = gic_lr_update(s, lr_num, cpu);
+
+    if (!is_free) {
+        /* Update the vIRQ -> LR entry cache */
+        irq = GICH_LR_VIRT_ID(entry);
+        gic_set_virq_cache(s, irq, cpu, lr_num);
+    }
+}
+
+static MemTxResult gic_hyp_read(void *opaque, hwaddr addr, uint64_t *data,
+                                unsigned size, MemTxAttrs attrs)
+{
+    GICState *s = ARM_GIC(opaque);
+    int cpu = gic_get_current_cpu(s);
+    int vcpu = gic_get_current_vcpu(s);
+
+    switch (addr) {
+    case 0x0: /* Hypervisor Control */
+        *data = s->h_hcr[cpu];
+        break;
+
+    case 0x4: /* VGIC Type */
+        *data = ((6 - GIC_VIRT_MIN_BPR) << 29)
+            | ((GIC_VIRT_MAX_GROUP_PRIO_BITS - 1) << 26)
+            | (GIC_NR_LR - 1);
+        break;
+
+    case 0x8: /* Virtual Machine Control */
+        *data = extract32(s->cpu_ctlr[vcpu], 0, 10);
+        *data |= extract32(s->abpr[vcpu], 0, 3) << 18;
+        *data |= extract32(s->bpr[vcpu], 0, 3) << 21;
+        *data |= extract32(s->priority_mask[vcpu], 3, 5) << 27;
+        break;
+
+    case 0x10: /* Maintenance Interrupt Status */
+        *data = s->h_misr[cpu];
+        break;
+
+    case 0x20: /* End of Interrupt Status 0 and 1 */
+    case 0x24:
+        *data = (uint32_t) extract64(s->h_eisr[cpu], (addr - 0x20) * 8, 32);
+        break;
+
+    case 0x30: /* Empty List Status 0 and 1 */
+    case 0x34:
+        *data = (uint32_t) extract64(s->h_elrsr[cpu], (addr - 0x30) * 8, 32);
+        break;
+
+    case 0xf0: /* Active Priorities */
+        *data = s->apr[0][vcpu];
+        break;
+
+    case 0x100 ... 0x1fc: /* List Registers */
+    {
+        int lr_num = (addr - 0x100) / 4;
+
+        if (lr_num > GIC_NR_LR) {
+            *data = 0;
+        } else {
+            *data = s->h_lr[lr_num][cpu];
+        }
+        break;
+    }
+
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "gic_hyp_read: Bad offset %" HWADDR_PRIx "\n", addr);
+        return MEMTX_OK;
+    }
+
+    return MEMTX_OK;
+}
+
+static MemTxResult gic_hyp_write(void *opaque, hwaddr addr, uint64_t value,
+                                 unsigned size, MemTxAttrs attrs)
+{
+    GICState *s = ARM_GIC(opaque);
+    int cpu = gic_get_current_cpu(s);
+    int vcpu = gic_get_current_vcpu(s);
+
+    switch (addr) {
+    case 0x0: /* Hypervisor Control */
+        s->h_hcr[cpu] = value & 0xf80000ff;
+        break;
+
+    case 0x8: /* Virtual Machine Control */
+        gic_vmcr_write(s, value);
+        break;
+
+    case 0xf0: /* Active Priorities */
+        s->apr[0][vcpu] = value;
+        break;
+
+    case 0x100 ... 0x1fc: /* List Registers */
+    {
+        int lr_num = (addr - 0x100) / 4;
+
+        if (lr_num > GIC_NR_LR) {
+            return MEMTX_OK;
+        }
+
+        gic_set_lr_entry(s, cpu, lr_num, value & 0xff8fffff);
+        break;
+    }
+
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "gic_hyp_write: Bad offset %" HWADDR_PRIx "\n", addr);
+        return MEMTX_OK;
+    }
+
+    gic_update_virt(s);
+    return MEMTX_OK;
+}
+
+static void arm_gic_post_load(GICState *s)
+{
+    if (s->virt_extn) {
+        gic_recompute_virt_cache(s);
+    }
 }
 
 static const MemoryRegionOps gic_ops[2] = {
@@ -1405,6 +1820,25 @@ static const MemoryRegionOps gic_cpu_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
+static const MemoryRegionOps gic_virt_ops[2] = {
+    {
+        .read_with_attrs = gic_hyp_read,
+        .write_with_attrs = gic_hyp_write,
+        .endianness = DEVICE_NATIVE_ENDIAN,
+    },
+    {
+        .read_with_attrs = gic_thisvcpu_read,
+        .write_with_attrs = gic_thisvcpu_write,
+        .endianness = DEVICE_NATIVE_ENDIAN,
+    }
+};
+
+static const MemoryRegionOps gic_vcpu_ops = {
+    .read_with_attrs = gic_do_vcpu_read,
+    .write_with_attrs = gic_do_vcpu_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
 static void arm_gic_realize(DeviceState *dev, Error **errp)
 {
     /* Device instance realize function for the GIC sysbus device */
@@ -1427,7 +1861,7 @@ static void arm_gic_realize(DeviceState *dev, Error **errp)
     }
 
     /* This creates distributor and main CPU interface (s->cpuiomem[0]) */
-    gic_init_irqs_and_mmio(s, gic_set_irq, gic_ops, NULL);
+    gic_init_irqs_and_mmio(s, gic_set_irq, gic_ops, gic_virt_ops);
 
     /* Extra core-specific regions for the CPU interfaces. This is
      * necessary for "franken-GIC" implementations, for example on
@@ -1439,17 +1873,29 @@ static void arm_gic_realize(DeviceState *dev, Error **errp)
      */
     for (i = 0; i < s->num_cpu; i++) {
         s->backref[i] = s;
-        memory_region_init_io(&s->cpuiomem[i+1], OBJECT(s), &gic_cpu_ops,
+        memory_region_init_io(&s->cpuiomem[i + 1], OBJECT(s), &gic_cpu_ops,
                               &s->backref[i], "gic_cpu", 0x100);
-        sysbus_init_mmio(sbd, &s->cpuiomem[i+1]);
+        sysbus_init_mmio(sbd, &s->cpuiomem[i + 1]);
     }
+
+    if (s->virt_extn) {
+        for (i = 0; i < s->num_cpu; i++) {
+            memory_region_init_io(&s->vcpuiomem[i + 1], OBJECT(s),
+                                  &gic_vcpu_ops, &s->backref[i],
+                                  "gic_vcpu", 0x2000);
+            sysbus_init_mmio(sbd, &s->vcpuiomem[i + 1]);
+        }
+    }
+
 }
 
 static void arm_gic_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
+    ARMGICCommonClass *agcc = ARM_GIC_COMMON_CLASS(klass);
     ARMGICClass *agc = ARM_GIC_CLASS(klass);
 
+    agcc->post_load = arm_gic_post_load;
     device_class_set_parent_realize(dc, arm_gic_realize, &agc->parent_realize);
 }
 
diff --git a/hw/intc/gic_internal.h b/hw/intc/gic_internal.h
index c85427c8e3..998e4b7854 100644
--- a/hw/intc/gic_internal.h
+++ b/hw/intc/gic_internal.h
@@ -64,6 +64,34 @@
 #define GICC_CTLR_EOIMODE    (1U << 9)
 #define GICC_CTLR_EOIMODE_NS (1U << 10)
 
+#define GICH_HCR_EN       (1U << 0)
+#define GICH_HCR_UIE      (1U << 1)
+#define GICH_HCR_LRENPIE  (1U << 2)
+#define GICH_HCR_NPIE     (1U << 3)
+#define GICH_HCR_VGRP0EIE (1U << 4)
+#define GICH_HCR_VGRP0DIE (1U << 5)
+#define GICH_HCR_VGRP1EIE (1U << 6)
+#define GICH_HCR_VGRP1DIE (1U << 7)
+#define GICH_HCR_EOICOUNT (0x1fU << 27)
+
+#define GICH_LR_STATE_INVALID         0
+#define GICH_LR_STATE_PENDING         1
+#define GICH_LR_STATE_ACTIVE          2
+#define GICH_LR_STATE_ACTIVE_PENDING  3
+
+#define GICH_LR_VIRT_ID(entry) (extract32(entry, 0, 10))
+#define GICH_LR_PHYS_ID(entry) (extract32(entry, 10, 10))
+#define GICH_LR_PRIORITY(entry) (extract32(entry, 23, 5) << 3)
+#define GICH_LR_STATE(entry) (extract32(entry, 28, 2))
+#define GICH_LR_GROUP(entry) (extract32(entry, 30, 1))
+#define GICH_LR_HW(entry) (extract32(entry, 31, 1))
+#define GICH_LR_EOI(entry) (extract32(entry, 19, 1))
+#define GICH_LR_CPUID(entry) (extract32(entry, 10, 3))
+
+#define GICH_LR_CLEAR_PENDING(entry) ((entry) &= ~(1 << 28))
+#define GICH_LR_SET_ACTIVE(entry) ((entry) |= (1 << 29))
+#define GICH_LR_CLEAR_ACTIVE(entry) ((entry) &= ~(1 << 29))
+
 /* Valid bits for GICC_CTLR for GICv1, v1 with security extensions,
  * GICv2 and GICv2 with security extensions:
  */
@@ -99,4 +127,174 @@ static inline bool gic_is_vcpu(int cpu)
     return cpu >= GIC_NCPU;
 }
 
+static inline int gic_get_vcpu_real_id(int cpu)
+{
+    return (cpu >= GIC_NCPU) ? (cpu - GIC_NCPU) : cpu;
+}
+
+static inline bool gic_lr_entry_is_free(uint32_t entry)
+{
+    return (GICH_LR_STATE(entry) == GICH_LR_STATE_INVALID)
+        && (GICH_LR_HW(entry) || !GICH_LR_EOI(entry));
+}
+
+static inline bool gic_lr_entry_is_eoi(uint32_t entry)
+{
+    return (GICH_LR_STATE(entry) == GICH_LR_STATE_INVALID)
+        && !GICH_LR_HW(entry) && GICH_LR_EOI(entry);
+}
+
+static inline bool gic_virq_is_valid(GICState *s, int irq, int vcpu)
+{
+    int cpu = gic_get_vcpu_real_id(vcpu);
+    return s->virq_lr_entry[irq][cpu] != GIC_NR_LR;
+}
+
+/* Return a pointer on the LR entry for a given (irq,vcpu) couple.
+ * This function requires that the entry actually exists somewhere in
+ * the LRs. */
+static inline uint32_t *gic_get_lr_entry(GICState *s, int irq, int vcpu)
+{
+    int cpu = gic_get_vcpu_real_id(vcpu);
+    int lr_num = s->virq_lr_entry[irq][cpu];
+
+    assert(lr_num >= 0 && lr_num < GIC_NR_LR);
+    return &s->h_lr[lr_num][cpu];
+}
+
+static inline void gic_set_virq_cache(GICState *s, int irq,
+                                      int vcpu, int lr_num)
+{
+    int cpu = gic_get_vcpu_real_id(vcpu);
+    s->virq_lr_entry[irq][cpu] = lr_num;
+}
+
+static inline void gic_clear_virq_cache(GICState *s, int irq, int vcpu)
+{
+    gic_set_virq_cache(s, irq, vcpu, GIC_NR_LR);
+}
+
+static inline bool gic_lr_update(GICState *s, int lr_num, int vcpu)
+{
+    int cpu = gic_get_vcpu_real_id(vcpu);
+
+    assert(lr_num >= 0 && lr_num < GIC_NR_LR);
+    uint32_t *entry = &s->h_lr[lr_num][cpu];
+
+    int is_eoi = gic_lr_entry_is_eoi(*entry);
+    int is_free = gic_lr_entry_is_free(*entry);
+    int is_pending = (GICH_LR_STATE(*entry) == GICH_LR_STATE_PENDING);
+
+    s->h_eisr[cpu] = deposit64(s->h_eisr[cpu], lr_num, 1, is_eoi);
+    s->h_elrsr[cpu] = deposit64(s->h_elrsr[cpu], lr_num, 1, is_free);
+    s->pending_lrs[cpu] = deposit64(s->pending_lrs[cpu], lr_num, 1, is_pending);
+
+    return is_free;
+}
+
+static inline void gic_lr_update_by_irq(GICState *s, int irq, int vcpu)
+{
+    int cpu = gic_get_vcpu_real_id(vcpu);
+    int lr_num = s->virq_lr_entry[irq][cpu];
+
+    assert(lr_num != GIC_NR_LR);
+    bool is_free = gic_lr_update(s, lr_num, vcpu);
+
+    if (is_free) {
+        gic_clear_virq_cache(s, irq, vcpu);
+    }
+}
+
+/* Recompute the whole virt cache, including the vIRQ to LR mapping, the EISR
+ * and ELRSR registers, and the LRs in the pending state.
+ * This function is called after restoring the GIC state from a VMState. */
+static inline void gic_recompute_virt_cache(GICState *s)
+{
+    int cpu, lr_num;
+
+    for (cpu = 0; cpu < s->num_cpu; cpu++) {
+        for (lr_num = 0; lr_num < GIC_NR_LR; lr_num++) {
+            bool is_free = gic_lr_update(s, lr_num, cpu);
+            uint32_t entry = s->h_lr[lr_num][cpu];
+
+            if (!is_free) {
+                int irq = GICH_LR_VIRT_ID(entry);
+                gic_set_virq_cache(s, irq, cpu, lr_num);
+            }
+        }
+    }
+}
+
+static inline bool gic_test_group(GICState *s, int irq, int cpu)
+{
+    if (gic_is_vcpu(cpu)) {
+        uint32_t *entry = gic_get_lr_entry(s, irq, cpu);
+        return GICH_LR_GROUP(*entry);
+    } else {
+        return GIC_DIST_TEST_GROUP(irq, 1 << cpu);
+    }
+}
+
+static inline void gic_clear_pending(GICState *s, int irq, int cpu)
+{
+    if (gic_is_vcpu(cpu)) {
+        uint32_t *entry = gic_get_lr_entry(s, irq, cpu);
+        GICH_LR_CLEAR_PENDING(*entry);
+        /* Don't recompute the LR cache yet as a clear pending request is
+         * always followed by a set active one. */
+    } else {
+        /* Clear pending state for both level and edge triggered
+         * interrupts. (level triggered interrupts with an active line
+         * remain pending, see gic_test_pending)
+         */
+        GIC_DIST_CLEAR_PENDING(irq, GIC_DIST_TEST_MODEL(irq) ? ALL_CPU_MASK
+                                                             : (1 << cpu));
+    }
+}
+
+static inline void gic_set_active(GICState *s, int irq, int cpu)
+{
+    if (gic_is_vcpu(cpu)) {
+        uint32_t *entry = gic_get_lr_entry(s, irq, cpu);
+        GICH_LR_SET_ACTIVE(*entry);
+        gic_lr_update_by_irq(s, irq, cpu);
+    } else {
+        GIC_DIST_SET_ACTIVE(irq, 1 << cpu);
+    }
+}
+
+static inline void gic_clear_active(GICState *s, int irq, int cpu)
+{
+    if (gic_is_vcpu(cpu)) {
+        uint32_t *entry = gic_get_lr_entry(s, irq, cpu);
+        GICH_LR_CLEAR_ACTIVE(*entry);
+
+        if (GICH_LR_HW(*entry)) {
+            /* Hardware interrupt. We must forward the deactivation request to
+             * the distributor */
+            int phys_irq = GICH_LR_PHYS_ID(*entry);
+            int rcpu = gic_get_vcpu_real_id(cpu);
+
+            /* Group 0 IRQs deactivation requests are ignored. */
+            if (GIC_DIST_TEST_GROUP(phys_irq, 1 << rcpu)) {
+                GIC_DIST_CLEAR_ACTIVE(phys_irq, 1 << rcpu);
+            }
+        }
+
+        gic_lr_update_by_irq(s, irq, cpu);
+    } else {
+        GIC_DIST_CLEAR_ACTIVE(irq, 1 << cpu);
+    }
+}
+
+static inline int gic_get_priority(GICState *s, int irq, int cpu)
+{
+    if (gic_is_vcpu(cpu)) {
+        uint32_t *entry = gic_get_lr_entry(s, irq, cpu);
+        return GICH_LR_PRIORITY(*entry);
+    } else {
+        return GIC_DIST_GET_PRIORITY(irq, cpu);
+    }
+}
+
 #endif /* QEMU_ARM_GIC_INTERNAL_H */
-- 
2.17.0

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

* [Qemu-devel] [PATCH 5/6] intc/arm_gic: Improve traces
  2018-06-06  9:30 [Qemu-devel] [PATCH 0/6] arm_gic: add virtualization extensions support luc.michel
                   ` (3 preceding siblings ...)
  2018-06-06  9:30 ` [Qemu-devel] [PATCH 4/6] intc/arm_gic: Add virtualization extensions logic luc.michel
@ 2018-06-06  9:31 ` luc.michel
  2018-06-06 13:36   ` [Qemu-devel] [Qemu-arm] " Philippe Mathieu-Daudé
  2018-06-06  9:31 ` [Qemu-devel] [PATCH 6/6] xlnx-zynqmp: Improve GIC wiring and MMIO mapping luc.michel
  2018-06-06 11:02 ` [Qemu-devel] [PATCH 0/6] arm_gic: add virtualization extensions support Jan Kiszka
  6 siblings, 1 reply; 17+ messages in thread
From: luc.michel @ 2018-06-06  9:31 UTC (permalink / raw)
  To: qemu-devel
  Cc: Luc MICHEL, qemu-arm, Peter Maydell, saipava, edgari, mark.burton,
	Jan Kiszka

From: Luc MICHEL <luc.michel@greensocs.com>

Add some traces to the ARM GIC to catch register accesses (distributor,
(v)cpu interface and virtual interface), and to take into account
virtualization extensions (print `vcpu` instead of `cpu` when needed).

Also add some virtualization extensions specific traces: LR updating
and maintenance IRQ generation.

Signed-off-by: Luc MICHEL <luc.michel@greensocs.com>
---
 hw/intc/arm_gic.c    | 31 +++++++++++++++++++++++++------
 hw/intc/trace-events | 12 ++++++++++--
 2 files changed, 35 insertions(+), 8 deletions(-)

diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
index 6105f930c4..a0ffd85fdf 100644
--- a/hw/intc/arm_gic.c
+++ b/hw/intc/arm_gic.c
@@ -179,8 +179,10 @@ static inline void gic_update_internal(GICState *s, bool virt)
         }
 
         if (best_irq != 1023) {
-            trace_gic_update_bestirq(cpu, best_irq, best_prio,
-                s->priority_mask[cpu_iface], s->running_priority[cpu_iface]);
+            trace_gic_update_bestirq(virt ? "vcpu" : "cpu", cpu,
+                                     best_irq, best_prio,
+                                     s->priority_mask[cpu_iface],
+                                     s->running_priority[cpu_iface]);
         }
 
         irq_level = fiq_level = 0;
@@ -268,6 +270,7 @@ static void gic_update_maintenance(GICState *s)
         gic_compute_misr(s, cpu);
         maint_level = (s->h_hcr[cpu] & GICH_HCR_EN) && s->h_misr[cpu];
 
+        trace_gic_update_maintenance_irq(cpu, maint_level);
         qemu_set_irq(s->maintenance_irq[cpu], maint_level);
     }
 }
@@ -524,7 +527,8 @@ uint32_t gic_acknowledge_irq(GICState *s, int cpu, MemTxAttrs attrs)
      * is in the wrong group.
      */
     irq = gic_get_current_pending_irq(s, cpu, attrs);
-    trace_gic_acknowledge_irq(gic_get_vcpu_real_id(cpu), irq);
+    trace_gic_acknowledge_irq(gic_is_vcpu(cpu) ? "vcpu" : "cpu",
+                              gic_get_vcpu_real_id(cpu), irq);
 
     if (irq >= GIC_MAXIRQ) {
         DPRINTF("ACK, no pending interrupt or it is hidden: %d\n", irq);
@@ -1002,20 +1006,23 @@ static MemTxResult gic_dist_read(void *opaque, hwaddr offset, uint64_t *data,
     switch (size) {
     case 1:
         *data = gic_dist_readb(opaque, offset, attrs);
-        return MEMTX_OK;
+        break;
     case 2:
         *data = gic_dist_readb(opaque, offset, attrs);
         *data |= gic_dist_readb(opaque, offset + 1, attrs) << 8;
-        return MEMTX_OK;
+        break;
     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;
+        break;
     default:
         return MEMTX_ERROR;
     }
+
+    trace_gic_dist_read(offset, size, *data);
+    return MEMTX_OK;
 }
 
 static void gic_dist_writeb(void *opaque, hwaddr offset,
@@ -1309,6 +1316,8 @@ static void gic_dist_writel(void *opaque, hwaddr offset,
 static MemTxResult gic_dist_write(void *opaque, hwaddr offset, uint64_t data,
                                   unsigned size, MemTxAttrs attrs)
 {
+    trace_gic_dist_write(offset, size, data);
+
     switch (size) {
     case 1:
         gic_dist_writeb(opaque, offset, data, attrs);
@@ -1463,12 +1472,18 @@ static MemTxResult gic_cpu_read(GICState *s, int cpu, int offset,
         *data = 0;
         break;
     }
+
+    trace_gic_cpu_read(gic_is_vcpu(cpu) ? "vcpu" : "cpu",
+                       gic_get_vcpu_real_id(cpu), offset, *data);
     return MEMTX_OK;
 }
 
 static MemTxResult gic_cpu_write(GICState *s, int cpu, int offset,
                                  uint32_t value, MemTxAttrs attrs)
 {
+    trace_gic_cpu_write(gic_is_vcpu(cpu) ? "vcpu" : "cpu",
+                        gic_get_vcpu_real_id(cpu), offset, value);
+
     switch (offset) {
     case 0x00: /* Control */
         gic_set_cpu_control(s, cpu, value, attrs);
@@ -1679,6 +1694,7 @@ static void gic_set_lr_entry(GICState *s, int cpu, int lr_num, uint32_t entry)
     }
 
     s->h_lr[lr_num][cpu] = entry;
+    trace_gic_lr_entry(cpu, lr_num, entry);
     is_free = gic_lr_update(s, lr_num, cpu);
 
     if (!is_free) {
@@ -1749,6 +1765,7 @@ static MemTxResult gic_hyp_read(void *opaque, hwaddr addr, uint64_t *data,
         return MEMTX_OK;
     }
 
+    trace_gic_hyp_read(addr, *data);
     return MEMTX_OK;
 }
 
@@ -1759,6 +1776,8 @@ static MemTxResult gic_hyp_write(void *opaque, hwaddr addr, uint64_t value,
     int cpu = gic_get_current_cpu(s);
     int vcpu = gic_get_current_vcpu(s);
 
+    trace_gic_hyp_write(addr, value);
+
     switch (addr) {
     case 0x0: /* Hypervisor Control */
         s->h_hcr[cpu] = value & 0xf80000ff;
diff --git a/hw/intc/trace-events b/hw/intc/trace-events
index 55e8c2570c..16d02fa8cf 100644
--- a/hw/intc/trace-events
+++ b/hw/intc/trace-events
@@ -92,9 +92,17 @@ aspeed_vic_write(uint64_t offset, unsigned size, uint32_t data) "To 0x%" PRIx64
 gic_enable_irq(int irq) "irq %d enabled"
 gic_disable_irq(int irq) "irq %d disabled"
 gic_set_irq(int irq, int level, int cpumask, int target) "irq %d level %d cpumask 0x%x target 0x%x"
-gic_update_bestirq(int cpu, int irq, int prio, int priority_mask, int running_priority) "cpu %d irq %d priority %d cpu priority mask %d cpu running priority %d"
+gic_update_bestirq(const char *s, int cpu, int irq, int prio, int priority_mask, int running_priority) "%s %d irq %d priority %d cpu priority mask %d cpu running priority %d"
 gic_update_set_irq(int cpu, const char *name, int level) "cpu[%d]: %s = %d"
-gic_acknowledge_irq(int cpu, int irq) "cpu %d acknowledged irq %d"
+gic_acknowledge_irq(const char *s, int cpu, int irq) "%s %d acknowledged irq %d"
+gic_cpu_write(const char *s, int cpu, int addr, uint32_t val) "%s %d iface write at 0x%08x 0x%08" PRIx32
+gic_cpu_read(const char *s, int cpu, int addr, uint32_t val) "%s %d iface write at 0x%08x: 0x%08" PRIx32
+gic_hyp_read(int addr, uint32_t val) "hyp read at 0x%08x: 0x%08" PRIx32
+gic_hyp_write(int addr, uint32_t val) "hyp write at 0x%08x: 0x%08" PRIx32
+gic_dist_read(int addr, unsigned int size, uint32_t val) "dist read at 0x%08x size %u: 0x%08" PRIx32
+gic_dist_write(int addr, unsigned int size, uint32_t val) "dist write at 0x%08x size %u: 0x%08" PRIx32
+gic_lr_entry(int cpu, int entry, uint32_t val) "cpu %d: new lr entry %d: 0x%08" PRIx32
+gic_update_maintenance_irq(int cpu, int val) "cpu %d: maintenance = %d"
 
 # hw/intc/arm_gicv3_cpuif.c
 gicv3_icc_pmr_read(uint32_t cpu, uint64_t val) "GICv3 ICC_PMR read cpu 0x%x value 0x%" PRIx64
-- 
2.17.0

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

* Re: [Qemu-devel] [Qemu-arm] [PATCH 5/6] intc/arm_gic: Improve traces
  2018-06-06  9:31 ` [Qemu-devel] [PATCH 5/6] intc/arm_gic: Improve traces luc.michel
@ 2018-06-06 13:36   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 17+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-06-06 13:36 UTC (permalink / raw)
  To: luc.michel, qemu-devel
  Cc: Peter Maydell, mark.burton, saipava, edgari, qemu-arm, Jan Kiszka

On 06/06/2018 06:31 AM, luc.michel@greensocs.com wrote:
> From: Luc MICHEL <luc.michel@greensocs.com>
> 
> Add some traces to the ARM GIC to catch register accesses (distributor,
> (v)cpu interface and virtual interface), and to take into account
> virtualization extensions (print `vcpu` instead of `cpu` when needed).
> 
> Also add some virtualization extensions specific traces: LR updating
> and maintenance IRQ generation.
> 
> Signed-off-by: Luc MICHEL <luc.michel@greensocs.com>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  hw/intc/arm_gic.c    | 31 +++++++++++++++++++++++++------
>  hw/intc/trace-events | 12 ++++++++++--
>  2 files changed, 35 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
> index 6105f930c4..a0ffd85fdf 100644
> --- a/hw/intc/arm_gic.c
> +++ b/hw/intc/arm_gic.c
> @@ -179,8 +179,10 @@ static inline void gic_update_internal(GICState *s, bool virt)
>          }
>  
>          if (best_irq != 1023) {
> -            trace_gic_update_bestirq(cpu, best_irq, best_prio,
> -                s->priority_mask[cpu_iface], s->running_priority[cpu_iface]);
> +            trace_gic_update_bestirq(virt ? "vcpu" : "cpu", cpu,
> +                                     best_irq, best_prio,
> +                                     s->priority_mask[cpu_iface],
> +                                     s->running_priority[cpu_iface]);
>          }
>  
>          irq_level = fiq_level = 0;
> @@ -268,6 +270,7 @@ static void gic_update_maintenance(GICState *s)
>          gic_compute_misr(s, cpu);
>          maint_level = (s->h_hcr[cpu] & GICH_HCR_EN) && s->h_misr[cpu];
>  
> +        trace_gic_update_maintenance_irq(cpu, maint_level);
>          qemu_set_irq(s->maintenance_irq[cpu], maint_level);
>      }
>  }
> @@ -524,7 +527,8 @@ uint32_t gic_acknowledge_irq(GICState *s, int cpu, MemTxAttrs attrs)
>       * is in the wrong group.
>       */
>      irq = gic_get_current_pending_irq(s, cpu, attrs);
> -    trace_gic_acknowledge_irq(gic_get_vcpu_real_id(cpu), irq);
> +    trace_gic_acknowledge_irq(gic_is_vcpu(cpu) ? "vcpu" : "cpu",
> +                              gic_get_vcpu_real_id(cpu), irq);
>  
>      if (irq >= GIC_MAXIRQ) {
>          DPRINTF("ACK, no pending interrupt or it is hidden: %d\n", irq);
> @@ -1002,20 +1006,23 @@ static MemTxResult gic_dist_read(void *opaque, hwaddr offset, uint64_t *data,
>      switch (size) {
>      case 1:
>          *data = gic_dist_readb(opaque, offset, attrs);
> -        return MEMTX_OK;
> +        break;
>      case 2:
>          *data = gic_dist_readb(opaque, offset, attrs);
>          *data |= gic_dist_readb(opaque, offset + 1, attrs) << 8;
> -        return MEMTX_OK;
> +        break;
>      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;
> +        break;
>      default:
>          return MEMTX_ERROR;
>      }
> +
> +    trace_gic_dist_read(offset, size, *data);
> +    return MEMTX_OK;
>  }
>  
>  static void gic_dist_writeb(void *opaque, hwaddr offset,
> @@ -1309,6 +1316,8 @@ static void gic_dist_writel(void *opaque, hwaddr offset,
>  static MemTxResult gic_dist_write(void *opaque, hwaddr offset, uint64_t data,
>                                    unsigned size, MemTxAttrs attrs)
>  {
> +    trace_gic_dist_write(offset, size, data);
> +
>      switch (size) {
>      case 1:
>          gic_dist_writeb(opaque, offset, data, attrs);
> @@ -1463,12 +1472,18 @@ static MemTxResult gic_cpu_read(GICState *s, int cpu, int offset,
>          *data = 0;
>          break;
>      }
> +
> +    trace_gic_cpu_read(gic_is_vcpu(cpu) ? "vcpu" : "cpu",
> +                       gic_get_vcpu_real_id(cpu), offset, *data);
>      return MEMTX_OK;
>  }
>  
>  static MemTxResult gic_cpu_write(GICState *s, int cpu, int offset,
>                                   uint32_t value, MemTxAttrs attrs)
>  {
> +    trace_gic_cpu_write(gic_is_vcpu(cpu) ? "vcpu" : "cpu",
> +                        gic_get_vcpu_real_id(cpu), offset, value);
> +
>      switch (offset) {
>      case 0x00: /* Control */
>          gic_set_cpu_control(s, cpu, value, attrs);
> @@ -1679,6 +1694,7 @@ static void gic_set_lr_entry(GICState *s, int cpu, int lr_num, uint32_t entry)
>      }
>  
>      s->h_lr[lr_num][cpu] = entry;
> +    trace_gic_lr_entry(cpu, lr_num, entry);
>      is_free = gic_lr_update(s, lr_num, cpu);
>  
>      if (!is_free) {
> @@ -1749,6 +1765,7 @@ static MemTxResult gic_hyp_read(void *opaque, hwaddr addr, uint64_t *data,
>          return MEMTX_OK;
>      }
>  
> +    trace_gic_hyp_read(addr, *data);
>      return MEMTX_OK;
>  }
>  
> @@ -1759,6 +1776,8 @@ static MemTxResult gic_hyp_write(void *opaque, hwaddr addr, uint64_t value,
>      int cpu = gic_get_current_cpu(s);
>      int vcpu = gic_get_current_vcpu(s);
>  
> +    trace_gic_hyp_write(addr, value);
> +
>      switch (addr) {
>      case 0x0: /* Hypervisor Control */
>          s->h_hcr[cpu] = value & 0xf80000ff;
> diff --git a/hw/intc/trace-events b/hw/intc/trace-events
> index 55e8c2570c..16d02fa8cf 100644
> --- a/hw/intc/trace-events
> +++ b/hw/intc/trace-events
> @@ -92,9 +92,17 @@ aspeed_vic_write(uint64_t offset, unsigned size, uint32_t data) "To 0x%" PRIx64
>  gic_enable_irq(int irq) "irq %d enabled"
>  gic_disable_irq(int irq) "irq %d disabled"
>  gic_set_irq(int irq, int level, int cpumask, int target) "irq %d level %d cpumask 0x%x target 0x%x"
> -gic_update_bestirq(int cpu, int irq, int prio, int priority_mask, int running_priority) "cpu %d irq %d priority %d cpu priority mask %d cpu running priority %d"
> +gic_update_bestirq(const char *s, int cpu, int irq, int prio, int priority_mask, int running_priority) "%s %d irq %d priority %d cpu priority mask %d cpu running priority %d"
>  gic_update_set_irq(int cpu, const char *name, int level) "cpu[%d]: %s = %d"
> -gic_acknowledge_irq(int cpu, int irq) "cpu %d acknowledged irq %d"
> +gic_acknowledge_irq(const char *s, int cpu, int irq) "%s %d acknowledged irq %d"
> +gic_cpu_write(const char *s, int cpu, int addr, uint32_t val) "%s %d iface write at 0x%08x 0x%08" PRIx32
> +gic_cpu_read(const char *s, int cpu, int addr, uint32_t val) "%s %d iface write at 0x%08x: 0x%08" PRIx32
> +gic_hyp_read(int addr, uint32_t val) "hyp read at 0x%08x: 0x%08" PRIx32
> +gic_hyp_write(int addr, uint32_t val) "hyp write at 0x%08x: 0x%08" PRIx32
> +gic_dist_read(int addr, unsigned int size, uint32_t val) "dist read at 0x%08x size %u: 0x%08" PRIx32
> +gic_dist_write(int addr, unsigned int size, uint32_t val) "dist write at 0x%08x size %u: 0x%08" PRIx32
> +gic_lr_entry(int cpu, int entry, uint32_t val) "cpu %d: new lr entry %d: 0x%08" PRIx32
> +gic_update_maintenance_irq(int cpu, int val) "cpu %d: maintenance = %d"
>  
>  # hw/intc/arm_gicv3_cpuif.c
>  gicv3_icc_pmr_read(uint32_t cpu, uint64_t val) "GICv3 ICC_PMR read cpu 0x%x value 0x%" PRIx64
> 

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

* [Qemu-devel] [PATCH 6/6] xlnx-zynqmp: Improve GIC wiring and MMIO mapping
  2018-06-06  9:30 [Qemu-devel] [PATCH 0/6] arm_gic: add virtualization extensions support luc.michel
                   ` (4 preceding siblings ...)
  2018-06-06  9:31 ` [Qemu-devel] [PATCH 5/6] intc/arm_gic: Improve traces luc.michel
@ 2018-06-06  9:31 ` luc.michel
  2018-06-06 11:02 ` [Qemu-devel] [PATCH 0/6] arm_gic: add virtualization extensions support Jan Kiszka
  6 siblings, 0 replies; 17+ messages in thread
From: luc.michel @ 2018-06-06  9:31 UTC (permalink / raw)
  To: qemu-devel
  Cc: Luc MICHEL, qemu-arm, Peter Maydell, saipava, edgari, mark.burton,
	Jan Kiszka

From: Luc MICHEL <luc.michel@greensocs.com>

This commit improve the way the GIC is realized and connected in the
ZynqMP SoC. The security extensions are enabled only if requested in the
machine state. The same goes for the virtualization extensions.

All the GIC to APU CPU(s) IRQ lines are now connected, including FIQ,
vIRQ and vFIQ. The missing CPU to GIC timers IRQ connections are also
added (HYP and SEC timers).

The GIC maintenance IRQs are back-wired to the correct GIC PPIs.

Finally, the MMIO mappings are reworked to take into account the ZynqMP
specificities. the GIC (v)CPU interface is aliased 16 times:
  * for the firsts 0x1000 bytes from 0xf9010000 to 0xf901f000
  * for the seconds 0x1000 bytes from 0xf9020000 to 0xf902f000
Mappings of the virtual interface and virtual CPU interface are mapped
only when virtualization extensions are requested. The
XlnxZynqMPGICRegion struct has been enhanced to be able to catch all
this information.

Signed-off-by: Luc MICHEL <luc.michel@greensocs.com>
---
 hw/arm/xlnx-zynqmp.c         | 92 ++++++++++++++++++++++++++++++++----
 include/hw/arm/xlnx-zynqmp.h |  4 +-
 2 files changed, 86 insertions(+), 10 deletions(-)

diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index 2045b9d71e..4d17c70ec6 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -29,12 +29,17 @@
 
 #define ARM_PHYS_TIMER_PPI  30
 #define ARM_VIRT_TIMER_PPI  27
+#define ARM_HYP_TIMER_PPI   26
+#define ARM_SEC_TIMER_PPI   29
+#define GIC_MAINTENANCE_PPI 25
 
 #define GEM_REVISION        0x40070106
 
 #define GIC_BASE_ADDR       0xf9000000
 #define GIC_DIST_ADDR       0xf9010000
 #define GIC_CPU_ADDR        0xf9020000
+#define GIC_VIFACE_ADDR     0xf9040000
+#define GIC_VCPU_ADDR       0xf9060000
 
 #define SATA_INTR           133
 #define SATA_ADDR           0xFD0C0000
@@ -111,11 +116,54 @@ static const int adma_ch_intr[XLNX_ZYNQMP_NUM_ADMA_CH] = {
 typedef struct XlnxZynqMPGICRegion {
     int region_index;
     uint32_t address;
+    uint32_t offset;
+    bool virt;
 } XlnxZynqMPGICRegion;
 
 static const XlnxZynqMPGICRegion xlnx_zynqmp_gic_regions[] = {
-    { .region_index = 0, .address = GIC_DIST_ADDR, },
-    { .region_index = 1, .address = GIC_CPU_ADDR,  },
+    /* Distributor */
+    {
+        .region_index = 0,
+        .address = GIC_DIST_ADDR,
+        .offset = 0,
+        .virt = false
+    },
+
+    /* CPU interface */
+    {
+        .region_index = 1,
+        .address = GIC_CPU_ADDR,
+        .offset = 0,
+        .virt = false
+    },
+    {
+        .region_index = 1,
+        .address = GIC_CPU_ADDR + 0x10000,
+        .offset = 0x1000,
+        .virt = false
+    },
+
+    /* Virtual interface */
+    {
+        .region_index = 2,
+        .address = GIC_VIFACE_ADDR,
+        .offset = 0,
+        .virt = true
+    },
+
+    /* Virtual CPU interface */
+    {
+        .region_index = 3,
+        .address = GIC_VCPU_ADDR,
+        .offset = 0,
+        .virt = true
+    },
+    {
+        .region_index = 3,
+        .address = GIC_VCPU_ADDR + 0x10000,
+        .offset = 0x1000,
+        .virt = true
+    },
 };
 
 static inline int arm_gic_ppi_index(int cpu_nr, int ppi_index)
@@ -286,6 +334,9 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
     qdev_prop_set_uint32(DEVICE(&s->gic), "num-irq", GIC_NUM_SPI_INTR + 32);
     qdev_prop_set_uint32(DEVICE(&s->gic), "revision", 2);
     qdev_prop_set_uint32(DEVICE(&s->gic), "num-cpu", num_apus);
+    qdev_prop_set_bit(DEVICE(&s->gic), "has-security-extensions", s->secure);
+    qdev_prop_set_bit(DEVICE(&s->gic),
+                      "has-virtualization-extensions", s->virt);
 
     /* Realize APUs before realizing the GIC. KVM requires this.  */
     for (i = 0; i < num_apus; i++) {
@@ -330,19 +381,23 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
     for (i = 0; i < XLNX_ZYNQMP_GIC_REGIONS; i++) {
         SysBusDevice *gic = SYS_BUS_DEVICE(&s->gic);
         const XlnxZynqMPGICRegion *r = &xlnx_zynqmp_gic_regions[i];
-        MemoryRegion *mr = sysbus_mmio_get_region(gic, r->region_index);
+        MemoryRegion *mr;
         uint32_t addr = r->address;
         int j;
 
-        sysbus_mmio_map(gic, r->region_index, addr);
+        if (r->virt && !s->virt) {
+            continue;
+        }
 
+        mr = sysbus_mmio_get_region(gic, r->region_index);
         for (j = 0; j < XLNX_ZYNQMP_GIC_ALIASES; j++) {
             MemoryRegion *alias = &s->gic_mr[i][j];
 
-            addr += XLNX_ZYNQMP_GIC_REGION_SIZE;
             memory_region_init_alias(alias, OBJECT(s), "zynqmp-gic-alias", mr,
-                                     0, XLNX_ZYNQMP_GIC_REGION_SIZE);
+                                     r->offset, XLNX_ZYNQMP_GIC_REGION_SIZE);
             memory_region_add_subregion(system_memory, addr, alias);
+
+            addr += XLNX_ZYNQMP_GIC_REGION_SIZE;
         }
     }
 
@@ -352,12 +407,33 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
         sysbus_connect_irq(SYS_BUS_DEVICE(&s->gic), i,
                            qdev_get_gpio_in(DEVICE(&s->apu_cpu[i]),
                                             ARM_CPU_IRQ));
+        sysbus_connect_irq(SYS_BUS_DEVICE(&s->gic), i + num_apus,
+                           qdev_get_gpio_in(DEVICE(&s->apu_cpu[i]),
+                                            ARM_CPU_FIQ));
+        sysbus_connect_irq(SYS_BUS_DEVICE(&s->gic), i + num_apus * 2,
+                           qdev_get_gpio_in(DEVICE(&s->apu_cpu[i]),
+                                            ARM_CPU_VIRQ));
+        sysbus_connect_irq(SYS_BUS_DEVICE(&s->gic), i + num_apus * 3,
+                           qdev_get_gpio_in(DEVICE(&s->apu_cpu[i]),
+                                            ARM_CPU_VFIQ));
         irq = qdev_get_gpio_in(DEVICE(&s->gic),
                                arm_gic_ppi_index(i, ARM_PHYS_TIMER_PPI));
-        qdev_connect_gpio_out(DEVICE(&s->apu_cpu[i]), 0, irq);
+        qdev_connect_gpio_out(DEVICE(&s->apu_cpu[i]), GTIMER_PHYS, irq);
         irq = qdev_get_gpio_in(DEVICE(&s->gic),
                                arm_gic_ppi_index(i, ARM_VIRT_TIMER_PPI));
-        qdev_connect_gpio_out(DEVICE(&s->apu_cpu[i]), 1, irq);
+        qdev_connect_gpio_out(DEVICE(&s->apu_cpu[i]), GTIMER_VIRT, irq);
+        irq = qdev_get_gpio_in(DEVICE(&s->gic),
+                               arm_gic_ppi_index(i, ARM_HYP_TIMER_PPI));
+        qdev_connect_gpio_out(DEVICE(&s->apu_cpu[i]), GTIMER_HYP, irq);
+        irq = qdev_get_gpio_in(DEVICE(&s->gic),
+                               arm_gic_ppi_index(i, ARM_SEC_TIMER_PPI));
+        qdev_connect_gpio_out(DEVICE(&s->apu_cpu[i]), GTIMER_SEC, irq);
+
+        if (s->virt) {
+            irq = qdev_get_gpio_in(DEVICE(&s->gic),
+                                   arm_gic_ppi_index(i, GIC_MAINTENANCE_PPI));
+            sysbus_connect_irq(SYS_BUS_DEVICE(&s->gic), i + num_apus * 4, irq);
+        }
     }
 
     if (s->has_rpu) {
diff --git a/include/hw/arm/xlnx-zynqmp.h b/include/hw/arm/xlnx-zynqmp.h
index 82b6ec2486..98f925ab84 100644
--- a/include/hw/arm/xlnx-zynqmp.h
+++ b/include/hw/arm/xlnx-zynqmp.h
@@ -53,7 +53,7 @@
 #define XLNX_ZYNQMP_OCM_RAM_0_ADDRESS 0xFFFC0000
 #define XLNX_ZYNQMP_OCM_RAM_SIZE 0x10000
 
-#define XLNX_ZYNQMP_GIC_REGIONS 2
+#define XLNX_ZYNQMP_GIC_REGIONS 6
 
 /* ZynqMP maps the ARM GIC regions (GICC, GICD ...) at consecutive 64k offsets
  * and under-decodes the 64k region. This mirrors the 4k regions to every 4k
@@ -62,7 +62,7 @@
  */
 
 #define XLNX_ZYNQMP_GIC_REGION_SIZE 0x1000
-#define XLNX_ZYNQMP_GIC_ALIASES     (0x10000 / XLNX_ZYNQMP_GIC_REGION_SIZE - 1)
+#define XLNX_ZYNQMP_GIC_ALIASES     (0x10000 / XLNX_ZYNQMP_GIC_REGION_SIZE)
 
 #define XLNX_ZYNQMP_MAX_LOW_RAM_SIZE    0x80000000ull
 
-- 
2.17.0

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

* Re: [Qemu-devel] [PATCH 0/6] arm_gic: add virtualization extensions support
  2018-06-06  9:30 [Qemu-devel] [PATCH 0/6] arm_gic: add virtualization extensions support luc.michel
                   ` (5 preceding siblings ...)
  2018-06-06  9:31 ` [Qemu-devel] [PATCH 6/6] xlnx-zynqmp: Improve GIC wiring and MMIO mapping luc.michel
@ 2018-06-06 11:02 ` Jan Kiszka
  6 siblings, 0 replies; 17+ messages in thread
From: Jan Kiszka @ 2018-06-06 11:02 UTC (permalink / raw)
  To: luc.michel, qemu-devel
  Cc: qemu-arm, Peter Maydell, saipava, edgari, mark.burton, Jailhouse

On 2018-06-06 11:30, luc.michel@greensocs.com wrote:
> From: Luc MICHEL <luc.michel@greensocs.com>
> 
> This patch series add support for the virtualization extensions in the
> ARM GICv2 interrupt controller.
> 
> The first two commits do some refactoring to prepare for the
> implementation. Commits 3 and 4 are the actual implementation. The last
> commit updates the ZynqMP implementation to support virtualization.
> 
> The current state allows to boot Xen (tested with 4.8 and 4.10) with
> Linux Dom0 guest properly.
> 
> I also tested in SMP. It does not work directly because Xen expects to
> find CPU IDs in the GIC ITARGETSR0 register. This behavior is not
> documented in the GICv2 specification, and is not implemented in QEMU.
> By hacking this register, I was able to get the whole thing to boot in
> SMP properly. This hack is not part of those patches though.
> 
> I also tested migration, it works fine AFAIK. I had to add the HYP and
> SEC timers in the ARM CPU VMState though (Xen uses the HYP one) (not
> part of those patches).
> 
> I want to thanks the Xilinx's QEMU team who sponsored this work for
> their collaboration.
> 
> Luc MICHEL (6):
>   intc/arm_gic: Refactor operations on the distributor
>   intc/arm_gic: Remove some dead code and put some functions static
>   intc/arm_gic: Add the virtualization extensions to the GIC state
>   intc/arm_gic: Add virtualization extensions logic
>   intc/arm_gic: Improve traces
>   xlnx-zynqmp: Improve GIC wiring and MMIO mapping
> 
>  hw/arm/xlnx-zynqmp.c             |  92 +++-
>  hw/intc/arm_gic.c                | 768 ++++++++++++++++++++++++-------
>  hw/intc/arm_gic_common.c         | 159 +++++--
>  hw/intc/arm_gic_kvm.c            |  31 +-
>  hw/intc/gic_internal.h           | 258 +++++++++--
>  hw/intc/trace-events             |  12 +-
>  include/hw/arm/xlnx-zynqmp.h     |   4 +-
>  include/hw/intc/arm_gic_common.h |  52 ++-
>  8 files changed, 1128 insertions(+), 248 deletions(-)
> 
> --
> 2.17.0
> 

Cool! We will give this a try with Jailhouse as well.

Jan

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

end of thread, other threads:[~2018-06-18 12:11 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-06-06  9:30 [Qemu-devel] [PATCH 0/6] arm_gic: add virtualization extensions support luc.michel
2018-06-06  9:30 ` [Qemu-devel] [PATCH 1/6] intc/arm_gic: Refactor operations on the distributor luc.michel
2018-06-06 13:38   ` [Qemu-devel] [Qemu-arm] " Philippe Mathieu-Daudé
2018-06-07  6:08   ` [Qemu-devel] " Sai Pavan Boddu
2018-06-11 13:39   ` Peter Maydell
2018-06-06  9:30 ` [Qemu-devel] [PATCH 2/6] intc/arm_gic: Remove some dead code and put some functions static luc.michel
2018-06-06 13:39   ` [Qemu-devel] [Qemu-arm] " Philippe Mathieu-Daudé
2018-06-11 13:39   ` [Qemu-devel] " Peter Maydell
2018-06-06  9:30 ` [Qemu-devel] [PATCH 3/6] intc/arm_gic: Add the virtualization extensions to the GIC state luc.michel
2018-06-11 13:38   ` Peter Maydell
2018-06-18 11:50     ` Luc Michel
2018-06-18 12:11       ` Peter Maydell
2018-06-06  9:30 ` [Qemu-devel] [PATCH 4/6] intc/arm_gic: Add virtualization extensions logic luc.michel
2018-06-06  9:31 ` [Qemu-devel] [PATCH 5/6] intc/arm_gic: Improve traces luc.michel
2018-06-06 13:36   ` [Qemu-devel] [Qemu-arm] " Philippe Mathieu-Daudé
2018-06-06  9:31 ` [Qemu-devel] [PATCH 6/6] xlnx-zynqmp: Improve GIC wiring and MMIO mapping luc.michel
2018-06-06 11:02 ` [Qemu-devel] [PATCH 0/6] arm_gic: add virtualization extensions support Jan Kiszka

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).