From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:52797)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1XXTYH-0007Fy-Cx
	for qemu-devel@nongnu.org; Fri, 26 Sep 2014 07:19:10 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1XXTYB-0002hj-Jl
	for qemu-devel@nongnu.org; Fri, 26 Sep 2014 07:19:05 -0400
Received: from mx1.redhat.com ([209.132.183.28]:2887)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1XXTYB-0002fx-Bl
	for qemu-devel@nongnu.org; Fri, 26 Sep 2014 07:18:59 -0400
Message-ID: <54254B99.2020707@redhat.com>
Date: Fri, 26 Sep 2014 13:18:49 +0200
From: Paolo Bonzini <pbonzini@redhat.com>
MIME-Version: 1.0
References: <1410530338-17615-1-git-send-email-pbonzini@redhat.com>
	<1410530338-17615-14-git-send-email-pbonzini@redhat.com>
	<001001cfd97b$8e5edc60$ab1c9520$@Dovgaluk@ispras.ru>
In-Reply-To: <001001cfd97b$8e5edc60$ab1c9520$@Dovgaluk@ispras.ru>
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PULL 13/21] apic_common: vapic_paddr
	synchronization fix
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Pavel Dovgaluk <Pavel.Dovgaluk@ispras.ru>, qemu-devel@nongnu.org

Il 26/09/2014 13:18, Pavel Dovgaluk ha scritto:
>> 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.

Thanks.

Paolo

>> +        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
>