On Thu, 14 Nov 2013 09:40:12 +0200 "Michael S. Tsirkin" <m...@redhat.com> wrote:
> 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" <m...@redhat.com> 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. Thanks for explanation, I'll resend this patch with machine compatibility changes. > > > -- > > Regards, > > Igor -- Regards, Igor