From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52539) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y827I-0008Hy-46 for qemu-devel@nongnu.org; Mon, 05 Jan 2015 02:30:20 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Y827E-00050p-2W for qemu-devel@nongnu.org; Mon, 05 Jan 2015 02:30:20 -0500 Received: from mx1.redhat.com ([209.132.183.28]:36341) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y827D-00050b-R1 for qemu-devel@nongnu.org; Mon, 05 Jan 2015 02:30:15 -0500 Date: Mon, 5 Jan 2015 13:00:06 +0530 From: Amit Shah Message-ID: <20150105073006.GC4609@grmbl.mre> References: <1418961447-16287-1-git-send-email-david@gibson.dropbear.id.au> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1418961447-16287-1-git-send-email-david@gibson.dropbear.id.au> Subject: Re: [Qemu-devel] [PATCHv3 0/2] Fix virtio-serial migration on bi-endian targets List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Gibson Cc: mst@redhat.com, aik@ozlabs.ru, rusty@rustcorp.com.au, qemu-devel@nongnu.org, agraf@suse.de, mdroth@us.ibm.com On (Fri) 19 Dec 2014 [14:57:25], David Gibson wrote: > On a bi-endian target, with a guest in the non-default endian mode, > attempting to migrate twice in a row with a virtio-serial device wil > cause a qemu SEGV on the second outgoing migration. > > The problem is that virtio_serial_save_device() (and other places) expect > VirtIOSerial->config to be in current guest endianness. On a fresh boot, > virtio_serial_device_realize() will initialize VirtIOSerial->config in > default endianness. It's assumed the guest OS will make its true > endianness known before the device is reset and initialized, then > vser_reset adjusts VirtIOSerial->config into the new endianness. > > But on an incoming migration, the device isn't reset (after all the guest > has a running driver as far as it's concerned), which means that > VirtIOSerial->config retains its default endianness value from > virtio_serial_device_realize(). > > On a subsequent outgoing migration, virtio_serial_save_device() attempts > to interpret VirtIOSerial->config.max_nr_ports in current endianness when > its actually in default endianness and then runs off the end of the > ports_map array in the loop immediately afterwards. > > We could fix this by adjusting VirtIOSerial->config into the correct > current endianness after an incoming migration. But in fact we > already have a host endian copy of the max number of ports readily > available, so it's simpler to just use that instead. Patch 1/2 does > that. > > Once that's done, it becomes clear that there's really no reason to > keep the guest-endian copy of the config space around persistently > (config accesses aren't a fast path, so it can be constructed when > necessary). Patch 2/2 makes that cleanup. Thanks, applied. Amit