* David Gibson (da...@gibson.dropbear.id.au) wrote:
> 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 <da...@gibson.dropbear.id.au>
Reviewed-by: Dr. David Alan Gilbert <dgilb...@redhat.com>
> ---
> 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 e429c94..c62c1cb 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;
> + }
> + }
> }
>
> /*
> @@ -1680,6 +1696,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,
> @@ -1688,11 +1711,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
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
