[Qemu-devel] [PATCH for-2.1 v2 0/2] Fix migration failure due to ACPI tables size changes

[Qemu-devel] [PATCH for-2.1 v2 0/2] Fix migration failure due to ACPI tables size changes

[Igor Mammedov]

Changes since v2:
  - addressed Laszlo's comments
     * fixing typos, rewording comments
     * dropping enum-ification of RAMBlock flags
     * adding zeroing out destination ramblock
     * replacing 'if' with assert() 
    
Changing the ACPI table size causes migration to break, and the memory
hotplug work opened our eyes on how horribly we were breaking things in
2.0 already.

To trigger issue start
  QEMU-1.7 with -M pc-i440fx-1.7 -device pci-bridge,chassis_nr=1
and try to migrate to QEMU-2.1 or QEMU-2.0 as result target will fail with:
  qemu-system-x86_64: Length mismatch: /rom@etc/acpi/tables: 2000 in != 3000

This fix allows target QEMU to load smaller RAMBlock into a bigger one
and fixes regression which was introduced since 2.0, allowing
forward migration from 1.7/2.0 to 2.1
Fix is also suitable for stable-2.0.


Igor Mammedov (2):
  migration: load smaller RAMBlock to a bigger one if permitted
  acpi: mark ACPI tables ROM blob as extend-able on migration

 arch_init.c             | 22 +++++++++++++++++-----
 exec.c                  |  8 ++++++++
 hw/core/loader.c        |  6 +++++-
 hw/i386/acpi-build.c    |  2 +-
 include/exec/memory.h   | 11 +++++++++++
 include/exec/ram_addr.h |  3 +++
 include/hw/loader.h     |  5 +++--
 memory.c                |  5 +++++
 8 files changed, 53 insertions(+), 9 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH for-2.1 v2 1/2] migration: load smaller RAMBlock to a bigger one if permitted

[Igor Mammedov]

Add API to mark memory region as extend-able on migration,
to allow migration code to load smaller RAMBlock into
a bigger one on destination QEMU instance.

This will allow to fix broken migration from QEMU 1.7/2.0 to
QEMU 2.1 due to  ACPI tables size changes across 1.7/2.0/2.1
versions by marking ACPI tables ROM blob as extend-able.
So that smaller tables from previous version could be always
migrated to a bigger rom blob on new version.

Credits-for-idea: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
v2:
  fixed patch as suggested by Laszlo
---
 arch_init.c             | 22 +++++++++++++++++-----
 exec.c                  |  8 ++++++++
 include/exec/memory.h   | 11 +++++++++++
 include/exec/ram_addr.h |  3 +++
 memory.c                |  5 +++++
 5 files changed, 44 insertions(+), 5 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index 8ddaf35..2c0c238 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -1071,11 +1071,23 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
 
                 QTAILQ_FOREACH(block, &ram_list.blocks, next) {
                     if (!strncmp(id, block->idstr, sizeof(id))) {
-                        if (block->length != length) {
-                            error_report("Length mismatch: %s: " RAM_ADDR_FMT
-                                         " in != " RAM_ADDR_FMT, id, length,
-                                         block->length);
-                            ret =  -EINVAL;
+                        if (block->flags & RAM_EXTENDABLE_ON_MIGRATE) {
+                            if (block->length < length) {
+                                error_report("Length too big: %s: " RAM_ADDR_FMT
+                                             " in > " RAM_ADDR_FMT, id, length,
+                                             block->length);
+                                ret =  -EINVAL;
+                            } else {
+                                memset(block->host, 0, block->length);
+                            }
+                        } else {
+                            if (block->length != length) {
+                                error_report("Length mismatch: %s: "
+                                             RAM_ADDR_FMT " in != "
+                                             RAM_ADDR_FMT,
+                                             id, length, block->length);
+                                ret =  -EINVAL;
+                            }
                         }
                         break;
                     }
diff --git a/exec.c b/exec.c
index 765bd94..02536f8e 100644
--- a/exec.c
+++ b/exec.c
@@ -1214,6 +1214,14 @@ void qemu_ram_unset_idstr(ram_addr_t addr)
     }
 }
 
