* [Qemu-devel] [PATCH v2 0/4] ARM: Add support for a generic PCI Express host bridge
@ 2015-01-21 16:18 Alexander Graf
2015-01-21 16:18 ` [Qemu-devel] [PATCH v2 1/4] pci: Split pcie_host_mmcfg_map() Alexander Graf
` (3 more replies)
0 siblings, 4 replies; 24+ messages in thread
From: Alexander Graf @ 2015-01-21 16:18 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, ard.biesheuvel, rob.herring, mst, claudio.fontana,
stuart.yoder, a.rigo
Linux implements a nice binding to describe a "generic" PCI Express host bridge
using only device tree.
This patch set adds enough emulation logic to expose the parts that are
"generic" as a simple sysbus device and maps it into ARM's virt machine.
With this patch set, we can finally spawn PCI devices on ARM VMs. I was able
to have a fully DRM enabled virtual machine with VGA, e1000 and XHCI (for
keyboard and mouse) up and working.
It's only a small step for QEMU, but a big step for ARM VM's usability.
v1 -> v2:
- Add documentation links
- Remove mmio_window_size
- Add define for pci range types
- Use 4 PCI INTX IRQ lines
Alexander Graf (4):
pci: Split pcie_host_mmcfg_map()
pci: Add generic PCIe host bridge
arm: Add PCIe host bridge in virt machine
pci: Move PCI VGA to pci.mak
default-configs/alpha-softmmu.mak | 2 -
default-configs/arm-softmmu.mak | 2 +
default-configs/i386-softmmu.mak | 2 -
default-configs/mips-softmmu.mak | 2 -
default-configs/mips64-softmmu.mak | 2 -
default-configs/mips64el-softmmu.mak | 2 -
default-configs/mipsel-softmmu.mak | 2 -
default-configs/pci.mak | 2 +
default-configs/ppc-softmmu.mak | 2 -
default-configs/ppc64-softmmu.mak | 2 -
default-configs/ppcemb-softmmu.mak | 2 -
default-configs/sparc64-softmmu.mak | 2 -
default-configs/x86_64-softmmu.mak | 2 -
hw/arm/virt.c | 112 +++++++++++++++++++++++--
hw/pci-host/Makefile.objs | 1 +
hw/pci-host/gpex.c | 153 +++++++++++++++++++++++++++++++++++
hw/pci/pcie_host.c | 9 ++-
include/hw/pci-host/gpex.h | 54 +++++++++++++
include/hw/pci/pcie_host.h | 1 +
include/sysemu/device_tree.h | 9 +++
20 files changed, 336 insertions(+), 29 deletions(-)
create mode 100644 hw/pci-host/gpex.c
create mode 100644 include/hw/pci-host/gpex.h
--
1.7.12.4
^ permalink raw reply [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH v2 1/4] pci: Split pcie_host_mmcfg_map()
2015-01-21 16:18 [Qemu-devel] [PATCH v2 0/4] ARM: Add support for a generic PCI Express host bridge Alexander Graf
@ 2015-01-21 16:18 ` Alexander Graf
2015-01-27 13:55 ` Peter Maydell
2015-01-21 16:18 ` [Qemu-devel] [PATCH v2 2/4] pci: Add generic PCIe host bridge Alexander Graf
` (2 subsequent siblings)
3 siblings, 1 reply; 24+ messages in thread
From: Alexander Graf @ 2015-01-21 16:18 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, ard.biesheuvel, rob.herring, mst, claudio.fontana,
stuart.yoder, a.rigo
The mmcfg space is a memory region that allows access to PCI config space
in the PCIe world. To maintain abstraction layers, I would like to expose
the mmcfg space as a sysbus mmio region rather than have it mapped straight
into the system's memory address space though.
So this patch splits the initialization of the mmcfg space from the actual
mapping, allowing us to only have an mmfg memory region without the map.
Signed-off-by: Alexander Graf <agraf@suse.de>
Reviewed-by: Claudio Fontana <claudio.fontana@huawei.com>
Tested-by: Claudio Fontana <claudio.fontana@huawei.com>
---
hw/pci/pcie_host.c | 9 +++++++--
include/hw/pci/pcie_host.h | 1 +
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/hw/pci/pcie_host.c b/hw/pci/pcie_host.c
index 3db038f..dfb4a2b 100644
--- a/hw/pci/pcie_host.c
+++ b/hw/pci/pcie_host.c
@@ -98,8 +98,7 @@ void pcie_host_mmcfg_unmap(PCIExpressHost *e)
}
}
-void pcie_host_mmcfg_map(PCIExpressHost *e, hwaddr addr,
- uint32_t size)
+void pcie_host_mmcfg_init(PCIExpressHost *e, uint32_t size)
{
assert(!(size & (size - 1))); /* power of 2 */
assert(size >= PCIE_MMCFG_SIZE_MIN);
@@ -107,6 +106,12 @@ void pcie_host_mmcfg_map(PCIExpressHost *e, hwaddr addr,
e->size = size;
memory_region_init_io(&e->mmio, OBJECT(e), &pcie_mmcfg_ops, e,
"pcie-mmcfg", e->size);
+}
+
+void pcie_host_mmcfg_map(PCIExpressHost *e, hwaddr addr,
+ uint32_t size)
+{
+ pcie_host_mmcfg_init(e, size);
e->base_addr = addr;
memory_region_add_subregion(get_system_memory(), e->base_addr, &e->mmio);
}
diff --git a/include/hw/pci/pcie_host.h b/include/hw/pci/pcie_host.h
index ff44ef6..4d23c80 100644
--- a/include/hw/pci/pcie_host.h
+++ b/include/hw/pci/pcie_host.h
@@ -50,6 +50,7 @@ struct PCIExpressHost {
};
void pcie_host_mmcfg_unmap(PCIExpressHost *e);
+void pcie_host_mmcfg_init(PCIExpressHost *e, uint32_t size);
void pcie_host_mmcfg_map(PCIExpressHost *e, hwaddr addr, uint32_t size);
void pcie_host_mmcfg_update(PCIExpressHost *e,
int enable,
--
1.7.12.4
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH v2 2/4] pci: Add generic PCIe host bridge
2015-01-21 16:18 [Qemu-devel] [PATCH v2 0/4] ARM: Add support for a generic PCI Express host bridge Alexander Graf
2015-01-21 16:18 ` [Qemu-devel] [PATCH v2 1/4] pci: Split pcie_host_mmcfg_map() Alexander Graf
@ 2015-01-21 16:18 ` Alexander Graf
2015-01-22 16:32 ` B02008
2015-01-27 15:31 ` Peter Maydell
2015-01-21 16:18 ` [Qemu-devel] [PATCH v2 3/4] arm: Add PCIe host bridge in virt machine Alexander Graf
2015-01-21 16:18 ` [Qemu-devel] [PATCH v2 4/4] pci: Move PCI VGA to pci.mak Alexander Graf
3 siblings, 2 replies; 24+ messages in thread
From: Alexander Graf @ 2015-01-21 16:18 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, ard.biesheuvel, rob.herring, mst, claudio.fontana,
stuart.yoder, a.rigo
With simple exposure of MMFG, ioport window, mmio window and an IRQ line we
can successfully create a workable PCIe host bridge that can be mapped anywhere
and only needs to get described to the OS using whatever means it likes.
This patch implements such a "generic" host bridge. It only supports a single
legacy IRQ line so far. MSIs need to be handled external to the host bridge.
This device is particularly useful for the "pci-host-ecam-generic" driver in
Linux.
Signed-off-by: Alexander Graf <agraf@suse.de>
Reviewed-by: Claudio Fontana <claudio.fontana@huawei.com>
Tested-by: Claudio Fontana <claudio.fontana@huawei.com>
---
v1 -> v2:
- Add documentation links
- Remove mmio_window_size
- Expose 4 INTX IRQs
---
hw/pci-host/Makefile.objs | 1 +
hw/pci-host/gpex.c | 153 +++++++++++++++++++++++++++++++++++++++++++++
include/hw/pci-host/gpex.h | 54 ++++++++++++++++
3 files changed, 208 insertions(+)
create mode 100644 hw/pci-host/gpex.c
create mode 100644 include/hw/pci-host/gpex.h
diff --git a/hw/pci-host/Makefile.objs b/hw/pci-host/Makefile.objs
index bb65f9c..45f1f0e 100644
--- a/hw/pci-host/Makefile.objs
+++ b/hw/pci-host/Makefile.objs
@@ -15,3 +15,4 @@ common-obj-$(CONFIG_PCI_APB) += apb.o
common-obj-$(CONFIG_FULONG) += bonito.o
common-obj-$(CONFIG_PCI_PIIX) += piix.o
common-obj-$(CONFIG_PCI_Q35) += q35.o
+common-obj-$(CONFIG_PCI_GENERIC) += gpex.o
diff --git a/hw/pci-host/gpex.c b/hw/pci-host/gpex.c
new file mode 100644
index 0000000..11aaf5b
--- /dev/null
+++ b/hw/pci-host/gpex.c
@@ -0,0 +1,153 @@
+/*
+ * QEMU Generic PCI Express Bridge Emulation
+ *
+ * Copyright (C) 2015 Alexander Graf <agraf@suse.de>
+ *
+ * Code loosely based on q35.c.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ *
+ * Check out these documents for more information on the device:
+ *
+ * http://www.kernel.org/doc/Documentation/devicetree/bindings/pci/host-generic-pci.txt
+ * http://www.firmware.org/1275/practice/imap/imap0_9d.pdf
+ */
+#include "hw/hw.h"
+#include "hw/pci-host/gpex.h"
+
+/****************************************************************************
+ * GPEX host
+ */
+
+static void gpex_set_irq(void *opaque, int irq_num, int level)
+{
+ GPEXHost *s = opaque;
+
+ qemu_set_irq(s->irq[irq_num], level);
+}
+
+static void gpex_host_realize(DeviceState *dev, Error **errp)
+{
+ PCIHostState *pci = PCI_HOST_BRIDGE(dev);
+ GPEXHost *s = GPEX_HOST(dev);
+ SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+ PCIExpressHost *pex = PCIE_HOST_BRIDGE(dev);
+ int i;
+
+ pcie_host_mmcfg_init(pex, PCIE_MMCFG_SIZE_MIN);
+ memory_region_init(&s->io_mmio, OBJECT(s), "gpex_mmio", UINT64_MAX);
+ memory_region_init(&s->io_ioport, OBJECT(s), "gpex_ioport", 64 * 1024);
+
+ sysbus_init_mmio(sbd, &pex->mmio);
+ sysbus_init_mmio(sbd, &s->io_mmio);
+ sysbus_init_mmio(sbd, &s->io_ioport);
+ for (i = 0; i < 4; i++) {
+ sysbus_init_irq(sbd, &s->irq[i]);
+ }
+
+ pci->bus = pci_register_bus(dev, "pcie.0", gpex_set_irq,
+ pci_swizzle_map_irq_fn, s, &s->io_mmio,
+ &s->io_ioport, 0, 4, TYPE_PCIE_BUS);
+
+ qdev_set_parent_bus(DEVICE(&s->gpex_root), BUS(pci->bus));
+ qdev_init_nofail(DEVICE(&s->gpex_root));
+}
+
+static const char *gpex_host_root_bus_path(PCIHostState *host_bridge,
+ PCIBus *rootbus)
+{
+ return "0000:00";
+}
+
+static void gpex_host_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+ PCIHostBridgeClass *hc = PCI_HOST_BRIDGE_CLASS(klass);
+
+ hc->root_bus_path = gpex_host_root_bus_path;
+ dc->realize = gpex_host_realize;
+ set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
+ dc->fw_name = "pci";
+}
+
+static void gpex_host_initfn(Object *obj)
+{
+ GPEXHost *s = GPEX_HOST(obj);
+
+ object_initialize(&s->gpex_root, sizeof(s->gpex_root), TYPE_GPEX_ROOT_DEVICE);
+ object_property_add_child(OBJECT(s), "gpex_root", OBJECT(&s->gpex_root), NULL);
+ qdev_prop_set_uint32(DEVICE(&s->gpex_root), "addr", PCI_DEVFN(0, 0));
+ qdev_prop_set_bit(DEVICE(&s->gpex_root), "multifunction", false);
+}
+
+static const TypeInfo gpex_host_info = {
+ .name = TYPE_GPEX_HOST,
+ .parent = TYPE_PCIE_HOST_BRIDGE,
+ .instance_size = sizeof(GPEXHost),
+ .instance_init = gpex_host_initfn,
+ .class_init = gpex_host_class_init,
+};
+
+/****************************************************************************
+ * GPEX Root D0:F0
+ */
+
+static const VMStateDescription vmstate_gpex_root = {
+ .name = "gpex_root",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .fields = (VMStateField[]) {
+ VMSTATE_PCI_DEVICE(parent_obj, GPEXRootState),
+ VMSTATE_END_OF_LIST()
+ }
+};
+
+static void gpex_root_class_init(ObjectClass *klass, void *data)
+{
+ PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+ DeviceClass *dc = DEVICE_CLASS(klass);
+
+ set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
+ dc->desc = "Host bridge";
+ dc->vmsd = &vmstate_gpex_root;
+ k->vendor_id = PCI_VENDOR_ID_REDHAT;
+ k->device_id = PCI_DEVICE_ID_REDHAT_BRIDGE;
+ k->revision = 0;
+ k->class_id = PCI_CLASS_BRIDGE_HOST;
+ /*
+ * PCI-facing part of the host bridge, not usable without the
+ * host-facing part, which can't be device_add'ed, yet.
+ */
+ dc->cannot_instantiate_with_device_add_yet = true;
+}
+
+static const TypeInfo gpex_root_info = {
+ .name = TYPE_GPEX_ROOT_DEVICE,
+ .parent = TYPE_PCI_DEVICE,
+ .instance_size = sizeof(GPEXRootState),
+ .class_init = gpex_root_class_init,
+};
+
+static void gpex_register(void)
+{
+ type_register_static(&gpex_root_info);
+ type_register_static(&gpex_host_info);
+}
+
+type_init(gpex_register);
diff --git a/include/hw/pci-host/gpex.h b/include/hw/pci-host/gpex.h
new file mode 100644
index 0000000..b465667
--- /dev/null
+++ b/include/hw/pci-host/gpex.h
@@ -0,0 +1,54 @@
+/*
+ * gpex.h
+ *
+ * Copyright (C) 2015 Alexander Graf <agraf@suse.de>
+ *
+ * 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 Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>
+ */
+
+#ifndef HW_GPEX_H
+#define HW_GPEX_H
+
+#include "hw/hw.h"
+#include "hw/sysbus.h"
+#include "hw/pci/pci.h"
+#include "hw/pci/pcie_host.h"
+
+#define TYPE_GPEX_HOST "gpex-pcihost"
+#define GPEX_HOST(obj) \
+ OBJECT_CHECK(GPEXHost, (obj), TYPE_GPEX_HOST)
+
+#define TYPE_GPEX_ROOT_DEVICE "gpex-root"
+#define MCH_PCI_DEVICE(obj) \
+ OBJECT_CHECK(GPEXRootState, (obj), TYPE_GPEX_ROOT_DEVICE)
+
+typedef struct GPEXRootState {
+ /*< private >*/
+ PCIDevice parent_obj;
+ /*< public >*/
+} GPEXRootState;
+
+typedef struct GPEXHost {
+ /*< private >*/
+ PCIExpressHost parent_obj;
+ /*< public >*/
+
+ GPEXRootState gpex_root;
+
+ MemoryRegion io_ioport;
+ MemoryRegion io_mmio;
+ qemu_irq irq[4];
+} GPEXHost;
+
+#endif /* HW_GPEX_H */
--
1.7.12.4
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH v2 3/4] arm: Add PCIe host bridge in virt machine
2015-01-21 16:18 [Qemu-devel] [PATCH v2 0/4] ARM: Add support for a generic PCI Express host bridge Alexander Graf
2015-01-21 16:18 ` [Qemu-devel] [PATCH v2 1/4] pci: Split pcie_host_mmcfg_map() Alexander Graf
2015-01-21 16:18 ` [Qemu-devel] [PATCH v2 2/4] pci: Add generic PCIe host bridge Alexander Graf
@ 2015-01-21 16:18 ` Alexander Graf
2015-01-22 15:28 ` Claudio Fontana
2015-01-27 16:52 ` Peter Maydell
2015-01-21 16:18 ` [Qemu-devel] [PATCH v2 4/4] pci: Move PCI VGA to pci.mak Alexander Graf
3 siblings, 2 replies; 24+ messages in thread
From: Alexander Graf @ 2015-01-21 16:18 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, ard.biesheuvel, rob.herring, mst, claudio.fontana,
stuart.yoder, a.rigo
Now that we have a working "generic" PCIe host bridge driver, we can plug
it into ARMs virt machine to always have PCIe available to normal ARM VMs.
I've successfully managed to expose a Bochs VGA device, XHCI and an e1000
into an AArch64 VM with this and they all lived happily ever after.
Signed-off-by: Alexander Graf <agraf@suse.de>
Tested-by: Claudio Fontana <claudio.fontana@huawei.com>
---
Linux 3.19 only supports the generic PCIe host bridge driver for 32bit ARM
systems. If you want to use it with AArch64 guests, please apply the following
patch or wait until upstream cleaned up the code properly:
http://csgraf.de/agraf/pci/pci-3.19.patch
v1 -> v2:
- Add define for pci range types
- Remove mmio_window_size
- Use 4 PCI INTX IRQ lines
---
default-configs/arm-softmmu.mak | 2 +
hw/arm/virt.c | 112 ++++++++++++++++++++++++++++++++++++++--
include/sysemu/device_tree.h | 9 ++++
3 files changed, 118 insertions(+), 5 deletions(-)
diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index f3513fa..7671ee2 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -82,6 +82,8 @@ CONFIG_ZYNQ=y
CONFIG_VERSATILE_PCI=y
CONFIG_VERSATILE_I2C=y
+CONFIG_PCI_GENERIC=y
+
CONFIG_SDHCI=y
CONFIG_INTEGRATOR_DEBUG=y
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 2353440..a727b4e 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -42,6 +42,7 @@
#include "exec/address-spaces.h"
#include "qemu/bitops.h"
#include "qemu/error-report.h"
+#include "hw/pci-host/gpex.h"
#define NUM_VIRTIO_TRANSPORTS 32
@@ -69,6 +70,7 @@ enum {
VIRT_MMIO,
VIRT_RTC,
VIRT_FW_CFG,
+ VIRT_PCIE,
};
typedef struct MemMapEntry {
@@ -129,13 +131,14 @@ static const MemMapEntry a15memmap[] = {
[VIRT_FW_CFG] = { 0x09020000, 0x0000000a },
[VIRT_MMIO] = { 0x0a000000, 0x00000200 },
/* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */
- /* 0x10000000 .. 0x40000000 reserved for PCI */
+ [VIRT_PCIE] = { 0x10000000, 0x30000000 },
[VIRT_MEM] = { 0x40000000, 30ULL * 1024 * 1024 * 1024 },
};
static const int a15irqmap[] = {
[VIRT_UART] = 1,
[VIRT_RTC] = 2,
+ [VIRT_PCIE] = 3, /* ... to 6 */
[VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */
};
@@ -312,7 +315,7 @@ static void fdt_add_cpu_nodes(const VirtBoardInfo *vbi)
}
}
-static void fdt_add_gic_node(const VirtBoardInfo *vbi)
+static uint32_t fdt_add_gic_node(const VirtBoardInfo *vbi)
{
uint32_t gic_phandle;
@@ -331,9 +334,11 @@ static void fdt_add_gic_node(const VirtBoardInfo *vbi)
2, vbi->memmap[VIRT_GIC_CPU].base,
2, vbi->memmap[VIRT_GIC_CPU].size);
qemu_fdt_setprop_cell(vbi->fdt, "/intc", "phandle", gic_phandle);
+
+ return gic_phandle;
}
-static void create_gic(const VirtBoardInfo *vbi, qemu_irq *pic)
+static uint32_t create_gic(const VirtBoardInfo *vbi, qemu_irq *pic)
{
/* We create a standalone GIC v2 */
DeviceState *gicdev;
@@ -380,7 +385,7 @@ static void create_gic(const VirtBoardInfo *vbi, qemu_irq *pic)
pic[i] = qdev_get_gpio_in(gicdev, i);
}
- fdt_add_gic_node(vbi);
+ return fdt_add_gic_node(vbi);
}
static void create_uart(const VirtBoardInfo *vbi, qemu_irq *pic)
@@ -556,6 +561,100 @@ static void create_fw_cfg(const VirtBoardInfo *vbi)
g_free(nodename);
}
+static void create_pcie_irq_map(const VirtBoardInfo *vbi, uint32_t gic_phandle,
+ int first_irq, const char *nodename)
+{
+ int devfn, pin;
+ uint32_t full_irq_map[4 * 4 * 8] = { 0 };
+ uint32_t *irq_map = full_irq_map;
+
+ for (devfn = 0; devfn <= 0x18; devfn+=0x8) {
+ for (pin = 0; pin < 4; pin++) {
+ int irq_type = GIC_FDT_IRQ_TYPE_SPI;
+ int irq_nr = first_irq + ((pin + PCI_SLOT(devfn)) % PCI_NUM_PINS);
+ int irq_level = GIC_FDT_IRQ_FLAGS_LEVEL_HI;
+ int i;
+
+ uint32_t map[] = {
+ devfn << 8, 0, 0, /* devfn */
+ pin + 1, /* PCI pin */
+ gic_phandle, irq_type, irq_nr, irq_level }; /* GIC irq */
+
+ /* Convert map to big endian */
+ for (i = 0; i < 8; i++) {
+ irq_map[i] = cpu_to_be32(map[i]);
+ }
+ irq_map += 8;
+ }
+ }
+
+ qemu_fdt_setprop(vbi->fdt, nodename, "interrupt-map",
+ full_irq_map, sizeof(full_irq_map));
+}
+
+static void create_pcie(const VirtBoardInfo *vbi, qemu_irq *pic,
+ uint32_t gic_phandle)
+{
+ hwaddr base = vbi->memmap[VIRT_PCIE].base;
+ hwaddr size = vbi->memmap[VIRT_PCIE].size;
+ hwaddr size_ioport = 64 * 1024;
+ hwaddr size_ecam = PCIE_MMCFG_SIZE_MIN;
+ hwaddr size_mmio = size - size_ecam - size_ioport;
+ hwaddr base_mmio = base;
+ hwaddr base_ioport = base_mmio + size_mmio;
+ hwaddr base_ecam = base_ioport + size_ioport;
+ int irq = vbi->irqmap[VIRT_PCIE];
+ MemoryRegion *mmio_alias;
+ MemoryRegion *mmio_reg;
+ DeviceState *dev;
+ char *nodename;
+ int i;
+
+ dev = qdev_create(NULL, TYPE_GPEX_HOST);
+ qdev_init_nofail(dev);
+
+ sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base_ecam);
+ sysbus_mmio_map(SYS_BUS_DEVICE(dev), 2, base_ioport);
+
+ /* Map the MMIO window at the same spot in bus and cpu layouts */
+ mmio_alias = g_new0(MemoryRegion, 1);
+ mmio_reg = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 1);
+ memory_region_init_alias(mmio_alias, OBJECT(dev), "pcie-mmio",
+ mmio_reg, base_mmio, size_mmio);
+ memory_region_add_subregion(get_system_memory(), base_mmio, mmio_alias);
+
+ for (i = 0; i < 4; i++) {
+ sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, pic[irq + i]);
+ }
+
+ nodename = g_strdup_printf("/pcie@%" PRIx64, base);
+ qemu_fdt_add_subnode(vbi->fdt, nodename);
+ qemu_fdt_setprop_string(vbi->fdt, nodename,
+ "compatible", "pci-host-ecam-generic");
+ qemu_fdt_setprop_string(vbi->fdt, nodename, "device_type", "pci");
+ qemu_fdt_setprop_cell(vbi->fdt, nodename, "#address-cells", 3);
+ qemu_fdt_setprop_cell(vbi->fdt, nodename, "#size-cells", 2);
+ qemu_fdt_setprop_cells(vbi->fdt, nodename, "bus-range", 0, 1);
+
+ qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "reg",
+ 2, base_ecam, 2, size_ecam);
+ qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "ranges",
+ 1, FDT_PCI_RANGE_IOPORT, 2, 0,
+ 2, base_ioport, 2, size_ioport,
+
+ 1, FDT_PCI_RANGE_MMIO, 2, base_mmio,
+ 2, base_mmio, 2, size_mmio);
+
+ qemu_fdt_setprop_cell(vbi->fdt, nodename, "#interrupt-cells", 1);
+ create_pcie_irq_map(vbi, gic_phandle, irq, nodename);
+
+ qemu_fdt_setprop_cells(vbi->fdt, nodename, "interrupt-map-mask",
+ 0x1800, 0, 0, /* devfn (PCI_SLOT(3)) */
+ 0x7 /* PCI irq */);
+
+ g_free(nodename);
+}
+
static void *machvirt_dtb(const struct arm_boot_info *binfo, int *fdt_size)
{
const VirtBoardInfo *board = (const VirtBoardInfo *)binfo;
@@ -573,6 +672,7 @@ static void machvirt_init(MachineState *machine)
MemoryRegion *ram = g_new(MemoryRegion, 1);
const char *cpu_model = machine->cpu_model;
VirtBoardInfo *vbi;
+ uint32_t gic_phandle;
if (!cpu_model) {
cpu_model = "cortex-a15";
@@ -634,12 +734,14 @@ static void machvirt_init(MachineState *machine)
create_flash(vbi);
- create_gic(vbi, pic);
+ gic_phandle = create_gic(vbi, pic);
create_uart(vbi, pic);
create_rtc(vbi, pic);
+ create_pcie(vbi, pic, gic_phandle);
+
/* Create mmio transports, so the user can create virtio backends
* (which will be automatically plugged in to the transports). If
* no backend is created the transport will just sit harmlessly idle.
diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h
index 899f05c..979169b 100644
--- a/include/sysemu/device_tree.h
+++ b/include/sysemu/device_tree.h
@@ -110,4 +110,13 @@ int qemu_fdt_setprop_sized_cells_from_array(void *fdt,
qdt_tmp); \
})
+#define FDT_PCI_RANGE_RELOCATABLE 0x80000000
+#define FDT_PCI_RANGE_PREFETCHABLE 0x40000000
+#define FDT_PCI_RANGE_ALIASED 0x20000000
+#define FDT_PCI_RANGE_TYPE 0x03000000
+#define FDT_PCI_RANGE_MMIO_64BIT 0x03000000
+#define FDT_PCI_RANGE_MMIO 0x02000000
+#define FDT_PCI_RANGE_IOPORT 0x01000000
+#define FDT_PCI_RANGE_CONFIG 0x00000000
+
#endif /* __DEVICE_TREE_H__ */
--
1.7.12.4
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH v2 4/4] pci: Move PCI VGA to pci.mak
2015-01-21 16:18 [Qemu-devel] [PATCH v2 0/4] ARM: Add support for a generic PCI Express host bridge Alexander Graf
` (2 preceding siblings ...)
2015-01-21 16:18 ` [Qemu-devel] [PATCH v2 3/4] arm: Add PCIe host bridge in virt machine Alexander Graf
@ 2015-01-21 16:18 ` Alexander Graf
3 siblings, 0 replies; 24+ messages in thread
From: Alexander Graf @ 2015-01-21 16:18 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, ard.biesheuvel, rob.herring, mst, claudio.fontana,
stuart.yoder, a.rigo
Every platform that supports PCI can also spawn the Bochs VGA PCI adapter. Move
it to pci.mak to enable it for everyone.
Signed-off-by: Alexander Graf <agraf@suse.de>
---
default-configs/alpha-softmmu.mak | 2 --
default-configs/i386-softmmu.mak | 2 --
default-configs/mips-softmmu.mak | 2 --
default-configs/mips64-softmmu.mak | 2 --
default-configs/mips64el-softmmu.mak | 2 --
default-configs/mipsel-softmmu.mak | 2 --
default-configs/pci.mak | 2 ++
default-configs/ppc-softmmu.mak | 2 --
default-configs/ppc64-softmmu.mak | 2 --
default-configs/ppcemb-softmmu.mak | 2 --
default-configs/sparc64-softmmu.mak | 2 --
default-configs/x86_64-softmmu.mak | 2 --
12 files changed, 2 insertions(+), 22 deletions(-)
diff --git a/default-configs/alpha-softmmu.mak b/default-configs/alpha-softmmu.mak
index bc07600..7f6161e 100644
--- a/default-configs/alpha-softmmu.mak
+++ b/default-configs/alpha-softmmu.mak
@@ -5,8 +5,6 @@ include usb.mak
CONFIG_SERIAL=y
CONFIG_I8254=y
CONFIG_PCKBD=y
-CONFIG_VGA=y
-CONFIG_VGA_PCI=y
CONFIG_VGA_CIRRUS=y
CONFIG_IDE_CORE=y
CONFIG_IDE_QDEV=y
diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
index 8e08841..bd99af9 100644
--- a/default-configs/i386-softmmu.mak
+++ b/default-configs/i386-softmmu.mak
@@ -3,9 +3,7 @@
include pci.mak
include sound.mak
include usb.mak
-CONFIG_VGA=y
CONFIG_QXL=$(CONFIG_SPICE)
-CONFIG_VGA_PCI=y
CONFIG_VGA_ISA=y
CONFIG_VGA_CIRRUS=y
CONFIG_VMWARE_VGA=y
diff --git a/default-configs/mips-softmmu.mak b/default-configs/mips-softmmu.mak
index 2a80b04..cce2c81 100644
--- a/default-configs/mips-softmmu.mak
+++ b/default-configs/mips-softmmu.mak
@@ -4,8 +4,6 @@ include pci.mak
include sound.mak
include usb.mak
CONFIG_ESP=y
-CONFIG_VGA=y
-CONFIG_VGA_PCI=y
CONFIG_VGA_ISA=y
CONFIG_VGA_ISA_MM=y
CONFIG_VGA_CIRRUS=y
diff --git a/default-configs/mips64-softmmu.mak b/default-configs/mips64-softmmu.mak
index f1f933b..7a88a08 100644
--- a/default-configs/mips64-softmmu.mak
+++ b/default-configs/mips64-softmmu.mak
@@ -4,8 +4,6 @@ include pci.mak
include sound.mak
include usb.mak
CONFIG_ESP=y
-CONFIG_VGA=y
-CONFIG_VGA_PCI=y
CONFIG_VGA_ISA=y
CONFIG_VGA_ISA_MM=y
CONFIG_VGA_CIRRUS=y
diff --git a/default-configs/mips64el-softmmu.mak b/default-configs/mips64el-softmmu.mak
index 317b151..095de43 100644
--- a/default-configs/mips64el-softmmu.mak
+++ b/default-configs/mips64el-softmmu.mak
@@ -4,8 +4,6 @@ include pci.mak
include sound.mak
include usb.mak
CONFIG_ESP=y
-CONFIG_VGA=y
-CONFIG_VGA_PCI=y
CONFIG_VGA_ISA=y
CONFIG_VGA_ISA_MM=y
CONFIG_VGA_CIRRUS=y
diff --git a/default-configs/mipsel-softmmu.mak b/default-configs/mipsel-softmmu.mak
index 7708185..0e25108 100644
--- a/default-configs/mipsel-softmmu.mak
+++ b/default-configs/mipsel-softmmu.mak
@@ -4,8 +4,6 @@ include pci.mak
include sound.mak
include usb.mak
CONFIG_ESP=y
-CONFIG_VGA=y
-CONFIG_VGA_PCI=y
CONFIG_VGA_ISA=y
CONFIG_VGA_ISA_MM=y
CONFIG_VGA_CIRRUS=y
diff --git a/default-configs/pci.mak b/default-configs/pci.mak
index a186c39..89986f1 100644
--- a/default-configs/pci.mak
+++ b/default-configs/pci.mak
@@ -32,3 +32,5 @@ CONFIG_PCI_TESTDEV=y
CONFIG_NVME_PCI=y
CONFIG_SD=y
CONFIG_SDHCI=y
+CONFIG_VGA=y
+CONFIG_VGA_PCI=y
diff --git a/default-configs/ppc-softmmu.mak b/default-configs/ppc-softmmu.mak
index d725b23..aebfab9 100644
--- a/default-configs/ppc-softmmu.mak
+++ b/default-configs/ppc-softmmu.mak
@@ -6,8 +6,6 @@ include usb.mak
CONFIG_ISA_MMIO=y
CONFIG_ESCC=y
CONFIG_M48T59=y
-CONFIG_VGA=y
-CONFIG_VGA_PCI=y
CONFIG_SERIAL=y
CONFIG_PARALLEL=y
CONFIG_I8254=y
diff --git a/default-configs/ppc64-softmmu.mak b/default-configs/ppc64-softmmu.mak
index bd30d69..f195a87 100644
--- a/default-configs/ppc64-softmmu.mak
+++ b/default-configs/ppc64-softmmu.mak
@@ -6,8 +6,6 @@ include usb.mak
CONFIG_ISA_MMIO=y
CONFIG_ESCC=y
CONFIG_M48T59=y
-CONFIG_VGA=y
-CONFIG_VGA_PCI=y
CONFIG_SERIAL=y
CONFIG_PARALLEL=y
CONFIG_I8254=y
diff --git a/default-configs/ppcemb-softmmu.mak b/default-configs/ppcemb-softmmu.mak
index e032761..a1b3d5f 100644
--- a/default-configs/ppcemb-softmmu.mak
+++ b/default-configs/ppcemb-softmmu.mak
@@ -4,8 +4,6 @@ include pci.mak
include sound.mak
include usb.mak
CONFIG_M48T59=y
-CONFIG_VGA=y
-CONFIG_VGA_PCI=y
CONFIG_SERIAL=y
CONFIG_I8257=y
CONFIG_OPENPIC=y
diff --git a/default-configs/sparc64-softmmu.mak b/default-configs/sparc64-softmmu.mak
index 299c97b..123bb99 100644
--- a/default-configs/sparc64-softmmu.mak
+++ b/default-configs/sparc64-softmmu.mak
@@ -5,8 +5,6 @@ include usb.mak
CONFIG_ISA_MMIO=y
CONFIG_M48T59=y
CONFIG_PTIMER=y
-CONFIG_VGA=y
-CONFIG_VGA_PCI=y
CONFIG_SERIAL=y
CONFIG_PARALLEL=y
CONFIG_PCKBD=y
diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak
index 66557ac..e7c2734 100644
--- a/default-configs/x86_64-softmmu.mak
+++ b/default-configs/x86_64-softmmu.mak
@@ -3,9 +3,7 @@
include pci.mak
include sound.mak
include usb.mak
-CONFIG_VGA=y
CONFIG_QXL=$(CONFIG_SPICE)
-CONFIG_VGA_PCI=y
CONFIG_VGA_ISA=y
CONFIG_VGA_CIRRUS=y
CONFIG_VMWARE_VGA=y
--
1.7.12.4
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/4] arm: Add PCIe host bridge in virt machine
2015-01-21 16:18 ` [Qemu-devel] [PATCH v2 3/4] arm: Add PCIe host bridge in virt machine Alexander Graf
@ 2015-01-22 15:28 ` Claudio Fontana
2015-01-22 15:52 ` Alexander Graf
2015-01-27 16:52 ` Peter Maydell
1 sibling, 1 reply; 24+ messages in thread
From: Claudio Fontana @ 2015-01-22 15:28 UTC (permalink / raw)
To: Alexander Graf, qemu-devel
Cc: Peter Maydell, ard.biesheuvel, mst, rob.herring, stuart.yoder, a.rigo
Hi Alexander,
thank you for the respin. I retested with the new mappings on OSv for AArch64,
and they show up ok in the guest.
Just a couple minor comments below, otherwise I think this is good.
On 21.01.2015 17:18, Alexander Graf wrote:
> Now that we have a working "generic" PCIe host bridge driver, we can plug
> it into ARMs virt machine to always have PCIe available to normal ARM VMs.
>
> I've successfully managed to expose a Bochs VGA device, XHCI and an e1000
> into an AArch64 VM with this and they all lived happily ever after.
>
> Signed-off-by: Alexander Graf <agraf@suse.de>
> Tested-by: Claudio Fontana <claudio.fontana@huawei.com>
>
> ---
>
> Linux 3.19 only supports the generic PCIe host bridge driver for 32bit ARM
> systems. If you want to use it with AArch64 guests, please apply the following
> patch or wait until upstream cleaned up the code properly:
>
> http://csgraf.de/agraf/pci/pci-3.19.patch
>
> v1 -> v2:
>
> - Add define for pci range types
> - Remove mmio_window_size
> - Use 4 PCI INTX IRQ lines
> ---
> default-configs/arm-softmmu.mak | 2 +
> hw/arm/virt.c | 112 ++++++++++++++++++++++++++++++++++++++--
> include/sysemu/device_tree.h | 9 ++++
> 3 files changed, 118 insertions(+), 5 deletions(-)
>
> diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
> index f3513fa..7671ee2 100644
> --- a/default-configs/arm-softmmu.mak
> +++ b/default-configs/arm-softmmu.mak
> @@ -82,6 +82,8 @@ CONFIG_ZYNQ=y
> CONFIG_VERSATILE_PCI=y
> CONFIG_VERSATILE_I2C=y
>
> +CONFIG_PCI_GENERIC=y
> +
> CONFIG_SDHCI=y
> CONFIG_INTEGRATOR_DEBUG=y
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 2353440..a727b4e 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -42,6 +42,7 @@
> #include "exec/address-spaces.h"
> #include "qemu/bitops.h"
> #include "qemu/error-report.h"
> +#include "hw/pci-host/gpex.h"
>
> #define NUM_VIRTIO_TRANSPORTS 32
>
> @@ -69,6 +70,7 @@ enum {
> VIRT_MMIO,
> VIRT_RTC,
> VIRT_FW_CFG,
> + VIRT_PCIE,
> };
>
> typedef struct MemMapEntry {
> @@ -129,13 +131,14 @@ static const MemMapEntry a15memmap[] = {
> [VIRT_FW_CFG] = { 0x09020000, 0x0000000a },
> [VIRT_MMIO] = { 0x0a000000, 0x00000200 },
> /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */
> - /* 0x10000000 .. 0x40000000 reserved for PCI */
> + [VIRT_PCIE] = { 0x10000000, 0x30000000 },
> [VIRT_MEM] = { 0x40000000, 30ULL * 1024 * 1024 * 1024 },
> };
>
> static const int a15irqmap[] = {
> [VIRT_UART] = 1,
> [VIRT_RTC] = 2,
> + [VIRT_PCIE] = 3, /* ... to 6 */
> [VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */
> };
>
> @@ -312,7 +315,7 @@ static void fdt_add_cpu_nodes(const VirtBoardInfo *vbi)
> }
> }
>
> -static void fdt_add_gic_node(const VirtBoardInfo *vbi)
> +static uint32_t fdt_add_gic_node(const VirtBoardInfo *vbi)
> {
> uint32_t gic_phandle;
>
> @@ -331,9 +334,11 @@ static void fdt_add_gic_node(const VirtBoardInfo *vbi)
> 2, vbi->memmap[VIRT_GIC_CPU].base,
> 2, vbi->memmap[VIRT_GIC_CPU].size);
> qemu_fdt_setprop_cell(vbi->fdt, "/intc", "phandle", gic_phandle);
> +
> + return gic_phandle;
> }
>
> -static void create_gic(const VirtBoardInfo *vbi, qemu_irq *pic)
> +static uint32_t create_gic(const VirtBoardInfo *vbi, qemu_irq *pic)
> {
> /* We create a standalone GIC v2 */
> DeviceState *gicdev;
> @@ -380,7 +385,7 @@ static void create_gic(const VirtBoardInfo *vbi, qemu_irq *pic)
> pic[i] = qdev_get_gpio_in(gicdev, i);
> }
>
> - fdt_add_gic_node(vbi);
> + return fdt_add_gic_node(vbi);
> }
>
> static void create_uart(const VirtBoardInfo *vbi, qemu_irq *pic)
> @@ -556,6 +561,100 @@ static void create_fw_cfg(const VirtBoardInfo *vbi)
> g_free(nodename);
> }
>
> +static void create_pcie_irq_map(const VirtBoardInfo *vbi, uint32_t gic_phandle,
> + int first_irq, const char *nodename)
> +{
> + int devfn, pin;
> + uint32_t full_irq_map[4 * 4 * 8] = { 0 };
> + uint32_t *irq_map = full_irq_map;
> +
> + for (devfn = 0; devfn <= 0x18; devfn+=0x8) {
devfn += 0x8 (spaces)
> + for (pin = 0; pin < 4; pin++) {
> + int irq_type = GIC_FDT_IRQ_TYPE_SPI;
> + int irq_nr = first_irq + ((pin + PCI_SLOT(devfn)) % PCI_NUM_PINS);
> + int irq_level = GIC_FDT_IRQ_FLAGS_LEVEL_HI;
> + int i;
> +
> + uint32_t map[] = {
> + devfn << 8, 0, 0, /* devfn */
> + pin + 1, /* PCI pin */
> + gic_phandle, irq_type, irq_nr, irq_level }; /* GIC irq */
> +
> + /* Convert map to big endian */
> + for (i = 0; i < 8; i++) {
> + irq_map[i] = cpu_to_be32(map[i]);
> + }
> + irq_map += 8;
> + }
> + }
> +
> + qemu_fdt_setprop(vbi->fdt, nodename, "interrupt-map",
> + full_irq_map, sizeof(full_irq_map));
> +}
I raise again (? or maybe for the first time) the suggestion to add a comment like:
/* see the openfirmware specs at
* http://www.firmware.org/1275/practice/imap/imap0_9d.pdf
*/
and optionally also a reference to the linux host-generic-pci bindings:
/* see the linux Documentation about device tree bindings at:
* https://www.kernel.org/doc/Documentation/devicetree/bindings/pci/host-generic-pci.txt
*/
Peter mentioned that the documentation there is not perfect, but at least it's a starting point for the reader to understand what's going on.
> +
> +static void create_pcie(const VirtBoardInfo *vbi, qemu_irq *pic,
> + uint32_t gic_phandle)
> +{
> + hwaddr base = vbi->memmap[VIRT_PCIE].base;
> + hwaddr size = vbi->memmap[VIRT_PCIE].size;
> + hwaddr size_ioport = 64 * 1024;
> + hwaddr size_ecam = PCIE_MMCFG_SIZE_MIN;
> + hwaddr size_mmio = size - size_ecam - size_ioport;
> + hwaddr base_mmio = base;
> + hwaddr base_ioport = base_mmio + size_mmio;
> + hwaddr base_ecam = base_ioport + size_ioport;
> + int irq = vbi->irqmap[VIRT_PCIE];
> + MemoryRegion *mmio_alias;
> + MemoryRegion *mmio_reg;
> + DeviceState *dev;
> + char *nodename;
> + int i;
> +
> + dev = qdev_create(NULL, TYPE_GPEX_HOST);
> + qdev_init_nofail(dev);
> +
> + sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base_ecam);
> + sysbus_mmio_map(SYS_BUS_DEVICE(dev), 2, base_ioport);
> +
> + /* Map the MMIO window at the same spot in bus and cpu layouts */
> + mmio_alias = g_new0(MemoryRegion, 1);
> + mmio_reg = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 1);
> + memory_region_init_alias(mmio_alias, OBJECT(dev), "pcie-mmio",
> + mmio_reg, base_mmio, size_mmio);
> + memory_region_add_subregion(get_system_memory(), base_mmio, mmio_alias);
> +
> + for (i = 0; i < 4; i++) {
> + sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, pic[irq + i]);
> + }
> +
> + nodename = g_strdup_printf("/pcie@%" PRIx64, base);
> + qemu_fdt_add_subnode(vbi->fdt, nodename);
> + qemu_fdt_setprop_string(vbi->fdt, nodename,
> + "compatible", "pci-host-ecam-generic");
> + qemu_fdt_setprop_string(vbi->fdt, nodename, "device_type", "pci");
> + qemu_fdt_setprop_cell(vbi->fdt, nodename, "#address-cells", 3);
> + qemu_fdt_setprop_cell(vbi->fdt, nodename, "#size-cells", 2);
> + qemu_fdt_setprop_cells(vbi->fdt, nodename, "bus-range", 0, 1);
> +
> + qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "reg",
> + 2, base_ecam, 2, size_ecam);
> + qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "ranges",
> + 1, FDT_PCI_RANGE_IOPORT, 2, 0,
> + 2, base_ioport, 2, size_ioport,
> +
> + 1, FDT_PCI_RANGE_MMIO, 2, base_mmio,
> + 2, base_mmio, 2, size_mmio);
> +
> + qemu_fdt_setprop_cell(vbi->fdt, nodename, "#interrupt-cells", 1);
> + create_pcie_irq_map(vbi, gic_phandle, irq, nodename);
> +
> + qemu_fdt_setprop_cells(vbi->fdt, nodename, "interrupt-map-mask",
> + 0x1800, 0, 0, /* devfn (PCI_SLOT(3)) */
> + 0x7 /* PCI irq */);
> +
> + g_free(nodename);
> +}
> +
> static void *machvirt_dtb(const struct arm_boot_info *binfo, int *fdt_size)
> {
> const VirtBoardInfo *board = (const VirtBoardInfo *)binfo;
> @@ -573,6 +672,7 @@ static void machvirt_init(MachineState *machine)
> MemoryRegion *ram = g_new(MemoryRegion, 1);
> const char *cpu_model = machine->cpu_model;
> VirtBoardInfo *vbi;
> + uint32_t gic_phandle;
>
> if (!cpu_model) {
> cpu_model = "cortex-a15";
> @@ -634,12 +734,14 @@ static void machvirt_init(MachineState *machine)
>
> create_flash(vbi);
>
> - create_gic(vbi, pic);
> + gic_phandle = create_gic(vbi, pic);
>
> create_uart(vbi, pic);
>
> create_rtc(vbi, pic);
>
> + create_pcie(vbi, pic, gic_phandle);
> +
> /* Create mmio transports, so the user can create virtio backends
> * (which will be automatically plugged in to the transports). If
> * no backend is created the transport will just sit harmlessly idle.
> diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h
> index 899f05c..979169b 100644
> --- a/include/sysemu/device_tree.h
> +++ b/include/sysemu/device_tree.h
> @@ -110,4 +110,13 @@ int qemu_fdt_setprop_sized_cells_from_array(void *fdt,
> qdt_tmp); \
> })
>
> +#define FDT_PCI_RANGE_RELOCATABLE 0x80000000
> +#define FDT_PCI_RANGE_PREFETCHABLE 0x40000000
> +#define FDT_PCI_RANGE_ALIASED 0x20000000
> +#define FDT_PCI_RANGE_TYPE 0x03000000
> +#define FDT_PCI_RANGE_MMIO_64BIT 0x03000000
> +#define FDT_PCI_RANGE_MMIO 0x02000000
> +#define FDT_PCI_RANGE_IOPORT 0x01000000
> +#define FDT_PCI_RANGE_CONFIG 0x00000000
> +
> #endif /* __DEVICE_TREE_H__ */
>
I appreciate those defines.
Thank you,
Claudio
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/4] arm: Add PCIe host bridge in virt machine
2015-01-22 15:28 ` Claudio Fontana
@ 2015-01-22 15:52 ` Alexander Graf
2015-01-27 9:24 ` Claudio Fontana
0 siblings, 1 reply; 24+ messages in thread
From: Alexander Graf @ 2015-01-22 15:52 UTC (permalink / raw)
To: Claudio Fontana, qemu-devel
Cc: Peter Maydell, ard.biesheuvel, mst, rob.herring, stuart.yoder, a.rigo
On 22.01.15 16:28, Claudio Fontana wrote:
> Hi Alexander,
>
> thank you for the respin. I retested with the new mappings on OSv for AArch64,
> and they show up ok in the guest.
>
> Just a couple minor comments below, otherwise I think this is good.
>
> On 21.01.2015 17:18, Alexander Graf wrote:
>> Now that we have a working "generic" PCIe host bridge driver, we can plug
>> it into ARMs virt machine to always have PCIe available to normal ARM VMs.
>>
>> I've successfully managed to expose a Bochs VGA device, XHCI and an e1000
>> into an AArch64 VM with this and they all lived happily ever after.
>>
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>> Tested-by: Claudio Fontana <claudio.fontana@huawei.com>
>>
>> ---
>>
>> Linux 3.19 only supports the generic PCIe host bridge driver for 32bit ARM
>> systems. If you want to use it with AArch64 guests, please apply the following
>> patch or wait until upstream cleaned up the code properly:
>>
>> http://csgraf.de/agraf/pci/pci-3.19.patch
>>
>> v1 -> v2:
>>
>> - Add define for pci range types
>> - Remove mmio_window_size
>> - Use 4 PCI INTX IRQ lines
>> ---
>> default-configs/arm-softmmu.mak | 2 +
>> hw/arm/virt.c | 112 ++++++++++++++++++++++++++++++++++++++--
>> include/sysemu/device_tree.h | 9 ++++
>> 3 files changed, 118 insertions(+), 5 deletions(-)
>>
>> diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
>> index f3513fa..7671ee2 100644
>> --- a/default-configs/arm-softmmu.mak
>> +++ b/default-configs/arm-softmmu.mak
>> @@ -82,6 +82,8 @@ CONFIG_ZYNQ=y
>> CONFIG_VERSATILE_PCI=y
>> CONFIG_VERSATILE_I2C=y
>>
>> +CONFIG_PCI_GENERIC=y
>> +
>> CONFIG_SDHCI=y
>> CONFIG_INTEGRATOR_DEBUG=y
>>
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index 2353440..a727b4e 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -42,6 +42,7 @@
>> #include "exec/address-spaces.h"
>> #include "qemu/bitops.h"
>> #include "qemu/error-report.h"
>> +#include "hw/pci-host/gpex.h"
>>
>> #define NUM_VIRTIO_TRANSPORTS 32
>>
>> @@ -69,6 +70,7 @@ enum {
>> VIRT_MMIO,
>> VIRT_RTC,
>> VIRT_FW_CFG,
>> + VIRT_PCIE,
>> };
>>
>> typedef struct MemMapEntry {
>> @@ -129,13 +131,14 @@ static const MemMapEntry a15memmap[] = {
>> [VIRT_FW_CFG] = { 0x09020000, 0x0000000a },
>> [VIRT_MMIO] = { 0x0a000000, 0x00000200 },
>> /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */
>> - /* 0x10000000 .. 0x40000000 reserved for PCI */
>> + [VIRT_PCIE] = { 0x10000000, 0x30000000 },
>> [VIRT_MEM] = { 0x40000000, 30ULL * 1024 * 1024 * 1024 },
>> };
>>
>> static const int a15irqmap[] = {
>> [VIRT_UART] = 1,
>> [VIRT_RTC] = 2,
>> + [VIRT_PCIE] = 3, /* ... to 6 */
>> [VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */
>> };
>>
>> @@ -312,7 +315,7 @@ static void fdt_add_cpu_nodes(const VirtBoardInfo *vbi)
>> }
>> }
>>
>> -static void fdt_add_gic_node(const VirtBoardInfo *vbi)
>> +static uint32_t fdt_add_gic_node(const VirtBoardInfo *vbi)
>> {
>> uint32_t gic_phandle;
>>
>> @@ -331,9 +334,11 @@ static void fdt_add_gic_node(const VirtBoardInfo *vbi)
>> 2, vbi->memmap[VIRT_GIC_CPU].base,
>> 2, vbi->memmap[VIRT_GIC_CPU].size);
>> qemu_fdt_setprop_cell(vbi->fdt, "/intc", "phandle", gic_phandle);
>> +
>> + return gic_phandle;
>> }
>>
>> -static void create_gic(const VirtBoardInfo *vbi, qemu_irq *pic)
>> +static uint32_t create_gic(const VirtBoardInfo *vbi, qemu_irq *pic)
>> {
>> /* We create a standalone GIC v2 */
>> DeviceState *gicdev;
>> @@ -380,7 +385,7 @@ static void create_gic(const VirtBoardInfo *vbi, qemu_irq *pic)
>> pic[i] = qdev_get_gpio_in(gicdev, i);
>> }
>>
>> - fdt_add_gic_node(vbi);
>> + return fdt_add_gic_node(vbi);
>> }
>>
>> static void create_uart(const VirtBoardInfo *vbi, qemu_irq *pic)
>> @@ -556,6 +561,100 @@ static void create_fw_cfg(const VirtBoardInfo *vbi)
>> g_free(nodename);
>> }
>>
>> +static void create_pcie_irq_map(const VirtBoardInfo *vbi, uint32_t gic_phandle,
>> + int first_irq, const char *nodename)
>> +{
>> + int devfn, pin;
>> + uint32_t full_irq_map[4 * 4 * 8] = { 0 };
>> + uint32_t *irq_map = full_irq_map;
>> +
>> + for (devfn = 0; devfn <= 0x18; devfn+=0x8) {
>
> devfn += 0x8 (spaces)
Yeah, I had it like that and it looked uglier. I guess it's a matter of
personal preference?
>
>> + for (pin = 0; pin < 4; pin++) {
>> + int irq_type = GIC_FDT_IRQ_TYPE_SPI;
>> + int irq_nr = first_irq + ((pin + PCI_SLOT(devfn)) % PCI_NUM_PINS);
>> + int irq_level = GIC_FDT_IRQ_FLAGS_LEVEL_HI;
>> + int i;
>> +
>> + uint32_t map[] = {
>> + devfn << 8, 0, 0, /* devfn */
>> + pin + 1, /* PCI pin */
>> + gic_phandle, irq_type, irq_nr, irq_level }; /* GIC irq */
>> +
>> + /* Convert map to big endian */
>> + for (i = 0; i < 8; i++) {
>> + irq_map[i] = cpu_to_be32(map[i]);
>> + }
>> + irq_map += 8;
>> + }
>> + }
>> +
>> + qemu_fdt_setprop(vbi->fdt, nodename, "interrupt-map",
>> + full_irq_map, sizeof(full_irq_map));
>> +}
>
> I raise again (? or maybe for the first time) the suggestion to add a comment like:
>
> /* see the openfirmware specs at
> * http://www.firmware.org/1275/practice/imap/imap0_9d.pdf
> */
>
> and optionally also a reference to the linux host-generic-pci bindings:
>
> /* see the linux Documentation about device tree bindings at:
> * https://www.kernel.org/doc/Documentation/devicetree/bindings/pci/host-generic-pci.txt
> */
I added those links to the big description comment at the top of this file.
Alex
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/4] pci: Add generic PCIe host bridge
2015-01-21 16:18 ` [Qemu-devel] [PATCH v2 2/4] pci: Add generic PCIe host bridge Alexander Graf
@ 2015-01-22 16:32 ` B02008
2015-01-27 15:31 ` Peter Maydell
1 sibling, 0 replies; 24+ messages in thread
From: B02008 @ 2015-01-22 16:32 UTC (permalink / raw)
To: Alexander Graf, qemu-devel
Cc: Peter Maydell, mst, ard.biesheuvel, claudio.fontana,
stuart.yoder, a.rigo, rob.herring
Looks that you forgot the "single legacy IRQ line" comment from v1.
-Mike
On 01/21/2015 06:18 PM, Alexander Graf wrote:
> With simple exposure of MMFG, ioport window, mmio window and an IRQ line we
> can successfully create a workable PCIe host bridge that can be mapped anywhere
> and only needs to get described to the OS using whatever means it likes.
>
> This patch implements such a "generic" host bridge. It only supports a single
> legacy IRQ line so far. MSIs need to be handled external to the host bridge.
>
> This device is particularly useful for the "pci-host-ecam-generic" driver in
> Linux.
>
> Signed-off-by: Alexander Graf <agraf@suse.de>
> Reviewed-by: Claudio Fontana <claudio.fontana@huawei.com>
> Tested-by: Claudio Fontana <claudio.fontana@huawei.com>
>
> ---
>
> v1 -> v2:
>
> - Add documentation links
> - Remove mmio_window_size
> - Expose 4 INTX IRQs
> ---
> hw/pci-host/Makefile.objs | 1 +
> hw/pci-host/gpex.c | 153 +++++++++++++++++++++++++++++++++++++++++++++
> include/hw/pci-host/gpex.h | 54 ++++++++++++++++
> 3 files changed, 208 insertions(+)
> create mode 100644 hw/pci-host/gpex.c
> create mode 100644 include/hw/pci-host/gpex.h
>
> diff --git a/hw/pci-host/Makefile.objs b/hw/pci-host/Makefile.objs
> index bb65f9c..45f1f0e 100644
> --- a/hw/pci-host/Makefile.objs
> +++ b/hw/pci-host/Makefile.objs
> @@ -15,3 +15,4 @@ common-obj-$(CONFIG_PCI_APB) += apb.o
> common-obj-$(CONFIG_FULONG) += bonito.o
> common-obj-$(CONFIG_PCI_PIIX) += piix.o
> common-obj-$(CONFIG_PCI_Q35) += q35.o
> +common-obj-$(CONFIG_PCI_GENERIC) += gpex.o
> diff --git a/hw/pci-host/gpex.c b/hw/pci-host/gpex.c
> new file mode 100644
> index 0000000..11aaf5b
> --- /dev/null
> +++ b/hw/pci-host/gpex.c
> @@ -0,0 +1,153 @@
> +/*
> + * QEMU Generic PCI Express Bridge Emulation
> + *
> + * Copyright (C) 2015 Alexander Graf <agraf@suse.de>
> + *
> + * Code loosely based on q35.c.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + *
> + * Check out these documents for more information on the device:
> + *
> + * http://www.kernel.org/doc/Documentation/devicetree/bindings/pci/host-generic-pci.txt
> + * http://www.firmware.org/1275/practice/imap/imap0_9d.pdf
> + */
> +#include "hw/hw.h"
> +#include "hw/pci-host/gpex.h"
> +
> +/****************************************************************************
> + * GPEX host
> + */
> +
> +static void gpex_set_irq(void *opaque, int irq_num, int level)
> +{
> + GPEXHost *s = opaque;
> +
> + qemu_set_irq(s->irq[irq_num], level);
> +}
> +
> +static void gpex_host_realize(DeviceState *dev, Error **errp)
> +{
> + PCIHostState *pci = PCI_HOST_BRIDGE(dev);
> + GPEXHost *s = GPEX_HOST(dev);
> + SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> + PCIExpressHost *pex = PCIE_HOST_BRIDGE(dev);
> + int i;
> +
> + pcie_host_mmcfg_init(pex, PCIE_MMCFG_SIZE_MIN);
> + memory_region_init(&s->io_mmio, OBJECT(s), "gpex_mmio", UINT64_MAX);
> + memory_region_init(&s->io_ioport, OBJECT(s), "gpex_ioport", 64 * 1024);
> +
> + sysbus_init_mmio(sbd, &pex->mmio);
> + sysbus_init_mmio(sbd, &s->io_mmio);
> + sysbus_init_mmio(sbd, &s->io_ioport);
> + for (i = 0; i < 4; i++) {
> + sysbus_init_irq(sbd, &s->irq[i]);
> + }
> +
> + pci->bus = pci_register_bus(dev, "pcie.0", gpex_set_irq,
> + pci_swizzle_map_irq_fn, s, &s->io_mmio,
> + &s->io_ioport, 0, 4, TYPE_PCIE_BUS);
> +
> + qdev_set_parent_bus(DEVICE(&s->gpex_root), BUS(pci->bus));
> + qdev_init_nofail(DEVICE(&s->gpex_root));
> +}
> +
> +static const char *gpex_host_root_bus_path(PCIHostState *host_bridge,
> + PCIBus *rootbus)
> +{
> + return "0000:00";
> +}
> +
> +static void gpex_host_class_init(ObjectClass *klass, void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(klass);
> + PCIHostBridgeClass *hc = PCI_HOST_BRIDGE_CLASS(klass);
> +
> + hc->root_bus_path = gpex_host_root_bus_path;
> + dc->realize = gpex_host_realize;
> + set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
> + dc->fw_name = "pci";
> +}
> +
> +static void gpex_host_initfn(Object *obj)
> +{
> + GPEXHost *s = GPEX_HOST(obj);
> +
> + object_initialize(&s->gpex_root, sizeof(s->gpex_root), TYPE_GPEX_ROOT_DEVICE);
> + object_property_add_child(OBJECT(s), "gpex_root", OBJECT(&s->gpex_root), NULL);
> + qdev_prop_set_uint32(DEVICE(&s->gpex_root), "addr", PCI_DEVFN(0, 0));
> + qdev_prop_set_bit(DEVICE(&s->gpex_root), "multifunction", false);
> +}
> +
> +static const TypeInfo gpex_host_info = {
> + .name = TYPE_GPEX_HOST,
> + .parent = TYPE_PCIE_HOST_BRIDGE,
> + .instance_size = sizeof(GPEXHost),
> + .instance_init = gpex_host_initfn,
> + .class_init = gpex_host_class_init,
> +};
> +
> +/****************************************************************************
> + * GPEX Root D0:F0
> + */
> +
> +static const VMStateDescription vmstate_gpex_root = {
> + .name = "gpex_root",
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .fields = (VMStateField[]) {
> + VMSTATE_PCI_DEVICE(parent_obj, GPEXRootState),
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> +static void gpex_root_class_init(ObjectClass *klass, void *data)
> +{
> + PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> + DeviceClass *dc = DEVICE_CLASS(klass);
> +
> + set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
> + dc->desc = "Host bridge";
> + dc->vmsd = &vmstate_gpex_root;
> + k->vendor_id = PCI_VENDOR_ID_REDHAT;
> + k->device_id = PCI_DEVICE_ID_REDHAT_BRIDGE;
> + k->revision = 0;
> + k->class_id = PCI_CLASS_BRIDGE_HOST;
> + /*
> + * PCI-facing part of the host bridge, not usable without the
> + * host-facing part, which can't be device_add'ed, yet.
> + */
> + dc->cannot_instantiate_with_device_add_yet = true;
> +}
> +
> +static const TypeInfo gpex_root_info = {
> + .name = TYPE_GPEX_ROOT_DEVICE,
> + .parent = TYPE_PCI_DEVICE,
> + .instance_size = sizeof(GPEXRootState),
> + .class_init = gpex_root_class_init,
> +};
> +
> +static void gpex_register(void)
> +{
> + type_register_static(&gpex_root_info);
> + type_register_static(&gpex_host_info);
> +}
> +
> +type_init(gpex_register);
> diff --git a/include/hw/pci-host/gpex.h b/include/hw/pci-host/gpex.h
> new file mode 100644
> index 0000000..b465667
> --- /dev/null
> +++ b/include/hw/pci-host/gpex.h
> @@ -0,0 +1,54 @@
> +/*
> + * gpex.h
> + *
> + * Copyright (C) 2015 Alexander Graf <agraf@suse.de>
> + *
> + * 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 Lesser General Public
> + * License along with this library; if not, see <http://www.gnu.org/licenses/>
> + */
> +
> +#ifndef HW_GPEX_H
> +#define HW_GPEX_H
> +
> +#include "hw/hw.h"
> +#include "hw/sysbus.h"
> +#include "hw/pci/pci.h"
> +#include "hw/pci/pcie_host.h"
> +
> +#define TYPE_GPEX_HOST "gpex-pcihost"
> +#define GPEX_HOST(obj) \
> + OBJECT_CHECK(GPEXHost, (obj), TYPE_GPEX_HOST)
> +
> +#define TYPE_GPEX_ROOT_DEVICE "gpex-root"
> +#define MCH_PCI_DEVICE(obj) \
> + OBJECT_CHECK(GPEXRootState, (obj), TYPE_GPEX_ROOT_DEVICE)
> +
> +typedef struct GPEXRootState {
> + /*< private >*/
> + PCIDevice parent_obj;
> + /*< public >*/
> +} GPEXRootState;
> +
> +typedef struct GPEXHost {
> + /*< private >*/
> + PCIExpressHost parent_obj;
> + /*< public >*/
> +
> + GPEXRootState gpex_root;
> +
> + MemoryRegion io_ioport;
> + MemoryRegion io_mmio;
> + qemu_irq irq[4];
> +} GPEXHost;
> +
> +#endif /* HW_GPEX_H */
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/4] arm: Add PCIe host bridge in virt machine
2015-01-22 15:52 ` Alexander Graf
@ 2015-01-27 9:24 ` Claudio Fontana
2015-01-27 10:09 ` Peter Maydell
0 siblings, 1 reply; 24+ messages in thread
From: Claudio Fontana @ 2015-01-27 9:24 UTC (permalink / raw)
To: Alexander Graf, qemu-devel
Cc: Peter Maydell, ard.biesheuvel, mst, rob.herring, stuart.yoder, a.rigo
On 22.01.2015 16:52, Alexander Graf wrote:
>
>
> On 22.01.15 16:28, Claudio Fontana wrote:
>> Hi Alexander,
>>
>> thank you for the respin. I retested with the new mappings on OSv for AArch64,
>> and they show up ok in the guest.
>>
>> Just a couple minor comments below, otherwise I think this is good.
>>
>> On 21.01.2015 17:18, Alexander Graf wrote:
>>> Now that we have a working "generic" PCIe host bridge driver, we can plug
>>> it into ARMs virt machine to always have PCIe available to normal ARM VMs.
>>>
>>> I've successfully managed to expose a Bochs VGA device, XHCI and an e1000
>>> into an AArch64 VM with this and they all lived happily ever after.
>>>
>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>> Tested-by: Claudio Fontana <claudio.fontana@huawei.com>
>>>
>>> ---
>>>
>>> Linux 3.19 only supports the generic PCIe host bridge driver for 32bit ARM
>>> systems. If you want to use it with AArch64 guests, please apply the following
>>> patch or wait until upstream cleaned up the code properly:
>>>
>>> http://csgraf.de/agraf/pci/pci-3.19.patch
>>>
>>> v1 -> v2:
>>>
>>> - Add define for pci range types
>>> - Remove mmio_window_size
>>> - Use 4 PCI INTX IRQ lines
>>> ---
>>> default-configs/arm-softmmu.mak | 2 +
>>> hw/arm/virt.c | 112 ++++++++++++++++++++++++++++++++++++++--
>>> include/sysemu/device_tree.h | 9 ++++
>>> 3 files changed, 118 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
>>> index f3513fa..7671ee2 100644
>>> --- a/default-configs/arm-softmmu.mak
>>> +++ b/default-configs/arm-softmmu.mak
>>> @@ -82,6 +82,8 @@ CONFIG_ZYNQ=y
>>> CONFIG_VERSATILE_PCI=y
>>> CONFIG_VERSATILE_I2C=y
>>>
>>> +CONFIG_PCI_GENERIC=y
>>> +
>>> CONFIG_SDHCI=y
>>> CONFIG_INTEGRATOR_DEBUG=y
>>>
>>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>>> index 2353440..a727b4e 100644
>>> --- a/hw/arm/virt.c
>>> +++ b/hw/arm/virt.c
>>> @@ -42,6 +42,7 @@
>>> #include "exec/address-spaces.h"
>>> #include "qemu/bitops.h"
>>> #include "qemu/error-report.h"
>>> +#include "hw/pci-host/gpex.h"
>>>
>>> #define NUM_VIRTIO_TRANSPORTS 32
>>>
>>> @@ -69,6 +70,7 @@ enum {
>>> VIRT_MMIO,
>>> VIRT_RTC,
>>> VIRT_FW_CFG,
>>> + VIRT_PCIE,
>>> };
>>>
>>> typedef struct MemMapEntry {
>>> @@ -129,13 +131,14 @@ static const MemMapEntry a15memmap[] = {
>>> [VIRT_FW_CFG] = { 0x09020000, 0x0000000a },
>>> [VIRT_MMIO] = { 0x0a000000, 0x00000200 },
>>> /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */
>>> - /* 0x10000000 .. 0x40000000 reserved for PCI */
>>> + [VIRT_PCIE] = { 0x10000000, 0x30000000 },
>>> [VIRT_MEM] = { 0x40000000, 30ULL * 1024 * 1024 * 1024 },
>>> };
>>>
>>> static const int a15irqmap[] = {
>>> [VIRT_UART] = 1,
>>> [VIRT_RTC] = 2,
>>> + [VIRT_PCIE] = 3, /* ... to 6 */
>>> [VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */
>>> };
>>>
>>> @@ -312,7 +315,7 @@ static void fdt_add_cpu_nodes(const VirtBoardInfo *vbi)
>>> }
>>> }
>>>
>>> -static void fdt_add_gic_node(const VirtBoardInfo *vbi)
>>> +static uint32_t fdt_add_gic_node(const VirtBoardInfo *vbi)
>>> {
>>> uint32_t gic_phandle;
>>>
>>> @@ -331,9 +334,11 @@ static void fdt_add_gic_node(const VirtBoardInfo *vbi)
>>> 2, vbi->memmap[VIRT_GIC_CPU].base,
>>> 2, vbi->memmap[VIRT_GIC_CPU].size);
>>> qemu_fdt_setprop_cell(vbi->fdt, "/intc", "phandle", gic_phandle);
>>> +
>>> + return gic_phandle;
>>> }
>>>
>>> -static void create_gic(const VirtBoardInfo *vbi, qemu_irq *pic)
>>> +static uint32_t create_gic(const VirtBoardInfo *vbi, qemu_irq *pic)
>>> {
>>> /* We create a standalone GIC v2 */
>>> DeviceState *gicdev;
>>> @@ -380,7 +385,7 @@ static void create_gic(const VirtBoardInfo *vbi, qemu_irq *pic)
>>> pic[i] = qdev_get_gpio_in(gicdev, i);
>>> }
>>>
>>> - fdt_add_gic_node(vbi);
>>> + return fdt_add_gic_node(vbi);
>>> }
>>>
>>> static void create_uart(const VirtBoardInfo *vbi, qemu_irq *pic)
>>> @@ -556,6 +561,100 @@ static void create_fw_cfg(const VirtBoardInfo *vbi)
>>> g_free(nodename);
>>> }
>>>
>>> +static void create_pcie_irq_map(const VirtBoardInfo *vbi, uint32_t gic_phandle,
>>> + int first_irq, const char *nodename)
>>> +{
>>> + int devfn, pin;
>>> + uint32_t full_irq_map[4 * 4 * 8] = { 0 };
>>> + uint32_t *irq_map = full_irq_map;
>>> +
>>> + for (devfn = 0; devfn <= 0x18; devfn+=0x8) {
>>
>> devfn += 0x8 (spaces)
>
> Yeah, I had it like that and it looked uglier. I guess it's a matter of
> personal preference?
You don't have coding standards for this ?
>
>>
>>> + for (pin = 0; pin < 4; pin++) {
>>> + int irq_type = GIC_FDT_IRQ_TYPE_SPI;
>>> + int irq_nr = first_irq + ((pin + PCI_SLOT(devfn)) % PCI_NUM_PINS);
>>> + int irq_level = GIC_FDT_IRQ_FLAGS_LEVEL_HI;
>>> + int i;
>>> +
>>> + uint32_t map[] = {
>>> + devfn << 8, 0, 0, /* devfn */
>>> + pin + 1, /* PCI pin */
>>> + gic_phandle, irq_type, irq_nr, irq_level }; /* GIC irq */
>>> +
>>> + /* Convert map to big endian */
>>> + for (i = 0; i < 8; i++) {
>>> + irq_map[i] = cpu_to_be32(map[i]);
>>> + }
>>> + irq_map += 8;
>>> + }
>>> + }
>>> +
>>> + qemu_fdt_setprop(vbi->fdt, nodename, "interrupt-map",
>>> + full_irq_map, sizeof(full_irq_map));
>>> +}
>>
>> I raise again (? or maybe for the first time) the suggestion to add a comment like:
>>
>> /* see the openfirmware specs at
>> * http://www.firmware.org/1275/practice/imap/imap0_9d.pdf
>> */
>>
>> and optionally also a reference to the linux host-generic-pci bindings:
>>
>> /* see the linux Documentation about device tree bindings at:
>> * https://www.kernel.org/doc/Documentation/devicetree/bindings/pci/host-generic-pci.txt
>> */
>
> I added those links to the big description comment at the top of this file.
>
It is not on top of this file unfortunately.
You put the description comment in an unrelated .h file: the pointers to the docs belong here I think.
Ciao,
Claudio
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/4] arm: Add PCIe host bridge in virt machine
2015-01-27 9:24 ` Claudio Fontana
@ 2015-01-27 10:09 ` Peter Maydell
2015-01-27 14:37 ` Claudio Fontana
0 siblings, 1 reply; 24+ messages in thread
From: Peter Maydell @ 2015-01-27 10:09 UTC (permalink / raw)
To: Claudio Fontana
Cc: Rob Herring, Michael S. Tsirkin, QEMU Developers, Ard Biesheuvel,
Alvise Rigo, Stuart Yoder, Alexander Graf
On 27 January 2015 at 09:24, Claudio Fontana <claudio.fontana@huawei.com> wrote:
> On 22.01.2015 16:52, Alexander Graf wrote:
>> On 22.01.15 16:28, Claudio Fontana wrote:
>>> Alex wrote;
>>>> + for (devfn = 0; devfn <= 0x18; devfn+=0x8) {
>>>
>>> devfn += 0x8 (spaces)
>>
>> Yeah, I had it like that and it looked uglier. I guess it's a matter of
>> personal preference?
>
> You don't have coding standards for this ?
Not formal ones, but (a) I think += should have spaces aronud
it and (b) I'm a bit surprised if checkpatch doesn't catch
this (it certainly attempts to...)
-- PMM
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/4] pci: Split pcie_host_mmcfg_map()
2015-01-21 16:18 ` [Qemu-devel] [PATCH v2 1/4] pci: Split pcie_host_mmcfg_map() Alexander Graf
@ 2015-01-27 13:55 ` Peter Maydell
2015-01-27 14:24 ` Michael S. Tsirkin
0 siblings, 1 reply; 24+ messages in thread
From: Peter Maydell @ 2015-01-27 13:55 UTC (permalink / raw)
To: Alexander Graf
Cc: Rob Herring, Michael S. Tsirkin, QEMU Developers, Ard Biesheuvel,
Claudio Fontana, Alvise Rigo, Stuart Yoder
On 21 January 2015 at 16:18, Alexander Graf <agraf@suse.de> wrote:
> The mmcfg space is a memory region that allows access to PCI config space
> in the PCIe world. To maintain abstraction layers, I would like to expose
> the mmcfg space as a sysbus mmio region rather than have it mapped straight
> into the system's memory address space though.
>
> So this patch splits the initialization of the mmcfg space from the actual
> mapping, allowing us to only have an mmfg memory region without the map.
>
> Signed-off-by: Alexander Graf <agraf@suse.de>
> Reviewed-by: Claudio Fontana <claudio.fontana@huawei.com>
> Tested-by: Claudio Fontana <claudio.fontana@huawei.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
...as far as it goes, but:
Really the pcie_host_mmcfg_map/unmap/update() function is just totally
misguided. This functionality should be pushed upwards into
hw/pci-host/q35.c which can handle its own mapping of the MMIO region
into the system address space at the appropriate location/size.
In particular, at the moment q35.c will leak a bunch of stuff
every time the guest unmaps and remaps the mmcfg space, because
we call memory_region_init_io() over and over again on the same
MMIO object (which isn't valid).
Any time you see a device with its own base address in its
device struct it's a red flag that the design's probably wrong...
The size of the MMCFG region should probably be a device property.
Then the subclass realize could just rely on the baseclass realize
to always create the mmio region, rather than having to explicitly
call a function to get it to do the right thing.
thanks
-- PMM
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/4] pci: Split pcie_host_mmcfg_map()
2015-01-27 13:55 ` Peter Maydell
@ 2015-01-27 14:24 ` Michael S. Tsirkin
2015-01-27 14:40 ` Paolo Bonzini
0 siblings, 1 reply; 24+ messages in thread
From: Michael S. Tsirkin @ 2015-01-27 14:24 UTC (permalink / raw)
To: Peter Maydell
Cc: Rob Herring, Ard Biesheuvel, QEMU Developers, Claudio Fontana,
Alvise Rigo, Stuart Yoder, Alexander Graf, pbonzini
On Tue, Jan 27, 2015 at 01:55:32PM +0000, Peter Maydell wrote:
> On 21 January 2015 at 16:18, Alexander Graf <agraf@suse.de> wrote:
> > The mmcfg space is a memory region that allows access to PCI config space
> > in the PCIe world. To maintain abstraction layers, I would like to expose
> > the mmcfg space as a sysbus mmio region rather than have it mapped straight
> > into the system's memory address space though.
> >
> > So this patch splits the initialization of the mmcfg space from the actual
> > mapping, allowing us to only have an mmfg memory region without the map.
> >
> > Signed-off-by: Alexander Graf <agraf@suse.de>
> > Reviewed-by: Claudio Fontana <claudio.fontana@huawei.com>
> > Tested-by: Claudio Fontana <claudio.fontana@huawei.com>
>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> ...as far as it goes, but:
>
> Really the pcie_host_mmcfg_map/unmap/update() function is just totally
> misguided. This functionality should be pushed upwards into
> hw/pci-host/q35.c which can handle its own mapping of the MMIO region
> into the system address space at the appropriate location/size.
>
> In particular, at the moment q35.c will leak a bunch of stuff
> every time the guest unmaps and remaps the mmcfg space, because
> we call memory_region_init_io() over and over again on the same
> MMIO object (which isn't valid).
I used to be fine before the QOM conversion I think?
Take a look at this one (and previous patch):
commit 469b046ead0671932ff3af8d6f95045b19b186ef
Author: Paolo Bonzini <pbonzini@redhat.com>
Date: Wed Jun 11 12:50:43 2014 +0200
memory: remove memory_region_destroy
The function is empty after the previous patch, so remove it.
Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>
> Any time you see a device with its own base address in its
> device struct it's a red flag that the design's probably wrong...
I suspect this is not the only device that leaks memory now.
Paolo?
> The size of the MMCFG region should probably be a device property.
> Then the subclass realize could just rely on the baseclass realize
> to always create the mmio region, rather than having to explicitly
> call a function to get it to do the right thing.
>
> thanks
> -- PMM
--
MST
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/4] arm: Add PCIe host bridge in virt machine
2015-01-27 10:09 ` Peter Maydell
@ 2015-01-27 14:37 ` Claudio Fontana
0 siblings, 0 replies; 24+ messages in thread
From: Claudio Fontana @ 2015-01-27 14:37 UTC (permalink / raw)
To: Peter Maydell
Cc: Rob Herring, Michael S. Tsirkin, QEMU Developers, Ard Biesheuvel,
Alvise Rigo, Stuart Yoder, Alexander Graf
On 27.01.2015 11:09, Peter Maydell wrote:
> On 27 January 2015 at 09:24, Claudio Fontana <claudio.fontana@huawei.com> wrote:
>> On 22.01.2015 16:52, Alexander Graf wrote:
>>> On 22.01.15 16:28, Claudio Fontana wrote:
>>>> Alex wrote;
>>>>> + for (devfn = 0; devfn <= 0x18; devfn+=0x8) {
>>>>
>>>> devfn += 0x8 (spaces)
>>>
>>> Yeah, I had it like that and it looked uglier. I guess it's a matter of
>>> personal preference?
>>
>> You don't have coding standards for this ?
>
> Not formal ones, but (a) I think += should have spaces aronud
> it and (b) I'm a bit surprised if checkpatch doesn't catch
> this (it certainly attempts to...)
>
> -- PMM
It does:
ERROR: spaces required around that '+=' (ctx:VxV)
#167: FILE: hw/arm/virt.c:571:
+ for (devfn = 0; devfn <= 0x18; devfn+=0x8) {
^
Claudio
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/4] pci: Split pcie_host_mmcfg_map()
2015-01-27 14:24 ` Michael S. Tsirkin
@ 2015-01-27 14:40 ` Paolo Bonzini
0 siblings, 0 replies; 24+ messages in thread
From: Paolo Bonzini @ 2015-01-27 14:40 UTC (permalink / raw)
To: Michael S. Tsirkin, Peter Maydell
Cc: Rob Herring, Ard Biesheuvel, QEMU Developers, Claudio Fontana,
Alvise Rigo, Stuart Yoder, Alexander Graf
On 27/01/2015 15:24, Michael S. Tsirkin wrote:
> I suspect this is not the only device that leaks memory now.
> Paolo?
It should be, this is a bug.
Commit d8d95814609e89e5438a3318a647ec322fc4ff16 missed this place
due to a rebase error. (I can tell it's a rebase error because I
have the fixed patch in a private branch).
Paolo
Author: Paolo Bonzini <pbonzini@redhat.com>
Date: Wed Jun 11 12:42:01 2014 +0200
memory: convert memory_region_destroy to object_unparent
Explicitly call object_unparent in the few places where we
will re-create the memory region. If the memory region is
simply being destroyed as part of device teardown, let QOM
handle it.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/4] pci: Add generic PCIe host bridge
2015-01-21 16:18 ` [Qemu-devel] [PATCH v2 2/4] pci: Add generic PCIe host bridge Alexander Graf
2015-01-22 16:32 ` B02008
@ 2015-01-27 15:31 ` Peter Maydell
2015-01-29 13:59 ` Alexander Graf
1 sibling, 1 reply; 24+ messages in thread
From: Peter Maydell @ 2015-01-27 15:31 UTC (permalink / raw)
To: Alexander Graf
Cc: Rob Herring, Michael S. Tsirkin, QEMU Developers, Ard Biesheuvel,
Claudio Fontana, Alvise Rigo, Stuart Yoder
On 21 January 2015 at 16:18, Alexander Graf <agraf@suse.de> wrote:
> With simple exposure of MMFG, ioport window, mmio window and an IRQ line we
Four :-)
> can successfully create a workable PCIe host bridge that can be mapped anywhere
> and only needs to get described to the OS using whatever means it likes.
>
> This patch implements such a "generic" host bridge. It only supports a single
> legacy IRQ line so far. MSIs need to be handled external to the host bridge.
>
> This device is particularly useful for the "pci-host-ecam-generic" driver in
> Linux.
>
> Signed-off-by: Alexander Graf <agraf@suse.de>
> Reviewed-by: Claudio Fontana <claudio.fontana@huawei.com>
> Tested-by: Claudio Fontana <claudio.fontana@huawei.com>
Checkpatch complains about a few over-80-chars lines;
the URL is an unavoidable one but could you fold the others,
please?
> +static void gpex_host_realize(DeviceState *dev, Error **errp)
> +{
> + PCIHostState *pci = PCI_HOST_BRIDGE(dev);
> + GPEXHost *s = GPEX_HOST(dev);
> + SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> + PCIExpressHost *pex = PCIE_HOST_BRIDGE(dev);
> + int i;
> +
> + pcie_host_mmcfg_init(pex, PCIE_MMCFG_SIZE_MIN);
So this gives us 1MB of ECAM (config) space. That means
we can't specify a target bus, so we're restricted to
the 31 devices directly attached to the root complex.
Among other things, this will mean we can't do PCIe
hotplug. I think we should make this at least a bit bigger,
even if we don't go up to the full 256MB.
Probably the best thing for the gpex device itself is
to make the config space be the spec maximum (PCIE_MMCFG_SIZE_MAX)
and let the user map only a portion of that into their
address space if they want.
Ideally we should just do that in the base class, and
get the q35 subclass to do the right thing with aliases
to handle the dynamic resizing it wants. Then we wouldn't
need to track "size of cfg region" in the baseclass either.
> + memory_region_init(&s->io_mmio, OBJECT(s), "gpex_mmio", UINT64_MAX);
> + memory_region_init(&s->io_ioport, OBJECT(s), "gpex_ioport", 64 * 1024);
> +
> + sysbus_init_mmio(sbd, &pex->mmio);
> + sysbus_init_mmio(sbd, &s->io_mmio);
> + sysbus_init_mmio(sbd, &s->io_ioport);
> + for (i = 0; i < 4; i++) {
Can we at least have a constant rather than hardcoded 4s ?
(qdev property for number-of-irqs if you're really feeling
enthusiastic...)
> +
> +/****************************************************************************
> + * GPEX Root D0:F0
> + */
What does "D0:F0" mean here?
> +
> +static const VMStateDescription vmstate_gpex_root = {
> + .name = "gpex_root",
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .fields = (VMStateField[]) {
> + VMSTATE_PCI_DEVICE(parent_obj, GPEXRootState),
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> +static void gpex_root_class_init(ObjectClass *klass, void *data)
> +{
> + PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> + DeviceClass *dc = DEVICE_CLASS(klass);
> +
> + set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
> + dc->desc = "Host bridge";
We can be a bit less terse: "QEMU generic PCIe host bridge".
> + dc->vmsd = &vmstate_gpex_root;
> + k->vendor_id = PCI_VENDOR_ID_REDHAT;
> + k->device_id = PCI_DEVICE_ID_REDHAT_BRIDGE;
Pretty sure we shouldn't be reusing the PCI bridge IDs
for a host bridge. We should allocate ourselves another
device ID in the range...
> + k->revision = 0;
> + k->class_id = PCI_CLASS_BRIDGE_HOST;
> + /*
> + * PCI-facing part of the host bridge, not usable without the
> + * host-facing part, which can't be device_add'ed, yet.
> + */
> + dc->cannot_instantiate_with_device_add_yet = true;
> +}
> +
> +static const TypeInfo gpex_root_info = {
> + .name = TYPE_GPEX_ROOT_DEVICE,
> + .parent = TYPE_PCI_DEVICE,
> + .instance_size = sizeof(GPEXRootState),
> + .class_init = gpex_root_class_init,
> +};
> +
> +static void gpex_register(void)
> +{
> + type_register_static(&gpex_root_info);
> + type_register_static(&gpex_host_info);
> +}
> +
> +type_init(gpex_register);
We seem to prefer no trailing ';' on type_init by a ratio
of more than 10:1.
> diff --git a/include/hw/pci-host/gpex.h b/include/hw/pci-host/gpex.h
> new file mode 100644
> index 0000000..b465667
> --- /dev/null
> +++ b/include/hw/pci-host/gpex.h
> @@ -0,0 +1,54 @@
> +/*
> + * gpex.h
Would be nice to say "QEMU Generic PCI Express Bridge Emulation" here
(like the .c file's header does).
thanks
-- PMM
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/4] arm: Add PCIe host bridge in virt machine
2015-01-21 16:18 ` [Qemu-devel] [PATCH v2 3/4] arm: Add PCIe host bridge in virt machine Alexander Graf
2015-01-22 15:28 ` Claudio Fontana
@ 2015-01-27 16:52 ` Peter Maydell
2015-01-29 14:31 ` Alexander Graf
1 sibling, 1 reply; 24+ messages in thread
From: Peter Maydell @ 2015-01-27 16:52 UTC (permalink / raw)
To: Alexander Graf
Cc: Rob Herring, Michael S. Tsirkin, QEMU Developers, Ard Biesheuvel,
Claudio Fontana, Alvise Rigo, Stuart Yoder
On 21 January 2015 at 16:18, Alexander Graf <agraf@suse.de> wrote:
> Now that we have a working "generic" PCIe host bridge driver, we can plug
> it into ARMs virt machine to always have PCIe available to normal ARM VMs.
"ARM's".
>
> I've successfully managed to expose a Bochs VGA device, XHCI and an e1000
> into an AArch64 VM with this and they all lived happily ever after.
>
> Signed-off-by: Alexander Graf <agraf@suse.de>
> Tested-by: Claudio Fontana <claudio.fontana@huawei.com>
>
> ---
>
> Linux 3.19 only supports the generic PCIe host bridge driver for 32bit ARM
> systems. If you want to use it with AArch64 guests, please apply the following
> patch or wait until upstream cleaned up the code properly:
>
> http://csgraf.de/agraf/pci/pci-3.19.patch
>
> v1 -> v2:
>
> - Add define for pci range types
> - Remove mmio_window_size
> - Use 4 PCI INTX IRQ lines
> ---
> default-configs/arm-softmmu.mak | 2 +
> hw/arm/virt.c | 112 ++++++++++++++++++++++++++++++++++++++--
> include/sysemu/device_tree.h | 9 ++++
> 3 files changed, 118 insertions(+), 5 deletions(-)
>
> diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
> index f3513fa..7671ee2 100644
> --- a/default-configs/arm-softmmu.mak
> +++ b/default-configs/arm-softmmu.mak
> @@ -82,6 +82,8 @@ CONFIG_ZYNQ=y
> CONFIG_VERSATILE_PCI=y
> CONFIG_VERSATILE_I2C=y
>
> +CONFIG_PCI_GENERIC=y
> +
> CONFIG_SDHCI=y
> CONFIG_INTEGRATOR_DEBUG=y
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 2353440..a727b4e 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -42,6 +42,7 @@
> #include "exec/address-spaces.h"
> #include "qemu/bitops.h"
> #include "qemu/error-report.h"
> +#include "hw/pci-host/gpex.h"
>
> #define NUM_VIRTIO_TRANSPORTS 32
>
> @@ -69,6 +70,7 @@ enum {
> VIRT_MMIO,
> VIRT_RTC,
> VIRT_FW_CFG,
> + VIRT_PCIE,
> };
>
> typedef struct MemMapEntry {
> @@ -129,13 +131,14 @@ static const MemMapEntry a15memmap[] = {
> [VIRT_FW_CFG] = { 0x09020000, 0x0000000a },
> [VIRT_MMIO] = { 0x0a000000, 0x00000200 },
> /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */
> - /* 0x10000000 .. 0x40000000 reserved for PCI */
> + [VIRT_PCIE] = { 0x10000000, 0x30000000 },
Might be nice to have a comment about how this is split up between
MMIO, IO and config space.
> [VIRT_MEM] = { 0x40000000, 30ULL * 1024 * 1024 * 1024 },
> };
>
> static const int a15irqmap[] = {
> [VIRT_UART] = 1,
> [VIRT_RTC] = 2,
> + [VIRT_PCIE] = 3, /* ... to 6 */
> [VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */
> };
>
> @@ -312,7 +315,7 @@ static void fdt_add_cpu_nodes(const VirtBoardInfo *vbi)
> }
> }
>
> -static void fdt_add_gic_node(const VirtBoardInfo *vbi)
> +static uint32_t fdt_add_gic_node(const VirtBoardInfo *vbi)
> {
> uint32_t gic_phandle;
>
> @@ -331,9 +334,11 @@ static void fdt_add_gic_node(const VirtBoardInfo *vbi)
> 2, vbi->memmap[VIRT_GIC_CPU].base,
> 2, vbi->memmap[VIRT_GIC_CPU].size);
> qemu_fdt_setprop_cell(vbi->fdt, "/intc", "phandle", gic_phandle);
> +
> + return gic_phandle;
> }
>
> -static void create_gic(const VirtBoardInfo *vbi, qemu_irq *pic)
> +static uint32_t create_gic(const VirtBoardInfo *vbi, qemu_irq *pic)
> {
> /* We create a standalone GIC v2 */
> DeviceState *gicdev;
> @@ -380,7 +385,7 @@ static void create_gic(const VirtBoardInfo *vbi, qemu_irq *pic)
> pic[i] = qdev_get_gpio_in(gicdev, i);
> }
>
> - fdt_add_gic_node(vbi);
> + return fdt_add_gic_node(vbi);
> }
>
> static void create_uart(const VirtBoardInfo *vbi, qemu_irq *pic)
> @@ -556,6 +561,100 @@ static void create_fw_cfg(const VirtBoardInfo *vbi)
> g_free(nodename);
> }
>
> +static void create_pcie_irq_map(const VirtBoardInfo *vbi, uint32_t gic_phandle,
> + int first_irq, const char *nodename)
> +{
> + int devfn, pin;
> + uint32_t full_irq_map[4 * 4 * 8] = { 0 };
> + uint32_t *irq_map = full_irq_map;
> +
> + for (devfn = 0; devfn <= 0x18; devfn+=0x8) {
> + for (pin = 0; pin < 4; pin++) {
> + int irq_type = GIC_FDT_IRQ_TYPE_SPI;
> + int irq_nr = first_irq + ((pin + PCI_SLOT(devfn)) % PCI_NUM_PINS);
> + int irq_level = GIC_FDT_IRQ_FLAGS_LEVEL_HI;
> + int i;
> +
> + uint32_t map[] = {
> + devfn << 8, 0, 0, /* devfn */
> + pin + 1, /* PCI pin */
> + gic_phandle, irq_type, irq_nr, irq_level }; /* GIC irq */
> +
> + /* Convert map to big endian */
> + for (i = 0; i < 8; i++) {
> + irq_map[i] = cpu_to_be32(map[i]);
> + }
> + irq_map += 8;
> + }
> + }
> +
> + qemu_fdt_setprop(vbi->fdt, nodename, "interrupt-map",
> + full_irq_map, sizeof(full_irq_map));
> +}
> +
> +static void create_pcie(const VirtBoardInfo *vbi, qemu_irq *pic,
> + uint32_t gic_phandle)
> +{
> + hwaddr base = vbi->memmap[VIRT_PCIE].base;
> + hwaddr size = vbi->memmap[VIRT_PCIE].size;
> + hwaddr size_ioport = 64 * 1024;
> + hwaddr size_ecam = PCIE_MMCFG_SIZE_MIN;
See remarks in previous patch about wanting more than the minimum
ECAM space.
> + hwaddr size_mmio = size - size_ecam - size_ioport;
> + hwaddr base_mmio = base;
> + hwaddr base_ioport = base_mmio + size_mmio;
> + hwaddr base_ecam = base_ioport + size_ioport;
PCIe spec says the ECAM window has to be aligned to its size
(ie if it's 1MB in size it has to be 1MB aligned, if it's
256MB aligned it has to be 256MB aligned). I think this is
dumb, but it's what the spec says...
> + int irq = vbi->irqmap[VIRT_PCIE];
> + MemoryRegion *mmio_alias;
> + MemoryRegion *mmio_reg;
> + DeviceState *dev;
> + char *nodename;
> + int i;
> +
> + dev = qdev_create(NULL, TYPE_GPEX_HOST);
> + qdev_init_nofail(dev);
> +
> + sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base_ecam);
> + sysbus_mmio_map(SYS_BUS_DEVICE(dev), 2, base_ioport);
> +
> + /* Map the MMIO window at the same spot in bus and cpu layouts */
> + mmio_alias = g_new0(MemoryRegion, 1);
> + mmio_reg = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 1);
> + memory_region_init_alias(mmio_alias, OBJECT(dev), "pcie-mmio",
> + mmio_reg, base_mmio, size_mmio);
> + memory_region_add_subregion(get_system_memory(), base_mmio, mmio_alias);
The comment claims to be mapping the MMIO window twice (in the
system memory space and in the PCI mmio address space) but the
code only seems to be mapping something into system memory space?
> +
> + for (i = 0; i < 4; i++) {
> + sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, pic[irq + i]);
> + }
> +
> + nodename = g_strdup_printf("/pcie@%" PRIx64, base);
> + qemu_fdt_add_subnode(vbi->fdt, nodename);
> + qemu_fdt_setprop_string(vbi->fdt, nodename,
> + "compatible", "pci-host-ecam-generic");
> + qemu_fdt_setprop_string(vbi->fdt, nodename, "device_type", "pci");
> + qemu_fdt_setprop_cell(vbi->fdt, nodename, "#address-cells", 3);
> + qemu_fdt_setprop_cell(vbi->fdt, nodename, "#size-cells", 2);
> + qemu_fdt_setprop_cells(vbi->fdt, nodename, "bus-range", 0, 1);
> +
> + qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "reg",
> + 2, base_ecam, 2, size_ecam);
> + qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "ranges",
> + 1, FDT_PCI_RANGE_IOPORT, 2, 0,
> + 2, base_ioport, 2, size_ioport,
> +
> + 1, FDT_PCI_RANGE_MMIO, 2, base_mmio,
> + 2, base_mmio, 2, size_mmio);
> +
> + qemu_fdt_setprop_cell(vbi->fdt, nodename, "#interrupt-cells", 1);
> + create_pcie_irq_map(vbi, gic_phandle, irq, nodename);
> +
> + qemu_fdt_setprop_cells(vbi->fdt, nodename, "interrupt-map-mask",
> + 0x1800, 0, 0, /* devfn (PCI_SLOT(3)) */
> + 0x7 /* PCI irq */);
I think at least the interrupt-map-mask and maybe also the
#interrupt-cells setprop calls should go inside the
create_pcie_irq_map() function. In particular the interrupt-map
and interrupt-map-mask are meaningless unless interpreted together
so they should go in the same function.
> +
> + g_free(nodename);
> +}
> +
> static void *machvirt_dtb(const struct arm_boot_info *binfo, int *fdt_size)
> {
> const VirtBoardInfo *board = (const VirtBoardInfo *)binfo;
> @@ -573,6 +672,7 @@ static void machvirt_init(MachineState *machine)
> MemoryRegion *ram = g_new(MemoryRegion, 1);
> const char *cpu_model = machine->cpu_model;
> VirtBoardInfo *vbi;
> + uint32_t gic_phandle;
>
> if (!cpu_model) {
> cpu_model = "cortex-a15";
> @@ -634,12 +734,14 @@ static void machvirt_init(MachineState *machine)
>
> create_flash(vbi);
>
> - create_gic(vbi, pic);
> + gic_phandle = create_gic(vbi, pic);
>
> create_uart(vbi, pic);
>
> create_rtc(vbi, pic);
>
> + create_pcie(vbi, pic, gic_phandle);
> +
> /* Create mmio transports, so the user can create virtio backends
> * (which will be automatically plugged in to the transports). If
> * no backend is created the transport will just sit harmlessly idle.
> diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h
> index 899f05c..979169b 100644
> --- a/include/sysemu/device_tree.h
> +++ b/include/sysemu/device_tree.h
> @@ -110,4 +110,13 @@ int qemu_fdt_setprop_sized_cells_from_array(void *fdt,
> qdt_tmp); \
> })
>
> +#define FDT_PCI_RANGE_RELOCATABLE 0x80000000
> +#define FDT_PCI_RANGE_PREFETCHABLE 0x40000000
> +#define FDT_PCI_RANGE_ALIASED 0x20000000
> +#define FDT_PCI_RANGE_TYPE 0x03000000
> +#define FDT_PCI_RANGE_MMIO_64BIT 0x03000000
I had to go dig up the spec to figure out that it wasn't an error that
these two had the same values. Calling the first _TYPE_MASK might help?
> +#define FDT_PCI_RANGE_MMIO 0x02000000
> +#define FDT_PCI_RANGE_IOPORT 0x01000000
> +#define FDT_PCI_RANGE_CONFIG 0x00000000
thanks
-- PMM
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/4] pci: Add generic PCIe host bridge
2015-01-27 15:31 ` Peter Maydell
@ 2015-01-29 13:59 ` Alexander Graf
2015-01-29 14:25 ` Peter Maydell
0 siblings, 1 reply; 24+ messages in thread
From: Alexander Graf @ 2015-01-29 13:59 UTC (permalink / raw)
To: Peter Maydell
Cc: Rob Herring, Michael S. Tsirkin, QEMU Developers, Ard Biesheuvel,
Claudio Fontana, Alvise Rigo, Stuart Yoder
On 27.01.15 16:31, Peter Maydell wrote:
> On 21 January 2015 at 16:18, Alexander Graf <agraf@suse.de> wrote:
>> With simple exposure of MMFG, ioport window, mmio window and an IRQ line we
>
> Four :-)
>
>> can successfully create a workable PCIe host bridge that can be mapped anywhere
>> and only needs to get described to the OS using whatever means it likes.
>>
>> This patch implements such a "generic" host bridge. It only supports a single
>> legacy IRQ line so far. MSIs need to be handled external to the host bridge.
>>
>> This device is particularly useful for the "pci-host-ecam-generic" driver in
>> Linux.
>>
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>> Reviewed-by: Claudio Fontana <claudio.fontana@huawei.com>
>> Tested-by: Claudio Fontana <claudio.fontana@huawei.com>
>
> Checkpatch complains about a few over-80-chars lines;
> the URL is an unavoidable one but could you fold the others,
> please?
Sure
>
>> +static void gpex_host_realize(DeviceState *dev, Error **errp)
>> +{
>> + PCIHostState *pci = PCI_HOST_BRIDGE(dev);
>> + GPEXHost *s = GPEX_HOST(dev);
>> + SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>> + PCIExpressHost *pex = PCIE_HOST_BRIDGE(dev);
>> + int i;
>> +
>> + pcie_host_mmcfg_init(pex, PCIE_MMCFG_SIZE_MIN);
>
> So this gives us 1MB of ECAM (config) space. That means
> we can't specify a target bus, so we're restricted to
> the 31 devices directly attached to the root complex.
> Among other things, this will mean we can't do PCIe
> hotplug. I think we should make this at least a bit bigger,
> even if we don't go up to the full 256MB.
>
> Probably the best thing for the gpex device itself is
> to make the config space be the spec maximum (PCIE_MMCFG_SIZE_MAX)
> and let the user map only a portion of that into their
> address space if they want.
Sounds like a good plan. Will do.
>
> Ideally we should just do that in the base class, and
> get the q35 subclass to do the right thing with aliases
> to handle the dynamic resizing it wants. Then we wouldn't
> need to track "size of cfg region" in the baseclass either.
>
>> + memory_region_init(&s->io_mmio, OBJECT(s), "gpex_mmio", UINT64_MAX);
>> + memory_region_init(&s->io_ioport, OBJECT(s), "gpex_ioport", 64 * 1024);
>> +
>> + sysbus_init_mmio(sbd, &pex->mmio);
>> + sysbus_init_mmio(sbd, &s->io_mmio);
>> + sysbus_init_mmio(sbd, &s->io_ioport);
>> + for (i = 0; i < 4; i++) {
>
> Can we at least have a constant rather than hardcoded 4s ?
> (qdev property for number-of-irqs if you're really feeling
> enthusiastic...)
Sure.
>
>> +
>> +/****************************************************************************
>> + * GPEX Root D0:F0
>> + */
>
> What does "D0:F0" mean here?
Device 0 Function 0. It's pretty common PCI speech ;).
>
>> +
>> +static const VMStateDescription vmstate_gpex_root = {
>> + .name = "gpex_root",
>> + .version_id = 1,
>> + .minimum_version_id = 1,
>> + .fields = (VMStateField[]) {
>> + VMSTATE_PCI_DEVICE(parent_obj, GPEXRootState),
>> + VMSTATE_END_OF_LIST()
>> + }
>> +};
>> +
>> +static void gpex_root_class_init(ObjectClass *klass, void *data)
>> +{
>> + PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>> + DeviceClass *dc = DEVICE_CLASS(klass);
>> +
>> + set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
>> + dc->desc = "Host bridge";
>
> We can be a bit less terse: "QEMU generic PCIe host bridge".
>
>> + dc->vmsd = &vmstate_gpex_root;
>> + k->vendor_id = PCI_VENDOR_ID_REDHAT;
>> + k->device_id = PCI_DEVICE_ID_REDHAT_BRIDGE;
>
> Pretty sure we shouldn't be reusing the PCI bridge IDs
> for a host bridge. We should allocate ourselves another
> device ID in the range...
Ah, right, I forgot about that. Unfortunately I can't reserve PCI IDs in
the Red Hat PCI range.
Michael, what's the process here?
>
>> + k->revision = 0;
>> + k->class_id = PCI_CLASS_BRIDGE_HOST;
>> + /*
>> + * PCI-facing part of the host bridge, not usable without the
>> + * host-facing part, which can't be device_add'ed, yet.
>> + */
>> + dc->cannot_instantiate_with_device_add_yet = true;
>> +}
>> +
>> +static const TypeInfo gpex_root_info = {
>> + .name = TYPE_GPEX_ROOT_DEVICE,
>> + .parent = TYPE_PCI_DEVICE,
>> + .instance_size = sizeof(GPEXRootState),
>> + .class_init = gpex_root_class_init,
>> +};
>> +
>> +static void gpex_register(void)
>> +{
>> + type_register_static(&gpex_root_info);
>> + type_register_static(&gpex_host_info);
>> +}
>> +
>> +type_init(gpex_register);
>
> We seem to prefer no trailing ';' on type_init by a ratio
> of more than 10:1.
Ok
>
>> diff --git a/include/hw/pci-host/gpex.h b/include/hw/pci-host/gpex.h
>> new file mode 100644
>> index 0000000..b465667
>> --- /dev/null
>> +++ b/include/hw/pci-host/gpex.h
>> @@ -0,0 +1,54 @@
>> +/*
>> + * gpex.h
>
> Would be nice to say "QEMU Generic PCI Express Bridge Emulation" here
> (like the .c file's header does).
Ok
Alex
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/4] pci: Add generic PCIe host bridge
2015-01-29 13:59 ` Alexander Graf
@ 2015-01-29 14:25 ` Peter Maydell
2015-01-30 10:25 ` Paolo Bonzini
0 siblings, 1 reply; 24+ messages in thread
From: Peter Maydell @ 2015-01-29 14:25 UTC (permalink / raw)
To: Alexander Graf
Cc: Rob Herring, Michael S. Tsirkin, QEMU Developers, Ard Biesheuvel,
Claudio Fontana, Alvise Rigo, Stuart Yoder, Paolo Bonzini
On 29 January 2015 at 13:59, Alexander Graf <agraf@suse.de> wrote:
> On 27.01.15 16:31, Peter Maydell wrote:
>> On 21 January 2015 at 16:18, Alexander Graf <agraf@suse.de> wrote:
>>> + dc->vmsd = &vmstate_gpex_root;
>>> + k->vendor_id = PCI_VENDOR_ID_REDHAT;
>>> + k->device_id = PCI_DEVICE_ID_REDHAT_BRIDGE;
>>
>> Pretty sure we shouldn't be reusing the PCI bridge IDs
>> for a host bridge. We should allocate ourselves another
>> device ID in the range...
>
> Ah, right, I forgot about that. Unfortunately I can't reserve PCI IDs in
> the Red Hat PCI range.
>
> Michael, what's the process here?
Last time this came up I think Paolo said the official registration
was getting a patch into QEMU master that added a line to the
list in pci.h :-) [ie QEMU is the master source for these IDs]
-- PMM
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/4] arm: Add PCIe host bridge in virt machine
2015-01-27 16:52 ` Peter Maydell
@ 2015-01-29 14:31 ` Alexander Graf
2015-01-29 14:34 ` Peter Maydell
0 siblings, 1 reply; 24+ messages in thread
From: Alexander Graf @ 2015-01-29 14:31 UTC (permalink / raw)
To: Peter Maydell
Cc: Rob Herring, Michael S. Tsirkin, QEMU Developers, Ard Biesheuvel,
Claudio Fontana, Alvise Rigo, Stuart Yoder
On 27.01.15 17:52, Peter Maydell wrote:
> On 21 January 2015 at 16:18, Alexander Graf <agraf@suse.de> wrote:
>> Now that we have a working "generic" PCIe host bridge driver, we can plug
>> it into ARMs virt machine to always have PCIe available to normal ARM VMs.
>
> "ARM's".
>
>>
>> I've successfully managed to expose a Bochs VGA device, XHCI and an e1000
>> into an AArch64 VM with this and they all lived happily ever after.
>>
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>> Tested-by: Claudio Fontana <claudio.fontana@huawei.com>
>>
>> ---
>>
>> Linux 3.19 only supports the generic PCIe host bridge driver for 32bit ARM
>> systems. If you want to use it with AArch64 guests, please apply the following
>> patch or wait until upstream cleaned up the code properly:
>>
>> http://csgraf.de/agraf/pci/pci-3.19.patch
>>
>> v1 -> v2:
>>
>> - Add define for pci range types
>> - Remove mmio_window_size
>> - Use 4 PCI INTX IRQ lines
>> ---
>> default-configs/arm-softmmu.mak | 2 +
>> hw/arm/virt.c | 112 ++++++++++++++++++++++++++++++++++++++--
>> include/sysemu/device_tree.h | 9 ++++
>> 3 files changed, 118 insertions(+), 5 deletions(-)
>>
>> diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
>> index f3513fa..7671ee2 100644
>> --- a/default-configs/arm-softmmu.mak
>> +++ b/default-configs/arm-softmmu.mak
>> @@ -82,6 +82,8 @@ CONFIG_ZYNQ=y
>> CONFIG_VERSATILE_PCI=y
>> CONFIG_VERSATILE_I2C=y
>>
>> +CONFIG_PCI_GENERIC=y
>> +
>> CONFIG_SDHCI=y
>> CONFIG_INTEGRATOR_DEBUG=y
>>
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index 2353440..a727b4e 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -42,6 +42,7 @@
>> #include "exec/address-spaces.h"
>> #include "qemu/bitops.h"
>> #include "qemu/error-report.h"
>> +#include "hw/pci-host/gpex.h"
>>
>> #define NUM_VIRTIO_TRANSPORTS 32
>>
>> @@ -69,6 +70,7 @@ enum {
>> VIRT_MMIO,
>> VIRT_RTC,
>> VIRT_FW_CFG,
>> + VIRT_PCIE,
>> };
>>
>> typedef struct MemMapEntry {
>> @@ -129,13 +131,14 @@ static const MemMapEntry a15memmap[] = {
>> [VIRT_FW_CFG] = { 0x09020000, 0x0000000a },
>> [VIRT_MMIO] = { 0x0a000000, 0x00000200 },
>> /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */
>> - /* 0x10000000 .. 0x40000000 reserved for PCI */
>> + [VIRT_PCIE] = { 0x10000000, 0x30000000 },
>
> Might be nice to have a comment about how this is split up between
> MMIO, IO and config space.
I'd prefer to keep the details of that in the allocation code so that we
don't potentially have 2 places that diverge eventually. But I'll
document the allocation code a bit nicer and add a small overview split
here.
>
>> [VIRT_MEM] = { 0x40000000, 30ULL * 1024 * 1024 * 1024 },
>> };
>>
>> static const int a15irqmap[] = {
>> [VIRT_UART] = 1,
>> [VIRT_RTC] = 2,
>> + [VIRT_PCIE] = 3, /* ... to 6 */
>> [VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */
>> };
>>
>> @@ -312,7 +315,7 @@ static void fdt_add_cpu_nodes(const VirtBoardInfo *vbi)
>> }
>> }
>>
>> -static void fdt_add_gic_node(const VirtBoardInfo *vbi)
>> +static uint32_t fdt_add_gic_node(const VirtBoardInfo *vbi)
>> {
>> uint32_t gic_phandle;
>>
>> @@ -331,9 +334,11 @@ static void fdt_add_gic_node(const VirtBoardInfo *vbi)
>> 2, vbi->memmap[VIRT_GIC_CPU].base,
>> 2, vbi->memmap[VIRT_GIC_CPU].size);
>> qemu_fdt_setprop_cell(vbi->fdt, "/intc", "phandle", gic_phandle);
>> +
>> + return gic_phandle;
>> }
>>
>> -static void create_gic(const VirtBoardInfo *vbi, qemu_irq *pic)
>> +static uint32_t create_gic(const VirtBoardInfo *vbi, qemu_irq *pic)
>> {
>> /* We create a standalone GIC v2 */
>> DeviceState *gicdev;
>> @@ -380,7 +385,7 @@ static void create_gic(const VirtBoardInfo *vbi, qemu_irq *pic)
>> pic[i] = qdev_get_gpio_in(gicdev, i);
>> }
>>
>> - fdt_add_gic_node(vbi);
>> + return fdt_add_gic_node(vbi);
>> }
>>
>> static void create_uart(const VirtBoardInfo *vbi, qemu_irq *pic)
>> @@ -556,6 +561,100 @@ static void create_fw_cfg(const VirtBoardInfo *vbi)
>> g_free(nodename);
>> }
>>
>> +static void create_pcie_irq_map(const VirtBoardInfo *vbi, uint32_t gic_phandle,
>> + int first_irq, const char *nodename)
>> +{
>> + int devfn, pin;
>> + uint32_t full_irq_map[4 * 4 * 8] = { 0 };
>> + uint32_t *irq_map = full_irq_map;
>> +
>> + for (devfn = 0; devfn <= 0x18; devfn+=0x8) {
>> + for (pin = 0; pin < 4; pin++) {
>> + int irq_type = GIC_FDT_IRQ_TYPE_SPI;
>> + int irq_nr = first_irq + ((pin + PCI_SLOT(devfn)) % PCI_NUM_PINS);
>> + int irq_level = GIC_FDT_IRQ_FLAGS_LEVEL_HI;
>> + int i;
>> +
>> + uint32_t map[] = {
>> + devfn << 8, 0, 0, /* devfn */
>> + pin + 1, /* PCI pin */
>> + gic_phandle, irq_type, irq_nr, irq_level }; /* GIC irq */
>> +
>> + /* Convert map to big endian */
>> + for (i = 0; i < 8; i++) {
>> + irq_map[i] = cpu_to_be32(map[i]);
>> + }
>> + irq_map += 8;
>> + }
>> + }
>> +
>> + qemu_fdt_setprop(vbi->fdt, nodename, "interrupt-map",
>> + full_irq_map, sizeof(full_irq_map));
>> +}
>> +
>> +static void create_pcie(const VirtBoardInfo *vbi, qemu_irq *pic,
>> + uint32_t gic_phandle)
>> +{
>> + hwaddr base = vbi->memmap[VIRT_PCIE].base;
>> + hwaddr size = vbi->memmap[VIRT_PCIE].size;
>> + hwaddr size_ioport = 64 * 1024;
>> + hwaddr size_ecam = PCIE_MMCFG_SIZE_MIN;
>
> See remarks in previous patch about wanting more than the minimum
> ECAM space.
>
>> + hwaddr size_mmio = size - size_ecam - size_ioport;
>> + hwaddr base_mmio = base;
>> + hwaddr base_ioport = base_mmio + size_mmio;
>> + hwaddr base_ecam = base_ioport + size_ioport;
>
> PCIe spec says the ECAM window has to be aligned to its size
> (ie if it's 1MB in size it has to be 1MB aligned, if it's
> 256MB aligned it has to be 256MB aligned). I think this is
> dumb, but it's what the spec says...
No problem, fortunately we have a good number of nice macros for this ;)
>
>> + int irq = vbi->irqmap[VIRT_PCIE];
>> + MemoryRegion *mmio_alias;
>> + MemoryRegion *mmio_reg;
>> + DeviceState *dev;
>> + char *nodename;
>> + int i;
>> +
>> + dev = qdev_create(NULL, TYPE_GPEX_HOST);
>> + qdev_init_nofail(dev);
>> +
>> + sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base_ecam);
>> + sysbus_mmio_map(SYS_BUS_DEVICE(dev), 2, base_ioport);
>> +
>> + /* Map the MMIO window at the same spot in bus and cpu layouts */
>> + mmio_alias = g_new0(MemoryRegion, 1);
>> + mmio_reg = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 1);
>> + memory_region_init_alias(mmio_alias, OBJECT(dev), "pcie-mmio",
>> + mmio_reg, base_mmio, size_mmio);
>> + memory_region_add_subregion(get_system_memory(), base_mmio, mmio_alias);
>
> The comment claims to be mapping the MMIO window twice (in the
> system memory space and in the PCI mmio address space) but the
> code only seems to be mapping something into system memory space?
The comment claims to map it at the same spot. It means the offset in
system memory is the same offset as the one in the mmio window that gets
exported by the PHB.
The PHB exports a UINT64_MAX MMIO region as id 1. This is basically a
full window into the PCI address space. What we do here is to map a 1:1
window between CPU address space and PCI address space.
>
>> +
>> + for (i = 0; i < 4; i++) {
>> + sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, pic[irq + i]);
>> + }
>> +
>> + nodename = g_strdup_printf("/pcie@%" PRIx64, base);
>> + qemu_fdt_add_subnode(vbi->fdt, nodename);
>> + qemu_fdt_setprop_string(vbi->fdt, nodename,
>> + "compatible", "pci-host-ecam-generic");
>> + qemu_fdt_setprop_string(vbi->fdt, nodename, "device_type", "pci");
>> + qemu_fdt_setprop_cell(vbi->fdt, nodename, "#address-cells", 3);
>> + qemu_fdt_setprop_cell(vbi->fdt, nodename, "#size-cells", 2);
>> + qemu_fdt_setprop_cells(vbi->fdt, nodename, "bus-range", 0, 1);
>> +
>> + qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "reg",
>> + 2, base_ecam, 2, size_ecam);
>> + qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "ranges",
>> + 1, FDT_PCI_RANGE_IOPORT, 2, 0,
>> + 2, base_ioport, 2, size_ioport,
>> +
>> + 1, FDT_PCI_RANGE_MMIO, 2, base_mmio,
>> + 2, base_mmio, 2, size_mmio);
>> +
>> + qemu_fdt_setprop_cell(vbi->fdt, nodename, "#interrupt-cells", 1);
>> + create_pcie_irq_map(vbi, gic_phandle, irq, nodename);
>> +
>> + qemu_fdt_setprop_cells(vbi->fdt, nodename, "interrupt-map-mask",
>> + 0x1800, 0, 0, /* devfn (PCI_SLOT(3)) */
>> + 0x7 /* PCI irq */);
>
> I think at least the interrupt-map-mask and maybe also the
> #interrupt-cells setprop calls should go inside the
> create_pcie_irq_map() function. In particular the interrupt-map
> and interrupt-map-mask are meaningless unless interpreted together
> so they should go in the same function.
Sure, works for me.
>
>> +
>> + g_free(nodename);
>> +}
>> +
>> static void *machvirt_dtb(const struct arm_boot_info *binfo, int *fdt_size)
>> {
>> const VirtBoardInfo *board = (const VirtBoardInfo *)binfo;
>> @@ -573,6 +672,7 @@ static void machvirt_init(MachineState *machine)
>> MemoryRegion *ram = g_new(MemoryRegion, 1);
>> const char *cpu_model = machine->cpu_model;
>> VirtBoardInfo *vbi;
>> + uint32_t gic_phandle;
>>
>> if (!cpu_model) {
>> cpu_model = "cortex-a15";
>> @@ -634,12 +734,14 @@ static void machvirt_init(MachineState *machine)
>>
>> create_flash(vbi);
>>
>> - create_gic(vbi, pic);
>> + gic_phandle = create_gic(vbi, pic);
>>
>> create_uart(vbi, pic);
>>
>> create_rtc(vbi, pic);
>>
>> + create_pcie(vbi, pic, gic_phandle);
>> +
>> /* Create mmio transports, so the user can create virtio backends
>> * (which will be automatically plugged in to the transports). If
>> * no backend is created the transport will just sit harmlessly idle.
>> diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h
>> index 899f05c..979169b 100644
>> --- a/include/sysemu/device_tree.h
>> +++ b/include/sysemu/device_tree.h
>> @@ -110,4 +110,13 @@ int qemu_fdt_setprop_sized_cells_from_array(void *fdt,
>> qdt_tmp); \
>> })
>>
>> +#define FDT_PCI_RANGE_RELOCATABLE 0x80000000
>> +#define FDT_PCI_RANGE_PREFETCHABLE 0x40000000
>> +#define FDT_PCI_RANGE_ALIASED 0x20000000
>> +#define FDT_PCI_RANGE_TYPE 0x03000000
>> +#define FDT_PCI_RANGE_MMIO_64BIT 0x03000000
>
> I had to go dig up the spec to figure out that it wasn't an error that
> these two had the same values. Calling the first _TYPE_MASK might help?
Probably :)
Alex
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/4] arm: Add PCIe host bridge in virt machine
2015-01-29 14:31 ` Alexander Graf
@ 2015-01-29 14:34 ` Peter Maydell
2015-01-29 14:37 ` Alexander Graf
0 siblings, 1 reply; 24+ messages in thread
From: Peter Maydell @ 2015-01-29 14:34 UTC (permalink / raw)
To: Alexander Graf
Cc: Rob Herring, Michael S. Tsirkin, QEMU Developers, Ard Biesheuvel,
Claudio Fontana, Alvise Rigo, Stuart Yoder
On 29 January 2015 at 14:31, Alexander Graf <agraf@suse.de> wrote:
>
>
> On 27.01.15 17:52, Peter Maydell wrote:
>> On 21 January 2015 at 16:18, Alexander Graf <agraf@suse.de> wrote:
>>> + /* Map the MMIO window at the same spot in bus and cpu layouts */
>>> + mmio_alias = g_new0(MemoryRegion, 1);
>>> + mmio_reg = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 1);
>>> + memory_region_init_alias(mmio_alias, OBJECT(dev), "pcie-mmio",
>>> + mmio_reg, base_mmio, size_mmio);
>>> + memory_region_add_subregion(get_system_memory(), base_mmio, mmio_alias);
>>
>> The comment claims to be mapping the MMIO window twice (in the
>> system memory space and in the PCI mmio address space) but the
>> code only seems to be mapping something into system memory space?
>
> The comment claims to map it at the same spot. It means the offset in
> system memory is the same offset as the one in the mmio window that gets
> exported by the PHB.
>
> The PHB exports a UINT64_MAX MMIO region as id 1. This is basically a
> full window into the PCI address space. What we do here is to map a 1:1
> window between CPU address space and PCI address space.
I kind of see, but isn't this just a window from CPU address
space into PCI address space, not vice-versa?
DMA by PCI devices bus-mastering into system memory must be
being set up elsewhere, I think.
-- PMM
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/4] arm: Add PCIe host bridge in virt machine
2015-01-29 14:34 ` Peter Maydell
@ 2015-01-29 14:37 ` Alexander Graf
2015-01-29 14:45 ` Peter Maydell
0 siblings, 1 reply; 24+ messages in thread
From: Alexander Graf @ 2015-01-29 14:37 UTC (permalink / raw)
To: Peter Maydell
Cc: Rob Herring, Michael S. Tsirkin, QEMU Developers, Ard Biesheuvel,
Claudio Fontana, Alvise Rigo, Stuart Yoder
On 29.01.15 15:34, Peter Maydell wrote:
> On 29 January 2015 at 14:31, Alexander Graf <agraf@suse.de> wrote:
>>
>>
>> On 27.01.15 17:52, Peter Maydell wrote:
>>> On 21 January 2015 at 16:18, Alexander Graf <agraf@suse.de> wrote:
>>>> + /* Map the MMIO window at the same spot in bus and cpu layouts */
>>>> + mmio_alias = g_new0(MemoryRegion, 1);
>>>> + mmio_reg = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 1);
>>>> + memory_region_init_alias(mmio_alias, OBJECT(dev), "pcie-mmio",
>>>> + mmio_reg, base_mmio, size_mmio);
>>>> + memory_region_add_subregion(get_system_memory(), base_mmio, mmio_alias);
>>>
>>> The comment claims to be mapping the MMIO window twice (in the
>>> system memory space and in the PCI mmio address space) but the
>>> code only seems to be mapping something into system memory space?
>>
>> The comment claims to map it at the same spot. It means the offset in
>> system memory is the same offset as the one in the mmio window that gets
>> exported by the PHB.
>>
>> The PHB exports a UINT64_MAX MMIO region as id 1. This is basically a
>> full window into the PCI address space. What we do here is to map a 1:1
>> window between CPU address space and PCI address space.
>
> I kind of see, but isn't this just a window from CPU address
> space into PCI address space, not vice-versa?
Yup, exactly. But PCI devices need to map themselves somewhere into the
PCI address space. So if I configure a BAR to live at 0x10000000, it
should also show up at 0x10000000 when accessed from the CPU. That's
what the mapping above is about.
> DMA by PCI devices bus-mastering into system memory must be
> being set up elsewhere, I think.
Yes, that's a different mechanism that's not implemented yet for GPEX
:). On ARM this would happen via SMMU emulation.
Alex
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/4] arm: Add PCIe host bridge in virt machine
2015-01-29 14:37 ` Alexander Graf
@ 2015-01-29 14:45 ` Peter Maydell
2015-01-29 14:49 ` Alexander Graf
0 siblings, 1 reply; 24+ messages in thread
From: Peter Maydell @ 2015-01-29 14:45 UTC (permalink / raw)
To: Alexander Graf
Cc: Rob Herring, Michael S. Tsirkin, QEMU Developers, Ard Biesheuvel,
Claudio Fontana, Alvise Rigo, Stuart Yoder
On 29 January 2015 at 14:37, Alexander Graf <agraf@suse.de> wrote:
> On 29.01.15 15:34, Peter Maydell wrote:
>> I kind of see, but isn't this just a window from CPU address
>> space into PCI address space, not vice-versa?
>
> Yup, exactly. But PCI devices need to map themselves somewhere into the
> PCI address space. So if I configure a BAR to live at 0x10000000, it
> should also show up at 0x10000000 when accessed from the CPU. That's
> what the mapping above is about.
No, it doesn't have to. It's a choice to make the mapping
be such that the system address for a BAR matches the address
in PCI memory space, not a requirement. I agree it's a
sensible choice, though.
But as I say, this code is setting up one mapping (the
system address -> PCI space mapping), not two.
>> DMA by PCI devices bus-mastering into system memory must be
>> being set up elsewhere, I think.
>
> Yes, that's a different mechanism that's not implemented yet for GPEX
> :).
We can't not implement DMA, it would break lots of the usual
PCI devices people want to use. In fact I thought the PCI
core code implemented a default of "DMA by PCI devices goes
to the system address space" if you didn't specifically
set up something else by calling pci_setup_iommu(). This is
definitely how it works for plain PCI host bridges, are
PCIe bridges different?
> On ARM this would happen via SMMU emulation.
There's no requirement for a PCI host controller to be
sat behind an SMMU -- that's a system design choice. We
don't need to implement the SMMU yet (or perhaps ever?);
we definitely need to support PCI DMA.
-- PMM
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/4] arm: Add PCIe host bridge in virt machine
2015-01-29 14:45 ` Peter Maydell
@ 2015-01-29 14:49 ` Alexander Graf
0 siblings, 0 replies; 24+ messages in thread
From: Alexander Graf @ 2015-01-29 14:49 UTC (permalink / raw)
To: Peter Maydell
Cc: Rob Herring, Michael S. Tsirkin, QEMU Developers, Ard Biesheuvel,
Claudio Fontana, Alvise Rigo, Stuart Yoder
On 29.01.15 15:45, Peter Maydell wrote:
> On 29 January 2015 at 14:37, Alexander Graf <agraf@suse.de> wrote:
>> On 29.01.15 15:34, Peter Maydell wrote:
>>> I kind of see, but isn't this just a window from CPU address
>>> space into PCI address space, not vice-versa?
>>
>> Yup, exactly. But PCI devices need to map themselves somewhere into the
>> PCI address space. So if I configure a BAR to live at 0x10000000, it
>> should also show up at 0x10000000 when accessed from the CPU. That's
>> what the mapping above is about.
>
> No, it doesn't have to. It's a choice to make the mapping
> be such that the system address for a BAR matches the address
> in PCI memory space, not a requirement. I agree it's a
> sensible choice, though.
>
> But as I say, this code is setting up one mapping (the
> system address -> PCI space mapping), not two.
Yes, the other one is done implicitly by the OS based on what device
tree tells it to do. If we map it at 0, our good old if (BAR == 0)
break; friend hits us again though - and any other arbitrary offset is
as good as a 1:1 map.
>
>>> DMA by PCI devices bus-mastering into system memory must be
>>> being set up elsewhere, I think.
>>
>> Yes, that's a different mechanism that's not implemented yet for GPEX
>> :).
>
> We can't not implement DMA, it would break lots of the usual
> PCI devices people want to use. In fact I thought the PCI
> core code implemented a default of "DMA by PCI devices goes
> to the system address space" if you didn't specifically
> set up something else by calling pci_setup_iommu(). This is
> definitely how it works for plain PCI host bridges, are
> PCIe bridges different?
Nono, this is exactly the way it works. The thing that's not implemented
is the SMMU to make that dynamic.
>> On ARM this would happen via SMMU emulation.
>
> There's no requirement for a PCI host controller to be
> sat behind an SMMU -- that's a system design choice. We
> don't need to implement the SMMU yet (or perhaps ever?);
The main benefit of implementing a guest SMMU is that you don't have to
pin all guest memory at all times. Apart from that and the usual
security aid it only makes things slower ;).
> we definitely need to support PCI DMA.
We do.
Alex
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/4] pci: Add generic PCIe host bridge
2015-01-29 14:25 ` Peter Maydell
@ 2015-01-30 10:25 ` Paolo Bonzini
0 siblings, 0 replies; 24+ messages in thread
From: Paolo Bonzini @ 2015-01-30 10:25 UTC (permalink / raw)
To: Peter Maydell, Alexander Graf
Cc: Rob Herring, Michael S. Tsirkin, QEMU Developers, Ard Biesheuvel,
Claudio Fontana, Alvise Rigo, Stuart Yoder
On 29/01/2015 15:25, Peter Maydell wrote:
> > Ah, right, I forgot about that. Unfortunately I can't reserve PCI IDs in
> > the Red Hat PCI range.
> >
> > Michael, what's the process here?
>
> Last time this came up I think Paolo said the official registration
> was getting a patch into QEMU master that added a line to the
> list in pci.h :-) [ie QEMU is the master source for these IDs]
Yeah, rocker was the first case of someone not a QEMU developer needing
an id. So let's make QEMU the master source.
Paolo
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2015-01-30 10:25 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-21 16:18 [Qemu-devel] [PATCH v2 0/4] ARM: Add support for a generic PCI Express host bridge Alexander Graf
2015-01-21 16:18 ` [Qemu-devel] [PATCH v2 1/4] pci: Split pcie_host_mmcfg_map() Alexander Graf
2015-01-27 13:55 ` Peter Maydell
2015-01-27 14:24 ` Michael S. Tsirkin
2015-01-27 14:40 ` Paolo Bonzini
2015-01-21 16:18 ` [Qemu-devel] [PATCH v2 2/4] pci: Add generic PCIe host bridge Alexander Graf
2015-01-22 16:32 ` B02008
2015-01-27 15:31 ` Peter Maydell
2015-01-29 13:59 ` Alexander Graf
2015-01-29 14:25 ` Peter Maydell
2015-01-30 10:25 ` Paolo Bonzini
2015-01-21 16:18 ` [Qemu-devel] [PATCH v2 3/4] arm: Add PCIe host bridge in virt machine Alexander Graf
2015-01-22 15:28 ` Claudio Fontana
2015-01-22 15:52 ` Alexander Graf
2015-01-27 9:24 ` Claudio Fontana
2015-01-27 10:09 ` Peter Maydell
2015-01-27 14:37 ` Claudio Fontana
2015-01-27 16:52 ` Peter Maydell
2015-01-29 14:31 ` Alexander Graf
2015-01-29 14:34 ` Peter Maydell
2015-01-29 14:37 ` Alexander Graf
2015-01-29 14:45 ` Peter Maydell
2015-01-29 14:49 ` Alexander Graf
2015-01-21 16:18 ` [Qemu-devel] [PATCH v2 4/4] pci: Move PCI VGA to pci.mak Alexander Graf
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.