[Qemu-devel] [RFC v3 0/2] Add Generic PCI host device update

[Qemu-devel] [RFC v3 0/2] Add Generic PCI host device update

[Alvise Rigo]

This is just an update to address the comments received on v2.

This patch series is based on the previous work [1] and [2] by Rob Herring.
This is a stand alone work, but I will not hesitate to split the patches in a
more accurate way if needed.

Changes from [1] and [2]:
- memory topology definition has been revisited
- memory addresses and sizes are now defined as qdev properties
- four system interrupts are swizzled between all the PCI devices and their
  pins
- other minor changes
- removed implicit creation of lsi and pci-ohci devices

Changes from v2:
- Some comments from Peter Maydell have been addressed: the device tree node
  generation is moved back to the mach-virt platform as in the original work.
  For this reason, mach-virt is the only supported machine model at the moment.
  Moreover, the device tree node is statically created at the device init,
  while in the previous versions it was generated according to the PCI devices
  attached to the bus.
- The creation of the device memory regions has been postponed in the realize
  function (thanks to Alexander Graf).
- Various comments from Claudio Fontana have been addressed to better explain
  some crucial points of the adopted PCI specifications (see below).

There are two specifications used by these patches. The first one regards
mainly how to describe a PCI regions inside the dt node "ranges" property. Look
at "PCI Bus binding: IEEE Std 1275-1994" for more details.  The second instead
regards mainly the properties "interrupt-map-mask" and "interrupt-map" that are
described in "Open Firmware Recommended Practice: Interrupt Mapping". This
document propose a way to route PCI interrupts to system interrupts.  All these
specifications have been firstly implemented, in kernel side, by the
host-generic-pci driver.

This work has been tested attaching several PCI devices to the mach-virt
platform using an ARMv7 CPU. The tested devices are: virtio-blk-pci,
virtio-net-pci, lsi53c895a and pci-ohci (all attached at the same time).

TODO:
- Add MSI, MSI-X support
- PCI-E support. For this, you would probably look at Alexander Graf work [3]

Thank you, alvise

[1]
"[Qemu-devel] [RFC PATCH 1/2] hw/pci-host: add a generic PCI host"
http://lists.gnu.org/archive/html/qemu-devel/2014-06/msg03482.html
[2]
"[Qemu-devel] [RFC PATCH 2/2] hw/arm/virt: Add generic PCI host device"
http://lists.gnu.org/archive/html/qemu-devel/2014-06/msg03483.html
[3]
"[Qemu-devel] [PATCH 0/4] ARM: Add support for a generic PCI Express host bridge"
https://www.mail-archive.com/qemu-devel@nongnu.org/msg272648.html



Alvise Rigo (2):
  pci/pci-host: Add generic-pci PCI host controller device
  hw/arm/virt: add generic-pci PCI host controller

 hw/arm/virt.c                     | 112 +++++++++++++++++++++++++++++-
 hw/pci-host/Makefile.objs         |   2 +-
 hw/pci-host/generic-pci.c         | 140 ++++++++++++++++++++++++++++++++++++++
 include/hw/pci-host/generic-pci.h |  45 ++++++++++++
 4 files changed, 297 insertions(+), 2 deletions(-)
 create mode 100644 hw/pci-host/generic-pci.c
 create mode 100644 include/hw/pci-host/generic-pci.h

-- 
2.1.0

[Qemu-devel] [RFC v3 1/2] pci/pci-host: Add generic-pci PCI host controller device

[Alvise Rigo]

Add a generic PCI host controller for virtual platforms, based on the
previous work by Rob Herring:
http://lists.gnu.org/archive/html/qemu-devel/2014-06/msg03482.html

The controller relies on a configuration memory region and provides two
PCI memory regions for I/O (one port and one memory mapped).  The device
needs the following qdev properties to configure the memory regions:
- cfg_win_size: size of the configuration memory
- pio_win_size: size of the port I/O space
- mmio_win_size: size of the MMIO space
- mmio_win_addr: offset of MMIO space in the system memory

Signed-off-by: Alvise Rigo <a.rigo@virtualopensystems.com>
---
 hw/pci-host/Makefile.objs         |   2 +-
 hw/pci-host/generic-pci.c         | 140 ++++++++++++++++++++++++++++++++++++++
 include/hw/pci-host/generic-pci.h |  45 ++++++++++++
 3 files changed, 186 insertions(+), 1 deletion(-)
 create mode 100644 hw/pci-host/generic-pci.c
 create mode 100644 include/hw/pci-host/generic-pci.h

diff --git a/hw/pci-host/Makefile.objs b/hw/pci-host/Makefile.objs
index bb65f9c..8ef9fac 100644
--- a/hw/pci-host/Makefile.objs
+++ b/hw/pci-host/Makefile.objs
@@ -1,4 +1,4 @@
-common-obj-y += pam.o
+common-obj-y += pam.o generic-pci.o
 
 # PPC devices
 common-obj-$(CONFIG_PREP_PCI) += prep.o
diff --git a/hw/pci-host/generic-pci.c b/hw/pci-host/generic-pci.c
new file mode 100644
index 0000000..54c9647
--- /dev/null
+++ b/hw/pci-host/generic-pci.c
@@ -0,0 +1,140 @@
+/*
+ * Generic PCI host controller
+ *
+ * Copyright (c) 2014 Linaro, Ltd.
+ * Author: Rob Herring <rob.herring@linaro.org>
+ *
+ * Based on ARM Versatile PCI controller (hw/pci-host/versatile.c):
+ * Copyright (c) 2006-2009 CodeSourcery.
+ * Written by Paul Brook
+ *
+ * This code is licensed under the LGPL.
+ */
+
+#include "hw/sysbus.h"
+#include "hw/pci-host/generic-pci.h"
+#include "exec/address-spaces.h"
+#include "sysemu/device_tree.h"
+
+static const VMStateDescription pci_generic_host_vmstate = {
+    .name = "generic-host-pci",
+    .version_id = 1,
+    .minimum_version_id = 1,
+};
+
+static void pci_cam_config_write(void *opaque, hwaddr addr,
+                                 uint64_t val, unsigned size)
+{
+    PCIGenState *s = opaque;
+    pci_data_write(&s->pci_bus, addr, val, size);
+}
+
+static uint64_t pci_cam_config_read(void *opaque, hwaddr addr, unsigned size)
+{
+    PCIGenState *s = opaque;
+    uint32_t val;
+    val = pci_data_read(&s->pci_bus, addr, size);
+    return val;
+}
+
+static const MemoryRegionOps pci_generic_config_ops = {
+    .read = pci_cam_config_read,
+    .write = pci_cam_config_write,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+};
+
+static void pci_generic_set_irq(void *opaque, int irq_num, int level)
+{
+    qemu_irq *pic = opaque;
+    qemu_set_irq(pic[irq_num], level);
+}
+
+static void pci_generic_host_realize(DeviceState *dev, Error **errp)
+{
+    PCIHostState *h = PCI_HOST_BRIDGE(dev);
+    PCIGenState *s = PCI_GEN(dev);
+    GenericPCIHostState *gps = &s->pci_gen;
+    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+    int i;
+
+    memory_region_init(&s->pci_io_window, OBJECT(s), "pci_io", s->pio_win_size);
+    memory_region_init(&s->pci_mem_space, OBJECT(s), "pci_mem", 1ULL << 32);
+
+    pci_bus_new_inplace(&s->pci_bus, sizeof(s->pci_bus), dev, "pci",
+                        &s->pci_mem_space, &s->pci_io_window,
+                        PCI_DEVFN(0, 0), TYPE_PCI_BUS);
+    h->bus = &s->pci_bus;
+
+    object_initialize(gps, sizeof(*gps), TYPE_GENERIC_PCI_HOST);
+    qdev_set_parent_bus(DEVICE(gps), BUS(&s->pci_bus));
+
+    for (i = 0; i < s->irqs; i++) {
+        sysbus_init_irq(sbd, &s->irq[i]);
+    }
+
+    pci_bus_irqs(&s->pci_bus, pci_generic_set_irq, pci_swizzle_map_irq_fn,
+                                                         s->irq, s->irqs);
+    memory_region_init_io(&s->mem_config, OBJECT(s), &pci_generic_config_ops, s,
+                          "pci-config", s->cfg_win_size);
+    memory_region_init_alias(&s->pci_mem_window, OBJECT(s), "pci-mem-win",
+                    &s->pci_mem_space, s->mmio_win_addr, s->mmio_win_size);
+
+    sysbus_init_mmio(sbd, &s->mem_config);
+    sysbus_init_mmio(sbd, &s->pci_io_window);
+    sysbus_init_mmio(sbd, &s->pci_mem_window);
+}
+
+static void pci_generic_host_class_init(ObjectClass *klass, void *data)
+{
+    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    k->vendor_id = PCI_VENDOR_ID_REDHAT;
+    k->device_id = PCI_DEVICE_ID_REDHAT_BRIDGE;
+    k->class_id = PCI_CLASS_PROCESSOR_CO;
+    /*
+     * 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 pci_generic_host_info = {
+    .name          = TYPE_GENERIC_PCI_HOST,
+    .parent        = TYPE_PCI_DEVICE,
+    .instance_size = sizeof(GenericPCIHostState),
+    .class_init    = pci_generic_host_class_init,
+};
+
+static Property pci_generic_props[] = {
+    DEFINE_PROP_UINT32("cfg_win_size", PCIGenState, cfg_win_size, 1ULL << 20),
+    DEFINE_PROP_UINT32("pio_win_size", PCIGenState, pio_win_size, 64 * 1024),
+    DEFINE_PROP_UINT64("mmio_win_size", PCIGenState, mmio_win_size, 1ULL << 32),
+    DEFINE_PROP_UINT64("mmio_win_addr", PCIGenState, mmio_win_addr, 0),
+    DEFINE_PROP_UINT32("irqs", PCIGenState, irqs, 4),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void pci_generic_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->realize = pci_generic_host_realize;
+    dc->vmsd = &pci_generic_host_vmstate;
+    dc->props = pci_generic_props;
+}
+
+static const TypeInfo pci_generic_info = {
+    .name          = TYPE_GENERIC_PCI,
+    .parent        = TYPE_PCI_HOST_BRIDGE,
+    .instance_size = sizeof(PCIGenState),
+    .class_init    = pci_generic_class_init,
+};
+
+static void generic_pci_host_register_types(void)
+{
+    type_register_static(&pci_generic_info);
+    type_register_static(&pci_generic_host_info);
+}
+
+type_init(generic_pci_host_register_types)
\ No newline at end of file
diff --git a/include/hw/pci-host/generic-pci.h b/include/hw/pci-host/generic-pci.h
new file mode 100644
index 0000000..830542e
--- /dev/null
+++ b/include/hw/pci-host/generic-pci.h
@@ -0,0 +1,45 @@
+#ifndef QEMU_GENERIC_PCI_H
+#define QEMU_GENERIC_PCI_H
+
+#include "hw/pci/pci.h"
+#include "hw/pci/pci_bus.h"
+#include "hw/pci/pci_host.h"
+
+#define MAX_PCI_DEVICES (PCI_SLOT_MAX * PCI_FUNC_MAX)
+
+typedef struct {
+    /*< private >*/
+    PCIDevice parent_obj;
+} GenericPCIHostState;
+
+typedef struct PCIGenState {
+    /*< private >*/
+    PCIHostState parent_obj;
+
+    qemu_irq irq[MAX_PCI_DEVICES];
+    MemoryRegion mem_config;
+    /* Container representing the PCI address MMIO space */
+    MemoryRegion pci_mem_space;
+    /* Alias region into PCI address spaces which we expose as sysbus region */
+    MemoryRegion pci_mem_window;
+    /* PCI I/O region */
+    MemoryRegion pci_io_window;
+    PCIBus pci_bus;
+    GenericPCIHostState pci_gen;
+
+    uint32_t cfg_win_size;
+    uint32_t pio_win_size;
+    uint64_t mmio_win_addr; // offset of pci_mem_window inside pci_mem_space
+    uint64_t mmio_win_size;
+    uint32_t irqs;
+} PCIGenState;
+
+#define TYPE_GENERIC_PCI "generic_pci"
+#define PCI_GEN(obj) \
+    OBJECT_CHECK(PCIGenState, (obj), TYPE_GENERIC_PCI)
+
+#define TYPE_GENERIC_PCI_HOST "generic_pci_host"
+#define PCI_GEN_HOST(obj) \
+    OBJECT_CHECK(GenericPCIHostState, (obj), TYPE_GENERIC_PCI_HOST)
+
+#endif
\ No newline at end of file
-- 
2.1.0

