[Qemu-devel] [PULL 00/11] ppc-for-2.8 queue 20161123

[Qemu-devel] [PULL 00/11] ppc-for-2.8 queue 20161123

[David Gibson]

The following changes since commit 00227fefd2059464cd2f59aed29944874c630e2f:

  Update version for v2.8.0-rc1 release (2016-11-22 22:29:08 +0000)

are available in the git repository at:

  git://github.com/dgibson/qemu.git tags/ppc-for-2.8-20161123

for you to fetch changes up to 5c4537bded40640b166ec77e112592174b048c21:

  spapr: Fix 2.7<->2.8 migration of PCI host bridge (2016-11-23 12:00:48 +1100)

----------------------------------------------------------------
ppc patch queue 2016-11-23

Here's the first set of 2.8 hard freeze bugfixes for ppc.

The biggest thing here is a batch of fixes for migration breakages in
both 2.7 and current 2.8.  Alas, there is at least one more migration
problem, which prevents memory unplug after a migration.  I hoped to
include a fix for that here, but it turned out to have some problems
bigger than those it was solving.  So, I expect at least one more hard
freeze pull request.

There are also a few other assorted bug fixes.

----------------------------------------------------------------
Benjamin Herrenschmidt (1):
      ppc: Make uninorth interrupt swizzling identical to Grackle

David Gibson (5):
      target-ppc: Fix CPU migration from qemu-2.6 <-> later versions
      migration: Add VMSTATE_UINTTL_TEST()
      target-ppc: Allow eventual removal of old migration mistakes
      Revert "spapr: Fix migration of PCI host bridges from qemu-2.7"
      spapr: Fix 2.7<->2.8 migration of PCI host bridge

Jose Ricardo Ziviani (1):
      target-ppc: fix index array of national digits

Michael Roth (1):
      spapr: migration support for CAS-negotiated option vectors

Thomas Huth (2):
      tests/postcopy: Use KVM on ppc64 only if it is KVM-HV
      hw/char/spapr_vty: Return amount of free buffer entries in vty_can_receive()

Vladimir Svoboda (1):
      ppc: BOOK3E: nothing should be done when MSR:PR is set

 hw/char/spapr_vty.c         |  2 +-
 hw/pci-host/uninorth.c      |  4 +--
 hw/ppc/spapr.c              | 76 +++++++++++++++++++++++++++++++++++++++++++++
 hw/ppc/spapr_ovec.c         | 12 +++++++
 hw/ppc/spapr_pci.c          | 35 +++++++++++++++------
 include/hw/pci-host/spapr.h |  6 ++++
 include/hw/ppc/spapr_ovec.h |  4 +++
 include/migration/cpu.h     |  4 +++
 target-ppc/cpu.h            |  7 +++++
 target-ppc/helper_regs.h    | 11 ++++---
 target-ppc/int_helper.c     |  4 +--
 target-ppc/machine.c        | 39 ++++++++++++++++++++---
 target-ppc/translate_init.c |  6 ++++
 tests/postcopy-test.c       | 12 ++++---
 14 files changed, 195 insertions(+), 27 deletions(-)

[Qemu-devel] [PULL 01/11] tests/postcopy: Use KVM on ppc64 only if it is KVM-HV

[David Gibson]

From: Thomas Huth <thuth@redhat.com>

The ppc64 postcopy test does not work with KVM-PR, and it is also
causing annoying warning messages when run on a x86 host. So let's
use KVM here only if we know that we're running with KVM-HV (which
automatically also means that we're running on a ppc64 host), and
fall back to TCG otherwise.

Signed-off-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 tests/postcopy-test.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/tests/postcopy-test.c b/tests/postcopy-test.c
index d6613c5..dafe8be 100644
--- a/tests/postcopy-test.c
+++ b/tests/postcopy-test.c
@@ -380,17 +380,21 @@ static void test_migrate(void)
                                   " -incoming %s",
                                   tmpfs, bootpath, uri);
     } else if (strcmp(arch, "ppc64") == 0) {
+        const char *accel;
+
+        /* On ppc64, the test only works with kvm-hv, but not with kvm-pr */
+        accel = access("/sys/module/kvm_hv", F_OK) ? "tcg" : "kvm:tcg";
         init_bootfile_ppc(bootpath);
-        cmd_src = g_strdup_printf("-machine accel=kvm:tcg -m 256M"
+        cmd_src = g_strdup_printf("-machine accel=%s -m 256M"
                                   " -name pcsource,debug-threads=on"
                                   " -serial file:%s/src_serial"
                                   " -drive file=%s,if=pflash,format=raw",
-                                  tmpfs, bootpath);
-        cmd_dst = g_strdup_printf("-machine accel=kvm:tcg -m 256M"
+                                  accel, tmpfs, bootpath);
+        cmd_dst = g_strdup_printf("-machine accel=%s -m 256M"
                                   " -name pcdest,debug-threads=on"
                                   " -serial file:%s/dest_serial"
                                   " -incoming %s",
-                                  tmpfs, uri);
+                                  accel, tmpfs, uri);
     } else {
         g_assert_not_reached();
     }
-- 
2.7.4

[Qemu-devel] [PULL 02/11] spapr: migration support for CAS-negotiated option vectors

[David Gibson]

From: Michael Roth <mdroth@linux.vnet.ibm.com>

With the additional of the OV5_HP_EVT option vector, we now have
certain functionality (namely, memory unplug) that checks at run-time
for whether or not the guest negotiated the option via CAS. Because
we don't currently migrate these negotiated values, we are unable
to unplug memory from a guest after it's been migrated until after
the guest is rebooted and CAS-negotiation is repeated.

This patch fixes this by adding CAS-negotiated options to the
migration stream. We do this using a subsection, since the
negotiated value of OV5_HP_EVT is the only option currently needed
to maintain proper functionality for a running guest.

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/spapr.c              | 66 +++++++++++++++++++++++++++++++++++++++++++++
 hw/ppc/spapr_ovec.c         | 12 +++++++++
 include/hw/ppc/spapr_ovec.h |  4 +++
 3 files changed, 82 insertions(+)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 0cbab24..54b88d3 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1267,6 +1267,68 @@ static bool version_before_3(void *opaque, int version_id)
     return version_id < 3;
 }
 
