From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:57494) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XRLwC-0007QT-Ry for qemu-devel@nongnu.org; Tue, 09 Sep 2014 09:58:33 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XRLuh-00024w-Nc for qemu-devel@nongnu.org; Tue, 09 Sep 2014 09:57:00 -0400 Received: from mx1.redhat.com ([209.132.183.28]:30223) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XRLuh-00024h-HR for qemu-devel@nongnu.org; Tue, 09 Sep 2014 09:56:55 -0400 Date: Tue, 9 Sep 2014 17:00:04 +0300 From: "Michael S. Tsirkin" Message-ID: <20140909140004.GB13212@redhat.com> References: <1410265809-27247-1-git-send-email-pbonzini@redhat.com> <1410265809-27247-3-git-send-email-pbonzini@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1410265809-27247-3-git-send-email-pbonzini@redhat.com> Subject: Re: [Qemu-devel] [PATCH 02/10] apic_common: vapic_paddr synchronization fix List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: amit.shah@redhat.com, quintela@redhat.com, qemu-devel@nongnu.org, Pavel.Dovgaluk@ispras.ru, dgilbert@redhat.com On Tue, Sep 09, 2014 at 02:30:01PM +0200, Paolo Bonzini wrote: > From: Pavel Dovgalyuk > > 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 > Signed-off-by: Paolo Bonzini > --- > 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 > >