[Qemu-devel] [PATCH v6 1/4] kernel: Add definitions for GICv3 attributes

[Qemu-devel] [PATCH v6 1/4] 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 Kumamr K <Vijaya.Kumar@cavium.com>
---
 linux-headers/asm-arm/kvm.h   | 13 +++++++++++++
 linux-headers/asm-arm64/kvm.h | 13 +++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/linux-headers/asm-arm/kvm.h b/linux-headers/asm-arm/kvm.h
index 541268c..e3dd0e1 100644
--- a/linux-headers/asm-arm/kvm.h
+++ b/linux-headers/asm-arm/kvm.h
@@ -172,10 +172,23 @@ 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..6698bdd 100644
--- a/linux-headers/asm-arm64/kvm.h
+++ b/linux-headers/asm-arm64/kvm.h
@@ -201,10 +201,23 @@ 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 v6 4/4] 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, object interface is used, which is called from
arm_cpu_reset function.

Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
---
 hw/intc/arm_gicv3_kvm.c        | 37 +++++++++++++++++++++++++++++++++++++
 include/hw/arm/linux-boot-if.h | 28 ++++++++++++++++++++++++++++
 target-arm/cpu.c               | 31 +++++++++++++++++++++++++++++++
 3 files changed, 96 insertions(+)

diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c
index 77af32d..267c2d6 100644
--- a/hw/intc/arm_gicv3_kvm.c
+++ b/hw/intc/arm_gicv3_kvm.c
@@ -29,6 +29,7 @@
 #include "gicv3_internal.h"
 #include "vgic_common.h"
 #include "migration/migration.h"
+#include "hw/arm/linux-boot-if.h"
 
 #ifdef DEBUG_GICV3_KVM
 #define DPRINTF(fmt, ...) \
@@ -604,6 +605,36 @@ static void kvm_arm_gicv3_get(GICv3State *s)
     }
 }
 
+static void  arm_gicv3_reset_cpuif(ARMDeviceResetIf *obj,
+                                      unsigned int cpu_num)
+{
+    GICv3CPUState *c;
+    GICv3State *s = ARM_GICV3_COMMON(obj);
+
+    if (!s && !s->cpu) {
+        return;
+    }
+
+    c = &s->cpu[cpu_num];
+    if (!c) {
+        return;
+    }
+
+    /* Initialize to actual HW supported configuration */
+    kvm_gicc_access(s, ICC_CTLR_EL1, cpu_num,
+                    &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);
@@ -688,6 +719,7 @@ static void kvm_arm_gicv3_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     ARMGICv3CommonClass *agcc = ARM_GICV3_COMMON_CLASS(klass);
     KVMARMGICv3Class *kgc = KVM_ARM_GICV3_CLASS(klass);
+    ARMDeviceResetIfClass *adrifc = ARM_DEVICE_RESET_IF_CLASS(klass);
 
     agcc->pre_save = kvm_arm_gicv3_get;
     agcc->post_load = kvm_arm_gicv3_put;
@@ -695,6 +727,7 @@ static void kvm_arm_gicv3_class_init(ObjectClass *klass, void *data)
     kgc->parent_reset = dc->reset;
     dc->realize = kvm_arm_gicv3_realize;
     dc->reset = kvm_arm_gicv3_reset;
+    adrifc->arm_device_reset = arm_gicv3_reset_cpuif;
 }
 
 static const TypeInfo kvm_arm_gicv3_info = {
@@ -703,6 +736,10 @@ static const TypeInfo kvm_arm_gicv3_info = {
     .instance_size = sizeof(GICv3State),
     .class_init = kvm_arm_gicv3_class_init,
     .class_size = sizeof(KVMARMGICv3Class),
+    .interfaces = (InterfaceInfo []) {
+        { TYPE_ARM_DEVICE_RESET_IF },
+        { },
+    },
 };
 
 static void kvm_arm_gicv3_register_types(void)
diff --git a/include/hw/arm/linux-boot-if.h b/include/hw/arm/linux-boot-if.h
index aba4479..4a8affd 100644
--- a/include/hw/arm/linux-boot-if.h
+++ b/include/hw/arm/linux-boot-if.h
@@ -40,4 +40,32 @@ typedef struct ARMLinuxBootIfClass {
     void (*arm_linux_init)(ARMLinuxBootIf *obj, bool secure_boot);
 } ARMLinuxBootIfClass;
 
+#define TYPE_ARM_DEVICE_RESET_IF "arm-device-reset-if"
+#define ARM_DEVICE_RESET_IF_CLASS(klass) \
+    OBJECT_CLASS_CHECK(ARMDeviceResetIfClass, (klass), TYPE_ARM_DEVICE_RESET_IF)
+#define ARM_DEVICE_RESET_IF_GET_CLASS(obj) \
+    OBJECT_GET_CLASS(ARMDeviceResetIfClass, (obj), TYPE_ARM_DEVICE_RESET_IF)
+#define ARM_DEVICE_RESET_IF(obj) \
+    INTERFACE_CHECK(ARMDeviceResetIf, (obj), TYPE_ARM_DEVICE_RESET_IF)
+
+typedef struct ARMDeviceResetIf {
+    /*< private >*/
+    Object parent_obj;
+} ARMDeviceResetIf;
+
+typedef struct ARMDeviceResetIfClass {
+    /*< private >*/
+    InterfaceClass parent_class;
+
+    /*< public >*/
+    /** arm_device_reset: Reset the device when cpu is reset is
+     * called. Some device registers like GICv3 cpu interface registers
+     * required to be reset when CPU is reset instead of GICv3 device
+     * reset. This callback is called when arm_cpu_reset is called.
+     *
+     * @obj: the object implementing this interface
+     * @cpu_num: CPU number being reset
+     */
+    void (*arm_device_reset)(ARMDeviceResetIf *obj, unsigned int cpu_num);
+} ARMDeviceResetIfClass;
 #endif
diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index 99f0dbe..44806be 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -30,6 +30,7 @@
 #include "hw/loader.h"
 #endif
 #include "hw/arm/arm.h"
+#include "hw/arm/linux-boot-if.h"
 #include "sysemu/sysemu.h"
 #include "sysemu/kvm.h"
 #include "kvm_arm.h"
@@ -113,6 +114,21 @@ static void cp_reg_check_reset(gpointer key, gpointer value,  gpointer opaque)
     assert(oldvalue == newvalue);
 }
 
