qemu-ppc
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-ppc] [RFC for-2.12 4/8] spapr: Validate capabilities on migrat


From: Daniel Henrique Barboza
Subject: Re: [Qemu-ppc] [RFC for-2.12 4/8] spapr: Validate capabilities on migration
Date: Mon, 11 Dec 2017 09:57:33 -0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0



On 12/11/2017 05:08 AM, David Gibson wrote:
Now that the "pseries" machine type implements optional capabilities (well,
one so far) there's the possibility of having different capabilities
available at either end of a migration.  Although arguably a user error,
it would be nice to catch this situation and fail as gracefully as we can.

This adds code to migrate the capabilities flags.  These aren't pulled
directly into the destination's configuration since what the user has
specified on the destination command line should take precedence.  However,
they are checked against the destination capabilities.

If the source was using a capability which is absent on the destination,
we fail the migration, since that could easily cause a guest crash or other
bad behaviour.  If the source lacked a capability which is present on the
destination we warn, but allow the migration to proceed.

Signed-off-by: David Gibson <address@hidden>
---

Reviewed-by: Daniel Henrique Barboza <address@hidden>
  hw/ppc/spapr.c         |  6 ++++
  hw/ppc/spapr_caps.c    | 94 ++++++++++++++++++++++++++++++++++++++++++++++++--
  include/hw/ppc/spapr.h |  6 ++++
  3 files changed, 103 insertions(+), 3 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 0487d67894..4e4cf35587 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1587,6 +1587,11 @@ static int spapr_post_load(void *opaque, int version_id)
      sPAPRMachineState *spapr = (sPAPRMachineState *)opaque;
      int err = 0;

+    err = spapr_caps_post_migration(spapr);
+    if (err) {
+        return err;
+    }
+
      if (!object_dynamic_cast(OBJECT(spapr->ics), TYPE_ICS_KVM)) {
          CPUState *cs;
          CPU_FOREACH(cs) {
@@ -1753,6 +1758,7 @@ static const VMStateDescription vmstate_spapr = {
          &vmstate_spapr_ov5_cas,
          &vmstate_spapr_patb_entry,
          &vmstate_spapr_pending_events,
+        &vmstate_spapr_caps,
          NULL
      }
  };
diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
index 5afb4d9d5f..6bba04d60e 100644
--- a/hw/ppc/spapr_caps.c
+++ b/hw/ppc/spapr_caps.c
@@ -22,6 +22,7 @@
   * THE SOFTWARE.
   */
  #include "qemu/osdep.h"
+#include "qemu/error-report.h"
  #include "qapi/error.h"
  #include "qapi/visitor.h"
  #include "sysemu/hw_accel.h"
@@ -61,6 +62,91 @@ static sPAPRCapabilities 
default_caps_with_cpu(sPAPRMachineState *spapr, CPUStat
      return caps;
  }

+static bool spapr_caps_needed(void *opaque)
+{
+    sPAPRMachineState *spapr = opaque;
+
+    return (spapr->forced_caps.mask != 0) || (spapr->forbidden_caps.mask != 0);
+}
+
+/* This has to be called from the top-level spapr post_load, not the
+ * caps specific one.  Otherwise it wouldn't be called when the source
+ * caps are all defaults, which could still conflict with overridden
+ * caps on the destination */
+int spapr_caps_post_migration(sPAPRMachineState *spapr)
+{
+    uint64_t allcaps = 0;
+    int i;
+    bool ok = true;
+    sPAPRCapabilities dstcaps = spapr->effective_caps;
+    sPAPRCapabilities srccaps;
+
+    srccaps = default_caps_with_cpu(spapr, first_cpu);
+    srccaps.mask |= spapr->mig_forced_caps.mask;
+    srccaps.mask &= ~spapr->mig_forbidden_caps.mask;
+
+    for (i = 0; i < ARRAY_SIZE(capability_table); i++) {
+        sPAPRCapabilityInfo *info = &capability_table[i];
+
+        allcaps |= info->bit;
+
+        if ((srccaps.mask & info->bit) && !(dstcaps.mask & info->bit)) {
+            error_report("cap-%s=on in incoming stream, but off in 
destination",
+                         info->name);
+            ok = false;
+        }
+
+        if (!(srccaps.mask & info->bit) && (dstcaps.mask & info->bit)) {
+            error_report("Warning: cap-%s=off in incoming stream, but on in 
destination",
+                         info->name);
+        }
+    }
+
+    if (spapr->mig_forced_caps.mask & ~allcaps) {
+        error_report("Unknown capabilities 0x%"PRIx64" enabled in incoming 
stream",
+                     spapr->forced_caps.mask & ~allcaps);
+        ok = false;
+    }
+    if (spapr->mig_forbidden_caps.mask & ~allcaps) {
+        error_report("Warning: Unknown capabilities 0x%"PRIx64" disabled in 
incoming stream",
+                     spapr->forbidden_caps.mask & ~allcaps);
+    }
+
+    return ok ? 0 : -EINVAL;
+}
+
+static int spapr_caps_pre_save(void *opaque)
+{
+    sPAPRMachineState *spapr = opaque;
+
+    spapr->mig_forced_caps = spapr->forced_caps;
+    spapr->mig_forbidden_caps = spapr->forbidden_caps;
+    return 0;
+}
+
+static int spapr_caps_pre_load(void *opaque)
+{
+    sPAPRMachineState *spapr = opaque;
+
+    spapr->mig_forced_caps = spapr_caps(0);
+    spapr->mig_forbidden_caps = spapr_caps(0);
+    return 0;
+}
+
+const VMStateDescription vmstate_spapr_caps = {
+    .name = "spapr/caps",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = spapr_caps_needed,
+    .pre_save = spapr_caps_pre_save,
+    .pre_load = spapr_caps_pre_load,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT64(mig_forced_caps.mask, sPAPRMachineState),
+        VMSTATE_UINT64(mig_forbidden_caps.mask, sPAPRMachineState),
+        VMSTATE_END_OF_LIST()
+    },
+};
+
  static void spapr_allow_caps(sPAPRMachineState *spapr, Error **errp)
  {
      Error *err = NULL;
@@ -98,6 +184,11 @@ void spapr_caps_reset(sPAPRMachineState *spapr)

      caps = default_caps_with_cpu(spapr, first_cpu);

+    /* Remove unnecessary forced/forbidden bits (this will help us
+     * with migration) */
+    spapr->forced_caps.mask &= ~caps.mask;
+    spapr->forbidden_caps.mask &= caps.mask;
+
      caps.mask |= spapr->forced_caps.mask;
      caps.mask &= ~spapr->forbidden_caps.mask;

@@ -168,9 +259,6 @@ void spapr_validate_caps(sPAPRMachineState *spapr, Error 
**errp)
          error_setg(&local_err, "Some sPAPR capabilities set both on and off");
          return;
      }