+void qemu_ram_set_extendable_on_migration(ram_addr_t addr)
+{
+    RAMBlock *block = find_ram_block(addr);
+
+    assert(block != NULL);
+    block->flags |= RAM_EXTENDABLE_ON_MIGRATE;
+}
+
 static int memory_try_enable_merging(void *addr, size_t len)
 {
     if (!qemu_opt_get_bool(qemu_get_machine_opts(), "mem-merge", true)) {
diff --git a/include/exec/memory.h b/include/exec/memory.h
index e2c8e3e..f96ddbb 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -894,6 +894,17 @@ bool memory_region_present(MemoryRegion *container, hwaddr addr);
 bool memory_region_is_mapped(MemoryRegion *mr);
 
 /**
+ * memory_region_permit_extendable_migration: marks #MemoryRegion
+ * as extendable on migration, allowing the migration code to load
+ * source memory block of smaller size than destination memory block
+ * at migration time
+ *
+ * @mr: a #MemoryRegion whose #RAMBlock should be marked as
+ *      extendable on migration
+ */
+void memory_region_permit_extendable_migration(MemoryRegion *mr);
+
+/**
  * memory_region_find: translate an address/size relative to a
  * MemoryRegion into a #MemoryRegionSection.
  *
diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index 6593be1..7a6b782 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -34,6 +34,9 @@ void *qemu_get_ram_ptr(ram_addr_t addr);
 void qemu_ram_free(ram_addr_t addr);
 void qemu_ram_free_from_ptr(ram_addr_t addr);
 
+#define RAM_EXTENDABLE_ON_MIGRATE (1U << 31)
+void qemu_ram_set_extendable_on_migration(ram_addr_t addr);
+
 static inline bool cpu_physical_memory_get_dirty(ram_addr_t start,
                                                  ram_addr_t length,
                                                  unsigned client)
diff --git a/memory.c b/memory.c
index 64d7176..744c746 100644
--- a/memory.c
+++ b/memory.c
@@ -1791,6 +1791,11 @@ bool memory_region_is_mapped(MemoryRegion *mr)
     return mr->container ? true : false;
 }
 
+void memory_region_permit_extendable_migration(MemoryRegion *mr)
+{
+    qemu_ram_set_extendable_on_migration(mr->ram_addr);
+}
+
 MemoryRegionSection memory_region_find(MemoryRegion *mr,
                                        hwaddr addr, uint64_t size)
 {
-- 
1.8.3.1

[Qemu-devel] [PATCH for-2.1 v2 2/2] acpi: mark ACPI tables ROM blob as extend-able on migration

[Igor Mammedov]

It fixes migration failure for machine type pc-i440fx-1.7 from
QEMU 1.7/2.0 to QEMU 2.1

Migration fails due to ACPI tables size grows across 1.7-2.1
versions. That causes ACPI tables ROM blob to change its size
differently for the same configurations on different QEMU versions.
As result migration code bails out when attempting to load
smaller ROM blob into a bigger one on a newer QEMU.
To trigger failure it's enough to add pci-bridge device to QEMU CLI

Marking ACPI tables ROM blob as extend-able on migration allows
QEMU to load smaller ACPI tables from QEMU 1.7/2.0, fixing
forward migration failure introduced since 2.0 which affects
only configurations that cause ACPI ROM blob size change.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
 hw/core/loader.c     | 6 +++++-
 hw/i386/acpi-build.c | 2 +-
 include/hw/loader.h  | 5 +++--
 3 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/hw/core/loader.c b/hw/core/loader.c
index 2bf6b8f..d09b894 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -722,7 +722,8 @@ err:
 
 void *rom_add_blob(const char *name, const void *blob, size_t len,
                    hwaddr addr, const char *fw_file_name,
-                   FWCfgReadCallback fw_callback, void *callback_opaque)
+                   FWCfgReadCallback fw_callback, void *callback_opaque,
+                   bool extendable)
 {
     Rom *rom;
     void *data = NULL;
@@ -742,6 +743,9 @@ void *rom_add_blob(const char *name, const void *blob, size_t len,
 
         if (rom_file_has_mr) {
             data = rom_set_mr(rom, OBJECT(fw_cfg), devpath);
+            if (extendable) {
+                memory_region_permit_extendable_migration(rom->mr);
+            }
         } else {
             data = rom->data;
         }
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index ebc5f03..651c06b 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1558,7 +1558,7 @@ static void *acpi_add_rom_blob(AcpiBuildState *build_state, GArray *blob,
                                const char *name)
 {
     return rom_add_blob(name, blob->data, acpi_data_len(blob), -1, name,
-                        acpi_build_update, build_state);
+                        acpi_build_update, build_state, true);
 }
 
 static const VMStateDescription vmstate_acpi_build = {
diff --git a/include/hw/loader.h b/include/hw/loader.h
index 796cbf9..e5a24ac 100644
--- a/include/hw/loader.h
+++ b/include/hw/loader.h
@@ -57,7 +57,8 @@ int rom_add_file(const char *file, const char *fw_dir,
                  bool option_rom);
 void *rom_add_blob(const char *name, const void *blob, size_t len,
                    hwaddr addr, const char *fw_file_name,
-                   FWCfgReadCallback fw_callback, void *callback_opaque);
+                   FWCfgReadCallback fw_callback, void *callback_opaque,
+                   bool extendable);
 int rom_add_elf_program(const char *name, void *data, size_t datasize,
                         size_t romsize, hwaddr addr);
 int rom_load_all(void);
@@ -70,7 +71,7 @@ void do_info_roms(Monitor *mon, const QDict *qdict);
 #define rom_add_file_fixed(_f, _a, _i)          \
     rom_add_file(_f, NULL, _a, _i, false)
 #define rom_add_blob_fixed(_f, _b, _l, _a)      \
-    rom_add_blob(_f, _b, _l, _a, NULL, NULL, NULL)
+    rom_add_blob(_f, _b, _l, _a, NULL, NULL, NULL, false)
 
 #define PC_ROM_MIN_VGA     0xc0000
 #define PC_ROM_MIN_OPTION  0xc8000
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH for-2.1 v2 1/2] migration: load smaller RAMBlock to a bigger one if permitted

[Laszlo Ersek]

On 07/28/14 10:03, Igor Mammedov wrote:
> Add API to mark memory region as extend-able on migration,
> to allow migration code to load smaller RAMBlock into
> a bigger one on destination QEMU instance.
> 
> This will allow to fix broken migration from QEMU 1.7/2.0 to
> QEMU 2.1 due to  ACPI tables size changes across 1.7/2.0/2.1
> versions by marking ACPI tables ROM blob as extend-able.
> So that smaller tables from previous version could be always
> migrated to a bigger rom blob on new version.
> 
> Credits-for-idea: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> v2:
>   fixed patch as suggested by Laszlo
> ---
>  arch_init.c             | 22 +++++++++++++++++-----
>  exec.c                  |  8 ++++++++
>  include/exec/memory.h   | 11 +++++++++++
>  include/exec/ram_addr.h |  3 +++
>  memory.c                |  5 +++++
>  5 files changed, 44 insertions(+), 5 deletions(-)

Thank you.

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Re: [Qemu-devel] [PATCH for-2.1 v2 2/2] acpi: mark ACPI tables ROM blob as extend-able on migration

[Igor Mammedov]

On Mon, 28 Jul 2014 08:03:25 +0000
Igor Mammedov <imammedo@redhat.com> wrote:

> It fixes migration failure for machine type pc-i440fx-1.7 from
> QEMU 1.7/2.0 to QEMU 2.1
> 
> Migration fails due to ACPI tables size grows across 1.7-2.1
> versions. That causes ACPI tables ROM blob to change its size
> differently for the same configurations on different QEMU versions.
> As result migration code bails out when attempting to load
> smaller ROM blob into a bigger one on a newer QEMU.
> To trigger failure it's enough to add pci-bridge device to QEMU CLI
> 
> Marking ACPI tables ROM blob as extend-able on migration allows
> QEMU to load smaller ACPI tables from QEMU 1.7/2.0, fixing
> forward migration failure introduced since 2.0 which affects
> only configurations that cause ACPI ROM blob size change.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Self-NACK

I'm sorry for mess, I've tested it on for i386 target,
but patch breaks build for other targets.
I'll resubmit v3 shortly.

> ---
>  hw/core/loader.c     | 6 +++++-
>  hw/i386/acpi-build.c | 2 +-
>  include/hw/loader.h  | 5 +++--
>  3 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/core/loader.c b/hw/core/loader.c
> index 2bf6b8f..d09b894 100644
> --- a/hw/core/loader.c
> +++ b/hw/core/loader.c
> @@ -722,7 +722,8 @@ err:
>  
>  void *rom_add_blob(const char *name, const void *blob, size_t len,
>                     hwaddr addr, const char *fw_file_name,
> -                   FWCfgReadCallback fw_callback, void *callback_opaque)
> +                   FWCfgReadCallback fw_callback, void *callback_opaque,
> +                   bool extendable)
>  {
>      Rom *rom;
>      void *data = NULL;
> @@ -742,6 +743,9 @@ void *rom_add_blob(const char *name, const void *blob, size_t len,
>  
>          if (rom_file_has_mr) {
>              data = rom_set_mr(rom, OBJECT(fw_cfg), devpath);
> +            if (extendable) {
> +                memory_region_permit_extendable_migration(rom->mr);
> +            }
>          } else {
>              data = rom->data;
>          }
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index ebc5f03..651c06b 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1558,7 +1558,7 @@ static void *acpi_add_rom_blob(AcpiBuildState *build_state, GArray *blob,
>                                 const char *name)
>  {
>      return rom_add_blob(name, blob->data, acpi_data_len(blob), -1, name,
> -                        acpi_build_update, build_state);
> +                        acpi_build_update, build_state, true);
>  }
>  
>  static const VMStateDescription vmstate_acpi_build = {
> diff --git a/include/hw/loader.h b/include/hw/loader.h
> index 796cbf9..e5a24ac 100644
> --- a/include/hw/loader.h
> +++ b/include/hw/loader.h
> @@ -57,7 +57,8 @@ int rom_add_file(const char *file, const char *fw_dir,
>                   bool option_rom);
>  void *rom_add_blob(const char *name, const void *blob, size_t len,
>                     hwaddr addr, const char *fw_file_name,
> -                   FWCfgReadCallback fw_callback, void *callback_opaque);
> +                   FWCfgReadCallback fw_callback, void *callback_opaque,
> +                   bool extendable);
>  int rom_add_elf_program(const char *name, void *data, size_t datasize,
>                          size_t romsize, hwaddr addr);
>  int rom_load_all(void);
> @@ -70,7 +71,7 @@ void do_info_roms(Monitor *mon, const QDict *qdict);
>  #define rom_add_file_fixed(_f, _a, _i)          \
>      rom_add_file(_f, NULL, _a, _i, false)
>  #define rom_add_blob_fixed(_f, _b, _l, _a)      \
> -    rom_add_blob(_f, _b, _l, _a, NULL, NULL, NULL)
> +    rom_add_blob(_f, _b, _l, _a, NULL, NULL, NULL, false)
>  
>  #define PC_ROM_MIN_VGA     0xc0000
>  #define PC_ROM_MIN_OPTION  0xc8000

[Qemu-devel] [PATCH for-2.1 v3 2/2] acpi: mark ACPI tables ROM blob as extend-able on migration

[Igor Mammedov]

It fixes migration failure for machine type pc-i440fx-1.7 from
QEMU 1.7/2.0 to QEMU 2.1

Migration fails due to ACPI tables size grows across 1.7-2.1
versions. That causes ACPI tables ROM blob to change its size
differently for the same configurations on different QEMU versions.
As result migration code bails out when attempting to load
smaller ROM blob into a bigger one on a newer QEMU.
To trigger failure it's enough to add pci-bridge device to QEMU CLI

Marking ACPI tables ROM blob as extend-able on migration allows
QEMU to load smaller ACPI tables from QEMU 1.7/2.0, fixing
forward migration failure introduced since 2.0 which affects
only configurations that cause ACPI ROM blob size change.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
v3:
  fix build breakage of lm32 target
---
 hw/core/loader.c       | 6 +++++-
 hw/i386/acpi-build.c   | 2 +-
 hw/lm32/lm32_hwsetup.h | 2 +-
 include/hw/loader.h    | 5 +++--
 4 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/hw/core/loader.c b/hw/core/loader.c
index 2bf6b8f..d09b894 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -722,7 +722,8 @@ err:
 
 void *rom_add_blob(const char *name, const void *blob, size_t len,
                    hwaddr addr, const char *fw_file_name,
-                   FWCfgReadCallback fw_callback, void *callback_opaque)
+                   FWCfgReadCallback fw_callback, void *callback_opaque,
+                   bool extendable)
 {
     Rom *rom;
     void *data = NULL;
@@ -742,6 +743,9 @@ void *rom_add_blob(const char *name, const void *blob, size_t len,
 
         if (rom_file_has_mr) {
             data = rom_set_mr(rom, OBJECT(fw_cfg), devpath);
+            if (extendable) {
+                memory_region_permit_extendable_migration(rom->mr);
+            }
         } else {
             data = rom->data;
         }
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index ebc5f03..651c06b 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1558,7 +1558,7 @@ static void *acpi_add_rom_blob(AcpiBuildState *build_state, GArray *blob,
                                const char *name)
 {
     return rom_add_blob(name, blob->data, acpi_data_len(blob), -1, name,
-                        acpi_build_update, build_state);
+                        acpi_build_update, build_state, true);
 }
 
 static const VMStateDescription vmstate_acpi_build = {
diff --git a/hw/lm32/lm32_hwsetup.h b/hw/lm32/lm32_hwsetup.h
index 9fd5e69..943130c 100644
--- a/hw/lm32/lm32_hwsetup.h
+++ b/hw/lm32/lm32_hwsetup.h
@@ -73,7 +73,7 @@ static inline void hwsetup_free(HWSetup *hw)
 static inline void hwsetup_create_rom(HWSetup *hw,
         hwaddr base)
 {
-    rom_add_blob("hwsetup", hw->data, TARGET_PAGE_SIZE, base, NULL, NULL, NULL);
+    rom_add_blob_fixed("hwsetup", hw->data, TARGET_PAGE_SIZE, base);
 }
 
 static inline void hwsetup_add_u8(HWSetup *hw, uint8_t u)
diff --git a/include/hw/loader.h b/include/hw/loader.h
index 796cbf9..e5a24ac 100644
--- a/include/hw/loader.h
+++ b/include/hw/loader.h
@@ -57,7 +57,8 @@ int rom_add_file(const char *file, const char *fw_dir,
                  bool option_rom);
 void *rom_add_blob(const char *name, const void *blob, size_t len,
                    hwaddr addr, const char *fw_file_name,
-                   FWCfgReadCallback fw_callback, void *callback_opaque);
+                   FWCfgReadCallback fw_callback, void *callback_opaque,
+                   bool extendable);
 int rom_add_elf_program(const char *name, void *data, size_t datasize,
                         size_t romsize, hwaddr addr);
 int rom_load_all(void);
@@ -70,7 +71,7 @@ void do_info_roms(Monitor *mon, const QDict *qdict);
 #define rom_add_file_fixed(_f, _a, _i)          \
     rom_add_file(_f, NULL, _a, _i, false)
 #define rom_add_blob_fixed(_f, _b, _l, _a)      \
-    rom_add_blob(_f, _b, _l, _a, NULL, NULL, NULL)
+    rom_add_blob(_f, _b, _l, _a, NULL, NULL, NULL, false)
 
 #define PC_ROM_MIN_VGA     0xc0000
 #define PC_ROM_MIN_OPTION  0xc8000
-- 
1.8.3.1