From 888e98c6d61b16f59867b91e41af9c878f4d1193 Mon Sep 17 00:00:00 2001 From: Marcel Apfelbaum Date: Fri, 17 Nov 2017 16:26:38 +0100 Subject: [PATCH 29/30] hw/pci-host: Fix x86 Host Bridges 64bit PCI hole RH-Author: Marcel Apfelbaum Message-id: <20171117162638.34466-1-marcel@redhat.com> Patchwork-id: 77746 O-Subject: [RHEL-7.5 qemu-kvm-rhev PATCH] hw/pci-host: Fix x86 Host Bridges 64bit PCI hole Bugzilla: 1390346 RH-Acked-by: Michael S. Tsirkin RH-Acked-by: Eduardo Habkost RH-Acked-by: Laszlo Ersek Tests: Hotplug an ivshmem device with 2G on Q35 and I440fx machines (passes only if this patch is applied) Signed-off-by: Miroslav Rezanina Conflicts: - include/hw/i386/pc.h Moved x-pci-hole64-fix compat property from PC_COMPAT_2_10 to PC_RHEL7_4_COMPAT Currently there is no MMIO range over 4G reserved for PCI hotplug. Since the 32bit PCI hole depends on the number of cold-plugged PCI devices and other factors, it is very possible is too small to hotplug PCI devices with large BARs. Fix it by reserving 2G for I4400FX chipset in order to comply with older Win32 Guest OSes and 32G for Q35 chipset. Even if the new defaults of pci-hole64-size will appear in "info qtree" also for older machines, the property was not implemented so no changes will be visible to guests. Note this is a regression since prev QEMU versions had some range reserved for 64bit PCI hotplug. Reviewed-by: Laszlo Ersek Reviewed-by: Gerd Hoffmann Signed-off-by: Marcel Apfelbaum Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin (cherry picked from commit 9fa99d2519cbf71f871e46871df12cb446dc1c3e) Signed-off-by: Marcel Apfelbaum --- hw/i386/pc.c | 22 ++++++++++++++++++++++ hw/pci-host/piix.c | 32 ++++++++++++++++++++++++++++++-- hw/pci-host/q35.c | 42 +++++++++++++++++++++++++++++++++++++++--- include/hw/i386/pc.h | 12 +++++++++++- include/hw/pci-host/q35.h | 1 + 5 files changed, 103 insertions(+), 6 deletions(-) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 83df57f..f37d60a 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1482,6 +1482,28 @@ void pc_memory_init(PCMachineState *pcms, pcms->ioapic_as = &address_space_memory; } +/* + * The 64bit pci hole starts after "above 4G RAM" and + * potentially the space reserved for memory hotplug. + */ +uint64_t pc_pci_hole64_start(void) +{ + PCMachineState *pcms = PC_MACHINE(qdev_get_machine()); + PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms); + uint64_t hole64_start = 0; + + if (pcmc->has_reserved_memory && pcms->hotplug_memory.base) { + hole64_start = pcms->hotplug_memory.base; + if (!pcmc->broken_reserved_end) { + hole64_start += memory_region_size(&pcms->hotplug_memory.mr); + } + } else { + hole64_start = 0x100000000ULL + pcms->above_4g_mem_size; + } + + return ROUND_UP(hole64_start, 1ULL << 30); +} + qemu_irq pc_allocate_cpu_irq(void) { return qemu_allocate_irq(pic_irq_request, NULL, 0); diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c index 68c3922..dc37bdf 100644 --- a/hw/pci-host/piix.c +++ b/hw/pci-host/piix.c @@ -50,6 +50,7 @@ typedef struct I440FXState { PCIHostState parent_obj; Range pci_hole; uint64_t pci_hole64_size; + bool pci_hole64_fix; uint32_t short_root_bus; } I440FXState; @@ -112,6 +113,9 @@ struct PCII440FXState { #define I440FX_PAM_SIZE 7 #define I440FX_SMRAM 0x72 +/* Keep it 2G to comply with older win32 guests */ +#define I440FX_PCI_HOST_HOLE64_SIZE_DEFAULT (1ULL << 31) + /* Older coreboot versions (4.0 and older) read a config register that doesn't * exist in real hardware, to get the RAM size from QEMU. */ @@ -238,29 +242,52 @@ static void i440fx_pcihost_get_pci_hole_end(Object *obj, Visitor *v, visit_type_uint32(v, name, &value, errp); } +/* + * The 64bit PCI hole start is set by the Guest firmware + * as the address of the first 64bit PCI MEM resource. + * If no PCI device has resources on the 64bit area, + * the 64bit PCI hole will start after "over 4G RAM" and the + * reserved space for memory hotplug if any. + */ static void i440fx_pcihost_get_pci_hole64_start(Object *obj, Visitor *v, const char *name, void *opaque, Error **errp) { PCIHostState *h = PCI_HOST_BRIDGE(obj); + I440FXState *s = I440FX_PCI_HOST_BRIDGE(obj); Range w64; uint64_t value; pci_bus_get_w64_range(h->bus, &w64); value = range_is_empty(&w64) ? 0 : range_lob(&w64); + if (!value && s->pci_hole64_fix) { + value = pc_pci_hole64_start(); + } visit_type_uint64(v, name, &value, errp); } +/* + * The 64bit PCI hole end is set by the Guest firmware + * as the address of the last 64bit PCI MEM resource. + * Then it is expanded to the PCI_HOST_PROP_PCI_HOLE64_SIZE + * that can be configured by the user. + */ static void i440fx_pcihost_get_pci_hole64_end(Object *obj, Visitor *v, const char *name, void *opaque, Error **errp) { PCIHostState *h = PCI_HOST_BRIDGE(obj); + I440FXState *s = I440FX_PCI_HOST_BRIDGE(obj); + uint64_t hole64_start = pc_pci_hole64_start(); Range w64; - uint64_t value; + uint64_t value, hole64_end; pci_bus_get_w64_range(h->bus, &w64); value = range_is_empty(&w64) ? 0 : range_upb(&w64) + 1; + hole64_end = ROUND_UP(hole64_start + s->pci_hole64_size, 1ULL << 30); + if (s->pci_hole64_fix && value < hole64_end) { + value = hole64_end; + } visit_type_uint64(v, name, &value, errp); } @@ -863,8 +890,9 @@ static const char *i440fx_pcihost_root_bus_path(PCIHostState *host_bridge, static Property i440fx_props[] = { DEFINE_PROP_SIZE(PCI_HOST_PROP_PCI_HOLE64_SIZE, I440FXState, - pci_hole64_size, DEFAULT_PCI_HOLE64_SIZE), + pci_hole64_size, I440FX_PCI_HOST_HOLE64_SIZE_DEFAULT), DEFINE_PROP_UINT32("short_root_bus", I440FXState, short_root_bus, 0), + DEFINE_PROP_BOOL("x-pci-hole64-fix", I440FXState, pci_hole64_fix, true), DEFINE_PROP_END_OF_LIST(), }; diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c index 9cd07ce..d7cc898 100644 --- a/hw/pci-host/q35.c +++ b/hw/pci-host/q35.c @@ -37,6 +37,8 @@ * Q35 host */ +#define Q35_PCI_HOST_HOLE64_SIZE_DEFAULT (1ULL << 35) + static void q35_host_realize(DeviceState *dev, Error **errp) { PCIHostState *pci = PCI_HOST_BRIDGE(dev); @@ -99,29 +101,52 @@ static void q35_host_get_pci_hole_end(Object *obj, Visitor *v, visit_type_uint32(v, name, &value, errp); } +/* + * The 64bit PCI hole start is set by the Guest firmware + * as the address of the first 64bit PCI MEM resource. + * If no PCI device has resources on the 64bit area, + * the 64bit PCI hole will start after "over 4G RAM" and the + * reserved space for memory hotplug if any. + */ static void q35_host_get_pci_hole64_start(Object *obj, Visitor *v, const char *name, void *opaque, Error **errp) { PCIHostState *h = PCI_HOST_BRIDGE(obj); + Q35PCIHost *s = Q35_HOST_DEVICE(obj); Range w64; uint64_t value; pci_bus_get_w64_range(h->bus, &w64); value = range_is_empty(&w64) ? 0 : range_lob(&w64); + if (!value && s->pci_hole64_fix) { + value = pc_pci_hole64_start(); + } visit_type_uint64(v, name, &value, errp); } +/* + * The 64bit PCI hole end is set by the Guest firmware + * as the address of the last 64bit PCI MEM resource. + * Then it is expanded to the PCI_HOST_PROP_PCI_HOLE64_SIZE + * that can be configured by the user. + */ static void q35_host_get_pci_hole64_end(Object *obj, Visitor *v, const char *name, void *opaque, Error **errp) { PCIHostState *h = PCI_HOST_BRIDGE(obj); + Q35PCIHost *s = Q35_HOST_DEVICE(obj); + uint64_t hole64_start = pc_pci_hole64_start(); Range w64; - uint64_t value; + uint64_t value, hole64_end; pci_bus_get_w64_range(h->bus, &w64); value = range_is_empty(&w64) ? 0 : range_upb(&w64) + 1; + hole64_end = ROUND_UP(hole64_start + s->mch.pci_hole64_size, 1ULL << 30); + if (s->pci_hole64_fix && value < hole64_end) { + value = hole64_end; + } visit_type_uint64(v, name, &value, errp); } @@ -133,16 +158,25 @@ static void q35_host_get_mmcfg_size(Object *obj, Visitor *v, const char *name, visit_type_uint64(v, name, &e->size, errp); } +/* + * NOTE: setting defaults for the mch.* fields in this table + * doesn't work, because mch is a separate QOM object that is + * zeroed by the object_initialize(&s->mch, ...) call inside + * q35_host_initfn(). The default values for those + * properties need to be initialized manually by + * q35_host_initfn() after the object_initialize() call. + */ static Property q35_host_props[] = { DEFINE_PROP_UINT64(PCIE_HOST_MCFG_BASE, Q35PCIHost, parent_obj.base_addr, MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT), DEFINE_PROP_SIZE(PCI_HOST_PROP_PCI_HOLE64_SIZE, Q35PCIHost, - mch.pci_hole64_size, DEFAULT_PCI_HOLE64_SIZE), + mch.pci_hole64_size, Q35_PCI_HOST_HOLE64_SIZE_DEFAULT), DEFINE_PROP_UINT32("short_root_bus", Q35PCIHost, mch.short_root_bus, 0), DEFINE_PROP_SIZE(PCI_HOST_BELOW_4G_MEM_SIZE, Q35PCIHost, mch.below_4g_mem_size, 0), DEFINE_PROP_SIZE(PCI_HOST_ABOVE_4G_MEM_SIZE, Q35PCIHost, mch.above_4g_mem_size, 0), + DEFINE_PROP_BOOL("x-pci-hole64-fix", Q35PCIHost, pci_hole64_fix, true), DEFINE_PROP_END_OF_LIST(), }; @@ -174,7 +208,9 @@ static void q35_host_initfn(Object *obj) object_property_add_child(OBJECT(s), "mch", OBJECT(&s->mch), NULL); qdev_prop_set_int32(DEVICE(&s->mch), "addr", PCI_DEVFN(0, 0)); qdev_prop_set_bit(DEVICE(&s->mch), "multifunction", false); - + /* mch's object_initialize resets the default value, set it again */ + qdev_prop_set_uint64(DEVICE(s), PCI_HOST_PROP_PCI_HOLE64_SIZE, + Q35_PCI_HOST_HOLE64_SIZE_DEFAULT); object_property_add(obj, PCI_HOST_PROP_PCI_HOLE_START, "uint32", q35_host_get_pci_hole_start, NULL, NULL, NULL, NULL); diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h index 6f65d79..7b46121 100644 --- a/include/hw/i386/pc.h +++ b/include/hw/i386/pc.h @@ -241,7 +241,6 @@ void pc_guest_info_init(PCMachineState *pcms); #define PCI_HOST_PROP_PCI_HOLE64_SIZE "pci-hole64-size" #define PCI_HOST_BELOW_4G_MEM_SIZE "below-4g-mem-size" #define PCI_HOST_ABOVE_4G_MEM_SIZE "above-4g-mem-size" -#define DEFAULT_PCI_HOLE64_SIZE (~0x0ULL) void pc_pci_as_mapping_init(Object *owner, MemoryRegion *system_memory, @@ -252,6 +251,7 @@ void pc_memory_init(PCMachineState *pcms, MemoryRegion *system_memory, MemoryRegion *rom_memory, MemoryRegion **ram_memory); +uint64_t pc_pci_hole64_start(void); qemu_irq pc_allocate_cpu_irq(void); DeviceState *pc_vga_init(ISABus *isa_bus, PCIBus *pci_bus); void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi, @@ -1015,6 +1015,16 @@ extern void igd_passthrough_isa_bridge_create(PCIBus *bus, uint16_t gpu_dev_id); .driver = "ICH9-LPC",\ .property = "__com.redhat_force-rev1-fadt",\ .value = "on",\ + },\ + { /* PC_RHEL7_4_COMPAT from PC_COMPAT_2_10 */ \ + .driver = "i440FX-pcihost",\ + .property = "x-pci-hole64-fix",\ + .value = "off",\ + },\ + { /* PC_RHEL7_4_COMPAT from PC_COMPAT_2_10 */ \ + .driver = "q35-pcihost",\ + .property = "x-pci-hole64-fix",\ + .value = "off",\ }, diff --git a/include/hw/pci-host/q35.h b/include/hw/pci-host/q35.h index 58983c0..8f4ddde 100644 --- a/include/hw/pci-host/q35.h +++ b/include/hw/pci-host/q35.h @@ -68,6 +68,7 @@ typedef struct Q35PCIHost { PCIExpressHost parent_obj; /*< public >*/ + bool pci_hole64_fix; MCHPCIState mch; } Q35PCIHost; -- 1.8.3.1