Blame SOURCES/kvm-spapr-Validate-capabilities-on-migration.patch

9bac43
From a6549068139448cfd38e4b1e602fbaba5ed32a97 Mon Sep 17 00:00:00 2001
9bac43
From: David Gibson <dgibson@redhat.com>
9bac43
Date: Fri, 19 Jan 2018 02:34:38 +0100
9bac43
Subject: [PATCH 10/21] spapr: Validate capabilities on migration
9bac43
9bac43
RH-Author: David Gibson <dgibson@redhat.com>
9bac43
Message-id: <20180119023442.28577-4-dgibson@redhat.com>
9bac43
Patchwork-id: 78669
9bac43
O-Subject: [RHEL-7.5 qemu-kvm-rhev PATCH 3/7] spapr: Validate capabilities on migration
9bac43
Bugzilla: 1523414
9bac43
RH-Acked-by: Laurent Vivier <lvivier@redhat.com>
9bac43
RH-Acked-by: Thomas Huth <thuth@redhat.com>
9bac43
RH-Acked-by: Miroslav Rezanina <mrezanin@redhat.com>
9bac43
9bac43
From: David Gibson <david@gibson.dropbear.id.au>
9bac43
9bac43
Now that the "pseries" machine type implements optional capabilities (well,
9bac43
one so far) there's the possibility of having different capabilities
9bac43
available at either end of a migration.  Although arguably a user error,
9bac43
it would be nice to catch this situation and fail as gracefully as we can.
9bac43
9bac43
This adds code to migrate the capabilities flags.  These aren't pulled
9bac43
directly into the destination's configuration since what the user has
9bac43
specified on the destination command line should take precedence.  However,
9bac43
they are checked against the destination capabilities.
9bac43
9bac43
If the source was using a capability which is absent on the destination,
9bac43
we fail the migration, since that could easily cause a guest crash or other
9bac43
bad behaviour.  If the source lacked a capability which is present on the
9bac43
destination we warn, but allow the migration to proceed.
9bac43
9bac43
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
9bac43
Reviewed-by: Greg Kurz <groug@kaod.org>
9bac43
(cherry picked from commit be85537d654565e35e359a74b46fc08b7956525c)
9bac43
9bac43
Adjusted because we don't have 44b1ff31 "migration: pre_save return
9bac43
int" downstream, which means spapr_caps_pre_save() needs a different
9bac43
type.
9bac43
9bac43
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1523414
9bac43
9bac43
Signed-off-by: David Gibson <dgibson@redhat.com>
9bac43
Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
9bac43
---
9bac43
 hw/ppc/spapr.c         |  6 ++++
9bac43
 hw/ppc/spapr_caps.c    | 95 ++++++++++++++++++++++++++++++++++++++++++++++++--
9bac43
 include/hw/ppc/spapr.h |  6 ++++
9bac43
 3 files changed, 104 insertions(+), 3 deletions(-)
9bac43
9bac43
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
9bac43
index 2068a1f..846b906 100644
9bac43
--- a/hw/ppc/spapr.c
9bac43
+++ b/hw/ppc/spapr.c
9bac43
@@ -1547,6 +1547,11 @@ static int spapr_post_load(void *opaque, int version_id)
9bac43
     sPAPRMachineState *spapr = (sPAPRMachineState *)opaque;
9bac43
     int err = 0;
9bac43
 
