[Qemu-devel] [PATCH v8 0/5] GICv3 live migration support

[Qemu-devel] [PATCH v8 0/5] GICv3 live migration support

[vijay.kilari]

From: Vijaya Kumar K <Vijaya.Kumar@cavium.com>

This series introduces support for GICv3 live migration with
new VGIC implementation in 4.7-rc3 kernel.
In this series, patch 1 of the previous implementation
are ported.
https://lists.nongnu.org/archive/html/qemu-devel/2015-10/msg05284.html

Patch 2, is based on below implementation.
http://patchwork.ozlabs.org/patch/626746/

Latest kernel patches
https://www.spinics.net/lists/arm-kernel/msg558046.html

This API definition is as per version of VGICv3 specification
in linux kernel Documentation/virtual/kvm/devices/arm-vgic-v3.txt

Tested Live migration of Idle VM running with 4 VCPUs and 8GB RAM.

v7 => v8:
 - Introduced vmstate subsection to add icc_ctrl_el1 register to
   VMStateDescription
 - Introduced new function gicv3_set_gicv3state() in arm_gicv3_cpuif.c
   to update gicv3state variable in CPUARMState struct.
 - Used arm_cp_read_zero & arm_cp_write_ignore for ARMCPRegInfo[].

v6 => v7:
 - Rebased on top of v2.8.0-rc4 release.
 - Added patch to add icc_ctrl_el1 to vmstruct before live migration
   patch.
 - Added patch to add gicv3state variable to CPUARMState struct to
   store GICv3CPUState pointer.
 - Added patch to register ARMCPRegInfo[] struct and reset on CPU reset.

v5 => v6:
 - Added separate patch for Reseting ICC* register
 - Added seperate patch for save and restore of ICC_CTLR_EL1
 - Dropped translate_fn mechanism and coded open functions
   for edge_trigger and priority save and restore.
 - Save and Restore APnR registers based on ICC_CTLR_EL1.PRIBITS

v4 => v5:
 - Initialized ICC registers before reset.

v3 => v4:
 - Reintroduced offset GICR_SGI_OFFSET
 - Implement save and restore of ICC_SRE_EL1
 - Updated kvm.h header file in sync with KVM v4 patches

v2 => v3:
 - Dropped offset GICR_SGI_OFFSET
 - Implement save/restore of irq line level using
   KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO
 - Fixed bug with save/restore of edge_trigger
Vijaya Kumar K (5):
  kernel: Add definitions for GICv3 attributes
  hw/intc/arm_gicv3_kvm: Add ICC_SRE_EL1 register to vmstate
  hw/intc/arm_gicv3_kvm: Implement get/put functions
  target-arm: Add GICv3CPUState in CPUARMState struct
  hw/intc/arm_gicv3_kvm: Reset GICv3 cpu interface registers

 hw/intc/arm_gicv3_common.c         |  34 ++
 hw/intc/arm_gicv3_cpuif.c          |   8 +
 hw/intc/arm_gicv3_kvm.c            | 627 ++++++++++++++++++++++++++++++++++++-
 hw/intc/gicv3_internal.h           |   3 +
 include/hw/intc/arm_gicv3_common.h |   1 +
 linux-headers/asm-arm/kvm.h        |  12 +
 linux-headers/asm-arm64/kvm.h      |  12 +
 target/arm/cpu.h                   |   2 +
 8 files changed, 685 insertions(+), 14 deletions(-)

-- 
1.9.1

[Qemu-devel] [PATCH v8 1/5] kernel: Add definitions for GICv3 attributes

[vijay.kilari]

From: Vijaya Kumar K <Vijaya.Kumar@cavium.com>

This temporary patch adds kernel API definitions.
Use proper header update procedure after these features
are released.

Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
---
 linux-headers/asm-arm/kvm.h   | 12 ++++++++++++
 linux-headers/asm-arm64/kvm.h | 12 ++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/linux-headers/asm-arm/kvm.h b/linux-headers/asm-arm/kvm.h
