Skip to contentSkip to conversation

qemu-kvm.git uq/master queue

GIT PULL In review Marcelo Tosatti 2 participants 6 messages Nov 15, 2012 – Mar 21, 2017 KVM

lore
Share & tools
lore
#0 lore raw

The following changes since commit ce34cf72fe508b27a78f83c184142e8d1e6a048a:

Merge remote-tracking branch 'awilliam/tags/vfio-pci-for-qemu-1.3.0-rc0' into staging (2012-11-14 08:53:40 -0600)

are available in the git repository at:

git://git.kernel.org/pub/scm/virt/kvm/qemu-kvm.git uq/master

Jan Kiszka (1):
kvm: Actually remove software breakpoints from list on cleanup

Marcelo Tosatti (1):
acpi_piix4: fix migration of gpe fields

 hw/acpi_piix4.c |   50 ++++++++++++++++++++++++++++++++++++++++++++++----
 kvm-all.c       |    2 ++
 2 files changed, 48 insertions(+), 4 deletions(-)
#1 lore raw

PATCH 1/2 acpi_piix4: fix migration of gpe fields

Migrate 16 bytes for en/sts fields (which is the correct size), increase version to 3, and document how to support incoming migration from qemu-kvm 1.2.

---
 hw/acpi_piix4.c |   50 ++++++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 46 insertions(+), 4 deletions(-)
hw/acpi_piix4.c +46 -4
index 15275cf..519269a 100644
@@ -235,10 +235,9 @@ static int vmstate_acpi_post_load(void *opaque, int version_id)
  {                                                                   \
      .name       = (stringify(_field)),                              \
      .version_id = 0,                                                \
-     .num        = GPE_LEN,                                          \
      .info       = &vmstate_info_uint16,                             \
      .size       = sizeof(uint16_t),                                 \
-     .flags      = VMS_ARRAY | VMS_POINTER,                          \
+     .flags      = VMS_SINGLE | VMS_POINTER,                         \
      .offset     = vmstate_offset_pointer(_state, _field, uint8_t),  \
  }
 
@@ -267,11 +266,54 @@ static const VMStateDescription vmstate_pci_status = {
     }
 };
 