+static int do_arm_device_reset(Object *obj, void *opaque)
+{
+    if (object_dynamic_cast(obj, TYPE_ARM_DEVICE_RESET_IF)) {
+        ARMDeviceResetIf *adrif = ARM_DEVICE_RESET_IF(obj);
+        ARMDeviceResetIfClass *adrifc = ARM_DEVICE_RESET_IF_GET_CLASS(obj);
+        CPUState *cpu = opaque;
+
+        if (adrifc->arm_device_reset) {
+            adrifc->arm_device_reset(adrif, cpu->cpu_index);
+        }
+    }
+    return 0;
+}
+
+
 /* CPUClass::reset() */
 static void arm_cpu_reset(CPUState *s)
 {
@@ -228,6 +244,8 @@ static void arm_cpu_reset(CPUState *s)
                               &env->vfp.standard_fp_status);
     tlb_flush(s, 1);
 
+    object_child_foreach_recursive(object_get_root(),
+                                   do_arm_device_reset, s);
 #ifndef CONFIG_USER_ONLY
     if (kvm_enabled()) {
         kvm_arm_reset_vcpu(cpu);
@@ -1595,6 +1613,19 @@ static void cpu_register(const ARMCPUInfo *info)
     g_free((void *)type_info.name);
 }
 
+static const TypeInfo arm_device_reset_if_info = {
+    .name = TYPE_ARM_DEVICE_RESET_IF,
+    .parent = TYPE_INTERFACE,
+    .class_size = sizeof(ARMDeviceResetIfClass),
+};
+
+static void arm_device_reset_register_types(void)
+{
+    type_register_static(&arm_device_reset_if_info);
+}
+
+type_init(arm_device_reset_register_types)
+
 static const TypeInfo arm_cpu_type_info = {
     .name = TYPE_ARM_CPU,
     .parent = TYPE_CPU,
-- 
1.9.1

Re: [Qemu-devel] [PATCH v6 1/4] kernel: Add definitions for GICv3 attributes

[Auger Eric]

Hi Vijay,

On 23/11/2016 13:39, vijay.kilari@gmail.com wrote:
> 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.

Did you send the complete v6 series? I only see 1/4 and 4/4 of this v6
(https://lists.gnu.org/archive/html/qemu-devel/2016-11/threads.html#04318)?
Did I miss something?

Thanks

Eric
> 
> Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
> Signed-off-by: Vijaya Kumamr K <Vijaya.Kumar@cavium.com>
> ---
>  linux-headers/asm-arm/kvm.h   | 13 +++++++++++++
>  linux-headers/asm-arm64/kvm.h | 13 +++++++++++++
>  2 files changed, 26 insertions(+)
> 
> diff --git a/linux-headers/asm-arm/kvm.h b/linux-headers/asm-arm/kvm.h
> index 541268c..e3dd0e1 100644
> --- a/linux-headers/asm-arm/kvm.h
> +++ b/linux-headers/asm-arm/kvm.h
> @@ -172,10 +172,23 @@ 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..6698bdd 100644
> --- a/linux-headers/asm-arm64/kvm.h
> +++ b/linux-headers/asm-arm64/kvm.h
> @@ -201,10 +201,23 @@ 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 */
> 

Re: [Qemu-devel] [PATCH v6 1/4] kernel: Add definitions for GICv3 attributes

[Vijay Kilari]

On Fri, Nov 25, 2016 at 1:27 PM, Auger Eric <eric.auger@redhat.com> wrote:
> Hi Vijay,
>
> On 23/11/2016 13:39, vijay.kilari@gmail.com wrote:
>> 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.
>
> Did you send the complete v6 series? I only see 1/4 and 4/4 of this v6
> (https://lists.gnu.org/archive/html/qemu-devel/2016-11/threads.html#04318)?
> Did I miss something?

Strange!. Yes, I have sent complete series.

>
> Thanks
>
> Eric
>>
>> Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
>> Signed-off-by: Vijaya Kumamr K <Vijaya.Kumar@cavium.com>
>> ---
>>  linux-headers/asm-arm/kvm.h   | 13 +++++++++++++
>>  linux-headers/asm-arm64/kvm.h | 13 +++++++++++++
>>  2 files changed, 26 insertions(+)
>>
>> diff --git a/linux-headers/asm-arm/kvm.h b/linux-headers/asm-arm/kvm.h
>> index 541268c..e3dd0e1 100644
>> --- a/linux-headers/asm-arm/kvm.h
>> +++ b/linux-headers/asm-arm/kvm.h
>> @@ -172,10 +172,23 @@ 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..6698bdd 100644
>> --- a/linux-headers/asm-arm64/kvm.h
>> +++ b/linux-headers/asm-arm64/kvm.h
>> @@ -201,10 +201,23 @@ 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 */
>>

Re: [Qemu-devel] [PATCH v6 1/4] kernel: Add definitions for GICv3 attributes

[Christoffer Dall]

On Fri, Nov 25, 2016 at 02:12:12PM +0530, Vijay Kilari wrote:
> On Fri, Nov 25, 2016 at 1:27 PM, Auger Eric <eric.auger@redhat.com> wrote:
> > Hi Vijay,
> >
> > On 23/11/2016 13:39, vijay.kilari@gmail.com wrote:
> >> 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.
> >
> > Did you send the complete v6 series? I only see 1/4 and 4/4 of this v6
> > (https://lists.gnu.org/archive/html/qemu-devel/2016-11/threads.html#04318)?
> > Did I miss something?
> 
> Strange!. Yes, I have sent complete series.
> 
I am seeing the whole thing.

Thanks,
-Christoffer

Re: [Qemu-devel] [PATCH v6 0/2] GICv3 live migration support

[Auger Eric]

Hi Vijay, Christoffer,

On 23/11/2016 13:39, vijay.kilari@gmail.com wrote:
> 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

thanks for for your replies and bouncing/forwarding the series again. I
now have all the pieces (??)

Thanks

Eric
> 
> Patch 2, is based on below implementation.
> http://patchwork.ozlabs.org/patch/626746/
> 
> Kernel patches version 6 implement this functionality.
> 
> 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.
> 
> 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 (4):
>   kernel: Add definitions for GICv3 attributes
>   hw/intc/arm_gicv3_kvm: Implement get/put functions
>   hw/intc/arm_gicv3_kvm: Save and Restore ICC_SRE_EL1 register
>   hw/intc/arm_gicv3_kvm: Reset GICv3 cpu interface registers
> 
>  hw/intc/arm_gicv3_kvm.c            | 600 ++++++++++++++++++++++++++++++++++++-
>  hw/intc/gicv3_internal.h           |   1 +
>  include/hw/arm/linux-boot-if.h     |  28 ++
>  include/hw/intc/arm_gicv3_common.h |   1 +
>  linux-headers/asm-arm/kvm.h        |  13 +
>  linux-headers/asm-arm64/kvm.h      |  13 +
>  target-arm/cpu.c                   |  31 ++
>  7 files changed, 676 insertions(+), 11 deletions(-)
> 

Re: [Qemu-devel] [PATCH v6 3/4] hw/intc/arm_gicv3_kvm: Save and Restore ICC_SRE_EL1 register

[Peter Maydell]

On 23 November 2016 at 12:39,  <vijay.kilari@gmail.com> wrote:
> From: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>
> Save and Restore ICC_SRE_EL1 register. ICC_SRE_EL1 register
> value is used by kernel to check if SRE bit is set or not.
>
> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
> ---
>  hw/intc/arm_gicv3_kvm.c            | 4 ++++
>  include/hw/intc/arm_gicv3_common.h | 1 +
>  2 files changed, 5 insertions(+)
>
> diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c
> index a317fbf..77af32d 100644
> --- a/hw/intc/arm_gicv3_kvm.c
> +++ b/hw/intc/arm_gicv3_kvm.c
> @@ -65,6 +65,8 @@
>      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 \
> @@ -404,6 +406,7 @@ static void kvm_arm_gicv3_put(GICv3State *s)
>          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,
> @@ -557,6 +560,7 @@ static void kvm_arm_gicv3_get(GICv3State *s)
>          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,
> diff --git a/include/hw/intc/arm_gicv3_common.h b/include/hw/intc/arm_gicv3_common.h
> index 341a311..183c7f8 100644
> --- a/include/hw/intc/arm_gicv3_common.h
> +++ b/include/hw/intc/arm_gicv3_common.h
> @@ -166,6 +166,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];

If you're adding a new structure field you must also update
the vmstate struct in arm_gicv3_common.c.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v6 2/4] hw/intc/arm_gicv3_kvm: Implement get/put functions

[Peter Maydell]

On 23 November 2016 at 12:39,  <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>
> [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]
> ---
> ---
>  hw/intc/arm_gicv3_kvm.c  | 559 ++++++++++++++++++++++++++++++++++++++++++++++-
>  hw/intc/gicv3_internal.h |   1 +
>  2 files changed, 549 insertions(+), 11 deletions(-)

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

thanks
-- PMM

Re: [Qemu-devel] [PATCH v6 3/4] hw/intc/arm_gicv3_kvm: Save and Restore ICC_SRE_EL1 register

[Peter Maydell]

On 28 November 2016 at 11:54, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 23 November 2016 at 12:39,  <vijay.kilari@gmail.com> wrote:
>> From: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>>
>> Save and Restore ICC_SRE_EL1 register. ICC_SRE_EL1 register
>> value is used by kernel to check if SRE bit is set or not.
>>
>> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>> ---
>>  hw/intc/arm_gicv3_kvm.c            | 4 ++++
>>  include/hw/intc/arm_gicv3_common.h | 1 +
>>  2 files changed, 5 insertions(+)
>>
>> diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c
>> index a317fbf..77af32d 100644
>> --- a/hw/intc/arm_gicv3_kvm.c
>> +++ b/hw/intc/arm_gicv3_kvm.c
>> @@ -65,6 +65,8 @@
>>      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 \
>> @@ -404,6 +406,7 @@ static void kvm_arm_gicv3_put(GICv3State *s)
>>          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,
>> @@ -557,6 +560,7 @@ static void kvm_arm_gicv3_get(GICv3State *s)
>>          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,
>> diff --git a/include/hw/intc/arm_gicv3_common.h b/include/hw/intc/arm_gicv3_common.h
>> index 341a311..183c7f8 100644
>> --- a/include/hw/intc/arm_gicv3_common.h
>> +++ b/include/hw/intc/arm_gicv3_common.h
>> @@ -166,6 +166,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];
>
> If you're adding a new structure field you must also update
> the vmstate struct in arm_gicv3_common.c.

...and if you put the patch which adds the new struct field
to GICv3CPUState and the vmstate first, then you can avoid
bumping the version numbers in vmstate because migration will
have always been disabled. So it's better to do it that way
round.

thanks
-- PMM

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

[Peter Maydell]

On 23 November 2016 at 12:39,  <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, object interface is used, which is called from
> arm_cpu_reset function.
>
> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com>

This approach doesn't handle the SMP case correctly --
when a CPU is reset then the CPU interface for that CPU
(and only that CPU) should be reset. Your code will
reset every CPU interface every time any CPU is reset.

I think it would be better to use the same approach that
the arm_gicv3_cpuif.c code uses to arrange for cpu i/f
registers to be reset, perhaps by moving the appropriate
parts of that code into the common source file.

Having the reset state depend implicitly on the kernel's
internal state (as you have here for the ICC_CTLR_EL1
state) is something I'm a bit unsure about -- what goes
wrong if you don't do that?

thanks
-- PMM

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

[Vijay Kilari]

On Mon, Nov 28, 2016 at 6:31 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 23 November 2016 at 12:39,  <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, object interface is used, which is called from
>> arm_cpu_reset function.
>>
>> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>
> This approach doesn't handle the SMP case correctly --
> when a CPU is reset then the CPU interface for that CPU
> (and only that CPU) should be reset. Your code will
> reset every CPU interface every time any CPU is reset.

arm_cpu_reset is not called when particular cpu is reset?.
Is it called for all cpus?.
OR object_child_foreach_recursive() is calling to reset cpu interfaces of
all cpus?.

>
> I think it would be better to use the same approach that
> the arm_gicv3_cpuif.c code uses to arrange for cpu i/f
> registers to be reset, perhaps by moving the appropriate
> parts of that code into the common source file.
>
> Having the reset state depend implicitly on the kernel's
> internal state (as you have here for the ICC_CTLR_EL1
> state) is something I'm a bit unsure about -- what goes
> wrong if you don't do that?

During VM boots kvm_arm_gicv3_reset() writes all
the GIC registers with reset value. kernel does not allow writing ICC_CTLR_EL1
with zeros because it validates against hw supported values.
Similarly SRE_EL1.

>
> thanks
> -- PMM

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

[Peter Maydell]

On 28 November 2016 at 16:01, Vijay Kilari <vijay.kilari@gmail.com> wrote:
> On Mon, Nov 28, 2016 at 6:31 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> On 23 November 2016 at 12:39,  <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, object interface is used, which is called from
>>> arm_cpu_reset function.
>>>
>>> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>>
>> This approach doesn't handle the SMP case correctly --
>> when a CPU is reset then the CPU interface for that CPU
>> (and only that CPU) should be reset. Your code will
>> reset every CPU interface every time any CPU is reset.
>
> arm_cpu_reset is not called when particular cpu is reset?.
> Is it called for all cpus?.

It's called to reset a particular CPU (so it will be called
once for each CPU).