index 2fb7859..1798c93 100644
--- a/linux-headers/asm-arm/kvm.h
+++ b/linux-headers/asm-arm/kvm.h
@@ -179,10 +179,22 @@ struct kvm_arch_memory_slot {
 #define KVM_DEV_ARM_VGIC_GRP_CPU_REGS	2
 #define   KVM_DEV_ARM_VGIC_CPUID_SHIFT	32
 #define   KVM_DEV_ARM_VGIC_CPUID_MASK	(0xffULL << KVM_DEV_ARM_VGIC_CPUID_SHIFT)
+#define   KVM_DEV_ARM_VGIC_V3_MPIDR_SHIFT 32
+#define   KVM_DEV_ARM_VGIC_V3_MPIDR_MASK \
+			(0xffffffffULL << KVM_DEV_ARM_VGIC_V3_MPIDR_SHIFT)
 #define   KVM_DEV_ARM_VGIC_OFFSET_SHIFT	0
 #define   KVM_DEV_ARM_VGIC_OFFSET_MASK	(0xffffffffULL << KVM_DEV_ARM_VGIC_OFFSET_SHIFT)
+#define   KVM_DEV_ARM_VGIC_SYSREG_INSTR_MASK (0xffff)
 #define KVM_DEV_ARM_VGIC_GRP_NR_IRQS	3
 #define KVM_DEV_ARM_VGIC_GRP_CTRL       4
+#define KVM_DEV_ARM_VGIC_GRP_REDIST_REGS 5
+#define KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS 6
+#define KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO  7
+#define KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_SHIFT	10
+#define KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_MASK \
+			(0x3fffffULL << KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_SHIFT)
+#define KVM_DEV_ARM_VGIC_LINE_LEVEL_INTID_MASK 0x3ff
+#define VGIC_LEVEL_INFO_LINE_LEVEL	0
 #define   KVM_DEV_ARM_VGIC_CTRL_INIT    0
 
 /* KVM_IRQ_LINE irq field index values */
diff --git a/linux-headers/asm-arm64/kvm.h b/linux-headers/asm-arm64/kvm.h
index fd5a276..b3f02ce 100644
--- a/linux-headers/asm-arm64/kvm.h
+++ b/linux-headers/asm-arm64/kvm.h
@@ -201,10 +201,22 @@ struct kvm_arch_memory_slot {
 #define KVM_DEV_ARM_VGIC_GRP_CPU_REGS	2
 #define   KVM_DEV_ARM_VGIC_CPUID_SHIFT	32
 #define   KVM_DEV_ARM_VGIC_CPUID_MASK	(0xffULL << KVM_DEV_ARM_VGIC_CPUID_SHIFT)
+#define   KVM_DEV_ARM_VGIC_V3_MPIDR_SHIFT 32
+#define   KVM_DEV_ARM_VGIC_V3_MPIDR_MASK \
+			(0xffffffffULL << KVM_DEV_ARM_VGIC_V3_MPIDR_SHIFT)
 #define   KVM_DEV_ARM_VGIC_OFFSET_SHIFT	0
 #define   KVM_DEV_ARM_VGIC_OFFSET_MASK	(0xffffffffULL << KVM_DEV_ARM_VGIC_OFFSET_SHIFT)
+#define   KVM_DEV_ARM_VGIC_SYSREG_INSTR_MASK (0xffff)
 #define KVM_DEV_ARM_VGIC_GRP_NR_IRQS	3
 #define KVM_DEV_ARM_VGIC_GRP_CTRL	4
+#define KVM_DEV_ARM_VGIC_GRP_REDIST_REGS 5
+#define KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS 6
+#define KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO  7
+#define KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_SHIFT	10
+#define KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_MASK \
+			(0x3fffffULL << KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_SHIFT)
+#define KVM_DEV_ARM_VGIC_LINE_LEVEL_INTID_MASK	0x3ff
+#define VGIC_LEVEL_INFO_LINE_LEVEL	0
 #define   KVM_DEV_ARM_VGIC_CTRL_INIT	0
 
 /* Device Control API on vcpu fd */
-- 
1.9.1

[Qemu-devel] [PATCH v8 2/5] hw/intc/arm_gicv3_kvm: Add ICC_SRE_EL1 register to vmstate

[vijay.kilari]

From: Vijaya Kumar K <Vijaya.Kumar@cavium.com>

To Save and Restore ICC_SRE_EL1 register introduce vmstate
subsection and load only if non-zero.
Also initialize icc_sre_el1 with to 0x7 in pre_load
function.

Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
---
 hw/intc/arm_gicv3_common.c         | 32 ++++++++++++++++++++++++++++++++
 include/hw/intc/arm_gicv3_common.h |  1 +
 2 files changed, 33 insertions(+)

diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
index 16b9b0f..e62480e 100644
--- a/hw/intc/arm_gicv3_common.c
+++ b/hw/intc/arm_gicv3_common.c
@@ -70,6 +70,34 @@ static const VMStateDescription vmstate_gicv3_cpu_virt = {
     }
 };
 
+static int icc_sre_el1_reg_pre_load(void *opaque)
+{
+    GICv3CPUState *cs = opaque;
+
+    /* By default enable SRE and disable IRQ & FIQ bypass. */
+    cs->icc_sre_el1 = 0x7;
+    return 0;
+}
+
+static bool icc_sre_el1_reg_needed(void *opaque)
+{
+    GICv3CPUState *cs = opaque;
+
+    return cs->icc_sre_el1 != 0;
+}
+
+const VMStateDescription vmstate_gicv3_cpu_sre_el1 = {
+    .name = "arm_gicv3_cpu/sre_el1",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .pre_load = icc_sre_el1_reg_pre_load,
+    .needed = icc_sre_el1_reg_needed,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT64(icc_sre_el1, GICv3CPUState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static const VMStateDescription vmstate_gicv3_cpu = {
     .name = "arm_gicv3_cpu",
     .version_id = 1,
@@ -100,6 +128,10 @@ static const VMStateDescription vmstate_gicv3_cpu = {
     .subsections = (const VMStateDescription * []) {
         &vmstate_gicv3_cpu_virt,
         NULL
+    },
+    .subsections = (const VMStateDescription * []) {
+        &vmstate_gicv3_cpu_sre_el1,
+        NULL
     }
 };
 
diff --git a/include/hw/intc/arm_gicv3_common.h b/include/hw/intc/arm_gicv3_common.h
index 4156051..bccdfe1 100644
--- a/include/hw/intc/arm_gicv3_common.h
+++ b/include/hw/intc/arm_gicv3_common.h
@@ -172,6 +172,7 @@ struct GICv3CPUState {
     uint8_t gicr_ipriorityr[GIC_INTERNAL];
 
     /* CPU interface */
+    uint64_t icc_sre_el1;
     uint64_t icc_ctlr_el1[2];
     uint64_t icc_pmr_el1;
     uint64_t icc_bpr[3];
-- 
1.9.1

[Qemu-devel] [PATCH v8 3/5] hw/intc/arm_gicv3_kvm: Implement get/put functions

[vijay.kilari]

From: Vijaya Kumar K <Vijaya.Kumar@cavium.com>

This actually implements pre_save and post_load methods for in-kernel
vGICv3.

Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
[PMM:
 * use decimal, not 0bnnn
 * fixed typo in names of ICC_APR0R_EL1 and ICC_AP1R_EL1
 * completely rearranged the get and put functions to read and write
   the state in a natural order, rather than mixing distributor and
   redistributor state together]
Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
[Vijay:
 * Update macro KVM_VGIC_ATTR
 * Use 32 bit access for gicd and gicr
 * GICD_IROUTER, GICD_TYPER, GICR_PROPBASER and GICR_PENDBASER reg
   access  are changed from 64-bit to 32-bit access
 * Add ICC_SRE_EL1 save and restore
 * Dropped translate_fn mechanism and coded functions to handle
   save and restore of edge_trigger and priority
 * Number of APnR register saved/restored based on number of
   priority bits supported]
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
---
 hw/intc/arm_gicv3_kvm.c  | 573 +++++++++++++++++++++++++++++++++++++++++++++--
 hw/intc/gicv3_internal.h |   1 +
 2 files changed, 558 insertions(+), 16 deletions(-)

diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c
index d69dc47..cda1af4 100644
--- a/hw/intc/arm_gicv3_kvm.c
+++ b/hw/intc/arm_gicv3_kvm.c
@@ -23,8 +23,10 @@
 #include "qapi/error.h"
 #include "hw/intc/arm_gicv3_common.h"
 #include "hw/sysbus.h"
+#include "qemu/error-report.h"
 #include "sysemu/kvm.h"
 #include "kvm_arm.h"
+#include "gicv3_internal.h"
 #include "vgic_common.h"
 #include "migration/migration.h"
 
@@ -44,6 +46,32 @@
 #define KVM_ARM_GICV3_GET_CLASS(obj) \
      OBJECT_GET_CLASS(KVMARMGICv3Class, (obj), TYPE_KVM_ARM_GICV3)
 
+#define   KVM_DEV_ARM_VGIC_SYSREG(op0, op1, crn, crm, op2)         \
+                             (ARM64_SYS_REG_SHIFT_MASK(op0, OP0) | \
+                              ARM64_SYS_REG_SHIFT_MASK(op1, OP1) | \
+                              ARM64_SYS_REG_SHIFT_MASK(crn, CRN) | \
+                              ARM64_SYS_REG_SHIFT_MASK(crm, CRM) | \
+                              ARM64_SYS_REG_SHIFT_MASK(op2, OP2))
+
+#define ICC_PMR_EL1     \
+    KVM_DEV_ARM_VGIC_SYSREG(3, 0, 4, 6, 0)
+#define ICC_BPR0_EL1    \
+    KVM_DEV_ARM_VGIC_SYSREG(3, 0, 12, 8, 3)
+#define ICC_AP0R_EL1(n) \
+    KVM_DEV_ARM_VGIC_SYSREG(3, 0, 12, 8, 4 | n)
+#define ICC_AP1R_EL1(n) \
+    KVM_DEV_ARM_VGIC_SYSREG(3, 0, 12, 9, n)
+#define ICC_BPR1_EL1    \
+    KVM_DEV_ARM_VGIC_SYSREG(3, 0, 12, 12, 3)
+#define ICC_CTLR_EL1    \
+    KVM_DEV_ARM_VGIC_SYSREG(3, 0, 12, 12, 4)
+#define ICC_SRE_EL1 \
+    KVM_DEV_ARM_VGIC_SYSREG(3, 0, 12, 12, 5)
+#define ICC_IGRPEN0_EL1 \
+    KVM_DEV_ARM_VGIC_SYSREG(3, 0, 12, 12, 6)
+#define ICC_IGRPEN1_EL1 \
+    KVM_DEV_ARM_VGIC_SYSREG(3, 0, 12, 12, 7)
+
 typedef struct KVMARMGICv3Class {
     ARMGICv3CommonClass parent_class;
     DeviceRealize parent_realize;
@@ -57,16 +85,523 @@ static void kvm_arm_gicv3_set_irq(void *opaque, int irq, int level)
     kvm_arm_gic_set_irq(s->num_irq, irq, level);
 }
 
+#define KVM_VGIC_ATTR(reg, typer) \
+    ((typer & KVM_DEV_ARM_VGIC_V3_MPIDR_MASK) | (reg))
+
+static inline void kvm_gicd_access(GICv3State *s, int offset,
+                                   uint32_t *val, bool write)
+{
+    kvm_device_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_DIST_REGS,
+                      KVM_VGIC_ATTR(offset, 0),
+                      val, write);
+}
+
+static inline void kvm_gicr_access(GICv3State *s, int offset, int cpu,
+                                   uint32_t *val, bool write)
+{
+    kvm_device_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_REDIST_REGS,
+                      KVM_VGIC_ATTR(offset, s->cpu[cpu].gicr_typer),
+                      val, write);
+}
+
+static inline void kvm_gicc_access(GICv3State *s, uint64_t reg, int cpu,
+                                   uint64_t *val, bool write)
+{
+    kvm_device_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS,
+                      KVM_VGIC_ATTR(reg, s->cpu[cpu].gicr_typer),
+                      val, write);
+}
+
+static inline void kvm_gic_line_level_access(GICv3State *s, int irq, int cpu,
+                                             uint32_t *val, bool write)
+{
+    kvm_device_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO,
+                      KVM_VGIC_ATTR(irq, s->cpu[cpu].gicr_typer) |
+                      (VGIC_LEVEL_INFO_LINE_LEVEL <<
+                       KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_SHIFT),
+                      val, write);
+}
+
+/* Loop through each distributor IRQ related register; since bits
+ * corresponding to SPIs and PPIs are RAZ/WI when affinity routing
+ * is enabled, we skip those.
+ */
+#define for_each_dist_irq_reg(_irq, _max, _field_width) \
+    for (_irq = GIC_INTERNAL; _irq < _max; _irq += (32 / _field_width))
+
+static void kvm_dist_get_priority(GICv3State *s, uint32_t offset, uint8_t *bmp)
+{
+    uint32_t reg, *field;
+    int irq;
+
+    field = (uint32_t *)bmp;
+    for_each_dist_irq_reg(irq, s->num_irq, 8) {
+        kvm_gicd_access(s, offset, &reg, false);
+        *field = reg;
+        offset += 4;
+        field++;
+    }
+}
+
+static void kvm_dist_put_priority(GICv3State *s, uint32_t offset, uint8_t *bmp)
+{
+    uint32_t reg, *field;
+    int irq;
+
+    field = (uint32_t *)bmp;
+    for_each_dist_irq_reg(irq, s->num_irq, 8) {
+        reg = *field;
+        kvm_gicd_access(s, offset, &reg, true);
+        offset += 4;
+        field++;
+    }
+}
+
+static void kvm_dist_get_edge_trigger(GICv3State *s, uint32_t offset,
+                                      uint32_t *bmp)
+{
+    uint32_t reg;
+    int irq;
+
+    for_each_dist_irq_reg(irq, s->num_irq, 2) {
+        kvm_gicd_access(s, offset, &reg, false);
+        reg = half_unshuffle32(reg >> 1);
+        if (irq % 32 != 0) {
+            reg = (reg << 16);
+        }
+        *gic_bmp_ptr32(bmp, irq) |=  reg;
+        offset += 4;
+    }
+}
+
+static void kvm_dist_put_edge_trigger(GICv3State *s, uint32_t offset,
+                                      uint32_t *bmp)
+{
+    uint32_t reg;
+    int irq;
+
+    for_each_dist_irq_reg(irq, s->num_irq, 2) {
+        reg = *gic_bmp_ptr32(bmp, irq);
+        if (irq % 32 != 0) {
+            reg = (reg & 0xffff0000) >> 16;
+        } else {
+            reg = reg & 0xffff;
+        }
+        reg = half_shuffle32(reg) << 1;
+        kvm_gicd_access(s, offset, &reg, true);
+        offset += 4;
+    }
+}
+
+static void kvm_gic_get_line_level_bmp(GICv3State *s, uint32_t *bmp)
+{
+    uint32_t reg;
+    int irq;
+
+    for_each_dist_irq_reg(irq, s->num_irq, 1) {
+        kvm_gic_line_level_access(s, irq, 0, &reg, false);
+        *gic_bmp_ptr32(bmp, irq) = reg;
+    }
+}
+
+static void kvm_gic_put_line_level_bmp(GICv3State *s, uint32_t *bmp)
+{
+    uint32_t reg;
+    int irq;
+
+    for_each_dist_irq_reg(irq, s->num_irq, 1) {
+        reg = *gic_bmp_ptr32(bmp, irq);
+        kvm_gic_line_level_access(s, irq, 0, &reg, true);
+    }
+}
+
+/* Read a bitmap register group from the kernel VGIC. */
+static void kvm_dist_getbmp(GICv3State *s, uint32_t offset, uint32_t *bmp)
+{
+    uint32_t reg;
+    int irq;
+
+    for_each_dist_irq_reg(irq, s->num_irq, 1) {
+        kvm_gicd_access(s, offset, &reg, false);
+        *gic_bmp_ptr32(bmp, irq) = reg;
+        offset += 4;
+    }
+}
+
+static void kvm_dist_putbmp(GICv3State *s, uint32_t offset,
+                            uint32_t clroffset, uint32_t *bmp)
+{
+    uint32_t reg;
+    int irq;
+
+    for_each_dist_irq_reg(irq, s->num_irq, 1) {
+        /* If this bitmap is a set/clear register pair, first write to the
+         * clear-reg to clear all bits before using the set-reg to write
+         * the 1 bits.
+         */
+        if (clroffset != 0) {
+            reg = 0;
+            kvm_gicd_access(s, clroffset, &reg, true);
+        }
+        reg = *gic_bmp_ptr32(bmp, irq);
+        kvm_gicd_access(s, offset, &reg, true);
+        offset += 4;
+    }
+}
+
+static void kvm_arm_gicv3_check(GICv3State *s)
+{
+    uint32_t reg;
+    uint32_t num_irq;
+
+    /* Sanity checking s->num_irq */
+    kvm_gicd_access(s, GICD_TYPER, &reg, false);
+    num_irq = ((reg & 0x1f) + 1) * 32;
+
+    if (num_irq < s->num_irq) {
+        error_report("Model requests %u IRQs, but kernel supports max %u",
+                     s->num_irq, num_irq);
+        abort();
+    }
+}
+
 static void kvm_arm_gicv3_put(GICv3State *s)
 {
-    /* TODO */
-    DPRINTF("Cannot put kernel gic state, no kernel interface\n");
+    uint32_t regl, regh, reg;
+    uint64_t reg64, redist_typer;
+    int ncpu, i;
+
+    kvm_arm_gicv3_check(s);
+
+    kvm_gicr_access(s, GICR_TYPER, 0, &regl, false);
+    kvm_gicr_access(s, GICR_TYPER + 4, 0, &regh, false);
+    redist_typer = ((uint64_t)regh << 32) | regl;
+
+    reg = s->gicd_ctlr;
+    kvm_gicd_access(s, GICD_CTLR, &reg, true);
+
+    if (redist_typer & GICR_TYPER_PLPIS) {
+        /* Set base addresses before LPIs are enabled by GICR_CTLR write */
+        for (ncpu = 0; ncpu < s->num_cpu; ncpu++) {
+            GICv3CPUState *c = &s->cpu[ncpu];
+
+            reg64 = c->gicr_propbaser;
+            regl = (uint32_t)reg64;
+            kvm_gicr_access(s, GICR_PROPBASER, ncpu, &regl, true);
+            regh = (uint32_t)(reg64 >> 32);
+            kvm_gicr_access(s, GICR_PROPBASER + 4, ncpu, &regh, true);
+
+            reg64 = c->gicr_pendbaser;
+            if (!c->gicr_ctlr & GICR_CTLR_ENABLE_LPIS) {
+                /* Setting PTZ is advised if LPIs are disabled, to reduce
+                 * GIC initialization time.
+                 */
+                reg64 |= GICR_PENDBASER_PTZ;
+            }
+            regl = (uint32_t)reg64;
+            kvm_gicr_access(s, GICR_PENDBASER, ncpu, &regl, true);
+            regh = (uint32_t)(reg64 >> 32);
+            kvm_gicr_access(s, GICR_PENDBASER + 4, ncpu, &regh, true);
+        }
+    }
+
+    /* Redistributor state (one per CPU) */
+
+    for (ncpu = 0; ncpu < s->num_cpu; ncpu++) {
+        GICv3CPUState *c = &s->cpu[ncpu];
+
+        reg = c->gicr_ctlr;
+        kvm_gicr_access(s, GICR_CTLR, ncpu, &reg, true);
+
+        reg = c->gicr_statusr[GICV3_NS];
+        kvm_gicr_access(s, GICR_STATUSR, ncpu, &reg, true);
+
+        reg = c->gicr_waker;
+        kvm_gicr_access(s, GICR_WAKER, ncpu, &reg, true);
+
+        reg = c->gicr_igroupr0;
+        kvm_gicr_access(s, GICR_IGROUPR0, ncpu, &reg, true);
+
+        reg = ~0;
+        kvm_gicr_access(s, GICR_ICENABLER0, ncpu, &reg, true);
+        reg = c->gicr_ienabler0;
+        kvm_gicr_access(s, GICR_ISENABLER0, ncpu, &reg, true);
+
+        /* Restore config before pending so we treat level/edge correctly */
+        reg = half_shuffle32(c->edge_trigger >> 16) << 1;
+        kvm_gicr_access(s, GICR_ICFGR1, ncpu, &reg, true);
+
+        reg = c->level;
+        kvm_gic_line_level_access(s, 0, ncpu, &reg, true);
+
+        reg = ~0;
+        kvm_gicr_access(s, GICR_ICPENDR0, ncpu, &reg, true);
+        reg = c->gicr_ipendr0;
+        kvm_gicr_access(s, GICR_ISPENDR0, ncpu, &reg, true);
+
+        reg = ~0;
+        kvm_gicr_access(s, GICR_ICACTIVER0, ncpu, &reg, true);
+        reg = c->gicr_iactiver0;
+        kvm_gicr_access(s, GICR_ISACTIVER0, ncpu, &reg, true);
+
+        for (i = 0; i < GIC_INTERNAL; i += 4) {
+            reg = c->gicr_ipriorityr[i] |
+                (c->gicr_ipriorityr[i + 1] << 8) |
+                (c->gicr_ipriorityr[i + 2] << 16) |
+                (c->gicr_ipriorityr[i + 3] << 24);
+            kvm_gicr_access(s, GICR_IPRIORITYR + i, ncpu, &reg, true);
+        }
+    }
+
+    /* Distributor state (shared between all CPUs */
+    reg = s->gicd_statusr[GICV3_NS];
+    kvm_gicd_access(s, GICD_STATUSR, &reg, true);
+
+    /* s->enable bitmap -> GICD_ISENABLERn */
+    kvm_dist_putbmp(s, GICD_ISENABLER, GICD_ICENABLER, s->enabled);
+
+    /* s->group bitmap -> GICD_IGROUPRn */
+    kvm_dist_putbmp(s, GICD_IGROUPR, 0, s->group);
+
+    /* Restore targets before pending to ensure the pending state is set on
+     * the appropriate CPU interfaces in the kernel
+     */
+
+    /* s->gicd_irouter[irq] -> GICD_IROUTERn
+     * We can't use kvm_dist_put() here because the registers are 64-bit
+     */
+    for (i = GIC_INTERNAL; i < s->num_irq; i++) {
+        uint32_t offset;
+
+        offset = GICD_IROUTER + (sizeof(uint32_t) * i);
+        reg = (uint32_t)s->gicd_irouter[i];
+        kvm_gicd_access(s, offset, &reg, true);
+
+        offset = GICD_IROUTER + (sizeof(uint32_t) * i) + 4;
+        reg = (uint32_t)(s->gicd_irouter[i] >> 32);
+        kvm_gicd_access(s, offset, &reg, true);
+    }
+
+    /* s->trigger bitmap -> GICD_ICFGRn
+     * (restore configuration registers before pending IRQs so we treat
+     * level/edge correctly)
+     */
+    kvm_dist_put_edge_trigger(s, GICD_ICFGR, s->edge_trigger);
+
+    /* s->level bitmap ->  line_level */
+    kvm_gic_put_line_level_bmp(s, s->level);
+
+    /* s->pending bitmap -> GICD_ISPENDRn */
+    kvm_dist_putbmp(s, GICD_ISPENDR, GICD_ICPENDR, s->pending);
+
+    /* s->active bitmap -> GICD_ISACTIVERn */
+    kvm_dist_putbmp(s, GICD_ISACTIVER, GICD_ICACTIVER, s->active);
+
+    /* s->gicd_ipriority[] -> GICD_IPRIORITYRn */
+    kvm_dist_put_priority(s, GICD_IPRIORITYR, s->gicd_ipriority);
+
+    /* CPU Interface state (one per CPU) */
+
+    for (ncpu = 0; ncpu < s->num_cpu; ncpu++) {
+        GICv3CPUState *c = &s->cpu[ncpu];
+        int num_pri_bits;
+
+        kvm_gicc_access(s, ICC_SRE_EL1, ncpu, &c->icc_sre_el1, true);
+        kvm_gicc_access(s, ICC_CTLR_EL1, ncpu,
+                        &c->icc_ctlr_el1[GICV3_NS], true);
+        kvm_gicc_access(s, ICC_IGRPEN0_EL1, ncpu,
+                        &c->icc_igrpen[GICV3_G0], true);
+        kvm_gicc_access(s, ICC_IGRPEN1_EL1, ncpu,
+                        &c->icc_igrpen[GICV3_G1NS], true);
+        kvm_gicc_access(s, ICC_PMR_EL1, ncpu, &c->icc_pmr_el1, true);
+        kvm_gicc_access(s, ICC_BPR0_EL1, ncpu, &c->icc_bpr[GICV3_G0], true);
+        kvm_gicc_access(s, ICC_BPR1_EL1, ncpu, &c->icc_bpr[GICV3_G1NS], true);
+
+        num_pri_bits = ((c->icc_ctlr_el1[GICV3_NS] &
+                        ICC_CTLR_EL1_PRIBITS_MASK) >>
+                        ICC_CTLR_EL1_PRIBITS_SHIFT) + 1;
+
+        switch (num_pri_bits) {
+        case 7:
+            reg64 = c->icc_apr[GICV3_G0][3];
+            kvm_gicc_access(s, ICC_AP0R_EL1(3), ncpu, &reg64, true);
+            reg64 = c->icc_apr[GICV3_G0][2];
+            kvm_gicc_access(s, ICC_AP0R_EL1(2), ncpu, &reg64, true);
+        case 6:
+            reg64 = c->icc_apr[GICV3_G0][1];
+            kvm_gicc_access(s, ICC_AP0R_EL1(1), ncpu, &reg64, true);
+        default:
+            reg64 = c->icc_apr[GICV3_G0][0];
+            kvm_gicc_access(s, ICC_AP0R_EL1(0), ncpu, &reg64, true);
+        }
+
+        switch (num_pri_bits) {
+        case 7:
+            reg64 = c->icc_apr[GICV3_G1NS][3];
+            kvm_gicc_access(s, ICC_AP1R_EL1(3), ncpu, &reg64, true);
+            reg64 = c->icc_apr[GICV3_G1NS][2];
+            kvm_gicc_access(s, ICC_AP1R_EL1(2), ncpu, &reg64, true);
+        case 6:
+            reg64 = c->icc_apr[GICV3_G1NS][1];
+            kvm_gicc_access(s, ICC_AP1R_EL1(1), ncpu, &reg64, true);
+        default:
+            reg64 = c->icc_apr[GICV3_G1NS][0];
+            kvm_gicc_access(s, ICC_AP1R_EL1(0), ncpu, &reg64, true);
+        }
+    }
 }
 
 static void kvm_arm_gicv3_get(GICv3State *s)
 {
-    /* TODO */
-    DPRINTF("Cannot get kernel gic state, no kernel interface\n");
+    uint32_t regl, regh, reg;
+    uint64_t reg64, redist_typer;
+    int ncpu, i;
+
+    kvm_arm_gicv3_check(s);
+
+    kvm_gicr_access(s, GICR_TYPER, 0, &regl, false);
+    kvm_gicr_access(s, GICR_TYPER + 4, 0, &regh, false);
+    redist_typer = ((uint64_t)regh << 32) | regl;
+
+    kvm_gicd_access(s, GICD_CTLR, &reg, false);
+    s->gicd_ctlr = reg;
+
+    /* Redistributor state (one per CPU) */
+
+    for (ncpu = 0; ncpu < s->num_cpu; ncpu++) {
+        GICv3CPUState *c = &s->cpu[ncpu];
+
+        kvm_gicr_access(s, GICR_CTLR, ncpu, &reg, false);
+        c->gicr_ctlr = reg;
+
+        kvm_gicr_access(s, GICR_STATUSR, ncpu, &reg, false);
+        c->gicr_statusr[GICV3_NS] = reg;
+
+        kvm_gicr_access(s, GICR_WAKER, ncpu, &reg, false);
+        c->gicr_waker = reg;
+
+        kvm_gicr_access(s, GICR_IGROUPR0, ncpu, &reg, false);
+        c->gicr_igroupr0 = reg;
+        kvm_gicr_access(s, GICR_ISENABLER0, ncpu, &reg, false);
+        c->gicr_ienabler0 = reg;
+        kvm_gicr_access(s, GICR_ICFGR1, ncpu, &reg, false);
+        c->edge_trigger = half_unshuffle32(reg >> 1) << 16;
+        kvm_gic_line_level_access(s, 0, ncpu, &reg, false);
+        c->level = reg;
+        kvm_gicr_access(s, GICR_ISPENDR0, ncpu, &reg, false);
+        c->gicr_ipendr0 = reg;
+        kvm_gicr_access(s, GICR_ISACTIVER0, ncpu, &reg, false);
+        c->gicr_iactiver0 = reg;
+
+        for (i = 0; i < GIC_INTERNAL; i += 4) {
+            kvm_gicr_access(s, GICR_IPRIORITYR + i, ncpu, &reg, false);
+            c->gicr_ipriorityr[i] = extract32(reg, 0, 8);
+            c->gicr_ipriorityr[i + 1] = extract32(reg, 8, 8);
+            c->gicr_ipriorityr[i + 2] = extract32(reg, 16, 8);
+            c->gicr_ipriorityr[i + 3] = extract32(reg, 24, 8);
+        }
+    }
+
+    if (redist_typer & GICR_TYPER_PLPIS) {
+        for (ncpu = 0; ncpu < s->num_cpu; ncpu++) {
+            GICv3CPUState *c = &s->cpu[ncpu];
+
+            kvm_gicr_access(s, GICR_PROPBASER, ncpu, &regl, false);
+            kvm_gicr_access(s, GICR_PROPBASER + 4, ncpu, &regh, false);
+            c->gicr_propbaser = ((uint64_t)regh << 32) | regl;
+
+            kvm_gicr_access(s, GICR_PENDBASER, ncpu, &regl, false);
+            kvm_gicr_access(s, GICR_PENDBASER + 4, ncpu, &regh, false);
+            c->gicr_pendbaser = ((uint64_t)regh << 32) | regl;
+        }
+    }
+
+    /* Distributor state (shared between all CPUs */
+
+    kvm_gicd_access(s, GICD_STATUSR, &reg, false);
+    s->gicd_statusr[GICV3_NS] = reg;
+
+    /* GICD_IGROUPRn -> s->group bitmap */
+    kvm_dist_getbmp(s, GICD_IGROUPR, s->group);
+
+    /* GICD_ISENABLERn -> s->enabled bitmap */
+    kvm_dist_getbmp(s, GICD_ISENABLER, s->enabled);
+
+    /* Line level of irq */
+    kvm_gic_get_line_level_bmp(s, s->level);
+    /* GICD_ISPENDRn -> s->pending bitmap */
+    kvm_dist_getbmp(s, GICD_ISPENDR, s->pending);
+
+    /* GICD_ISACTIVERn -> s->active bitmap */
+    kvm_dist_getbmp(s, GICD_ISACTIVER, s->active);
+
+    /* GICD_ICFGRn -> s->trigger bitmap */
+    kvm_dist_get_edge_trigger(s, GICD_ICFGR, s->edge_trigger);
+
+    /* GICD_IPRIORITYRn -> s->gicd_ipriority[] */
+    kvm_dist_get_priority(s, GICD_IPRIORITYR, s->gicd_ipriority);
+
+    /* GICD_IROUTERn -> s->gicd_irouter[irq] */
+    for (i = GIC_INTERNAL; i < s->num_irq; i++) {
+        uint32_t offset;
+
+        offset = GICD_IROUTER + (sizeof(uint32_t) * i);
+        kvm_gicd_access(s, offset, &regl, false);
+        offset = GICD_IROUTER + (sizeof(uint32_t) * i) + 4;
+        kvm_gicd_access(s, offset, &regh, false);
+        s->gicd_irouter[i] = ((uint64_t)regh << 32) | regl;
+    }
+
+    /*****************************************************************
+     * CPU Interface(s) State
+     */
+
+    for (ncpu = 0; ncpu < s->num_cpu; ncpu++) {
+        GICv3CPUState *c = &s->cpu[ncpu];
+        int num_pri_bits;
+
+        kvm_gicc_access(s, ICC_SRE_EL1, ncpu, &c->icc_sre_el1, false);
+        kvm_gicc_access(s, ICC_CTLR_EL1, ncpu,
+                        &c->icc_ctlr_el1[GICV3_NS], false);
+        kvm_gicc_access(s, ICC_IGRPEN0_EL1, ncpu,
+                        &c->icc_igrpen[GICV3_G0], false);
+        kvm_gicc_access(s, ICC_IGRPEN1_EL1, ncpu,
+                        &c->icc_igrpen[GICV3_G1NS], false);
+        kvm_gicc_access(s, ICC_PMR_EL1, ncpu, &c->icc_pmr_el1, false);
+        kvm_gicc_access(s, ICC_BPR0_EL1, ncpu, &c->icc_bpr[GICV3_G0], false);
+        kvm_gicc_access(s, ICC_BPR1_EL1, ncpu, &c->icc_bpr[GICV3_G1NS], false);
+        num_pri_bits = ((c->icc_ctlr_el1[GICV3_NS] &
+                        ICC_CTLR_EL1_PRIBITS_MASK) >>
+                        ICC_CTLR_EL1_PRIBITS_SHIFT) + 1;
+
+        switch (num_pri_bits) {
+        case 7:
+            kvm_gicc_access(s, ICC_AP0R_EL1(3), ncpu, &reg64, false);
+            c->icc_apr[GICV3_G0][3] = reg64;
+            kvm_gicc_access(s, ICC_AP0R_EL1(2), ncpu, &reg64, false);
+            c->icc_apr[GICV3_G0][2] = reg64;
+        case 6:
+            kvm_gicc_access(s, ICC_AP0R_EL1(1), ncpu, &reg64, false);
+            c->icc_apr[GICV3_G0][1] = reg64;
+        default:
+            kvm_gicc_access(s, ICC_AP0R_EL1(0), ncpu, &reg64, false);
+            c->icc_apr[GICV3_G0][0] = reg64;
+        }
+
+        switch (num_pri_bits) {
+        case 7:
+            kvm_gicc_access(s, ICC_AP1R_EL1(3), ncpu, &reg64, false);
+            c->icc_apr[GICV3_G1NS][3] = reg64;
+            kvm_gicc_access(s, ICC_AP1R_EL1(2), ncpu, &reg64, false);
+            c->icc_apr[GICV3_G1NS][2] = reg64;
+        case 6:
+            kvm_gicc_access(s, ICC_AP1R_EL1(1), ncpu, &reg64, false);
+            c->icc_apr[GICV3_G1NS][1] = reg64;
+        default:
+            kvm_gicc_access(s, ICC_AP1R_EL1(0), ncpu, &reg64, false);
+            c->icc_apr[GICV3_G1NS][0] = reg64;
+        }
+    }
 }
 
 static void kvm_arm_gicv3_reset(DeviceState *dev)
@@ -77,6 +612,12 @@ static void kvm_arm_gicv3_reset(DeviceState *dev)
     DPRINTF("Reset\n");
 
     kgc->parent_reset(dev);
+
+    if (s->migration_blocker) {
+        DPRINTF("Cannot put kernel gic state, no kernel interface\n");
+        return;
+    }
+
     kvm_arm_gicv3_put(s);
 }
 
