OVMF downloads the ACPI linker/loader script from QEMU when the edk2 PCI
Bus driver globally signals the firmware that PCI enumeration and resource
allocation have completed. At this point QEMU regenerates the ACPI payload
in an fw_cfg read callback, and this is when the PXB's _CRS gets
populated.
Unfortunately, when this happens, the PCI_COMMAND_MEMORY bit is clear in
the root bus's command register, *unlike* under SeaBIOS. The consequences
unfold as follows:
- When build_crs() fetches dev->io_regions[i].addr, it is all-bits-one,
because pci_update_mappings() --> pci_bar_address() calculated it as
PCI_BAR_UNMAPPED, due to the PCI_COMMAND_MEMORY bit being clear.
- Consequently, the SHPC MMIO BAR (bar 0) of the bridge is not added to
the _CRS, *despite* having been programmed in PCI config space.
- Similarly, the SHPC MMIO BAR of the PXB is not removed from the main
root bus's DWordMemory descriptor.
- Guest OSes (Linux and Windows alike) notice the pre-programmed SHPC BAR
within the PXB's config space, and notice that it conflicts with the
main root bus's memory resource descriptors. Linux reports
pci 0000:04:00.0: BAR 0: can't assign mem (size 0x100)
pci 0000:04:00.0: BAR 0: trying firmware assignment [mem
0x88200000-0x882000ff 64bit]
pci 0000:04:00.0: BAR 0: [mem 0x88200000-0x882000ff 64bit] conflicts
with PCI Bus 0000:00 [mem
0x88200000-0xfebfffff]
While Windows Server 2012 R2 reports
https://technet.microsoft.com/en-us/library/cc732199%28v=ws.10%29.aspx
This device cannot find enough free resources that it can use. If you
want to use this device, you will need to disable one of the other
devices on this system. (Code 12)
This issue was apparently encountered earlier, see the "hack" in:
https://lists.nongnu.org/archive/html/qemu-devel/2015-01/msg02983.html
and the current hole-punching logic in build_crs() and build_ssdt() is
probably supposed to remedy exactly that problem -- however, for OVMF they
don't work, because at the end of the PCI enumeration and resource
allocation, which cues the ACPI linker/loader client, the command register
is clear.
It has been suggested to implement the above "hack" more cleanly and
permanently. Unfortunately, we can't just disable the SHPC bar of
TYPE_PCI_BRIDGE_DEV based on a new property, because this device model has
class-level ties to hotplug, and the SHPC bar (and the interrupt)
originate from there.
Therefore implement a more basic bridge device type, to be used by PXB,
that has no SHPC / hotplug support at all, and consequently (see commit
c008ac0c), no need for an interrupt / MSI either.
Suggested-by: Marcel Apfelbaum <mar...@redhat.com>
Cc: Marcel Apfelbaum <mar...@redhat.com>
Cc: Michael S. Tsirkin <m...@redhat.com>
Signed-off-by: Laszlo Ersek <ler...@redhat.com>
Reviewed-by: Marcel Apfelbaum <mar...@redhat.com>
---
Notes:
v4:
- unchanged
- unlike in v2 and v3, in v4 I'm formatting this as any other patch,
without "--find-copies-harder" etc. That formatting was visible (for
the exact same patch) in v3 and v4, so let's format the patch now in
its "genuine glory" :) It shows that the patch is quite small.
v3:
- unchanged, added Marcel's R-b
v2:
- Replaced the corresponding v1 patch with a new approach. Instead of
considering the PXB's disabled SHPC BAR in the PXB's _CRS (and
correspondingly, punching it out of the main _CRS), this patch creates
a separate device model that lacks the SHPC functionality completely.
I already know from IRC that Michael doesn't like this, but that's
okay: I'm formatting this with "-C35% --find-copies-harder", so that
the differences are obvious against the clone origin, and Michael can
pinpoint the locations where and how I should use a new device
property instead, in the original device model.
(That said, I did test this code, with both Windows and Linux, and
compared the dumped / decompiled SSDTs too.)
hw/pci-bridge/Makefile.objs | 1 +
include/hw/pci/pci.h | 1 +
hw/pci-bridge/pci_basic_bridge_dev.c | 114 +++++++++++++++++++++++++++++++++++
hw/pci-bridge/pci_expander_bridge.c | 2 +-
4 files changed, 117 insertions(+), 1 deletion(-)
create mode 100644 hw/pci-bridge/pci_basic_bridge_dev.c
diff --git a/hw/pci-bridge/Makefile.objs b/hw/pci-bridge/Makefile.objs
index f2adfe3..bcfe753 100644
--- a/hw/pci-bridge/Makefile.objs
+++ b/hw/pci-bridge/Makefile.objs
@@ -1,4 +1,5 @@
common-obj-y += pci_bridge_dev.o
+common-obj-y += pci_basic_bridge_dev.o
common-obj-y += pci_expander_bridge.o
common-obj-$(CONFIG_XIO3130) += xio3130_upstream.o xio3130_downstream.o
common-obj-$(CONFIG_IOH3420) += ioh3420.o
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index d44bc84..619c31a 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -92,6 +92,7 @@
#define PCI_DEVICE_ID_REDHAT_SDHCI 0x0007
#define PCI_DEVICE_ID_REDHAT_PCIE_HOST 0x0008
#define PCI_DEVICE_ID_REDHAT_PXB 0x0009
+#define PCI_DEVICE_ID_REDHAT_BASIC_BRIDGE 0x000A
#define PCI_DEVICE_ID_REDHAT_QXL 0x0100
#define FMT_PCIBUS PRIx64
diff --git a/hw/pci-bridge/pci_basic_bridge_dev.c
b/hw/pci-bridge/pci_basic_bridge_dev.c
new file mode 100644
index 0000000..d8bfb6c
--- /dev/null
+++ b/hw/pci-bridge/pci_basic_bridge_dev.c
@@ -0,0 +1,114 @@
+/*
+ * PCI Bridge Device without interrupt and SHPC / hotplug support.
+ *
+ * Copyright (c) 2011, 2015 Red Hat Inc.
+ * Authors: Michael S. Tsirkin <m...@redhat.com>
+ * Laszlo Ersek <ler...@redhat.com>
+ *
+ *
http://www.pcisig.com/specifications/conventional/pci_to_pci_bridge_architecture/
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "hw/pci/pci_bridge.h"
+#include "hw/pci/pci_ids.h"
+#include "hw/pci/slotid_cap.h"
+#include "exec/memory.h"
+#include "hw/pci/pci_bus.h"
+
+#define TYPE_PCI_BASIC_BRIDGE_DEV "pci-basic-bridge"
+#define PCI_BASIC_BRIDGE_DEV(obj) \
+ OBJECT_CHECK(PCIBasicBridgeDev, (obj), TYPE_PCI_BASIC_BRIDGE_DEV)
+
+struct PCIBasicBridgeDev {
+ /*< private >*/
+ PCIBridge parent_obj;
+ /*< public >*/
+
+ uint8_t chassis_nr;
+};
+typedef struct PCIBasicBridgeDev PCIBasicBridgeDev;
+
+static int pci_basic_bridge_dev_initfn(PCIDevice *dev)
+{
+ PCIBasicBridgeDev *bridge_dev = PCI_BASIC_BRIDGE_DEV(dev);
+ int err;
+
+ err = pci_bridge_initfn(dev, TYPE_PCI_BUS);
+ if (err) {
+ goto bridge_error;
+ }
+ err = slotid_cap_init(dev, 0, bridge_dev->chassis_nr, 0);
+ if (err) {
+ goto slotid_error;
+ }
+ return 0;
+slotid_error:
+ pci_bridge_exitfn(dev);
+bridge_error:
+ return err;
+}
+
+static void pci_basic_bridge_dev_exitfn(PCIDevice *dev)
+{
+ slotid_cap_cleanup(dev);
+ pci_bridge_exitfn(dev);
+}
+
+static Property pci_basic_bridge_dev_properties[] = {
+ /* Note: 0 is not a legal chassis number. */
+ DEFINE_PROP_UINT8("chassis_nr", PCIBasicBridgeDev, chassis_nr, 0),
+ DEFINE_PROP_END_OF_LIST(),
+};
+
+static const VMStateDescription pci_basic_bridge_dev_vmstate = {
+ .name = "pci_basic_bridge",
+ .fields = (VMStateField[]) {
+ VMSTATE_PCI_DEVICE(parent_obj, PCIBridge),
+ VMSTATE_END_OF_LIST()
+ }
+};
+
+static void pci_basic_bridge_dev_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+ PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+
+ k->init = pci_basic_bridge_dev_initfn;
+ k->exit = pci_basic_bridge_dev_exitfn;
+ k->config_write = pci_bridge_write_config;
+ k->vendor_id = PCI_VENDOR_ID_REDHAT;
+ k->device_id = PCI_DEVICE_ID_REDHAT_BASIC_BRIDGE;
+ k->class_id = PCI_CLASS_BRIDGE_PCI;
+ k->is_bridge = 1,
+ dc->desc = "Basic PCI Bridge (no interrupt, no hotplug)";
+ dc->reset = pci_bridge_reset;
+ dc->props = pci_basic_bridge_dev_properties;
+ dc->vmsd = &pci_basic_bridge_dev_vmstate;
+ set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
+}
+
+static const TypeInfo pci_basic_bridge_dev_info = {
+ .name = TYPE_PCI_BASIC_BRIDGE_DEV,
+ .parent = TYPE_PCI_BRIDGE,
+ .instance_size = sizeof(PCIBasicBridgeDev),
+ .class_init = pci_basic_bridge_dev_class_init,
+};
+
+static void pci_basic_bridge_dev_register(void)
+{
+ type_register_static(&pci_basic_bridge_dev_info);
+}
+
+type_init(pci_basic_bridge_dev_register);
diff --git a/hw/pci-bridge/pci_expander_bridge.c
b/hw/pci-bridge/pci_expander_bridge.c
index ec2bb45..c7a085d 100644
--- a/hw/pci-bridge/pci_expander_bridge.c
+++ b/hw/pci-bridge/pci_expander_bridge.c
@@ -173,7 +173,7 @@ static int pxb_dev_initfn(PCIDevice *dev)
bus->address_space_io = dev->bus->address_space_io;
bus->map_irq = pxb_map_irq_fn;
- bds = qdev_create(BUS(bus), "pci-bridge");
+ bds = qdev_create(BUS(bus), "pci-basic-bridge");
bds->id = dev_name;
qdev_prop_set_uint8(bds, "chassis_nr", pxb->bus_nr);
--
1.8.3.1