[Qemu-devel] [RFC v3 2/2] hw/arm/virt: add generic-pci PCI host controller

[Alvise Rigo]

The platform memory map has now three more memory ranges to map the
device's memory regions (Configuration region, I/O region and Memory
region).

The dt node interrupt-map property tells how to route the PCI interrupts
to system interrupts. In the mach-virt case, four IRQs are swizzled
between all the possible interrupts coming from the PCI bus. In
particular, the mapping ensures that the first four devices will use a
system IRQ always different (supposing that only PIN_A of each device is
used).

Now that a PCI bus is provided, the machine can be launched with
multiple PCI devices through the -device option (e.g., -device
virtio-blk-pci).

Signed-off-by: Alvise Rigo <a.rigo@virtualopensystems.com>
---
 hw/arm/virt.c | 112 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 111 insertions(+), 1 deletion(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 2353440..7b0326f 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -44,6 +44,7 @@
 #include "qemu/error-report.h"
 
 #define NUM_VIRTIO_TRANSPORTS 32
+#define NUM_PCI_IRQS 4
 
 /* Number of external interrupt lines to configure the GIC with */
 #define NUM_IRQS 128
@@ -68,8 +69,12 @@ enum {
     VIRT_UART,
     VIRT_MMIO,
     VIRT_RTC,
+    VIRT_PCI_CFG,
+    VIRT_PCI_IO,
+    VIRT_PCI_MEM,
     VIRT_FW_CFG,
 };
+#define VIRT_PCI VIRT_PCI_CFG
 
 typedef struct MemMapEntry {
     hwaddr base;
@@ -129,13 +134,17 @@ 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 */
+    /* PCI regions */
+    [VIRT_PCI_CFG] =    { 0x10000000, 0x01000000 },
+    [VIRT_PCI_IO] =     { 0x11000000, 0x00010000 },
+    [VIRT_PCI_MEM] =    { 0x12000000, 0x2e000000 },
     [VIRT_MEM] =        { 0x40000000, 30ULL * 1024 * 1024 * 1024 },
 };
 
 static const int a15irqmap[] = {
     [VIRT_UART] = 1,
     [VIRT_RTC] = 2,
+    [VIRT_PCI] = 3, /* ...to 3 + NUM_PCI_IRQS - 1 */
     [VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */
 };
 
@@ -436,6 +445,105 @@ static void create_rtc(const VirtBoardInfo *vbi, qemu_irq *pic)
     g_free(nodename);
 }
 
+static void create_pci_host(const VirtBoardInfo *vbi, qemu_irq *pic)
+{
+    DeviceState *dev;
+    SysBusDevice *busdev;
+    int i, j, intmap_rows;
+    const MemMapEntry *mme = vbi->memmap;
+    uint32_t plat_acells;
+    uint32_t plat_scells;
+    uint32_t gic_phandle;
+    char *nodename;
+
+    dev = qdev_create(NULL, "generic_pci");
+    qdev_prop_set_uint64(dev, "mmio_win_size", mme[VIRT_PCI_MEM].size);
+    qdev_prop_set_uint64(dev, "mmio_win_addr", mme[VIRT_PCI_MEM].base);
+    qdev_prop_set_uint32(dev, "irqs", NUM_PCI_IRQS);
+    qdev_init_nofail(dev);
+
+    busdev = SYS_BUS_DEVICE(dev);
+    sysbus_mmio_map(busdev, 0, mme[VIRT_PCI_CFG].base); /* PCI config */
+    sysbus_mmio_map(busdev, 1, mme[VIRT_PCI_IO].base);  /* PCI I/O */
+    sysbus_mmio_map(busdev, 2, mme[VIRT_PCI_MEM].base); /* PCI memory window */
+
+    for ( i = 0; i < NUM_PCI_IRQS; i++) {
+        sysbus_connect_irq(busdev, i, pic[vbi->irqmap[VIRT_PCI] + i]);
+    }
+
+    /* add device tree node */
+    nodename = g_strdup_printf("/pci@%" PRIx64, mme[VIRT_PCI_CFG].base);
+    qemu_fdt_add_subnode(vbi->fdt, nodename);
+    qemu_fdt_setprop_string(vbi->fdt, nodename, "compatible",
+                                     "pci-host-cam-generic");
+    qemu_fdt_setprop_string(vbi->fdt, nodename, "device_type", "pci");
+    qemu_fdt_setprop_cell(vbi->fdt, nodename, "#address-cells", 0x3);
+    qemu_fdt_setprop_cell(vbi->fdt, nodename, "#size-cells", 0x2);
+    qemu_fdt_setprop_cell(vbi->fdt, nodename, "#interrupt-cells", 0x1);
+
+    plat_acells = qemu_fdt_getprop_cell(vbi->fdt, "/", "#address-cells");
+    plat_scells = qemu_fdt_getprop_cell(vbi->fdt, "/", "#size-cells");
+    qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "reg",
+                                    plat_acells, mme[VIRT_PCI_CFG].base,
+                                    plat_scells, mme[VIRT_PCI_CFG].size);
+
+    /* Two regions (second and third of *busdev):
+       - I/O region, PCI addr = 0x0, CPU addr = mme[VIRT_PCI_IO].base,
+         size = mme[VIRT_PCI_IO].size,
+       - 32bit MMIO region, PCI addr = CPU addr = mme[VIRT_PCI_MEM].base,
+         size = mme[VIRT_PCI_MEM].size */
+    qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "ranges",
+            1, 0x01000000, 2, 0x00000000, /* I/O space */
+            2, mme[VIRT_PCI_IO].base, 2, mme[VIRT_PCI_IO].size,
+            1, 0x02000000, 2, mme[VIRT_PCI_MEM].base, /* 32bit memory space */
+            2, mme[VIRT_PCI_MEM].base, 2, mme[VIRT_PCI_MEM].size);
+
+    gic_phandle = qemu_fdt_get_phandle(vbi->fdt, "/intc");
+
+#define IRQ_MAP_ENTRY_DESC_SZ 14
+#define PHYSH_DEV_SHIFT 11
+    /* Any interrupt from the PCI bus is represented by four 32bit values <physH
+       physM physL pin#> (unit-interrupt-specifier), where bits physH[11:15]
+       identify the device number and pin# the pin number that triggered. We use
+       the two bits physH[14:15] and pin# to route all the PCI interrupts to
+       four different system interrupts. The following mask is used to honour
+       exactly those bits. */
+    qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "interrupt-map-mask",
+                    1, 0x3 << PHYSH_DEV_SHIFT, 1, 0x0, 1, 0x0, 1, 0x7);
+
+    /* interrupt-map property
+       Two bits of physH[14:15] and four possible values of pin# give a maximum
+       of 16 different results of the mask operation:
+       <physH physM physL pin#> AND <interrupt-map-mask>
+       Cycling through the four system interrupts available, we assign to each
+       result an interrupt number (basically, 32 * 4 maximum PCI interrupts are
+       mapped to 4 system interrupts).
+       Each entry inside interrupt-map has the following form:
+       <PCI unit-interrupt-specifier, system unit-interrupt-specifier> */
+    intmap_rows = 16;
+    uint64_t *int_mapping_data = g_malloc0(IRQ_MAP_ENTRY_DESC_SZ *
+                                  sizeof(uint64_t) * intmap_rows);
+    uint64_t *int_map_data = int_mapping_data;
+    for (i = 0; i < 4; i++) {
+        for (j = 0; j < 4; j++) {
+           uint64_t physh_dev_nr = i << PHYSH_DEV_SHIFT;
+           uint64_t pci_pin_nr = j + 1;
+           uint64_t row[IRQ_MAP_ENTRY_DESC_SZ] =
+           {1, physh_dev_nr, 2, 0x0, 1, pci_pin_nr, // masked PCI interrupt...
+            1, gic_phandle, 1, GIC_FDT_IRQ_TYPE_SPI, // ...mapped to this IRQ
+            1, (vbi->irqmap[VIRT_PCI_CFG] + (j + i) % 4),
+            1, GIC_FDT_IRQ_FLAGS_LEVEL_HI};
+           int_map_data = mempcpy(int_map_data, row, sizeof(row));
+        }
+    }
+
+    qemu_fdt_setprop_sized_cells_from_array(vbi->fdt, nodename, "interrupt-map",
+                     (intmap_rows * IRQ_MAP_ENTRY_DESC_SZ)/2, int_mapping_data);
+
+    g_free(int_mapping_data);
+    g_free(nodename);
+}
+
 static void create_virtio_devices(const VirtBoardInfo *vbi, qemu_irq *pic)
 {
     int i;
@@ -640,6 +748,8 @@ static void machvirt_init(MachineState *machine)
 
     create_rtc(vbi, pic);
 
+    create_pci_host(vbi, pic);
+
     /* 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.
-- 
2.1.0

Re: [Qemu-devel] [RFC v3 2/2] hw/arm/virt: add generic-pci PCI host controller

[Claudio Fontana]

On 14.01.2015 11:16, Alvise Rigo wrote:
> The platform memory map has now three more memory ranges to map the
> device's memory regions (Configuration region, I/O region and Memory
> region).
> 
> The dt node interrupt-map property tells how to route the PCI interrupts
> to system interrupts. In the mach-virt case, four IRQs are swizzled
> between all the possible interrupts coming from the PCI bus. In
> particular, the mapping ensures that the first four devices will use a
> system IRQ always different (supposing that only PIN_A of each device is
> used).
> 
> Now that a PCI bus is provided, the machine can be launched with
> multiple PCI devices through the -device option (e.g., -device
> virtio-blk-pci).
> 
> Signed-off-by: Alvise Rigo <a.rigo@virtualopensystems.com>
> ---
>  hw/arm/virt.c | 112 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 111 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 2353440..7b0326f 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -44,6 +44,7 @@
>  #include "qemu/error-report.h"
>  
>  #define NUM_VIRTIO_TRANSPORTS 32
> +#define NUM_PCI_IRQS 4
>  
>  /* Number of external interrupt lines to configure the GIC with */
>  #define NUM_IRQS 128
> @@ -68,8 +69,12 @@ enum {
>      VIRT_UART,
>      VIRT_MMIO,
>      VIRT_RTC,
> +    VIRT_PCI_CFG,
> +    VIRT_PCI_IO,
> +    VIRT_PCI_MEM,
>      VIRT_FW_CFG,
>  };
> +#define VIRT_PCI VIRT_PCI_CFG
>  
>  typedef struct MemMapEntry {
>      hwaddr base;
> @@ -129,13 +134,17 @@ 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 */
> +    /* PCI regions */
> +    [VIRT_PCI_CFG] =    { 0x10000000, 0x01000000 },
> +    [VIRT_PCI_IO] =     { 0x11000000, 0x00010000 },
> +    [VIRT_PCI_MEM] =    { 0x12000000, 0x2e000000 },
>      [VIRT_MEM] =        { 0x40000000, 30ULL * 1024 * 1024 * 1024 },
>  };
>  
>  static const int a15irqmap[] = {
>      [VIRT_UART] = 1,
>      [VIRT_RTC] = 2,
> +    [VIRT_PCI] = 3, /* ...to 3 + NUM_PCI_IRQS - 1 */
>      [VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */
>  };
>  
> @@ -436,6 +445,105 @@ static void create_rtc(const VirtBoardInfo *vbi, qemu_irq *pic)
>      g_free(nodename);
>  }
>  
> +static void create_pci_host(const VirtBoardInfo *vbi, qemu_irq *pic)
> +{
> +    DeviceState *dev;
> +    SysBusDevice *busdev;
> +    int i, j, intmap_rows;
> +    const MemMapEntry *mme = vbi->memmap;
> +    uint32_t plat_acells;
> +    uint32_t plat_scells;
> +    uint32_t gic_phandle;
> +    char *nodename;
> +
> +    dev = qdev_create(NULL, "generic_pci");
> +    qdev_prop_set_uint64(dev, "mmio_win_size", mme[VIRT_PCI_MEM].size);
> +    qdev_prop_set_uint64(dev, "mmio_win_addr", mme[VIRT_PCI_MEM].base);
> +    qdev_prop_set_uint32(dev, "irqs", NUM_PCI_IRQS);
> +    qdev_init_nofail(dev);
> +
> +    busdev = SYS_BUS_DEVICE(dev);
> +    sysbus_mmio_map(busdev, 0, mme[VIRT_PCI_CFG].base); /* PCI config */
> +    sysbus_mmio_map(busdev, 1, mme[VIRT_PCI_IO].base);  /* PCI I/O */
> +    sysbus_mmio_map(busdev, 2, mme[VIRT_PCI_MEM].base); /* PCI memory window */
> +
> +    for ( i = 0; i < NUM_PCI_IRQS; i++) {
> +        sysbus_connect_irq(busdev, i, pic[vbi->irqmap[VIRT_PCI] + i]);
> +    }
> +
> +    /* add device tree node */
> +    nodename = g_strdup_printf("/pci@%" PRIx64, mme[VIRT_PCI_CFG].base);
> +    qemu_fdt_add_subnode(vbi->fdt, nodename);
> +    qemu_fdt_setprop_string(vbi->fdt, nodename, "compatible",
> +                                     "pci-host-cam-generic");
> +    qemu_fdt_setprop_string(vbi->fdt, nodename, "device_type", "pci");
> +    qemu_fdt_setprop_cell(vbi->fdt, nodename, "#address-cells", 0x3);
> +    qemu_fdt_setprop_cell(vbi->fdt, nodename, "#size-cells", 0x2);
> +    qemu_fdt_setprop_cell(vbi->fdt, nodename, "#interrupt-cells", 0x1);
> +
> +    plat_acells = qemu_fdt_getprop_cell(vbi->fdt, "/", "#address-cells");
> +    plat_scells = qemu_fdt_getprop_cell(vbi->fdt, "/", "#size-cells");
> +    qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "reg",
> +                                    plat_acells, mme[VIRT_PCI_CFG].base,
> +                                    plat_scells, mme[VIRT_PCI_CFG].size);
> +
> +    /* Two regions (second and third of *busdev):
> +       - I/O region, PCI addr = 0x0, CPU addr = mme[VIRT_PCI_IO].base,
> +         size = mme[VIRT_PCI_IO].size,
> +       - 32bit MMIO region, PCI addr = CPU addr = mme[VIRT_PCI_MEM].base,
> +         size = mme[VIRT_PCI_MEM].size */
> +    qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "ranges",
> +            1, 0x01000000, 2, 0x00000000, /* I/O space */
> +            2, mme[VIRT_PCI_IO].base, 2, mme[VIRT_PCI_IO].size,
> +            1, 0x02000000, 2, mme[VIRT_PCI_MEM].base, /* 32bit memory space */
> +            2, mme[VIRT_PCI_MEM].base, 2, mme[VIRT_PCI_MEM].size);
> +
> +    gic_phandle = qemu_fdt_get_phandle(vbi->fdt, "/intc");
> +
> +#define IRQ_MAP_ENTRY_DESC_SZ 14
> +#define PHYSH_DEV_SHIFT 11
> +    /* Any interrupt from the PCI bus is represented by four 32bit values <physH
> +       physM physL pin#> (unit-interrupt-specifier), where bits physH[11:15]
> +       identify the device number and pin# the pin number that triggered. We use
> +       the two bits physH[14:15] and pin# to route all the PCI interrupts to
> +       four different system interrupts. The following mask is used to honour
> +       exactly those bits. */
> +    qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "interrupt-map-mask",
> +                    1, 0x3 << PHYSH_DEV_SHIFT, 1, 0x0, 1, 0x0, 1, 0x7);
> +
> +    /* interrupt-map property
> +       Two bits of physH[14:15] and four possible values of pin# give a maximum
> +       of 16 different results of the mask operation:
> +       <physH physM physL pin#> AND <interrupt-map-mask>
> +       Cycling through the four system interrupts available, we assign to each
> +       result an interrupt number (basically, 32 * 4 maximum PCI interrupts are
> +       mapped to 4 system interrupts).
> +       Each entry inside interrupt-map has the following form:
> +       <PCI unit-interrupt-specifier, system unit-interrupt-specifier> */
> +    intmap_rows = 16;
> +    uint64_t *int_mapping_data = g_malloc0(IRQ_MAP_ENTRY_DESC_SZ *
> +                                  sizeof(uint64_t) * intmap_rows);
> +    uint64_t *int_map_data = int_mapping_data;
> +    for (i = 0; i < 4; i++) {
> +        for (j = 0; j < 4; j++) {
> +           uint64_t physh_dev_nr = i << PHYSH_DEV_SHIFT;
> +           uint64_t pci_pin_nr = j + 1;
> +           uint64_t row[IRQ_MAP_ENTRY_DESC_SZ] =
> +           {1, physh_dev_nr, 2, 0x0, 1, pci_pin_nr, // masked PCI interrupt...
> +            1, gic_phandle, 1, GIC_FDT_IRQ_TYPE_SPI, // ...mapped to this IRQ
> +            1, (vbi->irqmap[VIRT_PCI_CFG] + (j + i) % 4),
> +            1, GIC_FDT_IRQ_FLAGS_LEVEL_HI};
> +           int_map_data = mempcpy(int_map_data, row, sizeof(row));
> +        }
> +    }
> +