> OR object_child_foreach_recursive() is calling to reset cpu
> interfaces of
> all cpus?.

It does "look through the whole graph of objects in the
simulation and call the function on anything in the
graph that implements the interface". I've just seen that
your code is doing "ignore the call if the CPU that
triggered this isn't the one we care about", though --
I missed that the first time reading the code.

Still I would prefer it if we did this with the same
mechanism for both TCG and KVM. A generic mechanism for
"let the CPU reset trigger reset of many other devices in the
system" isn't widely useful because real hardware doesn't
have that kind of action-at-a-distance behaviour.

thanks
-- PMM

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

[Vijay Kilari]

On Mon, Nov 28, 2016 at 10:05 PM, Peter Maydell
<peter.maydell@linaro.org> wrote:
> On 28 November 2016 at 16:01, Vijay Kilari <vijay.kilari@gmail.com> wrote:
>> On Mon, Nov 28, 2016 at 6:31 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>>> On 23 November 2016 at 12:39,  <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, object interface is used, which is called from
>>>> arm_cpu_reset function.
>>>>
>>>> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>>>
>>> This approach doesn't handle the SMP case correctly --
>>> when a CPU is reset then the CPU interface for that CPU
>>> (and only that CPU) should be reset. Your code will
>>> reset every CPU interface every time any CPU is reset.
>>
>> arm_cpu_reset is not called when particular cpu is reset?.
>> Is it called for all cpus?.
>
> It's called to reset a particular CPU (so it will be called
> once for each CPU).
>
>> OR object_child_foreach_recursive() is calling to reset cpu
>> interfaces of
>> all cpus?.
>
> It does "look through the whole graph of objects in the
> simulation and call the function on anything in the
> graph that implements the interface". I've just seen that
> your code is doing "ignore the call if the CPU that
> triggered this isn't the one we care about", though --
> I missed that the first time reading the code.
>
> Still I would prefer it if we did this with the same
> mechanism for both TCG and KVM. A generic mechanism for
> "let the CPU reset trigger reset of many other devices in the
> system" isn't widely useful because real hardware doesn't
> have that kind of action-at-a-distance behaviour.

