[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 02/10] apic_common: vapic_paddr synchronization
From: |
Michael S. Tsirkin |
Subject: |
Re: [Qemu-devel] [PATCH 02/10] apic_common: vapic_paddr synchronization fix |
Date: |
Tue, 9 Sep 2014 17:00:04 +0300 |
On Tue, Sep 09, 2014 at 02:30:01PM +0200, Paolo Bonzini wrote:
> From: Pavel Dovgalyuk <address@hidden>
>
> This patch postpones vapic_paddr initialization, which is performed
> during migration. When vapic_paddr is synchronized within the migration
> process, apic_common functions could operate with incorrect apic state,
> if it hadn't loaded yet. This patch postpones the synchronization until
> the virtual machine is started, ensuring that the whole virtual machine
> state has been loaded.
>
> Signed-off-by: Pavel Dovgalyuk <address@hidden>
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
> hw/i386/kvmvapic.c | 37 ++++++++++++++++++++++++++-----------
> 1 file changed, 26 insertions(+), 11 deletions(-)
>
> diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c
> index ee95963..2bcc249 100644
> --- a/hw/i386/kvmvapic.c
> +++ b/hw/i386/kvmvapic.c
> @@ -59,6 +59,7 @@ typedef struct VAPICROMState {
> GuestROMState rom_state;
> size_t rom_size;
> bool rom_mapped_writable;
> + VMChangeStateEntry *vmsentry;
> } VAPICROMState;
>
> #define TYPE_VAPIC "kvmvapic"
> @@ -734,11 +735,34 @@ static void do_vapic_enable(void *data)
> vapic_enable(s, cpu);
> }
>
> -static int vapic_post_load(void *opaque, int version_id)
> +static void kvmvapic_vm_state_change(void *opaque, int running,
> + RunState state)
> {
> VAPICROMState *s = opaque;
> uint8_t *zero;
>
> + if (!running) {
> + return;
> + }
> +
> + if (s->state == VAPIC_ACTIVE) {
> + if (smp_cpus == 1) {
maybe add a comment explaining why we need
to special case smp_cpus?
> + run_on_cpu(first_cpu, do_vapic_enable, s);
Is this safe to do in vmstate handler? cpu isn't running yet is it?
> + } else {
> + zero = g_malloc0(s->rom_state.vapic_size);
> + cpu_physical_memory_write(s->vapic_paddr, zero,
> + s->rom_state.vapic_size);
> + g_free(zero);
> + }
> + }
> +
> + qemu_del_vm_change_state_handler(s->vmsentry);
> +}
> +
> +static int vapic_post_load(void *opaque, int version_id)
> +{
> + VAPICROMState *s = opaque;
> +
> /*
> * The old implementation of qemu-kvm did not provide the state
> * VAPIC_STANDBY. Reconstruct it.
> @@ -752,17 +776,8 @@ static int vapic_post_load(void *opaque, int version_id)
> return -1;
> }
> }
> - if (s->state == VAPIC_ACTIVE) {
> - if (smp_cpus == 1) {
> - run_on_cpu(first_cpu, do_vapic_enable, s);
> - } else {
> - zero = g_malloc0(s->rom_state.vapic_size);
> - cpu_physical_memory_write(s->vapic_paddr, zero,
> - s->rom_state.vapic_size);
> - g_free(zero);
> - }
> - }
>
> + s->vmsentry = qemu_add_vm_change_state_handler(kvmvapic_vm_state_change,
> s);
> return 0;
> }
>
> --
> 2.1.0
>
>