Re: [Qemu-devel] [RFC PATCH v3 07/49] kvmapic: fixing loading vmstate

Re: [Qemu-devel] [RFC PATCH v3 07/49] kvmapic: fixing loading vmstate

[Pavel Dovgalyuk]

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

Pre load is necessary, because we switched off resetting VM while loading in the replay mode. Calling reset handlers generates irqs, that make loading process non-deterministic.


Sent from mobile device



-------- Исходное сообщение --------
От: Paolo Bonzini <pbonzini@redhat.com> 
Дата:31.07.2014  17:01  (GMT+04:00) 
Кому: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>,qemu-devel@nongnu.org 
Копия: peter.maydell@linaro.org,peter.crosthwaite@xilinx.com,mark.burton@greensocs.com,real@ispras.ru,batuzovk@ispras.ru,maria.klimushenkova@ispras.ru,eblake@redhat.com,afaerber@suse.de,fred.konrad@greensocs.com 
Тема: Re: [RFC PATCH v3 07/49] kvmapic: fixing loading vmstate 

Il 31/07/2014 14:54, Pavel Dovgalyuk ha scritto:
> +static int apic_pre_load(void *opaque)
> +{
> +    APICCommonState *s = APIC_COMMON(opaque);
> +    s->sipi_vector = 0;
> +    s->wait_for_sipi = 0;
> +    s->vapic_paddr = 0;
> +    return 0;

Is this necessary?  Or does reset already do that?

> +}
> +
>  static void apic_dispatch_pre_save(void *opaque)
>  {
>      APICCommonState *s = APIC_COMMON(opaque);
> @@ -345,12 +354,46 @@ static int apic_dispatch_post_load(void *opaque, int version_id)
>      return 0;
>  }
>  
> +static bool apic_common_sipi_needed(void *opaque)
> +{
> +    APICCommonState *s = APIC_COMMON(opaque);
> +    return s->wait_for_sipi != 0;
> +}
> +
> +static const VMStateDescription vmstate_apic_common_sipi = {
> +    .name = "apic_sipi",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_INT32(sipi_vector, APICCommonState),
> +        VMSTATE_INT32(wait_for_sipi, APICCommonState),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +static bool apic_common_vapic_paddr_needed(void *opaque)
> +{
> +    APICCommonState *s = APIC_COMMON(opaque);
> +    return s->vapic_paddr != 0;
> +}
> +
> +static const VMStateDescription vmstate_apic_common_vapic_paddr = {
> +    .name = "apic_vapic_paddr",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT64(vapic_paddr, APICCommonState),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
>  static const VMStateDescription vmstate_apic_common = {
>      .name = "apic",
> -    .version_id = 3,
> +    .version_id = 4,

You know what I'm going to say. :)

Paolo

[-- Attachment #2: Type: text/html, Size: 3518 bytes --]

Re: [Qemu-devel] [RFC PATCH v3 07/49] kvmapic: fixing loading vmstate

[Paolo Bonzini]

Il 31/07/2014 17:21, Pavel Dovgalyuk ha scritto:
> Pre load is necessary, because we switched off resetting VM while
> loading in the replay mode.

Then you should not add it now, but rather when you add replay.  Treat
this part of the series as merely fixing migration bugs.  In fact, I
suggest that you start by progressively refining these patches.  You can
also post the other patches that I mentioned were good to go in my
review of v1.  Then pass to the next steps (in the meanwhile, Fred's
icount reverse-exec patches might get merged too).

Related to this: do _not_ assume that people review the entire series.
It's normal to notice that the initial part should stand on its own
legs, and then review it as if the remaining patches do not exist!  It's
up to _you_, in the commit messages, to annotate things that you do now
for the sake of future patches.

> Calling reset handlers generates irqs, that
> make loading process non-deterministic.

What irqs are these, and why do they make the loading process
non-deterministic?  Is it important that they not be generated, as long
as the final state is deterministic?

Have you audited all subsections and add a pre-load function there?
Where do you do this in the series?  These non-mechanical, sweeping
changes suggest to me that you find a way to keep resets during load.
Compare for example the patches at

   http://lists.gnu.org/archive/html/qemu-devel/2013-09/msg00477.html

with the series at

   http://lists.gnu.org/archive/html/qemu-devel/2014-06/msg02652.html
   http://lists.gnu.org/archive/html/qemu-devel/2014-07/msg03934.html

The former adds 230 lines of code and adds code to 40-odd files.

The latter includes a preparatory part that is complicated but only
touches 4 files, handling of the odd cases that touches about 10 files,
and the final sweeping change that is mechanical and hardly requires
review.  Overall it adds ~20 lines of code, and adds actual
functionality in addition to fixing the same bug as the first!

Paolo

Re: [Qemu-devel] [RFC PATCH v3 07/49] kvmapic: fixing loading vmstate

[Pavel Dovgaluk]

> From: Paolo Bonzini [mailto:pbonzini@redhat.com]
> Il 31/07/2014 17:21, Pavel Dovgalyuk ha scritto:
> > Pre load is necessary, because we switched off resetting VM while
> > loading in the replay mode.
> 
> Then you should not add it now, but rather when you add replay.  Treat
> this part of the series as merely fixing migration bugs.  In fact, I
> suggest that you start by progressively refining these patches.  You can
> also post the other patches that I mentioned were good to go in my
> review of v1.  Then pass to the next steps (in the meanwhile, Fred's
> icount reverse-exec patches might get merged too).

 Do you mean, that I should separate the migration patches from 
the replay ones and submit them in another series?

Pavel Dovgalyuk

Re: [Qemu-devel] [RFC PATCH v3 07/49] kvmapic: fixing loading vmstate

[Paolo Bonzini]

Il 25/08/2014 13:40, Pavel Dovgaluk ha scritto:
>  Do you mean, that I should separate the migration patches from 
> the replay ones and submit them in another series?

Yes, that will help getting the less controversial pieces in first.

Paolo

[Qemu-devel] [RFC PATCH v3 00/49] Deterministic replay and reverse execution

[Pavel Dovgalyuk]

This set of patches is related to the reverse execution and deterministic 
replay of qemu execution  Our implementation of deterministic replay can 
be used for deterministic and reverse debugging of guest code through gdb 
remote interface.

Execution recording writes non-deterministic events log, which can be later 
used for replaying the execution anywhere and for unlimited number of times. 
It also supports checkpointing for faster rewinding during reverse debugging. 
Execution replaying reads the log and replays all non-deterministic events 
including external input, hardware clocks, and interrupts.

Reverse execution has the following features:
 * Deterministically replays whole system execution and all contents of the memory,
   state of the hadrware devices, clocks, and screen of the VM.
 * Writes execution log into the file for latter replaying for multiple times 
   on different machines.
 * Supports i386, x86_64, and ARM hardware platforms.
 * Performs deterministic replay of all operations with keyboard, mouse, network adapters,
   audio devices, serial interfaces, and physical USB devices connected to the emulator.
 * Provides support for gdb reverse debugging commands like reverse-step and reverse-continue.
 * Supports auto-checkpointing for convenient reverse debugging.

Usage of the record/replay:
 * First, record the execution, by adding '-record fname=replay.bin' to the
   command line.
 * Then you can replay it for the multiple times by using another command
   line option: '-replay fname=replay.bin'
 * Virtual machine should have at least one virtual disk, which is used to
   store checkpoints. If you want to enable automatic checkpointing, simply
   add ',period=XX' to record options, where XX is the checkpointing period
   in seconds.
 * Using of the network adapters in record/replay mode is possible with 
   the following command-line options:
   - '-net user' (or another host adapter) in record mode
   - '-net replay' in replay mode. Every host network adapter should be
     replaced by 'replay' when replaying the execution.
 * Reverse debugging can be used through gdb remote interface.
   reverse-stepi and reverse-continue commands are supported. Other reverse
   commands should also work, because they reuse these ones.
 * Monitor is extended by the following commands:
   - replay_info - prints information about replay mode and current step
     (number of instructions executed)
   - replay_break - sets "breakpoint" at the specified instructions count.
   - replay_seek - rewinds (using the checkpoints, if possible) to the
     specified step of replay log.

Paper with short description of deterministic replay implementation:
http://www.computer.org/csdl/proceedings/csmr/2012/4666/00/4666a553-abs.html

Modifications of qemu include:
 * adding missed fields of the virtual devices' states to the vmstate 
   structures to allow deterministic saving and restoring the VM state
 * adding virtual clock-based timers to vmstate structures, because virtual 
   clock is the part of the virtual machine state
 * modification of block layer to support automatic creation of the overlay
   files to store the changes and snapshots while recording
 * disabling of system reset while loading VM state to avoid generating of
   interrupts by reset handlers
 * adding warpers for clock and time functions to save their return
   values in the log
 * saving different asynchronous events (e.g. system shutdown) into the log
 * synchronization of the bottom halves execution
 * synchronization of the threads from thread pool
 * recording/replaying user input (mouse and keyboard), input from virtual
   serial ports, incoming network packets, input from connected USB devices
 * adding HMP/QMP commands to monitor for controlling replay execution

v3 changes:
 * Fixed bug with replay of the aio write operations.
 * Added virtual clock based on replay icount.
 * Removed duplicated saving of interrupt_request CPU field.
 * Fixed some coding style issues.
 * Renamed QMP commands for controlling reverse execution (as suggested by Eric Blake)
 * Replay mode and submode implemented as QAPI enumerations (as suggested by Eric Blake)
 * Added description and example for replay-info command (as suggested by Eric Blake)
 * Added information about the current breakpoint to the output of replay-info (as suggested by Eric Blake)
 * Updated version id for HPET vmstate (as suggested by Paolo Bonzini)
 * Removed static fields from parallel vmstate (as suggested by Paolo Bonzini)
 * New vmstate fields for mc146818rtc, pckbd, kvmapic, serial, fdc, rtl8139 moved to subsection (as suggested by Paolo Bonzini)
 * Disabled textmode cursor blinking, when virtual machine is stopped (as suggested by Paolo Bonzini)
 * Extracted saving of exception_index to separate patch (as suggested by Paolo Bonzini)

v2 changes:
 * Patches are split to be reviewable and bisectable (as suggested by Kirill Batuzov)
 * Added QMP versions of replay commands (as suggested by Eric Blake)
 * Removed some optional features of replay to make patches cleaner
 * Minor changes and code cleanup were made

---

Pavel Dovgalyuk (49):
      acpi: accurate overflow check
      integratorcp: adding vmstate for save/restore
      pcspk: adding vmstate for save/restore
      fdc: adding vmstate for save/restore
      parallel: adding vmstate for save/restore
      serial: fixing vmstate for save/restore
      kvmapic: fixing loading vmstate
      hpet: fixing saving and loading process
      pckbd: adding new fields to vmstate
      rtl8139: adding new fields to vmstate
      piix: do not raise irq while loading vmstate
      mc146818rtc: add missed field to vmstate
      pl031: add missed field to vmstate
      ide pci: reset status field before loading the vmstate
      softmmu: fixing usage of cpu_st/ld* from helpers
      target-i386: update fp status fix
      migration: add vmstate for int8 and char arrays
      replay: global variables and function stubs
      block: add suffix parameter to bdrv_open functions
      sysemu: system functions for replay
      replay: internal functions for replay log
      cpu: invent instruction count for accurate replay
      target-arm: instructions counting code for replay
      target-i386: instructions counting code for replay
      replay: interrupts and exceptions
      vga: do not use virtual clock for blinking cursor
      replay: asynchronous events infrastructure
      replay: recording and replaying clock ticks
      replay: recording and replaying different timers
      replay: shutdown event
      replay: checkpoints
      vmclock: add virtual clock based on replay icount
      replay: bottom halves
      replay: replay aio requests
      replay: thread pool
      pl031: vmstate in replay mode
      replay: initialization and deinitialization
      replay: command line options
      replay: snapshotting the virtual machine
      replay: recording of the user input
      tap-win32: destroy the thread at exit
      replay: network packets record/replay
      replay: audio data record/replay
      replay: serial port
      replay: USB passthrough
      replay: replay_info command
      replay: replay_break command
      replay: replay_seek_step command
      gdbstub: reverse debugging


 Makefile.target                  |    1 
 arch_init.c                      |    8 
 async.c                          |   46 ++
 audio/audio.c                    |   14 +
 audio/audio_win_int.h            |    3 
 audio/winwaveaudio.c             |  167 +++++++--
 block.c                          |  140 ++++++-
 block/blkdebug.c                 |    2 
 block/blkverify.c                |    5 
 block/cow.c                      |    2 
 block/qcow.c                     |    2 
 block/qcow2.c                    |   10 -
 block/qed.c                      |    2 
 block/raw-posix.c                |    6 
 block/raw-win32.c                |    4 
 block/sheepdog.c                 |    4 
 block/vmdk.c                     |    8 
 block/vvfat.c                    |    2 
 blockdev.c                       |   14 -
 cpu-exec.c                       |   34 +-
 cpus.c                           |  101 +++++
 dma-helpers.c                    |   10 -
 exec.c                           |   12 +
 gdbstub.c                        |   79 +++-
 hmp-commands.hx                  |   41 ++
 hw/acpi/core.c                   |    7 
 hw/arm/integratorcp.c            |   38 ++
 hw/audio/pcspk.c                 |   18 +
 hw/block/fdc.c                   |   85 ++++
 hw/block/virtio-blk.c            |   10 -
 hw/char/parallel.c               |   20 +
 hw/char/serial.c                 |  264 ++++++++++++--
 hw/display/vga.c                 |    6 
 hw/i386/kvmvapic.c               |   22 +
 hw/ide/ahci.c                    |    4 
 hw/ide/atapi.c                   |    9 
 hw/ide/core.c                    |   18 +
 hw/ide/pci.c                     |   11 +
 hw/input/pckbd.c                 |   53 +++
 hw/intc/apic_common.c            |   56 +++
 hw/net/rtl8139.c                 |   50 +++
 hw/pci-host/piix.c               |   22 +
 hw/timer/arm_timer.c             |    2 
 hw/timer/hpet.c                  |   15 -
 hw/timer/mc146818rtc.c           |   37 ++
 hw/timer/pl031.c                 |   38 ++
 hw/usb/hcd-uhci.c                |    2 
 hw/usb/host-libusb.c             |  551 +++++++++++++++++++----------
 include/block/aio.h              |   18 +
 include/block/block.h            |   22 +
 include/block/thread-pool.h      |    4 
 include/exec/cpu-defs.h          |    2 
 include/exec/cpu_ldst_template.h |   31 +-
 include/exec/exec-all.h          |   31 ++
 include/hw/host-libusb.h         |  105 ++++++
 include/hw/i386/apic_internal.h  |    2 
 include/migration/vmstate.h      |   13 +
 include/qemu-common.h            |    3 
 include/qemu/main-loop.h         |    1 
 include/qemu/timer.h             |   48 ++-
 include/qom/cpu.h                |    3 
 include/sysemu/char.h            |   25 +
 include/sysemu/cpus.h            |    1 
 include/sysemu/sysemu.h          |    2 
 include/ui/input.h               |    2 
 main-loop.c                      |    5 
 monitor.c                        |   58 +++
 net/Makefile.objs                |    1 
 net/clients.h                    |    3 
 net/dump.c                       |    6 
 net/hub.c                        |    1 
 net/net-replay.c                 |   68 ++++
 net/net.c                        |    7 
 net/slirp.c                      |   14 +
 net/socket.c                     |   35 ++
 net/tap-win32.c                  |   25 +
 net/tap.c                        |   24 +
 net/vde.c                        |   14 +
 qapi-schema.json                 |  102 +++++
 qemu-char.c                      |   56 +++
 qemu-img.c                       |    6 
 qemu-io-cmds.c                   |    2 
 qemu-io.c                        |    7 
 qemu-nbd.c                       |    2 
 qemu-options.hx                  |   30 ++
 qemu-timer.c                     |   52 ++-
 qmp-commands.hx                  |   75 ++++
 replay/Makefile.objs             |   12 +
 replay/replay-audio.c            |  228 ++++++++++++
 replay/replay-char.c             |  100 +++++
 replay/replay-debug.c            |  154 ++++++++
 replay/replay-events.c           |  382 ++++++++++++++++++++
 replay/replay-icount.c           |  130 +++++++
 replay/replay-input.c            |  107 ++++++
 replay/replay-internal.c         |  159 ++++++++
 replay/replay-internal.h         |  224 ++++++++++++
 replay/replay-net.c              |  191 ++++++++++
 replay/replay-qmp.c              |   58 +++
 replay/replay-time.c             |  183 ++++++++++
 replay/replay-usb.c              |  188 ++++++++++
 replay/replay.c                  |  718 ++++++++++++++++++++++++++++++++++++++
 replay/replay.h                  |  214 +++++++++++
 savevm.c                         |   33 +-
 slirp/slirp.c                    |    9 
 softmmu_template.h               |   18 +
 stubs/Makefile.objs              |    1 
 stubs/replay.c                   |   47 ++
 target-arm/Makefile.objs         |    1 
 target-arm/helper.h              |    3 
 target-arm/replay_helper.c       |   38 ++
 target-arm/translate.c           |   62 +++
 target-i386/Makefile.objs        |    1 
 target-i386/cpu.c                |    1 
 target-i386/cpu.h                |    8 
 target-i386/fpu_helper.c         |    5 
 target-i386/helper.h             |    3 
 target-i386/machine.c            |    2 
 target-i386/replay_helper.c      |   38 ++
 target-i386/translate.c          |   93 ++++-
 tests/test-thread-pool.c         |    7 
 thread-pool.c                    |   53 ++-
 trace-events                     |    2 
 translate-all.c                  |   15 +
 ui/input.c                       |   80 +++-
 util/iov.c                       |    4 
 vl.c                             |  122 ++++++
 vmstate.c                        |    6 
 127 files changed, 6102 insertions(+), 559 deletions(-)
 create mode 100755 include/hw/host-libusb.h
 create mode 100755 net/net-replay.c
 create mode 100755 replay/Makefile.objs
 create mode 100755 replay/replay-audio.c
 create mode 100755 replay/replay-char.c
 create mode 100755 replay/replay-debug.c
 create mode 100755 replay/replay-events.c
 create mode 100755 replay/replay-icount.c
 create mode 100755 replay/replay-input.c
 create mode 100755 replay/replay-internal.c
 create mode 100755 replay/replay-internal.h
 create mode 100755 replay/replay-net.c
 create mode 100755 replay/replay-qmp.c
 create mode 100755 replay/replay-time.c
 create mode 100755 replay/replay-usb.c
 create mode 100755 replay/replay.c
 create mode 100755 replay/replay.h
 create mode 100755 stubs/replay.c
 create mode 100755 target-arm/replay_helper.c
 create mode 100755 target-i386/replay_helper.c

-- 
Pavel Dovgalyuk

[Qemu-devel] [RFC PATCH v3 07/49] kvmapic: fixing loading vmstate

[Pavel Dovgalyuk]

vapic state should not be synchronized with APIC while loading,
because APIC state could be not loaded yet at that moment.
We just save vapic_paddr in APIC VMState instead of synchronization.

Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
---
 hw/i386/kvmvapic.c              |   22 +++++++++++++++
 hw/intc/apic_common.c           |   56 ++++++++++++++++++++++++++++++++++++++-
 include/hw/i386/apic_internal.h |    2 +
 3 files changed, 77 insertions(+), 3 deletions(-)

diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c
index cb855c7..417ab6a 100644
--- a/hw/i386/kvmvapic.c
+++ b/hw/i386/kvmvapic.c
@@ -351,6 +351,24 @@ static int get_kpcr_number(X86CPU *cpu)
     return kpcr.number;
 }
 
+static int vapic_enable_post_load(VAPICROMState *s, X86CPU *cpu)
+{
+    int cpu_number = get_kpcr_number(cpu);
+    hwaddr vapic_paddr;
+    static const uint8_t enabled = 1;
+
+    if (cpu_number < 0) {
+        return -1;
+    }
+    vapic_paddr = s->vapic_paddr +
+        (((hwaddr)cpu_number) << VAPIC_CPU_SHIFT);
+    cpu_physical_memory_rw(vapic_paddr + offsetof(VAPICState, enabled),
+                           (void *)&enabled, sizeof(enabled), 1);
+    s->state = VAPIC_ACTIVE;
+
+    return 0;
+}
+
 static int vapic_enable(VAPICROMState *s, X86CPU *cpu)
 {
     int cpu_number = get_kpcr_number(cpu);
@@ -731,7 +749,9 @@ static void do_vapic_enable(void *data)
     VAPICROMState *s = data;
     X86CPU *cpu = X86_CPU(first_cpu);
 
-    vapic_enable(s, cpu);
+    /* Do not synchronize with APIC, because it was not loaded yet.
+       Just call the enable function which does not have synchronization. */
+    vapic_enable_post_load(s, cpu);
 }
 
 static int vapic_post_load(void *opaque, int version_id)
diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
index ce3d903..5afd5b2 100644
--- a/hw/intc/apic_common.c
+++ b/hw/intc/apic_common.c
@@ -324,6 +324,15 @@ static void apic_common_realize(DeviceState *dev, Error **errp)
 
 }
 
+static int apic_pre_load(void *opaque)
+{
+    APICCommonState *s = APIC_COMMON(opaque);
+    s->sipi_vector = 0;
+    s->wait_for_sipi = 0;
+    s->vapic_paddr = 0;
+    return 0;
+}
+
 static void apic_dispatch_pre_save(void *opaque)
 {
     APICCommonState *s = APIC_COMMON(opaque);
@@ -345,12 +354,46 @@ static int apic_dispatch_post_load(void *opaque, int version_id)
     return 0;
 }
 
+static bool apic_common_sipi_needed(void *opaque)
+{
+    APICCommonState *s = APIC_COMMON(opaque);
+    return s->wait_for_sipi != 0;
+}
+
+static const VMStateDescription vmstate_apic_common_sipi = {
+    .name = "apic_sipi",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_INT32(sipi_vector, APICCommonState),
+        VMSTATE_INT32(wait_for_sipi, APICCommonState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static bool apic_common_vapic_paddr_needed(void *opaque)
+{
+    APICCommonState *s = APIC_COMMON(opaque);
+    return s->vapic_paddr != 0;
+}
+
+static const VMStateDescription vmstate_apic_common_vapic_paddr = {
+    .name = "apic_vapic_paddr",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT64(vapic_paddr, APICCommonState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static const VMStateDescription vmstate_apic_common = {
     .name = "apic",
-    .version_id = 3,
+    .version_id = 4,
     .minimum_version_id = 3,
     .minimum_version_id_old = 1,
     .load_state_old = apic_load_old,
+    .pre_load = apic_pre_load,
     .pre_save = apic_dispatch_pre_save,
     .post_load = apic_dispatch_post_load,
     .fields = (VMStateField[]) {
@@ -375,6 +418,17 @@ static const VMStateDescription vmstate_apic_common = {
         VMSTATE_INT64(timer_expiry,
                       APICCommonState), /* open-coded timer state */
         VMSTATE_END_OF_LIST()
+    },
+    .subsections = (VMStateSubsection[]) {
+        {
+            .vmsd = &vmstate_apic_common_sipi,
+            .needed = apic_common_sipi_needed,
+        },
+        {
+            .vmsd = &vmstate_apic_common_vapic_paddr,
+            .needed = apic_common_vapic_paddr_needed,
+        },
+        VMSTATE_END_OF_LIST()
     }
 };
 
diff --git a/include/hw/i386/apic_internal.h b/include/hw/i386/apic_internal.h
index 83e2a42..df4381c 100644
--- a/include/hw/i386/apic_internal.h
+++ b/include/hw/i386/apic_internal.h
@@ -124,7 +124,7 @@ struct APICCommonState {
 
     uint32_t vapic_control;
     DeviceState *vapic;
-    hwaddr vapic_paddr; /* note: persistence via kvmvapic */
+    hwaddr vapic_paddr;
 };
 
 typedef struct VAPICState {

Re: [Qemu-devel] [RFC PATCH v3 07/49] kvmapic: fixing loading vmstate

[Paolo Bonzini]

Il 31/07/2014 14:54, Pavel Dovgalyuk ha scritto:
> +static int apic_pre_load(void *opaque)
> +{
> +    APICCommonState *s = APIC_COMMON(opaque);
> +    s->sipi_vector = 0;
> +    s->wait_for_sipi = 0;
> +    s->vapic_paddr = 0;
> +    return 0;

Is this necessary?  Or does reset already do that?

> +}
> +
>  static void apic_dispatch_pre_save(void *opaque)
>  {
>      APICCommonState *s = APIC_COMMON(opaque);
> @@ -345,12 +354,46 @@ static int apic_dispatch_post_load(void *opaque, int version_id)
>      return 0;
>  }
>  
> +static bool apic_common_sipi_needed(void *opaque)
> +{
> +    APICCommonState *s = APIC_COMMON(opaque);
> +    return s->wait_for_sipi != 0;
> +}
> +
> +static const VMStateDescription vmstate_apic_common_sipi = {
> +    .name = "apic_sipi",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_INT32(sipi_vector, APICCommonState),
> +        VMSTATE_INT32(wait_for_sipi, APICCommonState),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +static bool apic_common_vapic_paddr_needed(void *opaque)
> +{
> +    APICCommonState *s = APIC_COMMON(opaque);
> +    return s->vapic_paddr != 0;
> +}
> +
> +static const VMStateDescription vmstate_apic_common_vapic_paddr = {
> +    .name = "apic_vapic_paddr",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT64(vapic_paddr, APICCommonState),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
>  static const VMStateDescription vmstate_apic_common = {
>      .name = "apic",
> -    .version_id = 3,
> +    .version_id = 4,

You know what I'm going to say. :)

Paolo