To make direct call from arm_cpu_reset() to reset CPUIF,
I could not find a way to get GICv3CPUState from CPUARMState or
ARMCPU struct.

Any idea how to get GICv3CPUState?

In  hw/intc/arm_gicv3_cpuif.c implementation,
el_hook function is registered to fetch GICv3CPUState from CPUARMState
struct, but it is for TCG

>
> thanks
> -- PMM

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

[Peter Maydell]

On 30 November 2016 at 16:23, Vijay Kilari <vijay.kilari@gmail.com> wrote:
> On Mon, Nov 28, 2016 at 10:05 PM, Peter Maydell
> <peter.maydell@linaro.org> wrote:
>> Still I would prefer it if we did this with the same
>> mechanism for both TCG and KVM. A generic mechanism for
>> "let the CPU reset trigger reset of many other devices in the
>> system" isn't widely useful because real hardware doesn't
>> have that kind of action-at-a-distance behaviour.
>
> To make direct call from arm_cpu_reset() to reset CPUIF,
> I could not find a way to get GICv3CPUState from CPUARMState or
> ARMCPU struct.

You don't want to directly call from arm_cpu_reset().
Coprocessor regs registered via cpregs can have
reset functions, which get called automatically.
This is what the TCG gicv3 code already does to reset
the CPU i/f, the relevant code just needs to be
arranged so it's used for KVM too.