-
-    /* Check for any caps incompatible with other caps.  Nothing to do
-     * yet */
  }

  void spapr_capability_properties(sPAPRMachineClass *smc, Error **errp)
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 4215b113f4..9b80fe72ed 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -54,6 +54,8 @@ typedef enum {
   * Capabilities
   */

+/* These bits go in the migration stream, so they can't be reassigned */
+
  /* Hardware Transactional Memory */
  #define SPAPR_CAP_HTM               0x0000000000000001ULL

@@ -142,6 +144,7 @@ struct sPAPRMachineState {
      const char *icp_type;

      sPAPRCapabilities forced_caps, forbidden_caps;
+    sPAPRCapabilities mig_forced_caps, mig_forbidden_caps;
      sPAPRCapabilities effective_caps;
  };

@@ -743,6 +746,8 @@ qemu_irq spapr_qirq(sPAPRMachineState *spapr, int irq);
  /*
   * Handling of optional capabilities
   */
+extern const VMStateDescription vmstate_spapr_caps;
+
  static inline sPAPRCapabilities spapr_caps(uint64_t mask)
  {
      sPAPRCapabilities caps = { mask };
@@ -757,5 +762,6 @@ static inline bool spapr_has_cap(sPAPRMachineState *spapr, 
uint64_t cap)
  void spapr_caps_reset(sPAPRMachineState *spapr);
  void spapr_validate_caps(sPAPRMachineState *spapr, Error **errp);
  void spapr_capability_properties(sPAPRMachineClass *smc, Error **errp);
+int spapr_caps_post_migration(sPAPRMachineState *spapr);

  #endif /* HW_SPAPR_H */




reply via email to

[Prev in Thread] Current Thread [Next in Thread]