From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46116) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y1uN6-0005vr-G8 for qemu-devel@nongnu.org; Fri, 19 Dec 2014 05:01:27 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Y1uMw-0007p6-LG for qemu-devel@nongnu.org; Fri, 19 Dec 2014 05:01:20 -0500 Received: from cantor2.suse.de ([195.135.220.15]:51656 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y1uMw-0007p2-BQ for qemu-devel@nongnu.org; Fri, 19 Dec 2014 05:01:10 -0500 Message-ID: <5493F75F.8040002@suse.de> Date: Fri, 19 Dec 2014 11:01:03 +0100 From: Alexander Graf MIME-Version: 1.0 References: <1418961447-16287-1-git-send-email-david@gibson.dropbear.id.au> In-Reply-To: <1418961447-16287-1-git-send-email-david@gibson.dropbear.id.au> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit 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 , mst@redhat.com, amit.shah@redhat.com, rusty@rustcorp.com.au Cc: aik@ozlabs.ru, qemu-devel@nongnu.org, mdroth@us.ibm.com On 19.12.14 04:57, 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. Please provide a version history of changes between versions. Also for patches you'd expect PPC people to read, CC qemu-ppc (maybe doesn't apply to this patch, but does to others) ;) Alex