> Any idea how to get GICv3CPUState?
>
> In  hw/intc/arm_gicv3_cpuif.c implementation,
> el_hook function is registered to fetch GICv3CPUState
> from CPUARMState struct, but it is for TCG

Yes, you don't need the el hook.

thanks
-- PMM

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

[Vijay Kilari]

On Wed, Nov 30, 2016 at 10:29 PM, Peter Maydell
<peter.maydell@linaro.org> wrote:
> On 30 November 2016 at 16:23, Vijay Kilari <vijay.kilari@gmail.com> wrote:
>> On Mon, Nov 28, 2016 at 10:05 PM, Peter Maydell
>> <peter.maydell@linaro.org> wrote:
>>> Still I would prefer it if we did this with the same
>>> mechanism for both TCG and KVM. A generic mechanism for
>>> "let the CPU reset trigger reset of many other devices in the
>>> system" isn't widely useful because real hardware doesn't
>>> have that kind of action-at-a-distance behaviour.
>>
>> To make direct call from arm_cpu_reset() to reset CPUIF,
>> I could not find a way to get GICv3CPUState from CPUARMState or
>> ARMCPU struct.
>
> You don't want to directly call from arm_cpu_reset().
> Coprocessor regs registered via cpregs can have
> reset functions, which get called automatically.
> This is what the TCG gicv3 code already does to reset
> the CPU i/f, the relevant code just needs to be
> arranged so it's used for KVM too.

