[Qemu-devel] [PATCH] arm_gic_kvm: Disable live migration if not supported

[Qemu-devel] [PATCH] arm_gic_kvm: Disable live migration if not supported

[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.

Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
---
 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(-)

diff --git a/hw/intc/arm_gic_common.c b/hw/intc/arm_gic_common.c
index 2b08175..1af4590 100644
--- a/hw/intc/arm_gic_common.c
+++ b/hw/intc/arm_gic_common.c
@@ -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,
diff --git a/hw/intc/arm_gic_kvm.c b/hw/intc/arm_gic_kvm.c
index e8b2386..ad63081 100644
--- a/hw/intc/arm_gic_kvm.c
+++ b/hw/intc/arm_gic_kvm.c
@@ -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)
diff --git a/hw/intc/gic_internal.h b/hw/intc/gic_internal.h
index 20c1e8a..e846f1b 100644
--- a/hw/intc/gic_internal.h
+++ b/hw/intc/gic_internal.h
@@ -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);
-- 
1.9.5.msysgit.0

Re: [Qemu-devel] [PATCH] arm_gic_kvm: Disable live migration if not supported

[Peter Maydell]

On 15 October 2015 at 13:45, Pavel Fedin <p.fedin@samsung.com> wrote:
> 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

Re: [Qemu-devel] [PATCH] arm_gic_kvm: Disable live migration if not supported

[Pavel Fedin]

 Hello!

> 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

Re: [Qemu-devel] [PATCH] arm_gic_kvm: Disable live migration if not supported

[Peter Maydell]

On 16 October 2015 at 07:40, Pavel Fedin <p.fedin@samsung.com> wrote:
>  Hello!
>
>> 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

Re: [Qemu-devel] [PATCH] arm_gic_kvm: Disable live migration if not supported

[Pavel Fedin]

 Hello!

> 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

Re: [Qemu-devel] [PATCH] arm_gic_kvm: Disable live migration if not supported

[Peter Maydell]

On 16 October 2015 at 08:52, Pavel Fedin <p.fedin@samsung.com> wrote:
>  Hello!
>
>> 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.

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

-- PMM