From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42380) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b32gF-00084s-PD for qemu-devel@nongnu.org; Wed, 18 May 2016 10:42:36 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b32g9-0006XU-Rh for qemu-devel@nongnu.org; Wed, 18 May 2016 10:42:34 -0400 Received: from mx1.redhat.com ([209.132.183.28]:34557) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b32g9-0006XM-J7 for qemu-devel@nongnu.org; Wed, 18 May 2016 10:42:29 -0400 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 27A3EBDEE for ; Wed, 18 May 2016 14:42:29 +0000 (UTC) Date: Wed, 18 May 2016 17:42:26 +0300 From: "Michael S. Tsirkin" Message-ID: <20160518173737-mutt-send-email-mst@redhat.com> References: <1463340214-8721-1-git-send-email-marcel@redhat.com> <1463340214-8721-3-git-send-email-marcel@redhat.com> <20160518155929.01d7b07f@nial.brq.redhat.com> <20160518170958-mutt-send-email-mst@redhat.com> <573C7839.3090802@redhat.com> <20160518163146.7554ac6f@nial.brq.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160518163146.7554ac6f@nial.brq.redhat.com> Subject: Re: [Qemu-devel] [PATCH V2 2/4] pci: reserve 64 bit MMIO range for PCI hotplug List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Mammedov Cc: Marcel Apfelbaum , qemu-devel@nongnu.org, lersek@redhat.com, ehabkost@redhat.com On Wed, May 18, 2016 at 04:31:46PM +0200, Igor Mammedov wrote: > On Wed, 18 May 2016 17:12:09 +0300 > Marcel Apfelbaum wrote: > > > On 05/18/2016 05:11 PM, Michael S. Tsirkin wrote: > > > On Wed, May 18, 2016 at 03:59:29PM +0200, Igor Mammedov wrote: > > >> On Sun, 15 May 2016 22:23:32 +0300 > > >> Marcel Apfelbaum wrote: > > >> > > >>> Using the firmware assigned MMIO ranges for 64-bit PCI window > > >>> leads to zero space for hot-plugging PCI devices over 4G. > > >>> > > >>> PC machines can use the whole CPU addressable range after > > >>> the space reserved for memory-hotplug. > > >>> > > >>> Signed-off-by: Marcel Apfelbaum > > >> that patch also has side effect of unconditionally adding > > >> QWordMemory() resource in PCI0._CRS > > >> on all machine types with QEMU generated ACPI tables. > > >> > > >> Have you tested that it won't break boot of legacy OSes > > >> (XP, WS2003, old linux with 32bit kernel)? > > > > > > It's almost sure it break it. > > > Maybe you can check _REV in _CRS to work around this for XP. > > > > I'll try it. > but only after you check if just presence of QWord would crash XP, > so in case it doesn't crash we would keep _CRS simple static > structure. > > I very vaguely recall that XP ignored QWord in PCI0._CRS, > but it was long time ago so it won't hurt to recheck. I played with different guests (32 and 64 bit) at some point. Generally, windows tends to crash when CRS resources exceed the supported limits of physical memory (sometimes with weird off by one errors, e.g. win7 32 bit seems to survive with a 36 bit pci hole even though this means the max address is 2^37-1 which it can't address). This might depend on CPU as well. Which makes me ask: why don't we fix this in BIOS? If you want it to allocate a large window, do it. > > > > Thanks, > > Marcel > > > > > > > >>> --- > > >>> hw/pci/pci.c | 16 ++++++++++++++-- > > >>> 1 file changed, 14 insertions(+), 2 deletions(-) > > >>> > > >>> diff --git a/hw/pci/pci.c b/hw/pci/pci.c > > >>> index bb605ef..44dd949 100644 > > >>> --- a/hw/pci/pci.c > > >>> +++ b/hw/pci/pci.c > > >>> @@ -41,6 +41,7 @@ > > >>> #include "hw/hotplug.h" > > >>> #include "hw/boards.h" > > >>> #include "qemu/cutils.h" > > >>> +#include "hw/i386/pc.h" > > >>> > > >>> //#define DEBUG_PCI > > >>> #ifdef DEBUG_PCI > > >>> @@ -2467,8 +2468,19 @@ static void pci_dev_get_w64(PCIBus *b, PCIDevice *dev, void *opaque) > > >>> > > >>> void pci_bus_get_w64_range(PCIBus *bus, Range *range) > > >>> { > > >>> - range->begin = range->end = 0; > > >>> - pci_for_each_device_under_bus(bus, pci_dev_get_w64, range); > > >>> + Object *machine = qdev_get_machine(); > > >>> + if (object_dynamic_cast(machine, TYPE_PC_MACHINE)) { > > >>> + PCMachineState *pcms = PC_MACHINE(machine); > > >>> + range->begin = pc_machine_get_reserved_memory_end(pcms); > > >>> + if (!range->begin) { > > >>> + range->begin = ROUND_UP(0x100000000ULL + pcms->above_4g_mem_size, > > >>> + 1ULL << 30); > > >>> + } > > >>> + range->end = 1ULL << 40; /* 40 bits physical */ > > >>> + } else { > > >>> + range->begin = range->end = 0; > > >>> + pci_for_each_device_under_bus(bus, pci_dev_get_w64, range); > > >>> + } > > >>> } > > >>> > > >>> static bool pcie_has_upstream_port(PCIDevice *dev) > >