From patchwork Sun Apr 17 13:50:45 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Isaku Yamahata X-Patchwork-Id: 91558 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [140.186.70.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id DDCC0B6FDE for ; Sun, 17 Apr 2011 23:51:04 +1000 (EST) Received: from localhost ([::1]:48122 helo=lists2.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QBSNQ-0004ze-64 for incoming@patchwork.ozlabs.org; Sun, 17 Apr 2011 09:51:00 -0400 Received: from eggs.gnu.org ([140.186.70.92]:55990) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QBSNI-0004zO-Rl for qemu-devel@nongnu.org; Sun, 17 Apr 2011 09:50:53 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QBSNH-0006pF-Nj for qemu-devel@nongnu.org; Sun, 17 Apr 2011 09:50:52 -0400 Received: from mail.valinux.co.jp ([210.128.90.3]:47221) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QBSNH-0006oj-6V for qemu-devel@nongnu.org; Sun, 17 Apr 2011 09:50:51 -0400 Received: from ps.local.valinux.co.jp (vagw.valinux.co.jp [210.128.90.14]) by mail.valinux.co.jp (Postfix) with SMTP id 43AD928077; Sun, 17 Apr 2011 22:50:45 +0900 (JST) Received: (nullmailer pid 9020 invoked by uid 1000); Sun, 17 Apr 2011 13:50:45 -0000 Date: Sun, 17 Apr 2011 22:50:45 +0900 From: Isaku Yamahata To: Avi Kivity Message-ID: <20110417135045.GA8741@valinux.co.jp> References: <150737c6da67c205a17f0e9c5c8861e3d0a79531.1300266238.git.yamahata@valinux.co.jp> <4DAAE87F.6020603@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <4DAAE87F.6020603@redhat.com> User-Agent: Mutt/1.5.19 (2009-01-05) X-Virus-Scanned: clamav-milter 0.95.2 at va-mail.local.valinux.co.jp X-Virus-Status: Clean X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 3) X-Received-From: 210.128.90.3 Cc: qemu-devel@nongnu.org, Juan Quintela Subject: Re: [Qemu-devel] [PATCH 24/26] acpi, acpi_piix: factor out GPE logic X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org On Sun, Apr 17, 2011 at 04:17:51PM +0300, Avi Kivity wrote: > On 03/16/2011 11:29 AM, Isaku Yamahata wrote: >> factor out ACPI GPE logic. Later it will be used by ICH9 ACPI. >> > > I think this patch is causing qemu-kvm failures on migration: > (gdb) bt > #0 0x000000000049aff4 in qemu_put_be16s (f=0x1a74490, pv=0x2c02580, > size=2) at hw/hw.h:108 > #1 put_uint16 (f=0x1a74490, pv=0x2c02580, size=2) at savevm.c:855 > #2 0x000000000049c3e4 in vmstate_save_state (f=0x1a74490, > vmsd=0x6f0b00, opaque=0x1842ef0) at savevm.c:1436 > #3 0x000000000049c3b6 in vmstate_save_state (f=0x1a74490, > vmsd=0x6f0aa0, opaque=0x1842b90) at savevm.c:1434 > #4 0x000000000049c6f1 in vmstate_save (mon=, > f=0x1a74490) at savevm.c:1459 > #5 qemu_savevm_state_complete (mon=, f=0x1a74490) > at savevm.c:1600 > #6 0x000000000049455a in migrate_fd_put_ready (opaque=0x1847890) at > migration.c:383 > #7 0x00000000004ce2eb in qemu_run_timers (clock=) > at qemu-timer.c:505 > #8 0x00000000004ce806 in qemu_run_all_timers () at qemu-timer.c:619 > #9 0x0000000000419463 in main_loop_wait (nonblocking= out>) at /build/home/tlv/akivity/qemu-kvm/vl.c:1339 > #10 0x0000000000433927 in kvm_main_loop () at > /build/home/tlv/akivity/qemu-kvm/qemu-kvm.c:1590 > #11 0x000000000041a3a6 in main_loop (argc=, > argv=, envp=) > at /build/home/tlv/akivity/qemu-kvm/vl.c:1369 > #12 main (argc=, argv=, > envp=) at /build/home/tlv/akivity/qemu-kvm/vl.c:3257 > > The vmstate being migrated is "gpe". > > > >> >> +#define VMSTATE_GPE_ARRAY(_field, _state) \ >> + { \ >> + .name = (stringify(_field)), \ >> + .version_id = 0, \ >> + .num = GPE_LEN, \ >> + .info =&vmstate_info_uint16, \ >> + .size = sizeof(uint16_t), \ >> + .flags = VMS_ARRAY | VMS_POINTER, \ >> + .offset = vmstate_offset_pointer(_state, _field, uint8_t), \ >> + } >> + >> static const VMStateDescription vmstate_gpe = { >> .name = "gpe", >> .version_id = 1, >> .minimum_version_id = 1, >> .minimum_version_id_old = 1, >> .fields = (VMStateField []) { >> - VMSTATE_UINT16(sts, struct gpe_regs), >> - VMSTATE_UINT16(en, struct gpe_regs), >> + VMSTATE_GPE_ARRAY(sts, ACPIGPE), >> + VMSTATE_GPE_ARRAY(en, ACPIGPE), >> VMSTATE_END_OF_LIST() >> } >> }; > > I'm no vmstate expert, but this does look odd. Why both VMS_ARRAY and > VMS_POINTER? aren't we trying to save/restore a simple 16-bit value? Or > at least we did before this patch. That's right. the difference is, the new member type became uint8_t*. Does the following help? diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c index 96f5222..3a8fece 100644 --- a/hw/acpi_piix4.c +++ b/hw/acpi_piix4.c @@ -214,10 +214,9 @@ static int vmstate_acpi_post_load(void *opaque, int version_id) { \ .name = (stringify(_field)), \ .version_id = 0, \ - .num = GPE_LEN, \ .info = &vmstate_info_uint16, \ .size = sizeof(uint16_t), \ - .flags = VMS_ARRAY | VMS_POINTER, \ + .flags = VMS_SINGLE | VMS_POINTER, \ .offset = vmstate_offset_pointer(_state, _field, uint8_t), \ }