From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33422) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b3280-0002zm-57 for qemu-devel@nongnu.org; Wed, 18 May 2016 10:07:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b327y-0006j7-P3 for qemu-devel@nongnu.org; Wed, 18 May 2016 10:07:12 -0400 Received: from mx1.redhat.com ([209.132.183.28]:33562) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b327y-0006j3-GR for qemu-devel@nongnu.org; Wed, 18 May 2016 10:07:10 -0400 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) (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 8BABB8AE73 for ; Wed, 18 May 2016 14:07:09 +0000 (UTC) References: <1463340214-8721-1-git-send-email-marcel@redhat.com> <1463340214-8721-3-git-send-email-marcel@redhat.com> <20160516102419.43e4b016@nial.brq.redhat.com> <57399D9B.50407@redhat.com> <20160516161933.7e0c58cc@nial.brq.redhat.com> From: Marcel Apfelbaum Message-ID: <573C7709.7000109@redhat.com> Date: Wed, 18 May 2016 17:07:05 +0300 MIME-Version: 1.0 In-Reply-To: <20160516161933.7e0c58cc@nial.brq.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: Igor Mammedov Cc: qemu-devel@nongnu.org, mst@redhat.com, lersek@redhat.com, ehabkost@redhat.com, berrange@redhat.com On 05/16/2016 05:19 PM, Igor Mammedov wrote: > On Mon, 16 May 2016 13:14:51 +0300 > Marcel Apfelbaum wrote: > >> On 05/16/2016 11:24 AM, 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 >>>> --- >>>> 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) >>>> >> >> >> Hi Igor, >> Thanks for reviewing this series. >> >>>> 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); >>> that line should break linking on other targets which don't have >>> pc_machine_get_reserved_memory_end() >>> probably for got to add stub. >>> >> >> I cross-compiled all the targets with no problem. It seems no stub is required. > ./configure && make > > LINK aarch64-softmmu/qemu-system-aarch64 > ../hw/pci/pci.o: In function `pci_bus_get_w64_range': > /home/imammedo/builds/qemu/hw/pci/pci.c:2474: undefined reference to `pc_machine_get_reserved_memory_end' > collect2: error: ld returned 1 exit status Ooops, I did configured it to cross-compile everything, but I somehow missed it. I'll take care of it. > >> >> >>>> + if (!range->begin) { >>>> + range->begin = ROUND_UP(0x100000000ULL + pcms->above_4g_mem_size, >>>> + 1ULL << 30); >>>> + } > mayby move above hunk to pc_machine_get_reserved_memory_end() > so it would always return valid value. > >>>> + range->end = 1ULL << 40; /* 40 bits physical */ >>> x86 specific in generic code >>> >> >> I am aware I put x86 code in the generic pci code, but I limited it with >> if (object_dynamic_cast(machine, TYPE_PC_MACHINE)) { >> So we have a generic rule for getting the w64 range and a specific one for PC machines. >> >> The alternative would be a w64 range per host-bridge, not bus. >> Adding a pci_bus_get_w64_range function to PCIHostBridgeClass, >> defaulting in current implementation for the base class and >> overriding it for piix/q35 looks OK to you? >> >> I thought about it, but it seemed over-engineered as opposed >> to the 'simple' if statement, however I can try it if you think is better. >> >>> ARM also has 64-bit PCI MMIO /git grep VIRT_PCIE_MMIO_HIGH/ >> >> I had a look and ARM does not use this infrastructure, it has its own abstractions, >> a memmap list. This is the other reason I selected this implementation, >> because is not really used by other targets (but it can be used in the future) > > if it's only for x86 and whatever was programmed by BIOS is ignored, > I'd drop PCI_HOST_PROP_PCI_HOLE64_*/pci_bus_get_w64_range() indirection > and just directly use pc_machine_get_reserved_memory_end() > from acpi-build.c > > in that case you won't need a stub for pc_machine_get_reserved_memory_end() > as its usage will be limited only to hw/i386 scope. > Good point, I'll try this. > Given opportunity, I'd drop PCI_HOST_PROP_PCI_HOLE* properties, > but for it we need ACK from libvirt side in case they are using it > for some reason. It seems out of the scope of this series, however I can do it on top. Thanks, Marcel > >> >> >> Thanks, >> Marcel >> >>> perhaps range should be a property of PCI bus, >>> where a board sets its own values for start/size >>> >>>> + } 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) >>> >> >