+static bool spapr_ov5_cas_needed(void *opaque)
+{
+    sPAPRMachineState *spapr = opaque;
+    sPAPROptionVector *ov5_mask = spapr_ovec_new();
+    sPAPROptionVector *ov5_legacy = spapr_ovec_new();
+    sPAPROptionVector *ov5_removed = spapr_ovec_new();
+    bool cas_needed;
+
+    /* Prior to the introduction of sPAPROptionVector, we had two option
+     * vectors we dealt with: OV5_FORM1_AFFINITY, and OV5_DRCONF_MEMORY.
+     * Both of these options encode machine topology into the device-tree
+     * in such a way that the now-booted OS should still be able to interact
+     * appropriately with QEMU regardless of what options were actually
+     * negotiatied on the source side.
+     *
+     * As such, we can avoid migrating the CAS-negotiated options if these
+     * are the only options available on the current machine/platform.
+     * Since these are the only options available for pseries-2.7 and
+     * earlier, this allows us to maintain old->new/new->old migration
+     * compatibility.
+     *
+     * For QEMU 2.8+, there are additional CAS-negotiatable options available
+     * via default pseries-2.8 machines and explicit command-line parameters.
+     * Some of these options, like OV5_HP_EVT, *do* require QEMU to be aware
+     * of the actual CAS-negotiated values to continue working properly. For
+     * example, availability of memory unplug depends on knowing whether
+     * OV5_HP_EVT was negotiated via CAS.
+     *
+     * Thus, for any cases where the set of available CAS-negotiatable
+     * options extends beyond OV5_FORM1_AFFINITY and OV5_DRCONF_MEMORY, we
+     * include the CAS-negotiated options in the migration stream.
+     */
+    spapr_ovec_set(ov5_mask, OV5_FORM1_AFFINITY);
+    spapr_ovec_set(ov5_mask, OV5_DRCONF_MEMORY);
+
+    /* spapr_ovec_diff returns true if bits were removed. we avoid using
+     * the mask itself since in the future it's possible "legacy" bits may be
+     * removed via machine options, which could generate a false positive
+     * that breaks migration.
+     */
+    spapr_ovec_intersect(ov5_legacy, spapr->ov5, ov5_mask);
+    cas_needed = spapr_ovec_diff(ov5_removed, spapr->ov5, ov5_legacy);
+
+    spapr_ovec_cleanup(ov5_mask);
+    spapr_ovec_cleanup(ov5_legacy);
+    spapr_ovec_cleanup(ov5_removed);
+
+    return cas_needed;
+}
+
+static const VMStateDescription vmstate_spapr_ov5_cas = {
+    .name = "spapr_option_vector_ov5_cas",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = spapr_ov5_cas_needed,
+    .fields = (VMStateField[]) {
+        VMSTATE_STRUCT_POINTER_V(ov5_cas, sPAPRMachineState, 1,
+                                 vmstate_spapr_ovec, sPAPROptionVector),
+        VMSTATE_END_OF_LIST()
+    },
+};
+
 static const VMStateDescription vmstate_spapr = {
     .name = "spapr",
     .version_id = 3,
@@ -1282,6 +1344,10 @@ static const VMStateDescription vmstate_spapr = {
         VMSTATE_PPC_TIMEBASE_V(tb, sPAPRMachineState, 2),
         VMSTATE_END_OF_LIST()
     },
+    .subsections = (const VMStateDescription*[]) {
+        &vmstate_spapr_ov5_cas,
+        NULL
+    }
 };
 
 static int htab_save_setup(QEMUFile *f, void *opaque)
diff --git a/hw/ppc/spapr_ovec.c b/hw/ppc/spapr_ovec.c
index c2a0d18..3eb1d59 100644
--- a/hw/ppc/spapr_ovec.c
+++ b/hw/ppc/spapr_ovec.c
@@ -37,6 +37,17 @@
  */
 struct sPAPROptionVector {
     unsigned long *bitmap;
+    int32_t bitmap_size; /* only used for migration */
+};
+
+const VMStateDescription vmstate_spapr_ovec = {
+    .name = "spapr_option_vector",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_BITMAP(bitmap, sPAPROptionVector, 1, bitmap_size),
+        VMSTATE_END_OF_LIST()
+    }
 };
 
 sPAPROptionVector *spapr_ovec_new(void)
@@ -45,6 +56,7 @@ sPAPROptionVector *spapr_ovec_new(void)
 
     ov = g_new0(sPAPROptionVector, 1);
     ov->bitmap = bitmap_new(OV_MAXBITS);
+    ov->bitmap_size = OV_MAXBITS;
 
     return ov;
 }
diff --git a/include/hw/ppc/spapr_ovec.h b/include/hw/ppc/spapr_ovec.h
index 6a06da3..355a344 100644
--- a/include/hw/ppc/spapr_ovec.h
+++ b/include/hw/ppc/spapr_ovec.h
@@ -37,6 +37,7 @@
 #define _SPAPR_OVEC_H
 
 #include "cpu.h"
+#include "migration/vmstate.h"
 
 typedef struct sPAPROptionVector sPAPROptionVector;
 
@@ -64,4 +65,7 @@ sPAPROptionVector *spapr_ovec_parse_vector(target_ulong table_addr, int vector);
 int spapr_ovec_populate_dt(void *fdt, int fdt_offset,
                            sPAPROptionVector *ov, const char *name);
 
+/* migration */
+extern const VMStateDescription vmstate_spapr_ovec;
+
 #endif /* !defined (_SPAPR_OVEC_H) */
-- 
2.7.4

[Qemu-devel] [PULL 03/11] ppc: BOOK3E: nothing should be done when MSR:PR is set

[David Gibson]

From: Vladimir Svoboda <ze.vlad@gmail.com>

The server architecture (BOOK3S) specifies that any instruction that
sets MSR:PR will also set MSR:EE, IR and DR.
However there is no such behavior specification for the embedded
architecture (BOOK3E).

Signed-off-by: Vladimir Svoboda <ze.vlad@gmail.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
 target-ppc/helper_regs.h | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/target-ppc/helper_regs.h b/target-ppc/helper_regs.h
index bb9ce60..6213816 100644
--- a/target-ppc/helper_regs.h
+++ b/target-ppc/helper_regs.h
@@ -131,11 +131,14 @@ static inline int hreg_store_msr(CPUPPCState *env, target_ulong value,
     }
     /* If PR=1 then EE, IR and DR must be 1
      *
-     * Note: We only enforce this on 64-bit processors. It appears that
-     * 32-bit implementations supports PR=1 and EE/DR/IR=0 and MacOS
-     * exploits it.
+     * Note: We only enforce this on 64-bit server processors.
+     * It appears that:
+     * - 32-bit implementations supports PR=1 and EE/DR/IR=0 and MacOS
+     *   exploits it.
+     * - 64-bit embedded implementations do not need any operation to be
+     *   performed when PR is set.
      */
