From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39379) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V3N1E-0007D2-Eu for qemu-devel@nongnu.org; Sun, 28 Jul 2013 05:12:06 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1V3N17-0005J1-EM for qemu-devel@nongnu.org; Sun, 28 Jul 2013 05:12:00 -0400 Received: from mx1.redhat.com ([209.132.183.28]:56873) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V3N17-0005Iv-3z for qemu-devel@nongnu.org; Sun, 28 Jul 2013 05:11:53 -0400 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r6S9BqTe032068 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Sun, 28 Jul 2013 05:11:52 -0400 Date: Sun, 28 Jul 2013 12:13:17 +0300 From: "Michael S. Tsirkin" Message-ID: <20130728091317.GB14505@redhat.com> References: <1374996553-21828-1-git-send-email-imammedo@redhat.com> <1374996553-21828-7-git-send-email-imammedo@redhat.com> <20130728075712.GI12087@redhat.com> <20130728102156.12db54b9@thinkpad> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20130728102156.12db54b9@thinkpad> Subject: Re: [Qemu-devel] [PATCH 6/6] pc: limit 64 bit hole to 2G by default List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Mammedov Cc: qemu-devel@nongnu.org On Sun, Jul 28, 2013 at 10:21:56AM +0200, Igor Mammedov wrote: > On Sun, 28 Jul 2013 10:57:12 +0300 > "Michael S. Tsirkin" wrote: > > > On Sun, Jul 28, 2013 at 09:29:13AM +0200, Igor Mammedov wrote: > > > It turns out that some 32 bit windows guests crash > > > if 64 bit PCI hole size is >2G. > > > Limit it to 2G for piix and q35 by default. > > > User may override default boundaries by using > > > "pci_hole64_end " property. > > > > > > Examples: > > > -global i440FX-pcihost.pci_hole64_end=0xffffffffffffffff > > > > > > -global q35-pcihost.pci_hole64_end=0xffffffffffffffff > > > > IMO that's pretty bad as user interfaces go. > > In particular if you are not careful you can make > > end < start. > > Why not set the size, and include a patch that > > let people write "1G" or "2G" for convenience? > sure as convenience why not, on top of this patches. > > > > > > Reported-by: Igor Mammedov , > > > Signed-off-by: Michael S. Tsirkin > > > Signed-off-by: Igor Mammedov > > > --- > > > hw/i386/pc.c | 59 +++++++++++++++++++++++++++++------------------ > > > hw/i386/pc_piix.c | 14 +---------- > > > hw/pci-host/piix.c | 37 +++++++++++++++++++++++++---- > > > hw/pci-host/q35.c | 32 +++++++++++++++---------- > > > include/hw/i386/pc.h | 5 ++-- > > > include/hw/pci-host/q35.h | 1 + > > > 6 files changed, 94 insertions(+), 54 deletions(-) > > > > > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > > > index b0b98a8..acaeb6c 100644 > > > --- a/hw/i386/pc.c > > > +++ b/hw/i386/pc.c > > > @@ -1003,15 +1003,28 @@ typedef struct PcRomPciInfo { > > > static void pc_fw_cfg_guest_info(PcGuestInfo *guest_info) > > > { > > > PcRomPciInfo *info; > > > + Object *pci_info; > > > + > > > if (!guest_info->has_pci_info || !guest_info->fw_cfg) { > > > return; > > > } > > > + pci_info = object_resolve_path("/machine/i440fx", NULL); > > > + if (!pci_info) { > > > + pci_info = object_resolve_path("/machine/q35", NULL); > > > + if (!pci_info) { > > > + return; > > > + } > > > + } > > > > > > So is the path /machine/i440fx? /machine/i440FX? > > /machine/i440FX-pcihost? > > There's no way to check this code is right without > > actually running it. > that drawback of dynamic lookup, > QOM paths supposed to be stable. > > > > > How about i44fx_get_pci_info so we can have this > > knowledge of paths localized in specific chipset code? > I've seen objections from afaerber about this approach, so dropped > this idea. Could we lookup TYPE_PCI_HOST? This will make pc code independent of specific machine. -- MST