Yes, the reset functions of cpregs get CPUARMState as parameter
and still we cannot fetch GICv3CPUState from it.

The TCG code in arm_gicv3_cpuif.c is rely on el_hook to get
GICv3CPUState.
>
>> Any idea how to get GICv3CPUState?
>>
>> In  hw/intc/arm_gicv3_cpuif.c implementation,
>> el_hook function is registered to fetch GICv3CPUState
>> from CPUARMState struct, but it is for TCG
>
> Yes, you don't need the el hook.

Without this is there a way to get GICv3CPUState for KVM?
I am not familiar with this code.

>
> thanks
> -- PMM

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

[Vijay Kilari]

Hi Peter,

On Thu, Dec 1, 2016 at 3:40 PM, Vijay Kilari <vijay.kilari@gmail.com> wrote:
> On Wed, Nov 30, 2016 at 10:29 PM, Peter Maydell
> <peter.maydell@linaro.org> wrote:
>> On 30 November 2016 at 16:23, Vijay Kilari <vijay.kilari@gmail.com> wrote:
>>> On Mon, Nov 28, 2016 at 10:05 PM, Peter Maydell
>>> <peter.maydell@linaro.org> wrote:
>>>> Still I would prefer it if we did this with the same
>>>> mechanism for both TCG and KVM. A generic mechanism for
>>>> "let the CPU reset trigger reset of many other devices in the
>>>> system" isn't widely useful because real hardware doesn't
>>>> have that kind of action-at-a-distance behaviour.
>>>
>>> To make direct call from arm_cpu_reset() to reset CPUIF,
>>> I could not find a way to get GICv3CPUState from CPUARMState or
>>> ARMCPU struct.
>>
>> You don't want to directly call from arm_cpu_reset().
>> Coprocessor regs registered via cpregs can have
>> reset functions, which get called automatically.
>> This is what the TCG gicv3 code already does to reset
>> the CPU i/f, the relevant code just needs to be
>> arranged so it's used for KVM too.
>
> Yes, the reset functions of cpregs get CPUARMState as parameter
> and still we cannot fetch GICv3CPUState from it.