sorry for maybe being pedantic, but is this working as intended?
The same PHYS_HI is mapped multiple times to a different irq if I understand this correctly, ending up in these mappings:

B,D,F irqmap-mask   0x00001800
PHYS_HI > SPI irq mappings

0x00000000 -> SPI irq 3
0x00000000 -> SPI irq 4
0x00000000 -> SPI irq 5
0x00000000 -> SPI irq 6

0x00000800 -> SPI irq 3
0x00000800 -> SPI irq 4
0x00000800 -> SPI irq 5
0x00000800 -> SPI irq 6

0x00001000 -> SPI irq 3
0x00001000 -> SPI irq 4
0x00001000 -> SPI irq 5
0x00001000 -> SPI irq 6

0x00001800 -> SPI irq 3
0x00001800 -> SPI irq 4
0x00001800 -> SPI irq 5
0x00001800 -> SPI irq 6

did you not intend to do 

B,D,F irqmap-mask   0x00001800
PHYS_HI > SPI irq mappings

0x00000000 -> SPI irq 3
0x00000800 -> SPI irq 4
0x00001000 -> SPI irq 5
0x00001800 -> SPI irq 6

?

Thanks,

Claudio


> +    qemu_fdt_setprop_sized_cells_from_array(vbi->fdt, nodename, "interrupt-map",
> +                     (intmap_rows * IRQ_MAP_ENTRY_DESC_SZ)/2, int_mapping_data);
> +
> +    g_free(int_mapping_data);
> +    g_free(nodename);
> +}
> +
>  static void create_virtio_devices(const VirtBoardInfo *vbi, qemu_irq *pic)
>  {
>      int i;
> @@ -640,6 +748,8 @@ static void machvirt_init(MachineState *machine)
>  
>      create_rtc(vbi, pic);
>  
> +    create_pci_host(vbi, pic);
> +
>      /* 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.
> 


-- 
Claudio Fontana
Server Virtualization Architect
Huawei Technologies Duesseldorf GmbH
Riesstraße 25 - 80992 München

office: +49 89 158834 4135
mobile: +49 15253060158

Re: [Qemu-devel] [RFC v3 1/2] pci/pci-host: Add generic-pci PCI host controller device

[Claudio Fontana]

On 14.01.2015 11:16, Alvise Rigo wrote:
> Add a generic PCI host controller for virtual platforms, based on the
> previous work by Rob Herring:
> http://lists.gnu.org/archive/html/qemu-devel/2014-06/msg03482.html
> 
> The controller relies on a configuration memory region and provides two
> PCI memory regions for I/O (one port and one memory mapped).  The device
> needs the following qdev properties to configure the memory regions:
> - cfg_win_size: size of the configuration memory
> - pio_win_size: size of the port I/O space
> - mmio_win_size: size of the MMIO space
> - mmio_win_addr: offset of MMIO space in the system memory
> 
> Signed-off-by: Alvise Rigo <a.rigo@virtualopensystems.com>
> ---
>  hw/pci-host/Makefile.objs         |   2 +-
>  hw/pci-host/generic-pci.c         | 140 ++++++++++++++++++++++++++++++++++++++
>  include/hw/pci-host/generic-pci.h |  45 ++++++++++++
>  3 files changed, 186 insertions(+), 1 deletion(-)
>  create mode 100644 hw/pci-host/generic-pci.c
>  create mode 100644 include/hw/pci-host/generic-pci.h
> 
> diff --git a/hw/pci-host/Makefile.objs b/hw/pci-host/Makefile.objs
> index bb65f9c..8ef9fac 100644
> --- a/hw/pci-host/Makefile.objs
> +++ b/hw/pci-host/Makefile.objs
> @@ -1,4 +1,4 @@
> -common-obj-y += pam.o
> +common-obj-y += pam.o generic-pci.o
>  
>  # PPC devices
>  common-obj-$(CONFIG_PREP_PCI) += prep.o
> diff --git a/hw/pci-host/generic-pci.c b/hw/pci-host/generic-pci.c
> new file mode 100644
> index 0000000..54c9647
> --- /dev/null
> +++ b/hw/pci-host/generic-pci.c
> @@ -0,0 +1,140 @@
> +/*
> + * Generic PCI host controller
> + *
> + * Copyright (c) 2014 Linaro, Ltd.
> + * Author: Rob Herring <rob.herring@linaro.org>
> + *
> + * Based on ARM Versatile PCI controller (hw/pci-host/versatile.c):
> + * Copyright (c) 2006-2009 CodeSourcery.
> + * Written by Paul Brook
> + *
> + * This code is licensed under the LGPL.
> + */
> +
> +#include "hw/sysbus.h"
> +#include "hw/pci-host/generic-pci.h"
> +#include "exec/address-spaces.h"
> +#include "sysemu/device_tree.h"
> +
> +static const VMStateDescription pci_generic_host_vmstate = {
> +    .name = "generic-host-pci",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +};
> +
> +static void pci_cam_config_write(void *opaque, hwaddr addr,
> +                                 uint64_t val, unsigned size)
> +{
> +    PCIGenState *s = opaque;
> +    pci_data_write(&s->pci_bus, addr, val, size);
> +}
> +
> +static uint64_t pci_cam_config_read(void *opaque, hwaddr addr, unsigned size)
> +{
> +    PCIGenState *s = opaque;
> +    uint32_t val;
> +    val = pci_data_read(&s->pci_bus, addr, size);
> +    return val;
> +}
> +
> +static const MemoryRegionOps pci_generic_config_ops = {
> +    .read = pci_cam_config_read,
> +    .write = pci_cam_config_write,
> +    .endianness = DEVICE_LITTLE_ENDIAN,
> +};
> +
> +static void pci_generic_set_irq(void *opaque, int irq_num, int level)
> +{
> +    qemu_irq *pic = opaque;
> +    qemu_set_irq(pic[irq_num], level);
> +}
> +
> +static void pci_generic_host_realize(DeviceState *dev, Error **errp)
> +{
> +    PCIHostState *h = PCI_HOST_BRIDGE(dev);
> +    PCIGenState *s = PCI_GEN(dev);
> +    GenericPCIHostState *gps = &s->pci_gen;
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> +    int i;
> +
> +    memory_region_init(&s->pci_io_window, OBJECT(s), "pci_io", s->pio_win_size);
> +    memory_region_init(&s->pci_mem_space, OBJECT(s), "pci_mem", 1ULL << 32);
> +
> +    pci_bus_new_inplace(&s->pci_bus, sizeof(s->pci_bus), dev, "pci",
> +                        &s->pci_mem_space, &s->pci_io_window,
> +                        PCI_DEVFN(0, 0), TYPE_PCI_BUS);
> +    h->bus = &s->pci_bus;
> +
> +    object_initialize(gps, sizeof(*gps), TYPE_GENERIC_PCI_HOST);
> +    qdev_set_parent_bus(DEVICE(gps), BUS(&s->pci_bus));
> +
> +    for (i = 0; i < s->irqs; i++) {
> +        sysbus_init_irq(sbd, &s->irq[i]);
> +    }
> +
> +    pci_bus_irqs(&s->pci_bus, pci_generic_set_irq, pci_swizzle_map_irq_fn,
> +                                                         s->irq, s->irqs);
> +    memory_region_init_io(&s->mem_config, OBJECT(s), &pci_generic_config_ops, s,
> +                          "pci-config", s->cfg_win_size);
> +    memory_region_init_alias(&s->pci_mem_window, OBJECT(s), "pci-mem-win",
> +                    &s->pci_mem_space, s->mmio_win_addr, s->mmio_win_size);
> +
> +    sysbus_init_mmio(sbd, &s->mem_config);
> +    sysbus_init_mmio(sbd, &s->pci_io_window);
> +    sysbus_init_mmio(sbd, &s->pci_mem_window);
> +}
> +
> +static void pci_generic_host_class_init(ObjectClass *klass, void *data)
> +{
> +    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    k->vendor_id = PCI_VENDOR_ID_REDHAT;
> +    k->device_id = PCI_DEVICE_ID_REDHAT_BRIDGE;
> +    k->class_id = PCI_CLASS_PROCESSOR_CO;
> +    /*
> +     * 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 pci_generic_host_info = {
> +    .name          = TYPE_GENERIC_PCI_HOST,
> +    .parent        = TYPE_PCI_DEVICE,
> +    .instance_size = sizeof(GenericPCIHostState),
> +    .class_init    = pci_generic_host_class_init,
> +};
> +
> +static Property pci_generic_props[] = {
> +    DEFINE_PROP_UINT32("cfg_win_size", PCIGenState, cfg_win_size, 1ULL << 20),
> +    DEFINE_PROP_UINT32("pio_win_size", PCIGenState, pio_win_size, 64 * 1024),
> +    DEFINE_PROP_UINT64("mmio_win_size", PCIGenState, mmio_win_size, 1ULL << 32),
> +    DEFINE_PROP_UINT64("mmio_win_addr", PCIGenState, mmio_win_addr, 0),
> +    DEFINE_PROP_UINT32("irqs", PCIGenState, irqs, 4),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void pci_generic_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->realize = pci_generic_host_realize;
> +    dc->vmsd = &pci_generic_host_vmstate;
> +    dc->props = pci_generic_props;

why do some of these assigments have the & and some not?

> +}
> +
> +static const TypeInfo pci_generic_info = {
> +    .name          = TYPE_GENERIC_PCI,
> +    .parent        = TYPE_PCI_HOST_BRIDGE,
> +    .instance_size = sizeof(PCIGenState),
> +    .class_init    = pci_generic_class_init,
> +};
> +
> +static void generic_pci_host_register_types(void)
> +{
> +    type_register_static(&pci_generic_info);
> +    type_register_static(&pci_generic_host_info);
> +}
> +
> +type_init(generic_pci_host_register_types)
> \ No newline at end of file
> diff --git a/include/hw/pci-host/generic-pci.h b/include/hw/pci-host/generic-pci.h
> new file mode 100644
> index 0000000..830542e
> --- /dev/null
> +++ b/include/hw/pci-host/generic-pci.h
> @@ -0,0 +1,45 @@
> +#ifndef QEMU_GENERIC_PCI_H
> +#define QEMU_GENERIC_PCI_H
> +
> +#include "hw/pci/pci.h"
> +#include "hw/pci/pci_bus.h"
> +#include "hw/pci/pci_host.h"
> +
> +#define MAX_PCI_DEVICES (PCI_SLOT_MAX * PCI_FUNC_MAX)
> +
> +typedef struct {
> +    /*< private >*/
> +    PCIDevice parent_obj;
> +} GenericPCIHostState;
> +
> +typedef struct PCIGenState {
> +    /*< private >*/
> +    PCIHostState parent_obj;
> +
> +    qemu_irq irq[MAX_PCI_DEVICES];
> +    MemoryRegion mem_config;
> +    /* Container representing the PCI address MMIO space */
> +    MemoryRegion pci_mem_space;
> +    /* Alias region into PCI address spaces which we expose as sysbus region */
> +    MemoryRegion pci_mem_window;
> +    /* PCI I/O region */
> +    MemoryRegion pci_io_window;
> +    PCIBus pci_bus;
> +    GenericPCIHostState pci_gen;
> +
> +    uint32_t cfg_win_size;
> +    uint32_t pio_win_size;
> +    uint64_t mmio_win_addr; // offset of pci_mem_window inside pci_mem_space
> +    uint64_t mmio_win_size;
> +    uint32_t irqs;
> +} PCIGenState;
> +
> +#define TYPE_GENERIC_PCI "generic_pci"
> +#define PCI_GEN(obj) \
> +    OBJECT_CHECK(PCIGenState, (obj), TYPE_GENERIC_PCI)
> +
> +#define TYPE_GENERIC_PCI_HOST "generic_pci_host"
> +#define PCI_GEN_HOST(obj) \
> +    OBJECT_CHECK(GenericPCIHostState, (obj), TYPE_GENERIC_PCI_HOST)
> +
> +#endif
> \ No newline at end of file
> 


-- 
Claudio Fontana
Server Virtualization Architect
Huawei Technologies Duesseldorf GmbH
Riesstraße 25 - 80992 München

office: +49 89 158834 4135
mobile: +49 15253060158

Re: [Qemu-devel] [RFC v3 2/2] hw/arm/virt: add generic-pci PCI host controller

[alvise rigo]

Hi Claudio,

On Wed, Jan 14, 2015 at 2:10 PM, Claudio Fontana
<claudio.fontana@huawei.com> wrote:
> On 14.01.2015 11:16, Alvise Rigo wrote:
>> The platform memory map has now three more memory ranges to map the
>> device's memory regions (Configuration region, I/O region and Memory
>> region).
>>
>> The dt node interrupt-map property tells how to route the PCI interrupts
>> to system interrupts. In the mach-virt case, four IRQs are swizzled
>> between all the possible interrupts coming from the PCI bus. In
>> particular, the mapping ensures that the first four devices will use a
>> system IRQ always different (supposing that only PIN_A of each device is
>> used).
>>
>> Now that a PCI bus is provided, the machine can be launched with
>> multiple PCI devices through the -device option (e.g., -device
>> virtio-blk-pci).
>>
>> Signed-off-by: Alvise Rigo <a.rigo@virtualopensystems.com>
>> ---
>>  hw/arm/virt.c | 112 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 111 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index 2353440..7b0326f 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -44,6 +44,7 @@
>>  #include "qemu/error-report.h"
>>
>>  #define NUM_VIRTIO_TRANSPORTS 32
>> +#define NUM_PCI_IRQS 4
>>
>>  /* Number of external interrupt lines to configure the GIC with */
>>  #define NUM_IRQS 128
>> @@ -68,8 +69,12 @@ enum {
>>      VIRT_UART,
>>      VIRT_MMIO,
>>      VIRT_RTC,
>> +    VIRT_PCI_CFG,
>> +    VIRT_PCI_IO,
>> +    VIRT_PCI_MEM,
>>      VIRT_FW_CFG,
>>  };
>> +#define VIRT_PCI VIRT_PCI_CFG
>>
>>  typedef struct MemMapEntry {
>>      hwaddr base;
>> @@ -129,13 +134,17 @@ 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 */
>> +    /* PCI regions */
>> +    [VIRT_PCI_CFG] =    { 0x10000000, 0x01000000 },
>> +    [VIRT_PCI_IO] =     { 0x11000000, 0x00010000 },
>> +    [VIRT_PCI_MEM] =    { 0x12000000, 0x2e000000 },
>>      [VIRT_MEM] =        { 0x40000000, 30ULL * 1024 * 1024 * 1024 },
>>  };
>>
>>  static const int a15irqmap[] = {
>>      [VIRT_UART] = 1,
>>      [VIRT_RTC] = 2,
>> +    [VIRT_PCI] = 3, /* ...to 3 + NUM_PCI_IRQS - 1 */
>>      [VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */
>>  };
>>
>> @@ -436,6 +445,105 @@ static void create_rtc(const VirtBoardInfo *vbi, qemu_irq *pic)
>>      g_free(nodename);
>>  }
>>
>> +static void create_pci_host(const VirtBoardInfo *vbi, qemu_irq *pic)
>> +{
>> +    DeviceState *dev;
>> +    SysBusDevice *busdev;
>> +    int i, j, intmap_rows;
>> +    const MemMapEntry *mme = vbi->memmap;
>> +    uint32_t plat_acells;
>> +    uint32_t plat_scells;
>> +    uint32_t gic_phandle;
>> +    char *nodename;
>> +
>> +    dev = qdev_create(NULL, "generic_pci");
>> +    qdev_prop_set_uint64(dev, "mmio_win_size", mme[VIRT_PCI_MEM].size);
>> +    qdev_prop_set_uint64(dev, "mmio_win_addr", mme[VIRT_PCI_MEM].base);
>> +    qdev_prop_set_uint32(dev, "irqs", NUM_PCI_IRQS);
>> +    qdev_init_nofail(dev);
>> +
>> +    busdev = SYS_BUS_DEVICE(dev);
>> +    sysbus_mmio_map(busdev, 0, mme[VIRT_PCI_CFG].base); /* PCI config */
>> +    sysbus_mmio_map(busdev, 1, mme[VIRT_PCI_IO].base);  /* PCI I/O */
>> +    sysbus_mmio_map(busdev, 2, mme[VIRT_PCI_MEM].base); /* PCI memory window */
>> +
>> +    for ( i = 0; i < NUM_PCI_IRQS; i++) {
>> +        sysbus_connect_irq(busdev, i, pic[vbi->irqmap[VIRT_PCI] + i]);
>> +    }
>> +
>> +    /* add device tree node */
>> +    nodename = g_strdup_printf("/pci@%" PRIx64, mme[VIRT_PCI_CFG].base);
>> +    qemu_fdt_add_subnode(vbi->fdt, nodename);
>> +    qemu_fdt_setprop_string(vbi->fdt, nodename, "compatible",
>> +                                     "pci-host-cam-generic");
>> +    qemu_fdt_setprop_string(vbi->fdt, nodename, "device_type", "pci");
>> +    qemu_fdt_setprop_cell(vbi->fdt, nodename, "#address-cells", 0x3);
>> +    qemu_fdt_setprop_cell(vbi->fdt, nodename, "#size-cells", 0x2);
>> +    qemu_fdt_setprop_cell(vbi->fdt, nodename, "#interrupt-cells", 0x1);
>> +
>> +    plat_acells = qemu_fdt_getprop_cell(vbi->fdt, "/", "#address-cells");
>> +    plat_scells = qemu_fdt_getprop_cell(vbi->fdt, "/", "#size-cells");
>> +    qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "reg",
>> +                                    plat_acells, mme[VIRT_PCI_CFG].base,
>> +                                    plat_scells, mme[VIRT_PCI_CFG].size);
>> +
>> +    /* Two regions (second and third of *busdev):
>> +       - I/O region, PCI addr = 0x0, CPU addr = mme[VIRT_PCI_IO].base,
>> +         size = mme[VIRT_PCI_IO].size,
>> +       - 32bit MMIO region, PCI addr = CPU addr = mme[VIRT_PCI_MEM].base,
>> +         size = mme[VIRT_PCI_MEM].size */
>> +    qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "ranges",
>> +            1, 0x01000000, 2, 0x00000000, /* I/O space */
>> +            2, mme[VIRT_PCI_IO].base, 2, mme[VIRT_PCI_IO].size,
>> +            1, 0x02000000, 2, mme[VIRT_PCI_MEM].base, /* 32bit memory space */
>> +            2, mme[VIRT_PCI_MEM].base, 2, mme[VIRT_PCI_MEM].size);
>> +
>> +    gic_phandle = qemu_fdt_get_phandle(vbi->fdt, "/intc");
>> +
>> +#define IRQ_MAP_ENTRY_DESC_SZ 14
>> +#define PHYSH_DEV_SHIFT 11
>> +    /* Any interrupt from the PCI bus is represented by four 32bit values <physH
>> +       physM physL pin#> (unit-interrupt-specifier), where bits physH[11:15]
>> +       identify the device number and pin# the pin number that triggered. We use
>> +       the two bits physH[14:15] and pin# to route all the PCI interrupts to
>> +       four different system interrupts. The following mask is used to honour
>> +       exactly those bits. */
>> +    qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "interrupt-map-mask",
>> +                    1, 0x3 << PHYSH_DEV_SHIFT, 1, 0x0, 1, 0x0, 1, 0x7);
>> +
>> +    /* interrupt-map property
>> +       Two bits of physH[14:15] and four possible values of pin# give a maximum
>> +       of 16 different results of the mask operation:
>> +       <physH physM physL pin#> AND <interrupt-map-mask>
>> +       Cycling through the four system interrupts available, we assign to each
>> +       result an interrupt number (basically, 32 * 4 maximum PCI interrupts are
>> +       mapped to 4 system interrupts).
>> +       Each entry inside interrupt-map has the following form:
>> +       <PCI unit-interrupt-specifier, system unit-interrupt-specifier> */
>> +    intmap_rows = 16;
>> +    uint64_t *int_mapping_data = g_malloc0(IRQ_MAP_ENTRY_DESC_SZ *
>> +                                  sizeof(uint64_t) * intmap_rows);
>> +    uint64_t *int_map_data = int_mapping_data;
>> +    for (i = 0; i < 4; i++) {
>> +        for (j = 0; j < 4; j++) {
>> +           uint64_t physh_dev_nr = i << PHYSH_DEV_SHIFT;
>> +           uint64_t pci_pin_nr = j + 1;
>> +           uint64_t row[IRQ_MAP_ENTRY_DESC_SZ] =
>> +           {1, physh_dev_nr, 2, 0x0, 1, pci_pin_nr, // masked PCI interrupt...
>> +            1, gic_phandle, 1, GIC_FDT_IRQ_TYPE_SPI, // ...mapped to this IRQ
>> +            1, (vbi->irqmap[VIRT_PCI_CFG] + (j + i) % 4),
>> +            1, GIC_FDT_IRQ_FLAGS_LEVEL_HI};
>> +           int_map_data = mempcpy(int_map_data, row, sizeof(row));
>> +        }
>> +    }
>> +
>
> sorry for maybe being pedantic, but is this working as intended?