@@ -103,18 +644,6 @@ static void kvm_arm_gicv3_realize(DeviceState *dev, Error **errp)
 
     gicv3_init_irqs_and_mmio(s, kvm_arm_gicv3_set_irq, NULL);
 
-    /* Block migration of a KVM GICv3 device: the API for saving and restoring
-     * the state in the kernel is not yet finalised in the kernel or
-     * implemented in QEMU.
-     */
-    error_setg(&s->migration_blocker, "vGICv3 migration is not implemented");
-    migrate_add_blocker(s->migration_blocker, &local_err);
-    if (local_err) {
-        error_propagate(errp, local_err);
-        error_free(s->migration_blocker);
-        return;
-    }
-
     /* Try to create the device via the device control API */
     s->dev_fd = kvm_create_device(kvm_state, KVM_DEV_TYPE_ARM_VGIC_V3, false);
     if (s->dev_fd < 0) {
@@ -145,6 +674,18 @@ static void kvm_arm_gicv3_realize(DeviceState *dev, Error **errp)
 
         kvm_irqchip_commit_routes(kvm_state);
     }
+
+    if (!kvm_device_check_attr(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_DIST_REGS,
+                               GICD_CTLR)) {
+        error_setg(&s->migration_blocker, "This operating system kernel does "
+                                          "not support vGICv3 migration");
+        migrate_add_blocker(s->migration_blocker, &local_err);
+        if (local_err) {
+            error_propagate(errp, local_err);
+            error_free(s->migration_blocker);
+            return;
+        }
+    }
 }
 
 static void kvm_arm_gicv3_class_init(ObjectClass *klass, void *data)
diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h
index aeb801d..457118e 100644
--- a/hw/intc/gicv3_internal.h
+++ b/hw/intc/gicv3_internal.h
@@ -138,6 +138,7 @@
 #define ICC_CTLR_EL1_EOIMODE        (1U << 1)
 #define ICC_CTLR_EL1_PMHE           (1U << 6)
 #define ICC_CTLR_EL1_PRIBITS_SHIFT 8
+#define ICC_CTLR_EL1_PRIBITS_MASK   (7U << ICC_CTLR_EL1_PRIBITS_SHIFT)
 #define ICC_CTLR_EL1_IDBITS_SHIFT 11
 #define ICC_CTLR_EL1_SEIS           (1U << 14)
 #define ICC_CTLR_EL1_A3V            (1U << 15)
-- 
1.9.1

[Qemu-devel] [PATCH v8 4/5] target-arm: Add GICv3CPUState in CPUARMState struct

[vijay.kilari]

From: Vijaya Kumar K <Vijaya.Kumar@cavium.com>

Add gicv3state void pointer to CPUARMState struct
to store GICv3CPUState.

In case of usecase like CPU reset, we need to reset
GICv3CPUState of the CPU. In such scenario, this pointer
becomes handy.

This patch take care of only GICv3.

Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
---
 hw/intc/arm_gicv3_common.c | 2 ++
 hw/intc/arm_gicv3_cpuif.c  | 8 ++++++++
 hw/intc/gicv3_internal.h   | 2 ++
 target/arm/cpu.h           | 2 ++
 4 files changed, 14 insertions(+)

diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
index e62480e..79a5bd9 100644
--- a/hw/intc/arm_gicv3_common.c
+++ b/hw/intc/arm_gicv3_common.c
@@ -248,6 +248,8 @@ static void arm_gicv3_common_realize(DeviceState *dev, Error **errp)
 
         s->cpu[i].cpu = cpu;
         s->cpu[i].gic = s;
+        /* Store GICv3CPUState in CPUARMState gicv3state pointer */
+        gicv3_set_gicv3state(cpu, &s->cpu[i]);
 
         /* Pre-construct the GICR_TYPER:
          * For our implementation:
diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c
index c25ee03..7849783 100644
--- a/hw/intc/arm_gicv3_cpuif.c
+++ b/hw/intc/arm_gicv3_cpuif.c
@@ -18,6 +18,14 @@
 #include "gicv3_internal.h"
 #include "cpu.h"
 
+void gicv3_set_gicv3state(CPUState *cpu, GICv3CPUState *s)
+{
+    ARMCPU *arm_cpu = ARM_CPU(cpu);
+    CPUARMState *env = &arm_cpu->env;
+
+    env->gicv3state = (void *)s;
+};
+
 static GICv3CPUState *icc_cs_from_env(CPUARMState *env)
 {
     /* Given the CPU, find the right GICv3CPUState struct.
diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h
index 457118e..05303a5 100644
--- a/hw/intc/gicv3_internal.h
+++ b/hw/intc/gicv3_internal.h
@@ -408,4 +408,6 @@ static inline void gicv3_cache_all_target_cpustates(GICv3State *s)
     }
 }
 
+void gicv3_set_gicv3state(CPUState *cpu, GICv3CPUState *s);
+
 #endif /* QEMU_ARM_GICV3_INTERNAL_H */
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 0956a54..d2eb7bf 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -517,6 +517,8 @@ typedef struct CPUARMState {
 
     void *nvic;
     const struct arm_boot_info *boot_info;
+    /* Store GICv3CPUState to access from this struct */
+    void *gicv3state;
 } CPUARMState;
 
 /**
-- 
1.9.1

[Qemu-devel] [PATCH v8 5/5] hw/intc/arm_gicv3_kvm: Reset GICv3 cpu interface registers

[vijay.kilari]

From: Vijaya Kumar K <Vijaya.Kumar@cavium.com>

Reset CPU interface registers of GICv3 when CPU is reset.
For this, ARMCPRegInfo struct is registered with one ICC
register whose resetfn is called when cpu is reset.

All the ICC registers are reset under one single register
reset function instead of calling resetfn for each ICC
register.

Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
---
 hw/intc/arm_gicv3_kvm.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c
index cda1af4..6377dc3 100644
--- a/hw/intc/arm_gicv3_kvm.c
+++ b/hw/intc/arm_gicv3_kvm.c
@@ -604,6 +604,34 @@ static void kvm_arm_gicv3_get(GICv3State *s)
     }
 }
 
+static void arm_gicv3_icc_reset(CPUARMState *env, const ARMCPRegInfo *ri)
+{
+    ARMCPU *cpu;
+    GICv3State *s;
+    GICv3CPUState *c;
+
+    c = (GICv3CPUState *)env->gicv3state;
+    assert(!(!c || !c->cpu || !c->gic));
+
+    s = c->gic;
+    cpu = ARM_CPU(c->cpu);
+
+    /* Initialize to actual HW supported configuration */
+    kvm_device_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS,
+                      KVM_VGIC_ATTR(ICC_CTLR_EL1, cpu->mp_affinity),
+                      &c->icc_ctlr_el1[GICV3_NS], false);
+
+    c->icc_ctlr_el1[GICV3_S] = c->icc_ctlr_el1[GICV3_NS];
+    c->icc_pmr_el1 = 0;
+    c->icc_bpr[GICV3_G0] = GIC_MIN_BPR;
+    c->icc_bpr[GICV3_G1] = GIC_MIN_BPR;
+    c->icc_bpr[GICV3_G1NS] = GIC_MIN_BPR;
+
+    c->icc_sre_el1 = 0x7;
+    memset(c->icc_apr, 0, sizeof(c->icc_apr));
+    memset(c->icc_igrpen, 0, sizeof(c->icc_igrpen));
+}
+
 static void kvm_arm_gicv3_reset(DeviceState *dev)
 {
     GICv3State *s = ARM_GICV3_COMMON(dev);
@@ -621,6 +649,30 @@ static void kvm_arm_gicv3_reset(DeviceState *dev)
     kvm_arm_gicv3_put(s);
 }
 