I propose to add new variable to CPUARMState to store
GICV3CPUState to able to access when cpregs reset is called.
Is it ok?

>
> The TCG code in arm_gicv3_cpuif.c is rely on el_hook to get
> GICv3CPUState.
>>
>>> Any idea how to get GICv3CPUState?
>>>
>>> In  hw/intc/arm_gicv3_cpuif.c implementation,
>>> el_hook function is registered to fetch GICv3CPUState
>>> from CPUARMState struct, but it is for TCG
>>
>> Yes, you don't need the el hook.
>
> Without this is there a way to get GICv3CPUState for KVM?
> I am not familiar with this code.
>
>>
>> thanks
>> -- PMM

Re: [Qemu-devel] [PATCH v6 2/4] hw/intc/arm_gicv3_kvm: Implement get/put functions

[Auger Eric]

Hi Vijaya,

On 23/11/2016 13:39, 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>
> [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]
> ---
> ---
>  hw/intc/arm_gicv3_kvm.c  | 559 ++++++++++++++++++++++++++++++++++++++++++++++-
>  hw/intc/gicv3_internal.h |   1 +
>  2 files changed, 549 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c
> index 199a439..a317fbf 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,30 @@
>  #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_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 +83,521 @@ 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);
There is a mismatch here between the kernel define
(KVM_DEV_ARM_VGIC_CPU_SYSREGS in v10) and the QEMU define. I think we
should rather fix that at kernel API level.
./scripts/update-linux-headers.sh normally leaves the kernel header
untouched.

Thanks

Eric
> +}
> +
> +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_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_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 +608,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);
>  }
>  
> @@ -122,13 +659,6 @@ static void kvm_arm_gicv3_realize(DeviceState *dev, Error **errp)
>      kvm_arm_register_device(&s->iomem_redist, -1, KVM_DEV_ARM_VGIC_GRP_ADDR,
>                              KVM_VGIC_V3_ADDR_TYPE_REDIST, s->dev_fd);
>  
> -    /* 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);
> -
>      if (kvm_has_gsi_routing()) {
>          /* set up irq routing */
>          kvm_init_irq_routing(kvm_state);
> @@ -140,6 +670,13 @@ 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);
> +    }
>  }
>  
>  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 8f3567e..179556f 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)
>