Yes, four interrupts are used for all the PCI IRQs.

> The same PHYS_HI is mapped multiple times to a different irq if I understand this correctly, ending up in these mappings:
>
> B,D,F irqmap-mask   0x00001800
> PHYS_HI > SPI irq mappings
>
> 0x00000000 -> SPI irq 3
> 0x00000000 -> SPI irq 4
> 0x00000000 -> SPI irq 5
> 0x00000000 -> SPI irq 6
>
> 0x00000800 -> SPI irq 3
> 0x00000800 -> SPI irq 4
> 0x00000800 -> SPI irq 5
> 0x00000800 -> SPI irq 6
>
> 0x00001000 -> SPI irq 3
> 0x00001000 -> SPI irq 4
> 0x00001000 -> SPI irq 5
> 0x00001000 -> SPI irq 6
>
> 0x00001800 -> SPI irq 3
> 0x00001800 -> SPI irq 4
> 0x00001800 -> SPI irq 5
> 0x00001800 -> SPI irq 6

The interrupt-map generated is the following:

0x0    0x0 0x0 0x1 0x8001 0x0 0x3 0x4
0x0    0x0 0x0 0x2 0x8001 0x0 0x4 0x4
0x0    0x0 0x0 0x3 0x8001 0x0 0x5 0x4
0x0    0x0 0x0 0x4 0x8001 0x0 0x6 0x4
0x800  0x0 0x0 0x1 0x8001 0x0 0x4 0x4
0x800  0x0 0x0 0x2 0x8001 0x0 0x5 0x4
0x800  0x0 0x0 0x3 0x8001 0x0 0x6 0x4
0x800  0x0 0x0 0x4 0x8001 0x0 0x3 0x4
0x1000 0x0 0x0 0x1 0x8001 0x0 0x5 0x4
0x1000 0x0 0x0 0x2 0x8001 0x0 0x6 0x4
0x1000 0x0 0x0 0x3 0x8001 0x0 0x3 0x4
0x1000 0x0 0x0 0x4 0x8001 0x0 0x4 0x4
0x1800 0x0 0x0 0x1 0x8001 0x0 0x6 0x4
0x1800 0x0 0x0 0x2 0x8001 0x0 0x3 0x4
0x1800 0x0 0x0 0x3 0x8001 0x0 0x4 0x4
0x1800 0x0 0x0 0x4 0x8001 0x0 0x5 0x4