-    if ((env->insns_flags & PPC_64B) && ((value >> MSR_PR) & 1)) {
+    if ((env->insns_flags & PPC_SEGMENT_64B) && ((value >> MSR_PR) & 1)) {
         value |= (1 << MSR_EE) | (1 << MSR_DR) | (1 << MSR_IR);
     }
 #endif
-- 
2.7.4

[Qemu-devel] [PULL 04/11] hw/char/spapr_vty: Return amount of free buffer entries in vty_can_receive()

[David Gibson]

From: Thomas Huth <thuth@redhat.com>

The can_receive() callbacks of the character devices should return
the amount of characters that can be accepted at once, not just a
boolean value (which rather means only one character at a time).

Signed-off-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/char/spapr_vty.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/char/spapr_vty.c b/hw/char/spapr_vty.c
index 06b9b39..7c22b8b 100644
--- a/hw/char/spapr_vty.c
+++ b/hw/char/spapr_vty.c
@@ -25,7 +25,7 @@ static int vty_can_receive(void *opaque)
 {
     VIOsPAPRVTYDevice *dev = VIO_SPAPR_VTY_DEVICE(opaque);
 
-    return (dev->in - dev->out) < VTERM_BUFSIZE;
+    return VTERM_BUFSIZE - (dev->in - dev->out);
 }
 
 static void vty_receive(void *opaque, const uint8_t *buf, int size)
-- 
2.7.4

[Qemu-devel] [PULL 05/11] target-ppc: fix index array of national digits

[David Gibson]

From: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com>

Fixes the big endian array access of national digits, from commits
b815587 and e2106d7.

Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 target-ppc/int_helper.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target-ppc/int_helper.c b/target-ppc/int_helper.c
index 9ac204a..2d57c9a 100644
--- a/target-ppc/int_helper.c
+++ b/target-ppc/int_helper.c
@@ -2572,7 +2572,7 @@ static int bcd_cmp_zero(ppc_avr_t *bcd)
 static uint16_t get_national_digit(ppc_avr_t *reg, int n)
 {
 #if defined(HOST_WORDS_BIGENDIAN)
-    return reg->u16[8 - n];
+    return reg->u16[7 - n];
 #else
     return reg->u16[n];
 #endif
@@ -2581,7 +2581,7 @@ static uint16_t get_national_digit(ppc_avr_t *reg, int n)
 static void set_national_digit(ppc_avr_t *reg, uint8_t val, int n)
 {
 #if defined(HOST_WORDS_BIGENDIAN)
-    reg->u16[8 - n] = val;
+    reg->u16[7 - n] = val;
 #else
     reg->u16[n] = val;
 #endif
-- 
2.7.4

[Qemu-devel] [PULL 06/11] ppc: Make uninorth interrupt swizzling identical to Grackle

[David Gibson]

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>

It's currently broken as it uses an incorrect shift, it tries
to use the slot number but uses the top bits of the bus number
instead.

Note: Neither implementation matches what OpenBIOS ends up putting
in the device-tree either, which will have to be fixed separately.

This is not quite correct for modelling a real Mac since Apple
tend to tie all 4 interrupt lines of a slot together and have
separate interrupts for every slot and every motherboard devices
going straight to the PIC but we'll sort that out later.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/pci-host/uninorth.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/hw/pci-host/uninorth.c b/hw/pci-host/uninorth.c
index 7aac4d6..df342ac 100644
--- a/hw/pci-host/uninorth.c
+++ b/hw/pci-host/uninorth.c
@@ -62,9 +62,7 @@ typedef struct UNINState {
 
 static int pci_unin_map_irq(PCIDevice *pci_dev, int irq_num)
 {
-    int devfn = pci_dev->devfn & 0x00FFFFFF;
-
-    return (((devfn >> 11) & 0x1F) + irq_num) & 3;
+    return (irq_num + (pci_dev->devfn >> 3)) & 3;
 }
 
 static void pci_unin_set_irq(void *opaque, int irq_num, int level)
-- 
2.7.4

[Qemu-devel] [PULL 07/11] target-ppc: Fix CPU migration from qemu-2.6 <-> later versions

[David Gibson]

When migration for target-ppc was converted to vmstate, several
VMSTATE_EQUAL() checks were foolishly included of things that really
should be internal state.  Specifically we verified equality of the
insns_flags and insns_flags2 fields, which are used within TCG to
determine which groups of instructions are available on this cpu
model.  Between qemu-2.6 and qemu-2.7 we made some changes to these
classes which broke migration.

This path fixes migration both forwards and backwards.  On migration
from 2.6 to later versions we import the fields into teporary
variables, which we then ignore.  In migration backwards, we populate
the temporary fields from the runtime fields, but mask out the bits
which were added after qemu-2.6, allowing the VMSTATE_EQUAL in
qemu-2.6 to accept the stream.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
---
 target-ppc/cpu.h     |  6 ++++++
 target-ppc/machine.c | 29 +++++++++++++++++++++++++----
 2 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 1c90adb..7798b2e 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -1166,6 +1166,12 @@ struct PowerPCCPU {
     int cpu_dt_id;
     uint32_t max_compat;
     uint32_t cpu_version;
+
+    /* fields used only during migration for compatibility hacks */
+    target_ulong mig_msr_mask;
+    uint64_t mig_insns_flags;
+    uint64_t mig_insns_flags2;
+    uint32_t mig_nb_BATs;
 };
 
 static inline PowerPCCPU *ppc_env_get_cpu(CPUPPCState *env)
diff --git a/target-ppc/machine.c b/target-ppc/machine.c
index e43cb6c..fcac263 100644
--- a/target-ppc/machine.c
+++ b/target-ppc/machine.c
@@ -140,6 +140,21 @@ static void cpu_pre_save(void *opaque)
     PowerPCCPU *cpu = opaque;
     CPUPPCState *env = &cpu->env;
     int i;
+    uint64_t insns_compat_mask =
+        PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB
+        | PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES
+        | PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE | PPC_FLOAT_FRSQRTES
+        | PPC_FLOAT_STFIWX | PPC_FLOAT_EXT
+        | PPC_CACHE | PPC_CACHE_ICBI | PPC_CACHE_DCBZ
+        | PPC_MEM_SYNC | PPC_MEM_EIEIO | PPC_MEM_TLBIE | PPC_MEM_TLBSYNC
+        | PPC_64B | PPC_64BX | PPC_ALTIVEC
+        | PPC_SEGMENT_64B | PPC_SLBI | PPC_POPCNTB | PPC_POPCNTWD;
+    uint64_t insns_compat_mask2 = PPC2_VSX | PPC2_VSX207 | PPC2_DFP | PPC2_DBRX
+        | PPC2_PERM_ISA206 | PPC2_DIVE_ISA206
+        | PPC2_ATOMIC_ISA206 | PPC2_FP_CVT_ISA206
+        | PPC2_FP_TST_ISA206 | PPC2_BCTAR_ISA207
+        | PPC2_LSQ_ISA207 | PPC2_ALTIVEC_207
+        | PPC2_ISA205 | PPC2_ISA207S | PPC2_FP_CVT_S64 | PPC2_TM;
 
     env->spr[SPR_LR] = env->lr;
     env->spr[SPR_CTR] = env->ctr;
@@ -161,6 +176,12 @@ static void cpu_pre_save(void *opaque)
         env->spr[SPR_IBAT4U + 2*i] = env->IBAT[0][i+4];
         env->spr[SPR_IBAT4U + 2*i + 1] = env->IBAT[1][i+4];
     }
+
+    /* Hacks for migration compatibility between 2.6, 2.7 & 2.8 */
+    cpu->mig_msr_mask = env->msr_mask;
+    cpu->mig_insns_flags = env->insns_flags & insns_compat_mask;
+    cpu->mig_insns_flags2 = env->insns_flags2 & insns_compat_mask2;
+    cpu->mig_nb_BATs = env->nb_BATs;
 }
 
 static int cpu_post_load(void *opaque, int version_id)
@@ -561,10 +582,10 @@ const VMStateDescription vmstate_ppc_cpu = {
         /* FIXME: access_type? */
 
         /* Sanity checking */
-        VMSTATE_UINTTL_EQUAL(env.msr_mask, PowerPCCPU),
-        VMSTATE_UINT64_EQUAL(env.insns_flags, PowerPCCPU),
-        VMSTATE_UINT64_EQUAL(env.insns_flags2, PowerPCCPU),
-        VMSTATE_UINT32_EQUAL(env.nb_BATs, PowerPCCPU),
+        VMSTATE_UINTTL(mig_msr_mask, PowerPCCPU),
+        VMSTATE_UINT64(mig_insns_flags, PowerPCCPU),
+        VMSTATE_UINT64(mig_insns_flags2, PowerPCCPU),
+        VMSTATE_UINT32(mig_nb_BATs, PowerPCCPU),
         VMSTATE_END_OF_LIST()
     },
     .subsections = (const VMStateDescription*[]) {
-- 
2.7.4

Re: [Qemu-devel] [PULL 07/11] target-ppc: Fix CPU migration from qemu-2.6 <-> later versions

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 4911 bytes --]

On Wed, Nov 23, 2016 at 01:49:39PM +1100, David Gibson wrote:
> When migration for target-ppc was converted to vmstate, several
> VMSTATE_EQUAL() checks were foolishly included of things that really
> should be internal state.  Specifically we verified equality of the
> insns_flags and insns_flags2 fields, which are used within TCG to
> determine which groups of instructions are available on this cpu
> model.  Between qemu-2.6 and qemu-2.7 we made some changes to these
> classes which broke migration.
> 
> This path fixes migration both forwards and backwards.  On migration
> from 2.6 to later versions we import the fields into teporary
> variables, which we then ignore.  In migration backwards, we populate
> the temporary fields from the runtime fields, but mask out the bits
> which were added after qemu-2.6, allowing the VMSTATE_EQUAL in
> qemu-2.6 to accept the stream.

Mike,

This patch (alone) should also be applied to the v2.7 stable branch to
fix the v2.6<->v2.7 migration failure.  When I tried yesterday the
patch applied clean to v2.7.0, so it shouldn't need reworking.

Would you like me to resend specifically for the stable branch, or are
you happy to just pick it up from here?

> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> Reviewed-by: Greg Kurz <groug@kaod.org>
> ---
>  target-ppc/cpu.h     |  6 ++++++
>  target-ppc/machine.c | 29 +++++++++++++++++++++++++----
>  2 files changed, 31 insertions(+), 4 deletions(-)
> 
> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
> index 1c90adb..7798b2e 100644
> --- a/target-ppc/cpu.h
> +++ b/target-ppc/cpu.h
> @@ -1166,6 +1166,12 @@ struct PowerPCCPU {
>      int cpu_dt_id;
>      uint32_t max_compat;
>      uint32_t cpu_version;
> +
> +    /* fields used only during migration for compatibility hacks */
> +    target_ulong mig_msr_mask;
> +    uint64_t mig_insns_flags;
> +    uint64_t mig_insns_flags2;
> +    uint32_t mig_nb_BATs;
>  };
>  
>  static inline PowerPCCPU *ppc_env_get_cpu(CPUPPCState *env)
> diff --git a/target-ppc/machine.c b/target-ppc/machine.c
> index e43cb6c..fcac263 100644
> --- a/target-ppc/machine.c
> +++ b/target-ppc/machine.c
> @@ -140,6 +140,21 @@ static void cpu_pre_save(void *opaque)
>      PowerPCCPU *cpu = opaque;
>      CPUPPCState *env = &cpu->env;
>      int i;
> +    uint64_t insns_compat_mask =
> +        PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB
> +        | PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES
> +        | PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE | PPC_FLOAT_FRSQRTES
> +        | PPC_FLOAT_STFIWX | PPC_FLOAT_EXT
> +        | PPC_CACHE | PPC_CACHE_ICBI | PPC_CACHE_DCBZ
> +        | PPC_MEM_SYNC | PPC_MEM_EIEIO | PPC_MEM_TLBIE | PPC_MEM_TLBSYNC
> +        | PPC_64B | PPC_64BX | PPC_ALTIVEC
> +        | PPC_SEGMENT_64B | PPC_SLBI | PPC_POPCNTB | PPC_POPCNTWD;
> +    uint64_t insns_compat_mask2 = PPC2_VSX | PPC2_VSX207 | PPC2_DFP | PPC2_DBRX
> +        | PPC2_PERM_ISA206 | PPC2_DIVE_ISA206
> +        | PPC2_ATOMIC_ISA206 | PPC2_FP_CVT_ISA206
> +        | PPC2_FP_TST_ISA206 | PPC2_BCTAR_ISA207
> +        | PPC2_LSQ_ISA207 | PPC2_ALTIVEC_207
> +        | PPC2_ISA205 | PPC2_ISA207S | PPC2_FP_CVT_S64 | PPC2_TM;
>  
>      env->spr[SPR_LR] = env->lr;
>      env->spr[SPR_CTR] = env->ctr;
> @@ -161,6 +176,12 @@ static void cpu_pre_save(void *opaque)
>          env->spr[SPR_IBAT4U + 2*i] = env->IBAT[0][i+4];
>          env->spr[SPR_IBAT4U + 2*i + 1] = env->IBAT[1][i+4];
>      }
> +
> +    /* Hacks for migration compatibility between 2.6, 2.7 & 2.8 */
> +    cpu->mig_msr_mask = env->msr_mask;
> +    cpu->mig_insns_flags = env->insns_flags & insns_compat_mask;
> +    cpu->mig_insns_flags2 = env->insns_flags2 & insns_compat_mask2;
> +    cpu->mig_nb_BATs = env->nb_BATs;
>  }
>  
>  static int cpu_post_load(void *opaque, int version_id)
> @@ -561,10 +582,10 @@ const VMStateDescription vmstate_ppc_cpu = {
>          /* FIXME: access_type? */
>  
>          /* Sanity checking */
> -        VMSTATE_UINTTL_EQUAL(env.msr_mask, PowerPCCPU),
> -        VMSTATE_UINT64_EQUAL(env.insns_flags, PowerPCCPU),
> -        VMSTATE_UINT64_EQUAL(env.insns_flags2, PowerPCCPU),
> -        VMSTATE_UINT32_EQUAL(env.nb_BATs, PowerPCCPU),
> +        VMSTATE_UINTTL(mig_msr_mask, PowerPCCPU),
> +        VMSTATE_UINT64(mig_insns_flags, PowerPCCPU),
> +        VMSTATE_UINT64(mig_insns_flags2, PowerPCCPU),
> +        VMSTATE_UINT32(mig_nb_BATs, PowerPCCPU),
>          VMSTATE_END_OF_LIST()
>      },
>      .subsections = (const VMStateDescription*[]) {

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [PULL 07/11] target-ppc: Fix CPU migration from qemu-2.6 <-> later versions

[Michael Roth]

Quoting David Gibson (2016-11-22 23:10:09)
> On Wed, Nov 23, 2016 at 01:49:39PM +1100, David Gibson wrote:
> > When migration for target-ppc was converted to vmstate, several
> > VMSTATE_EQUAL() checks were foolishly included of things that really
> > should be internal state.  Specifically we verified equality of the
> > insns_flags and insns_flags2 fields, which are used within TCG to
> > determine which groups of instructions are available on this cpu
> > model.  Between qemu-2.6 and qemu-2.7 we made some changes to these
> > classes which broke migration.
> > 
> > This path fixes migration both forwards and backwards.  On migration
> > from 2.6 to later versions we import the fields into teporary
> > variables, which we then ignore.  In migration backwards, we populate
> > the temporary fields from the runtime fields, but mask out the bits
> > which were added after qemu-2.6, allowing the VMSTATE_EQUAL in
> > qemu-2.6 to accept the stream.
> 
> Mike,
> 
> This patch (alone) should also be applied to the v2.7 stable branch to
> fix the v2.6<->v2.7 migration failure.  When I tried yesterday the
> patch applied clean to v2.7.0, so it shouldn't need reworking.
> 
> Would you like me to resend specifically for the stable branch, or are
> you happy to just pick it up from here?

I can pick it up from here.

> 
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > Reviewed-by: Thomas Huth <thuth@redhat.com>
> > Reviewed-by: Greg Kurz <groug@kaod.org>
> > ---
> >  target-ppc/cpu.h     |  6 ++++++
> >  target-ppc/machine.c | 29 +++++++++++++++++++++++++----
> >  2 files changed, 31 insertions(+), 4 deletions(-)
> > 
> > diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
> > index 1c90adb..7798b2e 100644
> > --- a/target-ppc/cpu.h
> > +++ b/target-ppc/cpu.h
> > @@ -1166,6 +1166,12 @@ struct PowerPCCPU {
> >      int cpu_dt_id;
> >      uint32_t max_compat;
> >      uint32_t cpu_version;
> > +
> > +    /* fields used only during migration for compatibility hacks */
> > +    target_ulong mig_msr_mask;
> > +    uint64_t mig_insns_flags;
> > +    uint64_t mig_insns_flags2;
> > +    uint32_t mig_nb_BATs;
> >  };
> >  
> >  static inline PowerPCCPU *ppc_env_get_cpu(CPUPPCState *env)
> > diff --git a/target-ppc/machine.c b/target-ppc/machine.c
> > index e43cb6c..fcac263 100644
> > --- a/target-ppc/machine.c
> > +++ b/target-ppc/machine.c
> > @@ -140,6 +140,21 @@ static void cpu_pre_save(void *opaque)
> >      PowerPCCPU *cpu = opaque;
> >      CPUPPCState *env = &cpu->env;
> >      int i;
> > +    uint64_t insns_compat_mask =
> > +        PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB
> > +        | PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES
> > +        | PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE | PPC_FLOAT_FRSQRTES
> > +        | PPC_FLOAT_STFIWX | PPC_FLOAT_EXT
> > +        | PPC_CACHE | PPC_CACHE_ICBI | PPC_CACHE_DCBZ
> > +        | PPC_MEM_SYNC | PPC_MEM_EIEIO | PPC_MEM_TLBIE | PPC_MEM_TLBSYNC
> > +        | PPC_64B | PPC_64BX | PPC_ALTIVEC
> > +        | PPC_SEGMENT_64B | PPC_SLBI | PPC_POPCNTB | PPC_POPCNTWD;
> > +    uint64_t insns_compat_mask2 = PPC2_VSX | PPC2_VSX207 | PPC2_DFP | PPC2_DBRX
> > +        | PPC2_PERM_ISA206 | PPC2_DIVE_ISA206
> > +        | PPC2_ATOMIC_ISA206 | PPC2_FP_CVT_ISA206
> > +        | PPC2_FP_TST_ISA206 | PPC2_BCTAR_ISA207
> > +        | PPC2_LSQ_ISA207 | PPC2_ALTIVEC_207
> > +        | PPC2_ISA205 | PPC2_ISA207S | PPC2_FP_CVT_S64 | PPC2_TM;
> >  
> >      env->spr[SPR_LR] = env->lr;
> >      env->spr[SPR_CTR] = env->ctr;
> > @@ -161,6 +176,12 @@ static void cpu_pre_save(void *opaque)
> >          env->spr[SPR_IBAT4U + 2*i] = env->IBAT[0][i+4];
> >          env->spr[SPR_IBAT4U + 2*i + 1] = env->IBAT[1][i+4];
> >      }
> > +
> > +    /* Hacks for migration compatibility between 2.6, 2.7 & 2.8 */
> > +    cpu->mig_msr_mask = env->msr_mask;
> > +    cpu->mig_insns_flags = env->insns_flags & insns_compat_mask;
> > +    cpu->mig_insns_flags2 = env->insns_flags2 & insns_compat_mask2;
> > +    cpu->mig_nb_BATs = env->nb_BATs;
> >  }
> >  
> >  static int cpu_post_load(void *opaque, int version_id)
> > @@ -561,10 +582,10 @@ const VMStateDescription vmstate_ppc_cpu = {
> >          /* FIXME: access_type? */
> >  
> >          /* Sanity checking */
> > -        VMSTATE_UINTTL_EQUAL(env.msr_mask, PowerPCCPU),
> > -        VMSTATE_UINT64_EQUAL(env.insns_flags, PowerPCCPU),
> > -        VMSTATE_UINT64_EQUAL(env.insns_flags2, PowerPCCPU),
> > -        VMSTATE_UINT32_EQUAL(env.nb_BATs, PowerPCCPU),
> > +        VMSTATE_UINTTL(mig_msr_mask, PowerPCCPU),
> > +        VMSTATE_UINT64(mig_insns_flags, PowerPCCPU),
> > +        VMSTATE_UINT64(mig_insns_flags2, PowerPCCPU),
> > +        VMSTATE_UINT32(mig_nb_BATs, PowerPCCPU),
> >          VMSTATE_END_OF_LIST()
> >      },
> >      .subsections = (const VMStateDescription*[]) {
> 
> -- 
> David Gibson                    | I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
>                                 | _way_ _around_!
> http://www.ozlabs.org/~dgibson

[Qemu-devel] [PULL 08/11] migration: Add VMSTATE_UINTTL_TEST()

[David Gibson]

include/migration/cpu.h defines VMSTATE_UINTTL() and several variants
for migrating target_ulong fields.  It's defined in terms of
VMSTATE_UINT32() or VMSTATE_UINT64() as appropriate.

It doesn't, however, include a VMSTATE_UINTTL_TEST() variant, which
I'm going to need shortly.  So, add it.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
---
 include/migration/cpu.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/migration/cpu.h b/include/migration/cpu.h
index f3abbab..f3d5dfc 100644
--- a/include/migration/cpu.h
+++ b/include/migration/cpu.h
@@ -18,6 +18,8 @@
     VMSTATE_UINT64_EQUAL_V(_f, _s, _v)
 #define VMSTATE_UINTTL_ARRAY_V(_f, _s, _n, _v)                        \
     VMSTATE_UINT64_ARRAY_V(_f, _s, _n, _v)
+#define VMSTATE_UINTTL_TEST(_f, _s, _t)                               \
+    VMSTATE_UINT64_TEST(_f, _s, _t)
 #define vmstate_info_uinttl vmstate_info_uint64
 #else
 #define qemu_put_betl qemu_put_be32
@@ -35,6 +37,8 @@
     VMSTATE_UINT32_EQUAL_V(_f, _s, _v)
 #define VMSTATE_UINTTL_ARRAY_V(_f, _s, _n, _v)                        \
     VMSTATE_UINT32_ARRAY_V(_f, _s, _n, _v)
+#define VMSTATE_UINTTL_TEST(_f, _s, _t)                               \
+    VMSTATE_UINT32_TEST(_f, _s, _t)
 #define vmstate_info_uinttl vmstate_info_uint32
 #endif
 
-- 
2.7.4

[Qemu-devel] [PULL 09/11] target-ppc: Allow eventual removal of old migration mistakes

[David Gibson]

Until very recently, the vmstate for ppc cpus included some poorly
thought out VMSTATE_EQUAL() components, that can easily break
migration compatibility, and did so between qemu-2.6 and later
versions.  A hack was recently added which fixes this migration
breakage, but it leaves the unhelpful cruft of these fields in the
migration stream.

This patch adds a new cpu property allowing these fields to be removed
from the stream entirely.  For the pseries-2.8 machine type - which
comes after the fix - and for all non-pseries machine types - which
aren't mature enough to care about cross-version migration - we remove
the fields from the stream.

For pseries-2.7 and earlier, The migration hack remains in place,
allowing backwards and forwards migration with the older machine
types.

This restricts the migration compatibility cruft to older machine
types, and at least opens the possibility of eventually deprecating
and removing it entirely.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 hw/ppc/spapr.c              |  5 +++++
 target-ppc/cpu.h            |  3 ++-
 target-ppc/machine.c        | 26 ++++++++++++++++++--------
 target-ppc/translate_init.c |  6 ++++++
 4 files changed, 31 insertions(+), 9 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 54b88d3..775ad2e 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2767,6 +2767,11 @@ DEFINE_SPAPR_MACHINE(2_8, "2.8", true);
         .driver   = TYPE_SPAPR_PCI_HOST_BRIDGE,     \
         .property = "mem64_win_size",               \
         .value    = "0",                            \
+    },                                              \
+    {                                               \
+        .driver = TYPE_POWERPC_CPU,                 \
+        .property = "pre-2.8-migration",            \
+        .value    = "on",                           \
     },
 
 static void phb_placement_2_7(sPAPRMachineState *spapr, uint32_t index,
diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 7798b2e..2a50c43 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -1167,7 +1167,8 @@ struct PowerPCCPU {
     uint32_t max_compat;
     uint32_t cpu_version;
 
-    /* fields used only during migration for compatibility hacks */
+    /* Fields related to migration compatibility hacks */
+    bool pre_2_8_migration;
     target_ulong mig_msr_mask;
     uint64_t mig_insns_flags;
     uint64_t mig_insns_flags2;
diff --git a/target-ppc/machine.c b/target-ppc/machine.c
index fcac263..18c16d2 100644
--- a/target-ppc/machine.c
+++ b/target-ppc/machine.c
@@ -135,6 +135,13 @@ static const VMStateInfo vmstate_info_avr = {
 #define VMSTATE_AVR_ARRAY(_f, _s, _n)                             \
     VMSTATE_AVR_ARRAY_V(_f, _s, _n, 0)
 
+static bool cpu_pre_2_8_migration(void *opaque, int version_id)
+{
+    PowerPCCPU *cpu = opaque;
+
+    return cpu->pre_2_8_migration;
+}
+
 static void cpu_pre_save(void *opaque)
 {
     PowerPCCPU *cpu = opaque;
@@ -178,10 +185,12 @@ static void cpu_pre_save(void *opaque)
     }
 
     /* Hacks for migration compatibility between 2.6, 2.7 & 2.8 */
-    cpu->mig_msr_mask = env->msr_mask;
-    cpu->mig_insns_flags = env->insns_flags & insns_compat_mask;
-    cpu->mig_insns_flags2 = env->insns_flags2 & insns_compat_mask2;
-    cpu->mig_nb_BATs = env->nb_BATs;
+    if (cpu->pre_2_8_migration) {
+        cpu->mig_msr_mask = env->msr_mask;
+        cpu->mig_insns_flags = env->insns_flags & insns_compat_mask;
+        cpu->mig_insns_flags2 = env->insns_flags2 & insns_compat_mask2;
+        cpu->mig_nb_BATs = env->nb_BATs;
+    }
 }
 
 static int cpu_post_load(void *opaque, int version_id)
@@ -582,10 +591,11 @@ const VMStateDescription vmstate_ppc_cpu = {
         /* FIXME: access_type? */
 
         /* Sanity checking */
-        VMSTATE_UINTTL(mig_msr_mask, PowerPCCPU),
-        VMSTATE_UINT64(mig_insns_flags, PowerPCCPU),
-        VMSTATE_UINT64(mig_insns_flags2, PowerPCCPU),
-        VMSTATE_UINT32(mig_nb_BATs, PowerPCCPU),
+        VMSTATE_UINTTL_TEST(mig_msr_mask, PowerPCCPU, cpu_pre_2_8_migration),
+        VMSTATE_UINT64_TEST(mig_insns_flags, PowerPCCPU, cpu_pre_2_8_migration),
+        VMSTATE_UINT64_TEST(mig_insns_flags2, PowerPCCPU,
+                            cpu_pre_2_8_migration),
+        VMSTATE_UINT32_TEST(mig_nb_BATs, PowerPCCPU, cpu_pre_2_8_migration),
         VMSTATE_END_OF_LIST()
     },
     .subsections = (const VMStateDescription*[]) {
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 208fa1e..626e031 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -10520,6 +10520,11 @@ static gchar *ppc_gdb_arch_name(CPUState *cs)
 #endif
 }
 
+static Property ppc_cpu_properties[] = {
+    DEFINE_PROP_BOOL("pre-2.8-migration", PowerPCCPU, pre_2_8_migration, false),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
 static void ppc_cpu_class_init(ObjectClass *oc, void *data)
 {
     PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc);
@@ -10532,6 +10537,7 @@ static void ppc_cpu_class_init(ObjectClass *oc, void *data)
     pcc->interrupts_big_endian = ppc_cpu_interrupts_big_endian_always;
     dc->realize = ppc_cpu_realizefn;
     dc->unrealize = ppc_cpu_unrealizefn;
+    dc->props = ppc_cpu_properties;
 
     pcc->parent_reset = cc->reset;
     cc->reset = ppc_cpu_reset;
-- 
2.7.4

[Qemu-devel] [PULL 10/11] Revert "spapr: Fix migration of PCI host bridges from qemu-2.7"

[David Gibson]

This reverts commit 9b54ca0ba781012eeea4237b7c4832ba2ea81d89.

The commit above corrected a migration breakage between qemu-2.7 and
qemu-2.8.  However it did so by advancing the migration version for
the PCI host bridge, which obviously breaks migration backwards to
earlier qemu versions.

Although it's not totally essential, we'd like to maintain the
possibility for backwards migration, so revert the change in
preparation for a better fix.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 hw/ppc/spapr_pci.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index f9661b7..7cde30e 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -1658,25 +1658,19 @@ static int spapr_pci_post_load(void *opaque, int version_id)
     return 0;
 }
 
-static bool version_before_3(void *opaque, int version_id)
-{
-    return version_id < 3;
-}
-
 static const VMStateDescription vmstate_spapr_pci = {
     .name = "spapr_pci",
-    .version_id = 3,
+    .version_id = 2,
     .minimum_version_id = 2,
     .pre_save = spapr_pci_pre_save,
     .post_load = spapr_pci_post_load,
     .fields = (VMStateField[]) {
         VMSTATE_UINT64_EQUAL(buid, sPAPRPHBState),
-        VMSTATE_UNUSED_TEST(version_before_3,
-                            sizeof(uint32_t) /* dma_liobn[0] */
-                            + sizeof(uint64_t) /* mem_win_addr */
-                            + sizeof(uint64_t) /* mem_win_size */
-                            + sizeof(uint64_t) /* io_win_addr */
-                            + sizeof(uint64_t) /* io_win_size */),
+        VMSTATE_UINT32_EQUAL(dma_liobn[0], sPAPRPHBState),
+        VMSTATE_UINT64_EQUAL(mem_win_addr, sPAPRPHBState),
+        VMSTATE_UINT64_EQUAL(mem_win_size, sPAPRPHBState),
+        VMSTATE_UINT64_EQUAL(io_win_addr, sPAPRPHBState),
+        VMSTATE_UINT64_EQUAL(io_win_size, sPAPRPHBState),
         VMSTATE_STRUCT_ARRAY(lsi_table, sPAPRPHBState, PCI_NUM_PINS, 0,
                              vmstate_spapr_pci_lsi, struct spapr_pci_lsi),
         VMSTATE_INT32(msi_devs_num, sPAPRPHBState),
-- 
2.7.4

[Qemu-devel] [PULL 11/11] spapr: Fix 2.7<->2.8 migration of PCI host bridge

[David Gibson]

daa2369 "spapr_pci: Add a 64-bit MMIO window" subtly broke migration
from qemu-2.7 to the current version.  It split the device's MMIO
window into two pieces for 32-bit and 64-bit MMIO.

The patch included backwards compatibility code to convert the old
property into the new format.  However, the property value was also
transferred in the migration stream and compared with a (probably
unwise) VMSTATE_EQUAL.  So, the "raw" value from 2.7 is compared to
the new style converted value from (pre-)2.8 giving a mismatch and
migration failure.

Along with the actual field that caused the breakage, there are
several other ill-advised VMSTATE_EQUAL()s.  To fix forwards
migration, we read the values in the stream into scratch variables and
ignore them, instead of comparing for equality.  To fix backwards
migration, we populate those scratch variables in pre_save() with
adjusted values to match the old behaviour.

To permit the eventual possibility of removing this cruft from the
stream, we only include these compatibility fields if a new
'pre-2.8-migration' property is set.  We clear it on the pseries-2.8
machine type, which obviously can't be migrated backwards, but set it
on earlier machine type versions.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 hw/ppc/spapr.c              |  5 +++++
 hw/ppc/spapr_pci.c          | 33 ++++++++++++++++++++++++++++-----
 include/hw/pci-host/spapr.h |  6 ++++++
 3 files changed, 39 insertions(+), 5 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 775ad2e..c3269c7 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2772,6 +2772,11 @@ DEFINE_SPAPR_MACHINE(2_8, "2.8", true);
         .driver = TYPE_POWERPC_CPU,                 \
         .property = "pre-2.8-migration",            \
         .value    = "on",                           \
+    },                                              \
+    {                                               \
+        .driver = TYPE_SPAPR_PCI_HOST_BRIDGE,       \
+        .property = "pre-2.8-migration",            \
+        .value    = "on",                           \
     },
 
 static void phb_placement_2_7(sPAPRMachineState *spapr, uint32_t index,
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 7cde30e..fd6fc1d 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -1590,6 +1590,8 @@ static Property spapr_phb_properties[] = {
     DEFINE_PROP_UINT64("pgsz", sPAPRPHBState, page_size_mask,
                        (1ULL << 12) | (1ULL << 16)),
     DEFINE_PROP_UINT32("numa_node", sPAPRPHBState, numa_node, -1),
+    DEFINE_PROP_BOOL("pre-2.8-migration", sPAPRPHBState,
+                     pre_2_8_migration, false),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -1636,6 +1638,20 @@ static void spapr_pci_pre_save(void *opaque)
         sphb->msi_devs[i].key = *(uint32_t *) key;
         sphb->msi_devs[i].value = *(spapr_pci_msi *) value;
     }
+
+    if (sphb->pre_2_8_migration) {
+        sphb->mig_liobn = sphb->dma_liobn[0];
+        sphb->mig_mem_win_addr = sphb->mem_win_addr;
+        sphb->mig_mem_win_size = sphb->mem_win_size;
+        sphb->mig_io_win_addr = sphb->io_win_addr;
+        sphb->mig_io_win_size = sphb->io_win_size;
+
+        if ((sphb->mem64_win_size != 0)
+            && (sphb->mem64_win_addr
+                == (sphb->mem_win_addr + sphb->mem_win_size))) {
+            sphb->mig_mem_win_size += sphb->mem64_win_size;
+        }
+    }
 }
 
 static int spapr_pci_post_load(void *opaque, int version_id)
@@ -1658,6 +1674,13 @@ static int spapr_pci_post_load(void *opaque, int version_id)
     return 0;
 }
 
+static bool pre_2_8_migration(void *opaque, int version_id)
+{
+    sPAPRPHBState *sphb = opaque;
+
+    return sphb->pre_2_8_migration;
+}
+
 static const VMStateDescription vmstate_spapr_pci = {
     .name = "spapr_pci",
     .version_id = 2,
@@ -1666,11 +1689,11 @@ static const VMStateDescription vmstate_spapr_pci = {
     .post_load = spapr_pci_post_load,
     .fields = (VMStateField[]) {
         VMSTATE_UINT64_EQUAL(buid, sPAPRPHBState),
-        VMSTATE_UINT32_EQUAL(dma_liobn[0], sPAPRPHBState),
-        VMSTATE_UINT64_EQUAL(mem_win_addr, sPAPRPHBState),
-        VMSTATE_UINT64_EQUAL(mem_win_size, sPAPRPHBState),
-        VMSTATE_UINT64_EQUAL(io_win_addr, sPAPRPHBState),
-        VMSTATE_UINT64_EQUAL(io_win_size, sPAPRPHBState),
+        VMSTATE_UINT32_TEST(mig_liobn, sPAPRPHBState, pre_2_8_migration),
+        VMSTATE_UINT64_TEST(mig_mem_win_addr, sPAPRPHBState, pre_2_8_migration),
+        VMSTATE_UINT64_TEST(mig_mem_win_size, sPAPRPHBState, pre_2_8_migration),
+        VMSTATE_UINT64_TEST(mig_io_win_addr, sPAPRPHBState, pre_2_8_migration),
+        VMSTATE_UINT64_TEST(mig_io_win_size, sPAPRPHBState, pre_2_8_migration),
         VMSTATE_STRUCT_ARRAY(lsi_table, sPAPRPHBState, PCI_NUM_PINS, 0,
                              vmstate_spapr_pci_lsi, struct spapr_pci_lsi),
         VMSTATE_INT32(msi_devs_num, sPAPRPHBState),
diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
index b92c1b5..092294e 100644
--- a/include/hw/pci-host/spapr.h
+++ b/include/hw/pci-host/spapr.h
@@ -79,6 +79,12 @@ struct sPAPRPHBState {
     uint64_t dma64_win_addr;
 
     uint32_t numa_node;
+
+    /* Fields for migration compatibility hacks */
+    bool pre_2_8_migration;
+    uint32_t mig_liobn;
+    hwaddr mig_mem_win_addr, mig_mem_win_size;
+    hwaddr mig_io_win_addr, mig_io_win_size;
 };
 
 #define SPAPR_PCI_MEM_WIN_BUS_OFFSET 0x80000000ULL
-- 
2.7.4

Re: [Qemu-devel] [PULL 00/11] ppc-for-2.8 queue 20161123

[no-reply]

Hi,

Your series seems to have some coding style problems. See output below for
more information:

Subject: [Qemu-devel] [PULL 00/11] ppc-for-2.8 queue 20161123
Type: series
Message-id: 1479869383-16162-1-git-send-email-david@gibson.dropbear.id.au

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]         patchew/1479869383-16162-1-git-send-email-david@gibson.dropbear.id.au -> patchew/1479869383-16162-1-git-send-email-david@gibson.dropbear.id.au
Switched to a new branch 'test'
732b10b spapr: Fix 2.7<->2.8 migration of PCI host bridge
d03272e Revert "spapr: Fix migration of PCI host bridges from qemu-2.7"
8632e5b target-ppc: Allow eventual removal of old migration mistakes
2ed64f0 migration: Add VMSTATE_UINTTL_TEST()
5962a40 target-ppc: Fix CPU migration from qemu-2.6 <-> later versions
023b1c9 ppc: Make uninorth interrupt swizzling identical to Grackle
ef14b51 target-ppc: fix index array of national digits
6b3e478 hw/char/spapr_vty: Return amount of free buffer entries in vty_can_receive()
3714ee6 ppc: BOOK3E: nothing should be done when MSR:PR is set
2ae7725 spapr: migration support for CAS-negotiated option vectors
bf53fd9 tests/postcopy: Use KVM on ppc64 only if it is KVM-HV

=== OUTPUT BEGIN ===
Checking PATCH 1/11: tests/postcopy: Use KVM on ppc64 only if it is KVM-HV...
Checking PATCH 2/11: spapr: migration support for CAS-negotiated option vectors...
ERROR: spaces required around that '*' (ctx:VxV)
#99: FILE: hw/ppc/spapr.c:1347:
+    .subsections = (const VMStateDescription*[]) {
                                             ^

total: 1 errors, 0 warnings, 116 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 3/11: ppc: BOOK3E: nothing should be done when MSR:PR is set...
Checking PATCH 4/11: hw/char/spapr_vty: Return amount of free buffer entries in vty_can_receive()...
Checking PATCH 5/11: target-ppc: fix index array of national digits...
Checking PATCH 6/11: ppc: Make uninorth interrupt swizzling identical to Grackle...
Checking PATCH 7/11: target-ppc: Fix CPU migration from qemu-2.6 <-> later versions...
Checking PATCH 8/11: migration: Add VMSTATE_UINTTL_TEST()...
Checking PATCH 9/11: target-ppc: Allow eventual removal of old migration mistakes...
Checking PATCH 10/11: Revert "spapr: Fix migration of PCI host bridges from qemu-2.7"...
Checking PATCH 11/11: spapr: Fix 2.7<->2.8 migration of PCI host bridge...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org

Re: [Qemu-devel] [PULL 00/11] ppc-for-2.8 queue 20161123

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 448 bytes --]

On Tue, Nov 22, 2016 at 07:06:11PM -0800, no-reply@patchew.org wrote:
> === OUTPUT BEGIN ===
> Checking PATCH 1/11: tests/postcopy: Use KVM on ppc64 only if it is KVM-HV...
> Checking PATCH 2/11: spapr: migration support for CAS-negotiated option vectors...
> ERROR: spaces required around that '*' (ctx:VxV)
> #99: FILE: hw/ppc/spapr.c:1347:
> +    .subsections = (const VMStateDescription*[]) {

Is there a follow-up to fix this?

Thanks,
Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [Qemu-devel] [PULL 00/11] ppc-for-2.8 queue 20161123

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 914 bytes --]

On Thu, Nov 24, 2016 at 09:50:05AM +0000, Stefan Hajnoczi wrote:
> On Tue, Nov 22, 2016 at 07:06:11PM -0800, no-reply@patchew.org wrote:
> > === OUTPUT BEGIN ===
> > Checking PATCH 1/11: tests/postcopy: Use KVM on ppc64 only if it is KVM-HV...
> > Checking PATCH 2/11: spapr: migration support for CAS-negotiated option vectors...
> > ERROR: spaces required around that '*' (ctx:VxV)
> > #99: FILE: hw/ppc/spapr.c:1347:
> > +    .subsections = (const VMStateDescription*[]) {
> 
> Is there a follow-up to fix this?

AFAICT this is a bug in the checker - it seems to think this is a
multiply, not a pointer construct.  Every other .subsections
initializer seems to have formatting identical to the above.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [PULL 00/11] ppc-for-2.8 queue 20161123

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 2951 bytes --]

On Wed, Nov 23, 2016 at 01:49:32PM +1100, David Gibson wrote:
> The following changes since commit 00227fefd2059464cd2f59aed29944874c630e2f:
> 
>   Update version for v2.8.0-rc1 release (2016-11-22 22:29:08 +0000)
> 
> are available in the git repository at:
> 
>   git://github.com/dgibson/qemu.git tags/ppc-for-2.8-20161123
> 
> for you to fetch changes up to 5c4537bded40640b166ec77e112592174b048c21:
> 
>   spapr: Fix 2.7<->2.8 migration of PCI host bridge (2016-11-23 12:00:48 +1100)
> 
> ----------------------------------------------------------------
> ppc patch queue 2016-11-23
> 
> Here's the first set of 2.8 hard freeze bugfixes for ppc.
> 
> The biggest thing here is a batch of fixes for migration breakages in
> both 2.7 and current 2.8.  Alas, there is at least one more migration
> problem, which prevents memory unplug after a migration.  I hoped to
> include a fix for that here, but it turned out to have some problems
> bigger than those it was solving.  So, I expect at least one more hard
> freeze pull request.

Excellent, thank you for the summary.

> There are also a few other assorted bug fixes.
> 
> ----------------------------------------------------------------
> Benjamin Herrenschmidt (1):
>       ppc: Make uninorth interrupt swizzling identical to Grackle
> 
> David Gibson (5):
>       target-ppc: Fix CPU migration from qemu-2.6 <-> later versions
>       migration: Add VMSTATE_UINTTL_TEST()
>       target-ppc: Allow eventual removal of old migration mistakes
>       Revert "spapr: Fix migration of PCI host bridges from qemu-2.7"
>       spapr: Fix 2.7<->2.8 migration of PCI host bridge
> 
> Jose Ricardo Ziviani (1):
>       target-ppc: fix index array of national digits
> 
> Michael Roth (1):
>       spapr: migration support for CAS-negotiated option vectors
> 
> Thomas Huth (2):
>       tests/postcopy: Use KVM on ppc64 only if it is KVM-HV
>       hw/char/spapr_vty: Return amount of free buffer entries in vty_can_receive()
> 
> Vladimir Svoboda (1):
>       ppc: BOOK3E: nothing should be done when MSR:PR is set
> 
>  hw/char/spapr_vty.c         |  2 +-
>  hw/pci-host/uninorth.c      |  4 +--
>  hw/ppc/spapr.c              | 76 +++++++++++++++++++++++++++++++++++++++++++++
>  hw/ppc/spapr_ovec.c         | 12 +++++++
>  hw/ppc/spapr_pci.c          | 35 +++++++++++++++------
>  include/hw/pci-host/spapr.h |  6 ++++
>  include/hw/ppc/spapr_ovec.h |  4 +++
>  include/migration/cpu.h     |  4 +++
>  target-ppc/cpu.h            |  7 +++++
>  target-ppc/helper_regs.h    | 11 ++++---
>  target-ppc/int_helper.c     |  4 +--
>  target-ppc/machine.c        | 39 ++++++++++++++++++++---
>  target-ppc/translate_init.c |  6 ++++
>  tests/postcopy-test.c       | 12 ++++---
>  14 files changed, 195 insertions(+), 27 deletions(-)
> 

Thanks, applied to my staging tree:
https://github.com/stefanha/qemu/commits/staging

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]