From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57652) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VgrUQ-0007dJ-71 for qemu-devel@nongnu.org; Thu, 14 Nov 2013 02:37:28 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VgrUK-0001xx-7i for qemu-devel@nongnu.org; Thu, 14 Nov 2013 02:37:22 -0500 Received: from mx1.redhat.com ([209.132.183.28]:26832) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VgrUJ-0001xt-Tw for qemu-devel@nongnu.org; Thu, 14 Nov 2013 02:37:16 -0500 Date: Thu, 14 Nov 2013 09:40:12 +0200 From: "Michael S. Tsirkin" Message-ID: <20131114074012.GD12673@redhat.com> References: <1384264707-9947-1-git-send-email-imammedo@redhat.com> <1384264707-9947-3-git-send-email-imammedo@redhat.com> <528272BA.30402@redhat.com> <20131112221039.GA16838@redhat.com> <20131113130410.3ff7b082@thinkpad> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20131113130410.3ff7b082@thinkpad> Subject: Re: [Qemu-devel] [PATCH 2/2] pc: add 'etc/reserved-memory-end' fw_cfg interface for SeaBIOS List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Mammedov Cc: Paolo Bonzini , aliguori@amazon.com, qemu-devel@nongnu.org, afaerber@suse.de, kraxel@redhat.com On Wed, Nov 13, 2013 at 01:04:10PM +0100, Igor Mammedov wrote: > On Wed, 13 Nov 2013 00:10:39 +0200 > "Michael S. Tsirkin" wrote: > > > On Tue, Nov 12, 2013 at 07:26:02PM +0100, Paolo Bonzini wrote: > > > Il 12/11/2013 14:58, Igor Mammedov ha scritto: > > > > 'etc/reserved-memory-end' will allow QEMU to tell BIOS where PCI > > > > BARs mapping could safely start in high memory. > > > > > > > > Allowing BIOS to start mapping 64-bit PCI BARs at address where it > > > > wouldn't conflict with other mappings QEMU might place before it. > > > > > > > > That permits QEMU to reserve extra address space before > > > > 64-bit PCI hole for memory hotplug. > > > > > > I may be royally wrong, but I think the new file should only be added to > > > new machine types. Otherwise, after migrating old machine types from > > > new QEMU to old QEMU, you may end up with PCI BARs mapped outside the > > > "PCI windows" that exist until before patch 1/2 of this series. > > > > > > Does this make sense? > > > > Yes. > > Generally FW CFG must not be added/removed for a given machine types, > > otherwise guest that is migrated while reading it will > > get a corrupted result: half old and half new. > > Is it true for a file 'etc/reserved-memory-end' though? It's true for any FW CFG entry. > I've debugged SeaBIOS to learn more about it, and new->old migration with > following reboot, showed that file is not found by SeaBIOS (well since old > QEMU doesn't have it), as result SeaBIOS fallbacks to the old behavior > placing 64-PCI bars right above ram_over_4G as it was intended. > > And with 'etc/reserved-memory-end' == ram_over_4G_end as it is in this > patch, there isn't issue whatsoever. > > Looks like there is no migrations issues with files, provided SeaBIOS knows > how to deal with a missing file. Here's an example of the issue: qemu_cfg_read_entry(&count, QEMU_CFG_FILE_DIR, sizeof(count)); 1. <<<<<< count = be32_to_cpu(count); u32 e; for (e = 0; e < count; e++) { struct QemuCfgFile qfile; qemu_cfg_read(&qfile, sizeof(qfile)); qemu_romfile_add(qfile.name, be16_to_cpu(qfile.select) , 0, be32_to_cpu(qfile.size)); } 2. <<<<<<< If migration happens during qemu_cfg_read_entry before point 1, you can get a byte from old count and a byte from new, resulting in a corrupt count. If migration happens at point 2, you will get incorrect mapping between file selector and name. System will likely fail to boot. There are probably other issues. Bottom line: FW CFG is guest visible state. Migration must not change it. > -- > Regards, > Igor