Now, removing the INT_{B,C,D} information we have:
0x0       0x0 0x0 0x1 0x8001 0x0 0x3 0x4
0x800   0x0 0x0 0x1 0x8001 0x0 0x4 0x4
0x1000 0x0 0x0 0x1 0x8001 0x0 0x5 0x4
0x1800 0x0 0x0 0x1 0x8001 0x0 0x6 0x4

Which means that, when the two lower bits of the device number are
"00" (and so the mask result is 0x0 0x0 0x0 0x1, as the first entry of
the interrupt-map), the assigned IRQ will be 3. If the two lower bit
are "01" (mask result 0x800 0x0 0x0 0x1), the assigned IRQ number will
be 4, and so on.

Regards,
alvise

>
> did you not intend to do
>
> B,D,F irqmap-mask   0x00001800
> PHYS_HI > SPI irq mappings
>
> 0x00000000 -> SPI irq 3
> 0x00000800 -> SPI irq 4
> 0x00001000 -> SPI irq 5
> 0x00001800 -> SPI irq 6
>
> ?
>
> Thanks,
>
> Claudio
>
>
>> +    qemu_fdt_setprop_sized_cells_from_array(vbi->fdt, nodename, "interrupt-map",
>> +                     (intmap_rows * IRQ_MAP_ENTRY_DESC_SZ)/2, int_mapping_data);
>> +
>> +    g_free(int_mapping_data);
>> +    g_free(nodename);
>> +}
>> +
>>  static void create_virtio_devices(const VirtBoardInfo *vbi, qemu_irq *pic)
>>  {
>>      int i;
>> @@ -640,6 +748,8 @@ static void machvirt_init(MachineState *machine)
>>
>>      create_rtc(vbi, pic);
>>
>> +    create_pci_host(vbi, pic);
>> +
>>      /* 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.
>>
>
>
> --
> Claudio Fontana
> Server Virtualization Architect
> Huawei Technologies Duesseldorf GmbH
> Riesstraße 25 - 80992 München
>
> office: +49 89 158834 4135
> mobile: +49 15253060158

Re: [Qemu-devel] [RFC v3 1/2] pci/pci-host: Add generic-pci PCI host controller device

[alvise rigo]

Hi Claudio,

Sorry, I should have missed this one.

On Wed, Jan 14, 2015 at 2:12 PM, Claudio Fontana
<claudio.fontana@huawei.com> wrote:
> On 14.01.2015 11:16, Alvise Rigo wrote:
>> Add a generic PCI host controller for virtual platforms, based on the
>> previous work by Rob Herring:
>> http://lists.gnu.org/archive/html/qemu-devel/2014-06/msg03482.html
>>
>> The controller relies on a configuration memory region and provides two
>> PCI memory regions for I/O (one port and one memory mapped).  The device
>> needs the following qdev properties to configure the memory regions:
>> - cfg_win_size: size of the configuration memory
>> - pio_win_size: size of the port I/O space
>> - mmio_win_size: size of the MMIO space
>> - mmio_win_addr: offset of MMIO space in the system memory
>>
>> Signed-off-by: Alvise Rigo <a.rigo@virtualopensystems.com>
>> ---
>>  hw/pci-host/Makefile.objs         |   2 +-
>>  hw/pci-host/generic-pci.c         | 140 ++++++++++++++++++++++++++++++++++++++
>>  include/hw/pci-host/generic-pci.h |  45 ++++++++++++
>>  3 files changed, 186 insertions(+), 1 deletion(-)
>>  create mode 100644 hw/pci-host/generic-pci.c
>>  create mode 100644 include/hw/pci-host/generic-pci.h
>>
>> diff --git a/hw/pci-host/Makefile.objs b/hw/pci-host/Makefile.objs
>> index bb65f9c..8ef9fac 100644
>> --- a/hw/pci-host/Makefile.objs
>> +++ b/hw/pci-host/Makefile.objs
>> @@ -1,4 +1,4 @@
>> -common-obj-y += pam.o
>> +common-obj-y += pam.o generic-pci.o
>>
>>  # PPC devices
>>  common-obj-$(CONFIG_PREP_PCI) += prep.o
>> diff --git a/hw/pci-host/generic-pci.c b/hw/pci-host/generic-pci.c
>> new file mode 100644
>> index 0000000..54c9647
>> --- /dev/null
>> +++ b/hw/pci-host/generic-pci.c
>> @@ -0,0 +1,140 @@
>> +/*
>> + * Generic PCI host controller
>> + *
>> + * Copyright (c) 2014 Linaro, Ltd.
>> + * Author: Rob Herring <rob.herring@linaro.org>
>> + *
>> + * Based on ARM Versatile PCI controller (hw/pci-host/versatile.c):
>> + * Copyright (c) 2006-2009 CodeSourcery.
>> + * Written by Paul Brook
>> + *
>> + * This code is licensed under the LGPL.
>> + */
>> +
>> +#include "hw/sysbus.h"
>> +#include "hw/pci-host/generic-pci.h"
>> +#include "exec/address-spaces.h"
>> +#include "sysemu/device_tree.h"
>> +
>> +static const VMStateDescription pci_generic_host_vmstate = {
>> +    .name = "generic-host-pci",
>> +    .version_id = 1,
>> +    .minimum_version_id = 1,
>> +};
>> +
>> +static void pci_cam_config_write(void *opaque, hwaddr addr,
>> +                                 uint64_t val, unsigned size)
>> +{
>> +    PCIGenState *s = opaque;
>> +    pci_data_write(&s->pci_bus, addr, val, size);
>> +}
>> +
>> +static uint64_t pci_cam_config_read(void *opaque, hwaddr addr, unsigned size)
>> +{
>> +    PCIGenState *s = opaque;
>> +    uint32_t val;
>> +    val = pci_data_read(&s->pci_bus, addr, size);
>> +    return val;
>> +}
>> +
>> +static const MemoryRegionOps pci_generic_config_ops = {
>> +    .read = pci_cam_config_read,
>> +    .write = pci_cam_config_write,
>> +    .endianness = DEVICE_LITTLE_ENDIAN,
>> +};
>> +
>> +static void pci_generic_set_irq(void *opaque, int irq_num, int level)
>> +{
>> +    qemu_irq *pic = opaque;
>> +    qemu_set_irq(pic[irq_num], level);
>> +}
>> +
>> +static void pci_generic_host_realize(DeviceState *dev, Error **errp)
>> +{
>> +    PCIHostState *h = PCI_HOST_BRIDGE(dev);
>> +    PCIGenState *s = PCI_GEN(dev);
>> +    GenericPCIHostState *gps = &s->pci_gen;
>> +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>> +    int i;
>> +
>> +    memory_region_init(&s->pci_io_window, OBJECT(s), "pci_io", s->pio_win_size);
>> +    memory_region_init(&s->pci_mem_space, OBJECT(s), "pci_mem", 1ULL << 32);
>> +
>> +    pci_bus_new_inplace(&s->pci_bus, sizeof(s->pci_bus), dev, "pci",
>> +                        &s->pci_mem_space, &s->pci_io_window,
>> +                        PCI_DEVFN(0, 0), TYPE_PCI_BUS);
>> +    h->bus = &s->pci_bus;
>> +
>> +    object_initialize(gps, sizeof(*gps), TYPE_GENERIC_PCI_HOST);
>> +    qdev_set_parent_bus(DEVICE(gps), BUS(&s->pci_bus));
>> +
>> +    for (i = 0; i < s->irqs; i++) {
>> +        sysbus_init_irq(sbd, &s->irq[i]);
>> +    }
>> +
>> +    pci_bus_irqs(&s->pci_bus, pci_generic_set_irq, pci_swizzle_map_irq_fn,
>> +                                                         s->irq, s->irqs);
>> +    memory_region_init_io(&s->mem_config, OBJECT(s), &pci_generic_config_ops, s,
>> +                          "pci-config", s->cfg_win_size);
>> +    memory_region_init_alias(&s->pci_mem_window, OBJECT(s), "pci-mem-win",
>> +                    &s->pci_mem_space, s->mmio_win_addr, s->mmio_win_size);
>> +
>> +    sysbus_init_mmio(sbd, &s->mem_config);
>> +    sysbus_init_mmio(sbd, &s->pci_io_window);
>> +    sysbus_init_mmio(sbd, &s->pci_mem_window);
>> +}
>> +
>> +static void pci_generic_host_class_init(ObjectClass *klass, void *data)
>> +{
>> +    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>> +
>> +    k->vendor_id = PCI_VENDOR_ID_REDHAT;
>> +    k->device_id = PCI_DEVICE_ID_REDHAT_BRIDGE;
>> +    k->class_id = PCI_CLASS_PROCESSOR_CO;
>> +    /*
>> +     * 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 pci_generic_host_info = {
>> +    .name          = TYPE_GENERIC_PCI_HOST,
>> +    .parent        = TYPE_PCI_DEVICE,
>> +    .instance_size = sizeof(GenericPCIHostState),
>> +    .class_init    = pci_generic_host_class_init,
>> +};
>> +
>> +static Property pci_generic_props[] = {
>> +    DEFINE_PROP_UINT32("cfg_win_size", PCIGenState, cfg_win_size, 1ULL << 20),
>> +    DEFINE_PROP_UINT32("pio_win_size", PCIGenState, pio_win_size, 64 * 1024),
>> +    DEFINE_PROP_UINT64("mmio_win_size", PCIGenState, mmio_win_size, 1ULL << 32),
>> +    DEFINE_PROP_UINT64("mmio_win_addr", PCIGenState, mmio_win_addr, 0),
>> +    DEFINE_PROP_UINT32("irqs", PCIGenState, irqs, 4),
>> +    DEFINE_PROP_END_OF_LIST(),
>> +};
>> +
>> +static void pci_generic_class_init(ObjectClass *klass, void *data)
>> +{
>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>> +
>> +    dc->realize = pci_generic_host_realize;
>> +    dc->vmsd = &pci_generic_host_vmstate;
>> +    dc->props = pci_generic_props;
>
> why do some of these assigments have the & and some not?

pci_generic_host_vmstate is a VMStateDescription structure defined and
initialized at the top of the source file while vmsd is a pointer to
such a structure.

Regards,
alvise

>
>> +}
>> +
>> +static const TypeInfo pci_generic_info = {
>> +    .name          = TYPE_GENERIC_PCI,
>> +    .parent        = TYPE_PCI_HOST_BRIDGE,
>> +    .instance_size = sizeof(PCIGenState),
>> +    .class_init    = pci_generic_class_init,
>> +};
>> +
>> +static void generic_pci_host_register_types(void)
>> +{
>> +    type_register_static(&pci_generic_info);
>> +    type_register_static(&pci_generic_host_info);
>> +}
>> +
>> +type_init(generic_pci_host_register_types)
>> \ No newline at end of file
>> diff --git a/include/hw/pci-host/generic-pci.h b/include/hw/pci-host/generic-pci.h
>> new file mode 100644
>> index 0000000..830542e
>> --- /dev/null
>> +++ b/include/hw/pci-host/generic-pci.h
>> @@ -0,0 +1,45 @@
>> +#ifndef QEMU_GENERIC_PCI_H
>> +#define QEMU_GENERIC_PCI_H
>> +
>> +#include "hw/pci/pci.h"
>> +#include "hw/pci/pci_bus.h"
>> +#include "hw/pci/pci_host.h"
>> +
>> +#define MAX_PCI_DEVICES (PCI_SLOT_MAX * PCI_FUNC_MAX)
>> +
>> +typedef struct {
>> +    /*< private >*/
>> +    PCIDevice parent_obj;
>> +} GenericPCIHostState;
>> +
>> +typedef struct PCIGenState {
>> +    /*< private >*/
>> +    PCIHostState parent_obj;
>> +
>> +    qemu_irq irq[MAX_PCI_DEVICES];
>> +    MemoryRegion mem_config;
>> +    /* Container representing the PCI address MMIO space */
>> +    MemoryRegion pci_mem_space;
>> +    /* Alias region into PCI address spaces which we expose as sysbus region */
>> +    MemoryRegion pci_mem_window;
>> +    /* PCI I/O region */
>> +    MemoryRegion pci_io_window;
>> +    PCIBus pci_bus;
>> +    GenericPCIHostState pci_gen;
>> +
>> +    uint32_t cfg_win_size;
>> +    uint32_t pio_win_size;
>> +    uint64_t mmio_win_addr; // offset of pci_mem_window inside pci_mem_space
>> +    uint64_t mmio_win_size;
>> +    uint32_t irqs;
>> +} PCIGenState;
>> +
>> +#define TYPE_GENERIC_PCI "generic_pci"
>> +#define PCI_GEN(obj) \
>> +    OBJECT_CHECK(PCIGenState, (obj), TYPE_GENERIC_PCI)
>> +
>> +#define TYPE_GENERIC_PCI_HOST "generic_pci_host"
>> +#define PCI_GEN_HOST(obj) \
>> +    OBJECT_CHECK(GenericPCIHostState, (obj), TYPE_GENERIC_PCI_HOST)
>> +
>> +#endif
>> \ No newline at end of file
>>
>
>
> --
> Claudio Fontana
> Server Virtualization Architect
> Huawei Technologies Duesseldorf GmbH
> Riesstraße 25 - 80992 München
>
> office: +49 89 158834 4135
> mobile: +49 15253060158

Re: [Qemu-devel] [RFC v3 1/2] pci/pci-host: Add generic-pci PCI host controller device

[Claudio Fontana]

On 15.01.2015 09:19, alvise rigo wrote:
> Hi Claudio,
> 
> Sorry, I should have missed this one.
> 
> On Wed, Jan 14, 2015 at 2:12 PM, Claudio Fontana
> <claudio.fontana@huawei.com> wrote:
>> On 14.01.2015 11:16, Alvise Rigo wrote:
>>> Add a generic PCI host controller for virtual platforms, based on the
>>> previous work by Rob Herring:
>>> http://lists.gnu.org/archive/html/qemu-devel/2014-06/msg03482.html
>>>
>>> The controller relies on a configuration memory region and provides two
>>> PCI memory regions for I/O (one port and one memory mapped).  The device
>>> needs the following qdev properties to configure the memory regions:
>>> - cfg_win_size: size of the configuration memory
>>> - pio_win_size: size of the port I/O space
>>> - mmio_win_size: size of the MMIO space
>>> - mmio_win_addr: offset of MMIO space in the system memory
>>>
>>> Signed-off-by: Alvise Rigo <a.rigo@virtualopensystems.com>
>>> ---
>>>  hw/pci-host/Makefile.objs         |   2 +-
>>>  hw/pci-host/generic-pci.c         | 140 ++++++++++++++++++++++++++++++++++++++
>>>  include/hw/pci-host/generic-pci.h |  45 ++++++++++++
>>>  3 files changed, 186 insertions(+), 1 deletion(-)
>>>  create mode 100644 hw/pci-host/generic-pci.c
>>>  create mode 100644 include/hw/pci-host/generic-pci.h
>>>
>>> diff --git a/hw/pci-host/Makefile.objs b/hw/pci-host/Makefile.objs
>>> index bb65f9c..8ef9fac 100644
>>> --- a/hw/pci-host/Makefile.objs
>>> +++ b/hw/pci-host/Makefile.objs
>>> @@ -1,4 +1,4 @@
>>> -common-obj-y += pam.o
>>> +common-obj-y += pam.o generic-pci.o
>>>
>>>  # PPC devices
>>>  common-obj-$(CONFIG_PREP_PCI) += prep.o
>>> diff --git a/hw/pci-host/generic-pci.c b/hw/pci-host/generic-pci.c
>>> new file mode 100644
>>> index 0000000..54c9647
>>> --- /dev/null
>>> +++ b/hw/pci-host/generic-pci.c
>>> @@ -0,0 +1,140 @@
>>> +/*
>>> + * Generic PCI host controller
>>> + *
>>> + * Copyright (c) 2014 Linaro, Ltd.
>>> + * Author: Rob Herring <rob.herring@linaro.org>
>>> + *
>>> + * Based on ARM Versatile PCI controller (hw/pci-host/versatile.c):
>>> + * Copyright (c) 2006-2009 CodeSourcery.
>>> + * Written by Paul Brook
>>> + *
>>> + * This code is licensed under the LGPL.
>>> + */
>>> +
>>> +#include "hw/sysbus.h"
>>> +#include "hw/pci-host/generic-pci.h"
>>> +#include "exec/address-spaces.h"
>>> +#include "sysemu/device_tree.h"
>>> +
>>> +static const VMStateDescription pci_generic_host_vmstate = {
>>> +    .name = "generic-host-pci",
>>> +    .version_id = 1,
>>> +    .minimum_version_id = 1,
>>> +};
>>> +
>>> +static void pci_cam_config_write(void *opaque, hwaddr addr,
>>> +                                 uint64_t val, unsigned size)
>>> +{
>>> +    PCIGenState *s = opaque;
>>> +    pci_data_write(&s->pci_bus, addr, val, size);
>>> +}
>>> +
>>> +static uint64_t pci_cam_config_read(void *opaque, hwaddr addr, unsigned size)
>>> +{
>>> +    PCIGenState *s = opaque;
>>> +    uint32_t val;
>>> +    val = pci_data_read(&s->pci_bus, addr, size);
>>> +    return val;
>>> +}
>>> +
>>> +static const MemoryRegionOps pci_generic_config_ops = {
>>> +    .read = pci_cam_config_read,
>>> +    .write = pci_cam_config_write,
>>> +    .endianness = DEVICE_LITTLE_ENDIAN,
>>> +};
>>> +
>>> +static void pci_generic_set_irq(void *opaque, int irq_num, int level)
>>> +{
>>> +    qemu_irq *pic = opaque;
>>> +    qemu_set_irq(pic[irq_num], level);
>>> +}
>>> +
>>> +static void pci_generic_host_realize(DeviceState *dev, Error **errp)
>>> +{
>>> +    PCIHostState *h = PCI_HOST_BRIDGE(dev);
>>> +    PCIGenState *s = PCI_GEN(dev);
>>> +    GenericPCIHostState *gps = &s->pci_gen;
>>> +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>>> +    int i;
>>> +
>>> +    memory_region_init(&s->pci_io_window, OBJECT(s), "pci_io", s->pio_win_size);
>>> +    memory_region_init(&s->pci_mem_space, OBJECT(s), "pci_mem", 1ULL << 32);
>>> +
>>> +    pci_bus_new_inplace(&s->pci_bus, sizeof(s->pci_bus), dev, "pci",
>>> +                        &s->pci_mem_space, &s->pci_io_window,
>>> +                        PCI_DEVFN(0, 0), TYPE_PCI_BUS);
>>> +    h->bus = &s->pci_bus;
>>> +
>>> +    object_initialize(gps, sizeof(*gps), TYPE_GENERIC_PCI_HOST);
>>> +    qdev_set_parent_bus(DEVICE(gps), BUS(&s->pci_bus));
>>> +
>>> +    for (i = 0; i < s->irqs; i++) {
>>> +        sysbus_init_irq(sbd, &s->irq[i]);
>>> +    }
>>> +
>>> +    pci_bus_irqs(&s->pci_bus, pci_generic_set_irq, pci_swizzle_map_irq_fn,
>>> +                                                         s->irq, s->irqs);
>>> +    memory_region_init_io(&s->mem_config, OBJECT(s), &pci_generic_config_ops, s,
>>> +                          "pci-config", s->cfg_win_size);
>>> +    memory_region_init_alias(&s->pci_mem_window, OBJECT(s), "pci-mem-win",
>>> +                    &s->pci_mem_space, s->mmio_win_addr, s->mmio_win_size);
>>> +
>>> +    sysbus_init_mmio(sbd, &s->mem_config);
>>> +    sysbus_init_mmio(sbd, &s->pci_io_window);
>>> +    sysbus_init_mmio(sbd, &s->pci_mem_window);
>>> +}
>>> +
>>> +static void pci_generic_host_class_init(ObjectClass *klass, void *data)
>>> +{
>>> +    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>>> +
>>> +    k->vendor_id = PCI_VENDOR_ID_REDHAT;
>>> +    k->device_id = PCI_DEVICE_ID_REDHAT_BRIDGE;
>>> +    k->class_id = PCI_CLASS_PROCESSOR_CO;
>>> +    /*
>>> +     * 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 pci_generic_host_info = {
>>> +    .name          = TYPE_GENERIC_PCI_HOST,
>>> +    .parent        = TYPE_PCI_DEVICE,
>>> +    .instance_size = sizeof(GenericPCIHostState),
>>> +    .class_init    = pci_generic_host_class_init,
>>> +};
>>> +
>>> +static Property pci_generic_props[] = {
>>> +    DEFINE_PROP_UINT32("cfg_win_size", PCIGenState, cfg_win_size, 1ULL << 20),
>>> +    DEFINE_PROP_UINT32("pio_win_size", PCIGenState, pio_win_size, 64 * 1024),
>>> +    DEFINE_PROP_UINT64("mmio_win_size", PCIGenState, mmio_win_size, 1ULL << 32),
>>> +    DEFINE_PROP_UINT64("mmio_win_addr", PCIGenState, mmio_win_addr, 0),
>>> +    DEFINE_PROP_UINT32("irqs", PCIGenState, irqs, 4),
>>> +    DEFINE_PROP_END_OF_LIST(),
>>> +};
>>> +
>>> +static void pci_generic_class_init(ObjectClass *klass, void *data)
>>> +{
>>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>>> +
>>> +    dc->realize = pci_generic_host_realize;
>>> +    dc->vmsd = &pci_generic_host_vmstate;
>>> +    dc->props = pci_generic_props;
>>
>> why do some of these assigments have the & and some not?
> 
> pci_generic_host_vmstate is a VMStateDescription structure defined and
> initialized at the top of the source file while vmsd is a pointer to
> such a structure.
> 
> Regards,
> alvise
> 

doh.. of course. Thanks, C.

>>
>>> +}
>>> +
>>> +static const TypeInfo pci_generic_info = {
>>> +    .name          = TYPE_GENERIC_PCI,
>>> +    .parent        = TYPE_PCI_HOST_BRIDGE,
>>> +    .instance_size = sizeof(PCIGenState),
>>> +    .class_init    = pci_generic_class_init,
>>> +};
>>> +
>>> +static void generic_pci_host_register_types(void)
>>> +{
>>> +    type_register_static(&pci_generic_info);
>>> +    type_register_static(&pci_generic_host_info);
>>> +}
>>> +
>>> +type_init(generic_pci_host_register_types)
>>> \ No newline at end of file
>>> diff --git a/include/hw/pci-host/generic-pci.h b/include/hw/pci-host/generic-pci.h
>>> new file mode 100644
>>> index 0000000..830542e
>>> --- /dev/null
>>> +++ b/include/hw/pci-host/generic-pci.h
>>> @@ -0,0 +1,45 @@
>>> +#ifndef QEMU_GENERIC_PCI_H
>>> +#define QEMU_GENERIC_PCI_H
>>> +
>>> +#include "hw/pci/pci.h"
>>> +#include "hw/pci/pci_bus.h"
>>> +#include "hw/pci/pci_host.h"
>>> +
>>> +#define MAX_PCI_DEVICES (PCI_SLOT_MAX * PCI_FUNC_MAX)
>>> +
>>> +typedef struct {
>>> +    /*< private >*/
>>> +    PCIDevice parent_obj;
>>> +} GenericPCIHostState;
>>> +
>>> +typedef struct PCIGenState {
>>> +    /*< private >*/
>>> +    PCIHostState parent_obj;
>>> +
>>> +    qemu_irq irq[MAX_PCI_DEVICES];
>>> +    MemoryRegion mem_config;
>>> +    /* Container representing the PCI address MMIO space */
>>> +    MemoryRegion pci_mem_space;
>>> +    /* Alias region into PCI address spaces which we expose as sysbus region */
>>> +    MemoryRegion pci_mem_window;
>>> +    /* PCI I/O region */
>>> +    MemoryRegion pci_io_window;
>>> +    PCIBus pci_bus;
>>> +    GenericPCIHostState pci_gen;
>>> +
>>> +    uint32_t cfg_win_size;
>>> +    uint32_t pio_win_size;
>>> +    uint64_t mmio_win_addr; // offset of pci_mem_window inside pci_mem_space
>>> +    uint64_t mmio_win_size;
>>> +    uint32_t irqs;
>>> +} PCIGenState;
>>> +
>>> +#define TYPE_GENERIC_PCI "generic_pci"
>>> +#define PCI_GEN(obj) \
>>> +    OBJECT_CHECK(PCIGenState, (obj), TYPE_GENERIC_PCI)
>>> +
>>> +#define TYPE_GENERIC_PCI_HOST "generic_pci_host"
>>> +#define PCI_GEN_HOST(obj) \
>>> +    OBJECT_CHECK(GenericPCIHostState, (obj), TYPE_GENERIC_PCI_HOST)
>>> +
>>> +#endif
>>> \ No newline at end of file
>>>
>>
>>
>> --
>> Claudio Fontana
>> Server Virtualization Architect
>> Huawei Technologies Duesseldorf GmbH
>> Riesstraße 25 - 80992 München
>>
>> office: +49 89 158834 4135
>> mobile: +49 15253060158


-- 
Claudio Fontana
Server Virtualization Architect
Huawei Technologies Duesseldorf GmbH
Riesstraße 25 - 80992 München

office: +49 89 158834 4135
mobile: +49 15253060158

Re: [Qemu-devel] [RFC v3 0/2] Add Generic PCI host device update

[Claudio Fontana]

On 14.01.2015 11:16, Alvise Rigo wrote:
> This is just an update to address the comments received on v2.
> 
> This patch series is based on the previous work [1] and [2] by Rob Herring.
> This is a stand alone work, but I will not hesitate to split the patches in a
> more accurate way if needed.
> 
> Changes from [1] and [2]:
> - memory topology definition has been revisited
> - memory addresses and sizes are now defined as qdev properties
> - four system interrupts are swizzled between all the PCI devices and their
>   pins
> - other minor changes
> - removed implicit creation of lsi and pci-ohci devices
> 
> Changes from v2:
> - Some comments from Peter Maydell have been addressed: the device tree node
>   generation is moved back to the mach-virt platform as in the original work.
>   For this reason, mach-virt is the only supported machine model at the moment.
>   Moreover, the device tree node is statically created at the device init,
>   while in the previous versions it was generated according to the PCI devices
>   attached to the bus.
> - The creation of the device memory regions has been postponed in the realize
>   function (thanks to Alexander Graf).
> - Various comments from Claudio Fontana have been addressed to better explain
>   some crucial points of the adopted PCI specifications (see below).
> 
> There are two specifications used by these patches. The first one regards
> mainly how to describe a PCI regions inside the dt node "ranges" property. Look
> at "PCI Bus binding: IEEE Std 1275-1994" for more details.  The second instead
> regards mainly the properties "interrupt-map-mask" and "interrupt-map" that are
> described in "Open Firmware Recommended Practice: Interrupt Mapping". This
> document propose a way to route PCI interrupts to system interrupts.  All these
> specifications have been firstly implemented, in kernel side, by the
> host-generic-pci driver.
> 
> This work has been tested attaching several PCI devices to the mach-virt
> platform using an ARMv7 CPU. The tested devices are: virtio-blk-pci,
> virtio-net-pci, lsi53c895a and pci-ohci (all attached at the same time).
> 
> TODO:
> - Add MSI, MSI-X support
> - PCI-E support. For this, you would probably look at Alexander Graf work [3]
> 
> Thank you, alvise
> 
> [1]
> "[Qemu-devel] [RFC PATCH 1/2] hw/pci-host: add a generic PCI host"
> http://lists.gnu.org/archive/html/qemu-devel/2014-06/msg03482.html
> [2]
> "[Qemu-devel] [RFC PATCH 2/2] hw/arm/virt: Add generic PCI host device"
> http://lists.gnu.org/archive/html/qemu-devel/2014-06/msg03483.html
> [3]
> "[Qemu-devel] [PATCH 0/4] ARM: Add support for a generic PCI Express host bridge"
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg272648.html
> 
> 
> 
> Alvise Rigo (2):
>   pci/pci-host: Add generic-pci PCI host controller device
>   hw/arm/virt: add generic-pci PCI host controller
> 
>  hw/arm/virt.c                     | 112 +++++++++++++++++++++++++++++-
>  hw/pci-host/Makefile.objs         |   2 +-
>  hw/pci-host/generic-pci.c         | 140 ++++++++++++++++++++++++++++++++++++++
>  include/hw/pci-host/generic-pci.h |  45 ++++++++++++
>  4 files changed, 297 insertions(+), 2 deletions(-)
>  create mode 100644 hw/pci-host/generic-pci.c
>  create mode 100644 include/hw/pci-host/generic-pci.h
> 

Tested-by: Claudio Fontana <claudio.fontana@huawei.com>
Reviewed-by: Claudio Fontana <claudio.fontana@huawei.com>

Re: [Qemu-devel] [RFC v3 0/2] Add Generic PCI host device update

[Peter Maydell]

On 14 January 2015 at 10:16, Alvise Rigo <a.rigo@virtualopensystems.com> wrote:
> This is just an update to address the comments received on v2.
>
> This patch series is based on the previous work [1] and [2] by Rob Herring.
> This is a stand alone work, but I will not hesitate to split the patches in a
> more accurate way if needed.

> TODO:
> - Add MSI, MSI-X support
> - PCI-E support. For this, you would probably look at Alexander Graf work [3]

So, my current belief is that Alex's patchset provides everything
this patch does and more (since it gives us PCI-E). Am I missing
something that means we need this patchset as well ?

thanks
-- PMM

Re: [Qemu-devel] [RFC v3 0/2] Add Generic PCI host device update

[alvise rigo]

On Tue, Jan 27, 2015 at 7:48 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 14 January 2015 at 10:16, Alvise Rigo <a.rigo@virtualopensystems.com> wrote:
>> This is just an update to address the comments received on v2.
>>
>> This patch series is based on the previous work [1] and [2] by Rob Herring.
>> This is a stand alone work, but I will not hesitate to split the patches in a
>> more accurate way if needed.
>
>> TODO:
>> - Add MSI, MSI-X support
>> - PCI-E support. For this, you would probably look at Alexander Graf work [3]
>
> So, my current belief is that Alex's patchset provides everything
> this patch does and more (since it gives us PCI-E). Am I missing
> something that means we need this patchset as well ?

No you are not. The only major difference is that this series lacks of
PCI-E support.

Regards,
alvise

>
> thanks
> -- PMM