+/*
+ * CPU interface registers of GIC needs to be reset on CPU reset.
+ * For the calling arm_gicv3_icc_reset() on CPU reset, we register
+ * below ARMCPRegInfo. As we reset the whole cpu interface under single
+ * register reset, we define only one register of CPU interface instead
+ * of defining all the registers.
+ */
+static const ARMCPRegInfo gicv3_cpuif_reginfo[] = {
+    { .name = "ICC_CTLR_EL1", .state = ARM_CP_STATE_BOTH,
+      .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 12, .opc2 = 4,
+      .type = ARM_CP_NO_RAW,
+      .access = PL1_RW,
+      .readfn = arm_cp_read_zero,
+      .writefn = arm_cp_write_ignore,
+      /*
+       * We hang the whole cpu interface reset routine off here
+       * rather than parcelling it out into one little function
+       * per register
+       */
+      .resetfn = arm_gicv3_icc_reset,
+    },
+    REGINFO_SENTINEL
+};
+
 static void kvm_arm_gicv3_realize(DeviceState *dev, Error **errp)
 {
     GICv3State *s = KVM_ARM_GICV3(dev);
@@ -644,6 +696,12 @@ static void kvm_arm_gicv3_realize(DeviceState *dev, Error **errp)
 
     gicv3_init_irqs_and_mmio(s, kvm_arm_gicv3_set_irq, NULL);
 
+    for (i = 0; i < s->num_cpu; i++) {
+        ARMCPU *cpu = ARM_CPU(qemu_get_cpu(i));
+
+        define_arm_cp_regs(cpu, gicv3_cpuif_reginfo);
+    }
+
     /* Try to create the device via the device control API */
     s->dev_fd = kvm_create_device(kvm_state, KVM_DEV_TYPE_ARM_VGIC_V3, false);
     if (s->dev_fd < 0) {
-- 
1.9.1

Re: [Qemu-devel] [PATCH v8 2/5] hw/intc/arm_gicv3_kvm: Add ICC_SRE_EL1 register to vmstate

[Auger Eric]

Hi,

On 17/02/2017 07:31, vijay.kilari@gmail.com wrote:
> From: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
> 
> To Save and Restore ICC_SRE_EL1 register introduce vmstate
> subsection and load only if non-zero.
> Also initialize icc_sre_el1 with to 0x7 in pre_load
> function.
> 
> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
> ---
>  hw/intc/arm_gicv3_common.c         | 32 ++++++++++++++++++++++++++++++++
>  include/hw/intc/arm_gicv3_common.h |  1 +
>  2 files changed, 33 insertions(+)
> 
> diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
> index 16b9b0f..e62480e 100644
> --- a/hw/intc/arm_gicv3_common.c
> +++ b/hw/intc/arm_gicv3_common.c
> @@ -70,6 +70,34 @@ static const VMStateDescription vmstate_gicv3_cpu_virt = {
>      }
>  };
>  
> +static int icc_sre_el1_reg_pre_load(void *opaque)
> +{
> +    GICv3CPUState *cs = opaque;
> +
> +    /* By default enable SRE and disable IRQ & FIQ bypass. */
> +    cs->icc_sre_el1 = 0x7;
> +    return 0;
> +}
> +
> +static bool icc_sre_el1_reg_needed(void *opaque)
> +{
> +    GICv3CPUState *cs = opaque;
> +
> +    return cs->icc_sre_el1 != 0;
Don't you tell that the reset value is always != 0. In that case the
subsection will be always sent, right?

If we don't want to send it for TCG, shouldn't we compare against 0xf
(TCG reset value you mentioned in a previous email)

Thanks

Eric
> +}
> +
> +const VMStateDescription vmstate_gicv3_cpu_sre_el1 = {
> +    .name = "arm_gicv3_cpu/sre_el1",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .pre_load = icc_sre_el1_reg_pre_load,
> +    .needed = icc_sre_el1_reg_needed,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT64(icc_sre_el1, GICv3CPUState),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
>  static const VMStateDescription vmstate_gicv3_cpu = {
>      .name = "arm_gicv3_cpu",
>      .version_id = 1,
> @@ -100,6 +128,10 @@ static const VMStateDescription vmstate_gicv3_cpu = {
>      .subsections = (const VMStateDescription * []) {
>          &vmstate_gicv3_cpu_virt,
>          NULL
> +    },
> +    .subsections = (const VMStateDescription * []) {
> +        &vmstate_gicv3_cpu_sre_el1,
> +        NULL
>      }
>  };
>  
> diff --git a/include/hw/intc/arm_gicv3_common.h b/include/hw/intc/arm_gicv3_common.h
> index 4156051..bccdfe1 100644
> --- a/include/hw/intc/arm_gicv3_common.h
> +++ b/include/hw/intc/arm_gicv3_common.h
> @@ -172,6 +172,7 @@ struct GICv3CPUState {
>      uint8_t gicr_ipriorityr[GIC_INTERNAL];
>  
>      /* CPU interface */
> +    uint64_t icc_sre_el1;
>      uint64_t icc_ctlr_el1[2];
>      uint64_t icc_pmr_el1;
>      uint64_t icc_bpr[3];
> 

Re: [Qemu-devel] [PATCH v8 4/5] target-arm: Add GICv3CPUState in CPUARMState struct

[Peter Maydell]

On 17 February 2017 at 06:31,  <vijay.kilari@gmail.com> wrote:
> From: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>
> Add gicv3state void pointer to CPUARMState struct
> to store GICv3CPUState.
>
> In case of usecase like CPU reset, we need to reset
> GICv3CPUState of the CPU. In such scenario, this pointer
> becomes handy.
>
> This patch take care of only GICv3.

I'm not sure what you mean to say with this sentence ?

>
> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com>

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

thanks
-- PMM

Re: [Qemu-devel] [PATCH v8 5/5] hw/intc/arm_gicv3_kvm: Reset GICv3 cpu interface registers

[Peter Maydell]

On 17 February 2017 at 06:31,  <vijay.kilari@gmail.com> wrote:
> From: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>
> Reset CPU interface registers of GICv3 when CPU is reset.
> For this, ARMCPRegInfo struct is registered with one ICC
> register whose resetfn is called when cpu is reset.
>
> All the ICC registers are reset under one single register
> reset function instead of calling resetfn for each ICC
> register.
>
> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
> ---
>  hw/intc/arm_gicv3_kvm.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 58 insertions(+)
>
> diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c
> index cda1af4..6377dc3 100644
> --- a/hw/intc/arm_gicv3_kvm.c
> +++ b/hw/intc/arm_gicv3_kvm.c
> @@ -604,6 +604,34 @@ static void kvm_arm_gicv3_get(GICv3State *s)
>      }
>  }
>
> +static void arm_gicv3_icc_reset(CPUARMState *env, const ARMCPRegInfo *ri)
> +{
> +    ARMCPU *cpu;
> +    GICv3State *s;
> +    GICv3CPUState *c;
> +
> +    c = (GICv3CPUState *)env->gicv3state;
> +    assert(!(!c || !c->cpu || !c->gic));

I thought I'd made a comment about these asserts on a previous
version of this code... they're pretty unnecessary since if
any of them are untrue we'll just segfault in this function.

The aim of an assert is to turn a hard-to-debug failure that
only becomes visible late into an easy-to-debug failure that
happens earlier. Assertions which don't move the failure
significantly forward in time or turn an obscure problem into a
clear one aren't really worth having.

> +    s = c->gic;
> +    cpu = ARM_CPU(c->cpu);
> +
> +    /* Initialize to actual HW supported configuration */
> +    kvm_device_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS,
> +                      KVM_VGIC_ATTR(ICC_CTLR_EL1, cpu->mp_affinity),
> +                      &c->icc_ctlr_el1[GICV3_NS], false);
> +
> +    c->icc_ctlr_el1[GICV3_S] = c->icc_ctlr_el1[GICV3_NS];
> +    c->icc_pmr_el1 = 0;
> +    c->icc_bpr[GICV3_G0] = GIC_MIN_BPR;
> +    c->icc_bpr[GICV3_G1] = GIC_MIN_BPR;
> +    c->icc_bpr[GICV3_G1NS] = GIC_MIN_BPR;
> +
> +    c->icc_sre_el1 = 0x7;
> +    memset(c->icc_apr, 0, sizeof(c->icc_apr));
> +    memset(c->icc_igrpen, 0, sizeof(c->icc_igrpen));
> +}
> +
>  static void kvm_arm_gicv3_reset(DeviceState *dev)
>  {
>      GICv3State *s = ARM_GICV3_COMMON(dev);
> @@ -621,6 +649,30 @@ static void kvm_arm_gicv3_reset(DeviceState *dev)
>      kvm_arm_gicv3_put(s);
>  }
>
> +/*
> + * CPU interface registers of GIC needs to be reset on CPU reset.
> + * For the calling arm_gicv3_icc_reset() on CPU reset, we register
> + * below ARMCPRegInfo. As we reset the whole cpu interface under single
> + * register reset, we define only one register of CPU interface instead
> + * of defining all the registers.
> + */
> +static const ARMCPRegInfo gicv3_cpuif_reginfo[] = {
> +    { .name = "ICC_CTLR_EL1", .state = ARM_CP_STATE_BOTH,
> +      .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 12, .opc2 = 4,
> +      .type = ARM_CP_NO_RAW,
> +      .access = PL1_RW,
> +      .readfn = arm_cp_read_zero,
> +      .writefn = arm_cp_write_ignore,
> +      /*
> +       * We hang the whole cpu interface reset routine off here
> +       * rather than parcelling it out into one little function
> +       * per register
> +       */
> +      .resetfn = arm_gicv3_icc_reset,
> +    },
> +    REGINFO_SENTINEL

I asked for a comment saying why we can't use ARM_CP_NOP...

thanks
-- PMM

Re: [Qemu-devel] [PATCH v8 2/5] hw/intc/arm_gicv3_kvm: Add ICC_SRE_EL1 register to vmstate

[Peter Maydell]

On 17 February 2017 at 06:31,  <vijay.kilari@gmail.com> wrote:
> From: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>
> To Save and Restore ICC_SRE_EL1 register introduce vmstate
> subsection and load only if non-zero.
> Also initialize icc_sre_el1 with to 0x7 in pre_load
> function.
>
> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
> ---
>  hw/intc/arm_gicv3_common.c         | 32 ++++++++++++++++++++++++++++++++
>  include/hw/intc/arm_gicv3_common.h |  1 +
>  2 files changed, 33 insertions(+)
>
> diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
> index 16b9b0f..e62480e 100644
> --- a/hw/intc/arm_gicv3_common.c
> +++ b/hw/intc/arm_gicv3_common.c
> @@ -70,6 +70,34 @@ static const VMStateDescription vmstate_gicv3_cpu_virt = {
>      }
>  };
>
> +static int icc_sre_el1_reg_pre_load(void *opaque)
> +{
> +    GICv3CPUState *cs = opaque;
> +
> +    /* By default enable SRE and disable IRQ & FIQ bypass. */
> +    cs->icc_sre_el1 = 0x7;

Why do we need the pre_load function? I would have
expected that reset would have given us these defaults
already.

> +    return 0;
> +}
> +
> +static bool icc_sre_el1_reg_needed(void *opaque)
> +{
> +    GICv3CPUState *cs = opaque;
> +
> +    return cs->icc_sre_el1 != 0;

I expected this to say "we need to transfer the value if
it isn't 0x7" (since the current situation of migration
is "we assume that the value is 0x7").

Something should probably fail inbound migration for TCG
if the value isn't 0x7, for that matter.

Is there a situation where KVM might allow a value other
than 0x7?

> +}
> +
> +const VMStateDescription vmstate_gicv3_cpu_sre_el1 = {
> +    .name = "arm_gicv3_cpu/sre_el1",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .pre_load = icc_sre_el1_reg_pre_load,
> +    .needed = icc_sre_el1_reg_needed,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT64(icc_sre_el1, GICv3CPUState),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
>  static const VMStateDescription vmstate_gicv3_cpu = {
>      .name = "arm_gicv3_cpu",
>      .version_id = 1,
> @@ -100,6 +128,10 @@ static const VMStateDescription vmstate_gicv3_cpu = {
>      .subsections = (const VMStateDescription * []) {
>          &vmstate_gicv3_cpu_virt,
>          NULL
> +    },
> +    .subsections = (const VMStateDescription * []) {
> +        &vmstate_gicv3_cpu_sre_el1,
> +        NULL
>      }
>  };
>
> diff --git a/include/hw/intc/arm_gicv3_common.h b/include/hw/intc/arm_gicv3_common.h
> index 4156051..bccdfe1 100644
> --- a/include/hw/intc/arm_gicv3_common.h
> +++ b/include/hw/intc/arm_gicv3_common.h
> @@ -172,6 +172,7 @@ struct GICv3CPUState {
>      uint8_t gicr_ipriorityr[GIC_INTERNAL];
>
>      /* CPU interface */
> +    uint64_t icc_sre_el1;
>      uint64_t icc_ctlr_el1[2];
>      uint64_t icc_pmr_el1;
>      uint64_t icc_bpr[3];
> --
> 1.9.1

thanks
-- PMM

Re: [Qemu-devel] [PATCH v8 2/5] hw/intc/arm_gicv3_kvm: Add ICC_SRE_EL1 register to vmstate

[Vijay Kilari]

Hi Peter,

On Fri, Feb 17, 2017 at 7:25 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 17 February 2017 at 06:31,  <vijay.kilari@gmail.com> wrote:
>> From: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>>
>> To Save and Restore ICC_SRE_EL1 register introduce vmstate
>> subsection and load only if non-zero.
>> Also initialize icc_sre_el1 with to 0x7 in pre_load
>> function.
>>
>> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>> ---
>>  hw/intc/arm_gicv3_common.c         | 32 ++++++++++++++++++++++++++++++++
>>  include/hw/intc/arm_gicv3_common.h |  1 +
>>  2 files changed, 33 insertions(+)
>>
>> diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
>> index 16b9b0f..e62480e 100644
>> --- a/hw/intc/arm_gicv3_common.c
>> +++ b/hw/intc/arm_gicv3_common.c
>> @@ -70,6 +70,34 @@ static const VMStateDescription vmstate_gicv3_cpu_virt = {
>>      }
>>  };
>>
>> +static int icc_sre_el1_reg_pre_load(void *opaque)
>> +{
>> +    GICv3CPUState *cs = opaque;
>> +
>> +    /* By default enable SRE and disable IRQ & FIQ bypass. */
>> +    cs->icc_sre_el1 = 0x7;
>
> Why do we need the pre_load function? I would have
> expected that reset would have given us these defaults
> already.
>
>> +    return 0;
>> +}
>> +
>> +static bool icc_sre_el1_reg_needed(void *opaque)
>> +{
>> +    GICv3CPUState *cs = opaque;
>> +
>> +    return cs->icc_sre_el1 != 0;
>
> I expected this to say "we need to transfer the value if
> it isn't 0x7" (since the current situation of migration
> is "we assume that the value is 0x7").
>
> Something should probably fail inbound migration for TCG
> if the value isn't 0x7, for that matter.
>
> Is there a situation where KVM might allow a value other
> than 0x7?

In KVM, the SRE_EL1 value is 0x1. During save, value
read from KVM is 0x1 though we reset to 0x7.

>
>> +}
>> +
>> +const VMStateDescription vmstate_gicv3_cpu_sre_el1 = {
>> +    .name = "arm_gicv3_cpu/sre_el1",
>> +    .version_id = 1,
>> +    .minimum_version_id = 1,
>> +    .pre_load = icc_sre_el1_reg_pre_load,
>> +    .needed = icc_sre_el1_reg_needed,
>> +    .fields = (VMStateField[]) {
>> +        VMSTATE_UINT64(icc_sre_el1, GICv3CPUState),
>> +        VMSTATE_END_OF_LIST()
>> +    }
>> +};
>> +
>>  static const VMStateDescription vmstate_gicv3_cpu = {
>>      .name = "arm_gicv3_cpu",
>>      .version_id = 1,
>> @@ -100,6 +128,10 @@ static const VMStateDescription vmstate_gicv3_cpu = {
>>      .subsections = (const VMStateDescription * []) {
>>          &vmstate_gicv3_cpu_virt,
>>          NULL
>> +    },
>> +    .subsections = (const VMStateDescription * []) {
>> +        &vmstate_gicv3_cpu_sre_el1,
>> +        NULL
>>      }
>>  };
>>
>> diff --git a/include/hw/intc/arm_gicv3_common.h b/include/hw/intc/arm_gicv3_common.h
>> index 4156051..bccdfe1 100644
>> --- a/include/hw/intc/arm_gicv3_common.h
>> +++ b/include/hw/intc/arm_gicv3_common.h
>> @@ -172,6 +172,7 @@ struct GICv3CPUState {
>>      uint8_t gicr_ipriorityr[GIC_INTERNAL];
>>
>>      /* CPU interface */
>> +    uint64_t icc_sre_el1;
>>      uint64_t icc_ctlr_el1[2];
>>      uint64_t icc_pmr_el1;
>>      uint64_t icc_bpr[3];
>> --
>> 1.9.1
>
> thanks
> -- PMM

Re: [Qemu-devel] [PATCH v8 2/5] hw/intc/arm_gicv3_kvm: Add ICC_SRE_EL1 register to vmstate

[Peter Maydell]

On 20 February 2017 at 06:21, Vijay Kilari <vijay.kilari@gmail.com> wrote:
> Hi Peter,
>
> On Fri, Feb 17, 2017 at 7:25 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
[on the guest-visible ICC_SRE_EL1 value]
>> Is there a situation where KVM might allow a value other
>> than 0x7?
>
> In KVM, the SRE_EL1 value is 0x1. During save, value
> read from KVM is 0x1 though we reset to 0x7.

0x1 meanss "System Register Interface enabled, IRQ
bypass enabled, FIQ bypass enabled". This seems
rather a weird setting, because it means "the GICv3
CPU interface functionality is disabled and the GICv3
should signal interrupts via legacy IRQ and FIQ".
Does KVM really support IRQ/FIQ bypass and does Linux
really leave it enabled rather than turning it off
by writing the value to 1?

My expectation was that the KVM GICv3 emulation would
make these bits RAO/WI like the TCG implementation.
Is there maybe a bug in the kernel side where it
doesn't implement bypass but has made these bits be
RAZ/WI rather than RAO/WI ?

thanks
-- PMM

Re: [Qemu-devel] [PATCH v8 2/5] hw/intc/arm_gicv3_kvm: Add ICC_SRE_EL1 register to vmstate

[Vijay Kilari]

Hi Christoffer,

On Mon, Feb 20, 2017 at 3:21 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 20 February 2017 at 06:21, Vijay Kilari <vijay.kilari@gmail.com> wrote:
>> Hi Peter,
>>
>> On Fri, Feb 17, 2017 at 7:25 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> [on the guest-visible ICC_SRE_EL1 value]
>>> Is there a situation where KVM might allow a value other
>>> than 0x7?
>>
>> In KVM, the SRE_EL1 value is 0x1. During save, value
>> read from KVM is 0x1 though we reset to 0x7.
>
> 0x1 meanss "System Register Interface enabled, IRQ
> bypass enabled, FIQ bypass enabled". This seems
> rather a weird setting, because it means "the GICv3
> CPU interface functionality is disabled and the GICv3
> should signal interrupts via legacy IRQ and FIQ".
> Does KVM really support IRQ/FIQ bypass and does Linux
> really leave it enabled rather than turning it off
> by writing the value to 1?
>
> My expectation was that the KVM GICv3 emulation would
> make these bits RAO/WI like the TCG implementation.
> Is there maybe a bug in the kernel side where it
> doesn't implement bypass but has made these bits be
> RAZ/WI rather than RAO/WI ?

Do you have any inputs on this?

Re: [Qemu-devel] [PATCH v8 2/5] hw/intc/arm_gicv3_kvm: Add ICC_SRE_EL1 register to vmstate

[Peter Maydell]

On 22 February 2017 at 11:56, Vijay Kilari <vijay.kilari@gmail.com> wrote:
> On Mon, Feb 20, 2017 at 3:21 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> My expectation was that the KVM GICv3 emulation would
>> make these bits RAO/WI like the TCG implementation.
>> Is there maybe a bug in the kernel side where it
>> doesn't implement bypass but has made these bits be
>> RAZ/WI rather than RAO/WI ?
>
> Do you have any inputs on this?

I talked to Marc Z who agreed this is a KVM bug -- the kernel
should have these bits be RAO/WI like TCG. I think Marc
was going to write a patch...

thanks
-- PMM

Re: [Qemu-devel] [PATCH v8 2/5] hw/intc/arm_gicv3_kvm: Add ICC_SRE_EL1 register to vmstate

[Marc Zyngier]

On 22/02/17 12:05, Peter Maydell wrote:
> On 22 February 2017 at 11:56, Vijay Kilari <vijay.kilari@gmail.com> wrote:
>> On Mon, Feb 20, 2017 at 3:21 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>>> My expectation was that the KVM GICv3 emulation would
>>> make these bits RAO/WI like the TCG implementation.
>>> Is there maybe a bug in the kernel side where it
>>> doesn't implement bypass but has made these bits be
>>> RAZ/WI rather than RAO/WI ?
>>
>> Do you have any inputs on this?
> 
> I talked to Marc Z who agreed this is a KVM bug -- the kernel
> should have these bits be RAO/WI like TCG. I think Marc
> was going to write a patch...

I'll post that in a minute.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [Qemu-devel] [PATCH v8 2/5] hw/intc/arm_gicv3_kvm: Add ICC_SRE_EL1 register to vmstate

[Peter Maydell]

On 22 February 2017 at 12:05, Peter Maydell <peter.maydell@linaro.org> wrote:
> I talked to Marc Z who agreed this is a KVM bug -- the kernel
> should have these bits be RAO/WI like TCG. I think Marc
> was going to write a patch...

...so given that, what we want on the QEMU side is:
 * in a migration preload function:
   /* if the sre_el1 subsection is not transferred this
    * means SRE_EL1 is 0x7 (which might not be the same as
    * our reset value).
    */
   cs->icc_sre_el1 = 0x7;

 * the reg_needed function should be
   return cs->icc_sre_el1 != 0x7;

and the rest of this patch is OK I think.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v8 3/5] hw/intc/arm_gicv3_kvm: Implement get/put functions

[Auger Eric]

Hi,

On 17/02/2017 07:31, vijay.kilari@gmail.com wrote:
> From: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
> 
> This actually implements pre_save and post_load methods for in-kernel
> vGICv3.
> 
> Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
I noticed some saved/restored registers are not really emulated on
kernel side (see below) but looks a good practice to do the job
according to the spec anyway.

Reviewed-by: Eric Auger <eric.auger@redhat.com>

Thanks

Eric


> [PMM:
>  * use decimal, not 0bnnn
>  * fixed typo in names of ICC_APR0R_EL1 and ICC_AP1R_EL1
>  * completely rearranged the get and put functions to read and write
>    the state in a natural order, rather than mixing distributor and
>    redistributor state together]
> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
> [Vijay:
>  * Update macro KVM_VGIC_ATTR
>  * Use 32 bit access for gicd and gicr
>  * GICD_IROUTER, GICD_TYPER, GICR_PROPBASER and GICR_PENDBASER reg
>    access  are changed from 64-bit to 32-bit access
>  * Add ICC_SRE_EL1 save and restore
>  * Dropped translate_fn mechanism and coded functions to handle
>    save and restore of edge_trigger and priority
>  * Number of APnR register saved/restored based on number of
>    priority bits supported]
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> ---
>  hw/intc/arm_gicv3_kvm.c  | 573 +++++++++++++++++++++++++++++++++++++++++++++--
>  hw/intc/gicv3_internal.h |   1 +
>  2 files changed, 558 insertions(+), 16 deletions(-)
> 
> diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c
> index d69dc47..cda1af4 100644
> --- a/hw/intc/arm_gicv3_kvm.c
> +++ b/hw/intc/arm_gicv3_kvm.c
> @@ -23,8 +23,10 @@
>  #include "qapi/error.h"
>  #include "hw/intc/arm_gicv3_common.h"
>  #include "hw/sysbus.h"
> +#include "qemu/error-report.h"
>  #include "sysemu/kvm.h"
>  #include "kvm_arm.h"
> +#include "gicv3_internal.h"
>  #include "vgic_common.h"
>  #include "migration/migration.h"
>  
> @@ -44,6 +46,32 @@
>  #define KVM_ARM_GICV3_GET_CLASS(obj) \
>       OBJECT_GET_CLASS(KVMARMGICv3Class, (obj), TYPE_KVM_ARM_GICV3)
>  
> +#define   KVM_DEV_ARM_VGIC_SYSREG(op0, op1, crn, crm, op2)         \
> +                             (ARM64_SYS_REG_SHIFT_MASK(op0, OP0) | \
> +                              ARM64_SYS_REG_SHIFT_MASK(op1, OP1) | \
> +                              ARM64_SYS_REG_SHIFT_MASK(crn, CRN) | \
> +                              ARM64_SYS_REG_SHIFT_MASK(crm, CRM) | \
> +                              ARM64_SYS_REG_SHIFT_MASK(op2, OP2))
> +
> +#define ICC_PMR_EL1     \
> +    KVM_DEV_ARM_VGIC_SYSREG(3, 0, 4, 6, 0)
> +#define ICC_BPR0_EL1    \
> +    KVM_DEV_ARM_VGIC_SYSREG(3, 0, 12, 8, 3)
> +#define ICC_AP0R_EL1(n) \
> +    KVM_DEV_ARM_VGIC_SYSREG(3, 0, 12, 8, 4 | n)
> +#define ICC_AP1R_EL1(n) \
> +    KVM_DEV_ARM_VGIC_SYSREG(3, 0, 12, 9, n)
> +#define ICC_BPR1_EL1    \
> +    KVM_DEV_ARM_VGIC_SYSREG(3, 0, 12, 12, 3)
> +#define ICC_CTLR_EL1    \
> +    KVM_DEV_ARM_VGIC_SYSREG(3, 0, 12, 12, 4)
> +#define ICC_SRE_EL1 \
> +    KVM_DEV_ARM_VGIC_SYSREG(3, 0, 12, 12, 5)
> +#define ICC_IGRPEN0_EL1 \
> +    KVM_DEV_ARM_VGIC_SYSREG(3, 0, 12, 12, 6)
> +#define ICC_IGRPEN1_EL1 \
> +    KVM_DEV_ARM_VGIC_SYSREG(3, 0, 12, 12, 7)
> +
>  typedef struct KVMARMGICv3Class {
>      ARMGICv3CommonClass parent_class;
>      DeviceRealize parent_realize;
> @@ -57,16 +85,523 @@ static void kvm_arm_gicv3_set_irq(void *opaque, int irq, int level)
>      kvm_arm_gic_set_irq(s->num_irq, irq, level);
>  }
>  
> +#define KVM_VGIC_ATTR(reg, typer) \
> +    ((typer & KVM_DEV_ARM_VGIC_V3_MPIDR_MASK) | (reg))
> +
> +static inline void kvm_gicd_access(GICv3State *s, int offset,
> +                                   uint32_t *val, bool write)
> +{
> +    kvm_device_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_DIST_REGS,
> +                      KVM_VGIC_ATTR(offset, 0),
> +                      val, write);
> +}
> +
> +static inline void kvm_gicr_access(GICv3State *s, int offset, int cpu,
> +                                   uint32_t *val, bool write)
> +{
> +    kvm_device_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_REDIST_REGS,
> +                      KVM_VGIC_ATTR(offset, s->cpu[cpu].gicr_typer),
> +                      val, write);
> +}
> +
> +static inline void kvm_gicc_access(GICv3State *s, uint64_t reg, int cpu,
> +                                   uint64_t *val, bool write)
> +{
> +    kvm_device_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS,
> +                      KVM_VGIC_ATTR(reg, s->cpu[cpu].gicr_typer),
> +                      val, write);
> +}
> +
> +static inline void kvm_gic_line_level_access(GICv3State *s, int irq, int cpu,
> +                                             uint32_t *val, bool write)
> +{
> +    kvm_device_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO,
> +                      KVM_VGIC_ATTR(irq, s->cpu[cpu].gicr_typer) |
> +                      (VGIC_LEVEL_INFO_LINE_LEVEL <<
> +                       KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_SHIFT),
> +                      val, write);
> +}
> +
> +/* Loop through each distributor IRQ related register; since bits
> + * corresponding to SPIs and PPIs are RAZ/WI when affinity routing
> + * is enabled, we skip those.
> + */
> +#define for_each_dist_irq_reg(_irq, _max, _field_width) \
> +    for (_irq = GIC_INTERNAL; _irq < _max; _irq += (32 / _field_width))
> +
> +static void kvm_dist_get_priority(GICv3State *s, uint32_t offset, uint8_t *bmp)
> +{
> +    uint32_t reg, *field;
> +    int irq;
> +
> +    field = (uint32_t *)bmp;
> +    for_each_dist_irq_reg(irq, s->num_irq, 8) {
> +        kvm_gicd_access(s, offset, &reg, false);
> +        *field = reg;
> +        offset += 4;
> +        field++;
> +    }
> +}
> +
> +static void kvm_dist_put_priority(GICv3State *s, uint32_t offset, uint8_t *bmp)
> +{
> +    uint32_t reg, *field;
> +    int irq;
> +
> +    field = (uint32_t *)bmp;
> +    for_each_dist_irq_reg(irq, s->num_irq, 8) {
> +        reg = *field;
> +        kvm_gicd_access(s, offset, &reg, true);
> +        offset += 4;
> +        field++;
> +    }
> +}
> +
> +static void kvm_dist_get_edge_trigger(GICv3State *s, uint32_t offset,
> +                                      uint32_t *bmp)
> +{
> +    uint32_t reg;
> +    int irq;
> +
> +    for_each_dist_irq_reg(irq, s->num_irq, 2) {
> +        kvm_gicd_access(s, offset, &reg, false);
> +        reg = half_unshuffle32(reg >> 1);
> +        if (irq % 32 != 0) {
> +            reg = (reg << 16);
> +        }
> +        *gic_bmp_ptr32(bmp, irq) |=  reg;
> +        offset += 4;
> +    }
> +}
> +
> +static void kvm_dist_put_edge_trigger(GICv3State *s, uint32_t offset,
> +                                      uint32_t *bmp)
> +{
> +    uint32_t reg;
> +    int irq;
> +
> +    for_each_dist_irq_reg(irq, s->num_irq, 2) {
> +        reg = *gic_bmp_ptr32(bmp, irq);
> +        if (irq % 32 != 0) {
> +            reg = (reg & 0xffff0000) >> 16;
> +        } else {
> +            reg = reg & 0xffff;
> +        }
> +        reg = half_shuffle32(reg) << 1;
> +        kvm_gicd_access(s, offset, &reg, true);
> +        offset += 4;
> +    }
> +}
> +
> +static void kvm_gic_get_line_level_bmp(GICv3State *s, uint32_t *bmp)
> +{
> +    uint32_t reg;
> +    int irq;
> +
> +    for_each_dist_irq_reg(irq, s->num_irq, 1) {
> +        kvm_gic_line_level_access(s, irq, 0, &reg, false);
> +        *gic_bmp_ptr32(bmp, irq) = reg;
> +    }
> +}
> +
> +static void kvm_gic_put_line_level_bmp(GICv3State *s, uint32_t *bmp)
> +{
> +    uint32_t reg;
> +    int irq;
> +
> +    for_each_dist_irq_reg(irq, s->num_irq, 1) {
> +        reg = *gic_bmp_ptr32(bmp, irq);
> +        kvm_gic_line_level_access(s, irq, 0, &reg, true);
> +    }
> +}
> +
> +/* Read a bitmap register group from the kernel VGIC. */
> +static void kvm_dist_getbmp(GICv3State *s, uint32_t offset, uint32_t *bmp)
> +{
> +    uint32_t reg;
> +    int irq;
> +
> +    for_each_dist_irq_reg(irq, s->num_irq, 1) {
> +        kvm_gicd_access(s, offset, &reg, false);
> +        *gic_bmp_ptr32(bmp, irq) = reg;
> +        offset += 4;
> +    }
> +}
> +
> +static void kvm_dist_putbmp(GICv3State *s, uint32_t offset,
> +                            uint32_t clroffset, uint32_t *bmp)
> +{
> +    uint32_t reg;
> +    int irq;
> +
> +    for_each_dist_irq_reg(irq, s->num_irq, 1) {
> +        /* If this bitmap is a set/clear register pair, first write to the
> +         * clear-reg to clear all bits before using the set-reg to write
> +         * the 1 bits.
> +         */
> +        if (clroffset != 0) {
> +            reg = 0;
> +            kvm_gicd_access(s, clroffset, &reg, true);
> +        }
> +        reg = *gic_bmp_ptr32(bmp, irq);
> +        kvm_gicd_access(s, offset, &reg, true);
> +        offset += 4;
> +    }
> +}
> +
> +static void kvm_arm_gicv3_check(GICv3State *s)
> +{
> +    uint32_t reg;
> +    uint32_t num_irq;
> +
> +    /* Sanity checking s->num_irq */
> +    kvm_gicd_access(s, GICD_TYPER, &reg, false);
> +    num_irq = ((reg & 0x1f) + 1) * 32;
> +
> +    if (num_irq < s->num_irq) {
> +        error_report("Model requests %u IRQs, but kernel supports max %u",
> +                     s->num_irq, num_irq);
> +        abort();
> +    }
> +}
> +
>  static void kvm_arm_gicv3_put(GICv3State *s)
>  {
> -    /* TODO */
> -    DPRINTF("Cannot put kernel gic state, no kernel interface\n");
> +    uint32_t regl, regh, reg;
> +    uint64_t reg64, redist_typer;
> +    int ncpu, i;
> +
> +    kvm_arm_gicv3_check(s);
> +
> +    kvm_gicr_access(s, GICR_TYPER, 0, &regl, false);
> +    kvm_gicr_access(s, GICR_TYPER + 4, 0, &regh, false);
> +    redist_typer = ((uint64_t)regh << 32) | regl;
> +
> +    reg = s->gicd_ctlr;
> +    kvm_gicd_access(s, GICD_CTLR, &reg, true);
> +
> +    if (redist_typer & GICR_TYPER_PLPIS) {
> +        /* Set base addresses before LPIs are enabled by GICR_CTLR write */
> +        for (ncpu = 0; ncpu < s->num_cpu; ncpu++) {
> +            GICv3CPUState *c = &s->cpu[ncpu];
> +
> +            reg64 = c->gicr_propbaser;
> +            regl = (uint32_t)reg64;
> +            kvm_gicr_access(s, GICR_PROPBASER, ncpu, &regl, true);
> +            regh = (uint32_t)(reg64 >> 32);
> +            kvm_gicr_access(s, GICR_PROPBASER + 4, ncpu, &regh, true);
> +
> +            reg64 = c->gicr_pendbaser;
> +            if (!c->gicr_ctlr & GICR_CTLR_ENABLE_LPIS) {
> +                /* Setting PTZ is advised if LPIs are disabled, to reduce
> +                 * GIC initialization time.
> +                 */
> +                reg64 |= GICR_PENDBASER_PTZ;
> +            }
> +            regl = (uint32_t)reg64;
> +            kvm_gicr_access(s, GICR_PENDBASER, ncpu, &regl, true);
> +            regh = (uint32_t)(reg64 >> 32);
> +            kvm_gicr_access(s, GICR_PENDBASER + 4, ncpu, &regh, true);
> +        }
> +    }
> +
> +    /* Redistributor state (one per CPU) */
> +
> +    for (ncpu = 0; ncpu < s->num_cpu; ncpu++) {
> +        GICv3CPUState *c = &s->cpu[ncpu];
> +
> +        reg = c->gicr_ctlr;
> +        kvm_gicr_access(s, GICR_CTLR, ncpu, &reg, true);
> +
> +        reg = c->gicr_statusr[GICV3_NS];
> +        kvm_gicr_access(s, GICR_STATUSR, ncpu, &reg, true);
I think this is vgic_mmio_read_raz, vgic_mmio_write_wi
> +
> +        reg = c->gicr_waker;
> +        kvm_gicr_access(s, GICR_WAKER, ncpu, &reg, true);
vgic_mmio_read_raz, vgic_mmio_write_wi
> +
> +        reg = c->gicr_igroupr0;
> +        kvm_gicr_access(s, GICR_IGROUPR0, ncpu, &reg, true);
vgic_mmio_read_rao, vgic_mmio_write_wi
> +
> +        reg = ~0;
> +        kvm_gicr_access(s, GICR_ICENABLER0, ncpu, &reg, true);
> +        reg = c->gicr_ienabler0;
> +        kvm_gicr_access(s, GICR_ISENABLER0, ncpu, &reg, true);
> +
> +        /* Restore config before pending so we treat level/edge correctly */
> +        reg = half_shuffle32(c->edge_trigger >> 16) << 1;
> +        kvm_gicr_access(s, GICR_ICFGR1, ncpu, &reg, true);
> +
> +        reg = c->level;
> +        kvm_gic_line_level_access(s, 0, ncpu, &reg, true);
> +
> +        reg = ~0;
> +        kvm_gicr_access(s, GICR_ICPENDR0, ncpu, &reg, true);
> +        reg = c->gicr_ipendr0;
> +        kvm_gicr_access(s, GICR_ISPENDR0, ncpu, &reg, true);
> +
> +        reg = ~0;
> +        kvm_gicr_access(s, GICR_ICACTIVER0, ncpu, &reg, true);
> +        reg = c->gicr_iactiver0;
> +        kvm_gicr_access(s, GICR_ISACTIVER0, ncpu, &reg, true);
> +
> +        for (i = 0; i < GIC_INTERNAL; i += 4) {
> +            reg = c->gicr_ipriorityr[i] |
> +                (c->gicr_ipriorityr[i + 1] << 8) |
> +                (c->gicr_ipriorityr[i + 2] << 16) |
> +                (c->gicr_ipriorityr[i + 3] << 24);
> +            kvm_gicr_access(s, GICR_IPRIORITYR + i, ncpu, &reg, true);
> +        }
> +    }
> +
> +    /* Distributor state (shared between all CPUs */
closing parenthesis
> +    reg = s->gicd_statusr[GICV3_NS];
> +    kvm_gicd_access(s, GICD_STATUSR, &reg, true);
> +
> +    /* s->enable bitmap -> GICD_ISENABLERn */
> +    kvm_dist_putbmp(s, GICD_ISENABLER, GICD_ICENABLER, s->enabled);
> +
> +    /* s->group bitmap -> GICD_IGROUPRn */
> +    kvm_dist_putbmp(s, GICD_IGROUPR, 0, s->group);
> +
> +    /* Restore targets before pending to ensure the pending state is set on
> +     * the appropriate CPU interfaces in the kernel
> +     */
> +
> +    /* s->gicd_irouter[irq] -> GICD_IROUTERn
> +     * We can't use kvm_dist_put() here because the registers are 64-bit
> +     */
> +    for (i = GIC_INTERNAL; i < s->num_irq; i++) {
> +        uint32_t offset;
> +
> +        offset = GICD_IROUTER + (sizeof(uint32_t) * i);
> +        reg = (uint32_t)s->gicd_irouter[i];
> +        kvm_gicd_access(s, offset, &reg, true);
> +
> +        offset = GICD_IROUTER + (sizeof(uint32_t) * i) + 4;
> +        reg = (uint32_t)(s->gicd_irouter[i] >> 32);
> +        kvm_gicd_access(s, offset, &reg, true);
> +    }
> +
> +    /* s->trigger bitmap -> GICD_ICFGRn
> +     * (restore configuration registers before pending IRQs so we treat
> +     * level/edge correctly)
> +     */
> +    kvm_dist_put_edge_trigger(s, GICD_ICFGR, s->edge_trigger);
> +
> +    /* s->level bitmap ->  line_level */
> +    kvm_gic_put_line_level_bmp(s, s->level);
> +
> +    /* s->pending bitmap -> GICD_ISPENDRn */
> +    kvm_dist_putbmp(s, GICD_ISPENDR, GICD_ICPENDR, s->pending);
> +
> +    /* s->active bitmap -> GICD_ISACTIVERn */
> +    kvm_dist_putbmp(s, GICD_ISACTIVER, GICD_ICACTIVER, s->active);
> +
> +    /* s->gicd_ipriority[] -> GICD_IPRIORITYRn */
> +    kvm_dist_put_priority(s, GICD_IPRIORITYR, s->gicd_ipriority);
> +
> +    /* CPU Interface state (one per CPU) */
> +
> +    for (ncpu = 0; ncpu < s->num_cpu; ncpu++) {
> +        GICv3CPUState *c = &s->cpu[ncpu];
> +        int num_pri_bits;
> +
> +        kvm_gicc_access(s, ICC_SRE_EL1, ncpu, &c->icc_sre_el1, true);
> +        kvm_gicc_access(s, ICC_CTLR_EL1, ncpu,
> +                        &c->icc_ctlr_el1[GICV3_NS], true);
> +        kvm_gicc_access(s, ICC_IGRPEN0_EL1, ncpu,
> +                        &c->icc_igrpen[GICV3_G0], true);
> +        kvm_gicc_access(s, ICC_IGRPEN1_EL1, ncpu,
> +                        &c->icc_igrpen[GICV3_G1NS], true);
> +        kvm_gicc_access(s, ICC_PMR_EL1, ncpu, &c->icc_pmr_el1, true);
> +        kvm_gicc_access(s, ICC_BPR0_EL1, ncpu, &c->icc_bpr[GICV3_G0], true);
> +        kvm_gicc_access(s, ICC_BPR1_EL1, ncpu, &c->icc_bpr[GICV3_G1NS], true);
> +
> +        num_pri_bits = ((c->icc_ctlr_el1[GICV3_NS] &
> +                        ICC_CTLR_EL1_PRIBITS_MASK) >>
> +                        ICC_CTLR_EL1_PRIBITS_SHIFT) + 1;
> +
> +        switch (num_pri_bits) {
> +        case 7:
> +            reg64 = c->icc_apr[GICV3_G0][3];
> +            kvm_gicc_access(s, ICC_AP0R_EL1(3), ncpu, &reg64, true);
> +            reg64 = c->icc_apr[GICV3_G0][2];
> +            kvm_gicc_access(s, ICC_AP0R_EL1(2), ncpu, &reg64, true);
> +        case 6:
> +            reg64 = c->icc_apr[GICV3_G0][1];
> +            kvm_gicc_access(s, ICC_AP0R_EL1(1), ncpu, &reg64, true);
> +        default:
> +            reg64 = c->icc_apr[GICV3_G0][0];
> +            kvm_gicc_access(s, ICC_AP0R_EL1(0), ncpu, &reg64, true);
> +        }
> +
> +        switch (num_pri_bits) {
> +        case 7:
> +            reg64 = c->icc_apr[GICV3_G1NS][3];
> +            kvm_gicc_access(s, ICC_AP1R_EL1(3), ncpu, &reg64, true);
> +            reg64 = c->icc_apr[GICV3_G1NS][2];
> +            kvm_gicc_access(s, ICC_AP1R_EL1(2), ncpu, &reg64, true);
> +        case 6:
> +            reg64 = c->icc_apr[GICV3_G1NS][1];
> +            kvm_gicc_access(s, ICC_AP1R_EL1(1), ncpu, &reg64, true);
> +        default:
> +            reg64 = c->icc_apr[GICV3_G1NS][0];
> +            kvm_gicc_access(s, ICC_AP1R_EL1(0), ncpu, &reg64, true);
> +        }
> +    }
>  }
>  
>  static void kvm_arm_gicv3_get(GICv3State *s)
>  {
> -    /* TODO */
> -    DPRINTF("Cannot get kernel gic state, no kernel interface\n");
> +    uint32_t regl, regh, reg;
> +    uint64_t reg64, redist_typer;
> +    int ncpu, i;
> +
> +    kvm_arm_gicv3_check(s);
> +
> +    kvm_gicr_access(s, GICR_TYPER, 0, &regl, false);
> +    kvm_gicr_access(s, GICR_TYPER + 4, 0, &regh, false);
> +    redist_typer = ((uint64_t)regh << 32) | regl;
> +
> +    kvm_gicd_access(s, GICD_CTLR, &reg, false);
> +    s->gicd_ctlr = reg;
> +
> +    /* Redistributor state (one per CPU) */
> +
> +    for (ncpu = 0; ncpu < s->num_cpu; ncpu++) {
> +        GICv3CPUState *c = &s->cpu[ncpu];
> +
> +        kvm_gicr_access(s, GICR_CTLR, ncpu, &reg, false);
> +        c->gicr_ctlr = reg;
> +
> +        kvm_gicr_access(s, GICR_STATUSR, ncpu, &reg, false);
> +        c->gicr_statusr[GICV3_NS] = reg;
> +
> +        kvm_gicr_access(s, GICR_WAKER, ncpu, &reg, false);
> +        c->gicr_waker = reg;
> +
> +        kvm_gicr_access(s, GICR_IGROUPR0, ncpu, &reg, false);
> +        c->gicr_igroupr0 = reg;
> +        kvm_gicr_access(s, GICR_ISENABLER0, ncpu, &reg, false);
> +        c->gicr_ienabler0 = reg;
> +        kvm_gicr_access(s, GICR_ICFGR1, ncpu, &reg, false);
> +        c->edge_trigger = half_unshuffle32(reg >> 1) << 16;
> +        kvm_gic_line_level_access(s, 0, ncpu, &reg, false);
> +        c->level = reg;
> +        kvm_gicr_access(s, GICR_ISPENDR0, ncpu, &reg, false);
> +        c->gicr_ipendr0 = reg;
> +        kvm_gicr_access(s, GICR_ISACTIVER0, ncpu, &reg, false);
> +        c->gicr_iactiver0 = reg;
> +
> +        for (i = 0; i < GIC_INTERNAL; i += 4) {
> +            kvm_gicr_access(s, GICR_IPRIORITYR + i, ncpu, &reg, false);
> +            c->gicr_ipriorityr[i] = extract32(reg, 0, 8);
> +            c->gicr_ipriorityr[i + 1] = extract32(reg, 8, 8);
> +            c->gicr_ipriorityr[i + 2] = extract32(reg, 16, 8);
> +            c->gicr_ipriorityr[i + 3] = extract32(reg, 24, 8);
> +        }
> +    }
> +
> +    if (redist_typer & GICR_TYPER_PLPIS) {
> +        for (ncpu = 0; ncpu < s->num_cpu; ncpu++) {
> +            GICv3CPUState *c = &s->cpu[ncpu];
> +
> +            kvm_gicr_access(s, GICR_PROPBASER, ncpu, &regl, false);
> +            kvm_gicr_access(s, GICR_PROPBASER + 4, ncpu, &regh, false);
> +            c->gicr_propbaser = ((uint64_t)regh << 32) | regl;
> +
> +            kvm_gicr_access(s, GICR_PENDBASER, ncpu, &regl, false);
> +            kvm_gicr_access(s, GICR_PENDBASER + 4, ncpu, &regh, false);
> +            c->gicr_pendbaser = ((uint64_t)regh << 32) | regl;
> +        }
> +    }
> +
> +    /* Distributor state (shared between all CPUs */
parenthesis
> +
> +    kvm_gicd_access(s, GICD_STATUSR, &reg, false);
> +    s->gicd_statusr[GICV3_NS] = reg;
> +
> +    /* GICD_IGROUPRn -> s->group bitmap */
> +    kvm_dist_getbmp(s, GICD_IGROUPR, s->group);
> +
> +    /* GICD_ISENABLERn -> s->enabled bitmap */
> +    kvm_dist_getbmp(s, GICD_ISENABLER, s->enabled);
> +
> +    /* Line level of irq */
> +    kvm_gic_get_line_level_bmp(s, s->level);
> +    /* GICD_ISPENDRn -> s->pending bitmap */
> +    kvm_dist_getbmp(s, GICD_ISPENDR, s->pending);
> +
> +    /* GICD_ISACTIVERn -> s->active bitmap */
> +    kvm_dist_getbmp(s, GICD_ISACTIVER, s->active);
> +
> +    /* GICD_ICFGRn -> s->trigger bitmap */
> +    kvm_dist_get_edge_trigger(s, GICD_ICFGR, s->edge_trigger);
> +
> +    /* GICD_IPRIORITYRn -> s->gicd_ipriority[] */
> +    kvm_dist_get_priority(s, GICD_IPRIORITYR, s->gicd_ipriority);
> +
> +    /* GICD_IROUTERn -> s->gicd_irouter[irq] */
> +    for (i = GIC_INTERNAL; i < s->num_irq; i++) {
> +        uint32_t offset;
> +
> +        offset = GICD_IROUTER + (sizeof(uint32_t) * i);
> +        kvm_gicd_access(s, offset, &regl, false);
> +        offset = GICD_IROUTER + (sizeof(uint32_t) * i) + 4;
> +        kvm_gicd_access(s, offset, &regh, false);
> +        s->gicd_irouter[i] = ((uint64_t)regh << 32) | regl;
> +    }
> +
> +    /*****************************************************************
> +     * CPU Interface(s) State
> +     */
> +
> +    for (ncpu = 0; ncpu < s->num_cpu; ncpu++) {
> +        GICv3CPUState *c = &s->cpu[ncpu];
> +        int num_pri_bits;
> +
> +        kvm_gicc_access(s, ICC_SRE_EL1, ncpu, &c->icc_sre_el1, false);
> +        kvm_gicc_access(s, ICC_CTLR_EL1, ncpu,
> +                        &c->icc_ctlr_el1[GICV3_NS], false);
> +        kvm_gicc_access(s, ICC_IGRPEN0_EL1, ncpu,
> +                        &c->icc_igrpen[GICV3_G0], false);
> +        kvm_gicc_access(s, ICC_IGRPEN1_EL1, ncpu,
> +                        &c->icc_igrpen[GICV3_G1NS], false);
> +        kvm_gicc_access(s, ICC_PMR_EL1, ncpu, &c->icc_pmr_el1, false);
> +        kvm_gicc_access(s, ICC_BPR0_EL1, ncpu, &c->icc_bpr[GICV3_G0], false);
> +        kvm_gicc_access(s, ICC_BPR1_EL1, ncpu, &c->icc_bpr[GICV3_G1NS], false);
> +        num_pri_bits = ((c->icc_ctlr_el1[GICV3_NS] &
> +                        ICC_CTLR_EL1_PRIBITS_MASK) >>
> +                        ICC_CTLR_EL1_PRIBITS_SHIFT) + 1;
> +
> +        switch (num_pri_bits) {
> +        case 7:
> +            kvm_gicc_access(s, ICC_AP0R_EL1(3), ncpu, &reg64, false);
> +            c->icc_apr[GICV3_G0][3] = reg64;
> +            kvm_gicc_access(s, ICC_AP0R_EL1(2), ncpu, &reg64, false);
> +            c->icc_apr[GICV3_G0][2] = reg64;
> +        case 6:
> +            kvm_gicc_access(s, ICC_AP0R_EL1(1), ncpu, &reg64, false);
> +            c->icc_apr[GICV3_G0][1] = reg64;
> +        default:
> +            kvm_gicc_access(s, ICC_AP0R_EL1(0), ncpu, &reg64, false);
> +            c->icc_apr[GICV3_G0][0] = reg64;
> +        }
> +
> +        switch (num_pri_bits) {
> +        case 7:
> +            kvm_gicc_access(s, ICC_AP1R_EL1(3), ncpu, &reg64, false);
> +            c->icc_apr[GICV3_G1NS][3] = reg64;
> +            kvm_gicc_access(s, ICC_AP1R_EL1(2), ncpu, &reg64, false);
> +            c->icc_apr[GICV3_G1NS][2] = reg64;
> +        case 6:
> +            kvm_gicc_access(s, ICC_AP1R_EL1(1), ncpu, &reg64, false);
> +            c->icc_apr[GICV3_G1NS][1] = reg64;
> +        default:
> +            kvm_gicc_access(s, ICC_AP1R_EL1(0), ncpu, &reg64, false);
> +            c->icc_apr[GICV3_G1NS][0] = reg64;
> +        }
> +    }
>  }
>  
>  static void kvm_arm_gicv3_reset(DeviceState *dev)
> @@ -77,6 +612,12 @@ static void kvm_arm_gicv3_reset(DeviceState *dev)
>      DPRINTF("Reset\n");
>  
>      kgc->parent_reset(dev);
> +
> +    if (s->migration_blocker) {
> +        DPRINTF("Cannot put kernel gic state, no kernel interface\n");
> +        return;
> +    }
> +
>      kvm_arm_gicv3_put(s);
>  }
>  
> @@ -103,18 +644,6 @@ static void kvm_arm_gicv3_realize(DeviceState *dev, Error **errp)
>  
>      gicv3_init_irqs_and_mmio(s, kvm_arm_gicv3_set_irq, NULL);
>  
> -    /* Block migration of a KVM GICv3 device: the API for saving and restoring
> -     * the state in the kernel is not yet finalised in the kernel or
> -     * implemented in QEMU.
> -     */
> -    error_setg(&s->migration_blocker, "vGICv3 migration is not implemented");
> -    migrate_add_blocker(s->migration_blocker, &local_err);
> -    if (local_err) {
> -        error_propagate(errp, local_err);
> -        error_free(s->migration_blocker);
> -        return;
> -    }
> -
>      /* Try to create the device via the device control API */
>      s->dev_fd = kvm_create_device(kvm_state, KVM_DEV_TYPE_ARM_VGIC_V3, false);
>      if (s->dev_fd < 0) {
> @@ -145,6 +674,18 @@ static void kvm_arm_gicv3_realize(DeviceState *dev, Error **errp)
>  
>          kvm_irqchip_commit_routes(kvm_state);
>      }
> +
> +    if (!kvm_device_check_attr(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_DIST_REGS,
> +                               GICD_CTLR)) {
> +        error_setg(&s->migration_blocker, "This operating system kernel does "
> +                                          "not support vGICv3 migration");
> +        migrate_add_blocker(s->migration_blocker, &local_err);
> +        if (local_err) {
> +            error_propagate(errp, local_err);
> +            error_free(s->migration_blocker);
> +            return;
> +        }
> +    }
>  }
>  
>  static void kvm_arm_gicv3_class_init(ObjectClass *klass, void *data)
> diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h
> index aeb801d..457118e 100644
> --- a/hw/intc/gicv3_internal.h
> +++ b/hw/intc/gicv3_internal.h
> @@ -138,6 +138,7 @@
>  #define ICC_CTLR_EL1_EOIMODE        (1U << 1)
>  #define ICC_CTLR_EL1_PMHE           (1U << 6)
>  #define ICC_CTLR_EL1_PRIBITS_SHIFT 8
> +#define ICC_CTLR_EL1_PRIBITS_MASK   (7U << ICC_CTLR_EL1_PRIBITS_SHIFT)
>  #define ICC_CTLR_EL1_IDBITS_SHIFT 11
>  #define ICC_CTLR_EL1_SEIS           (1U << 14)
>  #define ICC_CTLR_EL1_A3V            (1U << 15)
>