|
From: | Marcel Apfelbaum |
Subject: | Re: [Qemu-devel] [PATCH V2 2/4] pci: reserve 64 bit MMIO range for PCI hotplug |
Date: | Wed, 18 May 2016 17:07:05 +0300 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.0 |
On 05/16/2016 05:19 PM, Igor Mammedov wrote:
On Mon, 16 May 2016 13:14:51 +0300 Marcel Apfelbaum <address@hidden> wrote:On 05/16/2016 11:24 AM, Igor Mammedov wrote:On Sun, 15 May 2016 22:23:32 +0300 Marcel Apfelbaum <address@hidden> 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 <address@hidden> --- 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 codeI 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, Marcelperhaps 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)
[Prev in Thread] | Current Thread | [Next in Thread] |