Skip to contentSkip to conversation

arm_gic_kvm: Disable live migration if not supported

PATCH Superseded Pavel Fedin 2 participants 6 messages Oct 15, 2015 – Oct 16, 2015

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Pavel Fedin

author
#0 lore raw

PATCH 1/1 arm_gic_kvm: Disable live migration if not supported

Currently, if the kernel does not have live migration API, the migration will still be attempted, but vGIC save/restore functions will just not do anything. This will result in a broken machine state.

This patch fixes the problem by patching vmstate_gic.unmigratable flag in runtine.

Signed-off-by Pavel Fedin <[email protected]>
---
 hw/intc/arm_gic_common.c |  2 +-
 hw/intc/arm_gic_kvm.c    | 17 ++++++-----------
 hw/intc/gic_internal.h   |  2 ++
 3 files changed, 9 insertions(+), 12 deletions(-)
hw/intc/arm_gic_common.c +1 -1
index 2b08175..1af4590 100644
@@ -60,7 +60,7 @@ static const VMStateDescription vmstate_gic_irq_state = {
     }
 };
 
-static const VMStateDescription vmstate_gic = {
+VMStateDescription vmstate_gic = {
     .name = "arm_gic",
     .version_id = 12,
     .minimum_version_id = 12,
hw/intc/arm_gic_kvm.c +6 -11
index e8b2386..ad63081 100644
@@ -307,11 +307,6 @@ static void kvm_arm_gic_put(GICState *s)
     int num_cpu;
     int num_irq;
 
-    if (!kvm_arm_gic_can_save_restore(s)) {
-            DPRINTF("Cannot put kernel gic state, no kernel interface");
-            return;
-    }
-
     /* Note: We do the restore in a slightly different order than the save
      * (where the order doesn't matter and is simply ordered according to the
      * register offset values */

@@ -411,11 +406,6 @@ static void kvm_arm_gic_get(GICState *s)
     int i;
     int cpu;
 
-    if (!kvm_arm_gic_can_save_restore(s)) {
-            DPRINTF("Cannot get kernel gic state, no kernel interface");
-            return;
-    }
-
     /*****************************************************************
      * Distributor State
      */

@@ -503,7 +493,10 @@ static void kvm_arm_gic_reset(DeviceState *dev)
     KVMARMGICClass *kgc = KVM_ARM_GIC_GET_CLASS(s);
 
     kgc->parent_reset(dev);
-    kvm_arm_gic_put(s);
+
+    if (kvm_arm_gic_can_save_restore(s)) {
+        kvm_arm_gic_put(s);
+    }
 }
 
 static void kvm_arm_gic_realize(DeviceState *dev, Error **errp)

@@ -573,6 +566,8 @@ static void kvm_arm_gic_realize(DeviceState *dev, Error **errp)
                             KVM_DEV_ARM_VGIC_GRP_ADDR,
                             KVM_VGIC_V2_ADDR_TYPE_CPU,
                             s->dev_fd);
+
+    vmstate_gic.unmigratable = !kvm_arm_gic_can_save_restore(s);
 }
 
 static void kvm_arm_gic_class_init(ObjectClass *klass, void *data)
hw/intc/gic_internal.h +2 -0
index 20c1e8a..e846f1b 100644
@@ -77,6 +77,8 @@
 #define REV_11MPCORE 0
 #define REV_NVIC 0xffffffff
 
+extern VMStateDescription vmstate_gic;
+
 void gic_set_pending_private(GICState *s, int cpu, int irq);
 uint32_t gic_acknowledge_irq(GICState *s, int cpu, MemTxAttrs attrs);
 void gic_complete_irq(GICState *s, int cpu, int irq, MemTxAttrs attrs);
Signature · 1 line
1.9.5.msysgit.0
#1 lore raw
Quoted 6 lines · Pavel Fedin

Currently, if the kernel does not have live migration API, the migration will still be attempted, but vGIC save/restore functions will just not do anything. This will result in a broken machine state.

This patch fixes the problem by patching vmstate_gic.unmigratable flag in runtine.

What kernel are you running that doesn't support state save/restore in the GICv2 implementation?

thanks
-- PMM

author
#2 lore raw

Hello!

Quoted 2 lines

What kernel are you running that doesn't support state save/restore in the GICv2 implementation?

I am not running such an old kernel. Just realized that GICv3 will need something similar, and actually GICv2 needs it too. Actually this idea came to me when i occasionally tried to migrate a machine with an ITS. My ITS was missing unmigratable flag.

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

#3 lore raw
Quoted 7 lines · Pavel Fedin

Hello!

Quoted 2 lines

What kernel are you running that doesn't support state save/restore in the GICv2 implementation?

I am not running such an old kernel. Just realized that GICv3 will need something similar, and actually GICv2 needs it too. Actually this idea came to me when i occasionally tried to migrate a machine with an ITS. My ITS was missing unmigratable flag.

Yes, GICv3 will need to handle "no migration support". But this is the GICv2 code. Unless there are kernels out in the wild which support v2 but not migration, there's no need to check for that situation here.

thanks
-- PMM

author
#4 lore raw

Hello!

Quoted 3 lines

But this is the GICv2 code. Unless there are kernels out in the wild which support v2 but not migration, there's no need to check for that situation here.

Such kernels do exist for sure, otherwise why do we have kvm_gic_can_save_restore() at all? I remember even you pointed me at it. I've just checked LXR, we have vGIC support since 3.11, but KVM_DEV_TYPE_ARM_VGIC_V2 was introduced only in 3.14.

P.S. While studying migration code i discovered migrate_add_blocker(), i guess i need to use it instead of patching VMState, because checkpatch blames me for making it non-const. So, if you ACK the idea itself, i'll reimplement it and post v2.

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

#5 lore raw
Quoted 7 lines · Pavel Fedin

Hello!

Quoted 3 lines

But this is the GICv2 code. Unless there are kernels out in the wild which support v2 but not migration, there's no need to check for that situation here.

Such kernels do exist for sure, otherwise why do we have

kvm_gic_can_save_restore() at all? I remember even you pointed me at it.

Quoted 2 lines

I've just checked LXR, we have vGIC support since 3.11, but
KVM_DEV_TYPE_ARM_VGIC_V2 was introduced only in 3.14.

OK; it makes sense to specifically block migration in this case I think.

-- PMM