9bac43
+    err = spapr_caps_post_migration(spapr);
9bac43
+    if (err) {
9bac43
+        return err;
9bac43
+    }
9bac43
+
9bac43
     if (!object_dynamic_cast(OBJECT(spapr->ics), TYPE_ICS_KVM)) {
9bac43
         CPUState *cs;
9bac43
         CPU_FOREACH(cs) {
9bac43
@@ -1713,6 +1718,7 @@ static const VMStateDescription vmstate_spapr = {
9bac43
         &vmstate_spapr_ov5_cas,
9bac43
         &vmstate_spapr_patb_entry,
9bac43
         &vmstate_spapr_pending_events,
9bac43
+        &vmstate_spapr_caps,
9bac43
         NULL
9bac43
     }
9bac43
 };
9bac43
diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
9bac43
index 3b35b91..2bdc202 100644
9bac43
--- a/hw/ppc/spapr_caps.c
9bac43
+++ b/hw/ppc/spapr_caps.c
9bac43
@@ -22,6 +22,7 @@
9bac43
  * THE SOFTWARE.
9bac43
  */
9bac43
 #include "qemu/osdep.h"
9bac43
+#include "qemu/error-report.h"
9bac43
 #include "qapi/error.h"
9bac43
 #include "qapi/visitor.h"
9bac43
 #include "sysemu/hw_accel.h"
9bac43
@@ -83,6 +84,92 @@ static sPAPRCapabilities default_caps_with_cpu(sPAPRMachineState *spapr,
9bac43
     return caps;
9bac43
 }
9bac43
 
9bac43
+static bool spapr_caps_needed(void *opaque)
9bac43
+{
9bac43
+    sPAPRMachineState *spapr = opaque;
9bac43
+
9bac43
+    return (spapr->forced_caps.mask != 0) || (spapr->forbidden_caps.mask != 0);
9bac43
+}
9bac43
+
9bac43
+/* This has to be called from the top-level spapr post_load, not the
9bac43
+ * caps specific one.  Otherwise it wouldn't be called when the source
9bac43
+ * caps are all defaults, which could still conflict with overridden
9bac43
+ * caps on the destination */
9bac43
+int spapr_caps_post_migration(sPAPRMachineState *spapr)
9bac43
+{
9bac43
+    uint64_t allcaps = 0;
9bac43
+    int i;
9bac43
+    bool ok = true;
9bac43
+    sPAPRCapabilities dstcaps = spapr->effective_caps;
9bac43
+    sPAPRCapabilities srccaps;
9bac43
+
9bac43
+    srccaps = default_caps_with_cpu(spapr, first_cpu);
9bac43
+    srccaps.mask |= spapr->mig_forced_caps.mask;
9bac43
+    srccaps.mask &= ~spapr->mig_forbidden_caps.mask;
9bac43
+
9bac43
+    for (i = 0; i < ARRAY_SIZE(capability_table); i++) {
9bac43
+        sPAPRCapabilityInfo *info = &capability_table[i];
9bac43
+
9bac43
+        allcaps |= info->flag;
9bac43
+
9bac43
+        if ((srccaps.mask & info->flag) && !(dstcaps.mask & info->flag)) {
9bac43
+            error_report("cap-%s=on in incoming stream, but off in destination",
9bac43
+                         info->name);
9bac43
+            ok = false;
9bac43
+        }
9bac43
+
9bac43
+        if (!(srccaps.mask & info->flag) && (dstcaps.mask & info->flag)) {
9bac43
+            warn_report("cap-%s=off in incoming stream, but on in destination",
9bac43
+                         info->name);
9bac43
+        }
9bac43
+    }
9bac43
+
9bac43
+    if (spapr->mig_forced_caps.mask & ~allcaps) {
9bac43
+        error_report(
9bac43
+            "Unknown capabilities 0x%"PRIx64" enabled in incoming stream",
9bac43
+            spapr->mig_forced_caps.mask & ~allcaps);
9bac43
+        ok = false;
9bac43
+    }
9bac43
+    if (spapr->mig_forbidden_caps.mask & ~allcaps) {
9bac43
+        warn_report(
9bac43
+            "Unknown capabilities 0x%"PRIx64" disabled in incoming stream",
9bac43
+            spapr->mig_forbidden_caps.mask & ~allcaps);
9bac43
+    }
9bac43
+
9bac43
+    return ok ? 0 : -EINVAL;
9bac43
+}
9bac43
+
9bac43
+static void spapr_caps_pre_save(void *opaque)
9bac43
+{
9bac43
+    sPAPRMachineState *spapr = opaque;
9bac43
+
9bac43
+    spapr->mig_forced_caps = spapr->forced_caps;
9bac43
+    spapr->mig_forbidden_caps = spapr->forbidden_caps;
9bac43
+}
9bac43
+
9bac43
+static int spapr_caps_pre_load(void *opaque)
9bac43
+{
9bac43
+    sPAPRMachineState *spapr = opaque;
9bac43
+
9bac43
+    spapr->mig_forced_caps = spapr_caps(0);
9bac43
+    spapr->mig_forbidden_caps = spapr_caps(0);
9bac43
+    return 0;
9bac43
+}
9bac43
+
9bac43
+const VMStateDescription vmstate_spapr_caps = {
9bac43
+    .name = "spapr/caps",
9bac43
+    .version_id = 1,
9bac43
+    .minimum_version_id = 1,
9bac43
+    .needed = spapr_caps_needed,
9bac43
+    .pre_save = spapr_caps_pre_save,
9bac43
+    .pre_load = spapr_caps_pre_load,
9bac43
+    .fields = (VMStateField[]) {
9bac43
+        VMSTATE_UINT64(mig_forced_caps.mask, sPAPRMachineState),
9bac43
+        VMSTATE_UINT64(mig_forbidden_caps.mask, sPAPRMachineState),
9bac43
+        VMSTATE_END_OF_LIST()
9bac43
+    },
9bac43
+};
9bac43
+
9bac43
 void spapr_caps_reset(sPAPRMachineState *spapr)
9bac43
 {
9bac43
     Error *local_err = NULL;
9bac43
@@ -92,6 +179,11 @@ void spapr_caps_reset(sPAPRMachineState *spapr)
9bac43
     /* First compute the actual set of caps we're running with.. */
9bac43
     caps = default_caps_with_cpu(spapr, first_cpu);
9bac43
 
9bac43
+    /* Remove unnecessary forced/forbidden bits (this will help us
9bac43
+     * with migration) */
9bac43
+    spapr->forced_caps.mask &= ~caps.mask;
9bac43
+    spapr->forbidden_caps.mask &= caps.mask;
9bac43
+
9bac43
     caps.mask |= spapr->forced_caps.mask;
9bac43
     caps.mask &= ~spapr->forbidden_caps.mask;
9bac43
 
9bac43
@@ -175,9 +267,6 @@ void spapr_caps_validate(sPAPRMachineState *spapr, Error **errp)
9bac43
         error_setg(errp, "Some sPAPR capabilities set both on and off");
9bac43
         return;
9bac43
     }
9bac43
-
9bac43
-    /* Check for any caps incompatible with other caps.  Nothing to do
9bac43
-     * yet */
9bac43
 }