+static int acpi_load_old(QEMUFile *f, void *opaque, int version_id)
+{
+    PIIX4PMState *s = opaque;
+    int ret, i;
+    uint16_t temp;
+
+    ret = pci_device_load(&s->dev, f);
+    if (ret < 0) {
+        return ret;
+    }
+    qemu_get_be16s(f, &s->ar.pm1.evt.sts);
+    qemu_get_be16s(f, &s->ar.pm1.evt.en);
+    qemu_get_be16s(f, &s->ar.pm1.cnt.cnt);
+
+    ret = vmstate_load_state(f, &vmstate_apm, opaque, 1);
+    if (ret) {
+        return ret;
+    }
+
+    qemu_get_timer(f, s->ar.tmr.timer);
+    qemu_get_sbe64s(f, &s->ar.tmr.overflow_time);
+
+    qemu_get_be16s(f, (uint16_t *)s->ar.gpe.sts);
+    for (i = 0; i < 3; i++) {
+        qemu_get_be16s(f, &temp);
+    }
+
+    qemu_get_be16s(f, (uint16_t *)s->ar.gpe.en);
+    for (i = 0; i < 3; i++) {
+        qemu_get_be16s(f, &temp);
+    }
+
+    ret = vmstate_load_state(f, &vmstate_pci_status, opaque, 1);
+    return ret;
+}
+
+/* qemu-kvm 1.2 uses version 3 but advertised as 2
+ * To support incoming qemu-kvm 1.2 migration, change version_id
+ * and minimum_version_id to 2 below (which breaks migration from
+ * qemu 1.2).
+ *
+ */
 static const VMStateDescription vmstate_acpi = {
     .name = "piix4_pm",
-    .version_id = 2,
-    .minimum_version_id = 1,
+    .version_id = 3,
+    .minimum_version_id = 3,
     .minimum_version_id_old = 1,
+    .load_state_old = acpi_load_old,
     .post_load = vmstate_acpi_post_load,
     .fields      = (VMStateField []) {
         VMSTATE_PCI_DEVICE(dev, PIIX4PMState),
Signature · 1 line
1.7.6.4
#2 lore raw

Re: [PATCH 1/2] acpi_piix4: fix migration of gpe fields

Hello Marcelo, cc:qemu,

Sorry for re-using this old thread, but I have a problem loading some saved state from qemu-kvm-1.1.2, which fails for piix4_pm.

You following patch was committed as
<http://git.qemu-project.org/?p=qemu.git;a=commitdiff;h=b0b873a07872f7ab7f66f259c73fb9dd42aa66a9>:

Am 15.11.2012 um 01:11 schrieb Marcelo Tosatti:

Quoted 3 lines

Migrate 16 bytes for en/sts fields (which is the correct size), increase version to 3, and document how to support incoming migration from qemu-kvm 1.2.

I my case qemu-kvm-1.1.2/hw/acpi_piix4.c:284
| VMSTATE_STRUCT(ar.gpe, PIIX4PMState, 2, vmstate_gpe, ACPIGPE), only saves 4 bytes, not 16 bytes.

Quoted 23 lines
---
 hw/acpi_piix4.c |   50 ++++++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 46 insertions(+), 4 deletions(-)
hw/acpi_piix4.c +1 -2
index 15275cf..519269a 100644
@@ -235,10 +235,9 @@ static int vmstate_acpi_post_load(void *opaque, int version_id)
  {                                                                   \
      .name       = (stringify(_field)),                              \
      .version_id = 0,                                                \
-     .num        = GPE_LEN,                                          \
      .info       = &vmstate_info_uint16,                             \
      .size       = sizeof(uint16_t),                                 \
-     .flags      = VMS_ARRAY | VMS_POINTER,                          \
+     .flags      = VMS_SINGLE | VMS_POINTER,                         \
      .offset     = vmstate_offset_pointer(_state, _field, uint8_t),  \
  }
 

hw/acpi_piix4.c static const VMStateDescription vmstate_pci_status = { · line 267

Quoted 2 lines
hw/acpi_piix4.c +0 -0
@@ -267,11 +266,54 @@ static const VMStateDescription vmstate_pci_status = {
     }
 };

On load qemu-2.8 uses this function

Quoted 16 lines
hw/acpi_piix4.c +16 -0
+static int acpi_load_old(QEMUFile *f, void *opaque, int version_id)
+{
+    PIIX4PMState *s = opaque;
+    int ret, i;
+    uint16_t temp;
+
+    ret = pci_device_load(&s->dev, f);
+    if (ret < 0) {
+        return ret;
+    }
+    qemu_get_be16s(f, &s->ar.pm1.evt.sts);
+    qemu_get_be16s(f, &s->ar.pm1.evt.en);
+    qemu_get_be16s(f, &s->ar.pm1.cnt.cnt);
+
+    ret = vmstate_load_state(f, &vmstate_apm, opaque, 1);
+    if (ret) {

hw/acpi_piix4.c static const VMStateDescription vmstate_pci_status = { · line 293

Quoted 10 lines
hw/acpi_piix4.c +10 -0
+        return ret;
+    }
+
+    qemu_get_timer(f, s->ar.tmr.timer);
+    qemu_get_sbe64s(f, &s->ar.tmr.overflow_time);
+
+    qemu_get_be16s(f, (uint16_t *)s->ar.gpe.sts);
+    for (i = 0; i < 3; i++) {
+        qemu_get_be16s(f, &temp);
+    }

and the loop loads 6 extra bytes here

hw/acpi_piix4.c static const VMStateDescription vmstate_pci_status = { · line 298

Quoted 5 lines
hw/acpi_piix4.c +5 -0
+
+    qemu_get_be16s(f, (uint16_t *)s->ar.gpe.en);
+    for (i = 0; i < 3; i++) {
+        qemu_get_be16s(f, &temp);
+    }

and 6 here.
So in total 12 bytes are read too much and qemu_loadvm_state() ends somewhere in the middle of the next device state section. (patch 0001 helped me to identify that offset)

If I apply the attached 0002 patch, I can load the old VM state.

Quoted 13 lines
hw/acpi_piix4.c +11 -0
+
+    ret = vmstate_load_state(f, &vmstate_pci_status, opaque, 1);
+    return ret;
+}
+
+/* qemu-kvm 1.2 uses version 3 but advertised as 2
+ * To support incoming qemu-kvm 1.2 migration, change version_id
+ * and minimum_version_id to 2 below (which breaks migration from
+ * qemu 1.2).
+ *
+ */
 static const VMStateDescription vmstate_acpi = {
     .name = "piix4_pm",

hw/acpi_piix4.c static const VMStateDescription vmstate_pci_status = { · line 319

Quoted 10 lines
hw/acpi_piix4.c +3 -2
-    .version_id = 2,
-    .minimum_version_id = 1,
+    .version_id = 3,
+    .minimum_version_id = 3,
     .minimum_version_id_old = 1,
+    .load_state_old = acpi_load_old,
     .post_load = vmstate_acpi_post_load,
     .fields      = (VMStateField []) {
         VMSTATE_PCI_DEVICE(dev, PIIX4PMState),
 

Do you remember why 16 bytes were saved in your case?

Thank you in advance.

Philipp

From 4e43999b0f56975dbbd528e4956231b2ee64c071 Mon Sep 17 00:00:00 2001
Message-Id: <4e43999b0f56975dbbd528e4956231b2ee64c071.1490011194.git.hahn@univention.de>
In-Reply-To: <1313efdc0ac664169c5c271f832264efe6659743.1490011194.git.hahn@univention.de>
References: <1313efdc0ac664169c5c271f832264efe6659743.1490011194.git.hahn@univention.de>
From: Philipp Hahn <[email protected]>
Date: Mon, 20 Mar 2017 12:36:53 +0100
Subject: [PATCH 2/2] 0007-Bug-38877-fix-qemu-kvm-1.1-piix4_pm-incompatibility
Organization: Univention GmbH, Bremen, Germany
To: [email protected]

qemu-kvm-1.1 only saves 2*uint16, but qemu-2.8 tries to load 2*4*uint16.

The code was added by b0b873a07872f7ab7f66f259c73fb9dd42aa66a9 and the commit message claims 16 bytes are the supposed size, but as UCS never used qemu-kvm-1.2, fix the case for qemu-kvm-1.1

Gbp: Topic UCS

---
 hw/acpi/piix4.c | 4 ++++
 1 file changed, 4 insertions(+)
hw/acpi/piix4.c +4 -0
index 17d36bd..c756d18 100644
@@ -232,14 +232,18 @@ static int acpi_load_old(QEMUFile *f, void *opaque, int version_id)
     qemu_get_sbe64s(f, &s->ar.tmr.overflow_time);
 
     qemu_get_be16s(f, (uint16_t *)s->ar.gpe.sts);
+#if 0 // Bug #38877
     for (i = 0; i < 3; i++) {
         qemu_get_be16s(f, &temp);
     }
+#endif
 
     qemu_get_be16s(f, (uint16_t *)s->ar.gpe.en);
+#if 0 // Bug #38877
     for (i = 0; i < 3; i++) {
         qemu_get_be16s(f, &temp);
     }
+#endif
 
     ret = vmstate_load_state(f, &vmstate_pci_status,
         &s->acpi_pci_hotplug.acpi_pcihp_pci_status[ACPI_PCIHP_BSEL_DEFAULT], 1);
Signature · 1 line
2.1.4

From 1313efdc0ac664169c5c271f832264efe6659743 Mon Sep 17 00:00:00 2001
Message-Id: <1313efdc0ac664169c5c271f832264efe6659743.1490011194.git.hahn@univention.de>
From: Philipp Hahn <[email protected]>
Date: Fri, 17 Mar 2017 12:51:01 +0100
Subject: [PATCH 1/2] 0006-Bug-38877-debug-loadvm-offset
Organization: Univention GmbH, Bremen, Germany
To: [email protected]

debug: Print offset on section error:

[email protected]:qemu_loadvm_state_section 48@0x364da2

Gbp: Topic debug

---
 include/migration/qemu-file.h | 1 +
 migration/qemu-file.c         | 5 +++++
 migration/savevm.c            | 2 +-
 migration/trace-events        | 2 +-
 4 files changed, 8 insertions(+), 2 deletions(-)
include/migration/qemu-file.h +1 -0
index abedd46..a570836 100644
@@ -126,6 +126,7 @@ int qemu_get_fd(QEMUFile *f);
 int qemu_fclose(QEMUFile *f);
 int64_t qemu_ftell(QEMUFile *f);
 int64_t qemu_ftell_fast(QEMUFile *f);
+int64_t qemu_ftell_internal(QEMUFile *f);
 void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, size_t size);
 void qemu_put_byte(QEMUFile *f, int v);
 /*
migration/qemu-file.c +5 -0
index e9fae31..862b8c7 100644
@@ -543,6 +543,11 @@ int64_t qemu_ftell(QEMUFile *f)
     return f->pos;
 }
 
+int64_t qemu_ftell_internal(QEMUFile *f)
+{
+    return f->pos - f->buf_size + f->buf_index;
+}
+
 int qemu_file_rate_limit(QEMUFile *f)
 {
     if (qemu_file_get_error(f)) {
migration/savevm.c +1 -1
index 0363372..da2a98b 100644
@@ -1882,7 +1882,7 @@ static int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis)
 
     while ((section_type = qemu_get_byte(f)) != QEMU_VM_EOF) {
         ret = 0;
-        trace_qemu_loadvm_state_section(section_type);
+        trace_qemu_loadvm_state_section(section_type, qemu_ftell_internal(f) - 1);
         switch (section_type) {
         case QEMU_VM_SECTION_START:
         case QEMU_VM_SECTION_FULL:
migration/trace-events +1 -1
index 94134f7..49f8458 100644
@@ -1,7 +1,7 @@
 # See docs/tracing.txt for syntax documentation.
 
 # migration/savevm.c
-qemu_loadvm_state_section(unsigned int section_type) "%d"
+qemu_loadvm_state_section(unsigned int section_type, uint64_t offset) "%d@0x%" PRIx64
 qemu_loadvm_state_section_command(int ret) "%d"
 qemu_loadvm_state_section_partend(uint32_t section_id) "%u"
 qemu_loadvm_state_main(void) ""
Signature · 1 line
2.1.4
#3 lore raw

Re: [PATCH 1/2] acpi_piix4: fix migration of gpe fields

Quoted 8 lines · Philipp Hahn

Hello Marcelo, cc:qemu,

Sorry for re-using this old thread, but I have a problem loading some saved state from qemu-kvm-1.1.2, which fails for piix4_pm.

You following patch was committed as
<http://git.qemu-project.org/?p=qemu.git;a=commitdiff;h=b0b873a07872f7ab7f66f259c73fb9dd42aa66a9>:

Quoted 8 lines

Am 15.11.2012 um 01:11 schrieb Marcelo Tosatti:

Quoted 3 lines

Migrate 16 bytes for en/sts fields (which is the correct size), increase version to 3, and document how to support incoming migration from qemu-kvm 1.2.

I my case qemu-kvm-1.1.2/hw/acpi_piix4.c:284
| VMSTATE_STRUCT(ar.gpe, PIIX4PMState, 2, vmstate_gpe, ACPIGPE), only saves 4 bytes, not 16 bytes.

Hi Philipp,

IIRC qemu-1.1.2 saved 16 bytes, which is the correct size, not 4 bytes.

So while merging from qemu-kvm -> qemu, it was decided to maintain backwards compability with qemu, and not qemu-kvm.

Is there any way to differentiate between the two (qemu vs qemu-kvm, perhaps via some other field not in the VMState of ACPI PIIX4), so your patch can be integrated upstream?

#4 lore raw

Re: [PATCH 1/2] acpi_piix4: fix migration of gpe fields

Hello Marcelo,

Am 21.03.2017 um 00:20 schrieb Marcelo Tosatti:

Quoted 14 lines · Philipp Hahn
Quoted 13 lines

Hello Marcelo, cc:qemu,

Sorry for re-using this old thread, but I have a problem loading some saved state from qemu-kvm-1.1.2, which fails for piix4_pm.

You following patch was committed as
<http://git.qemu-project.org/?p=qemu.git;a=commitdiff;h=b0b873a07872f7ab7f66f259c73fb9dd42aa66a9>:

Am 15.11.2012 um 01:11 schrieb Marcelo Tosatti:

Quoted 3 lines

Migrate 16 bytes for en/sts fields (which is the correct size), increase version to 3, and document how to support incoming migration from qemu-kvm 1.2.

Quoted 5 lines
Quoted 3 lines

I my case qemu-kvm-1.1.2/hw/acpi_piix4.c:284
| VMSTATE_STRUCT(ar.gpe, PIIX4PMState, 2, vmstate_gpe, ACPIGPE), only saves 4 bytes, not 16 bytes.

IIRC qemu-1.1.2 saved 16 bytes, which is the correct size, not 4 bytes.

I found 23910d3f669d46073b403876e30a7314599633af in qemu, which changed "gpe" to be an array[4] instead of single "struct gpe_regs" without changing the version number. So the incompatibility was introduced there.

Quoted 6 lines

So while merging from qemu-kvm -> qemu, it was decided to maintain backwards compability with qemu, and not qemu-kvm.

Is there any way to differentiate between the two (qemu vs qemu-kvm, perhaps via some other field not in the VMState of ACPI PIIX4), so your patch can be integrated upstream?

I had a look and haven't found any reliable indicator to distinguish between "qemu" and "qemu-kvm" yet. I'll have another look if my time permits, but as I know "qemu-kvm" was used in my case, I will hard code that knowledge for now in my private build.

Thank you four sharing your knowledge.

Philipp

#5 lore raw

PATCH 2/2 kvm: Actually remove software breakpoints from list on cleanup

In reply to Marcelo Tosatti

From: Jan Kiszka <[email protected]>

So far we only removed them from the guest, leaving its states in the list. This made it impossible for gdb to re-enable breakpoints on the same address after re-attaching.

---
 kvm-all.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
kvm-all.c +2 -0
index b6d0483..3bc3347 100644
@@ -1905,6 +1905,8 @@ void kvm_remove_all_breakpoints(CPUArchState *current_env)
                 }
             }
         }
+        QTAILQ_REMOVE(&s->kvm_sw_breakpoints, bp, entry);
+        g_free(bp);
     }
     kvm_arch_remove_all_hw_breakpoints();
 
Signature · 1 line
1.7.6.4