From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47986) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b32yS-0004fF-MJ for qemu-devel@nongnu.org; Wed, 18 May 2016 11:01:26 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b32yN-0002uw-M4 for qemu-devel@nongnu.org; Wed, 18 May 2016 11:01:23 -0400 Received: from mx1.redhat.com ([209.132.183.28]:49015) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b32yN-0002uq-Ax for qemu-devel@nongnu.org; Wed, 18 May 2016 11:01:19 -0400 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (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 A9A9390E50 for ; Wed, 18 May 2016 15:01:18 +0000 (UTC) References: <1463340214-8721-1-git-send-email-marcel@redhat.com> <1463340214-8721-3-git-send-email-marcel@redhat.com> <20160518171144-mutt-send-email-mst@redhat.com> <573C7F99.2070504@redhat.com> <20160518174526-mutt-send-email-mst@redhat.com> From: Marcel Apfelbaum Message-ID: <573C83BB.4000002@redhat.com> Date: Wed, 18 May 2016 18:01:15 +0300 MIME-Version: 1.0 In-Reply-To: <20160518174526-mutt-send-email-mst@redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit 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: "Michael S. Tsirkin" Cc: qemu-devel@nongnu.org, imammedo@redhat.com, lersek@redhat.com, ehabkost@redhat.com On 05/18/2016 05:57 PM, Michael S. Tsirkin wrote: > On Wed, May 18, 2016 at 05:43:37PM +0300, Marcel Apfelbaum wrote: >> On 05/18/2016 05:14 PM, Michael S. Tsirkin wrote: >>> On Sun, May 15, 2016 at 10:23:32PM +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 >>>> --- >>>> 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 >>> >>> I don't want pci to depend on PC. >>> Pls find another way to do this. >>> >> >> Igor has an idea to not call pci_dev_get_w64 and make the computations >> in the acpi code. I'll follow this idea. >> >>> >>>> @@ -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(); >>> An empty line won't hurt here after the declaration. >>> >>>> + if (object_dynamic_cast(machine, TYPE_PC_MACHINE)) { >>>> + PCMachineState *pcms = PC_MACHINE(machine); >>> >>> An empty line won't hurt here after the declaration. >>> >>>> + range->begin = pc_machine_get_reserved_memory_end(pcms); >>>> + if (!range->begin) { >>>> + range->begin = ROUND_UP(0x100000000ULL + pcms->above_4g_mem_size, >>>> + 1ULL << 30); >>> >>> Why 30? what is the logic here? >>> >> >> Will put it inside pci_dev_get_w64 and explain. >> >>>> + } >>>> + range->end = 1ULL << 40; /* 40 bits physical */ >>> >>> This comment does not help. Physical what? And why is 40 bit right? >> >> It refers to how many bits are CPU addressable. (I will add a better comment) >> cpu_x86_cpuid from target-i386/cpu.c it always returns 40 >> so hard-coding it looked like a safe choice. >> >> Thanks, >> Marcel > > Besides being ugly (we should get this from CPU code, not hard-code it) > not all guests look at CPUID unfortunately. > E.g. try win7 32 bit. > I am open to better ideas, can you please advice? Thanks, Marcel > >>>> + } else { >>>> + range->begin = range->end = 0; >>>> + pci_for_each_device_under_bus(bus, pci_dev_get_w64, range); >>> >>> When does this trigger? >>> Pls add a comment. >>> >>>> + } >>>> } >>>> >>>> static bool pcie_has_upstream_port(PCIDevice *dev) >>>> -- >>>> 2.4.3