9bac43
 
9bac43
 void spapr_caps_add_properties(sPAPRMachineClass *smc, Error **errp)
9bac43
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
9bac43
index 77dc3fb..f32967a 100644
9bac43
--- a/include/hw/ppc/spapr.h
9bac43
+++ b/include/hw/ppc/spapr.h
9bac43
@@ -54,6 +54,8 @@ typedef enum {
9bac43
  * Capabilities
9bac43
  */
9bac43
 
9bac43
+/* These bits go in the migration stream, so they can't be reassigned */
9bac43
+
9bac43
 /* Hardware Transactional Memory */
9bac43
 #define SPAPR_CAP_HTM               0x0000000000000001ULL
9bac43
 
9bac43
@@ -144,6 +146,7 @@ struct sPAPRMachineState {
9bac43
     const char *icp_type;
9bac43
 
9bac43
     sPAPRCapabilities forced_caps, forbidden_caps;
9bac43
+    sPAPRCapabilities mig_forced_caps, mig_forbidden_caps;
9bac43
     sPAPRCapabilities effective_caps;
9bac43
 };
9bac43
 
9bac43
@@ -726,6 +729,8 @@ void spapr_do_system_reset_on_cpu(CPUState *cs, run_on_cpu_data arg);
9bac43
 /*
9bac43
  * Handling of optional capabilities
9bac43
  */
9bac43
+extern const VMStateDescription vmstate_spapr_caps;
9bac43
+
9bac43
 static inline sPAPRCapabilities spapr_caps(uint64_t mask)
9bac43
 {
9bac43
     sPAPRCapabilities caps = { mask };
9bac43
@@ -740,5 +745,6 @@ static inline bool spapr_has_cap(sPAPRMachineState *spapr, uint64_t cap)
9bac43
 void spapr_caps_reset(sPAPRMachineState *spapr);
9bac43
 void spapr_caps_validate(sPAPRMachineState *spapr, Error **errp);
9bac43
 void spapr_caps_add_properties(sPAPRMachineClass *smc, Error **errp);
9bac43
+int spapr_caps_post_migration(sPAPRMachineState *spapr);
9bac43
 
9bac43
 #endif /* HW_SPAPR_H */
9bac43
-- 
9bac43
1.8.3.1
9bac43