Re: [Qemu-devel] [PULL 13/21] apic_common: vapic_paddr synchronization fix

["Pavel Dovgaluk" <Pavel.Dovgaluk@ispras.ru>]

> From: Paolo Bonzini [mailto:paolo.bonzini@gmail.com] On Behalf Of Paolo Bonzini
> 
> 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 <Pavel.Dovgaluk@ispras.ru>
> Tested-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/i386/kvmvapic.c | 37 ++++++++++++++++++++++++++-----------
>  1 file changed, 26 insertions(+), 11 deletions(-)

One more issue for this patch.

> -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) {

Exitting here doesn't remove vmsentry. When we load VM state for multiple times,
list of the handlers will be filled with garbage.

> +        return;
> +    }
> +
> +    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);
> +        }
> +    }
> +
> +    qemu_del_vm_change_state_handler(s->vmsentry);
> +}


Pavel Dovgalyuk